[Add] Support for other Install strategies

This PR removes the requirement that AllNamespace mode should be supported always in a registry+v1 format. It does so by: 1. Adding a field to specify watch namespaces in BundleDeployment Spec. 2. While converting from registryV1 to plain bundle, we now generate roles for all specified targetNamespaces which are being watched. Signed-off-by: Varsha Prasad Narsing <[email protected]>
operator-framework · Jan 19, 2024 · 1018779 · 1018779
1 parent de8afc9
commit 1018779
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 22 deletions.
diff --git a/api/v1alpha2/bundledeployment_types.go b/api/v1alpha2/bundledeployment_types.go
@@ -47,6 +47,9 @@ const (
 	ReasonUpgradeFailed             = "UpgradeFailed"
 )
 
+// Add limit to the number of watchNamespaces allowed, as the estimated cost of this rule is linear per BD.
+//+kubebuilder:validation:XValidation:rule="!has(self.watchNamespaces) || size(self.watchNamespaces) <= 1 || (size(self.watchNamespaces) > 1 && !self.watchNamespaces.exists(e, e == ''))",message="Empty string not accepted if length of watchNamespaces is more than 1."
+
 // BundleDeploymentSpec defines the desired state of BundleDeployment
 type BundleDeploymentSpec struct {
 	//+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
@@ -57,6 +60,8 @@ type BundleDeploymentSpec struct {
 	// Config is provisioner specific configurations
 	// +kubebuilder:pruning:PreserveUnknownFields
 	Config runtime.RawExtension `json:"config,omitempty"`
+	// watchNamespaces indicates which namespaces the operator should watch.
+	WatchNamespaces []string `json:"watchNamespaces,omitempty"`
 }
 
 // BundleDeploymentStatus defines the observed state of BundleDeployment

diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go
diff --git a/internal/convert/registryv1.go b/internal/convert/registryv1.go
@@ -40,7 +40,7 @@ type Plain struct {
 	Objects []client.Object
 }
 
-func RegistryV1ToPlain(rv1 fs.FS) (fs.FS, error) {
+func RegistryV1ToPlain(rv1 fs.FS, watchNamespaces []string) (fs.FS, error) {
 	reg := RegistryV1{}
 	fileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml"))
 	if err != nil {
@@ -102,7 +102,7 @@ func RegistryV1ToPlain(rv1 fs.FS) (fs.FS, error) {
 		}
 	}
 
-	plain, err := Simple(reg)
+	plain, err := Convert(reg, "", watchNamespaces)
 	if err != nil {
 		return nil, err
 	}
@@ -158,17 +158,13 @@ func validateTargetNamespaces(supportedInstallModes sets.Set[string], installNam
 			return nil
 		}
 	default:
-		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) {
+		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") {
 			return nil
 		}
 	}
 	return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces)
 }
 
-func Simple(in RegistryV1) (*Plain, error) {
-	return Convert(in, "", nil)
-}
-
 func saNameOrDefault(saName string) string {
 	if saName == "" {
 		return "default"
@@ -189,10 +185,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 			supportedInstallModes.Insert(string(im.Type))
 		}
 	}
-	if !supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) {
-		return nil, fmt.Errorf("AllNamespace install mode must be enabled")
-	}
-	if targetNamespaces == nil {
+	if len(targetNamespaces) == 0 {
 		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) {
 			targetNamespaces = []string{""}
 		} else if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) {
@@ -274,15 +267,18 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 		permissions = nil
 	}
 
-	for _, permission := range permissions {
-		saName := saNameOrDefault(permission.ServiceAccountName)
-		name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
-		if err != nil {
-			return nil, err
+	for _, ns := range targetNamespaces {
+		for _, permission := range permissions {
+			saName := saNameOrDefault(permission.ServiceAccountName)
+			name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
+			if err != nil {
+				return nil, err
+			}
+			roles = append(roles, newRole(ns, name, permission.Rules))
+			roleBindings = append(roleBindings, newRoleBinding(ns, name, name, installNamespace, saName))
 		}
-		roles = append(roles, newRole(installNamespace, name, permission.Rules))
-		roleBindings = append(roleBindings, newRoleBinding(installNamespace, name, name, installNamespace, saName))
 	}
+
 	for _, permission := range clusterPermissions {
 		saName := saNameOrDefault(permission.ServiceAccountName)
 		name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)

diff --git a/internal/convert/registryv1_test.go b/internal/convert/registryv1_test.go
@@ -7,6 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -159,6 +160,133 @@ var _ = Describe("RegistryV1 Suite", func() {
 			})
 		})
 
+		Context("Should generate objects successfully based on target namespaces", func() {
+			var (
+				svc             corev1.Service
+				csv             v1alpha1.ClusterServiceVersion
+				watchNamespaces []string
+			)
+
+			BeforeEach(func() {
+				csv = v1alpha1.ClusterServiceVersion{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testCSV",
+					},
+					Spec: v1alpha1.ClusterServiceVersionSpec{
+						InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}},
+						InstallStrategy: v1alpha1.NamedInstallStrategy{
+							StrategySpec: v1alpha1.StrategyDetailsDeployment{
+								Permissions: []v1alpha1.StrategyDeploymentPermissions{
+									{
+										ServiceAccountName: "testServiceAccount",
+										Rules: []rbacv1.PolicyRule{
+											{
+												APIGroups: []string{"test"},
+												Resources: []string{"pods"},
+												Verbs:     []string{"*"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+				svc = corev1.Service{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testService",
+					},
+				}
+				svc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"})
+				installNamespace = "testInstallNamespace"
+			})
+
+			It("should convert into plain manifests successfully", func() {
+				By("creating a registry v1 bundle")
+				watchNamespaces = []string{"testWatchNs1", "testWatchNs2"}
+				unstructuredSvc := convertToUnstructured(svc)
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("verifying if plain bundle has required objects")
+				Expect(plainBundle).ShouldNot(BeNil())
+				Expect(len(plainBundle.Objects)).To(BeEquivalentTo(7))
+			})
+
+			It("should error when multinamespace mode is supported with an empty string in target namespaces", func() {
+				By("creating a registry v1 bundle")
+				watchNamespaces = []string{"testWatchNs1", ""}
+				unstructuredSvc := convertToUnstructured(svc)
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+				Expect(err).To(HaveOccurred())
+				Expect(plainBundle).To(BeNil())
+			})
+
+			It("should error when single namespace mode is disabled with more than one target namespaces", func() {
+				csv = v1alpha1.ClusterServiceVersion{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testCSV",
+					},
+					Spec: v1alpha1.ClusterServiceVersionSpec{
+						InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}},
+					},
+				}
+
+				By("creating a registry v1 bundle")
+				watchNamespaces = []string{"testWatchNs1", "testWatchNs2"}
+				unstructuredSvc := convertToUnstructured(svc)
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+				Expect(err).To(HaveOccurred())
+				Expect(plainBundle).To(BeNil())
+			})
+
+			It("should error when all namespace mode is disabled with target namespace containing an empty string", func() {
+				csv = v1alpha1.ClusterServiceVersion{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testCSV",
+					},
+					Spec: v1alpha1.ClusterServiceVersionSpec{
+						InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}},
+					},
+				}
+
+				By("creating a registry v1 bundle")
+				watchNamespaces = []string{""}
+				unstructuredSvc := convertToUnstructured(svc)
+				registryv1Bundle = RegistryV1{
+					PackageName: "testPkg",
+					CSV:         csv,
+					Others:      []unstructured.Unstructured{unstructuredSvc},
+				}
+
+				By("converting to plain")
+				plainBundle, err := Convert(registryv1Bundle, installNamespace, watchNamespaces)
+				Expect(err).To(HaveOccurred())
+				Expect(plainBundle).To(BeNil())
+			})
+		})
+
 	})
 })
 

diff --git a/internal/provisioner/registry/registry.go b/internal/provisioner/registry/registry.go
@@ -19,7 +19,7 @@ const (
 )
 
 func HandleBundleDeployment(ctx context.Context, fsys fs.FS, bd *rukpakv1alpha2.BundleDeployment) (*chart.Chart, chartutil.Values, error) {
-	plainFS, err := convert.RegistryV1ToPlain(fsys)
+	plainFS, err := convert.RegistryV1ToPlain(fsys, bd.Spec.WatchNamespaces)
 	if err != nil {
 		return nil, nil, fmt.Errorf("convert registry+v1 bundle to plain+v0 bundle: %v", err)
 	}

diff --git a/manifests/base/apis/crds/core.rukpak.io_bundledeployments.yaml b/manifests/base/apis/crds/core.rukpak.io_bundledeployments.yaml
@@ -220,10 +220,22 @@ spec:
                 required:
                 - type
                 type: object
+              watchNamespaces:
+                description: watchNamespaces indicates which namespaces the operator
+                  should watch.
+                items:
+                  type: string
+                type: array
             required:
             - provisionerClassName
             - source
             type: object
+            x-kubernetes-validations:
+            - message: Empty string not accepted if length of watchNamespaces is more
+                than 1.
+              rule: '!has(self.watchNamespaces) || size(self.watchNamespaces) <= 1
+                || (size(self.watchNamespaces) > 1 && !self.watchNamespaces.exists(e,
+                e == ''''))'
           status:
             description: BundleDeploymentStatus defines the observed state of BundleDeployment
             properties:

diff --git a/test/e2e/registry_provisioner_test.go b/test/e2e/registry_provisioner_test.go
@@ -87,6 +87,7 @@ var _ = Describe("registry provisioner bundle", func() {
 							Ref: fmt.Sprintf("%v/%v", ImageRepo, "registry:invalid"),
 						},
 					},
+					WatchNamespaces: []string{"test1"},
 				},
 			}
 			err := c.Create(ctx, bd)
@@ -108,7 +109,7 @@ var _ = Describe("registry provisioner bundle", func() {
 				WithTransform(func(c *metav1.Condition) string { return c.Type }, Equal(rukpakv1alpha2.TypeInstalled)),
 				WithTransform(func(c *metav1.Condition) metav1.ConditionStatus { return c.Status }, Equal(metav1.ConditionFalse)),
 				WithTransform(func(c *metav1.Condition) string { return c.Reason }, Equal(rukpakv1alpha2.ReasonInstallFailed)),
-				WithTransform(func(c *metav1.Condition) string { return c.Message }, ContainSubstring("convert registry+v1 bundle to plain+v0 bundle: AllNamespace install mode must be enabled")),
+				WithTransform(func(c *metav1.Condition) string { return c.Message }, ContainSubstring("convert registry+v1 bundle to plain+v0 bundle:")),
 			))
 		})
 	})

diff --git a/...ata/bundles/registry/invalid/manifests/update-service-operator.clusterserviceversion.yaml b/...ata/bundles/registry/invalid/manifests/update-service-operator.clusterserviceversion.yaml
@@ -9,11 +9,11 @@ metadata:
   namespace: placeholder
 spec:
   installModes:
-  - supported: true
+  - supported: false
     type: OwnNamespace
   - supported: false
     type: SingleNamespace
-  - supported: false
+  - supported: true
     type: MultiNamespace
   - supported: false
     type: AllNamespaces