Skip to content

Commit 6c0fd75

Browse files
author
Per Goncalves da Silva
committed
Add namespace format validation to bundle config schema
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent 46f9978 commit 6c0fd75

4 files changed

Lines changed: 96 additions & 50 deletions

File tree

internal/operator-controller/applier/provider_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
393393
})
394394
require.Error(t, err)
395395
require.Contains(t, err.Error(), "invalid ClusterExtension configuration:")
396-
require.Contains(t, err.Error(), "watchNamespace must be")
396+
require.Contains(t, err.Error(), "must be")
397397
require.Contains(t, err.Error(), "install-namespace")
398398
})
399399

internal/operator-controller/config/config.go

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"strings"
3232

3333
"github.com/santhosh-tekuri/jsonschema/v6"
34+
"github.com/santhosh-tekuri/jsonschema/v6/kind"
3435
"sigs.k8s.io/yaml"
3536
)
3637

@@ -208,7 +209,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install
208209
return fmt.Errorf("value must be a string")
209210
}
210211
if str != installNamespace {
211-
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)
212+
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)
212213
}
213214
return nil
214215
},
@@ -228,7 +229,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install
228229
return fmt.Errorf("value must be a string")
229230
}
230231
if str == installNamespace {
231-
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)
232+
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)
232233
}
233234
return nil
234235
},
@@ -294,25 +295,29 @@ func formatSchemaError(err error) error {
294295

295296
// formatSingleError formats a single validation error from the schema library.
296297
func formatSingleError(errUnit jsonschema.OutputUnit) string {
298+
if errUnit.Error == nil {
299+
return ""
300+
}
301+
297302
// Check the keyword location to identify the error type
298-
switch {
299-
case strings.Contains(errUnit.KeywordLocation, "/required"):
303+
switch errKind := errUnit.Error.Kind.(type) {
304+
case *kind.Required:
300305
// Missing required field
301306
fieldName := extractFieldNameFromMessage(errUnit.Error)
302307
if fieldName != "" {
303308
return fmt.Sprintf("required field %q is missing", fieldName)
304309
}
305310
return "required field is missing"
306311

307-
case strings.Contains(errUnit.KeywordLocation, "/additionalProperties"):
312+
case *kind.AdditionalProperties:
308313
// Unknown/additional field
309314
fieldName := extractFieldNameFromMessage(errUnit.Error)
310315
if fieldName != "" {
311316
return fmt.Sprintf("unknown field %q", fieldName)
312317
}
313318
return "unknown field"
314319

315-
case strings.Contains(errUnit.KeywordLocation, "/type"):
320+
case *kind.Type:
316321
// Type mismatch (e.g., got null, want string)
317322
fieldPath := buildFieldPath(errUnit.InstanceLocation)
318323
if fieldPath != "" {
@@ -324,16 +329,14 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string {
324329
}
325330
return fmt.Sprintf("invalid type: %s", errUnit.Error.String())
326331

327-
case strings.Contains(errUnit.KeywordLocation, "/format"):
328-
// Custom format validation (e.g., OwnNamespace, SingleNamespace constraints)
329-
// These already have good error messages from our custom validators
330-
if errUnit.Error != nil {
331-
return errUnit.Error.String()
332-
}
332+
case *kind.Format:
333333
fieldPath := buildFieldPath(errUnit.InstanceLocation)
334-
return fmt.Sprintf("invalid format for field %q", fieldPath)
334+
if fieldPath != "" {
335+
return fmt.Sprintf("invalid format for field %q: %s", fieldPath, errUnit.Error.String())
336+
}
337+
return fmt.Sprintf("invalid format: %s", errUnit.Error.String())
335338

336-
case strings.Contains(errUnit.KeywordLocation, "/anyOf"):
339+
case *kind.AnyOf:
337340
// anyOf validation failed - could be null or wrong type
338341
// This happens when a field accepts [null, string] but got something else
339342
fieldPath := buildFieldPath(errUnit.InstanceLocation)
@@ -342,13 +345,31 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string {
342345
}
343346
return "invalid value"
344347

348+
case *kind.MaxLength:
349+
fieldPath := buildFieldPath(errUnit.InstanceLocation)
350+
if fieldPath != "" {
351+
return fmt.Sprintf("field %q must have maximum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got)
352+
}
353+
return errUnit.Error.String()
354+
355+
case *kind.MinLength:
356+
fieldPath := buildFieldPath(errUnit.InstanceLocation)
357+
if fieldPath != "" {
358+
return fmt.Sprintf("field %q must have minimum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got)
359+
}
360+
return errUnit.Error.String()
361+
362+
case *kind.Pattern:
363+
fieldPath := buildFieldPath(errUnit.InstanceLocation)
364+
if fieldPath != "" {
365+
return fmt.Sprintf("field %q must match pattern %q", fieldPath, errKind.Want)
366+
}
367+
return errUnit.Error.String()
368+
345369
default:
346-
// Unknown error type - return the library's error message
370+
// Unhandled error type - return the library's error message
347371
// This serves as a fallback for future schema features we haven't customized yet
348-
if errUnit.Error != nil {
349-
return errUnit.Error.String()
350-
}
351-
return ""
372+
return errUnit.Error.String()
352373
}
353374
}
354375

internal/operator-controller/config/error_formatting_test.go

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config_test
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/stretchr/testify/require"
@@ -72,9 +73,9 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) {
7273
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
7374
installNamespace: "correct-namespace",
7475
expectedErrSubstrings: []string{
76+
"invalid format for field \"watchNamespace\"",
7577
"invalid value",
7678
"wrong-namespace",
77-
"watchNamespace must be",
7879
"correct-namespace",
7980
"the namespace where the operator is installed",
8081
"OwnNamespace install mode",
@@ -86,14 +87,38 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) {
8687
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
8788
installNamespace: "install-ns",
8889
expectedErrSubstrings: []string{
90+
"invalid format for field \"watchNamespace\"",
91+
"not valid singleNamespaceInstallMode",
8992
"invalid value",
9093
"install-ns",
91-
"watchNamespace must be different from",
94+
"must be different from",
9295
"the install namespace",
9396
"SingleNamespace install mode",
9497
"watch a different namespace",
9598
},
9699
},
100+
{
101+
name: "SingleNamespace constraint error bad namespace format",
102+
rawConfig: []byte(`{"watchNamespace": "---AAAA-BBBB-super-long-namespace-that-that-is-waaaaaaaaayyy-longer-than-sixty-three-characters"}`),
103+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
104+
installNamespace: "install-ns",
105+
expectedErrSubstrings: []string{
106+
"field \"watchNamespace\"",
107+
"must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"",
108+
},
109+
},
110+
{
111+
name: "Single- and OwnNamespace constraint error bad namespace format",
112+
rawConfig: []byte(`{"watchNamespace": ` + strings.Repeat("A", 254) + `"}`),
113+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
114+
installNamespace: "install-ns",
115+
expectedErrSubstrings: []string{
116+
"invalid configuration",
117+
"multiple errors found",
118+
"must have maximum length of 253 (len=255)",
119+
"must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"",
120+
},
121+
},
97122
} {
98123
t.Run(tc.name, func(t *testing.T) {
99124
rv1 := bundle.RegistryV1{

internal/operator-controller/rukpak/bundle/registryv1.go

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ import (
1515
)
1616

1717
const (
18-
BundleConfigWatchNamespaceKey = "watchNamespace"
19-
BundleConfigDeploymentConfigKey = "deploymentConfig"
18+
watchNamespaceConfigKey = "watchNamespace"
2019
)
2120

2221
var (
@@ -69,19 +68,19 @@ func buildBundleConfigSchema(installModes sets.Set[v1alpha1.InstallMode]) (map[s
6968
if isWatchNamespaceConfigurable(installModes) {
7069
// Replace the generic watchNamespace with install-mode-specific version
7170
watchNSProperty, isRequired := buildWatchNamespaceProperty(installModes)
72-
properties["watchNamespace"] = watchNSProperty
71+
properties[watchNamespaceConfigKey] = watchNSProperty
7372

7473
// Preserve existing required fields, only add/remove watchNamespace
7574
if isRequired {
76-
addToRequired(baseSchema, "watchNamespace")
75+
addToRequired(baseSchema, watchNamespaceConfigKey)
7776
} else {
78-
removeFromRequired(baseSchema, "watchNamespace")
77+
removeFromRequired(baseSchema, watchNamespaceConfigKey)
7978
}
8079
} else {
8180
// AllNamespaces only - remove watchNamespace property entirely
8281
// (operator always watches all namespaces, no configuration needed)
83-
delete(properties, "watchNamespace")
84-
removeFromRequired(baseSchema, "watchNamespace")
82+
delete(properties, watchNamespaceConfigKey)
83+
removeFromRequired(baseSchema, watchNamespaceConfigKey)
8584
}
8685

8786
return baseSchema, nil
@@ -138,44 +137,45 @@ func removeFromRequired(schema map[string]any, fieldName string) {
138137
//
139138
// Returns the validation rules and whether the field is required.
140139
func buildWatchNamespaceProperty(installModes sets.Set[v1alpha1.InstallMode]) (map[string]any, bool) {
141-
watchNSProperty := map[string]any{
142-
"description": "The namespace that the operator should watch for custom resources",
143-
}
140+
const description = "The namespace that the operator should watch for custom resources"
144141

145142
hasOwnNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true})
146143
hasSingleNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true})
147144

148145
format := selectNamespaceFormat(hasOwnNamespace, hasSingleNamespace)
149146

150-
if isWatchNamespaceConfigRequired(installModes) {
151-
watchNSProperty["type"] = "string"
152-
if format != "" {
153-
watchNSProperty["format"] = format
154-
}
155-
return watchNSProperty, true
156-
}
157-
158-
// allow null or valid namespace string
159-
stringSchema := map[string]any{
160-
"type": "string",
147+
watchNamespaceSchema := map[string]any{
148+
"type": "string",
149+
"minLength": 1,
150+
"maxLength": 253,
151+
// kubernetes namespace name format
152+
"pattern": "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$",
161153
}
162154
if format != "" {
163-
stringSchema["format"] = format
155+
watchNamespaceSchema["format"] = format
164156
}
165-
// Convert to []any for JSON schema compatibility
166-
watchNSProperty["anyOf"] = []any{
167-
map[string]any{"type": "null"},
168-
stringSchema,
157+
158+
if isWatchNamespaceConfigRequired(installModes) {
159+
watchNamespaceSchema["description"] = description
160+
return watchNamespaceSchema, true
169161
}
170162

171-
return watchNSProperty, false
163+
// allow null or valid namespace string
164+
return map[string]any{
165+
"description": description,
166+
"anyOf": []any{
167+
map[string]any{"type": "null"},
168+
watchNamespaceSchema,
169+
},
170+
}, false
172171
}
173172

174173
// selectNamespaceFormat picks which namespace constraint to apply.
175174
//
176175
// - OwnNamespace only: watchNamespace must equal install namespace
177176
// - SingleNamespace only: watchNamespace must be different from install namespace
178-
// - Both or neither: no constraint, any namespace name is valid
177+
// - Both: watchNamespace must be a valid namespace
178+
// - neither: no constraint
179179
func selectNamespaceFormat(hasOwnNamespace, hasSingleNamespace bool) string {
180180
if hasOwnNamespace && !hasSingleNamespace {
181181
return config.FormatOwnNamespaceInstallMode

0 commit comments

Comments
 (0)