From f969745db498e0127c332b6e3e9d9c13ac398caa Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 20 Jan 2026 16:06:19 +0000 Subject: [PATCH] integration: define sentinel errors for err113 compliance Add sentinel errors across integration test infrastructure and test files to comply with err113 linter requirements. This replaces inline dynamic errors with wrapped static sentinel errors. Files updated: - integration/tsic/tsic.go: Add errNoNetworkSet, errLogoutFailed, errNoIPsReturned, errNoIPv4AddressFound, errBackendStateTimeout, errPeerWaitTimeout, errPeerNotOnline, errPeerNoHostname, errPeerNoDERP, errFileEmpty, errTailscaleVersionRequired - integration/scenario.go: Add errUserAlreadyInNetwork, errNoNetworkNamed, errNoIPAMConfig, errHTTPClientNil, errLoginURLNil, errUnexpectedStatusCode, errNetworkDoesNotExist - integration/helpers.go: Add errExpectedStringNotFound, errUserNotFound, errNoNewClientFound, errUnexpectedClientCount - integration/hsic/hsic.go: Add errDatabaseEmptySchema, errDatabaseFileEmpty, errNoRegularFileInTar - integration/derp_verify_endpoint_test.go: Add errUnexpectedRecvType - cmd/mapresponses/main.go: Add errDirectoryRequired - hscontrol/auth_test.go: Add errNodeNotFoundAfterSetup, errInvalidAuthURLFormat - hscontrol/state/node_store_test.go: Add errTestUpdateNodeFailed, errTestGetNodeFailed, errTestPutNodeFailed --- cmd/mapresponses/main.go | 4 ++- hscontrol/auth_test.go | 10 +++++-- hscontrol/state/node_store_test.go | 18 ++++++++---- integration/derp_verify_endpoint_test.go | 5 +++- integration/helpers.go | 16 ++++++++--- integration/hsic/hsic.go | 10 +++++-- integration/scenario.go | 33 +++++++++++++--------- integration/tsic/tsic.go | 35 ++++++++++++++++-------- 8 files changed, 90 insertions(+), 41 deletions(-) diff --git a/cmd/mapresponses/main.go b/cmd/mapresponses/main.go index af35bc48..1951ca4b 100644 --- a/cmd/mapresponses/main.go +++ b/cmd/mapresponses/main.go @@ -12,6 +12,8 @@ import ( "github.com/juanfont/headscale/integration/integrationutil" ) +var errDirectoryRequired = errors.New("directory is required") + type MapConfig struct { Directory string `flag:"directory,Directory to read map responses from"` } @@ -41,7 +43,7 @@ func main() { // runIntegrationTest executes the integration test workflow. func runOnline(env *command.Env) error { if mapConfig.Directory == "" { - return errors.New("directory is required") + return errDirectoryRequired } resps, err := mapper.ReadMapResponsesFromDirectory(mapConfig.Directory) diff --git a/hscontrol/auth_test.go b/hscontrol/auth_test.go index 8a012ff6..73048d9e 100644 --- a/hscontrol/auth_test.go +++ b/hscontrol/auth_test.go @@ -17,6 +17,12 @@ import ( "tailscale.com/types/key" ) +// Test sentinel errors. +var ( + errNodeNotFoundAfterSetup = errors.New("node not found after setup") + errInvalidAuthURLFormat = errors.New("invalid AuthURL format") +) + // Interactive step type constants. const ( stepTypeInitialRequest = "initial_request" @@ -579,7 +585,7 @@ func TestAuthenticationFlows(t *testing.T) { }, 1*time.Second, 50*time.Millisecond, "waiting for node to be available in NodeStore") if !found { - return "", errors.New("node not found after setup") + return "", errNodeNotFoundAfterSetup } // Expire the node @@ -2716,7 +2722,7 @@ func extractRegistrationIDFromAuthURL(authURL string) (types.RegistrationID, err idx := strings.LastIndex(authURL, registerPrefix) if idx == -1 { - return "", fmt.Errorf("invalid AuthURL format: %s", authURL) + return "", fmt.Errorf("%w: %s", errInvalidAuthURLFormat, authURL) } idStr := authURL[idx+len(registerPrefix):] diff --git a/hscontrol/state/node_store_test.go b/hscontrol/state/node_store_test.go index b90956aa..9740d063 100644 --- a/hscontrol/state/node_store_test.go +++ b/hscontrol/state/node_store_test.go @@ -2,6 +2,7 @@ package state import ( "context" + "errors" "fmt" "net/netip" "runtime" @@ -15,6 +16,13 @@ import ( "tailscale.com/types/key" ) +// Test sentinel errors for concurrent operations. +var ( + errTestUpdateNodeFailed = errors.New("UpdateNode failed") + errTestGetNodeFailed = errors.New("GetNode failed") + errTestPutNodeFailed = errors.New("PutNode failed") +) + func TestSnapshotFromNodes(t *testing.T) { tests := []struct { name string @@ -1001,19 +1009,19 @@ func TestNodeStoreRaceConditions(t *testing.T) { n.Hostname = "race-updated" }) if !resultNode.Valid() { - errors <- fmt.Errorf("UpdateNode failed in goroutine %d, op %d", gid, j) + errors <- fmt.Errorf("%w in goroutine %d, op %d", errTestUpdateNodeFailed, gid, j) } case 1: retrieved, found := store.GetNode(nodeID) if !found || !retrieved.Valid() { - errors <- fmt.Errorf("GetNode failed in goroutine %d, op %d", gid, j) + errors <- fmt.Errorf("%w in goroutine %d, op %d", errTestGetNodeFailed, gid, j) } case 2: newNode := createConcurrentTestNode(nodeID, "race-put") resultNode := store.PutNode(newNode) if !resultNode.Valid() { - errors <- fmt.Errorf("PutNode failed in goroutine %d, op %d", gid, j) + errors <- fmt.Errorf("%w in goroutine %d, op %d", errTestPutNodeFailed, gid, j) } } } @@ -1113,7 +1121,7 @@ func TestNodeStoreOperationTimeout(t *testing.T) { fmt.Printf("[TestNodeStoreOperationTimeout] %s: PutNode(%d) finished, valid=%v, duration=%v\n", endPut.Format("15:04:05.000"), id, resultNode.Valid(), endPut.Sub(startPut)) if !resultNode.Valid() { - putResults[idx-1] = fmt.Errorf("PutNode failed for node %d", id) + putResults[idx-1] = fmt.Errorf("%w for node %d", errTestPutNodeFailed, id) } }(i, nodeID) } @@ -1140,7 +1148,7 @@ func TestNodeStoreOperationTimeout(t *testing.T) { fmt.Printf("[TestNodeStoreOperationTimeout] %s: UpdateNode(%d) finished, valid=%v, ok=%v, duration=%v\n", endUpdate.Format("15:04:05.000"), id, resultNode.Valid(), ok, endUpdate.Sub(startUpdate)) if !ok || !resultNode.Valid() { - updateResults[idx-1] = fmt.Errorf("UpdateNode failed for node %d", id) + updateResults[idx-1] = fmt.Errorf("%w for node %d", errTestUpdateNodeFailed, id) } }(i, nodeID) } diff --git a/integration/derp_verify_endpoint_test.go b/integration/derp_verify_endpoint_test.go index d2aec30f..c92a25ee 100644 --- a/integration/derp_verify_endpoint_test.go +++ b/integration/derp_verify_endpoint_test.go @@ -1,6 +1,7 @@ package integration import ( + "errors" "fmt" "net" "strconv" @@ -19,6 +20,8 @@ import ( "tailscale.com/types/key" ) +var errUnexpectedRecvType = errors.New("client first Recv was unexpected type") + func TestDERPVerifyEndpoint(t *testing.T) { IntegrationSkip(t) @@ -113,7 +116,7 @@ func DERPVerify( if m, err := c.Recv(); err != nil { result = fmt.Errorf("client first Recv: %w", err) } else if v, ok := m.(derp.ServerInfoMessage); !ok { - result = fmt.Errorf("client first Recv was unexpected type %T", v) + result = fmt.Errorf("%w: %T", errUnexpectedRecvType, v) } if expectSuccess && result != nil { diff --git a/integration/helpers.go b/integration/helpers.go index 59b87cff..38abfdb2 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -28,6 +28,14 @@ import ( "tailscale.com/tailcfg" ) +// Sentinel errors for integration test helpers. +var ( + errExpectedStringNotFound = errors.New("expected string not found in output") + errUserNotFound = errors.New("user not found") + errNoNewClientFound = errors.New("no new client found") + errUnexpectedClientCount = errors.New("unexpected client count") +) + const ( // derpPingTimeout defines the timeout for individual DERP ping operations // Used in DERP connectivity tests to verify relay server communication. @@ -646,7 +654,7 @@ func assertCommandOutputContains(t *testing.T, c TailscaleClient, command []stri } if !strings.Contains(stdout, contains) { - return struct{}{}, fmt.Errorf("executing command, expected string %q not found in %q", contains, stdout) + return struct{}{}, fmt.Errorf("executing command, %w: %q not found in %q", errExpectedStringNotFound, contains, stdout) } return struct{}{}, nil @@ -811,7 +819,7 @@ func GetUserByName(headscale ControlServer, username string) (*v1.User, error) { } } - return nil, fmt.Errorf("user %s not found", username) + return nil, fmt.Errorf("%w: %s", errUserNotFound, username) } // FindNewClient finds a client that is in the new list but not in the original list. @@ -833,7 +841,7 @@ func FindNewClient(original, updated []TailscaleClient) (TailscaleClient, error) } } - return nil, errors.New("no new client found") + return nil, errNoNewClientFound } // AddAndLoginClient adds a new tailscale client to a user and logs it in. @@ -873,7 +881,7 @@ func (s *Scenario) AddAndLoginClient( } if len(updatedClients) != len(originalClients)+1 { - return struct{}{}, fmt.Errorf("expected %d clients, got %d", len(originalClients)+1, len(updatedClients)) + return struct{}{}, fmt.Errorf("%w: expected %d clients, got %d", errUnexpectedClientCount, len(originalClients)+1, len(updatedClients)) } newClient, err = FindNewClient(originalClients, updatedClients) diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index c4eaeea8..f2fe5b30 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -53,6 +53,9 @@ var ( errInvalidHeadscaleImageFormat = errors.New("invalid HEADSCALE_INTEGRATION_HEADSCALE_IMAGE format, expected repository:tag") errHeadscaleImageRequiredInCI = errors.New("HEADSCALE_INTEGRATION_HEADSCALE_IMAGE must be set in CI") errInvalidPostgresImageFormat = errors.New("invalid HEADSCALE_INTEGRATION_POSTGRES_IMAGE format, expected repository:tag") + errDatabaseEmptySchema = errors.New("database file exists but has no schema") + errDatabaseFileEmpty = errors.New("database file is empty") + errNoRegularFileInTar = errors.New("no regular file found in database tar archive") ) type fileInContainer struct { @@ -861,7 +864,7 @@ func (t *HeadscaleInContainer) SaveDatabase(savePath string) error { } if strings.TrimSpace(schemaCheck) == "" { - return errors.New("database file exists but has no schema (empty database)") + return errDatabaseEmptySchema } tarFile, err := t.FetchPath("/tmp/integration_test_db.sqlite3") @@ -914,7 +917,8 @@ func (t *HeadscaleInContainer) SaveDatabase(savePath string) error { // Check if we actually wrote something if written == 0 { return fmt.Errorf( - "database file is empty (size: %d, header size: %d)", + "%w (size: %d, header size: %d)", + errDatabaseFileEmpty, written, header.Size, ) @@ -924,7 +928,7 @@ func (t *HeadscaleInContainer) SaveDatabase(savePath string) error { } } - return errors.New("no regular file found in database tar archive") + return errNoRegularFileInTar } // Execute runs a command inside the Headscale container and returns the diff --git a/integration/scenario.go b/integration/scenario.go index 0b388c0a..d1ebdd51 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -51,9 +51,16 @@ const ( var usePostgresForTest = envknob.Bool("HEADSCALE_INTEGRATION_POSTGRES") var ( - errNoHeadscaleAvailable = errors.New("no headscale available") - errNoUserAvailable = errors.New("no user available") - errNoClientFound = errors.New("client not found") + errNoHeadscaleAvailable = errors.New("no headscale available") + errNoUserAvailable = errors.New("no user available") + errNoClientFound = errors.New("client not found") + errUserAlreadyInNetwork = errors.New("users can only have nodes placed in one network") + errNoNetworkNamed = errors.New("no network named") + errNoIPAMConfig = errors.New("no IPAM config found in network") + errHTTPClientNil = errors.New("http client is nil") + errLoginURLNil = errors.New("login url is nil") + errUnexpectedStatusCode = errors.New("unexpected status code") + errNetworkDoesNotExist = errors.New("network does not exist") // AllVersions represents a list of Tailscale versions the suite // uses to test compatibility with the ControlServer. @@ -203,7 +210,7 @@ func NewScenario(spec ScenarioSpec) (*Scenario, error) { for _, user := range users { if n2, ok := userToNetwork[user]; ok { - return nil, fmt.Errorf("users can only have nodes placed in one network: %s into %s but already in %s", user, network.Network.Name, n2.Network.Name) + return nil, fmt.Errorf("%w: %s into %s but already in %s", errUserAlreadyInNetwork, user, network.Network.Name, n2.Network.Name) } mak.Set(&userToNetwork, user, network) @@ -280,7 +287,7 @@ func (s *Scenario) Networks() []*dockertest.Network { func (s *Scenario) Network(name string) (*dockertest.Network, error) { net, ok := s.networks[s.prefixedNetworkName(name)] if !ok { - return nil, fmt.Errorf("no network named: %s", name) + return nil, fmt.Errorf("%w: %s", errNoNetworkNamed, name) } return net, nil @@ -289,11 +296,11 @@ func (s *Scenario) Network(name string) (*dockertest.Network, error) { func (s *Scenario) SubnetOfNetwork(name string) (*netip.Prefix, error) { net, ok := s.networks[s.prefixedNetworkName(name)] if !ok { - return nil, fmt.Errorf("no network named: %s", name) + return nil, fmt.Errorf("%w: %s", errNoNetworkNamed, name) } if len(net.Network.IPAM.Config) == 0 { - return nil, fmt.Errorf("no IPAM config found in network: %s", name) + return nil, fmt.Errorf("%w: %s", errNoIPAMConfig, name) } pref, err := netip.ParsePrefix(net.Network.IPAM.Config[0].Subnet) @@ -307,7 +314,7 @@ func (s *Scenario) SubnetOfNetwork(name string) (*netip.Prefix, error) { func (s *Scenario) Services(name string) ([]*dockertest.Resource, error) { res, ok := s.extraServices[s.prefixedNetworkName(name)] if !ok { - return nil, fmt.Errorf("no network named: %s", name) + return nil, fmt.Errorf("%w: %s", errNoNetworkNamed, name) } return res, nil @@ -1070,11 +1077,11 @@ func doLoginURLWithClient(hostname string, loginURL *url.URL, hc *http.Client, f error, ) { if hc == nil { - return "", nil, fmt.Errorf("%s http client is nil", hostname) + return "", nil, fmt.Errorf("%s %w", hostname, errHTTPClientNil) } if loginURL == nil { - return "", nil, fmt.Errorf("%s login url is nil", hostname) + return "", nil, fmt.Errorf("%s %w", hostname, errLoginURLNil) } log.Printf("%s logging in with url: %s", hostname, loginURL.String()) @@ -1121,13 +1128,13 @@ func doLoginURLWithClient(hostname string, loginURL *url.URL, hc *http.Client, f if followRedirects && resp.StatusCode != http.StatusOK { log.Printf("body: %s", body) - return body, redirectURL, fmt.Errorf("%s unexpected status code %d", hostname, resp.StatusCode) + return body, redirectURL, fmt.Errorf("%s %w %d", hostname, errUnexpectedStatusCode, resp.StatusCode) } if resp.StatusCode >= http.StatusBadRequest { log.Printf("body: %s", body) - return body, redirectURL, fmt.Errorf("%s unexpected status code %d", hostname, resp.StatusCode) + return body, redirectURL, fmt.Errorf("%s %w %d", hostname, errUnexpectedStatusCode, resp.StatusCode) } if hc.Jar != nil { @@ -1506,7 +1513,7 @@ func Webservice(s *Scenario, networkName string) (*dockertest.Resource, error) { network, ok := s.networks[s.prefixedNetworkName(networkName)] if !ok { - return nil, fmt.Errorf("network does not exist: %s", networkName) + return nil, fmt.Errorf("%w: %s", errNetworkDoesNotExist, networkName) } webOpts := &dockertest.RunOptions{ diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index fb07896b..3136b6ae 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -59,6 +59,17 @@ var ( errTailscaleImageRequiredInCI = errors.New("HEADSCALE_INTEGRATION_TAILSCALE_IMAGE must be set in CI for HEAD version") errContainerNotInitialized = errors.New("container not initialized") errFQDNNotYetAvailable = errors.New("FQDN not yet available") + errNoNetworkSet = errors.New("no network set") + errLogoutFailed = errors.New("failed to logout") + errNoIPsReturned = errors.New("no IPs returned yet") + errNoIPv4AddressFound = errors.New("no IPv4 address found") + errBackendStateTimeout = errors.New("timeout waiting for backend state") + errPeerWaitTimeout = errors.New("timeout waiting for peers") + errPeerNotOnline = errors.New("peer is not online") + errPeerNoHostname = errors.New("peer does not have a hostname") + errPeerNoDERP = errors.New("peer does not have a DERP relay") + errFileEmpty = errors.New("file is empty") + errTailscaleVersionRequired = errors.New("tailscale version requirement not met") ) const ( @@ -338,7 +349,7 @@ func New( } if tsic.network == nil { - return nil, fmt.Errorf("no network set, called from: \n%s", string(debug.Stack())) + return nil, fmt.Errorf("%w, called from: \n%s", errNoNetworkSet, string(debug.Stack())) } tailscaleOptions := &dockertest.RunOptions{ @@ -720,7 +731,7 @@ func (t *TailscaleInContainer) Logout() error { stdout, stderr, _ = t.Execute([]string{"tailscale", "status"}) if !strings.Contains(stdout+stderr, "Logged out.") { - return fmt.Errorf("failed to logout, stdout: %s, stderr: %s", stdout, stderr) + return fmt.Errorf("%w: stdout: %s, stderr: %s", errLogoutFailed, stdout, stderr) } return t.waitForBackendState("NeedsLogin", integrationutil.PeerSyncTimeout()) @@ -832,7 +843,7 @@ func (t *TailscaleInContainer) IPs() ([]netip.Addr, error) { } if len(ips) == 0 { - return nil, fmt.Errorf("no IPs returned yet for %s", t.hostname) + return nil, fmt.Errorf("%w for %s", errNoIPsReturned, t.hostname) } return ips, nil @@ -866,7 +877,7 @@ func (t *TailscaleInContainer) IPv4() (netip.Addr, error) { } } - return netip.Addr{}, fmt.Errorf("no IPv4 address found for %s", t.hostname) + return netip.Addr{}, fmt.Errorf("%w for %s", errNoIPv4AddressFound, t.hostname) } func (t *TailscaleInContainer) MustIPv4() netip.Addr { @@ -1211,7 +1222,7 @@ func (t *TailscaleInContainer) waitForBackendState(state string, timeout time.Du for { select { case <-ctx.Done(): - return fmt.Errorf("timeout waiting for backend state %s on %s after %v", state, t.hostname, timeout) + return fmt.Errorf("%w %s on %s after %v", errBackendStateTimeout, state, t.hostname, timeout) case <-ticker.C: status, err := t.Status() if err != nil { @@ -1253,10 +1264,10 @@ func (t *TailscaleInContainer) WaitForPeers(expected int, timeout, retryInterval select { case <-ctx.Done(): if len(lastErrs) > 0 { - return fmt.Errorf("timeout waiting for %d peers on %s after %v, errors: %w", expected, t.hostname, timeout, multierr.New(lastErrs...)) + return fmt.Errorf("%w for %d peers on %s after %v, errors: %w", errPeerWaitTimeout, expected, t.hostname, timeout, multierr.New(lastErrs...)) } - return fmt.Errorf("timeout waiting for %d peers on %s after %v", expected, t.hostname, timeout) + return fmt.Errorf("%w for %d peers on %s after %v", errPeerWaitTimeout, expected, t.hostname, timeout) case <-ticker.C: status, err := t.Status() if err != nil { @@ -1284,15 +1295,15 @@ func (t *TailscaleInContainer) WaitForPeers(expected int, timeout, retryInterval peer := status.Peer[peerKey] if !peer.Online { - peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %s is not online", t.hostname, peer.HostName)) + peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %w: %s", t.hostname, errPeerNotOnline, peer.HostName)) } if peer.HostName == "" { - peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %s does not have a Hostname", t.hostname, peer.HostName)) + peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %w: %s", t.hostname, errPeerNoHostname, peer.HostName)) } if peer.Relay == "" { - peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %s does not have a DERP", t.hostname, peer.HostName)) + peerErrors = append(peerErrors, fmt.Errorf("[%s] peer count correct, but %w: %s", t.hostname, errPeerNoDERP, peer.HostName)) } } @@ -1578,7 +1589,7 @@ func (t *TailscaleInContainer) ReadFile(path string) ([]byte, error) { } if out.Len() == 0 { - return nil, errors.New("file is empty") + return nil, errFileEmpty } return out.Bytes(), nil @@ -1617,7 +1628,7 @@ func (t *TailscaleInContainer) GetNodePrivateKey() (*key.NodePrivate, error) { // This is useful for verifying that policy changes have propagated to the client. func (t *TailscaleInContainer) PacketFilter() ([]filter.Match, error) { if !util.TailscaleVersionNewerOrEqual("1.56", t.version) { - return nil, fmt.Errorf("tsic.PacketFilter() requires Tailscale 1.56+, current version: %s", t.version) + return nil, fmt.Errorf("%w: PacketFilter() requires Tailscale 1.56+, current version: %s", errTailscaleVersionRequired, t.version) } nm, err := t.Netmap()