From f26be512e94859a5f90f0c6f5446f43294f3dc7e Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 6 Feb 2025 17:37:32 +0100 Subject: [PATCH 1/7] Add feature gate Signed-off-by: Per Goncalves da Silva --- internal/operator-controller/features/features.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 7b308dae0..885f3b4db 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -8,7 +8,8 @@ import ( const ( // Add new feature gates constants (strings) // Ex: SomeFeature featuregate.Feature = "SomeFeature" - PreflightPermissions featuregate.Feature = "PreflightPermissions" + PreflightPermissions featuregate.Feature = "PreflightPermissions" + SingleOwnNamespaceInstallSupport featuregate.Feature = "SingleOwnNamespaceInstallSupport" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -19,6 +20,15 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature PreRelease: featuregate.Alpha, LockToDefault: false, }, + + // SingleOwnNamespaceInstallSupport enables support for installing + // registry+v1 cluster extensions with single or own namespaces modes + // i.e. with a single watch namespace. + SingleOwnNamespaceInstallSupport: { + Default: false, + PreRelease: featuregate.Alpha, + LockToDefault: false, + }, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() From cc6f1c444937d0a64c66ef7532cb1ade8fa71224 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 6 Feb 2025 17:38:05 +0100 Subject: [PATCH 2/7] Get extension watch namespace from annotation Signed-off-by: Per Goncalves da Silva --- internal/operator-controller/applier/helm.go | 7 +- .../applier/watchnamespace.go | 32 +++++++ .../applier/watchnamespace_test.go | 83 +++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 internal/operator-controller/applier/watchnamespace.go create mode 100644 internal/operator-controller/applier/watchnamespace_test.go diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 76df085cb..9a46b895c 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -15,7 +15,6 @@ import ( "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/client" @@ -79,7 +78,11 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu } func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) { - chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{corev1.NamespaceAll}) + watchNamespace, err := GetWatchNamespace(ext) + if err != nil { + return nil, "", err + } + chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{watchNamespace}) if err != nil { return nil, "", err } diff --git a/internal/operator-controller/applier/watchnamespace.go b/internal/operator-controller/applier/watchnamespace.go new file mode 100644 index 000000000..193f456b3 --- /dev/null +++ b/internal/operator-controller/applier/watchnamespace.go @@ -0,0 +1,32 @@ +package applier + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" +) + +const ( + AnnotationClusterExtensionWatchNamespace = "olm.operatorframework.io/watch-namespace" +) + +// GetWatchNamespace determines the watch namespace the ClusterExtension should use +// Note: this is a temporary artifice to enable gated use of single/own namespace install modes +// for registry+v1 bundles. This will go away once the ClusterExtension API is updated to include +// (opaque) runtime configuration. +func GetWatchNamespace(ext *ocv1.ClusterExtension) (string, error) { + if features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport) { + if ext != nil && ext.Annotations[AnnotationClusterExtensionWatchNamespace] != "" { + watchNamespace := ext.Annotations[AnnotationClusterExtensionWatchNamespace] + if errs := validation.IsDNS1123Subdomain(watchNamespace); len(errs) > 0 { + return "", fmt.Errorf("invalid watch namespace '%s': namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", watchNamespace) + } + return ext.Annotations[AnnotationClusterExtensionWatchNamespace], nil + } + } + return corev1.NamespaceAll, nil +} diff --git a/internal/operator-controller/applier/watchnamespace_test.go b/internal/operator-controller/applier/watchnamespace_test.go new file mode 100644 index 000000000..86a822201 --- /dev/null +++ b/internal/operator-controller/applier/watchnamespace_test.go @@ -0,0 +1,83 @@ +package applier_test + +import ( + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + featuregatetesting "k8s.io/component-base/featuregate/testing" + + v1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" +) + +func TestGetWatchNamespace(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) + + for _, tt := range []struct { + name string + want string + csv *v1.ClusterExtension + expectError bool + }{ + { + name: "cluster extension does not have watch namespace annotation", + want: corev1.NamespaceAll, + csv: &v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extension", + Annotations: nil, + }, + Spec: v1.ClusterExtensionSpec{}, + }, + expectError: false, + }, { + name: "cluster extension has valid namespace annotation", + want: "watch-namespace", + csv: &v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extension", + Annotations: map[string]string{ + "olm.operatorframework.io/watch-namespace": "watch-namespace", + }, + }, + Spec: v1.ClusterExtensionSpec{}, + }, + expectError: false, + }, { + name: "cluster extension has invalid namespace annotation: multiple watch namespaces", + want: "", + csv: &v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extension", + Annotations: map[string]string{ + "olm.operatorframework.io/watch-namespace": "watch-namespace,watch-namespace2,watch-namespace3", + }, + }, + Spec: v1.ClusterExtensionSpec{}, + }, + expectError: true, + }, { + name: "cluster extension has invalid namespace annotation: invalid name", + want: "", + csv: &v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extension", + Annotations: map[string]string{ + "olm.operatorframework.io/watch-namespace": "watch-namespace-", + }, + }, + Spec: v1.ClusterExtensionSpec{}, + }, + expectError: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + got, err := applier.GetWatchNamespace(tt.csv) + require.Equal(t, tt.want, got) + require.Equal(t, tt.expectError, err != nil) + }) + } +} From f8d88747ed2ea18222fe70abb1d8b152277b7470 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 11 Feb 2025 16:18:07 +0100 Subject: [PATCH 3/7] Make bundle->chart function configurable in applier and add tests Signed-off-by: Per Goncalves da Silva --- cmd/operator-controller/main.go | 6 +- internal/operator-controller/applier/helm.go | 25 ++- .../operator-controller/applier/helm_test.go | 160 ++++++++++++++++-- .../applier/watchnamespace_test.go | 2 +- .../rukpak/convert/registryv1.go | 18 +- .../rukpak/convert/registryv1_test.go | 9 +- 6 files changed, 177 insertions(+), 43 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index ee6450a05..56949ffd7 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -65,6 +65,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/finalizers" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/operator-controller/scheme" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" @@ -407,8 +408,9 @@ func run() error { } helmApplier := &applier.Helm{ - ActionClientGetter: acg, - Preflights: preflights, + ActionClientGetter: acg, + Preflights: preflights, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, } cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 9a46b895c..60a03477a 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -24,7 +24,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/features" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -52,9 +51,12 @@ type Preflight interface { Upgrade(context.Context, *release.Release) error } +type BundleToHelmChartFn func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) + type Helm struct { - ActionClientGetter helmclient.ActionClientGetter - Preflights []Preflight + ActionClientGetter helmclient.ActionClientGetter + Preflights []Preflight + BundleToHelmChartFn BundleToHelmChartFn } // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND @@ -78,11 +80,7 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu } func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) { - watchNamespace, err := GetWatchNamespace(ext) - if err != nil { - return nil, "", err - } - chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{watchNamespace}) + chrt, err := h.buildHelmChart(contentFS, ext) if err != nil { return nil, "", err } @@ -155,6 +153,17 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte return relObjects, state, nil } +func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + if h.BundleToHelmChartFn == nil { + return nil, errors.New("BundleToHelmChartFn is nil") + } + watchNamespace, err := GetWatchNamespace(ext) + if err != nil { + return nil, err + } + return h.BundleToHelmChartFn(bundleFS, ext.Spec.Namespace, watchNamespace) +} + func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) { currentRelease, err := cl.Get(ext.GetName()) if errors.Is(err, driver.ErrReleaseNotFound) { diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index b170d8a98..21fbdb4a4 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -3,6 +3,7 @@ package applier_test import ( "context" "errors" + "io/fs" "os" "testing" "testing/fstest" @@ -13,6 +14,8 @@ import ( "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" featuregatetesting "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,6 +24,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/features" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" ) type mockPreflight struct { @@ -106,6 +110,10 @@ metadata: olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' spec: installModes: + - type: SingleNamespace + supported: true + - type: OwnNamespace + supported: true - type: AllNamespaces supported: true`)}, } @@ -144,7 +152,10 @@ func TestApply_Base(t *testing.T) { t.Run("fails trying to obtain an action client", func(t *testing.T) { mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")} - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -155,7 +166,10 @@ func TestApply_Base(t *testing.T) { t.Run("fails getting current release and !driver.ErrReleaseNotFound", func(t *testing.T) { mockAcg := &mockActionGetter{getClientErr: errors.New("failed getting current release")} - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -171,7 +185,10 @@ func TestApply_Installation(t *testing.T) { getClientErr: driver.ErrReleaseNotFound, dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -186,7 +203,11 @@ func TestApply_Installation(t *testing.T) { installErr: errors.New("failed installing chart"), } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} - helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -200,7 +221,10 @@ func TestApply_Installation(t *testing.T) { getClientErr: driver.ErrReleaseNotFound, installErr: errors.New("failed installing chart"), } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -217,7 +241,10 @@ func TestApply_Installation(t *testing.T) { Manifest: validManifest, }, } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.NoError(t, err) @@ -236,7 +263,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { getClientErr: driver.ErrReleaseNotFound, dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -251,7 +281,11 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { installErr: errors.New("failed installing chart"), } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} - helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -265,7 +299,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { getClientErr: driver.ErrReleaseNotFound, installErr: errors.New("failed installing chart"), } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -282,7 +319,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { Manifest: validManifest, }, } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.NoError(t, err) @@ -302,7 +342,10 @@ func TestApply_Upgrade(t *testing.T) { mockAcg := &mockActionGetter{ dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"), } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -321,7 +364,11 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")} - helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -340,7 +387,10 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } mockPf := &mockPreflight{} - helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -359,7 +409,11 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } mockPf := &mockPreflight{} - helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.Error(t, err) @@ -376,7 +430,10 @@ func TestApply_Upgrade(t *testing.T) { currentRel: testCurrentRelease, desiredRel: &testDesiredRelease, } - helmApplier := applier.Helm{ActionClientGetter: mockAcg} + helmApplier := applier.Helm{ + ActionClientGetter: mockAcg, + BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) require.NoError(t, err) @@ -386,3 +443,76 @@ func TestApply_Upgrade(t *testing.T) { assert.Equal(t, "service-b", objs[1].GetName()) }) } + +func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) + + t.Run("generates bundle resources in SingleNamespace install mode when watch namespace is configured", func(t *testing.T) { + var expectedWatchNamespace = "watch-namespace" + + helmApplier := applier.Helm{ + ActionClientGetter: &mockActionGetter{ + getClientErr: driver.ErrReleaseNotFound, + desiredRel: &release.Release{ + Info: &release.Info{Status: release.StatusDeployed}, + Manifest: validManifest, + }, + }, + BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + require.Equal(t, expectedWatchNamespace, watchNamespace) + return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace) + }, + } + + testExt := &v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testExt", + Annotations: map[string]string{ + applier.AnnotationClusterExtensionWatchNamespace: expectedWatchNamespace, + }, + }, + } + + _, _, _ = helmApplier.Apply(context.TODO(), validFS, testExt, testObjectLabels, testStorageLabels) + }) +} + +func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { + t.Run("generates bundle resources in AllNamespaces install mode", func(t *testing.T) { + var expectedWatchNamespace = corev1.NamespaceAll + + helmApplier := applier.Helm{ + ActionClientGetter: &mockActionGetter{ + getClientErr: driver.ErrReleaseNotFound, + desiredRel: &release.Release{ + Info: &release.Info{Status: release.StatusDeployed}, + Manifest: validManifest, + }, + }, + BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + require.Equal(t, expectedWatchNamespace, watchNamespace) + return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace) + }, + } + + _, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) + }) + + t.Run("surfaces chart generation errors", func(t *testing.T) { + helmApplier := applier.Helm{ + ActionClientGetter: &mockActionGetter{ + getClientErr: driver.ErrReleaseNotFound, + desiredRel: &release.Release{ + Info: &release.Info{Status: release.StatusDeployed}, + Manifest: validManifest, + }, + }, + BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return nil, errors.New("some error") + }, + } + + _, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) + require.Error(t, err) + }) +} diff --git a/internal/operator-controller/applier/watchnamespace_test.go b/internal/operator-controller/applier/watchnamespace_test.go index 86a822201..6f6e341e8 100644 --- a/internal/operator-controller/applier/watchnamespace_test.go +++ b/internal/operator-controller/applier/watchnamespace_test.go @@ -1,7 +1,6 @@ package applier_test import ( - "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "testing" "github.com/stretchr/testify/require" @@ -10,6 +9,7 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" v1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/features" ) diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index b08c4a988..155b86be8 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -1,7 +1,6 @@ package convert import ( - "context" "crypto/sha256" "encoding/json" "errors" @@ -20,7 +19,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/resource" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/yaml" "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -41,13 +39,13 @@ type Plain struct { Objects []client.Object } -func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace string, watchNamespaces []string) (*chart.Chart, error) { - reg, err := ParseFS(ctx, rv1) +func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + reg, err := ParseFS(rv1) if err != nil { return nil, err } - plain, err := Convert(reg, installNamespace, watchNamespaces) + plain, err := Convert(reg, installNamespace, []string{watchNamespace}) if err != nil { return nil, err } @@ -77,9 +75,7 @@ func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace stri // - ... // // manifests directory does not contain subdirectories -func ParseFS(ctx context.Context, rv1 fs.FS) (RegistryV1, error) { - l := log.FromContext(ctx) - +func ParseFS(rv1 fs.FS) (RegistryV1, error) { reg := RegistryV1{} annotationsFileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml")) if err != nil { @@ -107,11 +103,7 @@ func ParseFS(ctx context.Context, rv1 fs.FS) (RegistryV1, error) { if err != nil { return err } - defer func() { - if err := manifestFile.Close(); err != nil { - l.Error(err, "error closing file", "path", path) - } - }() + defer manifestFile.Close() result := resource.NewLocalBuilder().Unstructured().Flatten().Stream(manifestFile, path).Do() if err := result.Err(); err != nil { diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index 42786fada..a79e4624f 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -1,7 +1,6 @@ package convert_test import ( - "context" "fmt" "io/fs" "os" @@ -464,7 +463,8 @@ func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) { t.Log("It should read the registry+v1 bundle filesystem correctly") t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata") fsys := os.DirFS("testdata/combine-properties-bundle") - chrt, err := convert.RegistryV1ToHelmChart(context.Background(), fsys, "", nil) + + chrt, err := convert.RegistryV1ToHelmChart(fsys, "", "") require.NoError(t, err) require.NotNil(t, chrt) require.NotNil(t, chrt.Metadata) @@ -492,7 +492,7 @@ func TestParseFSFails(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - _, err := convert.ParseFS(context.Background(), tt.FS) + _, err := convert.ParseFS(tt.FS) require.Error(t, err) }) } @@ -506,7 +506,8 @@ func TestRegistryV1SuiteReadBundleFileSystemFailsOnNoCSV(t *testing.T) { t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata") fsys := os.DirFS("testdata/combine-properties-bundle") - chrt, err := convert.RegistryV1ToHelmChart(context.Background(), fsys, "", nil) + chrt, err := convert.RegistryV1ToHelmChart(fsys, "", "") + require.NoError(t, err) require.NotNil(t, chrt) require.NotNil(t, chrt.Metadata) From 9190b9d6fe66d75669c1e3f764470ae47399e2f3 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 13 Feb 2025 10:09:11 +0100 Subject: [PATCH 4/7] Add resource dir env var to demo generation script Signed-off-by: Per Goncalves da Silva --- hack/demo/generate-asciidemo.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/demo/generate-asciidemo.sh b/hack/demo/generate-asciidemo.sh index 2897719fb..737c23088 100755 --- a/hack/demo/generate-asciidemo.sh +++ b/hack/demo/generate-asciidemo.sh @@ -3,6 +3,7 @@ trap cleanup SIGINT SIGTERM EXIT SCRIPTPATH="$( cd -- "$(dirname "$0")" > /dev/null 2>&1 ; pwd -P )" +export DEMO_RESOURCE_DIR="${SCRIPTPATH}/resources" check_prereq() { prog=$1 @@ -80,7 +81,6 @@ for prereq in "asciinema curl"; do check_prereq ${prereq} done - curl https://raw.githubusercontent.com/zechris/asciinema-rec_script/main/bin/asciinema-rec_script -o ${WKDIR}/asciinema-rec_script chmod +x ${WKDIR}/asciinema-rec_script screencast=${WKDIR}/${DEMO_NAME}.cast ${WKDIR}/asciinema-rec_script ${SCRIPTPATH}/${DEMO_SCRIPT} From 0e8f7a2db6da235140aad77a05dd248b0eb5beb2 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 12 Feb 2025 12:07:21 +0100 Subject: [PATCH 5/7] Add feature demo Signed-off-by: Per Goncalves da Silva --- hack/demo/own-namespace-demo.sh | 43 +++++++++++++++++ hack/demo/resources/own-namespace-demo.yaml | 16 +++++++ .../demo/resources/single-namespace-demo.yaml | 16 +++++++ hack/demo/single-own-namespace.sh | 46 +++++++++++++++++++ 4 files changed, 121 insertions(+) create mode 100755 hack/demo/own-namespace-demo.sh create mode 100644 hack/demo/resources/own-namespace-demo.yaml create mode 100644 hack/demo/resources/single-namespace-demo.yaml create mode 100755 hack/demo/single-own-namespace.sh diff --git a/hack/demo/own-namespace-demo.sh b/hack/demo/own-namespace-demo.sh new file mode 100755 index 000000000..6b867fbad --- /dev/null +++ b/hack/demo/own-namespace-demo.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash + +# +# Welcome to the OwnNamespace install mode demo +# +trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT + +# enable 'SingleOwnNamespaceInstallSupport' feature gate +kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' + +# wait for operator-controller to become available +kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager + +# create install namespace +kubectl create ns argocd-system + +# create installer service account +kubectl create serviceaccount -n argocd-system argocd-installer + +# give installer service account admin privileges (not for production environments) +kubectl create clusterrolebinding argocd-installer-crb --clusterrole=cluster-admin --serviceaccount=argocd-system:argocd-installer + +# install cluster extension in own namespace install mode (watch-namespace == install namespace == argocd-system) +cat ${DEMO_RESOURCE_DIR}/own-namespace-demo.yaml + +# apply cluster extension +kubectl apply -f ${DEMO_RESOURCE_DIR}/own-namespace-demo.yaml + +# wait for cluster extension installation to succeed +kubectl wait --for=condition=Installed clusterextension/argocd-operator --timeout="60s" + +# check argocd-operator controller deployment pod template olm.targetNamespaces annotation +kubectl get deployments -n argocd-system argocd-operator-controller-manager -o jsonpath="{.spec.template.metadata.annotations.olm\.targetNamespaces}" + +# check for argocd-operator rbac in watch namespace +kubectl get roles,rolebindings -n argocd-system -o name + +# get controller service-account name +kubectl get deployments -n argocd-system argocd-operator-controller-manager -o jsonpath="{.spec.template.spec.serviceAccount}" + +# check service account for role binding is the same as controller service-account +rolebinding=$(kubectl get rolebindings -n argocd-system -o name | grep 'argocd-operator' | head -n 1) +kubectl get -n argocd-system $rolebinding -o jsonpath='{.subjects}' | jq .[0] diff --git a/hack/demo/resources/own-namespace-demo.yaml b/hack/demo/resources/own-namespace-demo.yaml new file mode 100644 index 000000000..f1d6da927 --- /dev/null +++ b/hack/demo/resources/own-namespace-demo.yaml @@ -0,0 +1,16 @@ +apiVersion: olm.operatorframework.io/v1 +kind: ClusterExtension +metadata: + name: argocd-operator + annotations: + # watch namespace is the same as intall namespace + olm.operatorframework.io/watch-namespace: argocd-system +spec: + namespace: argocd-system + serviceAccount: + name: argocd-installer + source: + sourceType: Catalog + catalog: + packageName: argocd-operator + version: 0.6.0 diff --git a/hack/demo/resources/single-namespace-demo.yaml b/hack/demo/resources/single-namespace-demo.yaml new file mode 100644 index 000000000..2403bc6a8 --- /dev/null +++ b/hack/demo/resources/single-namespace-demo.yaml @@ -0,0 +1,16 @@ +apiVersion: olm.operatorframework.io/v1 +kind: ClusterExtension +metadata: + name: argocd-operator + annotations: + # watch-namespace is different from install namespace + olm.operatorframework.io/watch-namespace: argocd +spec: + namespace: argocd-system + serviceAccount: + name: argocd-installer + source: + sourceType: Catalog + catalog: + packageName: argocd-operator + version: 0.6.0 diff --git a/hack/demo/single-own-namespace.sh b/hack/demo/single-own-namespace.sh new file mode 100755 index 000000000..4e243f9df --- /dev/null +++ b/hack/demo/single-own-namespace.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash + +# +# Welcome to the SingleNamespace install mode demo +# +trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT + +# enable 'SingleOwnNamespaceInstallSupport' feature gate +kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' + +# wait for operator-controller to become available +kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager + +# create install namespace +kubectl create ns argocd-system + +# create installer service account +kubectl create serviceaccount -n argocd-system argocd-installer + +# give installer service account admin privileges (not for production environments) +kubectl create clusterrolebinding argocd-installer-crb --clusterrole=cluster-admin --serviceaccount=argocd-system:argocd-installer + +# create watch namespace +kubectl create namespace argocd + +# install cluster extension in single namespace install mode (watch namespace != install namespace) +cat ${DEMO_RESOURCE_DIR}/single-namespace-demo.yaml + +# apply cluster extension +kubectl apply -f ${DEMO_RESOURCE_DIR}/single-namespace-demo.yaml + +# wait for cluster extension installation to succeed +kubectl wait --for=condition=Installed clusterextension/argocd-operator --timeout="60s" + +# check argocd-operator controller deployment pod template olm.targetNamespaces annotation +kubectl get deployments -n argocd-system argocd-operator-controller-manager -o jsonpath="{.spec.template.metadata.annotations.olm\.targetNamespaces}" + +# check for argocd-operator rbac in watch namespace +kubectl get roles,rolebindings -n argocd -o name + +# get controller service-account name +kubectl get deployments -n argocd-system argocd-operator-controller-manager -o jsonpath="{.spec.template.spec.serviceAccount}" + +# check service account for role binding is the controller deployment service account +rolebinding=$(kubectl get rolebindings -n argocd -o name | grep 'argocd-operator' | head -n 1) +kubectl get -n argocd $rolebinding -o jsonpath='{.subjects}' | jq .[0] From b421152e1fae2185fc232ed280572f524a2c25c4 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Sat, 22 Feb 2025 14:13:31 +0100 Subject: [PATCH 6/7] Add docs Signed-off-by: Per Goncalves da Silva --- .../howto/single-ownnamespace-install.md | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 docs/draft/howto/single-ownnamespace-install.md diff --git a/docs/draft/howto/single-ownnamespace-install.md b/docs/draft/howto/single-ownnamespace-install.md new file mode 100644 index 000000000..2f440f32d --- /dev/null +++ b/docs/draft/howto/single-ownnamespace-install.md @@ -0,0 +1,105 @@ +## Description + +!!! note +This feature is still in *alpha* the `SingleOwnNamespaceInstallSupport` feature-gate must be enabled to make use of it. +See the instructions below on how to enable it. + +--- + +A component of OLMv0's multi-tenancy feature is its support of four [*installModes*](https://olm.operatorframework.io/docs/advanced-tasks/operator-scoping-with-operatorgroups/#targetnamespaces-and-their-relationship-to-installmodes): +for operator installation: + + - *OwnNamespace*: If supported, the operator can be configured to watch for events in the namespace it is deployed in. + - *SingleNamespace*: If supported, the operator can be configured to watch for events in a single namespace that the operator is not deployed in. + - *MultiNamespace*: If supported, the operator can be configured to watch for events in more than one namespace. + - *AllNamespaces*: If supported, the operator can be configured to watch for events in all namespaces. + +OLMv1 will not attempt multi-tenancy (see [design decisions document](../../project/olmv1_design_decisions.md)) and will think of operators +as globally installed, i.e. in OLMv0 parlance, as installed in *AllNamespaces* mode. However, there are operators that +were intended only for the *SingleNamespace* and *OwnNamespace* install modes. In order to make these operators installable in v1 while they +transition to the new model, v1 is adding support for these two new *installModes*. It should be noted that, in line with v1's no multi-tenancy policy, +users will not be able to install the same operator multiple times, and that in future iterations of the registry bundle format will not +include *installModes*. + +## Demos + +### SingleNamespace Install + +[![SingleNamespace Install Demo](https://asciinema.org/a/w1IW0xWi1S9cKQFb9jnR07mgh.svg)](https://asciinema.org/a/w1IW0xWi1S9cKQFb9jnR07mgh) + +### OwnNamespace Install + +[![OwnNamespace Install Demo](https://asciinema.org/a/Rxx6WUwAU016bXFDW74XLcM5i.svg)](https://asciinema.org/a/Rxx6WUwAU016bXFDW74XLcM5i) + +## Enabling the Feature-Gate + +!!! tip + +This guide assumes OLMv1 is already installed. If that is not the case, +you can follow the [getting started](../../getting-started/olmv1_getting_started.md) guide to install OLMv1. + +--- + +Patch the `operator-controller` `Deployment` adding `--feature-gates=SingleOwnNamespaceInstallSupport=true` to the +controller container arguments: + +```terminal title="Enable SingleOwnNamespaceInstallSupport feature-gate" +kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' +``` + +Wait for `Deployment` rollout: + +```terminal title="Wait for Deployment rollout" +kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager +``` + +## Configuring the `ClusterExtension` + +A `ClusterExtension` can be configured to install bundle in `Single-` or `OwnNamespace` mode through the +`olm.operatorframework.io/watch-namespace: ` annotation. The *installMode* is inferred in the following way: + + - *AllNamespaces*: `` is empty, or the annotation is not present + - *OwnNamespace*: `` is the install namespace (i.e. `.spec.namespace`) + - *SingleNamespace*: `` not the install namespace + +### Examples + +``` terminal title="SingleNamespace install mode example" +kubectl apply -f - < Date: Sat, 22 Feb 2025 14:13:52 +0100 Subject: [PATCH 7/7] Fixup tests Signed-off-by: Per Goncalves da Silva --- .../operator-controller/applier/helm_test.go | 4 +- .../applier/watchnamespace_test.go | 15 ++ .../rukpak/convert/registryv1_test.go | 167 ++++++++++-------- 3 files changed, 111 insertions(+), 75 deletions(-) diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 21fbdb4a4..faaa41783 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -447,7 +447,7 @@ func TestApply_Upgrade(t *testing.T) { func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) - t.Run("generates bundle resources in SingleNamespace install mode when watch namespace is configured", func(t *testing.T) { + t.Run("generates bundle resources using the configured watch namespace", func(t *testing.T) { var expectedWatchNamespace = "watch-namespace" helmApplier := applier.Helm{ @@ -464,7 +464,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin }, } - testExt := &v1.ClusterExtension{ + testExt := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Name: "testExt", Annotations: map[string]string{ diff --git a/internal/operator-controller/applier/watchnamespace_test.go b/internal/operator-controller/applier/watchnamespace_test.go index 6f6e341e8..90e018dc7 100644 --- a/internal/operator-controller/applier/watchnamespace_test.go +++ b/internal/operator-controller/applier/watchnamespace_test.go @@ -13,6 +13,21 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/features" ) +func TestGetWatchNamespacesWhenFeatureGateIsDisabled(t *testing.T) { + watchNamespace, err := applier.GetWatchNamespace(&v1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "extension", + Annotations: map[string]string{ + "olm.operatorframework.io/watch-namespace": "watch-namespace", + }, + }, + Spec: v1.ClusterExtensionSpec{}, + }) + require.NoError(t, err) + t.Log("Check watchNamespace is '' even if the annotation is set") + require.Equal(t, corev1.NamespaceAll, watchNamespace) +} + func TestGetWatchNamespace(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index a79e4624f..20ea7fc88 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -379,81 +379,102 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) { require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) } -func TestRegistryV1SuiteGenerateErrorMultiNamespaceEmpty(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should error when multinamespace mode is supported with an empty string in target namespaces") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{"testWatchNs1", ""} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.Error(t, err) - require.Nil(t, plainBundle) -} - -func TestRegistryV1SuiteGenerateErrorSingleNamespaceDisabled(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should error when single namespace mode is disabled with more than one target namespaces") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{"testWatchNs1", "testWatchNs2"} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.Error(t, err) - require.Nil(t, plainBundle) -} - -func TestRegistryV1SuiteGenerateErrorAllNamespaceDisabled(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should error when all namespace mode is disabled with target namespace containing an empty string") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - } +func TestConvertInstallModeValidation(t *testing.T) { + for _, tc := range []struct { + description string + installModes []v1alpha1.InstallMode + installNamespace string + watchNamespaces []string + }{ + { + description: "fails on AllNamespaces install mode when CSV does not support it", + installNamespace: "install-namespace", + watchNamespaces: []string{corev1.NamespaceAll}, + installModes: []v1alpha1.InstallMode{ + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, + }, + }, { + description: "fails on SingleNamespace install mode when CSV does not support it", + installNamespace: "install-namespace", + watchNamespaces: []string{"watch-namespace"}, + installModes: []v1alpha1.InstallMode{ + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, + }, + }, { + description: "fails on OwnNamespace install mode when CSV does not support it and watch namespace is not install namespace", + installNamespace: "install-namespace", + watchNamespaces: []string{"watch-namespace"}, + installModes: []v1alpha1.InstallMode{ + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, + }, + }, { + description: "fails on MultiNamespace install mode when CSV does not support it", + installNamespace: "install-namespace", + watchNamespaces: []string{"watch-namespace-one", "watch-namespace-two", "watch-namespace-three"}, + installModes: []v1alpha1.InstallMode{ + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: false}, + }, + }, { + description: "fails on MultiNamespace install mode when CSV supports it but watchNamespaces is empty", + installNamespace: "install-namespace", + watchNamespaces: []string{}, + installModes: []v1alpha1.InstallMode{ + // because install mode is inferred by the watchNamespaces parameter + // force MultiNamespace install by disabling other modes + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, + }, + }, { + description: "fails on MultiNamespace install mode when CSV supports it but watchNamespaces is nil", + installNamespace: "install-namespace", + watchNamespaces: nil, + installModes: []v1alpha1.InstallMode{ + // because install mode is inferred by the watchNamespaces parameter + // force MultiNamespace install by disabling other modes + {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, + {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, + {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, + }, + }, + } { + t.Run(tc.description, func(t *testing.T) { + t.Log("RegistryV1 Suite Convert") + t.Log("It should generate objects successfully based on target namespaces") + + t.Log("It should error when all namespace mode is disabled with target namespace containing an empty string") + baseCSV, svc := getBaseCsvAndService() + csv := baseCSV.DeepCopy() + csv.Spec.InstallModes = tc.installModes + + t.Log("By creating a registry v1 bundle") + unstructuredSvc := convertToUnstructured(t, svc) + registryv1Bundle := convert.RegistryV1{ + PackageName: "testPkg", + CSV: *csv, + Others: []unstructured.Unstructured{unstructuredSvc}, + } - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{""} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, + t.Log("By converting to plain") + plainBundle, err := convert.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces) + require.Error(t, err) + require.Nil(t, plainBundle) + }) } - - t.Log("By converting to plain") - plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.Error(t, err) - require.Nil(t, plainBundle) } func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) {