fleet/pkg/spec/gitops_validate_test.go
Scott Gress d5eee802eb
Detect unknown keys in GitOps (phase 1) (#40963)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #40496

# Details

This is the first phase of an effort to detect unknown keys in GitOps
.yml files. In the regular `fleetctl gitops` case, it will fail when
unknown keys are detected. This behavior can be changed with a new
`--allow-unknown-keys` flag which will log the issues and continue.

In this first phase we are detecting unknown keys in _most_ GitOps
sections, other than the top-level `org_settings:` and `settings:`
sections which have more complicated typing. I will tackle those
separately as they require a bit more thought. Also ultimately I'd like
us to be doing this validation in a more top-down fashion in one place,
rather than spreading it across the code by doing it in each individual
section, but this is a good first step.

As a bonus, I invited my pal Mr. Levenshtein to the party so that we can
make suggestions when unknown keys are detected, like:

```
 * unknown key "queyr" in "./lib/some-report.yml"; did you mean "query"?
```
> Note: the goal is to return as many validation errors as possible to
the user, so they don't have to keep running `fleetctl gitops` to get
the next error. I did _not_ update any other errors to stop returning
early, in an effort to keep this as low-touch as possible.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

## Testing

- [X] Added/updated automated tests
- [X] QA'd all new/changed functionality manually
- [X] Tested this against existing it-and-security folder and one with
updated keys from https://github.com/fleetdm/fleet/pull/40959; no
unknown keys detected
- [X] Added unknown keys at various levels, GitOps errored with helpful
messages
- [X] Same as above but with `--allow-unknown-keys`; GitOps outputted
helpful messages but continued.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* GitOps runs now fail when unknown or misspelled keys are present in
configuration files.
* New CLI flag --allow-unknown-keys lets unknown keys be treated as
warnings instead of errors.
* Unknown-key messages include suggested valid key names to help correct
mistakes.

* **Tests**
* Expanded test coverage to validate unknown-key detection and the
allow-as-warning option.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ian Littman <iansltx@gmail.com>
2026-03-06 16:16:17 -06:00

408 lines
13 KiB
Go

package spec
import (
"errors"
"fmt"
"reflect"
"testing"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestKnownJSONKeys(t *testing.T) {
t.Parallel()
t.Run("simple struct", func(t *testing.T) {
type Simple struct {
Name string `json:"name"`
Age int `json:"age,omitempty"`
}
keys := knownJSONKeys(reflect.TypeFor[Simple]())
require.Len(t, keys, 2)
assert.Contains(t, keys, "name")
assert.Contains(t, keys, "age")
})
t.Run("embedded struct", func(t *testing.T) {
// Label embeds BaseItem and fleet.LabelSpec
keys := knownJSONKeys(reflect.TypeFor[Label]())
// Should have path/paths from BaseItem + all LabelSpec fields
assert.Contains(t, keys, "path")
assert.Contains(t, keys, "paths")
assert.Contains(t, keys, "name")
assert.Contains(t, keys, "query")
assert.Contains(t, keys, "description")
})
t.Run("json dash excluded", func(t *testing.T) {
type WithDash struct {
Name string `json:"name"`
Ignored string `json:"-"`
}
keys := knownJSONKeys(reflect.TypeFor[WithDash]())
assert.Contains(t, keys, "name")
assert.NotContains(t, keys, "-")
assert.NotContains(t, keys, "Ignored")
})
t.Run("no json tag excluded", func(t *testing.T) {
type NoTag struct {
Name string `json:"name"`
NoTag string
Defined bool
}
keys := knownJSONKeys(reflect.TypeFor[NoTag]())
assert.Contains(t, keys, "name")
assert.NotContains(t, keys, "NoTag")
assert.NotContains(t, keys, "Defined")
})
t.Run("pointer type", func(t *testing.T) {
keys := knownJSONKeys(reflect.TypeFor[*GitOpsControls]())
assert.Contains(t, keys, "macos_updates")
assert.Contains(t, keys, "scripts")
// From BaseItem
assert.Contains(t, keys, "path")
})
t.Run("renameto alias accepted", func(t *testing.T) {
// PolicySpec has `json:"team" renameto:"fleet"` — both should be known
keys := knownJSONKeys(reflect.TypeFor[fleet.PolicySpec]())
assert.Contains(t, keys, "team")
assert.Contains(t, keys, "fleet")
// LabelSpec has `json:"team_id" renameto:"fleet_id"`
keys = knownJSONKeys(reflect.TypeFor[fleet.LabelSpec]())
assert.Contains(t, keys, "team_id")
assert.Contains(t, keys, "fleet_id")
})
t.Run("caching works", func(t *testing.T) {
t1 := reflect.TypeFor[Label]()
keys1 := knownJSONKeys(t1)
keys2 := knownJSONKeys(t1)
// Should return the same map (pointer equality after caching)
assert.Equal(t, keys1, keys2)
})
}
func TestValidateUnknownKeys(t *testing.T) {
t.Parallel()
t.Run("no unknown keys", func(t *testing.T) {
data := map[string]any{
"name": "test-query",
"query": "SELECT 1",
"description": "a test",
}
errs := validateUnknownKeys(data, reflect.TypeFor[fleet.QuerySpec](), []string{"reports", "[0]"}, "test.yml")
assert.Empty(t, errs)
})
t.Run("unknown key detected", func(t *testing.T) {
data := map[string]any{
"name": "test-query",
"query": "SELECT 1",
"unknown_field": "bad",
}
errs := validateUnknownKeys(data, reflect.TypeFor[fleet.QuerySpec](), []string{"reports", "[0]"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "unknown_field")
assert.Contains(t, errs[0].Error(), "reports.[0]")
assert.Contains(t, errs[0].Error(), "test.yml")
var unknownErr *ParseUnknownKeyError
require.ErrorAs(t, errs[0], &unknownErr)
assert.Equal(t, "unknown_field", unknownErr.Field)
})
t.Run("multiple unknown keys", func(t *testing.T) {
data := map[string]any{
"name": "test",
"bad1": "x",
"bad2": "y",
}
errs := validateUnknownKeys(data, reflect.TypeFor[fleet.QuerySpec](), []string{"reports"}, "test.yml")
assert.Len(t, errs, 2)
})
t.Run("scalar data no errors", func(t *testing.T) {
errs := validateUnknownKeys("just a string", reflect.TypeFor[fleet.QuerySpec](), nil, "test.yml")
assert.Empty(t, errs)
})
t.Run("nil data no errors", func(t *testing.T) {
errs := validateUnknownKeys(nil, reflect.TypeFor[fleet.QuerySpec](), nil, "test.yml")
assert.Empty(t, errs)
})
t.Run("nested struct validation", func(t *testing.T) {
// MacOSSetup is a pointer-to-struct field in GitOpsControls
data := map[string]any{
"macos_setup": map[string]any{
"bootstrap_package": "pkg.pkg",
"bad_nested_field": true,
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "bad_nested_field")
assert.Contains(t, errs[0].Error(), "controls.macos_setup")
})
t.Run("slice validation", func(t *testing.T) {
data := []any{
map[string]any{
"name": "query1",
"query": "SELECT 1",
"bad_field": "x",
},
map[string]any{
"name": "query2",
"query": "SELECT 2",
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[[]Query](), []string{"reports"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "bad_field")
})
t.Run("non-struct target type", func(t *testing.T) {
data := map[string]any{
"anything": "goes",
}
errs := validateUnknownKeys(data, reflect.TypeFor[string](), nil, "test.yml")
assert.Empty(t, errs)
})
}
func TestSuggestKey(t *testing.T) {
t.Parallel()
known := knownJSONKeys(reflect.TypeFor[fleet.QuerySpec]())
t.Run("close typo suggests match", func(t *testing.T) {
assert.Equal(t, "query", suggestKey("qurey", known))
assert.Equal(t, "query", suggestKey("qeury", known))
assert.Equal(t, "name", suggestKey("nme", known))
assert.Equal(t, "interval", suggestKey("intervl", known))
assert.Equal(t, "description", suggestKey("desciption", known))
})
t.Run("completely unrelated no suggestion", func(t *testing.T) {
assert.Empty(t, suggestKey("zzzzzzzzz", known))
assert.Empty(t, suggestKey("xylophone", known))
})
t.Run("suggestion included in error message", func(t *testing.T) {
data := map[string]any{
"name": "q",
"qurey": "SELECT 1",
}
errs := validateUnknownKeys(data, reflect.TypeFor[fleet.QuerySpec](), []string{"reports"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), `did you mean "query"`)
var unknownErr *ParseUnknownKeyError
require.ErrorAs(t, errs[0], &unknownErr)
assert.Equal(t, "query", unknownErr.Suggestion)
})
t.Run("no suggestion when too distant", func(t *testing.T) {
data := map[string]any{
"name": "q",
"xylophone": "SELECT 1",
}
errs := validateUnknownKeys(data, reflect.TypeFor[fleet.QuerySpec](), []string{"reports"}, "test.yml")
require.Len(t, errs, 1)
assert.NotContains(t, errs[0].Error(), "did you mean")
var unknownErr *ParseUnknownKeyError
require.ErrorAs(t, errs[0], &unknownErr)
assert.Empty(t, unknownErr.Suggestion)
})
t.Run("nested path recorded on error", func(t *testing.T) {
data := map[string]any{
"macos_updates": map[string]any{
"deadlinee": "2024-01-01",
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "team.yml")
require.Len(t, errs, 1)
var unknownErr *ParseUnknownKeyError
require.ErrorAs(t, errs[0], &unknownErr)
assert.Equal(t, "controls.macos_updates", unknownErr.Path)
assert.Equal(t, "deadlinee", unknownErr.Field)
assert.Equal(t, "team.yml", unknownErr.Filename)
})
}
func TestAnyFieldTypeRegistry(t *testing.T) {
t.Parallel()
t.Run("controls any-field recursion", func(t *testing.T) {
data := map[string]any{
"macos_updates": map[string]any{
"minimum_version": "14.0",
"deadline": "2024-01-01",
"deadlinee": "typo",
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "deadlinee")
assert.Contains(t, errs[0].Error(), "controls.macos_updates")
assert.Contains(t, errs[0].Error(), `did you mean "deadline"`)
})
t.Run("windows_updates any-field recursion", func(t *testing.T) {
data := map[string]any{
"windows_updates": map[string]any{
"deadline_days": 5,
"grace_period_das": 2, // typo
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "grace_period_das")
})
t.Run("macos_settings any-field recursion", func(t *testing.T) {
data := map[string]any{
"macos_settings": map[string]any{
"custom_settings": []any{},
"custom_settingss": "typo",
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "custom_settingss")
})
t.Run("android_settings any-field recursion", func(t *testing.T) {
data := map[string]any{
"android_settings": map[string]any{
"custom_settings": []any{},
"certificatess": "typo",
},
}
errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsControls](), []string{"controls"}, "test.yml")
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "certificatess")
})
t.Run("all registered types present", func(t *testing.T) {
overrides, ok := anyFieldTypes[reflect.TypeFor[GitOpsControls]()]
require.True(t, ok)
assert.Contains(t, overrides, "macos_updates")
assert.Contains(t, overrides, "ios_updates")
assert.Contains(t, overrides, "ipados_updates")
assert.Contains(t, overrides, "macos_migration")
assert.Contains(t, overrides, "windows_updates")
assert.Contains(t, overrides, "macos_settings")
assert.Contains(t, overrides, "windows_settings")
assert.Contains(t, overrides, "android_settings")
})
}
func TestValidateRawKeys(t *testing.T) {
t.Parallel()
t.Run("valid json", func(t *testing.T) {
raw := []byte(`{"name":"test","query":"SELECT 1"}`)
errs := validateRawKeys(raw, reflect.TypeFor[fleet.QuerySpec](), "test.yml", []string{"reports"})
assert.Empty(t, errs)
})
t.Run("unknown key in json", func(t *testing.T) {
raw := []byte(`{"name":"test","query":"SELECT 1","typo_field":"bad"}`)
errs := validateRawKeys(raw, reflect.TypeFor[fleet.QuerySpec](), "test.yml", []string{"reports"})
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "typo_field")
})
t.Run("invalid json returns parse error", func(t *testing.T) {
raw := []byte(`{invalid json`)
errs := validateRawKeys(raw, reflect.TypeFor[fleet.QuerySpec](), "test.yml", []string{"reports"})
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "invalid")
})
}
func TestFilterWarnings(t *testing.T) {
t.Parallel()
t.Run("nil multierror", func(t *testing.T) {
err := filterWarnings(nil, func(string, ...any) {}, reflect.TypeFor[*ParseUnknownKeyError]())
assert.NoError(t, err)
})
t.Run("filters matching errors and logs them", func(t *testing.T) {
multiError := &multierror.Error{}
multiError = multierror.Append(multiError,
&ParseUnknownKeyError{Filename: "test.yml", Field: "bad_key"},
errors.New("some other error"),
&ParseUnknownKeyError{Filename: "test.yml", Field: "another_bad"},
)
var warnings []string
logFn := func(format string, args ...any) {
warnings = append(warnings, fmt.Sprintf(format, args...))
}
result := filterWarnings(multiError, logFn, reflect.TypeFor[*ParseUnknownKeyError]())
require.Error(t, result)
assert.Contains(t, result.Error(), "some other error")
assert.NotContains(t, result.Error(), "bad_key")
assert.Len(t, warnings, 2)
assert.Contains(t, warnings[0], "bad_key")
assert.Contains(t, warnings[1], "another_bad")
})
t.Run("returns nil when all errors filtered", func(t *testing.T) {
multiError := &multierror.Error{}
multiError = multierror.Append(multiError,
&ParseUnknownKeyError{Filename: "test.yml", Field: "bad1"},
&ParseUnknownKeyError{Filename: "test.yml", Field: "bad2"},
)
result := filterWarnings(multiError, func(string, ...any) {}, reflect.TypeFor[*ParseUnknownKeyError]())
assert.NoError(t, result)
})
t.Run("preserves all errors when none match", func(t *testing.T) {
multiError := &multierror.Error{}
multiError = multierror.Append(multiError,
errors.New("error one"),
errors.New("error two"),
)
result := filterWarnings(multiError, func(string, ...any) {}, reflect.TypeFor[*ParseUnknownKeyError]())
require.Error(t, result)
var resultMulti *multierror.Error
require.True(t, errors.As(result, &resultMulti))
assert.Len(t, resultMulti.Errors, 2)
})
t.Run("multiple filter types", func(t *testing.T) {
multiError := &multierror.Error{}
multiError = multierror.Append(multiError,
&ParseUnknownKeyError{Filename: "test.yml", Field: "bad"},
&ParseTypeError{Filename: "test.yml", Keys: []string{"controls"}},
errors.New("kept error"),
)
result := filterWarnings(multiError, func(string, ...any) {},
reflect.TypeFor[*ParseUnknownKeyError](),
reflect.TypeFor[*ParseTypeError](),
)
require.Error(t, result)
assert.Contains(t, result.Error(), "kept error")
assert.NotContains(t, result.Error(), "bad")
})
}