fix: multi-DNS iptables rules + don't gate route-less peers

internal/iptables/iptables.go

@@ -87,10 +87,23 @@ func GenerateIptableRulesFromNetworkPolicies(policies v1alpha1.EgressNetworkPoli
 
 		// allow peer to communicate with itself
 		fmt.Sprintf("-A %s -d %s -j ACCEPT", peerChain, peerIp),
+	}
 
-		// allow peer to communicate with kube-dns (UDP and TCP for large DNS responses)
-		fmt.Sprintf("-A %s -d %s -p UDP --dport 53 -j ACCEPT", peerChain, kubeDnsIp),
-		fmt.Sprintf("-A %s -d %s -p TCP --dport 53 -j ACCEPT", peerChain, kubeDnsIp),
+	// allow peer to communicate with kube-dns (UDP and TCP for large DNS responses).
+	// kubeDnsIp may carry more than one address (e.g. an anycast resolver pair) as a
+	// comma-separated list. iptables-restore takes a single address per -d and
+	// tokenises on whitespace, so interpolating the list raw produces
+	// `-d 1.2.3.4, 5.6.7.8` and the whole restore aborts with "Bad argument" —
+	// leaving the ruleset half-applied and forwarding broken. Emit one rule per address.
+	for _, dns := range strings.Split(kubeDnsIp, ",") {
+		dns = strings.TrimSpace(dns)
+		if dns == "" {
+			continue
+		}
+		rules = append(rules,
+			fmt.Sprintf("-A %s -d %s -p UDP --dport 53 -j ACCEPT", peerChain, dns),
+			fmt.Sprintf("-A %s -d %s -p TCP --dport 53 -j ACCEPT", peerChain, dns),
+		)
 	}
 
 	for _, policy := range policies {

internal/iptables/iptables_test.go

@@ -73,6 +73,29 @@ func TestIptableRules(t *testing.T) {
 -A 10-8-0-9 -d 10.8.0.9 -j ACCEPT
 -A 10-8-0-9 -d 100.64.0.10 -p UDP --dport 53 -j ACCEPT
 -A 10-8-0-9 -d 100.64.0.10 -p TCP --dport 53 -j ACCEPT
+# end of rules for peer 10.8.0.9`,
+		},
+		{
+			// Regression: an anycast DNS pair arrives as a comma+space separated
+			// list ("172.31.255.253, 172.31.255.254"). Interpolated raw it yields
+			// `-d 172.31.255.253, 172.31.255.254`, which iptables-restore tokenises
+			// on whitespace and rejects with "Bad argument `172.31.255.254'" —
+			// aborting the whole restore and leaving forwarding rules broken.
+			// Each address must get its own rule.
+			name:            "Multiple DNS servers emit one rule per address",
+			peerIp:          "10.8.0.9",
+			kubeDnsIp:       "172.31.255.253, 172.31.255.254",
+			wgServerIp:      "10.8.0.1",
+			networkPolicies: v1alpha1.EgressNetworkPolicies{},
+			expectedIptableRules: `# start of rules for peer 10.8.0.9
+:10-8-0-9 - [0:0]
+-A FORWARD -s 10.8.0.9 -j 10-8-0-9
+-A 10-8-0-9 -d 10.8.0.1 -p icmp -j ACCEPT
+-A 10-8-0-9 -d 10.8.0.9 -j ACCEPT
+-A 10-8-0-9 -d 172.31.255.253 -p UDP --dport 53 -j ACCEPT
+-A 10-8-0-9 -d 172.31.255.253 -p TCP --dport 53 -j ACCEPT
+-A 10-8-0-9 -d 172.31.255.254 -p UDP --dport 53 -j ACCEPT
+-A 10-8-0-9 -d 172.31.255.254 -p TCP --dport 53 -j ACCEPT
 # end of rules for peer 10.8.0.9`,
 		},
 		{

internal/wireguard/liveness.go

@@ -467,6 +467,7 @@ type peerInfo struct {
 	Keepalive time.Duration
 	Address   string
 	Mode      string // explicit per-peer routeLiveness ("" = inherit)
+	HasRoutes bool   // peer declares downstream routes (v4 or v6)
 }
 
 // SetPeers resolves each peer's effective mode (peer > instance > cluster default)
@@ -475,7 +476,15 @@ type peerInfo struct {
 func (c *LivenessController) SetPeers(instanceMode string, peers []peerInfo) {
 	c.mu.Lock()
 	for _, p := range peers {
-		c.mode[p.PublicKey] = resolveMode(p.Mode, instanceMode, c.clusterDefault)
+		m := resolveMode(p.Mode, instanceMode, c.clusterDefault)
+		// Gating only ever adds or withholds a peer's downstream routes, so a peer
+		// that declares none has nothing to gate. Left gated it would still flip
+		// live/down (road-warrior laptops do this constantly) and every flip calls
+		// apply() → a full state push, for no routing benefit. Force ungated.
+		if !p.HasRoutes {
+			m = ModeDisabled
+		}
+		c.mode[p.PublicKey] = m
 	}
 	c.mu.Unlock()
 	for _, p := range peers {
@@ -503,7 +512,13 @@ func PeerInfos(state agent.State) []peerInfo {
 		if p.Spec.PersistentKeepalive != nil && *p.Spec.PersistentKeepalive > 0 {
 			k = time.Duration(*p.Spec.PersistentKeepalive) * time.Second
 		}
-		out = append(out, peerInfo{PublicKey: p.Spec.PublicKey, Keepalive: k, Address: p.Spec.Address, Mode: p.Spec.RouteLiveness})
+		out = append(out, peerInfo{
+			PublicKey: p.Spec.PublicKey,
+			Keepalive: k,
+			Address:   p.Spec.Address,
+			Mode:      p.Spec.RouteLiveness,
+			HasRoutes: len(p.Spec.Routes) > 0 || len(p.Spec.RoutesV6) > 0,
+		})
 	}
 	return out
 }

internal/wireguard/liveness_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/go-logr/logr"
 
 	"github.com/nccloud/wireguard-operator/api/v1alpha1"
+	"github.com/nccloud/wireguard-operator/internal/agent"
 )
 
 func TestBuildController_AlwaysBuiltAndClusterDefaultUngated(t *testing.T) {
@@ -56,9 +57,11 @@ func TestController_PerPeerModeResolution(t *testing.T) {
 		lastUp:         map[string]bool{},
 	}
 	// instance=active; DC1 explicitly disabled (fallback); DC2 inherits active.
+	// Both are site peers carrying downstream routes — without routes there would
+	// be nothing to gate and both would resolve to disabled.
 	c.SetPeers("active", []peerInfo{
-		{PublicKey: "dc1", Mode: "disabled", Keepalive: 25 * time.Second, Address: "172.31.255.11"},
-		{PublicKey: "dc2", Mode: "", Keepalive: 25 * time.Second, Address: "172.31.255.12"},
+		{PublicKey: "dc1", Mode: "disabled", Keepalive: 25 * time.Second, Address: "172.31.255.11", HasRoutes: true},
+		{PublicKey: "dc2", Mode: "", Keepalive: 25 * time.Second, Address: "172.31.255.12", HasRoutes: true},
 	})
 	if c.modeFor("dc1") != ModeDisabled {
 		t.Errorf("dc1 explicit disabled, got %s", c.modeFor("dc1"))
@@ -71,6 +74,77 @@ func TestController_PerPeerModeResolution(t *testing.T) {
 	}
 }
 
+func TestController_RouteLessPeerIsNeverGated(t *testing.T) {
+	// A peer with no routes has nothing to install or withdraw, so gating it can
+	// never change forwarding — it only produces pointless liveness transitions,
+	// each of which triggers an apply()/state push. Road-warrior peers (laptops
+	// that come and go) are exactly this shape, and their churn was driving
+	// fleet-wide state pushes. Such peers must be ungated regardless of mode.
+	clk := ts(1000)
+	pass := newPassiveLiveness(3, func() time.Time { return clk })
+	c := &LivenessController{
+		passive:        pass,
+		active:         newActiveLiveness(pass, 3, 15*time.Second, func() time.Time { return clk }, proberFunc(func(string) {})),
+		clusterDefault: ModeActive,
+		mode:           map[string]Mode{},
+		lastUp:         map[string]bool{},
+	}
+	c.SetPeers("active", []peerInfo{
+		{PublicKey: "roadwarrior", Mode: "", Keepalive: 25 * time.Second, Address: "172.31.255.8", HasRoutes: false},
+		{PublicKey: "site", Mode: "", Keepalive: 25 * time.Second, Address: "172.31.255.3", HasRoutes: true},
+	})
+	if got := c.modeFor("roadwarrior"); got != ModeDisabled {
+		t.Errorf("route-less peer must be ungated, got mode %q", got)
+	}
+	if !c.IsLive("roadwarrior") {
+		t.Error("route-less peer must always be live (nothing to gate)")
+	}
+	// A peer that does carry routes still inherits the active mode.
+	if got := c.modeFor("site"); got != ModeActive {
+		t.Errorf("peer with routes should inherit active, got %q", got)
+	}
+}
+
+func TestController_RouteLessPeerIgnoresExplicitActive(t *testing.T) {
+	// Even an explicit routeLiveness=active is meaningless without routes.
+	clk := ts(1000)
+	pass := newPassiveLiveness(3, func() time.Time { return clk })
+	c := &LivenessController{
+		passive:        pass,
+		active:         newActiveLiveness(pass, 3, 15*time.Second, func() time.Time { return clk }, proberFunc(func(string) {})),
+		clusterDefault: ModeDisabled,
+		mode:           map[string]Mode{},
+		lastUp:         map[string]bool{},
+	}
+	c.SetPeers("", []peerInfo{
+		{PublicKey: "rw", Mode: "active", Keepalive: 25 * time.Second, Address: "172.31.255.9", HasRoutes: false},
+	})
+	if got := c.modeFor("rw"); got != ModeDisabled {
+		t.Errorf("route-less peer must be ungated even when explicitly active, got %q", got)
+	}
+}
+
+func TestPeerInfos_DerivesHasRoutesFromSpec(t *testing.T) {
+	state := agent.State{Peers: []v1alpha1.WireguardPeer{
+		{Spec: v1alpha1.WireguardPeerSpec{PublicKey: "none", Address: "172.31.255.8"}},
+		{Spec: v1alpha1.WireguardPeerSpec{PublicKey: "v4", Address: "172.31.255.3", Routes: []string{"10.254.0.0/16"}}},
+		{Spec: v1alpha1.WireguardPeerSpec{PublicKey: "v6", Address: "172.31.255.4", RoutesV6: []string{"fd00::/64"}}},
+	}}
+	got := map[string]bool{}
+	for _, p := range PeerInfos(state) {
+		got[p.PublicKey] = p.HasRoutes
+	}
+	if got["none"] {
+		t.Error("peer without routes should have HasRoutes=false")
+	}
+	if !got["v4"] {
+		t.Error("peer with IPv4 routes should have HasRoutes=true")
+	}
+	if !got["v6"] {
+		t.Error("peer with only IPv6 routes should have HasRoutes=true")
+	}
+}
+
 func TestPeerStat_FakeReaderRoundTrips(t *testing.T) {
 	now := time.Unix(1000, 0)
 	r := fakeReader{peers: []peerStat{{PublicKey: "k", LastHandshakeTime: now, ReceiveBytes: 42}}}
@@ -181,7 +255,7 @@ func TestController_AppliesOnlyOnTransition(t *testing.T) {
 		t.Fatalf("quiet tick must not apply, got %d", applied)
 	}
 	clk = ts(1100) // 100s later, > 75s window, no new bytes
-	c.tickOnce() // live->not-live transition
+	c.tickOnce()   // live->not-live transition
 	if applied != 2 {
 		t.Fatalf("expected apply on down-transition, got %d", applied)
 	}