Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/operator-controller/applier/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})

Expand Down
59 changes: 40 additions & 19 deletions internal/operator-controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"strings"

"github.com/santhosh-tekuri/jsonschema/v6"
"github.com/santhosh-tekuri/jsonschema/v6/kind"
"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -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
},
Expand All @@ -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
},
Expand Down Expand Up @@ -294,25 +295,29 @@ 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 != "" {
return fmt.Sprintf("required field %q is missing", fieldName)
}
return "required field is missing"

case strings.Contains(errUnit.KeywordLocation, "/additionalProperties"):
case *kind.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"):
case *kind.Type:
// Type mismatch (e.g., got null, want string)
fieldPath := buildFieldPath(errUnit.InstanceLocation)
if fieldPath != "" {
Expand All @@ -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)
Expand All @@ -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()
}
}

Expand Down
29 changes: 27 additions & 2 deletions internal/operator-controller/config/error_formatting_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config_test

import (
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -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",
Expand All @@ -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{
Expand Down
55 changes: 28 additions & 27 deletions internal/operator-controller/rukpak/bundle/registryv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/features/install.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand Down
Loading