From 8a51bd3c64a9f3dae844ce7589fba251ebc1755c Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 11 Mar 2025 18:08:47 +0100 Subject: [PATCH] fix ups Signed-off-by: Kristoffer Dalby --- .../workflows/test-integration-policyv2.yaml | 1 - .github/workflows/test-integration.yaml | 1 - integration/acl_test.go | 1 + integration/auth_oidc_test.go | 16 +++++------ integration/cli_test.go | 24 +++++++++++------ integration/general_test.go | 4 +-- integration/scenario.go | 27 ++++++++++--------- integration/scenario_test.go | 3 ++- integration/tsic/tsic.go | 5 ++++ 9 files changed, 48 insertions(+), 34 deletions(-) diff --git a/.github/workflows/test-integration-policyv2.yaml b/.github/workflows/test-integration-policyv2.yaml index 73015603..9a26176f 100644 --- a/.github/workflows/test-integration-policyv2.yaml +++ b/.github/workflows/test-integration-policyv2.yaml @@ -71,7 +71,6 @@ jobs: - TestSubnetRouteACL - TestEnablingExitRoutes - TestHeadscale - - TestCreateTailscale - TestTailscaleNodesJoiningHeadcale - TestSSHOneUserToAll - TestSSHMultipleUsersAllToAll diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 2898b4ba..db817ff1 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -71,7 +71,6 @@ jobs: - TestSubnetRouteACL - TestEnablingExitRoutes - TestHeadscale - - TestCreateTailscale - TestTailscaleNodesJoiningHeadcale - TestSSHOneUserToAll - TestSSHMultipleUsersAllToAll diff --git a/integration/acl_test.go b/integration/acl_test.go index 7d1186f5..d1bf0342 100644 --- a/integration/acl_test.go +++ b/integration/acl_test.go @@ -931,6 +931,7 @@ func TestACLDevice1CanAccessDevice2(t *testing.T) { for name, testCase := range tests { t.Run(name, func(t *testing.T) { scenario := aclScenario(t, &testCase.policy, 1) + defer scenario.ShutdownAssertNoPanics(t) test1ip := netip.MustParseAddr("100.64.0.1") test1ip6 := netip.MustParseAddr("fd7a:115c:a1e0::1") diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index 33b9ffbc..c86138a8 100644 --- a/integration/auth_oidc_test.go +++ b/integration/auth_oidc_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "maps" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/juanfont/headscale/gen/go/headscale/v1" @@ -128,6 +130,7 @@ func TestOIDCExpireNodesBasedOnTokenExpiry(t *testing.T) { oidcMockUser("user1", true), oidcMockUser("user2", false), }, + OIDCAccessTTL: shortAccessTTL, } scenario, err := NewScenario(spec) @@ -295,24 +298,21 @@ func TestOIDC024UserCreation(t *testing.T) { spec.Users = append(spec.Users, user) } - scenario, err := NewScenario(spec) - assertNoErr(t, err) - defer scenario.ShutdownAssertNoPanics(t) - for _, user := range tt.oidcUsers { spec.OIDCUsers = append(spec.OIDCUsers, oidcMockUser(user, tt.emailVerified)) } + scenario, err := NewScenario(spec) + assertNoErr(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", } - - for k, v := range tt.config { - oidcMap[k] = v - } + maps.Copy(oidcMap, tt.config) err = scenario.CreateHeadscaleEnvWithLoginURL( nil, diff --git a/integration/cli_test.go b/integration/cli_test.go index c1b738fc..85b20702 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -528,7 +528,8 @@ func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) { user2 := "user2" spec := ScenarioSpec{ - Users: []string{user1, user2}, + NodesPerUser: 1, + Users: []string{user1}, } scenario, err := NewScenario(spec) @@ -546,6 +547,9 @@ func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) { headscale, err := scenario.Headscale() assertNoErr(t, err) + err = headscale.CreateUser(user2) + assertNoErr(t, err) + var user2Key v1.PreAuthKey err = executeAndUnmarshal( @@ -568,10 +572,15 @@ func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) { ) assertNoErr(t, err) + listNodes, err := headscale.ListNodes() + require.Nil(t, err) + require.Len(t, listNodes, 1) + assert.Equal(t, user1, listNodes[0].GetUser().GetName()) + allClients, err := scenario.ListTailscaleClients() assertNoErrListClients(t, err) - assert.Len(t, allClients, 1) + require.Len(t, allClients, 1) client := allClients[0] @@ -601,12 +610,11 @@ func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) { t.Fatalf("expected node to be logged in as userid:2, got: %s", status.Self.UserID.String()) } - listNodes, err := headscale.ListNodes() - assert.Nil(t, err) - assert.Len(t, listNodes, 2) - - assert.Equal(t, "user1", listNodes[0].GetUser().GetName()) - assert.Equal(t, "user2", listNodes[1].GetUser().GetName()) + listNodes, err = headscale.ListNodes() + require.Nil(t, err) + require.Len(t, listNodes, 2) + assert.Equal(t, user1, listNodes[0].GetUser().GetName()) + assert.Equal(t, user2, listNodes[1].GetUser().GetName()) } func TestApiKeyCommand(t *testing.T) { diff --git a/integration/general_test.go b/integration/general_test.go index ff22fd8f..0b55f0b7 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -138,7 +138,7 @@ func testEphemeralWithOptions(t *testing.T, opts ...hsic.Option) { t.Fatalf("failed to create user %s: %s", userName, err) } - err = scenario.CreateTailscaleNodesInUser(userName, "all", spec.NodesPerUser, []tsic.Option{}...) + err = scenario.CreateTailscaleNodesInUser(userName, "all", spec.NodesPerUser, tsic.WithNetwork(scenario.networks[TestDefaultNetwork])) if err != nil { t.Fatalf("failed to create tailscale nodes in user %s: %s", userName, err) } @@ -216,7 +216,7 @@ func TestEphemeral2006DeletedTooQuickly(t *testing.T) { t.Fatalf("failed to create user %s: %s", userName, err) } - err = scenario.CreateTailscaleNodesInUser(userName, "all", spec.NodesPerUser, []tsic.Option{}...) + err = scenario.CreateTailscaleNodesInUser(userName, "all", spec.NodesPerUser, tsic.WithNetwork(scenario.networks[TestDefaultNetwork])) if err != nil { t.Fatalf("failed to create tailscale nodes in user %s: %s", userName, err) } diff --git a/integration/scenario.go b/integration/scenario.go index 9825f0dd..d3816e10 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -130,7 +130,13 @@ type ScenarioSpec struct { // OIDCUsers, if populated, will start a Mock OIDC server and populate // the user login stack with the given users. - OIDCUsers []mockoidc.MockUser + // If the NodesPerUser is set, it should align with this list to ensure + // the correct users are logged in. + // This is because the MockOIDC server can only serve login + // requests based on a queue it has been given on startup. + // We currently only populates it with one login request per user. + OIDCUsers []mockoidc.MockUser + OIDCAccessTTL time.Duration MaxWait time.Duration } @@ -186,7 +192,11 @@ func NewScenario(spec ScenarioSpec) (*Scenario, error) { s.userToNetwork = userToNetwork if spec.OIDCUsers != nil && len(spec.OIDCUsers) != 0 { - err = s.runMockOIDC(defaultAccessTTL, spec.OIDCUsers) + ttl := defaultAccessTTL + if spec.OIDCAccessTTL != 0 { + ttl = spec.OIDCAccessTTL + } + err = s.runMockOIDC(ttl, spec.OIDCUsers) if err != nil { return nil, err } @@ -436,7 +446,7 @@ func (s *Scenario) CreateTailscaleNodesInUser( ) error { if user, ok := s.users[userStr]; ok { var versions []string - for i := 0; i < count; i++ { + for i := range count { version := requestedVersion if requestedVersion == "all" { version = MustTestVersions[i%len(MustTestVersions)] @@ -468,8 +478,7 @@ func (s *Scenario) CreateTailscaleNodesInUser( s.mu.Unlock() if err != nil { return fmt.Errorf( - "failed to create tailscale (%s) node: %w", - tsClient.Hostname(), + "failed to create tailscale node: %w", err, ) } @@ -608,14 +617,6 @@ func (s *Scenario) createHeadscaleEnv( return err } - if s.spec.OIDCUsers != nil && s.spec.NodesPerUser != 1 { - // OIDC scenario only supports one client per user. - // This is because the MockOIDC server can only serve login - // requests based on a queue it has been given on startup. - // We currently only populates it with one login request per user. - return fmt.Errorf("client count must be 1 for OIDC scenario.") - } - sort.Strings(s.spec.Users) for _, user := range s.spec.Users { err = s.CreateUser(user) diff --git a/integration/scenario_test.go b/integration/scenario_test.go index 1dfa4bf2..7f34fa77 100644 --- a/integration/scenario_test.go +++ b/integration/scenario_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/juanfont/headscale/integration/dockertestutil" + "github.com/juanfont/headscale/integration/tsic" ) // This file is intended to "test the test framework", by proxy it will also test @@ -110,7 +111,7 @@ func TestTailscaleNodesJoiningHeadcale(t *testing.T) { }) t.Run("create-tailscale", func(t *testing.T) { - err := scenario.CreateTailscaleNodesInUser(user, "unstable", count) + err := scenario.CreateTailscaleNodesInUser(user, "unstable", count, tsic.WithNetwork(scenario.networks[TestDefaultNetwork])) if err != nil { t.Fatalf("failed to add tailscale nodes: %s", err) } diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 80bec2c1..b60393f7 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "reflect" + "runtime/debug" "strconv" "strings" "time" @@ -226,6 +227,10 @@ func New( opt(tsic) } + if tsic.network == nil { + return nil, fmt.Errorf("no network set, called from: \n%s", string(debug.Stack())) + } + tailscaleOptions := &dockertest.RunOptions{ Name: hostname, Networks: []*dockertest.Network{tsic.network},