From 51d926dde12e919e7bcf3f03a81f3e3b764dd027 Mon Sep 17 00:00:00 2001 From: Denys Vitali Date: Thu, 12 Mar 2026 10:05:56 +0100 Subject: [PATCH] feat: add 'server validate' subcommand and unknown key warnings Atlantis silently ignores unknown keys in server config files via viper.Unmarshal(), which can lead to misconfiguration from typos (e.g. "allow-draft-pr" vs "allow-draft-prs") or invalid formats (e.g. "parallel_apply" vs "parallel-apply"). This adds: - `atlantis server validate --config ` subcommand that checks for unknown keys and exits non-zero if any are found - Warning on `atlantis server` startup when unknown keys are detected (non-breaking, doesn't block startup) - Helper functions buildKnownKeys() and findUnknownKeys() that derive valid keys from server.UserConfig mapstructure tags Signed-off-by: Denys Vitali --- cmd/server.go | 36 ++++++++ cmd/server_validate.go | 62 +++++++++++++ cmd/server_validate_test.go | 168 ++++++++++++++++++++++++++++++++++++ main.go | 8 +- 4 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 cmd/server_validate.go create mode 100644 cmd/server_validate_test.go diff --git a/cmd/server.go b/cmd/server.go index c0d5db4f8f..e601398f98 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -18,7 +18,9 @@ import ( "net/url" "os" "path/filepath" + "reflect" "slices" + "sort" "strings" homedir "github.com/mitchellh/go-homedir" @@ -839,6 +841,35 @@ func (s *ServerCmd) Init() *cobra.Command { return c } +// buildKnownKeys returns the set of config keys that are recognized by +// Atlantis, derived from the mapstructure tags on server.UserConfig plus the +// "config" meta-key. +func buildKnownKeys() map[string]struct{} { + keys := make(map[string]struct{}) + t := reflect.TypeFor[server.UserConfig]() + for i := 0; i < t.NumField(); i++ { + if tag := t.Field(i).Tag.Get("mapstructure"); tag != "" { + keys[tag] = struct{}{} + } + } + keys[ConfigFlag] = struct{}{} + return keys +} + +// findUnknownKeys returns config keys present in viper that do not correspond +// to any known Atlantis server flag. The result is sorted alphabetically. +func findUnknownKeys(v *viper.Viper) []string { + known := buildKnownKeys() + var unknown []string + for _, key := range v.AllKeys() { + if _, ok := known[key]; !ok { + unknown = append(unknown, key) + } + } + sort.Strings(unknown) + return unknown +} + func (s *ServerCmd) preRun() error { // If passed a config file then try and load it. configFile := s.Viper.GetString(ConfigFlag) @@ -852,6 +883,11 @@ func (s *ServerCmd) preRun() error { } func (s *ServerCmd) run() error { + if unknowns := findUnknownKeys(s.Viper); len(unknowns) > 0 { + s.Logger.Warn("unknown keys in config (will be ignored): %s", + strings.Join(unknowns, ", ")) + } + var userConfig server.UserConfig if err := s.Viper.Unmarshal(&userConfig); err != nil { return err diff --git a/cmd/server_validate.go b/cmd/server_validate.go new file mode 100644 index 0000000000..0477cb044e --- /dev/null +++ b/cmd/server_validate.go @@ -0,0 +1,62 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/runatlantis/atlantis/server/logging" +) + +// ServerValidateCmd validates an Atlantis server config file without starting +// the server. It checks for unknown keys that would be silently ignored. +type ServerValidateCmd struct { + Viper *viper.Viper + Logger logging.SimpleLogging +} + +// Init returns the runnable cobra command. +func (s *ServerValidateCmd) Init() *cobra.Command { + c := &cobra.Command{ + Use: "validate", + Short: "Validate Atlantis server config file", + Long: `Validate an Atlantis server config file for unknown keys. + +Unknown keys are silently ignored by Atlantis at startup, which can lead to +misconfiguration (e.g. typos like "allow-draft-pr" instead of "allow-draft-prs"). + +This command reads the config file without starting the server and reports any +unrecognized keys.`, + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + return s.run() + }, + } + + c.Flags().String(ConfigFlag, "", "Path to the Atlantis server config file (required)\n") + s.Viper.BindPFlag(ConfigFlag, c.Flags().Lookup(ConfigFlag)) // nolint: errcheck + + return c +} + +func (s *ServerValidateCmd) run() error { + configFile := s.Viper.GetString(ConfigFlag) + if configFile == "" { + return fmt.Errorf("--%s is required", ConfigFlag) + } + + s.Viper.SetConfigFile(configFile) + if err := s.Viper.ReadInConfig(); err != nil { + return fmt.Errorf("reading config %s: %w", configFile, err) + } + + unknowns := findUnknownKeys(s.Viper) + if len(unknowns) > 0 { + return fmt.Errorf("unknown keys in %s: %s", configFile, strings.Join(unknowns, ", ")) + } + + s.Logger.Info("config file %s is valid", configFile) + return nil +} diff --git a/cmd/server_validate_test.go b/cmd/server_validate_test.go new file mode 100644 index 0000000000..8462a1c3a4 --- /dev/null +++ b/cmd/server_validate_test.go @@ -0,0 +1,168 @@ +package cmd + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/spf13/viper" + + "github.com/runatlantis/atlantis/server/logging" + . "github.com/runatlantis/atlantis/testing" +) + +func setupValidate(flags map[string]any, t *testing.T) *ServerValidateCmd { + t.Helper() + v := viper.New() + for k, val := range flags { + v.Set(k, val) + } + return &ServerValidateCmd{ + Viper: v, + Logger: logging.NewNoopLogger(t), + } +} + +func TestValidate_MissingConfigFlag(t *testing.T) { + t.Log("Should error when --config is not provided.") + cmd := setupValidate(nil, t) + c := cmd.Init() + err := c.Execute() + ErrContains(t, "--config is required", err) +} + +func TestValidate_ConfigFileNotFound(t *testing.T) { + t.Log("Should error when config file does not exist.") + cmd := setupValidate(map[string]any{ + ConfigFlag: "does-not-exist.yaml", + }, t) + c := cmd.Init() + err := c.Execute() + ErrContains(t, "does-not-exist.yaml", err) +} + +func TestValidate_ValidConfig(t *testing.T) { + t.Log("Should succeed when config contains only known keys.") + tmpFile := tempFile(t, "repo-allowlist: github.com/test/*\nport: 4141\n") + defer os.Remove(tmpFile) // nolint: errcheck + cmd := setupValidate(map[string]any{ + ConfigFlag: tmpFile, + }, t) + c := cmd.Init() + err := c.Execute() + Ok(t, err) +} + +func TestValidate_UnknownKey(t *testing.T) { + t.Log("Should error when config contains unknown keys.") + tmpFile := tempFile(t, "repo-allowlist: github.com/test/*\nallow-draft-pr: true\nparallel_apply: true\n") + defer os.Remove(tmpFile) // nolint: errcheck + cmd := setupValidate(map[string]any{ + ConfigFlag: tmpFile, + }, t) + c := cmd.Init() + err := c.Execute() + Assert(t, err != nil, "should error on unknown keys") + ErrContains(t, "unknown keys", err) + ErrContains(t, "allow-draft-pr", err) + ErrContains(t, "parallel_apply", err) +} + +func TestValidate_InvalidYAML(t *testing.T) { + t.Log("Should error on invalid YAML.") + tmpFile := tempFile(t, "invalid: yaml: content: [") + defer os.Remove(tmpFile) // nolint: errcheck + cmd := setupValidate(map[string]any{ + ConfigFlag: tmpFile, + }, t) + c := cmd.Init() + err := c.Execute() + Assert(t, err != nil, "should error on invalid YAML") +} + +func TestFindUnknownKeys_AllKnown(t *testing.T) { + t.Log("Should return empty slice when all keys are known.") + v := viper.New() + v.Set("repo-allowlist", "github.com/test/*") + v.Set("port", 4141) + v.Set("allow-draft-prs", true) + unknowns := findUnknownKeys(v) + Equals(t, 0, len(unknowns)) +} + +func TestFindUnknownKeys_WithUnknown(t *testing.T) { + t.Log("Should return unknown keys sorted alphabetically.") + v := viper.New() + v.Set("repo-allowlist", "github.com/test/*") + v.Set("tofu-version", "1.11.5") + v.Set("allow-draft-pr", true) + unknowns := findUnknownKeys(v) + Equals(t, 2, len(unknowns)) + Equals(t, "allow-draft-pr", unknowns[0]) + Equals(t, "tofu-version", unknowns[1]) +} + +func TestBuildKnownKeys_ContainsExpectedKeys(t *testing.T) { + t.Log("Should contain known keys from UserConfig mapstructure tags.") + keys := buildKnownKeys() + expectedKeys := []string{ + "repo-allowlist", "port", "allow-draft-prs", "parallel-plan", + "parallel-apply", "autodiscover-mode", "config", + } + for _, key := range expectedKeys { + if _, ok := keys[key]; !ok { + t.Errorf("expected key %q not found in known keys", key) + } + } +} + +func TestRun_UnknownKeyWarning(t *testing.T) { + t.Log("Server run() should warn about unknown keys in config.") + cfgContents := "repo-allowlist: github.com/test/*\ntofu-version: 1.11.5\n" + tmpFile := tempFile(t, cfgContents) + defer os.Remove(tmpFile) // nolint: errcheck + + v := viper.New() + v.Set(ConfigFlag, tmpFile) + v.Set(GHUserFlag, "user") + v.Set(GHTokenFlag, "token") + v.Set(RepoAllowlistFlag, "*") + + logger := &captureLogger{} + s := &ServerCmd{ + ServerCreator: &ServerCreatorMock{}, + Viper: v, + SilenceOutput: true, + Logger: logger, + } + c := s.Init() + _ = c.Execute() + + found := false + for _, msg := range logger.warnings { + if strings.Contains(msg, "unknown keys") && strings.Contains(msg, "tofu-version") { + found = true + break + } + } + Assert(t, found, "expected warning about unknown key 'tofu-version'") +} + +// captureLogger captures log messages for testing. +type captureLogger struct { + warnings []string +} + +func (l *captureLogger) Debug(string, ...any) {} +func (l *captureLogger) Info(string, ...any) {} +func (l *captureLogger) Warn(format string, a ...any) { + l.warnings = append(l.warnings, fmt.Sprintf(format, a...)) +} +func (l *captureLogger) Err(string, ...any) {} +func (l *captureLogger) Log(logging.LogLevel, string, ...any) {} +func (l *captureLogger) SetLevel(logging.LogLevel) {} +func (l *captureLogger) With(...any) logging.SimpleLogging { return l } +func (l *captureLogger) WithHistory(...any) logging.SimpleLogging { return l } +func (l *captureLogger) GetHistory() string { return "" } +func (l *captureLogger) Flush() error { return nil } diff --git a/main.go b/main.go index ff6019859c..6426772c07 100644 --- a/main.go +++ b/main.go @@ -58,9 +58,15 @@ func main() { AtlantisVersion: atlantisVersion, Logger: logger, } + serverValidate := &cmd.ServerValidateCmd{ + Viper: viper.New(), + Logger: logger, + } version := &cmd.VersionCmd{AtlantisVersion: atlantisVersion} testdrive := &cmd.TestdriveCmd{} - cmd.RootCmd.AddCommand(server.Init()) + serverCmd := server.Init() + serverCmd.AddCommand(serverValidate.Init()) + cmd.RootCmd.AddCommand(serverCmd) cmd.RootCmd.AddCommand(version.Init()) cmd.RootCmd.AddCommand(testdrive.Init()) cmd.Execute()