From f85c7073c59a5305271ea9bf3cf71f980e713b09 Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Wed, 19 Nov 2025 06:12:34 +0000 Subject: [PATCH] (fix): Corrected an unclear validation error when configuring watchNamespace on an operator restricted to AllNamespaces mode. --- internal/operator-controller/config/config.go | 11 ++++++ .../operator-controller/config/config_test.go | 16 ++------- .../rukpak/bundle/registryv1.go | 34 +++++++++++++++++-- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/internal/operator-controller/config/config.go b/internal/operator-controller/config/config.go index 8fcadf40ad..c86db33f4c 100644 --- a/internal/operator-controller/config/config.go +++ b/internal/operator-controller/config/config.go @@ -45,6 +45,9 @@ const ( // FormatSingleNamespaceInstallMode defines the format check to ensure that // the watchNamespace must differ from install namespace FormatSingleNamespaceInstallMode = "singleNamespaceInstallMode" + // FormatAllNamespacesOnlyInstallMode defines the format check to reject + // watchNamespace when only AllNamespaces mode is supported (registry+v1 specific) + FormatAllNamespacesOnlyInstallMode = "allNamespacesOnlyInstallMode" ) // SchemaProvider lets each package format type describe what configuration it accepts. @@ -192,6 +195,14 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install return nil }, }) + compiler.RegisterFormat(&jsonschema.Format{ + Name: FormatAllNamespacesOnlyInstallMode, + Validate: func(value interface{}) error { + // Always reject - this format is used when AllNamespaces is the only supported mode + // and watchNamespace configuration doesn't make sense + return fmt.Errorf("watchNamespace configuration is not supported when the content only supports AllNamespaces install mode") + }, + }) if err := compiler.AddResource(configSchemaID, schema); err != nil { return fmt.Errorf("failed to load schema: %w", err) diff --git a/internal/operator-controller/config/config_test.go b/internal/operator-controller/config/config_test.go index 95bb98f0b4..12c9dd02d6 100644 --- a/internal/operator-controller/config/config_test.go +++ b/internal/operator-controller/config/config_test.go @@ -80,22 +80,10 @@ func Test_UnmarshalConfig(t *testing.T) { expectedErrMessage: `got object, want string`, }, { - name: "rejects with unknown field when install modes {AllNamespaces}", + name: "rejects with descriptive message 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"`, + expectedErrMessage: `watchNamespace configuration is not supported when the content only supports AllNamespaces install mode`, }, { name: "reject with required field when install modes {OwnNamespace} and watchNamespace is null", diff --git a/internal/operator-controller/rukpak/bundle/registryv1.go b/internal/operator-controller/rukpak/bundle/registryv1.go index 7fc3e3e185..0007e59c82 100644 --- a/internal/operator-controller/rukpak/bundle/registryv1.go +++ b/internal/operator-controller/rukpak/bundle/registryv1.go @@ -34,7 +34,7 @@ func (rv1 *RegistryV1) GetConfigSchema() (map[string]any, error) { // 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) +// - AllNamespaces only: watchNamespace is explicitly rejected with helpful error // - 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 @@ -48,8 +48,12 @@ func buildBundleConfigSchema(installModes sets.Set[v1alpha1.InstallMode]) (map[s properties := map[string]any{} var required []any - // Add watchNamespace property if the bundle supports it - if isWatchNamespaceConfigurable(installModes) { + // Special case: if ONLY AllNamespaces is supported, explicitly reject watchNamespace + // with a helpful error message (instead of generic "unknown field") + if isAllNamespacesOnly(installModes) { + properties["watchNamespace"] = buildRejectedWatchNamespaceProperty() + } else if isWatchNamespaceConfigurable(installModes) { + // Add watchNamespace property if the bundle supports it watchNSProperty, isRequired := buildWatchNamespaceProperty(installModes) properties["watchNamespace"] = watchNSProperty if isRequired { @@ -151,3 +155,27 @@ func isWatchNamespaceConfigRequired(installModes sets.Set[v1alpha1.InstallMode]) return isWatchNamespaceConfigurable(installModes) && !installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) } + +// isAllNamespacesOnly checks if only AllNamespaces install mode is supported. +// +// Returns true when: +// - Only AllNamespaces is supported (no OwnNamespace, no SingleNamespace) +// +// Returns false when: +// - OwnNamespace or SingleNamespace is also supported +func isAllNamespacesOnly(installModes sets.Set[v1alpha1.InstallMode]) bool { + hasAllNamespaces := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) + hasConfigurable := isWatchNamespaceConfigurable(installModes) + + return hasAllNamespaces && !hasConfigurable +} + +// buildRejectedWatchNamespaceProperty creates a schema property that always rejects +// watchNamespace with a descriptive error message for AllNamespaces-only operators. +func buildRejectedWatchNamespaceProperty() map[string]any { + return map[string]any{ + "type": "string", + "format": config.FormatAllNamespacesOnlyInstallMode, + "description": "This field is not supported for this operator's install mode configuration", + } +}