diff --git a/api/v1alpha2/bundledeployment_types.go b/api/v1alpha2/bundledeployment_types.go index eb67eb9a..2e44be59 100644 --- 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 index 7c12c15d..26ffb4b2 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -105,6 +105,11 @@ func (in *BundleDeploymentSpec) DeepCopyInto(out *BundleDeploymentSpec) { *out = *in in.Source.DeepCopyInto(&out.Source) in.Config.DeepCopyInto(&out.Config) + if in.WatchNamespaces != nil { + in, out := &in.WatchNamespaces, &out.WatchNamespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleDeploymentSpec. diff --git a/internal/convert/registryv1.go b/internal/convert/registryv1.go index c9ef7095..53864eb7 100644 --- 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 index e0db55b8..30b45ea9 100644 --- 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 index 8652c140..2dcd39a0 100644 --- 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 index e056dd52..e6003bdc 100644 --- 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 index f43f694a..29250c1b 100644 --- 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/testdata/bundles/registry/invalid/manifests/update-service-operator.clusterserviceversion.yaml b/testdata/bundles/registry/invalid/manifests/update-service-operator.clusterserviceversion.yaml index 4de987f5..4a7dee3a 100644 --- a/testdata/bundles/registry/invalid/manifests/update-service-operator.clusterserviceversion.yaml +++ b/testdata/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