Skip to content

Commit 228d5ab

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 228d5ab

5 files changed

Lines changed: 100 additions & 53 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 & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import (
1515
)
1616

1717
const (
18-
BundleConfigWatchNamespaceKey = "watchNamespace"
19-
BundleConfigDeploymentConfigKey = "deploymentConfig"
18+
watchNamespaceConfigKey = "watchNamespace"
19+
namespaceNamePattern = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
20+
namespaceNameMaxLength = 253
2021
)
2122

2223
var (
@@ -69,19 +70,19 @@ func buildBundleConfigSchema(installModes sets.Set[v1alpha1.InstallMode]) (map[s
6970
if isWatchNamespaceConfigurable(installModes) {
7071
// Replace the generic watchNamespace with install-mode-specific version
7172
watchNSProperty, isRequired := buildWatchNamespaceProperty(installModes)
72-
properties["watchNamespace"] = watchNSProperty
73+
properties[watchNamespaceConfigKey] = watchNSProperty
7374

7475
// Preserve existing required fields, only add/remove watchNamespace
7576
if isRequired {
76-
addToRequired(baseSchema, "watchNamespace")
77+
addToRequired(baseSchema, watchNamespaceConfigKey)
7778
} else {
78-
removeFromRequired(baseSchema, "watchNamespace")
79+
removeFromRequired(baseSchema, watchNamespaceConfigKey)
7980
}
8081
} else {
8182
// AllNamespaces only - remove watchNamespace property entirely
8283
// (operator always watches all namespaces, no configuration needed)
83-
delete(properties, "watchNamespace")
84-
removeFromRequired(baseSchema, "watchNamespace")
84+
delete(properties, watchNamespaceConfigKey)
85+
removeFromRequired(baseSchema, watchNamespaceConfigKey)
8586
}
8687

8788
return baseSchema, nil
@@ -138,37 +139,37 @@ func removeFromRequired(schema map[string]any, fieldName string) {
138139
//
139140
// Returns the validation rules and whether the field is required.
140141
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-
}
142+
const description = "The namespace that the operator should watch for custom resources"
144143

145144
hasOwnNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true})
146145
hasSingleNamespace := installModes.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true})
147146

148147
format := selectNamespaceFormat(hasOwnNamespace, hasSingleNamespace)
149148

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",
149+
watchNamespaceSchema := map[string]any{
150+
"type": "string",
151+
"minLength": 1,
152+
"maxLength": namespaceNameMaxLength,
153+
// kubernetes namespace name format
154+
"pattern": namespaceNamePattern,
161155
}
162156
if format != "" {
163-
stringSchema["format"] = format
157+
watchNamespaceSchema["format"] = format
164158
}
165-
// Convert to []any for JSON schema compatibility
166-
watchNSProperty["anyOf"] = []any{
167-
map[string]any{"type": "null"},
168-
stringSchema,
159+
160+
if isWatchNamespaceConfigRequired(installModes) {
161+
watchNamespaceSchema["description"] = description
162+
return watchNamespaceSchema, true
169163
}
170164

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

174175
// selectNamespaceFormat picks which namespace constraint to apply.

test/e2e/features/install.feature

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,10 @@ Feature: Install ClusterExtension
200200
And ClusterExtension reports Progressing as True with Reason Retrying and Message:
201201
"""
202202
error for resolved bundle "own-namespace-operator.1.0.0" with version
203-
"1.0.0": invalid ClusterExtension configuration: invalid configuration: 'some-ns'
204-
is not valid ownNamespaceInstallMode: invalid value "some-ns": watchNamespace
205-
must be "${TEST_NAMESPACE}" (the namespace where the operator is installed) because this
206-
operator only supports OwnNamespace install mode
203+
"1.0.0": invalid ClusterExtension configuration: invalid configuration: invalid
204+
format for field "watchNamespace":'some-ns' is not valid ownNamespaceInstallMode:
205+
invalid value "some-ns": watchNamespace must be "${TEST_NAMESPACE}" (the namespace where the
206+
operator is installed) because this operator only supports OwnNamespace install mode
207207
"""
208208
When ClusterExtension is updated to set watchNamespace to own namespace value
209209
"""

0 commit comments

Comments
 (0)