types: rework validateServerConfig to use ConfigError

Convert the errorText accumulator inside validateServerConfig to the
typed configValidator pattern. Each rule violation now renders as a
multi-line block naming the YAML key, the value the operator wrote,
and a hint pointing at the resolution. The dns.extra_records mutex
that previously crashed via log.Fatal is folded into the same
collector so an operator sees every problem in one go instead of
fixing them one startup attempt at a time.

Test wantErr assertions on validation output switch to substring
match because the rendered errors are now multi-line.

Updates #3227
This commit is contained in:
Kristoffer Dalby
2026-04-30 08:21:22 +00:00
parent 284a4891d7
commit 1b4b79901a
2 changed files with 91 additions and 52 deletions
+85 -47
View File
@@ -608,19 +608,39 @@ func validateServerConfig() error {
depr.Log()
v := &configValidator{}
if viper.IsSet("dns.extra_records") && viper.IsSet("dns.extra_records_path") {
log.Fatal().Msg("fatal config error: dns.extra_records and dns.extra_records_path are mutually exclusive. Please remove one of them from your config file")
v.Add(&ConfigError{
Reason: "dns.extra_records and dns.extra_records_path are mutually exclusive",
Current: []KV{
{"dns.extra_records_path", viper.GetString("dns.extra_records_path")},
{"dns.extra_records", "<inline records>"},
},
Hint: "keep one (a path is recommended for production); remove the other",
})
}
// Collect any validation errors and return them all at once
var errorText string
if (viper.GetString("tls_letsencrypt_hostname") != "") &&
((viper.GetString("tls_cert_path") != "") || (viper.GetString("tls_key_path") != "")) {
errorText += "Fatal config error: set either tls_letsencrypt_hostname or tls_cert_path/tls_key_path, not both\n"
v.Add(&ConfigError{
Reason: "tls_letsencrypt_hostname and tls_cert_path/tls_key_path are mutually exclusive",
Current: []KV{{"tls_letsencrypt_hostname", viper.GetString("tls_letsencrypt_hostname")}},
ConflictsWith: []KV{
{"tls_cert_path", viper.GetString("tls_cert_path")},
{"tls_key_path", viper.GetString("tls_key_path")},
},
Hint: "choose one TLS strategy and unset the other (Let's Encrypt OR a static keypair)",
See: "https://headscale.net/stable/ref/tls/",
})
}
if viper.GetString("noise.private_key_path") == "" {
errorText += "Fatal config error: headscale now requires a new `noise.private_key_path` field in the config file for the Tailscale v2 protocol\n"
v.Add(&ConfigError{
Reason: "noise.private_key_path is required",
Hint: "set noise.private_key_path: /var/lib/headscale/noise_private.key (or any writable path)",
See: "https://headscale.net/stable/setup/install/official/",
})
}
if (viper.GetString("tls_letsencrypt_hostname") != "") &&
@@ -631,17 +651,24 @@ func validateServerConfig() error {
Msg("Warning: when using tls_letsencrypt_hostname with TLS-ALPN-01 as challenge type, headscale must be reachable on port 443, i.e. listen_addr should probably end in :443")
}
v := &configValidator{}
validateListenerCollisions(v)
if (viper.GetString("tls_letsencrypt_challenge_type") != HTTP01ChallengeType) &&
(viper.GetString("tls_letsencrypt_challenge_type") != TLSALPN01ChallengeType) {
errorText += "Fatal config error: the only supported values for tls_letsencrypt_challenge_type are HTTP-01 and TLS-ALPN-01\n"
if ct := viper.GetString("tls_letsencrypt_challenge_type"); ct != HTTP01ChallengeType && ct != TLSALPN01ChallengeType {
v.Add(&ConfigError{
Reason: "tls_letsencrypt_challenge_type has an unsupported value",
Current: []KV{{"tls_letsencrypt_challenge_type", ct}},
Allowed: []string{HTTP01ChallengeType, TLSALPN01ChallengeType},
Hint: "pick one of the allowed values; HTTP-01 is the default",
})
}
if !strings.HasPrefix(viper.GetString("server_url"), "http://") &&
!strings.HasPrefix(viper.GetString("server_url"), "https://") {
errorText += "Fatal config error: server_url must start with https:// or http://\n"
serverURL := viper.GetString("server_url")
if !strings.HasPrefix(serverURL, "http://") && !strings.HasPrefix(serverURL, "https://") {
v.Add(&ConfigError{
Reason: "server_url is missing a scheme",
Current: []KV{{"server_url", serverURL}},
Hint: "prefix the URL with https:// (recommended) or http://",
})
}
// Minimum inactivity time out is keepalive timeout (60s) plus a few seconds
@@ -650,65 +677,76 @@ func validateServerConfig() error {
ephemeralTimeout := resolveEphemeralInactivityTimeout()
if ephemeralTimeout <= minInactivityTimeout {
errorText += fmt.Sprintf(
"Fatal config error: node.ephemeral.inactivity_timeout (%s) is set too low, must be more than %s",
ephemeralTimeout,
minInactivityTimeout,
)
v.Add(&ConfigError{
Reason: "node.ephemeral.inactivity_timeout is below the minimum",
Current: []KV{{"node.ephemeral.inactivity_timeout", ephemeralTimeout.String()}},
Minimum: minInactivityTimeout.String(),
Hint: "raise the value above the keepalive interval (60s) plus a safety margin",
})
}
if viper.GetBool("dns.override_local_dns") {
if global := viper.GetStringSlice("dns.nameservers.global"); len(global) == 0 {
errorText += "Fatal config error: dns.nameservers.global must be set when dns.override_local_dns is true\n"
v.Add(&ConfigError{
Reason: "dns.nameservers.global is required when dns.override_local_dns is true",
Current: []KV{
{"dns.override_local_dns", true},
{"dns.nameservers.global", "[]"},
},
Hint: "list at least one upstream nameserver, or set dns.override_local_dns: false",
See: "https://headscale.net/stable/ref/dns/",
})
}
}
// Validate HA health probing parameters
if haInterval := viper.GetDuration(
"node.routes.ha.probe_interval",
); haInterval > 0 {
if haInterval := viper.GetDuration("node.routes.ha.probe_interval"); haInterval > 0 {
if haInterval < 2*time.Second {
errorText += fmt.Sprintf(
"Fatal config error: node.routes.ha.probe_interval (%s) must be >= 2s\n",
haInterval,
)
v.Add(&ConfigError{
Reason: "node.routes.ha.probe_interval is below the minimum",
Current: []KV{{"node.routes.ha.probe_interval", haInterval.String()}},
Minimum: "2s",
Hint: "raise the value to at least the minimum",
})
}
haTimeout := viper.GetDuration("node.routes.ha.probe_timeout")
if haTimeout < 1*time.Second {
errorText += fmt.Sprintf(
"Fatal config error: node.routes.ha.probe_timeout (%s) must be >= 1s\n",
haTimeout,
)
v.Add(&ConfigError{
Reason: "node.routes.ha.probe_timeout is below the minimum",
Current: []KV{{"node.routes.ha.probe_timeout", haTimeout.String()}},
Minimum: "1s",
Hint: "raise the value to at least the minimum",
})
}
if haTimeout >= haInterval {
errorText += fmt.Sprintf(
"Fatal config error: node.routes.ha.probe_timeout (%s) must be less than node.routes.ha.probe_interval (%s)\n",
haTimeout,
haInterval,
)
v.Add(&ConfigError{
Reason: "node.routes.ha.probe_timeout must be less than node.routes.ha.probe_interval",
Current: []KV{
{"node.routes.ha.probe_timeout", haTimeout.String()},
{"node.routes.ha.probe_interval", haInterval.String()},
},
Hint: "lower probe_timeout below probe_interval (a probe must finish before the next one starts)",
})
}
}
// Validate tuning parameters
if size := viper.GetInt("tuning.node_store_batch_size"); size <= 0 {
errorText += fmt.Sprintf(
"Fatal config error: tuning.node_store_batch_size must be positive, got %d\n",
size,
)
v.Add(&ConfigError{
Reason: "tuning.node_store_batch_size must be positive",
Current: []KV{{"tuning.node_store_batch_size", size}},
Hint: fmt.Sprintf("set to a positive integer (default: %d)", defaultNodeStoreBatchSize),
})
}
if timeout := viper.GetDuration("tuning.node_store_batch_timeout"); timeout <= 0 {
errorText += fmt.Sprintf(
"Fatal config error: tuning.node_store_batch_timeout must be positive, got %s\n",
timeout,
)
}
if errorText != "" {
// nolint
v.AddErr(errors.New(strings.TrimSuffix(errorText, "\n")))
v.Add(&ConfigError{
Reason: "tuning.node_store_batch_timeout must be positive",
Current: []KV{{"tuning.node_store_batch_timeout", timeout.String()}},
Hint: "set to a positive duration (default: 500ms)",
})
}
return v.Err()
+6 -5
View File
@@ -192,7 +192,7 @@ func TestReadConfig(t *testing.T) {
setup: func(t *testing.T) (any, error) { //nolint:thelper
return LoadServerConfig()
},
wantErr: "Fatal config error: dns.nameservers.global must be set when dns.override_local_dns is true",
wantErr: "Fatal config error: dns.nameservers.global is required when dns.override_local_dns is true",
},
{
name: "dns-override-true",
@@ -251,7 +251,8 @@ func TestReadConfig(t *testing.T) {
conf, err := tt.setup(t)
if tt.wantErr != "" {
assert.Equal(t, tt.wantErr, err.Error())
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
@@ -381,17 +382,17 @@ noise:
assert.Contains(
t,
err.Error(),
"Fatal config error: set either tls_letsencrypt_hostname or tls_cert_path/tls_key_path, not both",
"Fatal config error: tls_letsencrypt_hostname and tls_cert_path/tls_key_path are mutually exclusive",
)
assert.Contains(
t,
err.Error(),
"Fatal config error: the only supported values for tls_letsencrypt_challenge_type are",
"Fatal config error: tls_letsencrypt_challenge_type has an unsupported value",
)
assert.Contains(
t,
err.Error(),
"Fatal config error: server_url must start with https:// or http://",
"Fatal config error: server_url is missing a scheme",
)
// Check configuration validation errors (2)