From ca9640227a4ec2f661309288cdbd3c2d56208c59 Mon Sep 17 00:00:00 2001 From: Maximilian Rink Date: Mon, 24 Aug 2026 21:46:05 +0200 Subject: [PATCH 1/3] test(endpoints): follow WireGuard clear refactor Signed-off-by: Maximilian Rink --- pkg/endpoints/endpoints_wireguard_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/endpoints/endpoints_wireguard_test.go b/pkg/endpoints/endpoints_wireguard_test.go index 3f9abc55..5173f6b4 100644 --- a/pkg/endpoints/endpoints_wireguard_test.go +++ b/pkg/endpoints/endpoints_wireguard_test.go @@ -6,7 +6,7 @@ import ( v1 "k8s.io/api/core/v1" ) -func TestWireguardDeleteDoesNotDereferenceNilServiceContext(t *testing.T) { +func TestWireguardClearDoesNotDereferenceNilServiceContext(t *testing.T) { worker := &wireguardWorker{} service := &v1.Service{} From b034c81befb6ad2c2a0c92d27dc527bdf8daff59 Mon Sep 17 00:00:00 2001 From: Maximilian Rink Date: Sun, 23 Aug 2026 23:35:17 +0200 Subject: [PATCH 2/3] fix(vip): harden DHCP client lifecycle and reply parsing Signed-off-by: Maximilian Rink --- pkg/vip/dhcpv4.go | 19 ++++++++---- pkg/vip/dhcpv4_backoff_test.go | 40 ++++++++++++++++++++++++ pkg/vip/dhcpv6.go | 12 +++++--- pkg/vip/dhcpv6_robustness_test.go | 51 +++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 pkg/vip/dhcpv4_backoff_test.go create mode 100644 pkg/vip/dhcpv6_robustness_test.go diff --git a/pkg/vip/dhcpv4.go b/pkg/vip/dhcpv4.go index c57bc185..34faa6b2 100644 --- a/pkg/vip/dhcpv4.go +++ b/pkg/vip/dhcpv4.go @@ -132,7 +132,10 @@ func (c *DHCPv4Client) Start(ctx context.Context) error { dhcpCtx, cancel := context.WithCancel(ctx) defer cancel() - lease := c.requestWithBackoff(dhcpCtx) + lease, err := c.requestWithBackoff(dhcpCtx) + if err != nil { + return fmt.Errorf("DHCPv4 client failed: %w", err) + } c.initRebootFlag = false c.lease = lease @@ -173,7 +176,12 @@ func (c *DHCPv4Client) Start(ctx context.Context) error { } log.Warn("[DHCPv4] ip may have changed", "ip", c.lease.ACK.YourIPAddr, "err", err) c.initRebootFlag = false - c.lease = c.requestWithBackoff(dhcpCtx) + lease, backoffErr := c.requestWithBackoff(dhcpCtx) + if backoffErr != nil { + log.Error("[DHCPv4] failed to reacquire lease", "err", backoffErr) + continue + } + c.lease = lease } t1.Reset(t1Timeout) t2.Reset(t2Timeout) @@ -204,7 +212,7 @@ func (c *DHCPv4Client) Start(ctx context.Context) error { // |ciaddr |zero | IP address |IP address| // -------------------------------------------------------- -func (c *DHCPv4Client) requestWithBackoff(ctx context.Context) *nclient4.Lease { +func (c *DHCPv4Client) requestWithBackoff(ctx context.Context) (*nclient4.Lease, error) { backoff := backoff.Backoff{ Factor: 2, Jitter: true, @@ -226,8 +234,7 @@ func (c *DHCPv4Client) requestWithBackoff(ctx context.Context) *nclient4.Lease { errMsg := fmt.Errorf("failed to get an IPv4 address after %d attempt(s), giving up, error: %s", c.backoffAttempts, err.Error()) log.Error(fmt.Sprintf("[DHCPv4] %s", errMsg.Error())) c.errorChan <- errMsg - c.Stop() - return nil + return nil, errMsg } log.Error("[DHCPv4] request failed", "attempt", backoff.Attempt(), "err", err.Error(), "waiting", dur) time.Sleep(dur) @@ -242,7 +249,7 @@ func (c *DHCPv4Client) requestWithBackoff(ctx context.Context) *nclient4.Lease { c.ipChan <- lease.ACK.YourIPAddr.String() } - return lease + return lease, nil } func (c *DHCPv4Client) request(ctx context.Context, rebind bool) (*nclient4.Lease, error) { diff --git a/pkg/vip/dhcpv4_backoff_test.go b/pkg/vip/dhcpv4_backoff_test.go new file mode 100644 index 00000000..0909af1b --- /dev/null +++ b/pkg/vip/dhcpv4_backoff_test.go @@ -0,0 +1,40 @@ +package vip + +import ( + "context" + "net" + "testing" + "time" +) + +func TestDHCPv4BackoffExhaustionDoesNotDeadlock(t *testing.T) { + // DEFECT: requestWithBackoff calls Stop from the Start/request goroutine before Start can close releasedChan, so exhausted DHCPv4 retries deadlock (pkg/vip/dhcpv4.go:229). + client := NewDHCPv4Client( + &net.Interface{Name: "definitely-not-a-kube-vip-interface"}, + false, + "", + 1, + false, + ) + + done := make(chan struct{}) + go func() { + client.requestWithBackoff(context.Background()) + close(done) + }() + + select { + case err := <-client.ErrorChannel(): + if err == nil { + t.Fatal("expected DHCPv4 request error") + } + case <-time.After(time.Second): + t.Fatal("DHCPv4 did not report exhausted backoff") + } + + select { + case <-done: + case <-time.After(100 * time.Millisecond): + t.Fatal("DHCPv4 backoff exhaustion deadlocked") + } +} diff --git a/pkg/vip/dhcpv6.go b/pkg/vip/dhcpv6.go index 4000a3be..c83c336c 100644 --- a/pkg/vip/dhcpv6.go +++ b/pkg/vip/dhcpv6.go @@ -90,6 +90,7 @@ func NewDHCPv6InternalClient(iface string) (*DHCPv6InternalClient, error) { type DHCPv6Client struct { iface *net.Interface + managerKey string ddnsHostName string initRebootFlag bool requestedIP net.IP @@ -117,6 +118,7 @@ func NewDHCPv6Client(iface *net.Interface, parent netlink.Link, initRebootFlag b return &DHCPv6Client{ iface: iface, + managerKey: name, stopChan: make(chan struct{}), releasedChan: make(chan struct{}), errorChan: make(chan error), @@ -140,7 +142,7 @@ func (c *DHCPv6Client) Stop() { close(c.stopChan) }) <-c.releasedChan - dhcpv6ClientManager.Delete(c.iface.Name) + dhcpv6ClientManager.Delete(c.managerKey) } // Gets the IPChannel for consumption @@ -247,7 +249,6 @@ func (c *DHCPv6Client) requestWithBackoff(ctx context.Context) (*dhcpv6.OptIAAdd errMsg := fmt.Errorf("failed to get an IPv4 address after %d attempt(s), giving up, error: %s", c.backoffAttempts, err.Error()) log.Error(fmt.Sprintf("[DHCPv6] %s", errMsg.Error())) c.errorChan <- errMsg - c.Stop() return nil, fmt.Errorf("failed to get IPv6 address: %w", err) } log.Error("[DHCPv6] request failed", "attempt", backoff.Attempt(), "err", err.Error(), "waiting", dur) @@ -385,9 +386,10 @@ func getAddress(iana []*dhcpv6.OptIANA) (*dhcpv6.OptIAAddress, error) { return nil, fmt.Errorf("failed to get IANA") } - if len(iana) < 1 { - return nil, fmt.Errorf("failed to get addresses data") + addrs := iana[0].Options.Addresses() + if len(addrs) == 0 { + return nil, fmt.Errorf("IANA contained no addresses") } - return iana[0].Options.Addresses()[0], nil + return addrs[0], nil } diff --git a/pkg/vip/dhcpv6_robustness_test.go b/pkg/vip/dhcpv6_robustness_test.go new file mode 100644 index 00000000..c313c15d --- /dev/null +++ b/pkg/vip/dhcpv6_robustness_test.go @@ -0,0 +1,51 @@ +package vip + +import ( + "net" + "sync/atomic" + "testing" + + "github.com/insomniacslk/dhcp/dhcpv6" +) + +func TestDHCPv6StopReleasesManagerReferenceForParentInterface(t *testing.T) { + // DEFECT: Stop deletes the manager entry using the VLAN child name even though NewDHCPv6Client keyed the shared client by its parent name (pkg/vip/dhcpv6.go:143). + previousManager := dhcpv6ClientManager + t.Cleanup(func() { dhcpv6ClientManager = previousManager }) + + references := &atomic.Int32{} + references.Store(2) + shared := &DHCPv6InternalClient{references: references} + dhcpv6ClientManager = &DHCPv6ClientManager{ + clients: map[string]*DHCPv6InternalClient{"parent0": shared}, + } + + client := &DHCPv6Client{ + iface: &net.Interface{Name: "vlan-child"}, + managerKey: "parent0", + ipChan: make(chan string), + stopChan: make(chan struct{}), + releasedChan: make(chan struct{}), + ic: shared, + } + close(client.releasedChan) + + client.Stop() + + if got := references.Load(); got != 1 { + t.Fatalf("manager reference count = %d, want 1 after stopping one VLAN client", got) + } +} + +func TestGetAddressRejectsIANAWithoutAddresses(t *testing.T) { + // DEFECT: getAddress indexes the first IAADDR without checking whether the IANA contains one, so a malformed/expired reply panics (pkg/vip/dhcpv6.go:392). + defer func() { + if recovered := recover(); recovered != nil { + t.Fatalf("getAddress panicked on an IANA without IAADDR: %v", recovered) + } + }() + + if _, err := getAddress([]*dhcpv6.OptIANA{{}}); err == nil { + t.Fatal("getAddress accepted an IANA without an IAADDR") + } +} From b0b12cfc131d974715e0005032430dbafafa3813 Mon Sep 17 00:00:00 2001 From: Maximilian Rink Date: Sun, 23 Aug 2026 23:50:38 +0200 Subject: [PATCH 3/3] test(vip): fix errcheck lint in DHCPv4 backoff test Signed-off-by: Maximilian Rink --- pkg/vip/dhcpv4_backoff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/vip/dhcpv4_backoff_test.go b/pkg/vip/dhcpv4_backoff_test.go index 0909af1b..fa8a9a94 100644 --- a/pkg/vip/dhcpv4_backoff_test.go +++ b/pkg/vip/dhcpv4_backoff_test.go @@ -19,7 +19,7 @@ func TestDHCPv4BackoffExhaustionDoesNotDeadlock(t *testing.T) { done := make(chan struct{}) go func() { - client.requestWithBackoff(context.Background()) + _, _ = client.requestWithBackoff(context.Background()) close(done) }()