From b2254a1c90d44145dcfbb117f40951456c775336 Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Sun, 9 Nov 2025 01:05:07 +0000 Subject: [PATCH] (chore): Add JSONSchema validation for bundle configuration ClusterExtension configuration is now validated using JSONSchema. Configuration errors (typos, missing required fields, wrong types) are caught immediately with clear error messages instead of failing during installation. Assisted-by: Cursor --- go.mod | 2 +- .../operator-controller/applier/provider.go | 20 +- .../applier/provider_test.go | 10 +- internal/operator-controller/config/config.go | 380 ++++++++++++ .../operator-controller/config/config_test.go | 574 ++++++++++++++++++ .../config/error_formatting_test.go | 174 ++++++ .../rukpak/bundle/config.go | 124 ---- .../rukpak/bundle/config_test.go | 310 ---------- .../rukpak/bundle/registryv1.go | 134 ++++ test/e2e/single_namespace_support_test.go | 8 +- 10 files changed, 1284 insertions(+), 452 deletions(-) create mode 100644 internal/operator-controller/config/config.go create mode 100644 internal/operator-controller/config/config_test.go create mode 100644 internal/operator-controller/config/error_formatting_test.go delete mode 100644 internal/operator-controller/rukpak/bundle/config.go delete mode 100644 internal/operator-controller/rukpak/bundle/config_test.go diff --git a/go.mod b/go.mod index 04a654811d..bf213d3772 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/operator-framework/operator-registry v1.61.0 github.com/prometheus/client_golang v1.23.2 github.com/prometheus/common v0.67.2 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/cobra v1.10.1 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 @@ -192,7 +193,6 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/rubenv/sql-migrate v1.8.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect - github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect github.com/secure-systems-lab/go-securesystemslib v0.9.1 // indirect github.com/shopspring/decimal v1.4.0 // indirect github.com/sigstore/fulcio v1.7.1 // indirect diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index ffb5eb559b..f34ea7585f 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -13,7 +13,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/config" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" ) @@ -69,19 +69,19 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens } if r.IsSingleOwnNamespaceEnabled { - bundleConfigBytes := extensionConfigBytes(ext) - // treat no config as empty to properly validate the configuration - // e.g. ensure that validation catches missing required fields - if bundleConfigBytes == nil { - bundleConfigBytes = []byte(`{}`) + schema, err := rv1.Get() + if err != nil { + return nil, fmt.Errorf("error getting configuration schema: %w", err) } - bundleConfig, err := bundle.UnmarshalConfig(bundleConfigBytes, rv1, ext.Spec.Namespace) + + bundleConfigBytes := extensionConfigBytes(ext) + bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace) if err != nil { - return nil, fmt.Errorf("invalid bundle configuration: %w", err) + return nil, fmt.Errorf("invalid ClusterExtension configuration: %w", err) } - if bundleConfig != nil && bundleConfig.WatchNamespace != nil { - opts = append(opts, render.WithTargetNamespaces(*bundleConfig.WatchNamespace)) + if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil { + opts = append(opts, render.WithTargetNamespaces(*watchNS)) } } diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 4ec20beadb..4138284a05 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -97,7 +97,7 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { _, err := provider.Get(bundleFS, ext) require.Error(t, err) - require.Contains(t, err.Error(), "invalid bundle configuration") + require.Contains(t, err.Error(), "invalid ClusterExtension configuration") }) t.Run("returns rendered manifests", func(t *testing.T) { @@ -326,7 +326,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing") + require.Contains(t, err.Error(), `required field "watchNamespace" is missing`) }) t.Run("accepts bundles with {OwnNamespace} install modes when the appropriate configuration is given", func(t *testing.T) { @@ -371,7 +371,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing") + require.Contains(t, err.Error(), `required field "watchNamespace" is missing`) }) t.Run("rejects bundles with {OwnNamespace} install modes when watchNamespace is not install namespace", func(t *testing.T) { @@ -392,7 +392,9 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { }, }) require.Error(t, err) - require.Contains(t, err.Error(), "invalid 'watchNamespace' \"not-install-namespace\": must be install namespace (install-namespace)") + require.Contains(t, err.Error(), "invalid ClusterExtension configuration:") + require.Contains(t, err.Error(), "watchNamespace must be") + require.Contains(t, err.Error(), "install-namespace") }) t.Run("rejects bundles without AllNamespaces, SingleNamespace, or OwnNamespace install mode support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) { diff --git a/internal/operator-controller/config/config.go b/internal/operator-controller/config/config.go new file mode 100644 index 0000000000..dc1e75d073 --- /dev/null +++ b/internal/operator-controller/config/config.go @@ -0,0 +1,380 @@ +// Package config validates configuration for different package format types. +// +// How it works: +// +// Each package format type (like registry+v1 or Helm) knows what configuration it accepts. +// When a user provides configuration, we validate it before creating a Config object. +// Once created, a Config is guaranteed to be valid - you never need to check it again. +// +// The validation uses JSON Schema: +// 1. Bundle provides its schema (what config is valid) +// 2. We validate the user's config against that schema +// 3. If valid, we create a Config object +// 4. If invalid, we return a helpful error message +// +// Design choices: +// +// - Validation happens once, when creating the Config. There's no Validate() method +// because once you have a Config, it's already been validated. +// +// - Config doesn't hold onto the schema. We only need the schema during validation, +// not after the Config is created. +// +// - You can't create a Config directly. You must go through UnmarshalConfig so that +// validation always happens. +package config + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + + "github.com/santhosh-tekuri/jsonschema/v6" + "sigs.k8s.io/yaml" +) + +const ( + // configSchemaID is a name we use to identify the config schema when compiling it. + // Think of it like a file name - it just needs to be consistent. + configSchemaID = "config-schema.json" + + // FormatOwnNamespaceInstallMode defines the format check to ensure that + // the watchNamespace must equal install namespace + FormatOwnNamespaceInstallMode = "ownNamespaceInstallMode" + // FormatSingleNamespaceInstallMode defines the format check to ensure that + // the watchNamespace must differ from install namespace + FormatSingleNamespaceInstallMode = "singleNamespaceInstallMode" +) + +// SchemaProvider lets each package format type describe what configuration it accepts. +// +// Different package format types provide schemas in different ways: +// - registry+v1: builds schema from the operator's install modes +// - Helm: reads schema from values.schema.json in the chart +// - registry+v2: (future) will have its own way +// +// Note: We don't store this in the Config struct. We only need it when validating +// the user's input. After that, the Config has the validated data and doesn't need +// the schema anymore. +type SchemaProvider interface { + // Get returns a JSON Schema describing what configuration is valid. + // Returns nil if this package format type doesn't need configuration validation. + Get() (map[string]any, error) +} + +// Config holds validated configuration data from a ClusterExtension. +// +// Different package format types have different configuration options, so we store +// the data in a flexible format and provide accessor methods to get values out. +// +// Why there's no Validate() method: +// We validate configuration when creating a Config. If you have a Config object, +// it's already been validated - you don't need to check it again. The unexported +// 'data' field means you can't create a Config yourself; you have to use +// UnmarshalConfig, which does the validation. +type Config struct { + data map[string]any +} + +// newConfig creates a Config from already-validated data. +// This is unexported so all Configs must be created through UnmarshalConfig, +// which ensures validation always happens. +func newConfig(data map[string]any) *Config { + return &Config{data: data} +} + +// GetWatchNamespace returns the watchNamespace value if present in the configuration. +// Returns nil if watchNamespace is not set or is explicitly set to null. +func (c *Config) GetWatchNamespace() *string { + if c == nil || c.data == nil { + return nil + } + val, exists := c.data["watchNamespace"] + if !exists { + return nil + } + // User set watchNamespace: null - treat as "not configured" + if val == nil { + return nil + } + // User set watchNamespace: "some-value" - return it + if str, ok := val.(string); ok { + return &str + } + // Shouldn't happen with schema validation, but handle non-string values + str := fmt.Sprintf("%v", val) + return &str +} + +// UnmarshalConfig takes user configuration, validates it, and creates a Config object. +// This is the only way to create a Config. +// +// What it does: +// 1. Checks the user's configuration against the schema (if provided) +// 2. If valid, creates a Config object +// 3. If invalid, returns an error explaining what's wrong +// +// Parameters: +// - bytes: the user's configuration in YAML or JSON. If nil, we treat it as empty ({}) +// - schema: describes what configuration is valid. If nil, we skip validation +// - installNamespace: the namespace where the operator will be installed. We use this +// to validate namespace constraints (e.g., OwnNamespace mode requires watchNamespace +// to equal installNamespace) +// +// If the user doesn't provide any configuration but the package format type requires some fields +// (like watchNamespace), validation will fail with a helpful error. +func UnmarshalConfig(bytes []byte, schema map[string]any, installNamespace string) (*Config, error) { + // nil config becomes {} so we can validate required fields + if bytes == nil { + bytes = []byte("{}") + } + + // Step 1: Validate against the schema if provided + if schema != nil { + if err := validateConfigWithSchema(bytes, schema, installNamespace); err != nil { + return nil, fmt.Errorf("invalid configuration: %w", err) + } + } + + // Step 2: Parse into Config struct + // We use yaml.Unmarshal to parse the validated config into an opaque map. + // Schema validation has already ensured correctness. + var configData map[string]any + if err := yaml.Unmarshal(bytes, &configData); err != nil { + return nil, fmt.Errorf("error unmarshalling configuration: %w", formatUnmarshalError(err)) + } + + return newConfig(configData), nil +} + +// validateConfigWithSchema checks if the user's config matches the schema. +// +// We create a fresh validator each time because the namespace constraints depend on +// which namespace this specific ClusterExtension is being installed into. Each +// ClusterExtension might have a different installNamespace, so we can't reuse validators. +func validateConfigWithSchema(configBytes []byte, schema map[string]any, installNamespace string) error { + var configData interface{} + if err := yaml.Unmarshal(configBytes, &configData); err != nil { + return formatUnmarshalError(err) + } + + compiler := jsonschema.NewCompiler() + + compiler.RegisterFormat(&jsonschema.Format{ + Name: FormatOwnNamespaceInstallMode, + Validate: func(value interface{}) error { + // Check it equals install namespace (if installNamespace is set) + // If installNamespace is empty, we can't validate the constraint properly, + // so we skip validation and accept any value. This is a fallback for edge + // cases where the install namespace isn't known yet. + if installNamespace == "" { + return nil + } + str, ok := value.(string) + if !ok { + return fmt.Errorf("value must be a string") + } + if str != installNamespace { + return fmt.Errorf("invalid value %q: watchNamespace must be %q (the namespace where the operator is installed) because this operator only supports OwnNamespace install mode", str, installNamespace) + } + return nil + }, + }) + compiler.RegisterFormat(&jsonschema.Format{ + Name: FormatSingleNamespaceInstallMode, + Validate: func(value interface{}) error { + // Check it does NOT equal install namespace (if installNamespace is set) + // If installNamespace is empty, we can't validate the constraint properly, + // so we skip validation and accept any value. This is a fallback for edge + // cases where the install namespace isn't known yet. + if installNamespace == "" { + return nil + } + str, ok := value.(string) + if !ok { + return fmt.Errorf("value must be a string") + } + if str == installNamespace { + return fmt.Errorf("invalid value %q: watchNamespace must be different from %q (the install namespace) because this operator uses SingleNamespace install mode to watch a different namespace", str, installNamespace) + } + return nil + }, + }) + + if err := compiler.AddResource(configSchemaID, schema); err != nil { + return fmt.Errorf("failed to load schema: %w", err) + } + + compiledSchema, err := compiler.Compile(configSchemaID) + if err != nil { + return fmt.Errorf("failed to compile schema: %w", err) + } + + if err := compiledSchema.Validate(configData); err != nil { + return formatSchemaError(err) + } + + return nil +} + +// formatSchemaError converts JSON schema validation errors into user-friendly messages. +// If multiple validation errors exist, it combines them into a single error message. +func formatSchemaError(err error) error { + ve := &jsonschema.ValidationError{} + ok := errors.As(err, &ve) + if !ok { + // Not a ValidationError, return as-is + // Caller (UnmarshalConfig) will add "invalid configuration:" prefix + return err + } + + // Use BasicOutput() to get structured error information + // This is more robust than parsing error strings + output := ve.BasicOutput() + if output == nil || len(output.Errors) == 0 { + // No structured errors available, fallback to error message + // Note: Using %s not %w since ve.Error() is already a formatted string + return fmt.Errorf("%s", ve.Error()) + } + + // Collect all error messages + var errorMessages []string + for _, errUnit := range output.Errors { + msg := formatSingleError(errUnit) + if msg != "" { + errorMessages = append(errorMessages, msg) + } + } + + if len(errorMessages) == 0 { + return fmt.Errorf("invalid configuration: %s", ve.Error()) + } + + // Single error - return it directly + if len(errorMessages) == 1 { + return errors.New(errorMessages[0]) + } + + // Multiple errors - combine them + return fmt.Errorf("multiple errors found:\n - %s", strings.Join(errorMessages, "\n - ")) +} + +// formatSingleError formats a single validation error from the schema library. +func formatSingleError(errUnit jsonschema.OutputUnit) string { + // Check the keyword location to identify the error type + switch { + case strings.Contains(errUnit.KeywordLocation, "/required"): + // Missing required field + fieldName := extractFieldNameFromMessage(errUnit.Error) + if fieldName != "" { + return fmt.Sprintf("required field %q is missing", fieldName) + } + return "required field is missing" + + case strings.Contains(errUnit.KeywordLocation, "/additionalProperties"): + // Unknown/additional field + fieldName := extractFieldNameFromMessage(errUnit.Error) + if fieldName != "" { + return fmt.Sprintf("unknown field %q", fieldName) + } + return "unknown field" + + case strings.Contains(errUnit.KeywordLocation, "/type"): + // Type mismatch (e.g., got null, want string) + fieldPath := buildFieldPath(errUnit.InstanceLocation) + if fieldPath != "" { + // Check if this is a "null instead of required value" case + if errUnit.Error != nil && strings.Contains(errUnit.Error.String(), "got null") { + return fmt.Sprintf("required field %q is missing", fieldPath) + } + return fmt.Sprintf("invalid type for field %q: %s", fieldPath, errUnit.Error.String()) + } + return fmt.Sprintf("invalid type: %s", errUnit.Error.String()) + + case strings.Contains(errUnit.KeywordLocation, "/format"): + // Custom format validation (e.g., OwnNamespace, SingleNamespace constraints) + // These already have good error messages from our custom validators + if errUnit.Error != nil { + return errUnit.Error.String() + } + fieldPath := buildFieldPath(errUnit.InstanceLocation) + return fmt.Sprintf("invalid format for field %q", fieldPath) + + case strings.Contains(errUnit.KeywordLocation, "/anyOf"): + // anyOf validation failed - could be null or wrong type + // This happens when a field accepts [null, string] but got something else + fieldPath := buildFieldPath(errUnit.InstanceLocation) + if fieldPath != "" { + return fmt.Sprintf("invalid value for field %q", fieldPath) + } + return "invalid value" + + default: + // Unknown error type - return the library's error message + // This serves as a fallback for future schema features we haven't customized yet + if errUnit.Error != nil { + return errUnit.Error.String() + } + return "" + } +} + +// extractFieldNameFromMessage extracts the field name from error messages. +// Example: "missing property 'watchNamespace'" -> "watchNamespace" +// Example: "additional properties 'unknownField' not allowed" -> "unknownField" +func extractFieldNameFromMessage(errOutput *jsonschema.OutputError) string { + if errOutput == nil { + return "" + } + msg := errOutput.String() + + // Look for field names in single quotes (library's format) + if idx := strings.Index(msg, "'"); idx != -1 { + remaining := msg[idx+1:] + if endIdx := strings.Index(remaining, "'"); endIdx != -1 { + return remaining[:endIdx] + } + } + + return "" +} + +// buildFieldPath constructs a field path from instance location array. +// Example: ["watchNamespace"] -> "watchNamespace" +// Example: ["spec", "namespace"] -> "spec.namespace" +func buildFieldPath(location string) string { + // Instance location comes as a JSON pointer like "/watchNamespace" + if location == "" || location == "/" { + return "" + } + // Remove leading slash + path := strings.TrimPrefix(location, "/") + // Replace JSON pointer slashes with dots for readability + path = strings.ReplaceAll(path, "/", ".") + return path +} + +// formatUnmarshalError makes YAML/JSON parsing errors easier to understand. +func formatUnmarshalError(err error) error { + var typeErr *json.UnmarshalTypeError + if errors.As(err, &typeErr) { + if typeErr.Field == "" { + return errors.New("input is not a valid JSON object") + } + return fmt.Errorf("invalid value type for field %q: expected %q but got %q", + typeErr.Field, typeErr.Type.String(), typeErr.Value) + } + + // Unwrap to core error and strip "json:" or "yaml:" prefix + current := err + for { + unwrapped := errors.Unwrap(current) + if unwrapped == nil { + parts := strings.Split(current.Error(), ":") + coreMessage := strings.TrimSpace(parts[len(parts)-1]) + return errors.New(coreMessage) + } + current = unwrapped + } +} diff --git a/internal/operator-controller/config/config_test.go b/internal/operator-controller/config/config_test.go new file mode 100644 index 0000000000..a98fdba7e3 --- /dev/null +++ b/internal/operator-controller/config/config_test.go @@ -0,0 +1,574 @@ +package config_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/config" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion" +) + +func Test_UnmarshalConfig(t *testing.T) { + for _, tc := range []struct { + name string + rawConfig []byte + supportedInstallModes []v1alpha1.InstallModeType + installNamespace string + expectedErrMessage string + expectedWatchNamespace *string // Expected value from GetWatchNamespace() + }{ + { + name: "accepts nil config when AllNamespaces is supported", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces}, + rawConfig: nil, + expectedWatchNamespace: nil, + }, + { + name: "rejects nil config when OwnNamespace-only requires watchNamespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: nil, + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects nil config when SingleNamespace-only requires watchNamespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: nil, + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "accepts json config", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "install-ns", // SingleNamespace requires watchNamespace != installNamespace + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "accepts yaml config", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`watchNamespace: some-namespace`), + installNamespace: "install-ns", // SingleNamespace requires watchNamespace != installNamespace + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "rejects invalid json", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"hello`), + expectedErrMessage: `invalid configuration: found unexpected end of stream`, + }, + { + name: "rejects valid json that isn't of object type", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`true`), + expectedErrMessage: `got boolean, want object`, + }, + { + name: "rejects additional fields", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces}, + rawConfig: []byte(`somekey: somevalue`), + expectedErrMessage: `unknown field "somekey"`, + }, + { + name: "rejects valid json but invalid registry+v1", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": {"hello": "there"}}`), + expectedErrMessage: `got object, want string`, + }, + { + name: "rejects with unknown field when install modes {AllNamespaces}", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + expectedErrMessage: `unknown field "watchNamespace"`, + }, + { + name: "rejects with unknown field when install modes {MultiNamespace}", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + expectedErrMessage: `unknown field "watchNamespace"`, + }, + { + name: "reject with unknown field when install modes {AllNamespaces, MultiNamespace}", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + expectedErrMessage: `unknown field "watchNamespace"`, + }, + { + name: "reject with required field when install modes {OwnNamespace} and watchNamespace is null", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "reject with required field when install modes {OwnNamespace} and watchNamespace is missing", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{}`), + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is null", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is missing", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{}`), + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "accepts when install modes {SingleNamespace} and watchNamespace != install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "install-ns", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "accepts when install modes {AllNamespaces, SingleNamespace} and watchNamespace != install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "install-ns", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "accepts when install modes {MultiNamespace, SingleNamespace} and watchNamespace != install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "install-ns", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "accepts when install modes {OwnNamespace, SingleNamespace} and watchNamespace != install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "not-namespace", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "rejects when install modes {SingleNamespace} and watchNamespace == install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "some-namespace", + expectedErrMessage: "invalid configuration:", + }, + { + name: "rejects when install modes {AllNamespaces, SingleNamespace} and watchNamespace == install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "some-namespace", + expectedErrMessage: "invalid configuration:", + }, + { + name: "rejects when install modes {MultiNamespace, SingleNamespace} and watchNamespace == install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "some-namespace", + expectedErrMessage: "invalid configuration:", + }, + { + name: "accepts when install modes {AllNamespaces, OwnNamespace} and watchNamespace == install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "some-namespace", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "accepts when install modes {OwnNamespace, SingleNamespace} and watchNamespace == install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "some-namespace", + expectedWatchNamespace: ptr.To("some-namespace"), + }, + { + name: "rejects when install modes {AllNamespaces, OwnNamespace} and watchNamespace != install namespace", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), + installNamespace: "not-some-namespace", + expectedErrMessage: "invalid configuration:", + }, + { + name: "rejects with required field error when install modes {SingleNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace} and watchNamespace is missing", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + rawConfig: []byte(`{}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is missing", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is missing", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace, MultiNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedErrMessage: `required field "watchNamespace" is missing`, + }, + { + name: "accepts null watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedWatchNamespace: nil, + }, + { + name: "accepts null watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{"watchNamespace": null}`), + installNamespace: "not-some-namespace", + expectedWatchNamespace: nil, + }, + { + name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{}`), + installNamespace: "not-some-namespace", + expectedWatchNamespace: nil, + }, + { + name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, + rawConfig: []byte(`{}`), + installNamespace: "not-some-namespace", + expectedWatchNamespace: nil, + }, + { + name: "skips validation when installNamespace is empty for OwnNamespace only", + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + rawConfig: []byte(`{"watchNamespace": "valid-ns"}`), + installNamespace: "", + expectedWatchNamespace: ptr.To("valid-ns"), + }, + } { + t.Run(tc.name, func(t *testing.T) { + var rv1 bundle.RegistryV1 + if tc.supportedInstallModes != nil { + rv1 = bundle.RegistryV1{ + CSV: clusterserviceversion.Builder(). + WithName("test-operator"). + WithInstallModeSupportFor(tc.supportedInstallModes...). + Build(), + } + } + + schema, err := rv1.Get() + require.NoError(t, err) + + cfg, err := config.UnmarshalConfig(tc.rawConfig, schema, tc.installNamespace) + if tc.expectedErrMessage != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErrMessage) + } else { + require.NoError(t, err) + require.NotNil(t, cfg) + if tc.expectedWatchNamespace == nil { + require.Nil(t, cfg.GetWatchNamespace()) + } else { + require.Equal(t, *tc.expectedWatchNamespace, *cfg.GetWatchNamespace()) + } + } + }) + } +} + +// Test_UnmarshalConfig_EmptySchema tests when a ClusterExtension doesn't provide a configuration schema. +func Test_UnmarshalConfig_EmptySchema(t *testing.T) { + for _, tc := range []struct { + name string + rawConfig []byte + expectedErrMessage string + expectedWatchNamespace *string + }{ + { + name: "no config provided", + rawConfig: nil, + expectedWatchNamespace: nil, + }, + { + name: "empty config provided", + rawConfig: []byte(`{}`), + expectedWatchNamespace: nil, + }, + { + name: "config with watchNamespace provided", + rawConfig: []byte(`{"watchNamespace": "some-ns"}`), + expectedWatchNamespace: ptr.To("some-ns"), + }, + { + name: "config with unknown fields provided", + rawConfig: []byte(`{"someField": "someValue"}`), + expectedWatchNamespace: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + emptySchemaBundle := &mockEmptySchemaBundle{} + schema, err := emptySchemaBundle.Get() + require.NoError(t, err) + + config, err := config.UnmarshalConfig(tc.rawConfig, schema, "my-namespace") + + if tc.expectedErrMessage != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErrMessage) + } else { + require.NoError(t, err) + require.NotNil(t, config) + if tc.expectedWatchNamespace == nil { + require.Nil(t, config.GetWatchNamespace()) + } else { + require.Equal(t, *tc.expectedWatchNamespace, *config.GetWatchNamespace()) + } + } + }) + } +} + +// Test_UnmarshalConfig_HelmLike proves validation works the same for ANY package format type. +// +// - registry+v1 -> generates schema from install modes +// - Helm -> reads values.schema.json from chart +// - registry+v2 -> (future) provides schema via its own mechanism +// +// Same validation process regardless of package format type. +func Test_UnmarshalConfig_HelmLike(t *testing.T) { + for _, tc := range []struct { + name string + rawConfig []byte + helmSchema string // what values.schema.json would contain + expectedErrMessage string + expectedWatchNamespace *string + }{ + { + name: "Helm chart with typical config values (no watchNamespace)", + rawConfig: []byte(`{ + "replicaCount": 3, + "image": {"tag": "v1.2.3"}, + "service": {"port": 8080} + }`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "replicaCount": {"type": "integer", "minimum": 1}, + "image": { + "type": "object", + "properties": { + "tag": {"type": "string"} + } + }, + "service": { + "type": "object", + "properties": { + "port": {"type": "integer"} + } + } + } + }`, + expectedWatchNamespace: nil, + }, + { + name: "Helm chart that ALSO uses watchNamespace (mixed config)", + rawConfig: []byte(`{ + "watchNamespace": "my-app-namespace", + "replicaCount": 2, + "debug": true + }`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "watchNamespace": {"type": "string"}, + "replicaCount": {"type": "integer"}, + "debug": {"type": "boolean"} + } + }`, + // watchNamespace gets extracted, other fields validated by schema + expectedWatchNamespace: ptr.To("my-app-namespace"), + }, + { + name: "Schema validation catches constraint violations (replicaCount below minimum)", + rawConfig: []byte(`{"replicaCount": 0}`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "replicaCount": {"type": "integer", "minimum": 1} + } + }`, + expectedErrMessage: "invalid configuration:", + }, + { + name: "Schema validation catches type mismatches", + rawConfig: []byte(`{"replicaCount": "three"}`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "replicaCount": {"type": "integer"} + } + }`, + expectedErrMessage: "invalid configuration:", + }, + { + name: "Empty config is valid when no required fields", + rawConfig: nil, + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "replicaCount": {"type": "integer", "default": 1} + } + }`, + expectedWatchNamespace: nil, + }, + { + name: "Required fields are enforced by schema", + rawConfig: []byte(`{"optional": "value"}`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "required": ["requiredField"], + "properties": { + "requiredField": {"type": "string"}, + "optional": {"type": "string"} + } + }`, + expectedErrMessage: `required field "requiredField" is missing`, + }, + { + name: "Helm with watchNamespace accepts any string value (K8s validates at runtime)", + rawConfig: []byte(`{ + "watchNamespace": "any-value-here", + "replicaCount": 2 + }`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "watchNamespace": {"type": "string"}, + "replicaCount": {"type": "integer"} + } + }`, + expectedWatchNamespace: ptr.To("any-value-here"), + }, + { + name: "Helm with watchNamespace using ownNamespaceInstallMode format (OwnNamespace-like)", + rawConfig: []byte(`{ + "watchNamespace": "wrong-namespace" + }`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "required": ["watchNamespace"], + "properties": { + "watchNamespace": {"type": "string", "format": "ownNamespaceInstallMode"} + } + }`, + expectedErrMessage: "invalid configuration:", + }, + { + name: "Helm with watchNamespace using singleNamespaceInstallMode format (SingleNamespace-like)", + rawConfig: []byte(`{ + "watchNamespace": "my-namespace" + }`), + helmSchema: `{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "required": ["watchNamespace"], + "properties": { + "watchNamespace": {"type": "string", "format": "singleNamespaceInstallMode"} + } + }`, + expectedErrMessage: "invalid configuration:", + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Create a mock Helm package (real Helm would read values.schema.json) + helmBundle := &mockHelmBundle{schema: tc.helmSchema} + schema, err := helmBundle.Get() + require.NoError(t, err) + + // Same validation function works for Helm, registry+v1, registry+v2, etc. + config, err := config.UnmarshalConfig(tc.rawConfig, schema, "my-namespace") + + if tc.expectedErrMessage != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErrMessage) + } else { + require.NoError(t, err) + require.NotNil(t, config) + if tc.expectedWatchNamespace == nil { + require.Nil(t, config.GetWatchNamespace()) + } else { + require.Equal(t, *tc.expectedWatchNamespace, *config.GetWatchNamespace()) + } + } + }) + } +} + +// mockHelmBundle shows how Helm would plug into the validation system. +// +// Real implementation would: +// 1. Read values.schema.json from the Helm chart package +// 2. Parse it into a map[string]any +// 3. Return it (just like registry+v1 returns its generated schema) +// 4. Let the shared validation logic handle the rest +type mockHelmBundle struct { + schema string +} + +// Get returns the schema (in real Helm, read from values.schema.json). +func (h *mockHelmBundle) Get() (map[string]any, error) { + if h.schema == "" { + return nil, nil + } + var schemaMap map[string]any + if err := json.Unmarshal([]byte(h.schema), &schemaMap); err != nil { + return nil, err + } + return schemaMap, nil +} + +// mockEmptySchemaBundle represents a ClusterExtension that doesn't provide a configuration schema. +type mockEmptySchemaBundle struct{} + +func (e *mockEmptySchemaBundle) Get() (map[string]any, error) { + return nil, nil +} diff --git a/internal/operator-controller/config/error_formatting_test.go b/internal/operator-controller/config/error_formatting_test.go new file mode 100644 index 0000000000..6d2a825df6 --- /dev/null +++ b/internal/operator-controller/config/error_formatting_test.go @@ -0,0 +1,174 @@ +package config_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/config" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion" +) + +// Test_ErrorFormatting_SchemaLibraryVersion verifies error messages from the JSON schema +// library and our custom format validators. +// +// These tests serve two purposes: +// 1. Guard against breaking changes if we upgrade github.com/santhosh-tekuri/jsonschema/v6 +// (tests for formatSchemaError parsing may need updates) +// 2. Document the actual error messages end users see (especially for namespace constraints) +func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) { + for _, tc := range []struct { + name string + rawConfig []byte + supportedInstallModes []v1alpha1.InstallModeType + installNamespace string + // We verify the error message contains these key phrases + expectedErrSubstrings []string + }{ + { + name: "Unknown field error formatting", + rawConfig: []byte(`{"unknownField": "value"}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces}, + expectedErrSubstrings: []string{ + "unknown field", + "unknownField", + }, + }, + { + name: "Required field missing error formatting", + rawConfig: []byte(`{}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + expectedErrSubstrings: []string{ + "required field", + "watchNamespace", + "is missing", + }, + }, + { + name: "Required field null error formatting", + rawConfig: []byte(`{"watchNamespace": null}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + expectedErrSubstrings: []string{ + "required field", + "watchNamespace", + "is missing", + }, + }, + { + name: "Type mismatch error formatting", + rawConfig: []byte(`{"watchNamespace": 123}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + expectedErrSubstrings: []string{ + "invalid type", + "watchNamespace", + }, + }, + { + name: "OwnNamespace constraint error formatting", + rawConfig: []byte(`{"watchNamespace": "wrong-namespace"}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, + installNamespace: "correct-namespace", + expectedErrSubstrings: []string{ + "invalid value", + "wrong-namespace", + "watchNamespace must be", + "correct-namespace", + "the namespace where the operator is installed", + "OwnNamespace install mode", + }, + }, + { + name: "SingleNamespace constraint error formatting", + rawConfig: []byte(`{"watchNamespace": "install-ns"}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + installNamespace: "install-ns", + expectedErrSubstrings: []string{ + "invalid value", + "install-ns", + "watchNamespace must be different from", + "the install namespace", + "SingleNamespace install mode", + "watch a different namespace", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + rv1 := bundle.RegistryV1{ + CSV: clusterserviceversion.Builder(). + WithName("test-operator"). + WithInstallModeSupportFor(tc.supportedInstallModes...). + Build(), + } + + schema, err := rv1.Get() + require.NoError(t, err) + + _, err = config.UnmarshalConfig(tc.rawConfig, schema, tc.installNamespace) + require.Error(t, err, "Expected validation error") + + errMsg := err.Error() + for _, substring := range tc.expectedErrSubstrings { + require.Contains(t, errMsg, substring, + "Error message should contain %q. Full error: %s", substring, errMsg) + } + }) + } +} + +// Test_ErrorFormatting_YAMLParseErrors verifies YAML/JSON parsing errors are formatted correctly. +func Test_ErrorFormatting_YAMLParseErrors(t *testing.T) { + for _, tc := range []struct { + name string + rawConfig []byte + expectedErrSubstrings []string + }{ + { + name: "Malformed JSON", + rawConfig: []byte(`{"incomplete`), + expectedErrSubstrings: []string{ + "unexpected end of stream", + }, + }, + { + name: "Non-object JSON", + rawConfig: []byte(`true`), + expectedErrSubstrings: []string{ + "invalid type", + "got boolean, want object", + }, + }, + { + name: "Wrong type for field", + rawConfig: []byte(`{"watchNamespace": {"nested": "object"}}`), + expectedErrSubstrings: []string{ + "invalid type", + "got object, want string", + "watchNamespace", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + rv1 := bundle.RegistryV1{ + CSV: clusterserviceversion.Builder(). + WithName("test-operator"). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace). + Build(), + } + + schema, err := rv1.Get() + require.NoError(t, err) + + _, err = config.UnmarshalConfig(tc.rawConfig, schema, "test-namespace") + require.Error(t, err, "Expected parse error") + + errMsg := err.Error() + for _, substring := range tc.expectedErrSubstrings { + require.Contains(t, errMsg, substring, + "Error message should contain %q. Full error: %s", substring, errMsg) + } + }) + } +} diff --git a/internal/operator-controller/rukpak/bundle/config.go b/internal/operator-controller/rukpak/bundle/config.go deleted file mode 100644 index 7ec4bfdf03..0000000000 --- a/internal/operator-controller/rukpak/bundle/config.go +++ /dev/null @@ -1,124 +0,0 @@ -package bundle - -import ( - "encoding/json" - "errors" - "fmt" - "strings" - - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation" - "sigs.k8s.io/yaml" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" -) - -type Config struct { - WatchNamespace *string `json:"watchNamespace"` -} - -// UnmarshalConfig returns a deserialized *bundle.Config based on bytes and validated -// against rv1 and the desired install namespaces. It will error if: -// - rv is nil -// - bytes is not a valid YAML/JSON object -// - bytes is a valid YAML/JSON object but does not follow the registry+v1 schema -// - if bytes is nil, a nil *bundle.Config is returned with no error -func UnmarshalConfig(bytes []byte, rv1 RegistryV1, installNamespace string) (*Config, error) { - if bytes == nil { - return nil, nil - } - - bundleConfig := &Config{} - if err := yaml.UnmarshalStrict(bytes, bundleConfig); err != nil { - return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshalError(err)) - } - - // collect bundle install modes - bundleInstallModeSet := sets.New(rv1.CSV.Spec.InstallModes...) - - if err := validateConfig(bundleConfig, installNamespace, bundleInstallModeSet); err != nil { - return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", err) - } - - return bundleConfig, nil -} - -// validateConfig validates a *bundle.Config against the bundle's supported install modes and the user-give installNamespace. -func validateConfig(config *Config, installNamespace string, bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) error { - // no config, no problem - if config == nil { - return nil - } - - // if the bundle does not support the watchNamespace configuration and it is set, treat it like any unknown field - if config.WatchNamespace != nil && !isWatchNamespaceConfigSupported(bundleInstallModeSet) { - return errors.New(`unknown field "watchNamespace"`) - } - - // if watchNamespace is required then ensure that it is set - if config.WatchNamespace == nil && isWatchNamespaceConfigRequired(bundleInstallModeSet) { - return errors.New(`required field "watchNamespace" is missing`) - } - - // if watchNamespace is set then ensure it is a valid namespace - if config.WatchNamespace != nil { - if errs := validation.IsDNS1123Subdomain(*config.WatchNamespace); len(errs) > 0 { - return fmt.Errorf("invalid 'watchNamespace' %q: namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", *config.WatchNamespace) - } - } - - // only accept install namespace if OwnNamespace install mode is supported - if config.WatchNamespace != nil && *config.WatchNamespace == installNamespace && - !bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}) { - return fmt.Errorf("invalid 'watchNamespace' %q: must not be install namespace (%s)", *config.WatchNamespace, installNamespace) - } - - // only accept non-install namespace is SingleNamespace is supported - if config.WatchNamespace != nil && *config.WatchNamespace != installNamespace && - !bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}) { - return fmt.Errorf("invalid 'watchNamespace' %q: must be install namespace (%s)", *config.WatchNamespace, installNamespace) - } - - return nil -} - -// isWatchNamespaceConfigSupported returns true when the bundle exposes a watchNamespace configuration. This happens when: -// - SingleNamespace and/or OwnNamespace install modes are supported -func isWatchNamespaceConfigSupported(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool { - return bundleInstallModeSet.HasAny( - v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, - v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - ) -} - -// isWatchNamespaceConfigRequired returns true if the watchNamespace configuration is required. This happens when -// AllNamespaces install mode is not supported and SingleNamespace and/or OwnNamespace is supported -func isWatchNamespaceConfigRequired(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool { - return isWatchNamespaceConfigSupported(bundleInstallModeSet) && - !bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) -} - -// formatUnmarshalError format JSON unmarshal errors to be more readable -func formatUnmarshalError(err error) error { - var unmarshalErr *json.UnmarshalTypeError - if errors.As(err, &unmarshalErr) { - if unmarshalErr.Field == "" { - return errors.New("input is not a valid JSON object") - } else { - return fmt.Errorf("invalid value type for field %q: expected %q but got %q", unmarshalErr.Field, unmarshalErr.Type.String(), unmarshalErr.Value) - } - } - - // unwrap error until the core and process it - for { - unwrapped := errors.Unwrap(err) - if unwrapped == nil { - // usually the errors present in the form json: or yaml: - // we want to extract if we can - errMessageComponents := strings.Split(err.Error(), ":") - coreErrMessage := strings.TrimSpace(errMessageComponents[len(errMessageComponents)-1]) - return errors.New(coreErrMessage) - } - err = unwrapped - } -} diff --git a/internal/operator-controller/rukpak/bundle/config_test.go b/internal/operator-controller/rukpak/bundle/config_test.go deleted file mode 100644 index a6c4b394a2..0000000000 --- a/internal/operator-controller/rukpak/bundle/config_test.go +++ /dev/null @@ -1,310 +0,0 @@ -package bundle_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - "k8s.io/utils/ptr" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" - - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion" -) - -func Test_UnmarshalConfig(t *testing.T) { - for _, tc := range []struct { - name string - rawConfig []byte - supportedInstallModes []v1alpha1.InstallModeType - installNamespace string - expectedErrMessage string - expectedConfig *bundle.Config - }{ - { - name: "returns nil for nil config", - rawConfig: nil, - expectedConfig: nil, - }, - { - name: "accepts json config", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "accepts yaml config", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`watchNamespace: some-namespace`), - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "rejects invalid json", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"hello`), - expectedErrMessage: `error unmarshalling registry+v1 configuration: found unexpected end of stream`, - }, - { - name: "rejects valid json that isn't of object type", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`true`), - expectedErrMessage: `error unmarshalling registry+v1 configuration: input is not a valid JSON object`, - }, - { - name: "rejects additional fields", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`somekey: somevalue`), - expectedErrMessage: `error unmarshalling registry+v1 configuration: unknown field "somekey"`, - }, - { - name: "rejects valid json but invalid registry+v1", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": {"hello": "there"}}`), - expectedErrMessage: `error unmarshalling registry+v1 configuration: invalid value type for field "watchNamespace": expected "string" but got "object"`, - }, - { - name: "rejects bad namespace format", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "bad-Namespace-"}`), - expectedErrMessage: "invalid 'watchNamespace' \"bad-Namespace-\": namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", - }, - { - name: "rejects with unknown field when install modes {AllNamespaces}", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedErrMessage: "unknown field \"watchNamespace\"", - }, - { - name: "rejects with unknown field when install modes {MultiNamespace}", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedErrMessage: "unknown field \"watchNamespace\"", - }, - { - name: "reject with unknown field when install modes {AllNamespaces, MultiNamespace}", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedErrMessage: "unknown field \"watchNamespace\"", - }, - { - name: "reject with required field when install modes {OwnNamespace} and watchNamespace is null", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "reject with required field when install modes {OwnNamespace} and watchNamespace is missing", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{}`), - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is null", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is missing", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{}`), - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "accepts when install modes {SingleNamespace} and watchNamespace != install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "accepts when install modes {AllNamespaces, SingleNamespace} and watchNamespace != install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "accepts when install modes {MultiNamespace, SingleNamespace} and watchNamespace != install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "accepts when install modes {OwnNamespace, SingleNamespace} and watchNamespace != install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "not-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "rejects when install modes {SingleNamespace} and watchNamespace == install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "some-namespace", - expectedErrMessage: "invalid 'watchNamespace' \"some-namespace\": must not be install namespace (some-namespace)", - }, - { - name: "rejects when install modes {AllNamespaces, SingleNamespace} and watchNamespace == install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "some-namespace", - expectedErrMessage: "invalid 'watchNamespace' \"some-namespace\": must not be install namespace (some-namespace)", - }, - { - name: "rejects when install modes {MultiNamespace, SingleNamespace} and watchNamespace == install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "some-namespace", - expectedErrMessage: "invalid 'watchNamespace' \"some-namespace\": must not be install namespace (some-namespace)", - }, - { - name: "accepts when install modes {AllNamespaces, OwnNamespace} and watchNamespace == install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "accepts when install modes {OwnNamespace, SingleNamespace} and watchNamespace == install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: ptr.To("some-namespace"), - }, - }, - { - name: "rejects when install modes {AllNamespaces, OwnNamespace} and watchNamespace != install namespace", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": "some-namespace"}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "invalid 'watchNamespace' \"some-namespace\": must be install namespace (not-some-namespace)", - }, - { - name: "rejects with required field error when install modes {SingleNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace} and watchNamespace is missing", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, - rawConfig: []byte(`{}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is missing", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is missing", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace, MultiNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "required field \"watchNamespace\" is missing", - }, - { - name: "accepts null watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: nil, - }, - }, - { - name: "accepts null watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{"watchNamespace": null}`), - installNamespace: "not-some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: nil, - }, - }, - { - name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{}`), - installNamespace: "not-some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: nil, - }, - }, - { - name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace}, - rawConfig: []byte(`{}`), - installNamespace: "not-some-namespace", - expectedConfig: &bundle.Config{ - WatchNamespace: nil, - }, - }, - { - name: "rejects with format error when install modes are {SingleNamespace, OwnNamespace} and watchNamespace is ''", - supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, - rawConfig: []byte(`{"watchNamespace": ""}`), - installNamespace: "not-some-namespace", - expectedErrMessage: "invalid 'watchNamespace' \"\": namespace must consist of lower case alphanumeric characters", - }, - } { - t.Run(tc.name, func(t *testing.T) { - var rv1 bundle.RegistryV1 - if tc.supportedInstallModes != nil { - rv1 = bundle.RegistryV1{ - CSV: clusterserviceversion.Builder(). - WithName("test-operator"). - WithInstallModeSupportFor(tc.supportedInstallModes...). - Build(), - } - } - - config, err := bundle.UnmarshalConfig(tc.rawConfig, rv1, tc.installNamespace) - require.Equal(t, tc.expectedConfig, config) - if tc.expectedErrMessage != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedErrMessage) - } else { - require.NoError(t, err) - } - }) - } -} diff --git a/internal/operator-controller/rukpak/bundle/registryv1.go b/internal/operator-controller/rukpak/bundle/registryv1.go index cffc374e91..966a81490f 100644 --- a/internal/operator-controller/rukpak/bundle/registryv1.go +++ b/internal/operator-controller/rukpak/bundle/registryv1.go @@ -3,8 +3,11 @@ package bundle import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/config" ) const ( @@ -17,3 +20,134 @@ type RegistryV1 struct { CRDs []apiextensionsv1.CustomResourceDefinition Others []unstructured.Unstructured } + +// Get builds a validation schema based on what install modes the operator supports. +// +// For registry+v1 bundles, we look at the CSV's install modes and generate a schema +// that matches. For example, if the operator only supports OwnNamespace mode, we'll +// require the user to provide a watchNamespace that equals the install namespace. +func (rv1 RegistryV1) Get() (map[string]any, error) { + installModes := sets.New(rv1.CSV.Spec.InstallModes...) + return buildBundleConfigSchema(installModes) +} + +// buildBundleConfigSchema creates validation rules based on what the operator supports. +// +// Examples of how install modes affect validation: +// - AllNamespaces only: user can't set watchNamespace (operator watches everything) +// - OwnNamespace only: user must set watchNamespace to the install namespace +// - SingleNamespace only: user must set watchNamespace to a different namespace +// - AllNamespaces + OwnNamespace: user can optionally set watchNamespace +func buildBundleConfigSchema(installModes sets.Set[v1alpha1.InstallMode]) (map[string]any, error) { + schema := map[string]any{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "additionalProperties": false, // Reject unknown fields (catches typos and misconfigurations) + } + + properties := map[string]any{} + var required []any + + // Add watchNamespace property if the bundle supports it + if isWatchNamespaceConfigurable(installModes) { + watchNSProperty, isRequired := buildWatchNamespaceProperty(installModes) + properties["watchNamespace"] = watchNSProperty + if isRequired { + required = append(required, "watchNamespace") + } + } + + schema["properties"] = properties + if len(required) > 0 { + schema["required"] = required + } + + return schema, nil +} + +// buildWatchNamespaceProperty creates the validation rules for the watchNamespace field. +// +// The rules depend on what install modes are supported: +// - AllNamespaces supported: watchNamespace is optional (can be null) +// - Only Single/Own supported: watchNamespace is required +// - Only OwnNamespace: must equal install namespace +// - Only SingleNamespace: must be different from install namespace +// +// Returns the validation rules and whether the field is required. +func buildWatchNamespaceProperty(installModes sets.Set[v1alpha1.InstallMode]) (map[string]any, bool) { + watchNSProperty := map[string]any{ + "description": "The namespace that the operator should watch for custom resources", + } + + hasOwnNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}) + hasSingleNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}) + + format := selectNamespaceFormat(hasOwnNamespace, hasSingleNamespace) + + if isWatchNamespaceConfigRequired(installModes) { + watchNSProperty["type"] = "string" + if format != "" { + watchNSProperty["format"] = format + } + return watchNSProperty, true + } + + // allow null or valid namespace string + stringSchema := map[string]any{ + "type": "string", + } + if format != "" { + stringSchema["format"] = format + } + // Convert to []any for JSON schema compatibility + watchNSProperty["anyOf"] = []any{ + map[string]any{"type": "null"}, + stringSchema, + } + + return watchNSProperty, false +} + +// selectNamespaceFormat picks which namespace constraint to apply. +// +// - OwnNamespace only: watchNamespace must equal install namespace +// - SingleNamespace only: watchNamespace must be different from install namespace +// - Both or neither: no constraint, any namespace name is valid +func selectNamespaceFormat(hasOwnNamespace, hasSingleNamespace bool) string { + if hasOwnNamespace && !hasSingleNamespace { + return config.FormatOwnNamespaceInstallMode + } + if hasSingleNamespace && !hasOwnNamespace { + return config.FormatSingleNamespaceInstallMode + } + return "" // No format constraint needed for generic case +} + +// isWatchNamespaceConfigurable checks if the user can set a watchNamespace. +// +// Returns true if: +// - SingleNamespace is supported (user picks a namespace to watch) +// - OwnNamespace is supported (user sets watchNamespace to the install namespace) +// +// Returns false if: +// - Only AllNamespaces is supported (operator always watches everything) +func isWatchNamespaceConfigurable(installModes sets.Set[v1alpha1.InstallMode]) bool { + return installModes.HasAny( + v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, + v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, + ) +} + +// isWatchNamespaceConfigRequired checks if the user must provide a watchNamespace. +// +// Returns true (required) when: +// - Only OwnNamespace is supported +// - Only SingleNamespace is supported +// - Both OwnNamespace and SingleNamespace are supported +// +// Returns false (optional) when: +// - AllNamespaces is supported (user can leave it unset to watch all namespaces) +func isWatchNamespaceConfigRequired(installModes sets.Set[v1alpha1.InstallMode]) bool { + return isWatchNamespaceConfigurable(installModes) && + !installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) +} diff --git a/test/e2e/single_namespace_support_test.go b/test/e2e/single_namespace_support_test.go index 7e8fe7bd5b..2c3b825a1a 100644 --- a/test/e2e/single_namespace_support_test.go +++ b/test/e2e/single_namespace_support_test.go @@ -148,7 +148,7 @@ func TestClusterExtensionSingleNamespaceSupport(t *testing.T) { require.NotNil(ct, cond) require.Equal(ct, metav1.ConditionTrue, cond.Status) require.Equal(ct, ocv1.ReasonRetrying, cond.Reason) - require.Contains(ct, cond.Message, "required field \"watchNamespace\" is missing") + require.Contains(ct, cond.Message, `required field "watchNamespace" is missing`) }, pollDuration, pollInterval) t.Log("By updating the ClusterExtension configuration with a watchNamespace") @@ -296,7 +296,7 @@ func TestClusterExtensionOwnNamespaceSupport(t *testing.T) { require.NotNil(ct, cond) require.Equal(ct, metav1.ConditionTrue, cond.Status) require.Equal(ct, ocv1.ReasonRetrying, cond.Reason) - require.Contains(ct, cond.Message, "required field \"watchNamespace\" is missing") + require.Contains(ct, cond.Message, `required field "watchNamespace" is missing`) }, pollDuration, pollInterval) t.Log("By updating the ClusterExtension configuration with a watchNamespace other than the install namespace") @@ -318,7 +318,9 @@ func TestClusterExtensionOwnNamespaceSupport(t *testing.T) { require.NotNil(ct, cond) require.Equal(ct, metav1.ConditionTrue, cond.Status) require.Equal(ct, ocv1.ReasonRetrying, cond.Reason) - require.Contains(ct, cond.Message, fmt.Sprintf("invalid 'watchNamespace' \"some-namespace\": must be install namespace (%s)", clusterExtension.Spec.Namespace)) + require.Contains(ct, cond.Message, "invalid ClusterExtension configuration") + require.Contains(ct, cond.Message, fmt.Sprintf("watchNamespace must be \"%s\"", clusterExtension.Spec.Namespace)) + require.Contains(ct, cond.Message, "OwnNamespace install mode") }, pollDuration, pollInterval) t.Log("By updating the ClusterExtension configuration with a watchNamespace = install namespace")