From 6e173e7cdae009e3190e2788d05886e1658d965e Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 29 Jan 2026 13:03:22 +0100 Subject: [PATCH] Add namespace format validation to bundle config schema Signed-off-by: Per Goncalves da Silva --- .../applier/provider_test.go | 2 +- internal/operator-controller/config/config.go | 59 +++++++++++++------ .../config/error_formatting_test.go | 29 ++++++++- .../rukpak/bundle/registryv1.go | 55 ++++++++--------- test/e2e/features/install.feature | 8 +-- 5 files changed, 100 insertions(+), 53 deletions(-) diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 4138284a05..f3fbc9ec45 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -393,7 +393,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { }) require.Error(t, err) require.Contains(t, err.Error(), "invalid ClusterExtension configuration:") - require.Contains(t, err.Error(), "watchNamespace must be") + require.Contains(t, err.Error(), "must be") require.Contains(t, err.Error(), "install-namespace") }) diff --git a/internal/operator-controller/config/config.go b/internal/operator-controller/config/config.go index 43f755762c..30f30951c4 100644 --- a/internal/operator-controller/config/config.go +++ b/internal/operator-controller/config/config.go @@ -31,6 +31,7 @@ import ( "strings" "github.com/santhosh-tekuri/jsonschema/v6" + "github.com/santhosh-tekuri/jsonschema/v6/kind" "sigs.k8s.io/yaml" ) @@ -208,7 +209,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install 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 fmt.Errorf("invalid value %q: must be %q (the namespace where the operator is installed) because this operator only supports OwnNamespace install mode", str, installNamespace) } return nil }, @@ -228,7 +229,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install 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 fmt.Errorf("invalid value %q: must be different from %q (the install namespace) because this operator uses SingleNamespace install mode to watch a different namespace", str, installNamespace) } return nil }, @@ -294,9 +295,13 @@ func formatSchemaError(err error) error { // formatSingleError formats a single validation error from the schema library. func formatSingleError(errUnit jsonschema.OutputUnit) string { + if errUnit.Error == nil { + return "" + } + // Check the keyword location to identify the error type - switch { - case strings.Contains(errUnit.KeywordLocation, "/required"): + switch errKind := errUnit.Error.Kind.(type) { + case *kind.Required: // Missing required field fieldName := extractFieldNameFromMessage(errUnit.Error) if fieldName != "" { @@ -304,7 +309,7 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string { } return "required field is missing" - case strings.Contains(errUnit.KeywordLocation, "/additionalProperties"): + case *kind.AdditionalProperties: // Unknown/additional field fieldName := extractFieldNameFromMessage(errUnit.Error) if fieldName != "" { @@ -312,7 +317,7 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string { } return "unknown field" - case strings.Contains(errUnit.KeywordLocation, "/type"): + case *kind.Type: // Type mismatch (e.g., got null, want string) fieldPath := buildFieldPath(errUnit.InstanceLocation) if fieldPath != "" { @@ -324,16 +329,14 @@ func formatSingleError(errUnit jsonschema.OutputUnit) 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() - } + case *kind.Format: fieldPath := buildFieldPath(errUnit.InstanceLocation) - return fmt.Sprintf("invalid format for field %q", fieldPath) + if fieldPath != "" { + return fmt.Sprintf("invalid format for field %q: %s", fieldPath, errUnit.Error.String()) + } + return fmt.Sprintf("invalid format: %s", errUnit.Error.String()) - case strings.Contains(errUnit.KeywordLocation, "/anyOf"): + case *kind.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) @@ -342,13 +345,31 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string { } return "invalid value" + case *kind.MaxLength: + fieldPath := buildFieldPath(errUnit.InstanceLocation) + if fieldPath != "" { + return fmt.Sprintf("field %q must have maximum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got) + } + return errUnit.Error.String() + + case *kind.MinLength: + fieldPath := buildFieldPath(errUnit.InstanceLocation) + if fieldPath != "" { + return fmt.Sprintf("field %q must have minimum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got) + } + return errUnit.Error.String() + + case *kind.Pattern: + fieldPath := buildFieldPath(errUnit.InstanceLocation) + if fieldPath != "" { + return fmt.Sprintf("field %q must match pattern %q", fieldPath, errKind.Want) + } + return errUnit.Error.String() + default: - // Unknown error type - return the library's error message + // Unhandled 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 "" + return errUnit.Error.String() } } diff --git a/internal/operator-controller/config/error_formatting_test.go b/internal/operator-controller/config/error_formatting_test.go index 557e21019b..6ec1ebf7de 100644 --- a/internal/operator-controller/config/error_formatting_test.go +++ b/internal/operator-controller/config/error_formatting_test.go @@ -1,6 +1,7 @@ package config_test import ( + "strings" "testing" "github.com/stretchr/testify/require" @@ -72,9 +73,9 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) { supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace}, installNamespace: "correct-namespace", expectedErrSubstrings: []string{ + "invalid format for field \"watchNamespace\"", "invalid value", "wrong-namespace", - "watchNamespace must be", "correct-namespace", "the namespace where the operator is installed", "OwnNamespace install mode", @@ -86,14 +87,38 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) { supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, installNamespace: "install-ns", expectedErrSubstrings: []string{ + "invalid format for field \"watchNamespace\"", + "not valid singleNamespaceInstallMode", "invalid value", "install-ns", - "watchNamespace must be different from", + "must be different from", "the install namespace", "SingleNamespace install mode", "watch a different namespace", }, }, + { + name: "SingleNamespace constraint error bad namespace format", + rawConfig: []byte(`{"watchNamespace": "---AAAA-BBBB-super-long-namespace-that-that-is-waaaaaaaaayyy-longer-than-sixty-three-characters"}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace}, + installNamespace: "install-ns", + expectedErrSubstrings: []string{ + "field \"watchNamespace\"", + "must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\"", + }, + }, + { + name: "Single- and OwnNamespace constraint error bad namespace format", + rawConfig: []byte(`{"watchNamespace": ` + strings.Repeat("A", 63) + `"}`), + supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace}, + installNamespace: "install-ns", + expectedErrSubstrings: []string{ + "invalid configuration", + "multiple errors found", + "must have maximum length of 63 (len=64)", + "must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\"", + }, + }, } { t.Run(tc.name, func(t *testing.T) { rv1 := bundle.RegistryV1{ diff --git a/internal/operator-controller/rukpak/bundle/registryv1.go b/internal/operator-controller/rukpak/bundle/registryv1.go index d8412fde66..5b0c2a8cd9 100644 --- a/internal/operator-controller/rukpak/bundle/registryv1.go +++ b/internal/operator-controller/rukpak/bundle/registryv1.go @@ -15,8 +15,9 @@ import ( ) const ( - BundleConfigWatchNamespaceKey = "watchNamespace" - BundleConfigDeploymentConfigKey = "deploymentConfig" + watchNamespaceConfigKey = "watchNamespace" + namespaceNamePattern = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" + namespaceNameMaxLength = 63 ) var ( @@ -69,19 +70,19 @@ func buildBundleConfigSchema(installModes sets.Set[v1alpha1.InstallMode]) (map[s if isWatchNamespaceConfigurable(installModes) { // Replace the generic watchNamespace with install-mode-specific version watchNSProperty, isRequired := buildWatchNamespaceProperty(installModes) - properties["watchNamespace"] = watchNSProperty + properties[watchNamespaceConfigKey] = watchNSProperty // Preserve existing required fields, only add/remove watchNamespace if isRequired { - addToRequired(baseSchema, "watchNamespace") + addToRequired(baseSchema, watchNamespaceConfigKey) } else { - removeFromRequired(baseSchema, "watchNamespace") + removeFromRequired(baseSchema, watchNamespaceConfigKey) } } else { // AllNamespaces only - remove watchNamespace property entirely // (operator always watches all namespaces, no configuration needed) - delete(properties, "watchNamespace") - removeFromRequired(baseSchema, "watchNamespace") + delete(properties, watchNamespaceConfigKey) + removeFromRequired(baseSchema, watchNamespaceConfigKey) } return baseSchema, nil @@ -138,37 +139,37 @@ func removeFromRequired(schema map[string]any, fieldName string) { // // 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", - } + const 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", + watchNamespaceSchema := map[string]any{ + "type": "string", + "minLength": 1, + "maxLength": namespaceNameMaxLength, + // kubernetes namespace name format + "pattern": namespaceNamePattern, } if format != "" { - stringSchema["format"] = format + watchNamespaceSchema["format"] = format } - // Convert to []any for JSON schema compatibility - watchNSProperty["anyOf"] = []any{ - map[string]any{"type": "null"}, - stringSchema, + + if isWatchNamespaceConfigRequired(installModes) { + watchNamespaceSchema["description"] = description + return watchNamespaceSchema, true } - return watchNSProperty, false + // allow null or valid namespace string + return map[string]any{ + "description": description, + "anyOf": []any{ + map[string]any{"type": "null"}, + watchNamespaceSchema, + }, + }, false } // selectNamespaceFormat picks which namespace constraint to apply. diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index ab87b5f31c..6ccbd25e87 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -200,10 +200,10 @@ Feature: Install ClusterExtension And ClusterExtension reports Progressing as True with Reason Retrying and Message: """ error for resolved bundle "own-namespace-operator.1.0.0" with version - "1.0.0": invalid ClusterExtension configuration: invalid configuration: 'some-ns' - is not valid ownNamespaceInstallMode: invalid value "some-ns": watchNamespace - must be "${TEST_NAMESPACE}" (the namespace where the operator is installed) because this - operator only supports OwnNamespace install mode + "1.0.0": invalid ClusterExtension configuration: invalid configuration: invalid + format for field "watchNamespace": 'some-ns' is not valid ownNamespaceInstallMode: + invalid value "some-ns": must be "${TEST_NAMESPACE}" (the namespace where the + operator is installed) because this operator only supports OwnNamespace install mode """ When ClusterExtension is updated to set watchNamespace to own namespace value """