Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 83
🐛 Fix an unclear validation error when configuring watchNamespace on an operator restricted to AllNamespaces mode.#2347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
camilamacedo86 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return fmt.Errorf("watchNamespace configuration is not supported when the content only supports AllNamespaces install mode") | ||
camilamacedo86 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }, | ||
camilamacedo86 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }) | ||
| if err := compiler.AddResource(configSchemaID, schema); err != nil { | ||
| return fmt.Errorf("failed to load schema: %w", err) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
camilamacedo86 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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. | ||
camilamacedo86 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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", | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.