From 4fe5cbe70327f940cc91431512296d82b59030d5 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 30 Nov 2025 19:02:15 +0100 Subject: [PATCH] hscontrol/oidc: fix ACL policy not applied to new OIDC nodes (#2890) Fixes #2888 Fixes #2896 --- .github/workflows/test-integration.yaml | 3 + CHANGELOG.md | 7 + hscontrol/auth.go | 14 +- hscontrol/grpcv1.go | 8 +- hscontrol/oidc.go | 46 +-- hscontrol/state/state.go | 53 ++- integration/auth_key_test.go | 182 +++++++++ integration/auth_oidc_test.go | 490 ++++++++++++++++++++++++ integration/route_test.go | 61 +-- 9 files changed, 757 insertions(+), 107 deletions(-) diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 2f82ba22..62ea3a97 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -33,6 +33,7 @@ jobs: - TestAuthKeyLogoutAndReloginNewUser - TestAuthKeyLogoutAndReloginSameUserExpiredKey - TestAuthKeyDeleteKey + - TestAuthKeyLogoutAndReloginRoutesPreserved - TestOIDCAuthenticationPingAll - TestOIDCExpireNodesBasedOnTokenExpiry - TestOIDC024UserCreation @@ -42,6 +43,8 @@ jobs: - TestOIDCMultipleOpenedLoginUrls - TestOIDCReloginSameNodeSameUser - TestOIDCExpiryAfterRestart + - TestOIDCACLPolicyOnJoin + - TestOIDCReloginSameUserRoutesPreserved - TestAuthWebFlowAuthenticationPingAll - TestAuthWebFlowLogoutAndReloginSameUser - TestAuthWebFlowLogoutAndReloginNewUser diff --git a/CHANGELOG.md b/CHANGELOG.md index 7669dfcb..82146a5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ ### Changes +## 0.27.2 (2025-xx-xx) + +### Changes + +- Fix ACL policy not applied to new OIDC nodes until client restart + [#2890](https://github.com/juanfont/headscale/pull/2890) + ## 0.27.1 (2025-11-11) **Minimum supported Tailscale client version: v1.64.0** diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 447035da..1ab22709 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -11,7 +11,6 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/types/change" "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "gorm.io/gorm" @@ -364,16 +363,13 @@ func (h *Headscale) handleRegisterWithAuthKey( // eventbus. // TODO(kradalby): This needs to be ran as part of the batcher maybe? // now since we dont update the node/pol here anymore - routeChange := h.state.AutoApproveRoutes(node) - - if _, _, err := h.state.SaveNode(node); err != nil { - return nil, fmt.Errorf("saving auto approved routes to node: %w", err) + routesChange, err := h.state.AutoApproveRoutes(node) + if err != nil { + return nil, fmt.Errorf("auto approving routes: %w", err) } - if routeChange && changed.Empty() { - changed = change.NodeAdded(node.ID()) - } - h.Change(changed) + // Send both changes. Empty changes are ignored by Change(). + h.Change(changed, routesChange) // TODO(kradalby): I think this is covered above, but we need to validate that. // // If policy changed due to node registration, send a separate policy change diff --git a/hscontrol/grpcv1.go b/hscontrol/grpcv1.go index f54835e9..3f8e75df 100644 --- a/hscontrol/grpcv1.go +++ b/hscontrol/grpcv1.go @@ -294,13 +294,13 @@ func (api headscaleV1APIServer) RegisterNode( // ensure we send an update. // This works, but might be another good candidate for doing some sort of // eventbus. - _ = api.h.state.AutoApproveRoutes(node) - _, _, err = api.h.state.SaveNode(node) + routeChange, err := api.h.state.AutoApproveRoutes(node) if err != nil { - return nil, fmt.Errorf("saving auto approved routes to node: %w", err) + return nil, fmt.Errorf("auto approving routes: %w", err) } - api.h.Change(nodeChange) + // Send both changes. Empty changes are ignored by Change(). + api.h.Change(nodeChange, routeChange) return &v1.RegisterNodeResponse{Node: node.Proto()}, nil } diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 7c7895c6..22ccab06 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -42,10 +42,6 @@ var ( errOIDCAllowedUsers = errors.New( "authenticated principal does not match any allowed user", ) - errOIDCInvalidNodeState = errors.New( - "requested node state key expired before authorisation completed", - ) - errOIDCNodeKeyMissing = errors.New("could not get node key from cache") ) // RegistrationInfo contains both machine key and verifier information for OIDC validation. @@ -108,16 +104,8 @@ func (a *AuthProviderOIDC) AuthURL(registrationID types.RegistrationID) string { registrationID.String()) } -func (a *AuthProviderOIDC) determineNodeExpiry(idTokenExpiration time.Time) time.Time { - if a.cfg.UseExpiryFromToken { - return idTokenExpiration - } - - return time.Now().Add(a.cfg.Expiry) -} - -// RegisterOIDC redirects to the OIDC provider for authentication -// Puts NodeKey in cache so the callback can retrieve it using the oidc state param +// RegisterHandler registers the OIDC callback handler with the given router. +// It puts NodeKey in cache so the callback can retrieve it using the oidc state param. // Listens in /register/:registration_id. func (a *AuthProviderOIDC) RegisterHandler( writer http.ResponseWriter, @@ -304,7 +292,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( return } - user, c, err := a.createOrUpdateUserFromClaim(&claims) + user, _, err := a.createOrUpdateUserFromClaim(&claims) if err != nil { log.Error(). Err(err). @@ -323,9 +311,6 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( return } - // Send policy update notifications if needed - a.h.Change(c) - // TODO(kradalby): Is this comment right? // If the node exists, then the node should be reauthenticated, // if the node does not exist, and the machine key exists, then @@ -372,6 +357,14 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( httpError(writer, NewHTTPError(http.StatusGone, "login session expired, try again", nil)) } +func (a *AuthProviderOIDC) determineNodeExpiry(idTokenExpiration time.Time) time.Time { + if a.cfg.UseExpiryFromToken { + return idTokenExpiration + } + + return time.Now().Add(a.cfg.Expiry) +} + func extractCodeAndStateParamFromRequest( req *http.Request, ) (string, string, error) { @@ -504,8 +497,8 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim( } // if the user is still not found, create a new empty user. - // TODO(kradalby): This might cause us to not have an ID below which - // is a problem. + // TODO(kradalby): This context is not inherited from the request, which is probably not ideal. + // However, we need a context to use the OIDC provider. if user == nil { newUser = true user = &types.User{} @@ -557,18 +550,13 @@ func (a *AuthProviderOIDC) handleRegistration( // ensure we send an update. // This works, but might be another good candidate for doing some sort of // eventbus. - _ = a.h.state.AutoApproveRoutes(node) - _, policyChange, err := a.h.state.SaveNode(node) + routesChange, err := a.h.state.AutoApproveRoutes(node) if err != nil { - return false, fmt.Errorf("saving auto approved routes to node: %w", err) + return false, fmt.Errorf("auto approving routes: %w", err) } - // Policy updates are full and take precedence over node changes. - if !policyChange.Empty() { - a.h.Change(policyChange) - } else { - a.h.Change(nodeChange) - } + // Send both changes. Empty changes are ignored by Change(). + a.h.Change(nodeChange, routesChange) return !nodeChange.Empty(), nil } diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index b447dfc6..768c5c3a 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -827,7 +827,7 @@ func (s *State) SetPolicy(pol []byte) (bool, error) { // AutoApproveRoutes checks if a node's routes should be auto-approved. // AutoApproveRoutes checks if any routes should be auto-approved for a node and updates them. -func (s *State) AutoApproveRoutes(nv types.NodeView) bool { +func (s *State) AutoApproveRoutes(nv types.NodeView) (change.ChangeSet, error) { approved, changed := policy.ApproveRoutesWithPolicy(s.polMan, nv, nv.ApprovedRoutes().AsSlice(), nv.AnnouncedRoutes()) if changed { log.Debug(). @@ -840,7 +840,7 @@ func (s *State) AutoApproveRoutes(nv types.NodeView) bool { // Persist the auto-approved routes to database and NodeStore via SetApprovedRoutes // This ensures consistency between database and NodeStore - _, _, err := s.SetApprovedRoutes(nv.ID(), approved) + _, c, err := s.SetApprovedRoutes(nv.ID(), approved) if err != nil { log.Error(). Uint64("node.id", nv.ID().Uint64()). @@ -848,13 +848,15 @@ func (s *State) AutoApproveRoutes(nv types.NodeView) bool { Err(err). Msg("Failed to persist auto-approved routes") - return false + return change.EmptySet, err } log.Info().Uint64("node.id", nv.ID().Uint64()).Str("node.name", nv.Hostname()).Strs("routes.approved", util.PrefixesToString(approved)).Msg("Routes approved") + + return c, nil } - return changed + return change.EmptySet, nil } // GetPolicy retrieves the current policy from the database. @@ -1637,6 +1639,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest var routeChange bool var hostinfoChanged bool var needsRouteApproval bool + var autoApprovedRoutes []netip.Prefix // We need to ensure we update the node as it is in the NodeStore at // the time of the request. updatedNode, ok := s.nodeStore.UpdateNode(id, func(currentNode *types.Node) { @@ -1661,7 +1664,6 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest } // Calculate route approval before NodeStore update to avoid calling View() inside callback - var autoApprovedRoutes []netip.Prefix var hasNewRoutes bool if hi := req.Hostinfo; hi != nil { hasNewRoutes = len(hi.RoutableIPs) > 0 @@ -1727,7 +1729,6 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest Strs("newApprovedRoutes", util.PrefixesToString(autoApprovedRoutes)). Bool("routeChanged", routeChange). Msg("applying route approval results") - currentNode.ApprovedRoutes = autoApprovedRoutes } } }) @@ -1736,6 +1737,24 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest return change.EmptySet, fmt.Errorf("node not found in NodeStore: %d", id) } + if routeChange { + log.Debug(). + Uint64("node.id", id.Uint64()). + Strs("autoApprovedRoutes", util.PrefixesToString(autoApprovedRoutes)). + Msg("Persisting auto-approved routes from MapRequest") + + // SetApprovedRoutes will update both database and PrimaryRoutes table + _, c, err := s.SetApprovedRoutes(id, autoApprovedRoutes) + if err != nil { + return change.EmptySet, fmt.Errorf("persisting auto-approved routes: %w", err) + } + + // If SetApprovedRoutes resulted in a policy change, return it + if !c.Empty() { + return c, nil + } + } // Continue with the rest of the processing using the updated node + nodeRouteChange := change.EmptySet // Handle route changes after NodeStore update @@ -1750,13 +1769,8 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest routesChangedButNotApproved = true } } - if routeChange { - needsRouteUpdate = true - log.Debug(). - Caller(). - Uint64("node.id", id.Uint64()). - Msg("updating routes because approved routes changed") - } else if routesChangedButNotApproved { + + if routesChangedButNotApproved { needsRouteUpdate = true log.Debug(). Caller(). @@ -1793,25 +1807,26 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest return change.NodeAdded(id), nil } -func hostinfoEqual(oldNode types.NodeView, new *tailcfg.Hostinfo) bool { - if !oldNode.Valid() && new == nil { +func hostinfoEqual(oldNode types.NodeView, newHI *tailcfg.Hostinfo) bool { + if !oldNode.Valid() && newHI == nil { return true } - if !oldNode.Valid() || new == nil { + + if !oldNode.Valid() || newHI == nil { return false } old := oldNode.AsStruct().Hostinfo - return old.Equal(new) + return old.Equal(newHI) } -func routesChanged(oldNode types.NodeView, new *tailcfg.Hostinfo) bool { +func routesChanged(oldNode types.NodeView, newHI *tailcfg.Hostinfo) bool { var oldRoutes []netip.Prefix if oldNode.Valid() && oldNode.AsStruct().Hostinfo != nil { oldRoutes = oldNode.AsStruct().Hostinfo.RoutableIPs } - newRoutes := new.RoutableIPs + newRoutes := newHI.RoutableIPs if newRoutes == nil { newRoutes = []netip.Prefix{} } diff --git a/integration/auth_key_test.go b/integration/auth_key_test.go index 514a6018..3faf793a 100644 --- a/integration/auth_key_test.go +++ b/integration/auth_key_test.go @@ -9,12 +9,15 @@ import ( "time" v1 "github.com/juanfont/headscale/gen/go/headscale/v1" + policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/integration/hsic" "github.com/juanfont/headscale/integration/tsic" "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/tailcfg" + "tailscale.com/types/ptr" ) func TestAuthKeyLogoutAndReloginSameUser(t *testing.T) { @@ -540,3 +543,182 @@ func TestAuthKeyDeleteKey(t *testing.T) { t.Logf("✓ Node successfully reconnected after its auth key was deleted") } + +// TestAuthKeyLogoutAndReloginRoutesPreserved tests that routes remain serving +// after a node logs out and re-authenticates with the same user. +// +// This test validates the fix for issue #2896: +// https://github.com/juanfont/headscale/issues/2896 +// +// Bug: When a node with already-approved routes restarts/re-authenticates, +// the routes show as "Approved" and "Available" but NOT "Serving" (Primary). +// A headscale restart would fix it, indicating a state management issue. +// +// The test scenario: +// 1. Node registers with auth key and advertises routes +// 2. Routes are auto-approved and verified as serving +// 3. Node logs out +// 4. Node re-authenticates with same auth key +// 5. Routes should STILL be serving (this is where the bug manifests) +func TestAuthKeyLogoutAndReloginRoutesPreserved(t *testing.T) { + IntegrationSkip(t) + + user := "routeuser" + advertiseRoute := "10.55.0.0/24" + + spec := ScenarioSpec{ + NodesPerUser: 1, + Users: []string{user}, + } + + scenario, err := NewScenario(spec) + require.NoError(t, err) + defer scenario.ShutdownAssertNoPanics(t) + + err = scenario.CreateHeadscaleEnv( + []tsic.Option{ + tsic.WithAcceptRoutes(), + // Advertise route on initial login + tsic.WithExtraLoginArgs([]string{"--advertise-routes=" + advertiseRoute}), + }, + hsic.WithTestName("routelogout"), + hsic.WithTLS(), + hsic.WithACLPolicy( + &policyv2.Policy{ + ACLs: []policyv2.ACL{ + { + Action: "accept", + Sources: []policyv2.Alias{policyv2.Wildcard}, + Destinations: []policyv2.AliasWithPorts{{Alias: policyv2.Wildcard, Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}}}, + }, + }, + AutoApprovers: policyv2.AutoApproverPolicy{ + Routes: map[netip.Prefix]policyv2.AutoApprovers{ + netip.MustParsePrefix(advertiseRoute): {ptr.To(policyv2.Username(user + "@test.no"))}, + }, + }, + }, + ), + ) + requireNoErrHeadscaleEnv(t, err) + + allClients, err := scenario.ListTailscaleClients() + requireNoErrListClients(t, err) + require.Len(t, allClients, 1) + + client := allClients[0] + + err = scenario.WaitForTailscaleSync() + requireNoErrSync(t, err) + + headscale, err := scenario.Headscale() + requireNoErrGetHeadscale(t, err) + + // Step 1: Verify initial route is advertised, approved, and SERVING + t.Logf("Step 1: Verifying initial route is advertised, approved, and SERVING at %s", time.Now().Format(TimestampFormat)) + + var initialNode *v1.Node + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Should have exactly 1 node") + + if len(nodes) == 1 { + initialNode = nodes[0] + // Check: 1 announced, 1 approved, 1 serving (subnet route) + assert.Lenf(c, initialNode.GetAvailableRoutes(), 1, + "Node should have 1 available route, got %v", initialNode.GetAvailableRoutes()) + assert.Lenf(c, initialNode.GetApprovedRoutes(), 1, + "Node should have 1 approved route, got %v", initialNode.GetApprovedRoutes()) + assert.Lenf(c, initialNode.GetSubnetRoutes(), 1, + "Node should have 1 serving (subnet) route, got %v - THIS IS THE BUG if empty", initialNode.GetSubnetRoutes()) + assert.Contains(c, initialNode.GetSubnetRoutes(), advertiseRoute, + "Subnet routes should contain %s", advertiseRoute) + } + }, 30*time.Second, 500*time.Millisecond, "initial route should be serving") + + require.NotNil(t, initialNode, "Initial node should be found") + initialNodeID := initialNode.GetId() + t.Logf("Initial node ID: %d, Available: %v, Approved: %v, Serving: %v", + initialNodeID, initialNode.GetAvailableRoutes(), initialNode.GetApprovedRoutes(), initialNode.GetSubnetRoutes()) + + // Step 2: Logout + t.Logf("Step 2: Logging out at %s", time.Now().Format(TimestampFormat)) + + err = client.Logout() + require.NoError(t, err) + + // Wait for logout to complete + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := client.Status() + assert.NoError(ct, err) + assert.Equal(ct, "NeedsLogin", status.BackendState, "Expected NeedsLogin state after logout") + }, 30*time.Second, 1*time.Second, "waiting for logout to complete") + + t.Logf("Logout completed, node should still exist in database") + + // Verify node still exists (routes should still be in DB) + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Node should persist in database after logout") + }, 10*time.Second, 500*time.Millisecond, "node should persist after logout") + + // Step 3: Re-authenticate with the SAME user (using auth key) + t.Logf("Step 3: Re-authenticating with same user at %s", time.Now().Format(TimestampFormat)) + + userMap, err := headscale.MapUsers() + require.NoError(t, err) + + key, err := scenario.CreatePreAuthKey(userMap[user].GetId(), true, false) + require.NoError(t, err) + + // Re-login - the container already has extraLoginArgs with --advertise-routes + // from the initial setup, so routes will be advertised on re-login + err = scenario.RunTailscaleUp(user, headscale.GetEndpoint(), key.GetKey()) + require.NoError(t, err) + + // Wait for client to be running + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := client.Status() + assert.NoError(ct, err) + assert.Equal(ct, "Running", status.BackendState, "Expected Running state after relogin") + }, 30*time.Second, 1*time.Second, "waiting for relogin to complete") + + t.Logf("Re-authentication completed at %s", time.Now().Format(TimestampFormat)) + + // Step 4: THE CRITICAL TEST - Verify routes are STILL SERVING after re-authentication + t.Logf("Step 4: Verifying routes are STILL SERVING after re-authentication at %s", time.Now().Format(TimestampFormat)) + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Should still have exactly 1 node after relogin") + + if len(nodes) == 1 { + node := nodes[0] + t.Logf("After relogin - Available: %v, Approved: %v, Serving: %v", + node.GetAvailableRoutes(), node.GetApprovedRoutes(), node.GetSubnetRoutes()) + + // This is where issue #2896 manifests: + // - Available shows the route (from Hostinfo.RoutableIPs) + // - Approved shows the route (from ApprovedRoutes) + // - BUT Serving (SubnetRoutes/PrimaryRoutes) is EMPTY! + assert.Lenf(c, node.GetAvailableRoutes(), 1, + "Node should have 1 available route after relogin, got %v", node.GetAvailableRoutes()) + assert.Lenf(c, node.GetApprovedRoutes(), 1, + "Node should have 1 approved route after relogin, got %v", node.GetApprovedRoutes()) + assert.Lenf(c, node.GetSubnetRoutes(), 1, + "BUG #2896: Node should have 1 SERVING route after relogin, got %v", node.GetSubnetRoutes()) + assert.Contains(c, node.GetSubnetRoutes(), advertiseRoute, + "BUG #2896: Subnet routes should contain %s after relogin", advertiseRoute) + + // Also verify node ID was preserved (same node, not new registration) + assert.Equal(c, initialNodeID, node.GetId(), + "Node ID should be preserved after same-user relogin") + } + }, 30*time.Second, 500*time.Millisecond, + "BUG #2896: routes should remain SERVING after logout/relogin with same user") + + t.Logf("Test completed - verifying issue #2896 fix") +} diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index 9040e5fd..8d2f648a 100644 --- a/integration/auth_oidc_test.go +++ b/integration/auth_oidc_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/juanfont/headscale/gen/go/headscale/v1" + policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/integration/hsic" "github.com/juanfont/headscale/integration/tsic" @@ -19,6 +20,8 @@ import ( "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/ipn/ipnstate" + "tailscale.com/tailcfg" ) func TestOIDCAuthenticationPingAll(t *testing.T) { @@ -1422,3 +1425,490 @@ func TestOIDCExpiryAfterRestart(t *testing.T) { } }, 30*time.Second, 1*time.Second, "validating expiry preservation after restart") } + +// TestOIDCACLPolicyOnJoin validates that ACL policies are correctly applied +// to newly joined OIDC nodes without requiring a client restart. +// +// This test validates the fix for issue #2888: +// https://github.com/juanfont/headscale/issues/2888 +// +// Bug: Nodes joining via OIDC authentication did not get the appropriate ACL +// policy applied until they restarted their client. This was a regression +// introduced in v0.27.0. +// +// The test scenario: +// 1. Creates a CLI user (gateway) with a node advertising a route +// 2. Sets up ACL policy allowing all nodes to access advertised routes +// 3. OIDC user authenticates and joins with a new node +// 4. Verifies that the OIDC user's node IMMEDIATELY sees the advertised route +// +// Expected behavior: +// - Without fix: OIDC node cannot see the route (PrimaryRoutes is nil/empty) +// - With fix: OIDC node immediately sees the route in PrimaryRoutes +// +// Root cause: The buggy code called a.h.Change(c) immediately after user +// creation but BEFORE node registration completed, creating a race condition +// where policy change notifications were sent asynchronously before the node +// was fully registered. +func TestOIDCACLPolicyOnJoin(t *testing.T) { + IntegrationSkip(t) + + gatewayUser := "gateway" + oidcUser := "oidcuser" + + spec := ScenarioSpec{ + NodesPerUser: 1, + Users: []string{gatewayUser}, + OIDCUsers: []mockoidc.MockUser{ + oidcMockUser(oidcUser, true), + }, + } + + scenario, err := NewScenario(spec) + + require.NoError(t, err) + defer scenario.ShutdownAssertNoPanics(t) + + oidcMap := map[string]string{ + "HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(), + "HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(), + "CREDENTIALS_DIRECTORY_TEST": "/tmp", + "HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret", + } + + // Create headscale environment with ACL policy that allows OIDC user + // to access routes advertised by gateway user + err = scenario.CreateHeadscaleEnvWithLoginURL( + []tsic.Option{ + tsic.WithAcceptRoutes(), + }, + hsic.WithTestName("oidcaclpolicy"), + hsic.WithConfigEnv(oidcMap), + hsic.WithTLS(), + hsic.WithFileInContainer("/tmp/hs_client_oidc_secret", []byte(scenario.mockOIDC.ClientSecret())), + hsic.WithACLPolicy( + &policyv2.Policy{ + ACLs: []policyv2.ACL{ + { + Action: "accept", + Sources: []policyv2.Alias{prefixp("100.64.0.0/10")}, + Destinations: []policyv2.AliasWithPorts{ + aliasWithPorts(prefixp("100.64.0.0/10"), tailcfg.PortRangeAny), + aliasWithPorts(prefixp("10.33.0.0/24"), tailcfg.PortRangeAny), + aliasWithPorts(prefixp("10.44.0.0/24"), tailcfg.PortRangeAny), + }, + }, + }, + AutoApprovers: policyv2.AutoApproverPolicy{ + Routes: map[netip.Prefix]policyv2.AutoApprovers{ + netip.MustParsePrefix("10.33.0.0/24"): {usernameApprover("gateway@test.no"), usernameApprover("oidcuser@headscale.net"), usernameApprover("jane.doe@example.com")}, + netip.MustParsePrefix("10.44.0.0/24"): {usernameApprover("gateway@test.no"), usernameApprover("oidcuser@headscale.net"), usernameApprover("jane.doe@example.com")}, + }, + }, + }, + ), + ) + requireNoErrHeadscaleEnv(t, err) + + headscale, err := scenario.Headscale() + require.NoError(t, err) + + // Get the gateway client (CLI user) - only one client at first + allClients, err := scenario.ListTailscaleClients() + requireNoErrListClients(t, err) + require.Len(t, allClients, 1, "Should have exactly 1 client (gateway) before OIDC login") + + gatewayClient := allClients[0] + + // Wait for initial sync (gateway logs in) + err = scenario.WaitForTailscaleSync() + requireNoErrSync(t, err) + + // Gateway advertises route 10.33.0.0/24 + advertiseRoute := "10.33.0.0/24" + command := []string{ + "tailscale", + "set", + "--advertise-routes=" + advertiseRoute, + } + _, _, err = gatewayClient.Execute(command) + require.NoErrorf(t, err, "failed to advertise route: %s", err) + + // Wait for route advertisement to propagate + var gatewayNodeID uint64 + + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(ct, err) + assert.Len(ct, nodes, 1) + + gatewayNode := nodes[0] + gatewayNodeID = gatewayNode.GetId() + assert.Len(ct, gatewayNode.GetAvailableRoutes(), 1) + assert.Contains(ct, gatewayNode.GetAvailableRoutes(), advertiseRoute) + }, 10*time.Second, 500*time.Millisecond, "route advertisement should propagate to headscale") + + // Approve the advertised route + _, err = headscale.ApproveRoutes( + gatewayNodeID, + []netip.Prefix{netip.MustParsePrefix(advertiseRoute)}, + ) + require.NoError(t, err) + + // Wait for route approval to propagate + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(ct, err) + assert.Len(ct, nodes, 1) + + gatewayNode := nodes[0] + assert.Len(ct, gatewayNode.GetApprovedRoutes(), 1) + assert.Contains(ct, gatewayNode.GetApprovedRoutes(), advertiseRoute) + }, 10*time.Second, 500*time.Millisecond, "route approval should propagate to headscale") + + // NOW create the OIDC user by having them join + // This is where issue #2888 manifests - the new OIDC node should immediately + // see the gateway's advertised route + t.Logf("OIDC user joining at %s", time.Now().Format(TimestampFormat)) + + // Create OIDC user's tailscale node + oidcAdvertiseRoute := "10.44.0.0/24" + oidcClient, err := scenario.CreateTailscaleNode( + "head", + tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]), + tsic.WithAcceptRoutes(), + tsic.WithExtraLoginArgs([]string{"--advertise-routes=" + oidcAdvertiseRoute}), + ) + require.NoError(t, err) + + // OIDC login happens automatically via LoginWithURL + loginURL, err := oidcClient.LoginWithURL(headscale.GetEndpoint()) + require.NoError(t, err) + + _, err = doLoginURL(oidcClient.Hostname(), loginURL) + require.NoError(t, err) + + t.Logf("OIDC user logged in successfully at %s", time.Now().Format(TimestampFormat)) + + // THE CRITICAL TEST: Verify that the OIDC user's node can IMMEDIATELY + // see the gateway's advertised route WITHOUT needing a client restart. + // + // This is where the bug manifests: + // - Without fix: PrimaryRoutes will be nil/empty + // - With fix: PrimaryRoutes immediately contains the advertised route + t.Logf("Verifying OIDC user can immediately see advertised routes at %s", time.Now().Format(TimestampFormat)) + + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := oidcClient.Status() + assert.NoError(ct, err) + + // Find the gateway peer in the OIDC user's peer list + var gatewayPeer *ipnstate.PeerStatus + + for _, peerKey := range status.Peers() { + peer := status.Peer[peerKey] + // Gateway is the peer that's not the OIDC user + if peer.UserID != status.Self.UserID { + gatewayPeer = peer + break + } + } + + assert.NotNil(ct, gatewayPeer, "OIDC user should see gateway as peer") + + if gatewayPeer != nil { + // This is the critical assertion - PrimaryRoutes should NOT be nil + assert.NotNil(ct, gatewayPeer.PrimaryRoutes, + "BUG #2888: Gateway peer PrimaryRoutes is nil - ACL policy not applied to new OIDC node!") + + if gatewayPeer.PrimaryRoutes != nil { + routes := gatewayPeer.PrimaryRoutes.AsSlice() + assert.Contains(ct, routes, netip.MustParsePrefix(advertiseRoute), + "OIDC user should immediately see gateway's advertised route %s in PrimaryRoutes", advertiseRoute) + t.Logf("SUCCESS: OIDC user can see advertised route %s in gateway's PrimaryRoutes", advertiseRoute) + } + + // Also verify AllowedIPs includes the route + if gatewayPeer.AllowedIPs != nil && gatewayPeer.AllowedIPs.Len() > 0 { + allowedIPs := gatewayPeer.AllowedIPs.AsSlice() + t.Logf("Gateway peer AllowedIPs: %v", allowedIPs) + } + } + }, 15*time.Second, 500*time.Millisecond, + "OIDC user should immediately see gateway's advertised route without client restart (issue #2888)") + + // Verify that the Gateway node sees the OIDC node's advertised route (AutoApproveRoutes check) + t.Logf("Verifying Gateway user can immediately see OIDC advertised routes at %s", time.Now().Format(TimestampFormat)) + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := gatewayClient.Status() + assert.NoError(ct, err) + + // Find the OIDC peer in the Gateway user's peer list + var oidcPeer *ipnstate.PeerStatus + + for _, peerKey := range status.Peers() { + peer := status.Peer[peerKey] + if peer.UserID != status.Self.UserID { + oidcPeer = peer + break + } + } + + assert.NotNil(ct, oidcPeer, "Gateway user should see OIDC user as peer") + + if oidcPeer != nil { + assert.NotNil(ct, oidcPeer.PrimaryRoutes, + "BUG: OIDC peer PrimaryRoutes is nil - AutoApproveRoutes failed or overwritten!") + + if oidcPeer.PrimaryRoutes != nil { + routes := oidcPeer.PrimaryRoutes.AsSlice() + assert.Contains(ct, routes, netip.MustParsePrefix(oidcAdvertiseRoute), + "Gateway user should immediately see OIDC's advertised route %s in PrimaryRoutes", oidcAdvertiseRoute) + } + } + }, 15*time.Second, 500*time.Millisecond, + "Gateway user should immediately see OIDC's advertised route (AutoApproveRoutes check)") + + // Additional validation: Verify nodes in headscale match expectations + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(ct, err) + assert.Len(ct, nodes, 2, "Should have 2 nodes (gateway + oidcuser)") + + // Verify OIDC user was created correctly + users, err := headscale.ListUsers() + assert.NoError(ct, err) + // Note: mockoidc may create additional default users (like jane.doe) + // so we check for at least 2 users, not exactly 2 + assert.GreaterOrEqual(ct, len(users), 2, "Should have at least 2 users (gateway CLI user + oidcuser)") + + // Find gateway CLI user + var gatewayUser *v1.User + + for _, user := range users { + if user.GetName() == "gateway" && user.GetProvider() == "" { + gatewayUser = user + break + } + } + + assert.NotNil(ct, gatewayUser, "Should have gateway CLI user") + + if gatewayUser != nil { + assert.Equal(ct, "gateway", gatewayUser.GetName()) + } + + // Find OIDC user + var oidcUserFound *v1.User + + for _, user := range users { + if user.GetName() == "oidcuser" && user.GetProvider() == "oidc" { + oidcUserFound = user + break + } + } + + assert.NotNil(ct, oidcUserFound, "Should have OIDC user") + + if oidcUserFound != nil { + assert.Equal(ct, "oidcuser", oidcUserFound.GetName()) + assert.Equal(ct, "oidcuser@headscale.net", oidcUserFound.GetEmail()) + } + }, 10*time.Second, 500*time.Millisecond, "headscale should have correct users and nodes") + + t.Logf("Test completed successfully - issue #2888 fix validated") +} + +// TestOIDCReloginSameUserRoutesPreserved tests the scenario where: +// - A node logs in via OIDC and advertises routes +// - Routes are auto-approved and verified as SERVING +// - The node logs out +// - The node logs back in as the same user +// - Routes should STILL be SERVING (not just approved/available) +// +// This test validates the fix for issue #2896: +// https://github.com/juanfont/headscale/issues/2896 +// +// Bug: When a node with already-approved routes restarts/re-authenticates, +// the routes show as "Approved" and "Available" but NOT "Serving" (Primary). +// A headscale restart would fix it, indicating a state management issue. +func TestOIDCReloginSameUserRoutesPreserved(t *testing.T) { + IntegrationSkip(t) + + advertiseRoute := "10.55.0.0/24" + + // Create scenario with same user for both login attempts + scenario, err := NewScenario(ScenarioSpec{ + OIDCUsers: []mockoidc.MockUser{ + oidcMockUser("user1", true), // Initial login + oidcMockUser("user1", true), // Relogin with same user + }, + }) + require.NoError(t, err) + defer scenario.ShutdownAssertNoPanics(t) + + oidcMap := map[string]string{ + "HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(), + "HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(), + "CREDENTIALS_DIRECTORY_TEST": "/tmp", + "HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret", + } + + err = scenario.CreateHeadscaleEnvWithLoginURL( + []tsic.Option{ + tsic.WithAcceptRoutes(), + }, + hsic.WithTestName("oidcrouterelogin"), + hsic.WithConfigEnv(oidcMap), + hsic.WithTLS(), + hsic.WithFileInContainer("/tmp/hs_client_oidc_secret", []byte(scenario.mockOIDC.ClientSecret())), + hsic.WithEmbeddedDERPServerOnly(), + hsic.WithDERPAsIP(), + hsic.WithACLPolicy( + &policyv2.Policy{ + ACLs: []policyv2.ACL{ + { + Action: "accept", + Sources: []policyv2.Alias{policyv2.Wildcard}, + Destinations: []policyv2.AliasWithPorts{{Alias: policyv2.Wildcard, Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}}}, + }, + }, + AutoApprovers: policyv2.AutoApproverPolicy{ + Routes: map[netip.Prefix]policyv2.AutoApprovers{ + netip.MustParsePrefix(advertiseRoute): {usernameApprover("user1@headscale.net")}, + }, + }, + }, + ), + ) + requireNoErrHeadscaleEnv(t, err) + + headscale, err := scenario.Headscale() + require.NoError(t, err) + + // Create client with route advertisement + ts, err := scenario.CreateTailscaleNode( + "unstable", + tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]), + tsic.WithAcceptRoutes(), + tsic.WithExtraLoginArgs([]string{"--advertise-routes=" + advertiseRoute}), + ) + require.NoError(t, err) + + // Initial login as user1 + u, err := ts.LoginWithURL(headscale.GetEndpoint()) + require.NoError(t, err) + + _, err = doLoginURL(ts.Hostname(), u) + require.NoError(t, err) + + // Wait for client to be running + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := ts.Status() + assert.NoError(ct, err) + assert.Equal(ct, "Running", status.BackendState) + }, 30*time.Second, 1*time.Second, "waiting for initial login to complete") + + // Step 1: Verify initial route is advertised, approved, and SERVING + t.Logf("Step 1: Verifying initial route is advertised, approved, and SERVING at %s", time.Now().Format(TimestampFormat)) + + var initialNode *v1.Node + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Should have exactly 1 node") + + if len(nodes) == 1 { + initialNode = nodes[0] + // Check: 1 announced, 1 approved, 1 serving (subnet route) + assert.Lenf(c, initialNode.GetAvailableRoutes(), 1, + "Node should have 1 available route, got %v", initialNode.GetAvailableRoutes()) + assert.Lenf(c, initialNode.GetApprovedRoutes(), 1, + "Node should have 1 approved route, got %v", initialNode.GetApprovedRoutes()) + assert.Lenf(c, initialNode.GetSubnetRoutes(), 1, + "Node should have 1 serving (subnet) route, got %v - THIS IS THE BUG if empty", initialNode.GetSubnetRoutes()) + assert.Contains(c, initialNode.GetSubnetRoutes(), advertiseRoute, + "Subnet routes should contain %s", advertiseRoute) + } + }, 30*time.Second, 500*time.Millisecond, "initial route should be serving") + + require.NotNil(t, initialNode, "Initial node should be found") + initialNodeID := initialNode.GetId() + t.Logf("Initial node ID: %d, Available: %v, Approved: %v, Serving: %v", + initialNodeID, initialNode.GetAvailableRoutes(), initialNode.GetApprovedRoutes(), initialNode.GetSubnetRoutes()) + + // Step 2: Logout + t.Logf("Step 2: Logging out at %s", time.Now().Format(TimestampFormat)) + + err = ts.Logout() + require.NoError(t, err) + + // Wait for logout to complete + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := ts.Status() + assert.NoError(ct, err) + assert.Equal(ct, "NeedsLogin", status.BackendState, "Expected NeedsLogin state after logout") + }, 30*time.Second, 1*time.Second, "waiting for logout to complete") + + t.Logf("Logout completed, node should still exist in database") + + // Verify node still exists (routes should still be in DB) + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Node should persist in database after logout") + }, 10*time.Second, 500*time.Millisecond, "node should persist after logout") + + // Step 3: Re-authenticate via OIDC as the same user + t.Logf("Step 3: Re-authenticating with same user via OIDC at %s", time.Now().Format(TimestampFormat)) + + u, err = ts.LoginWithURL(headscale.GetEndpoint()) + require.NoError(t, err) + + _, err = doLoginURL(ts.Hostname(), u) + require.NoError(t, err) + + // Wait for client to be running + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := ts.Status() + assert.NoError(ct, err) + assert.Equal(ct, "Running", status.BackendState, "Expected Running state after relogin") + }, 30*time.Second, 1*time.Second, "waiting for relogin to complete") + + t.Logf("Re-authentication completed at %s", time.Now().Format(TimestampFormat)) + + // Step 4: THE CRITICAL TEST - Verify routes are STILL SERVING after re-authentication + t.Logf("Step 4: Verifying routes are STILL SERVING after re-authentication at %s", time.Now().Format(TimestampFormat)) + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "Should still have exactly 1 node after relogin") + + if len(nodes) == 1 { + node := nodes[0] + t.Logf("After relogin - Available: %v, Approved: %v, Serving: %v", + node.GetAvailableRoutes(), node.GetApprovedRoutes(), node.GetSubnetRoutes()) + + // This is where issue #2896 manifests: + // - Available shows the route (from Hostinfo.RoutableIPs) + // - Approved shows the route (from ApprovedRoutes) + // - BUT Serving (SubnetRoutes/PrimaryRoutes) is EMPTY! + assert.Lenf(c, node.GetAvailableRoutes(), 1, + "Node should have 1 available route after relogin, got %v", node.GetAvailableRoutes()) + assert.Lenf(c, node.GetApprovedRoutes(), 1, + "Node should have 1 approved route after relogin, got %v", node.GetApprovedRoutes()) + assert.Lenf(c, node.GetSubnetRoutes(), 1, + "BUG #2896: Node should have 1 SERVING route after relogin, got %v", node.GetSubnetRoutes()) + assert.Contains(c, node.GetSubnetRoutes(), advertiseRoute, + "BUG #2896: Subnet routes should contain %s after relogin", advertiseRoute) + + // Also verify node ID was preserved (same node, not new registration) + assert.Equal(c, initialNodeID, node.GetId(), + "Node ID should be preserved after same-user relogin") + } + }, 30*time.Second, 500*time.Millisecond, + "BUG #2896: routes should remain SERVING after OIDC logout/relogin with same user") + + t.Logf("Test completed - verifying issue #2896 fix for OIDC") +} diff --git a/integration/route_test.go b/integration/route_test.go index 18abc9a8..39292806 100644 --- a/integration/route_test.go +++ b/integration/route_test.go @@ -2228,9 +2228,9 @@ func TestAutoApproveMultiNetwork(t *testing.T) { // - Both policy modes (database, file) // - Both advertiseDuringUp values (true, false) minimalTestSet := map[string]bool{ - "authkey-tag-advertiseduringup-false-pol-database": true, // authkey + database + tag + false - "webauth-user-advertiseduringup-true-pol-file": true, // webauth + file + user + true - "authkey-group-advertiseduringup-false-pol-file": true, // authkey + file + group + false + "authkey-tag-advertiseduringup-false-pol-database": true, // authkey + database + tag + false + "webauth-user-advertiseduringup-true-pol-file": true, // webauth + file + user + true + "authkey-group-advertiseduringup-false-pol-file": true, // authkey + file + group + false } for _, tt := range tests { @@ -2324,7 +2324,11 @@ func TestAutoApproveMultiNetwork(t *testing.T) { // into a HA node, which isn't something we are testing here. routerUsernet1, err := scenario.CreateTailscaleNode("head", tsOpts...) require.NoError(t, err) - defer routerUsernet1.Shutdown() + + defer func() { + _, _, err := routerUsernet1.Shutdown() + require.NoError(t, err) + }() if tt.withURL { u, err := routerUsernet1.LoginWithURL(headscale.GetEndpoint()) @@ -2333,7 +2337,8 @@ func TestAutoApproveMultiNetwork(t *testing.T) { body, err := doLoginURL(routerUsernet1.Hostname(), u) require.NoError(t, err) - scenario.runHeadscaleRegister("user1", body) + err = scenario.runHeadscaleRegister("user1", body) + require.NoError(t, err) // Wait for the client to sync with the server after webauth registration. // Unlike authkey login which blocks until complete, webauth registration @@ -2352,6 +2357,11 @@ func TestAutoApproveMultiNetwork(t *testing.T) { } // extra creation end. + // Wait for the node to be fully running before getting its ID + // This is especially important for webauth flow where login is asynchronous + err = routerUsernet1.WaitForRunning(30 * time.Second) + require.NoError(t, err) + routerUsernet1ID := routerUsernet1.MustID() web := services[0] @@ -2739,16 +2749,6 @@ func TestAutoApproveMultiNetwork(t *testing.T) { } } -func assertTracerouteViaIP(t *testing.T, tr util.Traceroute, ip netip.Addr) { - t.Helper() - - require.NotNil(t, tr) - require.True(t, tr.Success) - require.NoError(t, tr.Err) - require.NotEmpty(t, tr.Route) - require.Equal(t, tr.Route[0].IP, ip) -} - // assertTracerouteViaIPWithCollect is a version of assertTracerouteViaIP that works with assert.CollectT. func assertTracerouteViaIPWithCollect(c *assert.CollectT, tr util.Traceroute, ip netip.Addr) { assert.NotNil(c, tr) @@ -2762,30 +2762,6 @@ func assertTracerouteViaIPWithCollect(c *assert.CollectT, tr util.Traceroute, ip } } -// requirePeerSubnetRoutes asserts that the peer has the expected subnet routes. -func requirePeerSubnetRoutes(t *testing.T, status *ipnstate.PeerStatus, expected []netip.Prefix) { - t.Helper() - if status.AllowedIPs.Len() <= 2 && len(expected) != 0 { - t.Fatalf("peer %s (%s) has no subnet routes, expected %v", status.HostName, status.ID, expected) - return - } - - if len(expected) == 0 { - expected = []netip.Prefix{} - } - - got := slicesx.Filter(nil, status.AllowedIPs.AsSlice(), func(p netip.Prefix) bool { - if tsaddr.IsExitRoute(p) { - return true - } - return !slices.ContainsFunc(status.TailscaleIPs, p.Contains) - }) - - if diff := cmpdiff.Diff(expected, got, util.PrefixComparer, cmpopts.EquateEmpty()); diff != "" { - t.Fatalf("peer %s (%s) subnet routes, unexpected result (-want +got):\n%s", status.HostName, status.ID, diff) - } -} - func SortPeerStatus(a, b *ipnstate.PeerStatus) int { return cmp.Compare(a.ID, b.ID) } @@ -2830,13 +2806,6 @@ func requirePeerSubnetRoutesWithCollect(c *assert.CollectT, status *ipnstate.Pee } } -func requireNodeRouteCount(t *testing.T, node *v1.Node, announced, approved, subnet int) { - t.Helper() - require.Lenf(t, node.GetAvailableRoutes(), announced, "expected %q announced routes(%v) to have %d route, had %d", node.GetName(), node.GetAvailableRoutes(), announced, len(node.GetAvailableRoutes())) - require.Lenf(t, node.GetApprovedRoutes(), approved, "expected %q approved routes(%v) to have %d route, had %d", node.GetName(), node.GetApprovedRoutes(), approved, len(node.GetApprovedRoutes())) - require.Lenf(t, node.GetSubnetRoutes(), subnet, "expected %q subnet routes(%v) to have %d route, had %d", node.GetName(), node.GetSubnetRoutes(), subnet, len(node.GetSubnetRoutes())) -} - func requireNodeRouteCountWithCollect(c *assert.CollectT, node *v1.Node, announced, approved, subnet int) { assert.Lenf(c, node.GetAvailableRoutes(), announced, "expected %q announced routes(%v) to have %d route, had %d", node.GetName(), node.GetAvailableRoutes(), announced, len(node.GetAvailableRoutes())) assert.Lenf(c, node.GetApprovedRoutes(), approved, "expected %q approved routes(%v) to have %d route, had %d", node.GetName(), node.GetApprovedRoutes(), approved, len(node.GetApprovedRoutes()))