-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: add server validate subcommand and unknown key warnings
#6307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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 TestFindUnknownKeys_WithWebhooksNestedConfig(t *testing.T) { | |
| t.Log("Should not report unknown keys for valid nested webhooks config.") | |
| v := viper.New() | |
| v.Set("repo-allowlist", "github.com/test/*") | |
| v.Set("webhooks", []map[string]any{ | |
| { | |
| "event": "apply", | |
| "workspace-regex": ".*", | |
| "kind": "slack", | |
| "secret": "secret", | |
| "url": "https://example.com/hook", | |
| }, | |
| }) | |
| unknowns := findUnknownKeys(v) | |
| Equals(t, 0, len(unknowns)) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findUnknownKeys() iterates over viper.AllKeys(), which includes flattened nested keys (e.g. a valid
webhooks:config will typically produce keys likewebhooks.0.event,webhooks.0.kind, etc.). Since buildKnownKeys() only includes top-level UserConfig tags (likewebhooks), valid nested config currently gets reported as unknown. Consider either validating only top-level keys (e.g. by iterating over viper.AllSettings() map keys or de-duping by the first segment before ".") or extending known-key generation to include nested key paths for structured fields like WebhookConfig.