Merge pull request #1720 from MaxRink/fix/fix-dad-skip

fix(vip): apply per-call DAD skip instead of leaking it into persistent state

pkg/vip/address.go

@@ -469,9 +469,13 @@ func (configurator *network) AddIP(precheck bool, skipDAD bool, minLifetime ...i
 	// an address that we know should be ours (e.g., after DADFAILED recovery).
 	// We also allow to globally configure NODAD in case user knows they are running in an
 	// environment where multiple nodes may advertise the same VIP (e.g., ECMP routing).
-	if configurator.shouldSkipDAD(skipDAD) && utils.IsIPv6(configurator.address.IP.String()) {
+	if utils.IsIPv6(configurator.address.IP.String()) {
+		if configurator.shouldSkipDAD(skipDAD) {
 			configurator.address.Flags |= unix.IFA_F_NODAD
 			log.Debug("Setting IFA_F_NODAD flag for IPv6 address to skip DAD", "ip", configurator.address.IP.String())
+		} else {
+			configurator.address.Flags &^= unix.IFA_F_NODAD
+		}
 	}
 
 	log.Debug("replacing IP", "address", configurator.address)

pkg/vip/address_dad_linux_test.go

@@ -0,0 +1,37 @@
+//go:build linux
+
+package vip
+
+import (
+	"testing"
+
+	"github.com/kube-vip/kube-vip/pkg/networkinterface"
+	"github.com/vishvananda/netlink"
+	"golang.org/x/sys/unix"
+)
+
+func TestAddIPPerCallDADSkipDoesNotPersist(t *testing.T) {
+	address, err := netlink.ParseAddr("2001:db8::10/128")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	configurator := &network{
+		address: address,
+		link: &networkinterface.Link{
+			Intf: &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "kube-vip-dad-test"}},
+		},
+	}
+
+	// The netlink operation may fail without CAP_NET_ADMIN, but the address
+	// flags are set before that operation and are what this test exercises.
+	_, _ = configurator.AddIP(false, true)
+	if configurator.address.Flags&unix.IFA_F_NODAD == 0 {
+		t.Fatal("skipDAD=true did not set IFA_F_NODAD")
+	}
+
+	_, _ = configurator.AddIP(false, false)
+	if configurator.address.Flags&unix.IFA_F_NODAD != 0 {
+		t.Fatal("IFA_F_NODAD persisted into a normal AddIP call")
+	}
+}