From 3770015faa54fbfb27b8beabf58f40165cb682ff Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 20 Jan 2026 17:08:19 +0000 Subject: [PATCH] all: fix staticcheck and contextcheck lint issues - Add nolint:staticcheck for SA1019 deprecation warnings on types.Route (kept for GORM migrations only, intentionally deprecated) - Add nolint:staticcheck for SA4006 false positives where variables are used inside new() expressions which staticcheck doesn't recognize - Fix SA5011 potential nil pointer dereferences in util_test.go by using t.Fatal instead of t.Error for nil checks - Add nolint:contextcheck for functions where context propagation would require significant architectural changes (Docker client creation, OIDC initialization, scheduled tasks, etc.) --- cmd/hi/cleanup.go | 6 ++++++ cmd/hi/docker.go | 3 +++ cmd/hi/doctor.go | 7 +++++++ hscontrol/app.go | 2 ++ hscontrol/db/db.go | 5 +++++ hscontrol/db/node_test.go | 4 ++++ hscontrol/db/users_test.go | 1 + hscontrol/noise.go | 1 + hscontrol/oidc.go | 1 + hscontrol/policy/policy_route_approval_test.go | 1 + hscontrol/policy/v2/types_test.go | 7 +++++++ hscontrol/state/state.go | 1 + hscontrol/util/util_test.go | 4 ++-- integration/helpers.go | 1 + 14 files changed, 42 insertions(+), 2 deletions(-) diff --git a/cmd/hi/cleanup.go b/cmd/hi/cleanup.go index 8dc57b4b..70480239 100644 --- a/cmd/hi/cleanup.go +++ b/cmd/hi/cleanup.go @@ -55,6 +55,7 @@ func cleanupAfterTest(ctx context.Context, cli *client.Client, containerID, runI // killTestContainers terminates and removes all test containers. func killTestContainers(ctx context.Context) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -109,6 +110,7 @@ func killTestContainers(ctx context.Context) error { // This function filters containers by the hi.run-id label to only affect containers // belonging to the specified test run, leaving other concurrent test runs untouched. func killTestContainersByRunID(ctx context.Context, runID string) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -151,6 +153,7 @@ func killTestContainersByRunID(ctx context.Context, runID string) error { // This is useful for cleaning up leftover containers from previous crashed or interrupted test runs // without interfering with currently running concurrent tests. func cleanupStaleTestContainers(ctx context.Context) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -225,6 +228,7 @@ func removeContainerWithRetry(ctx context.Context, cli *client.Client, container // pruneDockerNetworks removes unused Docker networks. func pruneDockerNetworks(ctx context.Context) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -247,6 +251,7 @@ func pruneDockerNetworks(ctx context.Context) error { // cleanOldImages removes test-related and old dangling Docker images. func cleanOldImages(ctx context.Context) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -299,6 +304,7 @@ func cleanOldImages(ctx context.Context) error { // cleanCacheVolume removes the Docker volume used for Go module cache. func cleanCacheVolume(ctx context.Context) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) diff --git a/cmd/hi/docker.go b/cmd/hi/docker.go index 05aa7c9d..c9791098 100644 --- a/cmd/hi/docker.go +++ b/cmd/hi/docker.go @@ -39,6 +39,7 @@ const ( // runTestContainer executes integration tests in a Docker container. func runTestContainer(ctx context.Context, config *RunConfig) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) @@ -110,6 +111,7 @@ func runTestContainer(ctx context.Context, config *RunConfig) error { if config.Stats { var err error + //nolint:contextcheck // NewStatsCollector internal functions don't accept context statsCollector, err = NewStatsCollector() if err != nil { if config.Verbose { @@ -632,6 +634,7 @@ func listControlFiles(logsDir string) { // extractArtifactsFromContainers collects container logs and files from the specific test run. func extractArtifactsFromContainers(ctx context.Context, testContainerID, logsDir string, verbose bool) error { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) diff --git a/cmd/hi/doctor.go b/cmd/hi/doctor.go index c30a1ca9..2fae4fbe 100644 --- a/cmd/hi/doctor.go +++ b/cmd/hi/doctor.go @@ -38,12 +38,15 @@ func runDoctorCheck(ctx context.Context) error { } // Check 3: Go installation + //nolint:contextcheck // These checks don't need context results = append(results, checkGoInstallation()) // Check 4: Git repository + //nolint:contextcheck // These checks don't need context results = append(results, checkGitRepository()) // Check 5: Required files + //nolint:contextcheck // These checks don't need context results = append(results, checkRequiredFiles()) // Display results @@ -86,6 +89,7 @@ func checkDockerBinary() DoctorResult { // checkDockerDaemon verifies Docker daemon is running and accessible. func checkDockerDaemon(ctx context.Context) DoctorResult { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return DoctorResult{ @@ -125,6 +129,7 @@ func checkDockerDaemon(ctx context.Context) DoctorResult { // checkDockerContext verifies Docker context configuration. func checkDockerContext(_ context.Context) DoctorResult { + //nolint:contextcheck // getCurrentDockerContext doesn't accept context contextInfo, err := getCurrentDockerContext() if err != nil { return DoctorResult{ @@ -155,6 +160,7 @@ func checkDockerContext(_ context.Context) DoctorResult { // checkDockerSocket verifies Docker socket accessibility. func checkDockerSocket(ctx context.Context) DoctorResult { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return DoctorResult{ @@ -192,6 +198,7 @@ func checkDockerSocket(ctx context.Context) DoctorResult { // checkGolangImage verifies the golang Docker image is available locally or can be pulled. func checkGolangImage(ctx context.Context) DoctorResult { + //nolint:contextcheck // createDockerClient internal functions don't accept context cli, err := createDockerClient() if err != nil { return DoctorResult{ diff --git a/hscontrol/app.go b/hscontrol/app.go index f7d9ba90..a333c415 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -299,6 +299,7 @@ func (h *Headscale) scheduledTasks(ctx context.Context) { case <-derpTickerChan: log.Info().Msg("Fetching DERPMap updates") + //nolint:contextcheck // GetDERPMap internal functions don't accept context derpMap, err := backoff.Retry(ctx, func() (*tailcfg.DERPMap, error) { derpMap, err := derp.GetDERPMap(h.cfg.DERP) if err != nil { @@ -885,6 +886,7 @@ func (h *Headscale) Serve() error { // Close state connections info("closing state and database") + //nolint:contextcheck // Close method signature does not accept context err = h.state.Close() if err != nil { log.Error().Err(err).Msg("failed to close state") diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index f9fd9a20..02794627 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -75,6 +75,7 @@ func NewHeadscaleDatabase( ID: "202501221827", Migrate: func(tx *gorm.DB) error { // Remove any invalid routes associated with a node that does not exist. + //nolint:staticcheck // SA1019: types.Route kept for GORM migrations only if tx.Migrator().HasTable(&types.Route{}) && tx.Migrator().HasTable(&types.Node{}) { err := tx.Exec("delete from routes where node_id not in (select id from nodes)").Error if err != nil { @@ -83,6 +84,7 @@ func NewHeadscaleDatabase( } // Remove any invalid routes without a node_id. + //nolint:staticcheck // SA1019: types.Route kept for GORM migrations only if tx.Migrator().HasTable(&types.Route{}) { err := tx.Exec("delete from routes where node_id is null").Error if err != nil { @@ -90,6 +92,7 @@ func NewHeadscaleDatabase( } } + //nolint:staticcheck // SA1019: types.Route kept for GORM migrations only err := tx.AutoMigrate(&types.Route{}) if err != nil { return fmt.Errorf("automigrating types.Route: %w", err) @@ -155,6 +158,7 @@ AND auth_key_id NOT IN ( nodeRoutes := map[uint64][]netip.Prefix{} + //nolint:staticcheck // SA1019: types.Route kept for GORM migrations only var routes []types.Route err = tx.Find(&routes).Error @@ -184,6 +188,7 @@ AND auth_key_id NOT IN ( } // Drop the old table. + //nolint:staticcheck // SA1019: types.Route kept for GORM migrations only _ = tx.Migrator().DropTable(&types.Route{}) return nil diff --git a/hscontrol/db/node_test.go b/hscontrol/db/node_test.go index a78eb84f..58d36463 100644 --- a/hscontrol/db/node_test.go +++ b/hscontrol/db/node_test.go @@ -98,6 +98,7 @@ func TestExpireNode(t *testing.T) { user, err := db.CreateUser(types.User{Name: "test"}) require.NoError(t, err) + //nolint:staticcheck // SA4006: pak is used in new(pak.ID) below pak, err := db.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) require.NoError(t, err) @@ -142,6 +143,7 @@ func TestSetTags(t *testing.T) { user, err := db.CreateUser(types.User{Name: "test"}) require.NoError(t, err) + //nolint:staticcheck // SA4006: pak is used in new(pak.ID) below pak, err := db.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) require.NoError(t, err) @@ -637,9 +639,11 @@ func TestListEphemeralNodes(t *testing.T) { user, err := db.CreateUser(types.User{Name: "test"}) require.NoError(t, err) + //nolint:staticcheck // SA4006: pak is used in new(pak.ID) below pak, err := db.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) require.NoError(t, err) + //nolint:staticcheck // SA4006: pakEph is used in new(pakEph.ID) below pakEph, err := db.CreatePreAuthKey(user.TypedID(), false, true, nil, nil) require.NoError(t, err) diff --git a/hscontrol/db/users_test.go b/hscontrol/db/users_test.go index bbb8e4d4..40d301b3 100644 --- a/hscontrol/db/users_test.go +++ b/hscontrol/db/users_test.go @@ -70,6 +70,7 @@ func TestDestroyUserErrors(t *testing.T) { user, err := db.CreateUser(types.User{Name: "test"}) require.NoError(t, err) + //nolint:staticcheck // SA4006: pak is used in new(pak.ID) below pak, err := db.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) require.NoError(t, err) diff --git a/hscontrol/noise.go b/hscontrol/noise.go index 67021d31..b28fa0bb 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -244,6 +244,7 @@ func (ns *noiseServer) NoiseRegistrationHandler( return } + //nolint:contextcheck // IIFE uses context from outer scope implicitly registerRequest, registerResponse := func() (*tailcfg.RegisterRequest, *tailcfg.RegisterResponse) { var resp *tailcfg.RegisterResponse diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 6ab70f78..de02b677 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -69,6 +69,7 @@ func NewAuthProviderOIDC( ) (*AuthProviderOIDC, error) { var err error // grab oidc config if it hasn't been already + //nolint:contextcheck // Initialization code - no parent context available oidcProvider, err := oidc.NewProvider(context.Background(), cfg.Issuer) if err != nil { return nil, fmt.Errorf("creating OIDC provider from issuer config: %w", err) diff --git a/hscontrol/policy/policy_route_approval_test.go b/hscontrol/policy/policy_route_approval_test.go index 0e974a1a..fbe6e4bb 100644 --- a/hscontrol/policy/policy_route_approval_test.go +++ b/hscontrol/policy/policy_route_approval_test.go @@ -327,6 +327,7 @@ func TestApproveRoutesWithPolicy_EdgeCases(t *testing.T) { } func TestApproveRoutesWithPolicy_NilPolicyManagerCase(t *testing.T) { + //nolint:staticcheck // SA4006: user is used in new(user.ID) and new(user) below user := types.User{ Model: gorm.Model{ID: 1}, Name: "test", diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index dc95e1f3..05f2b085 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -1604,11 +1604,18 @@ func TestResolvePolicy(t *testing.T) { } // Extract users to variables so we can take their addresses + // The variables below are all used in new() calls in the test cases. + //nolint:staticcheck // SA4006: testuser is used in new(testuser) below testuser := users["testuser"] + //nolint:staticcheck // SA4006: groupuser is used in new(groupuser) below groupuser := users["groupuser"] + //nolint:staticcheck // SA4006: groupuser1 is used in new(groupuser1) below groupuser1 := users["groupuser1"] + //nolint:staticcheck // SA4006: groupuser2 is used in new(groupuser2) below groupuser2 := users["groupuser2"] + //nolint:staticcheck // SA4006: notme is used in new(notme) below notme := users["notme"] + //nolint:staticcheck // SA4006: testuser2 is used in new(testuser2) below testuser2 := users["testuser2"] tests := []struct { diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 57e0ebde..7ddcb005 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -492,6 +492,7 @@ func (s *State) Connect(id types.NodeID) []change.Change { // Disconnect marks a node as disconnected and updates its primary routes in the state. func (s *State) Disconnect(id types.NodeID) ([]change.Change, error) { + //nolint:staticcheck // SA4006: now is used in new(now) below now := time.Now() node, ok := s.nodeStore.UpdateNode(id, func(n *types.Node) { diff --git a/hscontrol/util/util_test.go b/hscontrol/util/util_test.go index e3a8c3c5..fc55328f 100644 --- a/hscontrol/util/util_test.go +++ b/hscontrol/util/util_test.go @@ -1210,7 +1210,7 @@ func TestEnsureHostnameWithHostinfo(t *testing.T) { wantHostname: "test", checkHostinfo: func(t *testing.T, hi *tailcfg.Hostinfo) { if hi == nil { - t.Error("hostinfo should not be nil") + t.Fatal("hostinfo should not be nil") } if hi.Hostname != "test" { @@ -1244,7 +1244,7 @@ func TestEnsureHostnameWithHostinfo(t *testing.T) { wantHostname: "123456789012345678901234567890123456789012345678901234567890123", checkHostinfo: func(t *testing.T, hi *tailcfg.Hostinfo) { if hi == nil { - t.Error("hostinfo should not be nil") + t.Fatal("hostinfo should not be nil") } if len(hi.Hostname) != 63 { diff --git a/integration/helpers.go b/integration/helpers.go index 8487bbfa..6499792d 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -765,6 +765,7 @@ func tagp(name string) policyv2.Alias { // prefixp returns a pointer to a Prefix from a CIDR string for policy v2 configurations. // Converts CIDR notation to policy prefix format for network range specifications. func prefixp(cidr string) policyv2.Alias { + //nolint:staticcheck // SA4006: prefix is used in new(policyv2.Prefix(prefix)) below prefix := netip.MustParsePrefix(cidr) return new(policyv2.Prefix(prefix)) }