Skip to content

Commit e392d2b

Browse files
author
Per G. da Silva
committed
Addressed reviewer comments
Signed-off-by: Per G. da Silva <[email protected]>
1 parent 1de21cd commit e392d2b

File tree

7 files changed

+38
-44
lines changed

7 files changed

+38
-44
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ install mode support, if the bundle:
6464
- bundle only supports *SingleNamespace* and/or *OwnNamespace* => `watchNamespace` is required
6565

6666
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 is the bundle supports the *SingleNamespace* install mode.
67+
it can only be any other namespace if the bundle supports the *SingleNamespace* install mode.
6868

6969
Examples:
7070

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,13 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
6969
}
7070

7171
if r.IsSingleOwnNamespaceEnabled {
72-
bundleConfig, err := bundle.UnmarshallConfig(ext.ExtensionConfigBytes(), rv1, ext.Spec.Namespace)
72+
bundleConfigBytes := ext.ExtensionConfigBytes()
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
}

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

Lines changed: 12 additions & 21 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

@@ -322,17 +313,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
322313
})
323314

324315
t.Run("rejects bundles with {SingleNamespace} install modes when no configuration is given", func(t *testing.T) {
325-
expectedWatchNamespace := "some-namespace"
326316
provider := applier.RegistryV1ManifestProvider{
327-
BundleRenderer: render.BundleRenderer{
328-
ResourceGenerators: []render.ResourceGenerator{
329-
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
330-
t.Log("ensure watch namespace is appropriately configured")
331-
require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces)
332-
return nil, nil
333-
},
334-
},
335-
},
336317
IsSingleOwnNamespaceEnabled: true,
337318
}
338319

@@ -349,18 +330,28 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
349330
})
350331

351332
t.Run("accepts bundles with {OwnNamespace} install modes when the appropriate configuration is given", func(t *testing.T) {
333+
installNamespace := "some-namespace"
352334
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+
},
353344
IsSingleOwnNamespaceEnabled: true,
354345
}
355346
bundleFS := bundlefs.Builder().WithPackageName("test").
356347
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
357348
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
358349
Spec: ocv1.ClusterExtensionSpec{
359-
Namespace: "install-namespace",
350+
Namespace: installNamespace,
360351
Config: &ocv1.ClusterExtensionConfig{
361352
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
362353
Inline: &apiextensionsv1.JSON{
363-
Raw: []byte(`{"watchNamespace": "install-namespace"}`),
354+
Raw: []byte(`{"watchNamespace": "` + installNamespace + `"}`),
364355
},
365356
},
366357
},

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

Lines changed: 7 additions & 7 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 it will be treated as an empty json object ({})
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 {
28-
bytes = []byte("{}")
28+
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
@@ -98,8 +98,8 @@ func isWatchNamespaceConfigRequired(bundleInstallModeSet sets.Set[v1alpha1.Insta
9898
!bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true})
9999
}
100100

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

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

Lines changed: 5 additions & 7 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,11 +22,9 @@ func Test_UnmarshallConfig(t *testing.T) {
2222
expectedConfig *bundle.Config
2323
}{
2424
{
25-
name: "treats nil config as {}",
26-
rawConfig: nil,
27-
supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
28-
expectedConfig: nil,
29-
expectedErrMessage: "required field \"watchNamespace\" is missing",
25+
name: "returns nil for nil config",
26+
rawConfig: nil,
27+
expectedConfig: nil,
3028
},
3129
{
3230
name: "accepts json config",
@@ -299,7 +297,7 @@ func Test_UnmarshallConfig(t *testing.T) {
299297
}
300298
}
301299

302-
config, err := bundle.UnmarshallConfig(tc.rawConfig, rv1, tc.installNamespace)
300+
config, err := bundle.UnmarshalConfig(tc.rawConfig, rv1, tc.installNamespace)
303301
require.Equal(t, tc.expectedConfig, config)
304302
if tc.expectedErrMessage != "" {
305303
require.Error(t, err)

‎internal/operator-controller/rukpak/render/render.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,8 @@ func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, t
162162
// If only OwnNamespace is supported, the default will be [install-namespace] -> only watch the install/own namespace
163163
if supportedInstallModes.Has(v1alpha1.InstallModeTypeMultiNamespace) {
164164
return errors.New("at least one target namespace must be specified")
165-
} else {
166-
return errors.New("exactly one target namespace must be specified")
167165
}
166+
return errors.New("exactly one target namespace must be specified")
168167
case set.Len() == 1 && set.Has(""):
169168
if supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
170169
return nil

‎testdata/images/bundles/own-namespace-operator/v1.0.0/manifests/ownnamespaceoperator.clusterserviceversion.yaml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ spec:
2727
apiservicedefinitions: {}
2828
customresourcedefinitions:
2929
owned:
30-
- description: A dummy resource for an operator that only supports own namespace install mode
31-
displayName: OwnNamespace
32-
kind: OwnNamespace
33-
name: ownnamespaces.olm.operatorframework.io
34-
version: v1
30+
- description: A dummy resource for an operator that only supports own namespace install mode
31+
displayName: OwnNamespace
32+
kind: OwnNamespace
33+
name: ownnamespaces.olm.operatorframework.io
34+
version: v1
3535
description: OLM OwnNamespace Testing Operator
3636
displayName: test-operator
3737
icon:

0 commit comments

Comments
 (0)