Skip to content

Commit 8b6debd

Browse files
perdasilvaPer G. da Silva
perdasilva
and
Per G. da Silva
authored
🐛 OPRUN-4217: OwnNamespace default handling (#2283)
* Add additional watchNamespace config unit tests Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Make watchNamespace config to be required for OwnNamespace bundles Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Update reg+v1 renderer default targetNamespace behavior and add tests Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Fix renderer / applier integration Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Update Single/OwnNamespace support e2e Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Update docs Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Addressed reviewer comments Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Fix testdata format Signed-off-by: Per G. da Silva <pegoncal@redhat.com> --------- Signed-off-by: Per G. da Silva <pegoncal@redhat.com> Co-authored-by: Per G. da Silva <pegoncal@redhat.com>
1 parent 3a6afed commit 8b6debd

File tree

15 files changed

+936
-70
lines changed

15 files changed

+936
-70
lines changed

‎docs/draft/howto/single-ownnamespace-install.md‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,43 @@ kubectl rollout status -n olmv1-system deployment/operator-controller-controller
5656
## Configuring the `ClusterExtension`
5757

5858
A `ClusterExtension` can be configured to install bundle in `Single-` or `OwnNamespace` mode through the
59-
`.spec.config.inline.watchNamespace` property. The *installMode* is inferred in the following way:
60-
61-
- *AllNamespaces*: `watchNamespace` is empty, or not set
62-
- *OwnNamespace*: `watchNamespace` is the install namespace (i.e. `.spec.namespace`)
63-
- *SingleNamespace*: `watchNamespace` *not* the install namespace
59+
`.spec.config.inline.watchNamespace` property which may or may not be present or required depending on the bundle's
60+
install mode support, if the bundle:
61+
62+
- only supports *AllNamespaces* mode => `watchNamespace` is not a configuration
63+
- supports *AllNamespaces* and *SingleNamespace* and/or *OwnNamespace* => `watchNamespace` is optional
64+
- bundle only supports *SingleNamespace* and/or *OwnNamespace* => `watchNamespace` is required
65+
66+
The `watchNamespace` configuration can only be the install namespace if the bundle supports the *OwnNamespace* install mode, and
67+
it can only be any other namespace if the bundle supports the *SingleNamespace* install mode.
68+
69+
Examples:
70+
71+
Bundle only supports *AllNamespaces*:
72+
- `watchNamespace` is not a configuration
73+
- bundle will be installed in *AllNamespaces* mode
74+
75+
Bundle only supports *OwnNamespace*:
76+
- `watchNamespace` is required
77+
- `watchNamespace` must be the install namespace
78+
- bundle will always be installed in *OwnNamespace* mode
79+
80+
Bundle supports *AllNamespace* and *OwnNamespace*:
81+
- `watchNamespace` is optional
82+
- if `watchNamespace` = install namespace => bundle will be installed in *OwnNamespace* mode
83+
- if `watchNamespace` is null or not set => bundle will be installed in *AllNamespaces* mode
84+
- if `watchNamespace` != install namespace => error
85+
86+
Bundle only supports *SingleNamespace*:
87+
- `watchNamespace` is required
88+
- `watchNamespace` must *NOT* be the install namespace
89+
- bundle will always be installed in *SingleNamespace* mode
90+
91+
Bundle supports *AllNamespaces*, *SingleNamespace*, and *OwnNamespace* install modes:
92+
- `watchNamespace` can be optionally configured
93+
- if `watchNamespace` = install namespace => bundle will be installed in *OwnNamespace* mode
94+
- if `watchNamespace` != install namespace => bundle will be installed in *SingleNamespace* mode
95+
- if `watchNamespace` is null or not set => bundle will be installed in *AllNamespaces* mode
6496

6597
### Examples
6698

‎internal/operator-controller/applier/provider.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,14 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
6868
render.WithCertificateProvider(r.CertificateProvider),
6969
}
7070

71-
if r.IsSingleOwnNamespaceEnabled && ext.Spec.Config != nil && ext.Spec.Config.ConfigType == ocv1.ClusterExtensionConfigTypeInline {
72-
bundleConfig, err := bundle.UnmarshallConfig(ext.Spec.Config.Inline.Raw, rv1, ext.Spec.Namespace)
71+
if r.IsSingleOwnNamespaceEnabled {
72+
bundleConfigBytes := extensionConfigBytes(ext)
73+
// treat no config as empty to properly validate the configuration
74+
// e.g. ensure that validation catches missing required fields
75+
if bundleConfigBytes == nil {
76+
bundleConfigBytes = []byte(`{}`)
77+
}
78+
bundleConfig, err := bundle.UnmarshalConfig(bundleConfigBytes, rv1, ext.Spec.Namespace)
7379
if err != nil {
7480
return nil, fmt.Errorf("invalid bundle configuration: %w", err)
7581
}
@@ -128,3 +134,17 @@ func (r *RegistryV1HelmChartProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExten
128134

129135
return chrt, nil
130136
}
137+
138+
// ExtensionConfigBytes returns the ClusterExtension configuration input by the user
139+
// through .spec.config as a byte slice.
140+
func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte {
141+
if ext.Spec.Config != nil {
142+
switch ext.Spec.Config.ConfigType {
143+
case ocv1.ClusterExtensionConfigTypeInline:
144+
if ext.Spec.Config.Inline != nil {
145+
return ext.Spec.Config.Inline.Raw
146+
}
147+
}
148+
}
149+
return nil
150+
}

‎internal/operator-controller/applier/provider_test.go‎

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,6 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
244244
t.Run("rejects bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
245245
expectedWatchNamespace := "some-namespace"
246246
provider := applier.RegistryV1ManifestProvider{
247-
BundleRenderer: render.BundleRenderer{
248-
ResourceGenerators: []render.ResourceGenerator{
249-
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
250-
t.Log("ensure watch namespace is appropriately configured")
251-
require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces)
252-
return nil, nil
253-
},
254-
},
255-
},
256247
IsSingleOwnNamespaceEnabled: false,
257248
}
258249

@@ -289,7 +280,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
289280
require.Contains(t, err.Error(), "unsupported bundle")
290281
})
291282

292-
t.Run("accepts bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
283+
t.Run("accepts bundles with install modes {SingleNamespace} when the appropriate configuration is given", func(t *testing.T) {
293284
expectedWatchNamespace := "some-namespace"
294285
provider := applier.RegistryV1ManifestProvider{
295286
BundleRenderer: render.BundleRenderer{
@@ -321,20 +312,89 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
321312
require.NoError(t, err)
322313
})
323314

324-
t.Run("accepts bundles without AllNamespaces install mode and with OwnNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
315+
t.Run("rejects bundles with {SingleNamespace} install modes when no configuration is given", func(t *testing.T) {
325316
provider := applier.RegistryV1ManifestProvider{
326317
IsSingleOwnNamespaceEnabled: true,
327318
}
319+
328320
bundleFS := bundlefs.Builder().WithPackageName("test").
329-
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
321+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()
322+
330323
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
331324
Spec: ocv1.ClusterExtensionSpec{
332325
Namespace: "install-namespace",
333326
},
334327
})
328+
require.Error(t, err)
329+
require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing")
330+
})
331+
332+
t.Run("accepts bundles with {OwnNamespace} install modes when the appropriate configuration is given", func(t *testing.T) {
333+
installNamespace := "some-namespace"
334+
provider := applier.RegistryV1ManifestProvider{
335+
BundleRenderer: render.BundleRenderer{
336+
ResourceGenerators: []render.ResourceGenerator{
337+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
338+
t.Log("ensure watch namespace is appropriately configured")
339+
require.Equal(t, []string{installNamespace}, opts.TargetNamespaces)
340+
return nil, nil
341+
},
342+
},
343+
},
344+
IsSingleOwnNamespaceEnabled: true,
345+
}
346+
bundleFS := bundlefs.Builder().WithPackageName("test").
347+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
348+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
349+
Spec: ocv1.ClusterExtensionSpec{
350+
Namespace: installNamespace,
351+
Config: &ocv1.ClusterExtensionConfig{
352+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
353+
Inline: &apiextensionsv1.JSON{
354+
Raw: []byte(`{"watchNamespace": "` + installNamespace + `"}`),
355+
},
356+
},
357+
},
358+
})
335359
require.NoError(t, err)
336360
})
337361

362+
t.Run("rejects bundles with {OwnNamespace} install modes when no configuration is given", func(t *testing.T) {
363+
provider := applier.RegistryV1ManifestProvider{
364+
IsSingleOwnNamespaceEnabled: true,
365+
}
366+
bundleFS := bundlefs.Builder().WithPackageName("test").
367+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
368+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
369+
Spec: ocv1.ClusterExtensionSpec{
370+
Namespace: "install-namespace",
371+
},
372+
})
373+
require.Error(t, err)
374+
require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing")
375+
})
376+
377+
t.Run("rejects bundles with {OwnNamespace} install modes when watchNamespace is not install namespace", func(t *testing.T) {
378+
provider := applier.RegistryV1ManifestProvider{
379+
IsSingleOwnNamespaceEnabled: true,
380+
}
381+
bundleFS := bundlefs.Builder().WithPackageName("test").
382+
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
383+
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
384+
Spec: ocv1.ClusterExtensionSpec{
385+
Namespace: "install-namespace",
386+
Config: &ocv1.ClusterExtensionConfig{
387+
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
388+
Inline: &apiextensionsv1.JSON{
389+
Raw: []byte(`{"watchNamespace": "not-install-namespace"}`),
390+
},
391+
},
392+
},
393+
})
394+
require.Error(t, err)
395+
require.Contains(t, err.Error(), "invalid 'watchNamespace' \"not-install-namespace\": must be install namespace (install-namespace)")
396+
})
397+
338398
t.Run("rejects bundles without AllNamespaces, SingleNamespace, or OwnNamespace install mode support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
339399
provider := applier.RegistryV1ManifestProvider{
340400
IsSingleOwnNamespaceEnabled: true,

‎internal/operator-controller/rukpak/bundle/config.go‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,20 @@ type Config struct {
1717
WatchNamespace *string `json:"watchNamespace"`
1818
}
1919

20-
// UnmarshallConfig returns a deserialized and validated *bundle.Config based on bytes and validated
20+
// UnmarshalConfig returns a deserialized *bundle.Config based on bytes and validated
2121
// against rv1 and the desired install namespaces. It will error if:
2222
// - rv is nil
2323
// - bytes is not a valid YAML/JSON object
2424
// - bytes is a valid YAML/JSON object but does not follow the registry+v1 schema
25-
// if bytes is nil a nil bundle.Config is returned
26-
func UnmarshallConfig(bytes []byte, rv1 RegistryV1, installNamespace string) (*Config, error) {
25+
// - if bytes is nil, a nil *bundle.Config is returned with no error
26+
func UnmarshalConfig(bytes []byte, rv1 RegistryV1, installNamespace string) (*Config, error) {
2727
if bytes == nil {
2828
return nil, nil
2929
}
3030

3131
bundleConfig := &Config{}
3232
if err := yaml.UnmarshalStrict(bytes, bundleConfig); err != nil {
33-
return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshallError(err))
33+
return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshalError(err))
3434
}
3535

3636
// collect bundle install modes
@@ -83,24 +83,23 @@ func validateConfig(config *Config, installNamespace string, bundleInstallModeSe
8383
}
8484

8585
// isWatchNamespaceConfigSupported returns true when the bundle exposes a watchNamespace configuration. This happens when:
86-
// - SingleNamespace install more is supported, or
87-
// - OwnNamespace and AllNamespaces install modes are supported
86+
// - SingleNamespace and/or OwnNamespace install modes are supported
8887
func isWatchNamespaceConfigSupported(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool {
89-
return bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}) ||
90-
bundleInstallModeSet.HasAll(
91-
v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true},
92-
v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true})
88+
return bundleInstallModeSet.HasAny(
89+
v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true},
90+
v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true},
91+
)
9392
}
9493

9594
// isWatchNamespaceConfigRequired returns true if the watchNamespace configuration is required. This happens when
96-
// AllNamespaces install mode is not supported and SingleNamespace is supported
95+
// AllNamespaces install mode is not supported and SingleNamespace and/or OwnNamespace is supported
9796
func isWatchNamespaceConfigRequired(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool {
98-
return !bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) &&
99-
bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true})
97+
return isWatchNamespaceConfigSupported(bundleInstallModeSet) &&
98+
!bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true})
10099
}
101100

102-
// formatUnmarshallError format JSON unmarshal errors to be more readable
103-
func formatUnmarshallError(err error) error {
101+
// formatUnmarshalError format JSON unmarshal errors to be more readable
102+
func formatUnmarshalError(err error) error {
104103
var unmarshalErr *json.UnmarshalTypeError
105104
if errors.As(err, &unmarshalErr) {
106105
if unmarshalErr.Field == "" {

‎internal/operator-controller/rukpak/bundle/config_test.go‎

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion"
1313
)
1414

15-
func Test_UnmarshallConfig(t *testing.T) {
15+
func Test_UnmarshalConfig(t *testing.T) {
1616
for _, tc := range []struct {
1717
name string
1818
rawConfig []byte
@@ -22,7 +22,7 @@ func Test_UnmarshallConfig(t *testing.T) {
2222
expectedConfig *bundle.Config
2323
}{
2424
{
25-
name: "accepts nil raw config",
25+
name: "returns nil for nil config",
2626
rawConfig: nil,
2727
expectedConfig: nil,
2828
},
@@ -91,16 +91,28 @@ func Test_UnmarshallConfig(t *testing.T) {
9191
expectedErrMessage: "unknown field \"watchNamespace\"",
9292
},
9393
{
94-
name: "reject with unknown field when install modes {OwnNamespace}",
94+
name: "reject with required field when install modes {OwnNamespace} and watchNamespace is null",
9595
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
96-
rawConfig: []byte(`{"watchNamespace": "some-namespace"}`),
97-
expectedErrMessage: "unknown field \"watchNamespace\"",
96+
rawConfig: []byte(`{"watchNamespace": null}`),
97+
expectedErrMessage: "required field \"watchNamespace\" is missing",
98+
},
99+
{
100+
name: "reject with required field when install modes {OwnNamespace} and watchNamespace is missing",
101+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
102+
rawConfig: []byte(`{}`),
103+
expectedErrMessage: "required field \"watchNamespace\" is missing",
98104
},
99105
{
100-
name: "reject with unknown field when install modes {MultiNamespace, OwnNamespace}",
106+
name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is null",
101107
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace},
102-
rawConfig: []byte(`{"watchNamespace": "some-namespace"}`),
103-
expectedErrMessage: "unknown field \"watchNamespace\"",
108+
rawConfig: []byte(`{"watchNamespace": null}`),
109+
expectedErrMessage: "required field \"watchNamespace\" is missing",
110+
},
111+
{
112+
name: "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is missing",
113+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace},
114+
rawConfig: []byte(`{}`),
115+
expectedErrMessage: "required field \"watchNamespace\" is missing",
104116
},
105117
{
106118
name: "accepts when install modes {SingleNamespace} and watchNamespace != install namespace",
@@ -202,6 +214,27 @@ func Test_UnmarshallConfig(t *testing.T) {
202214
installNamespace: "not-some-namespace",
203215
expectedErrMessage: "required field \"watchNamespace\" is missing",
204216
},
217+
{
218+
name: "rejects with required field error when install modes {SingleNamespace} and watchNamespace is missing",
219+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
220+
rawConfig: []byte(`{}`),
221+
installNamespace: "not-some-namespace",
222+
expectedErrMessage: "required field \"watchNamespace\" is missing",
223+
},
224+
{
225+
name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is missing",
226+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
227+
rawConfig: []byte(`{}`),
228+
installNamespace: "not-some-namespace",
229+
expectedErrMessage: "required field \"watchNamespace\" is missing",
230+
},
231+
{
232+
name: "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is missing",
233+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace},
234+
rawConfig: []byte(`{}`),
235+
installNamespace: "not-some-namespace",
236+
expectedErrMessage: "required field \"watchNamespace\" is missing",
237+
},
205238
{
206239
name: "rejects with required field error when install modes {SingleNamespace, OwnNamespace, MultiNamespace} and watchNamespace is nil",
207240
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace},
@@ -227,6 +260,24 @@ func Test_UnmarshallConfig(t *testing.T) {
227260
WatchNamespace: nil,
228261
},
229262
},
263+
{
264+
name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil",
265+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace},
266+
rawConfig: []byte(`{}`),
267+
installNamespace: "not-some-namespace",
268+
expectedConfig: &bundle.Config{
269+
WatchNamespace: nil,
270+
},
271+
},
272+
{
273+
name: "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil",
274+
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace},
275+
rawConfig: []byte(`{}`),
276+
installNamespace: "not-some-namespace",
277+
expectedConfig: &bundle.Config{
278+
WatchNamespace: nil,
279+
},
280+
},
230281
{
231282
name: "rejects with format error when install modes are {SingleNamespace, OwnNamespace} and watchNamespace is ''",
232283
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
@@ -246,7 +297,7 @@ func Test_UnmarshallConfig(t *testing.T) {
246297
}
247298
}
248299

249-
config, err := bundle.UnmarshallConfig(tc.rawConfig, rv1, tc.installNamespace)
300+
config, err := bundle.UnmarshalConfig(tc.rawConfig, rv1, tc.installNamespace)
250301
require.Equal(t, tc.expectedConfig, config)
251302
if tc.expectedErrMessage != "" {
252303
require.Error(t, err)

0 commit comments

Comments
 (0)