1
0
mirror of https://github.com/juanfont/headscale.git synced 2026-02-07 20:04:00 +01:00

hscontrol: fix err113 lint issues (batch 7)

Add sentinel errors and use %w wrapping for dynamic errors in:
- mapper/batcher.go: ErrNodeConnectionNil, ErrNodeNotFoundMapper
- mapper/batcher_lockfree.go: ErrInitialMapSendTimeout, ErrBatcherShuttingDown,
  ErrConnectionSendTimeout
- mapper/builder.go: use ErrNodeNotFoundMapper (remove errors import)
- types/common.go: ErrInvalidRegistrationIDLength
- types/config.go: ErrNoPrefixConfigured, ErrInvalidAllocationStrategy
- types/node.go: use existing ErrInvalidNodeView
- types/users.go: ErrCannotParseBoolean
- util/util.go: ErrTracerouteDidNotReach
- util/util_test.go: use ErrTracerouteDidNotReach

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Kristoffer Dalby 2026-02-06 07:59:27 +00:00
parent c164b15503
commit 0cb49ab343
9 changed files with 50 additions and 32 deletions

View File

@ -18,8 +18,10 @@ import (
// Mapper errors.
var (
ErrInvalidNodeID = errors.New("invalid nodeID")
ErrMapperNil = errors.New("mapper is nil")
ErrInvalidNodeID = errors.New("invalid nodeID")
ErrMapperNil = errors.New("mapper is nil")
ErrNodeConnectionNil = errors.New("nodeConnection is nil")
ErrNodeNotFoundMapper = errors.New("node not found")
)
var mapResponseGenerated = promauto.NewCounterVec(prometheus.CounterOpts{
@ -142,7 +144,7 @@ func generateMapResponse(nc nodeConnection, mapper *mapper, r change.Change) (*t
// handleNodeChange generates and sends a [tailcfg.MapResponse] for a given node and [change.Change].
func handleNodeChange(nc nodeConnection, mapper *mapper, r change.Change) error {
if nc == nil {
return errors.New("nodeConnection is nil")
return ErrNodeConnectionNil
}
nodeID := nc.nodeID()

View File

@ -19,7 +19,13 @@ import (
"tailscale.com/types/ptr"
)
var errConnectionClosed = errors.New("connection channel already closed")
// LockFreeBatcher errors.
var (
errConnectionClosed = errors.New("connection channel already closed")
ErrInitialMapSendTimeout = errors.New("sending initial map: timeout")
ErrBatcherShuttingDown = errors.New("batcher shutting down")
ErrConnectionSendTimeout = errors.New("timeout sending to channel (likely stale connection)")
)
// LockFreeBatcher uses atomic operations and concurrent maps to eliminate mutex contention.
type LockFreeBatcher struct {
@ -92,12 +98,12 @@ func (b *LockFreeBatcher) AddNode(id types.NodeID, c chan<- *tailcfg.MapResponse
case c <- initialMap:
// Success
case <-time.After(5 * time.Second): //nolint:mnd
nlog.Error().Err(errors.New("timeout")).Msg("initial map send timeout") //nolint:err113
nlog.Debug().Caller().Dur("timeout.duration", 5*time.Second). //nolint:mnd
Msg("initial map send timed out because channel was blocked or receiver not ready")
nlog.Error().Err(ErrInitialMapSendTimeout).Msg("initial map send timeout")
nlog.Debug().Caller().Dur("timeout.duration", 5*time.Second). //nolint:mnd
Msg("initial map send timed out because channel was blocked or receiver not ready")
nodeConn.removeConnectionByChannel(c)
return fmt.Errorf("sending initial map to node %d: timeout", id)
return fmt.Errorf("%w for node %d", ErrInitialMapSendTimeout, id)
}
// Update connection status
@ -241,7 +247,7 @@ func (b *LockFreeBatcher) worker(workerID int) {
nc.updateSentPeers(result.mapResponse)
}
} else {
result.err = fmt.Errorf("node %d not found", w.nodeID)
result.err = fmt.Errorf("%w: %d", ErrNodeNotFoundMapper, w.nodeID)
b.workErrors.Add(1)
wlog.Error().Err(result.err).
@ -497,7 +503,7 @@ func (b *LockFreeBatcher) MapResponseFromChange(id types.NodeID, ch change.Chang
case result := <-resultCh:
return result.mapResponse, result.err
case <-b.done:
return nil, fmt.Errorf("batcher shutting down while generating map response for node %d", id)
return nil, fmt.Errorf("%w while generating map response for node %d", ErrBatcherShuttingDown, id)
}
}
@ -714,7 +720,7 @@ func (entry *connectionEntry) send(data *tailcfg.MapResponse) error {
case <-time.After(50 * time.Millisecond):
// Connection is likely stale - client isn't reading from channel
// This catches the case where Docker containers are killed but channels remain open
return fmt.Errorf("connection %s: timeout sending to channel (likely stale connection)", entry.id)
return fmt.Errorf("connection %s: %w", entry.id, ErrConnectionSendTimeout)
}
}

View File

@ -1,7 +1,6 @@
package mapper
import (
"errors"
"net/netip"
"sort"
"time"
@ -70,7 +69,7 @@ func (b *MapResponseBuilder) WithCapabilityVersion(capVer tailcfg.CapabilityVers
func (b *MapResponseBuilder) WithSelfNode() *MapResponseBuilder {
nv, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
b.addError(errors.New("node not found"))
b.addError(ErrNodeNotFoundMapper)
return b
}
@ -132,7 +131,7 @@ func (b *MapResponseBuilder) WithDebugConfig() *MapResponseBuilder {
func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
node, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
b.addError(errors.New("node not found"))
b.addError(ErrNodeNotFoundMapper)
return b
}
@ -151,7 +150,7 @@ func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
func (b *MapResponseBuilder) WithDNSConfig() *MapResponseBuilder {
node, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
b.addError(errors.New("node not found"))
b.addError(ErrNodeNotFoundMapper)
return b
}
@ -164,7 +163,7 @@ func (b *MapResponseBuilder) WithDNSConfig() *MapResponseBuilder {
func (b *MapResponseBuilder) WithUserProfiles(peers views.Slice[types.NodeView]) *MapResponseBuilder {
node, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
b.addError(errors.New("node not found"))
b.addError(ErrNodeNotFoundMapper)
return b
}
@ -177,7 +176,7 @@ func (b *MapResponseBuilder) WithUserProfiles(peers views.Slice[types.NodeView])
func (b *MapResponseBuilder) WithPacketFilters() *MapResponseBuilder {
node, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
b.addError(errors.New("node not found"))
b.addError(ErrNodeNotFoundMapper)
return b
}
@ -231,7 +230,7 @@ func (b *MapResponseBuilder) WithPeerChanges(peers views.Slice[types.NodeView])
func (b *MapResponseBuilder) buildTailPeers(peers views.Slice[types.NodeView]) ([]*tailcfg.Node, error) {
node, ok := b.mapper.state.GetNodeByID(b.nodeID)
if !ok {
return nil, errors.New("node not found")
return nil, ErrNodeNotFoundMapper
}
// Get unreduced matchers for peer relationship determination.

View File

@ -20,7 +20,11 @@ const (
DatabaseSqlite = "sqlite3"
)
var ErrCannotParsePrefix = errors.New("cannot parse prefix")
// Common errors.
var (
ErrCannotParsePrefix = errors.New("cannot parse prefix")
ErrInvalidRegistrationIDLength = errors.New("registration ID has invalid length")
)
type StateUpdateType int
@ -175,7 +179,7 @@ func MustRegistrationID() RegistrationID {
func RegistrationIDFromString(str string) (RegistrationID, error) {
if len(str) != RegistrationIDLength {
return "", fmt.Errorf("registration ID must be %d characters long", RegistrationIDLength)
return "", fmt.Errorf("%w: expected %d, got %d", ErrInvalidRegistrationIDLength, RegistrationIDLength, len(str))
}
return RegistrationID(str), nil

View File

@ -33,10 +33,12 @@ const (
)
var (
errOidcMutuallyExclusive = errors.New("oidc_client_secret and oidc_client_secret_path are mutually exclusive")
errServerURLSuffix = errors.New("server_url cannot be part of base_domain in a way that could make the DERP and headscale server unreachable")
errServerURLSame = errors.New("server_url cannot use the same domain as base_domain in a way that could make the DERP and headscale server unreachable")
errInvalidPKCEMethod = errors.New("pkce.method must be either 'plain' or 'S256'")
errOidcMutuallyExclusive = errors.New("oidc_client_secret and oidc_client_secret_path are mutually exclusive")
errServerURLSuffix = errors.New("server_url cannot be part of base_domain in a way that could make the DERP and headscale server unreachable")
errServerURLSame = errors.New("server_url cannot use the same domain as base_domain in a way that could make the DERP and headscale server unreachable")
errInvalidPKCEMethod = errors.New("pkce.method must be either 'plain' or 'S256'")
ErrNoPrefixConfigured = errors.New("no IPv4 or IPv6 prefix configured, minimum one prefix is required")
ErrInvalidAllocationStrategy = errors.New("invalid prefix allocation strategy")
)
type IPAllocationStrategy string
@ -931,7 +933,7 @@ func LoadServerConfig() (*Config, error) {
}
if prefix4 == nil && prefix6 == nil {
return nil, errors.New("no IPv4 or IPv6 prefix configured, minimum one prefix is required")
return nil, ErrNoPrefixConfigured
}
allocStr := viper.GetString("prefixes.allocation")
@ -945,7 +947,8 @@ func LoadServerConfig() (*Config, error) {
alloc = IPAllocationStrategyRandom
default:
return nil, fmt.Errorf(
"config error, prefixes.allocation is set to %s, which is not a valid strategy, allowed options: %s, %s",
"%w: %q, allowed options: %s, %s",
ErrInvalidAllocationStrategy,
allocStr,
IPAllocationStrategySequential,
IPAllocationStrategyRandom,

View File

@ -901,7 +901,7 @@ func (nv NodeView) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.Peer
// GetFQDN returns the fully qualified domain name for the node.
func (nv NodeView) GetFQDN(baseDomain string) (string, error) {
if !nv.Valid() {
return "", errors.New("creating valid FQDN: node view is invalid")
return "", fmt.Errorf("creating valid FQDN: %w", ErrInvalidNodeView)
}
return nv.ж.GetFQDN(baseDomain)

View File

@ -4,6 +4,7 @@ import (
"cmp"
"database/sql"
"encoding/json"
"errors"
"fmt"
"net/mail"
"net/url"
@ -20,6 +21,9 @@ import (
"tailscale.com/tailcfg"
)
// ErrCannotParseBoolean is returned when a value cannot be parsed as boolean.
var ErrCannotParseBoolean = errors.New("cannot parse value as boolean")
type UserID uint64
type Users []User
@ -252,7 +256,7 @@ func (bit *FlexibleBoolean) UnmarshalJSON(data []byte) error {
*bit = FlexibleBoolean(pv)
default:
return fmt.Errorf("parsing %v as boolean", v)
return fmt.Errorf("%w: %v", ErrCannotParseBoolean, v)
}
return nil

View File

@ -22,6 +22,7 @@ var (
ErrNoURLFound = errors.New("no URL found")
ErrEmptyTracerouteOutput = errors.New("empty traceroute output")
ErrTracerouteHeaderParse = errors.New("parsing traceroute header")
ErrTracerouteDidNotReach = errors.New("traceroute did not reach target")
)
func TailscaleVersionNewerOrEqual(minimum, toCheck string) bool {
@ -263,7 +264,7 @@ func ParseTraceroute(output string) (Traceroute, error) {
// If we didn't reach the target, it's unsuccessful
if !result.Success {
result.Err = errors.New("traceroute did not reach target")
result.Err = ErrTracerouteDidNotReach
}
return result, nil

View File

@ -1,7 +1,6 @@
package util
import (
"errors"
"net/netip"
"strings"
"testing"
@ -323,7 +322,7 @@ func TestParseTraceroute(t *testing.T) {
},
},
Success: false,
Err: errors.New("traceroute did not reach target"),
Err: ErrTracerouteDidNotReach,
},
wantErr: false,
},
@ -491,7 +490,7 @@ over a maximum of 30 hops:
},
},
Success: false,
Err: errors.New("traceroute did not reach target"),
Err: ErrTracerouteDidNotReach,
},
wantErr: false,
},