pkg: deduplicate peer IP addresses

We need to defensively deduplicate peer allowed IPs.
If two peers claim the same IP, the WireGuard configuration
could flap, causing the interface to churn.

cmd/kgctl/showconf.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"net"
@@ -68,7 +69,7 @@ func showConf() *cobra.Command {
 	}
 	cmd.PersistentFlags().BoolVar(&showConfOpts.asPeer, "as-peer", false, "Should the resource be shown as a peer? Useful to configure this resource as a peer of another WireGuard interface.")
 	cmd.PersistentFlags().StringVarP(&showConfOpts.output, "output", "o", "wireguard", fmt.Sprintf("The output format of the resource. Only valid when combined with 'as-peer'. Possible values: %s", availableOutputFormats))
-	cmd.PersistentFlags().StringSliceVar(&allowedIPs, "allowed-ips", []string{}, "Override the allowed IPs of the configuration. Only valid when combined with 'as-peer'.")
+	cmd.PersistentFlags().StringSliceVar(&allowedIPs, "allowed-ips", []string{}, "Add the given IPs to the allowed IPs of the configuration. Only valid when combined with 'as-peer'.")
 
 	return cmd
 }
@@ -151,6 +152,18 @@ func runShowConfNode(_ *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to create topology: %v", err)
 	}
 
+	var found bool
+	for _, p := range t.PeerConf("").Peers {
+		if bytes.Equal(p.PublicKey, nodes[hostname].Key) {
+			found = true
+			break
+		}
+	}
+	if !found {
+		_, err := os.Stderr.WriteString(fmt.Sprintf("Node %q is not a leader node\n", hostname))
+		return err
+	}
+
 	if !showConfOpts.asPeer {
 		c, err := t.Conf().Bytes()
 		if err != nil {
@@ -164,17 +177,16 @@ func runShowConfNode(_ *cobra.Command, args []string) error {
 	case outputFormatJSON:
 		fallthrough
 	case outputFormatYAML:
-		p := translatePeer(t.AsPeer())
+		p := t.AsPeer()
-		p.Name = hostname
+		p.AllowedIPs = append(p.AllowedIPs, showConfOpts.allowedIPs...)
-		if len(showConfOpts.allowedIPs) != 0 {
+		p.DeduplicateIPs()
-			p.Spec.AllowedIPs = allowedIPs
+		k8sp := translatePeer(p)
-		}
+		k8sp.Name = hostname
-		return showConfOpts.serializer.Encode(p, os.Stdout)
+		return showConfOpts.serializer.Encode(k8sp, os.Stdout)
 	case outputFormatWireGuard:
 		p := t.AsPeer()
-		if len(showConfOpts.allowedIPs) != 0 {
+		p.AllowedIPs = append(p.AllowedIPs, showConfOpts.allowedIPs...)
-			p.AllowedIPs = showConfOpts.allowedIPs
+		p.DeduplicateIPs()
-		}
 		c, err := (&wireguard.Conf{
 			Peers: []*wireguard.Peer{p},
 		}).Bytes()
@@ -241,17 +253,16 @@ func runShowConfPeer(_ *cobra.Command, args []string) error {
 	case outputFormatJSON:
 		fallthrough
 	case outputFormatYAML:
-		p := translatePeer(&peers[peer].Peer)
+		p := peers[peer]
-		p.Name = peer
+		p.AllowedIPs = append(p.AllowedIPs, showConfOpts.allowedIPs...)
-		if len(showConfOpts.allowedIPs) != 0 {
+		p.DeduplicateIPs()
-			p.Spec.AllowedIPs = allowedIPs
+		k8sp := translatePeer(&p.Peer)
-		}
+		k8sp.Name = peer
-		return showConfOpts.serializer.Encode(p, os.Stdout)
+		return showConfOpts.serializer.Encode(k8sp, os.Stdout)
 	case outputFormatWireGuard:
 		p := &peers[peer].Peer
-		if len(showConfOpts.allowedIPs) != 0 {
+		p.AllowedIPs = append(p.AllowedIPs, showConfOpts.allowedIPs...)
-			p.AllowedIPs = showConfOpts.allowedIPs
+		p.DeduplicateIPs()
-		}
 		c, err := (&wireguard.Conf{
 			Peers: []*wireguard.Peer{p},
 		}).Bytes()

pkg/mesh/topology.go

@@ -138,7 +138,9 @@ func NewTopology(nodes map[string]*Node, peers map[string]*Peer, granularity Gra
 	sort.Slice(t.peers, func(i, j int) bool {
 		return t.peers[i].Name < t.peers[j].Name
 	})
-
+	// We need to defensively deduplicate peer allowed IPs. If two peers claim the same IP,
+	// the WireGuard configuration could flap, causing the interface to churn.
+	t.peers = deduplicatePeerIPs(t.peers)
 	// Allocate IPs to the segment leaders in a stable, coordination-free manner.
 	a := newAllocator(*subnet)
 	for _, segment := range t.segments {
@@ -417,3 +419,27 @@ func findLeader(nodes []*Node) int {
 	}
 	return 0
 }
+
+func deduplicatePeerIPs(peers []*Peer) []*Peer {
+	ps := make([]*Peer, len(peers))
+	ips := make(map[string]struct{})
+	for i, peer := range peers {
+		p := Peer{
+			Name: peer.Name,
+			Peer: wireguard.Peer{
+				Endpoint:            peer.Endpoint,
+				PersistentKeepalive: peer.PersistentKeepalive,
+				PublicKey:           peer.PublicKey,
+			},
+		}
+		for _, ip := range peer.AllowedIPs {
+			if _, ok := ips[ip.String()]; ok {
+				continue
+			}
+			p.AllowedIPs = append(p.AllowedIPs, ip)
+			ips[ip.String()] = struct{}{}
+		}
+		ps[i] = &p
+	}
+	return ps
+}

pkg/mesh/topology_test.go

@@ -1246,3 +1246,147 @@ func TestFindLeader(t *testing.T) {
 		}
 	}
 }
+
+func TestDeduplicatePeerIPs(t *testing.T) {
+	p1 := &Peer{
+		Name: "1",
+		Peer: wireguard.Peer{
+			PublicKey: []byte("key1"),
+			AllowedIPs: []*net.IPNet{
+				{IP: net.ParseIP("10.0.0.1"), Mask: net.CIDRMask(24, 32)},
+				{IP: net.ParseIP("10.0.0.2"), Mask: net.CIDRMask(24, 32)},
+			},
+		},
+	}
+	p2 := &Peer{
+		Name: "2",
+		Peer: wireguard.Peer{
+			PublicKey: []byte("key2"),
+			AllowedIPs: []*net.IPNet{
+				{IP: net.ParseIP("10.0.0.1"), Mask: net.CIDRMask(24, 32)},
+				{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+			},
+		},
+	}
+	p3 := &Peer{
+		Name: "3",
+		Peer: wireguard.Peer{
+			PublicKey: []byte("key3"),
+			AllowedIPs: []*net.IPNet{
+				{IP: net.ParseIP("10.0.0.2"), Mask: net.CIDRMask(24, 32)},
+				{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+				{IP: net.ParseIP("10.0.0.1"), Mask: net.CIDRMask(24, 32)},
+			},
+		},
+	}
+
+	p4 := &Peer{
+		Name: "4",
+		Peer: wireguard.Peer{
+			PublicKey: []byte("key4"),
+			AllowedIPs: []*net.IPNet{
+				{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+				{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+			},
+		},
+	}
+
+	for _, tc := range []struct {
+		name  string
+		peers []*Peer
+		out   []*Peer
+	}{
+		{
+			name:  "nil",
+			peers: nil,
+			out:   nil,
+		},
+		{
+			name:  "simple dupe",
+			peers: []*Peer{p1, p2},
+			out: []*Peer{
+				p1,
+				{
+					Name: "2",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key2"),
+						AllowedIPs: []*net.IPNet{
+							{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:  "simple dupe reversed",
+			peers: []*Peer{p2, p1},
+			out: []*Peer{
+				p2,
+				{
+					Name: "1",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key1"),
+						AllowedIPs: []*net.IPNet{
+							{IP: net.ParseIP("10.0.0.2"), Mask: net.CIDRMask(24, 32)},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:  "one duplicates all",
+			peers: []*Peer{p3, p2, p1, p4},
+			out: []*Peer{
+				p3,
+				{
+					Name: "2",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key2"),
+					},
+				},
+				{
+					Name: "1",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key1"),
+					},
+				},
+				{
+					Name: "4",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key4"),
+					},
+				},
+			},
+		},
+		{
+			name:  "one duplicates itself",
+			peers: []*Peer{p4, p1},
+			out: []*Peer{
+				{
+					Name: "4",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key4"),
+						AllowedIPs: []*net.IPNet{
+							{IP: net.ParseIP("10.0.0.3"), Mask: net.CIDRMask(24, 32)},
+						},
+					},
+				},
+				{
+					Name: "1",
+					Peer: wireguard.Peer{
+						PublicKey: []byte("key1"),
+						AllowedIPs: []*net.IPNet{
+							{IP: net.ParseIP("10.0.0.1"), Mask: net.CIDRMask(24, 32)},
+							{IP: net.ParseIP("10.0.0.2"), Mask: net.CIDRMask(24, 32)},
+						},
+					},
+				},
+			},
+		},
+	} {
+		out := deduplicatePeerIPs(tc.peers)
+		if diff := pretty.Compare(out, tc.out); diff != "" {
+			t.Errorf("test case %q: got diff: %v", tc.name, diff)
+		}
+	}
+}

pkg/wireguard/conf.go

@@ -59,6 +59,20 @@ type Peer struct {
 	PublicKey           []byte
 }
 
+// DeduplicateIPs eliminates duplicate allowed IPs.
+func (p *Peer) DeduplicateIPs() {
+	var ips []*net.IPNet
+	seen := make(map[string]struct{})
+	for _, ip := range p.AllowedIPs {
+		if _, ok := seen[ip.String()]; ok {
+			continue
+		}
+		ips = append(ips, ip)
+		seen[ip.String()] = struct{}{}
+	}
+	p.AllowedIPs = ips
+}
+
 // Endpoint represents an `endpoint` key of a `peer` section.
 type Endpoint struct {
 	IP   net.IP