diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index a0f18ffbf5..3a9ddfb7b6 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -1046,16 +1046,8 @@ func (cmd *GenerateGitopsCommand) generateEULA() (string, error) { return path, nil } -// This struct is used to represent the MDM configuration that is used with GitOps. -// It includes an additonal end user license agreement (EULA) field, which is -// not present in the fleet.MDM struct. -type gitopsMDM struct { - fleet.MDM - EndUserLicenseAgreement string `json:"end_user_license_agreement,omitempty"` -} - func (cmd *GenerateGitopsCommand) generateMDM(mdm *fleet.MDM) (map[string]interface{}, error) { - t := reflect.TypeOf(gitopsMDM{}) + t := reflect.TypeFor[spec.GitOpsMDM]() result := map[string]interface{}{ jsonFieldName(t, "AppleServerURL"): mdm.AppleServerURL, jsonFieldName(t, "EndUserAuthentication"): mdm.EndUserAuthentication, diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index b23448872e..6f29019a7b 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -292,6 +292,29 @@ type Software struct { FleetMaintainedApps []fleet.MaintainedAppSpec `json:"fleet_maintained_apps"` } +// GitOpsMDM extends fleet.MDM with gitops-only fields that are not part of the server type. +type GitOpsMDM struct { + fleet.MDM + EndUserLicenseAgreement any `json:"end_user_license_agreement,omitempty"` +} + +// GitOpsOrgSettings defines the valid keys for the top-level `org_settings:` section. +// It embeds fleet.AppConfig for all standard settings and adds gitops-only keys +// that are extracted before the config is sent to the server API. +type GitOpsOrgSettings struct { + fleet.AppConfig + Secrets any `json:"secrets"` + CertificateAuthorities any `json:"certificate_authorities"` +} + +// GitOpsFleetSettings defines the valid keys for the top-level `settings:` section (fleet-level). +// It embeds fleet.TeamConfig for all standard settings and adds gitops-only keys +// that are extracted before the config is sent to the server API. +type GitOpsFleetSettings struct { + fleet.TeamConfig + Secrets any `json:"secrets"` +} + type GitOps struct { TeamID *uint TeamName *string @@ -527,7 +550,9 @@ func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, fileP return multierror.Append(multiError, MaybeParseTypeError(filePath, []string{"org_settings"}, err)) } noError := true + settingsFilePath := filePath if orgSettingsTop.Path != nil { + settingsFilePath = *orgSettingsTop.Path fileBytes, err := os.ReadFile(resolveApplyRelativePath(baseDir, *orgSettingsTop.Path)) if err != nil { noError = false @@ -568,6 +593,8 @@ func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, fileP } else { multiError = parseSecrets(result, multiError) } + // Validate unknown keys in org_settings section. + multiError = multierror.Append(multiError, validateYAMLKeys(raw, reflect.TypeFor[GitOpsOrgSettings](), settingsFilePath, []string{"org_settings"})...) // TODO: Validate that integrations.(jira|zendesk)[].api_token is not empty or fleet.MaskedPassword } return multiError @@ -579,7 +606,9 @@ func parseTeamSettings(raw json.RawMessage, result *GitOps, baseDir string, file return multierror.Append(multiError, MaybeParseTypeError(filePath, []string{"settings"}, err)) } noError := true + settingsFilePath := filePath if teamSettingsTop.Path != nil { + settingsFilePath = *teamSettingsTop.Path fileBytes, err := os.ReadFile(resolveApplyRelativePath(baseDir, *teamSettingsTop.Path)) if err != nil { noError = false @@ -622,6 +651,8 @@ func parseTeamSettings(raw json.RawMessage, result *GitOps, baseDir string, file // Validate webhook settings for regular teams multiError = validateTeamWebhookSettings(result.TeamSettings, multiError) } + // Validate unknown keys in team settings section. + multiError = multierror.Append(multiError, validateYAMLKeys(raw, reflect.TypeFor[GitOpsFleetSettings](), settingsFilePath, []string{"settings"})...) } return multiError } diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 4e75f9d0e7..3db40a605d 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -2445,6 +2445,160 @@ software: assert.Contains(t, err.Error(), "unknown_array_field") }) + t.Run("unknown key in org_settings", func(t *testing.T) { + t.Parallel() + config := ` +org_settings: + server_settings: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + unknown_org_field: true + secrets: +controls: +agent_options: +reports: +policies: +` + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, nil, nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown_org_field") + }) + + t.Run("unknown nested key in org_settings", func(t *testing.T) { + t.Parallel() + config := ` +org_settings: + server_settings: + server_url: https://fleet.example.com + unknown_server_field: true + org_info: + contact_url: https://example.com/contact + org_name: Test Org + secrets: +controls: +agent_options: +reports: +policies: +` + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, nil, nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown key "org_settings.server_settings.unknown_server_field"`) + }) + + t.Run("unknown key in org_settings with typo suggestion", func(t *testing.T) { + t.Parallel() + config := ` +org_settings: + server_settigns: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + secrets: +controls: +agent_options: +reports: +policies: +` + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, nil, nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), "org_settings.server_settigns") + assert.Contains(t, err.Error(), `did you mean "server_settings"?`) + }) + + t.Run("unknown key in fleet settings", func(t *testing.T) { + t.Parallel() + config := ` +name: FleetName +settings: + secrets: + unknown_fleet_field: true +agent_options: +controls: +reports: +policies: +software: +` + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown_fleet_field") + }) + + t.Run("unknown nested key in fleet settings webhook_settings", func(t *testing.T) { + t.Parallel() + config := ` +name: FleetName +settings: + secrets: + webhook_settings: + unknown_webhook_field: true +agent_options: +controls: +reports: +policies: +software: +` + path, basePath := createTempFile(t, "", config) + _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown key "settings.webhook_settings.unknown_webhook_field"`) + }) + + t.Run("unknown key in org_settings via path", func(t *testing.T) { + t.Parallel() + config := ` +org_settings: + path: org_settings.yml +controls: +agent_options: +reports: +policies: +` + path, basePath := createTempFile(t, "", config) + orgSettingsYAML := ` +server_settings: + server_url: https://fleet.example.com +org_info: + contact_url: https://example.com/contact + org_name: Test Org +unknown_org_path_field: true +secrets: +` + require.NoError(t, os.WriteFile(filepath.Join(basePath, "org_settings.yml"), []byte(orgSettingsYAML), 0o644)) + _, err := GitOpsFromFile(path, basePath, nil, nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown key "org_settings.unknown_org_path_field" in "org_settings.yml"`) + }) + + t.Run("unknown key in fleet settings via path", func(t *testing.T) { + t.Parallel() + config := ` +name: FleetName +settings: + path: fleet_settings.yml +agent_options: +controls: +reports: +policies: +software: +` + path, basePath := createTempFile(t, "", config) + fleetSettingsYAML := ` +secrets: +unknown_fleet_path_field: true +` + require.NoError(t, os.WriteFile(filepath.Join(basePath, "fleet_settings.yml"), []byte(fleetSettingsYAML), 0o644)) + _, err := GitOpsFromFile(path, basePath, premiumAppConfig(), nopLogf) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown key "settings.unknown_fleet_path_field" in "fleet_settings.yml"`) + }) + t.Run("unknown key in policy install_software package_path", func(t *testing.T) { t.Parallel() config := getTeamConfig([]string{"policies", "software"}) diff --git a/pkg/spec/gitops_validate.go b/pkg/spec/gitops_validate.go index 6a7a752271..f6747bfb9f 100644 --- a/pkg/spec/gitops_validate.go +++ b/pkg/spec/gitops_validate.go @@ -19,6 +19,14 @@ type fieldInfo struct { typ reflect.Type } +// ValidKeysProvider is implemented by types with custom JSON marshaling +// that want to declare valid keys for gitops unknown-key validation. +type ValidKeysProvider interface { + ValidKeys() []string +} + +var validKeysProviderType = reflect.TypeFor[ValidKeysProvider]() + var ( knownKeysCache = make(map[reflect.Type]map[string]fieldInfo) knownKeysCacheMu sync.Mutex @@ -26,6 +34,7 @@ var ( // knownJSONKeys extracts the set of valid JSON field names from a struct type, // including fields from embedded structs. Results are cached per type. +// For types implementing ValidKeysProvider, the declared keys are used instead. func knownJSONKeys(t reflect.Type) map[string]fieldInfo { if t.Kind() == reflect.Ptr { t = t.Elem() @@ -42,7 +51,22 @@ func knownJSONKeys(t reflect.Type) map[string]fieldInfo { } keys := make(map[string]fieldInfo) - collectFields(t, keys) + + // If the type (or pointer to it) implements ValidKeysProvider, use those + // keys instead of reflecting on struct fields. This handles types with + // custom JSON marshaling (e.g. GoogleCalendarApiKey). + if reflect.PointerTo(t).Implements(validKeysProviderType) || t.Implements(validKeysProviderType) { + provider := reflect.New(t).Interface().(ValidKeysProvider) + for _, name := range provider.ValidKeys() { + keys[name] = fieldInfo{ + jsonName: name, + typ: reflect.TypeFor[any](), + } + } + } else { + collectFields(t, keys) + } + knownKeysCache[t] = keys return keys } @@ -108,6 +132,10 @@ var anyFieldTypes = map[reflect.Type]map[string]reflect.Type{ "windows_settings": reflect.TypeFor[fleet.WindowsSettings](), "android_settings": reflect.TypeFor[fleet.AndroidSettings](), }, + reflect.TypeFor[GitOpsOrgSettings](): { + "certificate_authorities": reflect.TypeFor[fleet.GroupedCertificateAuthorities](), + "mdm": reflect.TypeFor[GitOpsMDM](), + }, } // suggestKey returns the closest known key name if one is within a reasonable @@ -159,7 +187,9 @@ func validateMapKeys(data map[string]any, targetType reflect.Type, path []string } known := knownJSONKeys(targetType) - if known == nil { + if len(known) == 0 { + // No JSON-tagged fields: either not a struct or a struct with custom + // serialization. Skip validation since we don't know the expected keys. return nil } @@ -181,13 +211,13 @@ func validateMapKeys(data map[string]any, targetType reflect.Type, path []string // Determine the type to recurse into. fieldType := fi.typ - // If the field type is `any` (interface{}), check the override registry. - if fieldType.Kind() == reflect.Interface { - if override, ok := parentOverrides[key]; ok { // indexing a nil map is safe; ok will be false - fieldType = override - } else { - continue // any-typed field with no override, skip - } + // Check the override registry for this field. This handles two cases: + // 1. `any`/`interface{}` fields that need a concrete type for recursion + // 2. Struct fields that need a gitops-extended type (e.g. fleet.MDM -> GitOpsMDM) + if override, ok := parentOverrides[key]; ok { + fieldType = override + } else if fieldType.Kind() == reflect.Interface { + continue // any-typed field with no override, skip } // Recurse into nested structs or slices. diff --git a/pkg/spec/gitops_validate_test.go b/pkg/spec/gitops_validate_test.go index 0c5cec9b55..10bee47175 100644 --- a/pkg/spec/gitops_validate_test.go +++ b/pkg/spec/gitops_validate_test.go @@ -129,6 +129,42 @@ func TestValidateUnknownKeys(t *testing.T) { assert.Len(t, errs, 2) }) + t.Run("ValidKeysProvider accepts declared keys", func(t *testing.T) { + // GoogleCalendarApiKey implements ValidKeysProvider to declare accepted + // keys for its custom JSON marshaling. + data := map[string]any{ + "google_calendar": []any{ + map[string]any{ + "domain": "example.com", + "api_key_json": map[string]any{ + "client_email": "test@example.com", + "private_key": "some value", + }, + }, + }, + } + errs := validateUnknownKeys(data, reflect.TypeFor[fleet.Integrations](), []string{"org_settings", "integrations"}, "test.yml") + assert.Empty(t, errs) + }) + + t.Run("ValidKeysProvider rejects undeclared keys", func(t *testing.T) { + data := map[string]any{ + "google_calendar": []any{ + map[string]any{ + "domain": "example.com", + "api_key_json": map[string]any{ + "client_email": "test@example.com", + "private_key": "nothing to see here", + "bad_field": "unknown", + }, + }, + }, + } + errs := validateUnknownKeys(data, reflect.TypeFor[fleet.Integrations](), []string{"org_settings", "integrations"}, "test.yml") + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Error(), "bad_field") + }) + 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) @@ -308,6 +344,26 @@ func TestAnyFieldTypeRegistry(t *testing.T) { assert.Contains(t, overrides, "windows_settings") assert.Contains(t, overrides, "android_settings") }) + + t.Run("org_settings certificate_authorities any-field recursion", func(t *testing.T) { + data := map[string]any{ + "certificate_authorities": map[string]any{ + "ndes_scep_proxy": map[string]any{}, + "digicert": []any{}, + "unknown_ca_type": "bad", + }, + } + errs := validateUnknownKeys(data, reflect.TypeFor[GitOpsOrgSettings](), []string{"org_settings"}, "test.yml") + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Error(), "unknown_ca_type") + assert.Contains(t, errs[0].Error(), "org_settings.certificate_authorities") + }) + + t.Run("org_settings registered types present", func(t *testing.T) { + overrides, ok := anyFieldTypes[reflect.TypeFor[GitOpsOrgSettings]()] + require.True(t, ok) + assert.Contains(t, overrides, "certificate_authorities") + }) } func TestValidateRawKeys(t *testing.T) { diff --git a/server/fleet/integrations.go b/server/fleet/integrations.go index 479e51cb8b..d9ea478cc9 100644 --- a/server/fleet/integrations.go +++ b/server/fleet/integrations.go @@ -389,6 +389,10 @@ const ( GoogleCalendarPrivateKey = "private_key" ) +// googleCalendarKeyNames lists all valid JSON keys for GoogleCalendarApiKey, +// used by ValidKeys() for gitops unknown-key validation. +var googleCalendarKeyNames = []string{GoogleCalendarEmail, GoogleCalendarPrivateKey} + // GoogleCalendarApiKey is a custom type for the Google Calendar API key JSON. // It handles JSON marshaling/unmarshaling with support for masking sensitive data. // When marshaled in masked state, it serializes to just "********". @@ -400,6 +404,13 @@ type GoogleCalendarApiKey struct { masked bool } +// ValidKeys returns the set of accepted JSON keys for this type. +// This is used by gitops validation to check for unknown keys in types +// with custom JSON marshaling. +func (GoogleCalendarApiKey) ValidKeys() []string { + return googleCalendarKeyNames +} + // MarshalJSON implements json.Marshaler. When masked, returns "********". // Otherwise, returns the JSON object representation of the values. func (k GoogleCalendarApiKey) MarshalJSON() ([]byte, error) { @@ -566,7 +577,7 @@ func ValidateGoogleCalendarIntegrations(intgs []*GoogleCalendarIntegration, inva ) } } - if privateKey, ok := intg.ApiKey.Values["private_key"]; !ok { + if privateKey, ok := intg.ApiKey.Values[GoogleCalendarPrivateKey]; !ok { invalid.Append( fmt.Sprintf("integrations.google_calendar.api_key_json.%s", GoogleCalendarPrivateKey), fmt.Sprintf("%s is required", GoogleCalendarPrivateKey),