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.)
This commit is contained in:
Kristoffer Dalby 2026-01-20 17:08:19 +00:00
parent 00321fc282
commit 3770015faa
14 changed files with 42 additions and 2 deletions

View file

@ -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)

View file

@ -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)

View file

@ -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{

View file

@ -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")

View file

@ -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

View file

@ -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)

View file

@ -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)

View file

@ -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

View file

@ -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)

View file

@ -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",

View file

@ -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 {

View file

@ -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) {

View file

@ -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 {

View file

@ -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))
}