🐛 OPRUN-4217: OwnNamespace default handling

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

@@ -56,11 +56,43 @@ kubectl rollout status -n olmv1-system deployment/operator-controller-controller
 ## Configuring the `ClusterExtension`
 
 A `ClusterExtension` can be configured to install bundle in `Single-` or `OwnNamespace` mode through the
-`.spec.config.inline.watchNamespace` property. The *installMode* is inferred in the following way:
-
- - *AllNamespaces*: `watchNamespace` is empty, or not set
- - *OwnNamespace*: `watchNamespace` is the install namespace (i.e. `.spec.namespace`)
- - *SingleNamespace*: `watchNamespace` *not* the install namespace
+`.spec.config.inline.watchNamespace` property which may or may not be present or required depending on the bundle's
+install mode support, if the bundle:
+
+ - only supports *AllNamespaces* mode => `watchNamespace` is not a configuration
+ - supports *AllNamespaces* and *SingleNamespace* and/or *OwnNamespace* => `watchNamespace` is optional
+ - bundle only supports *SingleNamespace* and/or *OwnNamespace* => `watchNamespace` is required
+
+The `watchNamespace` configuration can only be the install namespace if the bundle supports the *OwnNamespace* install mode, and
+it can only be any other namespace if the bundle supports the *SingleNamespace* install mode.
+
+Examples:
+
+Bundle only supports *AllNamespaces*:
+- `watchNamespace` is not a configuration
+- bundle will be installed in *AllNamespaces* mode
+
+Bundle only supports *OwnNamespace*:
+- `watchNamespace` is required
+- `watchNamespace` must be the install namespace
+- bundle will always be installed in *OwnNamespace* mode
+
+Bundle supports *AllNamespace* and *OwnNamespace*:
+- `watchNamespace` is optional
+- if `watchNamespace` = install namespace => bundle will be installed in *OwnNamespace* mode
+- if `watchNamespace` is null or not set => bundle will be installed in *AllNamespaces* mode
+- if `watchNamespace` != install namespace => error
+
+Bundle only supports *SingleNamespace*:
+- `watchNamespace` is required
+- `watchNamespace` must *NOT* be the install namespace
+- bundle will always be installed in *SingleNamespace* mode
+
+Bundle supports *AllNamespaces*, *SingleNamespace*, and *OwnNamespace* install modes:
+- `watchNamespace` can be optionally configured
+- if `watchNamespace` = install namespace => bundle will be installed in *OwnNamespace* mode
+- if `watchNamespace` != install namespace => bundle will be installed in *SingleNamespace* mode
+- if `watchNamespace` is null or not set => bundle will be installed in *AllNamespaces* mode
 
 ### Examples
 

internal/operator-controller/applier/provider.go

@@ -68,8 +68,14 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
 		render.WithCertificateProvider(r.CertificateProvider),
 	}
 
-	if r.IsSingleOwnNamespaceEnabled && ext.Spec.Config != nil && ext.Spec.Config.ConfigType == ocv1.ClusterExtensionConfigTypeInline {
-		bundleConfig, err := bundle.UnmarshallConfig(ext.Spec.Config.Inline.Raw, rv1, ext.Spec.Namespace)
+	if r.IsSingleOwnNamespaceEnabled {
+		bundleConfigBytes := extensionConfigBytes(ext)
+		// treat no config as empty to properly validate the configuration
+		// e.g. ensure that validation catches missing required fields
+		if bundleConfigBytes == nil {
+			bundleConfigBytes = []byte(`{}`)
+		}
+		bundleConfig, err := bundle.UnmarshalConfig(bundleConfigBytes, rv1, ext.Spec.Namespace)
 		if err != nil {
 			return nil, fmt.Errorf("invalid bundle configuration: %w", err)
 		}
@@ -128,3 +134,17 @@ func (r *RegistryV1HelmChartProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExten
 
 	return chrt, nil
 }
+
+// ExtensionConfigBytes returns the ClusterExtension configuration input by the user
+// through .spec.config as a byte slice.
+func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte {
+	if ext.Spec.Config != nil {
+		switch ext.Spec.Config.ConfigType {
+		case ocv1.ClusterExtensionConfigTypeInline:
+			if ext.Spec.Config.Inline != nil {
+				return ext.Spec.Config.Inline.Raw
+			}
+		}
+	}
+	return nil
+}

internal/operator-controller/applier/provider_test.go

@@ -244,15 +244,6 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
 	t.Run("rejects bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
 		expectedWatchNamespace := "some-namespace"
 		provider := applier.RegistryV1ManifestProvider{
-			BundleRenderer: render.BundleRenderer{
-				ResourceGenerators: []render.ResourceGenerator{
-					func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
-						t.Log("ensure watch namespace is appropriately configured")
-						require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces)
-						return nil, nil
-					},
-				},
-			},
 			IsSingleOwnNamespaceEnabled: false,
 		}
 
@@ -289,7 +280,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
 		require.Contains(t, err.Error(), "unsupported bundle")
 	})
 
-	t.Run("accepts bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
+	t.Run("accepts bundles with install modes {SingleNamespace} when the appropriate configuration is given", func(t *testing.T) {
 		expectedWatchNamespace := "some-namespace"
 		provider := applier.RegistryV1ManifestProvider{
 			BundleRenderer: render.BundleRenderer{
@@ -321,20 +312,89 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
 		require.NoError(t, err)
 	})
 
-	t.Run("accepts bundles without AllNamespaces install mode and with OwnNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
+	t.Run("rejects bundles with {SingleNamespace} install modes when no configuration is given", func(t *testing.T) {
 		provider := applier.RegistryV1ManifestProvider{
 			IsSingleOwnNamespaceEnabled: true,
 		}
+
 		bundleFS := bundlefs.Builder().WithPackageName("test").
-			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
+			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()
+
 		_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
 			Spec: ocv1.ClusterExtensionSpec{
 				Namespace: "install-namespace",
 			},
 		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing")
+	})
+
+	t.Run("accepts bundles with {OwnNamespace} install modes when the appropriate configuration is given", func(t *testing.T) {
+		installNamespace := "some-namespace"
+		provider := applier.RegistryV1ManifestProvider{
+			BundleRenderer: render.BundleRenderer{
+				ResourceGenerators: []render.ResourceGenerator{
+					func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
+						t.Log("ensure watch namespace is appropriately configured")
+						require.Equal(t, []string{installNamespace}, opts.TargetNamespaces)
+						return nil, nil
+					},
+				},
+			},
+			IsSingleOwnNamespaceEnabled: true,
+		}
+		bundleFS := bundlefs.Builder().WithPackageName("test").
+			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
+		_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
+			Spec: ocv1.ClusterExtensionSpec{
+				Namespace: installNamespace,
+				Config: &ocv1.ClusterExtensionConfig{
+					ConfigType: ocv1.ClusterExtensionConfigTypeInline,
+					Inline: &apiextensionsv1.JSON{
+						Raw: []byte(`{"watchNamespace": "` + installNamespace + `"}`),
+					},
+				},
+			},
+		})
 		require.NoError(t, err)
 	})
 
+	t.Run("rejects bundles with {OwnNamespace} install modes when no configuration is given", func(t *testing.T) {
+		provider := applier.RegistryV1ManifestProvider{
+			IsSingleOwnNamespaceEnabled: true,
+		}
+		bundleFS := bundlefs.Builder().WithPackageName("test").
+			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
+		_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
+			Spec: ocv1.ClusterExtensionSpec{
+				Namespace: "install-namespace",
+			},
+		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "required field \"watchNamespace\" is missing")
+	})
+
+	t.Run("rejects bundles with {OwnNamespace} install modes when watchNamespace is not install namespace", func(t *testing.T) {
+		provider := applier.RegistryV1ManifestProvider{
+			IsSingleOwnNamespaceEnabled: true,
+		}
+		bundleFS := bundlefs.Builder().WithPackageName("test").
+			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
+		_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
+			Spec: ocv1.ClusterExtensionSpec{
+				Namespace: "install-namespace",
+				Config: &ocv1.ClusterExtensionConfig{
+					ConfigType: ocv1.ClusterExtensionConfigTypeInline,
+					Inline: &apiextensionsv1.JSON{
+						Raw: []byte(`{"watchNamespace": "not-install-namespace"}`),
+					},
+				},
+			},
+		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "invalid 'watchNamespace' \"not-install-namespace\": must be install namespace (install-namespace)")
+	})
+
 	t.Run("rejects bundles without AllNamespaces, SingleNamespace, or OwnNamespace install mode support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
 		provider := applier.RegistryV1ManifestProvider{
 			IsSingleOwnNamespaceEnabled: true,

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

@@ -17,20 +17,20 @@ type Config struct {
 	WatchNamespace *string `json:"watchNamespace"`
 }
 
-// UnmarshallConfig returns a deserialized and validated *bundle.Config based on bytes and validated
+// UnmarshalConfig returns a deserialized *bundle.Config based on bytes and validated
 // against rv1 and the desired install namespaces. It will error if:
 // - rv is nil
 // - bytes is not a valid YAML/JSON object
 // - bytes is a valid YAML/JSON object but does not follow the registry+v1 schema
-// if bytes is nil a nil bundle.Config is returned
-func UnmarshallConfig(bytes []byte, rv1 RegistryV1, installNamespace string) (*Config, error) {
+// - if bytes is nil, a nil *bundle.Config is returned with no error
+func UnmarshalConfig(bytes []byte, rv1 RegistryV1, installNamespace string) (*Config, error) {
 	if bytes == nil {
 		return nil, nil
 	}
 
 	bundleConfig := &Config{}
 	if err := yaml.UnmarshalStrict(bytes, bundleConfig); err != nil {
-		return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshallError(err))
+		return nil, fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshalError(err))
 	}
 
 	// collect bundle install modes
@@ -83,24 +83,23 @@ func validateConfig(config *Config, installNamespace string, bundleInstallModeSe
 }
 
 // isWatchNamespaceConfigSupported returns true when the bundle exposes a watchNamespace configuration. This happens when:
-// - SingleNamespace install more is supported, or
-// - OwnNamespace and AllNamespaces install modes are supported
+// - SingleNamespace and/or OwnNamespace install modes are supported
 func isWatchNamespaceConfigSupported(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool {
-	return bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}) ||
-		bundleInstallModeSet.HasAll(
-			v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true},
-			v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true})
+	return bundleInstallModeSet.HasAny(
+		v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true},
+		v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true},
+	)
 }
 
 // isWatchNamespaceConfigRequired returns true if the watchNamespace configuration is required. This happens when
-// AllNamespaces install mode is not supported and SingleNamespace is supported
+// AllNamespaces install mode is not supported and SingleNamespace and/or OwnNamespace is supported
 func isWatchNamespaceConfigRequired(bundleInstallModeSet sets.Set[v1alpha1.InstallMode]) bool {
-	return !bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}) &&
-		bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true})
+	return isWatchNamespaceConfigSupported(bundleInstallModeSet) &&
+		!bundleInstallModeSet.Has(v1alpha1.InstallMode{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true})
 }
 
-// formatUnmarshallError format JSON unmarshal errors to be more readable
-func formatUnmarshallError(err error) error {
+// formatUnmarshalError format JSON unmarshal errors to be more readable
+func formatUnmarshalError(err error) error {
 	var unmarshalErr *json.UnmarshalTypeError
 	if errors.As(err, &unmarshalErr) {
 		if unmarshalErr.Field == "" {

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

@@ -12,7 +12,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion"
 )
 
-func Test_UnmarshallConfig(t *testing.T) {
+func Test_UnmarshalConfig(t *testing.T) {
 	for _, tc := range []struct {
 		name                  string
 		rawConfig             []byte
@@ -22,7 +22,7 @@ func Test_UnmarshallConfig(t *testing.T) {
 		expectedConfig        *bundle.Config
 	}{
 		{
-			name:           "accepts nil raw config",
+			name:           "returns nil for nil config",
 			rawConfig:      nil,
 			expectedConfig: nil,
 		},
@@ -91,16 +91,28 @@ func Test_UnmarshallConfig(t *testing.T) {
 			expectedErrMessage:    "unknown field \"watchNamespace\"",
 		},
 		{
-			name:                  "reject with unknown field when install modes {OwnNamespace}",
+			name:                  "reject with required field when install modes {OwnNamespace} and watchNamespace is null",
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
-			rawConfig:             []byte(`{"watchNamespace": "some-namespace"}`),
-			expectedErrMessage:    "unknown field \"watchNamespace\"",
+			rawConfig:             []byte(`{"watchNamespace": null}`),
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
+		},
+		{
+			name:                  "reject with required field when install modes {OwnNamespace} and watchNamespace is missing",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
+			rawConfig:             []byte(`{}`),
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
 		},
 		{
-			name:                  "reject with unknown field when install modes {MultiNamespace, OwnNamespace}",
+			name:                  "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is null",
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace},
-			rawConfig:             []byte(`{"watchNamespace": "some-namespace"}`),
-			expectedErrMessage:    "unknown field \"watchNamespace\"",
+			rawConfig:             []byte(`{"watchNamespace": null}`),
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
+		},
+		{
+			name:                  "reject with required field when install modes {MultiNamespace, OwnNamespace} and watchNamespace is missing",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeMultiNamespace, v1alpha1.InstallModeTypeOwnNamespace},
+			rawConfig:             []byte(`{}`),
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
 		},
 		{
 			name:                  "accepts when install modes {SingleNamespace} and watchNamespace != install namespace",
@@ -202,6 +214,27 @@ func Test_UnmarshallConfig(t *testing.T) {
 			installNamespace:      "not-some-namespace",
 			expectedErrMessage:    "required field \"watchNamespace\" is missing",
 		},
+		{
+			name:                  "rejects with required field error when install modes {SingleNamespace} and watchNamespace is missing",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
+			rawConfig:             []byte(`{}`),
+			installNamespace:      "not-some-namespace",
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
+		},
+		{
+			name:                  "rejects with required field error when install modes {SingleNamespace, OwnNamespace} and watchNamespace is missing",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
+			rawConfig:             []byte(`{}`),
+			installNamespace:      "not-some-namespace",
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
+		},
+		{
+			name:                  "rejects with required field error when install modes {SingleNamespace, MultiNamespace} and watchNamespace is missing",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace},
+			rawConfig:             []byte(`{}`),
+			installNamespace:      "not-some-namespace",
+			expectedErrMessage:    "required field \"watchNamespace\" is missing",
+		},
 		{
 			name:                  "rejects with required field error when install modes {SingleNamespace, OwnNamespace, MultiNamespace} and watchNamespace is nil",
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace},
@@ -227,6 +260,24 @@ func Test_UnmarshallConfig(t *testing.T) {
 				WatchNamespace: nil,
 			},
 		},
+		{
+			name:                  "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace} and watchNamespace is nil",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace},
+			rawConfig:             []byte(`{}`),
+			installNamespace:      "not-some-namespace",
+			expectedConfig: &bundle.Config{
+				WatchNamespace: nil,
+			},
+		},
+		{
+			name:                  "accepts no watchNamespace when install modes {AllNamespaces, OwnNamespace, MultiNamespace} and watchNamespace is nil",
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace},
+			rawConfig:             []byte(`{}`),
+			installNamespace:      "not-some-namespace",
+			expectedConfig: &bundle.Config{
+				WatchNamespace: nil,
+			},
+		},
 		{
 			name:                  "rejects with format error when install modes are {SingleNamespace, OwnNamespace} and watchNamespace is ''",
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
@@ -246,7 +297,7 @@ func Test_UnmarshallConfig(t *testing.T) {
 				}
 			}
 
-			config, err := bundle.UnmarshallConfig(tc.rawConfig, rv1, tc.installNamespace)
+			config, err := bundle.UnmarshalConfig(tc.rawConfig, rv1, tc.installNamespace)
 			require.Equal(t, tc.expectedConfig, config)
 			if tc.expectedErrMessage != "" {
 				require.Error(t, err)