From 8a43f4f9439c239972644b4a6ec9440c929deb7e Mon Sep 17 00:00:00 2001 From: Riccardo Manfrin <3090891+riccardomanfrin@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:42:57 +0200 Subject: [PATCH] [client] fix build: add ReapplyMatching to dedicated AllowedIPsRefCounter (#6935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes #6799 turned `AllowedIPsRefCounter` from an alias of the generic `Counter` into a dedicated peer-aware type. A change merged in parallel — `DefaultManager.ReconcilePeerAllowedIPs` (lazy-connection idle→wake reconvergence) — calls `allowedIPsRefCounter.ReapplyMatching`, which only existed on the generic `Counter`. Each PR built alone; the merged `main` did not: ``` client/internal/routemanager/manager.go: m.allowedIPsRefCounter.ReapplyMatching undefined ``` Add `ReapplyMatching(pred, apply)` to the dedicated type, matching the generic contract (keyed on the active/`Out` peer): it re-applies every prefix whose currently installed peer satisfies `pred`, skipping prefixes with no active peer (reconciled by the next Increment/Decrement). Also update `reconcile_test.go` to construct the counter via `refcounter.NewAllowedIPs` — the generic `refcounter.New` return value is no longer assignable to the dedicated type. Verified with `cd client && CGO_ENABLED=1 go build .` (the failing CI step) and the `TestReconcilePeerAllowedIPs` / refcounter / routemanager tests. ## Issue ticket number and link No public issue — fixes a `main` build break from a semantic merge conflict between #6799 and the `ReconcilePeerAllowedIPs` change. Failing run: https://github.com/netbirdio/netbird/actions/runs/30330882389/job/90185591366 ## Stack ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Internal build fix restoring a method on the AllowedIPs refcounter. No public API, CLI, config, or behavior change. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: N/A --- View with [code]smith Autofix with [code]smith Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled. ## Summary by CodeRabbit * **Bug Fixes** * Improved route reconciliation so all applicable allowed IP prefixes are reliably re-applied for the correct peer. * Prevented reconciliation from affecting routes assigned to other peers. * Improved handling of errors encountered while restoring multiple routes, providing more consistent results. --- .../internal/routemanager/reconcile_test.go | 2 +- .../routemanager/refcounter/allowedips.go | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/client/internal/routemanager/reconcile_test.go b/client/internal/routemanager/reconcile_test.go index 2a8a4dc10..c6806a6cd 100644 --- a/client/internal/routemanager/reconcile_test.go +++ b/client/internal/routemanager/reconcile_test.go @@ -54,7 +54,7 @@ func (m *reconcileWGMock) GetNet() *netstack.Net { return n func TestReconcilePeerAllowedIPs(t *testing.T) { wg := &reconcileWGMock{} m := &DefaultManager{wgInterface: wg} - m.allowedIPsRefCounter = refcounter.New[netip.Prefix, string, string]( + m.allowedIPsRefCounter = refcounter.NewAllowedIPs( func(_ netip.Prefix, peerKey string) (string, error) { return peerKey, nil }, func(netip.Prefix, string) error { return nil }, ) diff --git a/client/internal/routemanager/refcounter/allowedips.go b/client/internal/routemanager/refcounter/allowedips.go index 6f23162f6..6d682e8a9 100644 --- a/client/internal/routemanager/refcounter/allowedips.go +++ b/client/internal/routemanager/refcounter/allowedips.go @@ -169,6 +169,27 @@ func (rm *AllowedIPsRefCounter) Flush() error { return nberrors.FormatErrorOrNil(merr) } +// ReapplyMatching calls apply for every prefix whose currently installed (active) peer satisfies +// pred, holding the lock for the whole pass. It is used to re-push allowed IPs onto a peer whose +// WireGuard entry was rebuilt (e.g. a lazy connection cycling idle->wake) without a matching +// refcounter change, which would otherwise leave the prefix installed in the counter but missing +// on the device. Only the active peer is considered — a prefix that lost its installed peer to a +// failed swap is skipped here and reconciled by the next Increment/Decrement. +func (rm *AllowedIPsRefCounter) ReapplyMatching(pred func(out string) bool, apply func(key netip.Prefix) error) error { + rm.mu.Lock() + defer rm.mu.Unlock() + + var merr *multierror.Error + for prefix, e := range rm.entries { + if e.active != "" && pred(e.active) { + if err := apply(prefix); err != nil { + merr = multierror.Append(merr, err) + } + } + } + return nberrors.FormatErrorOrNil(merr) +} + // pickSurvivor deterministically selects a peer still referencing the prefix. WireGuard cannot do // multipath for a single prefix, so any surviving peer is a valid winner; the choice is made stable // (lowest peerKey) for predictable behavior and testability.