diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index cf12ebe42..68244d313 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -7,37 +7,25 @@ import ( "fmt" "io/fs" "path/filepath" - "strings" "helm.sh/helm/v3/pkg/chart" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/resource" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/alpha/property" - registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" registry "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/operator-registry" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" ) -type RegistryV1 struct { - PackageName string - CSV v1alpha1.ClusterServiceVersion - CRDs []apiextensionsv1.CustomResourceDefinition - Others []unstructured.Unstructured -} - type Plain struct { Objects []client.Object } @@ -70,7 +58,7 @@ func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace st return chrt, nil } -// ParseFS converts the rv1 filesystem into a RegistryV1. +// ParseFS converts the rv1 filesystem into a render.RegistryV1. // ParseFS expects the filesystem to conform to the registry+v1 format: // metadata/annotations.yaml // manifests/ @@ -78,8 +66,8 @@ func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace st // - ... // // manifests directory does not contain subdirectories -func ParseFS(rv1 fs.FS) (RegistryV1, error) { - reg := RegistryV1{} +func ParseFS(rv1 fs.FS) (render.RegistryV1, error) { + reg := render.RegistryV1{} annotationsFileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml")) if err != nil { return reg, err @@ -224,22 +212,23 @@ func validateTargetNamespaces(supportedInstallModes sets.Set[string], installNam return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces) } -func saNameOrDefault(saName string) string { - if saName == "" { - return "default" - } - return saName +var PlainConverter = Converter{ + BundleRenderer: render.BundleRenderer{ + BundleValidator: validators.RegistryV1BundleValidator, + ResourceGenerators: []render.ResourceGenerator{ + generators.BundleCSVRBACResourceGenerator.ResourceGenerator(), + generators.BundleCRDGenerator, + generators.BundleAdditionalResourcesGenerator, + generators.BundleCSVDeploymentGenerator, + }, + }, } type Converter struct { - BundleValidator BundleValidator + render.BundleRenderer } -func (c Converter) Convert(rv1 RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) { - if err := c.BundleValidator.Validate(&rv1); err != nil { - return nil, err - } - +func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) { if installNamespace == "" { installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"] } @@ -272,246 +261,9 @@ func (c Converter) Convert(rv1 RegistryV1, installNamespace string, targetNamesp return nil, fmt.Errorf("webhookDefinitions are not supported") } - deployments := []appsv1.Deployment{} - serviceAccounts := map[string]corev1.ServiceAccount{} - for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { - annotations := util.MergeMaps(rv1.CSV.Annotations, depSpec.Spec.Template.Annotations) - annotations["olm.targetNamespaces"] = strings.Join(targetNamespaces, ",") - depSpec.Spec.Template.Annotations = annotations - - // Hardcode the deployment with RevisionHistoryLimit=1 to replicate OLMv0 behavior - // https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/deployment.go#L181 - depSpec.Spec.RevisionHistoryLimit = ptr.To(int32(1)) - - deployments = append(deployments, appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "Deployment", - APIVersion: appsv1.SchemeGroupVersion.String(), - }, - - ObjectMeta: metav1.ObjectMeta{ - Namespace: installNamespace, - Name: depSpec.Name, - Labels: depSpec.Label, - }, - Spec: depSpec.Spec, - }) - saName := saNameOrDefault(depSpec.Spec.Template.Spec.ServiceAccountName) - serviceAccounts[saName] = newServiceAccount(installNamespace, saName) - } - - // NOTES: - // 1. There's an extra Role for OperatorConditions: get/update/patch; resourceName=csv.name - // - This is managed by the OperatorConditions controller here: https://github.com/operator-framework/operator-lifecycle-manager/blob/9ced412f3e263b8827680dc0ad3477327cd9a508/pkg/controller/operators/operatorcondition_controller.go#L106-L109 - // 2. There's an extra RoleBinding for the above mentioned role. - // - Every SA mentioned in the OperatorCondition.spec.serviceAccounts is a subject for this role binding: https://github.com/operator-framework/operator-lifecycle-manager/blob/9ced412f3e263b8827680dc0ad3477327cd9a508/pkg/controller/operators/operatorcondition_controller.go#L171-L177 - // 3. strategySpec.permissions are _also_ given a clusterrole/clusterrole binding. - // - (for AllNamespaces mode only?) - // - (where does the extra namespaces get/list/watch rule come from?) - - roles := []rbacv1.Role{} - roleBindings := []rbacv1.RoleBinding{} - clusterRoles := []rbacv1.ClusterRole{} - clusterRoleBindings := []rbacv1.ClusterRoleBinding{} - - permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions - clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions - allPermissions := append(permissions, clusterPermissions...) - - // Create all the service accounts - for _, permission := range allPermissions { - saName := saNameOrDefault(permission.ServiceAccountName) - if _, ok := serviceAccounts[saName]; !ok { - serviceAccounts[saName] = newServiceAccount(installNamespace, saName) - } - } - - // If we're in AllNamespaces mode, promote the permissions to clusterPermissions - if len(targetNamespaces) == 1 && targetNamespaces[0] == "" { - for _, p := range permissions { - p.Rules = append(p.Rules, rbacv1.PolicyRule{ - Verbs: []string{"get", "list", "watch"}, - APIGroups: []string{corev1.GroupName}, - Resources: []string{"namespaces"}, - }) - clusterPermissions = append(clusterPermissions, p) - } - permissions = nil - } - - for _, ns := range targetNamespaces { - for _, permission := range permissions { - saName := saNameOrDefault(permission.ServiceAccountName) - name, err := generateName(fmt.Sprintf("%s-%s", rv1.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)) - } - } - - for _, permission := range clusterPermissions { - saName := saNameOrDefault(permission.ServiceAccountName) - name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission) - if err != nil { - return nil, err - } - clusterRoles = append(clusterRoles, newClusterRole(name, permission.Rules)) - clusterRoleBindings = append(clusterRoleBindings, newClusterRoleBinding(name, name, installNamespace, saName)) - } - - objs := []client.Object{} - for _, obj := range serviceAccounts { - obj := obj - if obj.GetName() != "default" { - objs = append(objs, &obj) - } - } - for _, obj := range roles { - obj := obj - objs = append(objs, &obj) - } - for _, obj := range roleBindings { - obj := obj - objs = append(objs, &obj) - } - for _, obj := range clusterRoles { - obj := obj - objs = append(objs, &obj) - } - for _, obj := range clusterRoleBindings { - obj := obj - objs = append(objs, &obj) - } - for _, obj := range rv1.CRDs { - objs = append(objs, &obj) - } - for _, obj := range rv1.Others { - obj := obj - supported, namespaced := registrybundle.IsSupported(obj.GetKind()) - if !supported { - return nil, fmt.Errorf("bundle contains unsupported resource: Name: %v, Kind: %v", obj.GetName(), obj.GetKind()) - } - if namespaced { - obj.SetNamespace(installNamespace) - } - objs = append(objs, &obj) - } - for _, obj := range deployments { - obj := obj - objs = append(objs, &obj) - } - return &Plain{Objects: objs}, nil -} - -var PlainConverter = Converter{ - BundleValidator: RegistryV1BundleValidator, -} - -const maxNameLength = 63 - -func generateName(base string, o interface{}) (string, error) { - hashStr, err := util.DeepHashObject(o) + objs, err := c.BundleRenderer.Render(rv1, installNamespace, targetNamespaces) if err != nil { - return "", err - } - if len(base)+len(hashStr) > maxNameLength { - base = base[:maxNameLength-len(hashStr)-1] - } - - return fmt.Sprintf("%s-%s", base, hashStr), nil -} - -func newServiceAccount(namespace, name string) corev1.ServiceAccount { - return corev1.ServiceAccount{ - TypeMeta: metav1.TypeMeta{ - Kind: "ServiceAccount", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - } -} - -func newRole(namespace, name string, rules []rbacv1.PolicyRule) rbacv1.Role { - return rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Rules: rules, - } -} - -func newClusterRole(name string, rules []rbacv1.PolicyRule) rbacv1.ClusterRole { - return rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Rules: rules, - } -} - -func newRoleBinding(namespace, name, roleName, saNamespace string, saNames ...string) rbacv1.RoleBinding { - subjects := make([]rbacv1.Subject, 0, len(saNames)) - for _, saName := range saNames { - subjects = append(subjects, rbacv1.Subject{ - Kind: "ServiceAccount", - Namespace: saNamespace, - Name: saName, - }) - } - return rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Subjects: subjects, - RoleRef: rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "Role", - Name: roleName, - }, - } -} - -func newClusterRoleBinding(name, roleName, saNamespace string, saNames ...string) rbacv1.ClusterRoleBinding { - subjects := make([]rbacv1.Subject, 0, len(saNames)) - for _, saName := range saNames { - subjects = append(subjects, rbacv1.Subject{ - Kind: "ServiceAccount", - Namespace: saNamespace, - Name: saName, - }) - } - return rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Subjects: subjects, - RoleRef: rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "ClusterRole", - Name: roleName, - }, + return nil, err } + return &Plain{Objects: objs}, nil } diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index 8d6b90a29..dffb15cb4 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 ( - "errors" "fmt" "io/fs" "os" @@ -26,6 +25,8 @@ import ( "github.com/operator-framework/operator-registry/alpha/property" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" filterutil "github.com/operator-framework/operator-controller/internal/shared/util/filter" ) @@ -50,22 +51,8 @@ func getCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) { return csv, svc } -func TestConverterValidatesBundle(t *testing.T) { - converter := convert.Converter{ - BundleValidator: []func(rv1 *convert.RegistryV1) []error{ - func(rv1 *convert.RegistryV1) []error { - return []error{errors.New("test error")} - }, - }, - } - - _, err := converter.Convert(convert.RegistryV1{}, "installNamespace", []string{"watchNamespace"}) - require.Error(t, err) - require.Contains(t, err.Error(), "test error") -} - func TestPlainConverterUsedRegV1Validator(t *testing.T) { - require.Equal(t, convert.RegistryV1BundleValidator, convert.PlainConverter.BundleValidator) + require.Equal(t, validators.RegistryV1BundleValidator, convert.PlainConverter.BundleValidator) } func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) { @@ -79,7 +66,7 @@ func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) { csv, svc := getCsvAndService() unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -113,7 +100,7 @@ func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) { unstructuredSvc := convertToUnstructured(t, svc) unstructuredSvc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -154,7 +141,7 @@ func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) { unstructuredEvt := convertToUnstructured(t, event) unstructuredEvt.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Event"}) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, Others: []unstructured.Unstructured{unstructuredEvt}, @@ -188,7 +175,7 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) { unstructuredpriorityclass := convertToUnstructured(t, pc) unstructuredpriorityclass.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "PriorityClass"}) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, Others: []unstructured.Unstructured{unstructuredpriorityclass}, @@ -267,7 +254,7 @@ func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) { t.Log("By creating a registry v1 bundle") watchNamespaces := []string{""} unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: *csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -300,7 +287,7 @@ func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) { t.Log("By creating a registry v1 bundle") watchNamespaces := []string{"testWatchNs1", "testWatchNs2"} unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: *csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -333,7 +320,7 @@ func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) { t.Log("By creating a registry v1 bundle") watchNamespaces := []string{"testWatchNs1"} unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: *csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -366,7 +353,7 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) { t.Log("By creating a registry v1 bundle") watchNamespaces := []string{installNamespace} unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: *csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -471,7 +458,7 @@ func TestConvertInstallModeValidation(t *testing.T) { t.Log("By creating a registry v1 bundle") unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: *csv, Others: []unstructured.Unstructured{unstructuredSvc}, @@ -561,7 +548,7 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) { }, } watchNamespaces := []string{metav1.NamespaceAll} - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, } @@ -592,7 +579,7 @@ func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) { }, } watchNamespaces := []string{metav1.NamespaceAll} - registryv1Bundle := convert.RegistryV1{ + registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, } @@ -607,7 +594,7 @@ func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) { func Test_Convert_DeploymentResourceGeneration(t *testing.T) { for _, tc := range []struct { name string - bundle convert.RegistryV1 + bundle render.RegistryV1 installNamespace string targetNamespaces []string expectedResources []client.Object @@ -616,7 +603,7 @@ func Test_Convert_DeploymentResourceGeneration(t *testing.T) { name: "generates deployment resources", installNamespace: "install-namespace", targetNamespaces: []string{""}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithAnnotations(map[string]string{ @@ -702,7 +689,10 @@ func Test_Convert_DeploymentResourceGeneration(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) require.NoError(t, err) for _, expectedObj := range tc.expectedResources { @@ -720,14 +710,14 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { name string installNamespace string targetNamespaces []string - bundle convert.RegistryV1 + bundle render.RegistryV1 expectedResources []client.Object }{ { name: "does not generate any resources when in AllNamespaces mode (target namespace is [''])", installNamespace: "install-namespace", targetNamespaces: []string{metav1.NamespaceAll}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -751,7 +741,7 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { name: "generates role and rolebinding for permission service-account when in Single/OwnNamespace mode (target namespace contains a single namespace)", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), @@ -824,7 +814,7 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { name: "generates role and rolebinding for permission service-account for each target namespace when in MultiNamespace install mode (target namespace contains multiple namespaces)", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace", "watch-namespace-two"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeMultiNamespace), WithName("csv"), @@ -941,7 +931,7 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { name: "generates role and rolebinding for each permission service-account", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -1056,7 +1046,7 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { name: "treats empty service account as 'default' service account", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -1119,7 +1109,10 @@ func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) require.NoError(t, err) for _, expectedObj := range tc.expectedResources { @@ -1137,14 +1130,14 @@ func Test_Convert_ClusterRoleClusterRoleBindingResourceGeneration(t *testing.T) name string installNamespace string targetNamespaces []string - bundle convert.RegistryV1 + bundle render.RegistryV1 expectedResources []client.Object }{ { name: "promotes permissions to clusters permissions and adds namespace policy rule when in AllNamespaces mode (target namespace is [''])", installNamespace: "install-namespace", targetNamespaces: []string{""}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -1263,7 +1256,7 @@ func Test_Convert_ClusterRoleClusterRoleBindingResourceGeneration(t *testing.T) name: "generates clusterroles and clusterrolebindings for clusterpermissions", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), WithName("csv"), @@ -1374,7 +1367,7 @@ func Test_Convert_ClusterRoleClusterRoleBindingResourceGeneration(t *testing.T) name: "treats empty service accounts as 'default' service account", installNamespace: "install-namespace", targetNamespaces: []string{"watch-namespace"}, - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), WithName("csv"), @@ -1435,7 +1428,11 @@ func Test_Convert_ClusterRoleClusterRoleBindingResourceGeneration(t *testing.T) }, } { t.Run(tc.name, func(t *testing.T) { - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil + plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) require.NoError(t, err) for _, expectedObj := range tc.expectedResources { @@ -1453,13 +1450,13 @@ func Test_Convert_ServiceAccountResourceGeneration(t *testing.T) { name string installNamespace string targetNamespaces []string - bundle convert.RegistryV1 + bundle render.RegistryV1 expectedResources []client.Object }{ { name: "generates unique set of clusterpermissions and permissions service accounts in the install namespace", installNamespace: "install-namespace", - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -1545,7 +1542,7 @@ func Test_Convert_ServiceAccountResourceGeneration(t *testing.T) { { name: "treats empty service accounts as default and doesn't generate them", installNamespace: "install-namespace", - bundle: convert.RegistryV1{ + bundle: render.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithName("csv"), @@ -1579,7 +1576,10 @@ func Test_Convert_ServiceAccountResourceGeneration(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) require.NoError(t, err) for _, expectedObj := range tc.expectedResources { @@ -1593,7 +1593,7 @@ func Test_Convert_ServiceAccountResourceGeneration(t *testing.T) { } func Test_Convert_BundleCRDGeneration(t *testing.T) { - bundle := convert.RegistryV1{ + bundle := render.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, @@ -1601,7 +1601,10 @@ func Test_Convert_BundleCRDGeneration(t *testing.T) { CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), } - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil plain, err := conv.Convert(bundle, "install-namespace", []string{""}) require.NoError(t, err) expectedResources := []client.Object{ @@ -1618,7 +1621,7 @@ func Test_Convert_BundleCRDGeneration(t *testing.T) { } func Test_Convert_AdditionalResourcesGeneration(t *testing.T) { - bundle := convert.RegistryV1{ + bundle := render.RegistryV1{ CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), Others: []unstructured.Unstructured{ convertToUnstructured(t, @@ -1646,7 +1649,10 @@ func Test_Convert_AdditionalResourcesGeneration(t *testing.T) { }, } - conv := convert.Converter{} + // ignore bundle validation for these unit tests as we only want to test + // the specific resource generation logic + conv := convert.PlainConverter + conv.BundleValidator = nil plain, err := conv.Convert(bundle, "install-namespace", []string{""}) require.NoError(t, err) expectedResources := []unstructured.Unstructured{ diff --git a/internal/operator-controller/rukpak/render/generators/generators.go b/internal/operator-controller/rukpak/render/generators/generators.go new file mode 100644 index 000000000..dfc73a2ab --- /dev/null +++ b/internal/operator-controller/rukpak/render/generators/generators.go @@ -0,0 +1,211 @@ +package generators + +import ( + "cmp" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +// BundleCSVRBACResourceGenerator generates all ServiceAccounts, ClusterRoles, ClusterRoleBindings, Roles, RoleBindings +// defined in the RegistryV1 bundle's cluster service version (CSV) +var BundleCSVRBACResourceGenerator = render.ResourceGenerators{ + BundleCSVServiceAccountGenerator, + BundleCSVPermissionsGenerator, + BundleCSVClusterPermissionsGenerator, +} + +// BundleCSVDeploymentGenerator generates all deployments defined in rv1's cluster service version (CSV). The generated +// resource aim to have parity with OLMv0 generated Deployment resources: +// - olm.targetNamespaces annotation is set with the opts.TargetNamespace value +// - the deployment spec's revision history limit is set to 1 +// - merges csv annotations to the deployment template's annotations +func BundleCSVDeploymentGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + objs := make([]client.Object, 0, len(rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs)) + for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + // Add CSV annotations to template annotations + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/deployment.go#L142 + annotations := util.MergeMaps(rv1.CSV.Annotations, depSpec.Spec.Template.Annotations) + + // In OLMv0 CSVs are annotated with the OperatorGroup's .spec.targetNamespaces + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/operators/olm/operatorgroup.go#L279 + // When the CSVs annotations are copied to the deployment template's annotations, they bring with it this annotation + annotations["olm.targetNamespaces"] = strings.Join(opts.TargetNamespaces, ",") + depSpec.Spec.Template.Annotations = annotations + + // Hardcode the deployment with RevisionHistoryLimit=1 to maintain parity with OLMv0 behaviour. + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/deployment.go#L177-L180 + depSpec.Spec.RevisionHistoryLimit = ptr.To(int32(1)) + + objs = append(objs, + CreateDeploymentResource( + depSpec.Name, + opts.InstallNamespace, + WithDeploymentSpec(depSpec.Spec), + WithLabels(depSpec.Label), + ), + ) + } + return objs, nil +} + +// BundleCSVPermissionsGenerator generates the Roles and RoleBindings based on bundle's cluster service version +// permission spec. If the bundle is being installed in AllNamespaces mode (opts.TargetNamespaces = [”]) +// no resources will be generated as these permissions will be promoted to ClusterRole/Bunding(s) +func BundleCSVPermissionsGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + + // If we're in AllNamespaces mode permissions will be treated as clusterPermissions + if len(opts.TargetNamespaces) == 1 && opts.TargetNamespaces[0] == "" { + return nil, nil + } + + permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions + + objs := make([]client.Object, 0, 2*len(opts.TargetNamespaces)*len(permissions)) + for _, ns := range opts.TargetNamespaces { + for _, permission := range permissions { + saName := saNameOrDefault(permission.ServiceAccountName) + name, err := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission) + if err != nil { + return nil, err + } + + objs = append(objs, + CreateRoleResource(name, ns, WithRules(permission.Rules...)), + CreateRoleBindingResource( + name, + ns, + WithSubjects(rbacv1.Subject{Kind: "ServiceAccount", Namespace: opts.InstallNamespace, Name: saName}), + WithRoleRef(rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: name}), + ), + ) + } + } + return objs, nil +} + +// BundleCSVClusterPermissionsGenerator generates ClusterRoles and ClusterRoleBindings based on the bundle's +// cluster service version clusterPermission spec. If the bundle is being installed in AllNamespaces mode +// (opts.TargetNamespaces = [”]), the CSV's permission spec will be promoted to ClusterRole and ClusterRoleBinding +// resources. To keep parity with OLMv0, these will also include an extra rule to get, list, watch namespaces +// (see https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/operators/olm/operatorgroup.go#L539) +func BundleCSVClusterPermissionsGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions + + // If we're in AllNamespaces mode, promote the permissions to clusterPermissions + if len(opts.TargetNamespaces) == 1 && opts.TargetNamespaces[0] == "" { + for _, p := range rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions { + p.Rules = append(p.Rules, rbacv1.PolicyRule{ + Verbs: []string{"get", "list", "watch"}, + APIGroups: []string{corev1.GroupName}, + Resources: []string{"namespaces"}, + }) + clusterPermissions = append(clusterPermissions, p) + } + } + + objs := make([]client.Object, 0, 2*len(clusterPermissions)) + for _, permission := range clusterPermissions { + saName := saNameOrDefault(permission.ServiceAccountName) + name, err := opts.UniqueNameGenerator(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission) + if err != nil { + return nil, err + } + objs = append(objs, + CreateClusterRoleResource(name, WithRules(permission.Rules...)), + CreateClusterRoleBindingResource( + name, + WithSubjects(rbacv1.Subject{Kind: "ServiceAccount", Namespace: opts.InstallNamespace, Name: saName}), + WithRoleRef(rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: name}), + ), + ) + } + return objs, nil +} + +// BundleCSVServiceAccountGenerator generates ServiceAccount resources based on the bundle's cluster service version +// permission and clusterPermission spec. One ServiceAccount resource is created / referenced service account (i.e. +// if multiple permissions reference the same service account, only one resource will be generated). +// If a clusterPermission, or permission, references an empty (”) service account, this is considered to be the +// namespace 'default' service account. A resource for the namespace 'default' service account is not generated. +func BundleCSVServiceAccountGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + allPermissions := append( + rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions, + rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions..., + ) + + serviceAccountNames := sets.Set[string]{} + for _, permission := range allPermissions { + serviceAccountNames.Insert(saNameOrDefault(permission.ServiceAccountName)) + } + + objs := make([]client.Object, 0, len(serviceAccountNames)) + for _, serviceAccountName := range serviceAccountNames.UnsortedList() { + // no need to generate the default service account + if serviceAccountName != "default" { + objs = append(objs, CreateServiceAccountResource(serviceAccountName, opts.InstallNamespace)) + } + } + return objs, nil +} + +// BundleCRDGenerator generates CustomResourceDefinition resources from the registry+v1 bundle +func BundleCRDGenerator(rv1 *render.RegistryV1, _ render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + objs := make([]client.Object, 0, len(rv1.CRDs)) + for _, crd := range rv1.CRDs { + objs = append(objs, crd.DeepCopy()) + } + return objs, nil +} + +// BundleAdditionalResourcesGenerator generates resources for the additional resources included in the +// bundle. If the bundle resource is namespace scoped, its namespace will be set to the value of opts.InstallNamespace. +func BundleAdditionalResourcesGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + objs := make([]client.Object, 0, len(rv1.Others)) + for _, res := range rv1.Others { + supported, namespaced := registrybundle.IsSupported(res.GetKind()) + if !supported { + return nil, fmt.Errorf("bundle contains unsupported resource: Name: %v, Kind: %v", res.GetName(), res.GetKind()) + } + + obj := res.DeepCopy() + if namespaced { + obj.SetNamespace(opts.InstallNamespace) + } + + objs = append(objs, obj) + } + return objs, nil +} + +func saNameOrDefault(saName string) string { + return cmp.Or(saName, "default") +} diff --git a/internal/operator-controller/rukpak/render/generators/generators_test.go b/internal/operator-controller/rukpak/render/generators/generators_test.go new file mode 100644 index 000000000..d3151f829 --- /dev/null +++ b/internal/operator-controller/rukpak/render/generators/generators_test.go @@ -0,0 +1,1183 @@ +package generators_test + +import ( + "cmp" + "fmt" + "reflect" + "slices" + "testing" + + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +func Test_BundleCSVRBACResourceGenerator_HasCorrectGenerators(t *testing.T) { + expectedResourceGenerators := []render.ResourceGenerator{ + generators.BundleCSVServiceAccountGenerator, + generators.BundleCSVPermissionsGenerator, + generators.BundleCSVClusterPermissionsGenerator, + } + actualResourceGenerators := generators.BundleCSVRBACResourceGenerator + + require.Equal(t, len(expectedResourceGenerators), len(actualResourceGenerators)) + for i := range expectedResourceGenerators { + require.Equal(t, reflect.ValueOf(expectedResourceGenerators[i]).Pointer(), reflect.ValueOf(actualResourceGenerators[i]).Pointer(), "bundle validator has unexpected validation function") + } +} + +func Test_ResourceGenerators(t *testing.T) { + g := render.ResourceGenerators{ + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&corev1.Service{}}, nil + }, + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&corev1.ConfigMap{}}, nil + }, + } + + objs, err := g.GenerateResources(&render.RegistryV1{}, render.Options{}) + require.NoError(t, err) + require.Equal(t, []client.Object{&corev1.Service{}, &corev1.ConfigMap{}}, objs) +} + +func Test_ResourceGenerators_Errors(t *testing.T) { + g := render.ResourceGenerators{ + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&corev1.Service{}}, nil + }, + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return nil, fmt.Errorf("generator error") + }, + } + + objs, err := g.GenerateResources(&render.RegistryV1{}, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "generator error") +} + +func Test_BundleCSVDeploymentGenerator_Succeeds(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + opts render.Options + expectedResources []client.Object + }{ + { + name: "generates deployment resources", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithAnnotations(map[string]string{ + "csv": "annotation", + }), + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "deployment-one", + Label: map[string]string{ + "bar": "foo", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "pod": "annotation", + }, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "some-service-account", + }, + }, + }, + }, + v1alpha1.StrategyDeploymentSpec{ + Name: "deployment-two", + Spec: appsv1.DeploymentSpec{}, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: appsv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "install-namespace", + Name: "deployment-one", + Labels: map[string]string{ + "bar": "foo", + }, + }, + Spec: appsv1.DeploymentSpec{ + RevisionHistoryLimit: ptr.To(int32(1)), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "csv": "annotation", + "olm.targetNamespaces": "watch-namespace-one,watch-namespace-two", + "pod": "annotation", + }, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "some-service-account", + }, + }, + }, + }, + &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: appsv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "install-namespace", + Name: "deployment-two", + }, + Spec: appsv1.DeploymentSpec{ + RevisionHistoryLimit: ptr.To(int32(1)), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "csv": "annotation", + "olm.targetNamespaces": "watch-namespace-one,watch-namespace-two", + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleCSVDeploymentGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + require.Equal(t, tc.expectedResources, objs) + }) + } +} + +func Test_BundleCSVDeploymentGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCSVDeploymentGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { + fakeUniqueNameGenerator := func(base string, _ interface{}) (string, error) { + return base, nil + } + + for _, tc := range []struct { + name string + opts render.Options + bundle *render.RegistryV1 + expectedResources []client.Object + }{ + { + name: "does not generate any resources when in AllNamespaces mode (target namespace is [''])", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + ), + ), + }, + expectedResources: nil, + }, + { + name: "generates role and rolebinding for permission service-account when in Single/OwnNamespace mode (target namespace contains a single namespace)", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-service-account-one", + }, + }, + }, + }, + { + name: "generates role and rolebinding for permission service-account for each target namespace when in MultiNamespace install mode (target namespace contains multiple namespaces)", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace", "watch-namespace-two"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-service-account-one", + }, + }, + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace-two", + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace-two", + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-service-account-one", + }, + }, + }, + }, + { + name: "generates role and rolebinding for each permission service-account", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-two", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-service-account-one", + }, + }, + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-two", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-service-account-two", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-two", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-service-account-two", + }, + }, + }, + }, + { + name: "treats empty service account as 'default' service account", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-default", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "watch-namespace", + Name: "csv-default", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "default", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: "csv-default", + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleCSVPermissionsGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + for i := range objs { + require.Equal(t, tc.expectedResources[i], objs[i], "failed to find expected resource at index %d", i) + } + require.Equal(t, len(tc.expectedResources), len(objs)) + }) + } +} + +func Test_BundleCSVPermissionGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCSVPermissionsGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) { + fakeUniqueNameGenerator := func(base string, _ interface{}) (string, error) { + return base, nil + } + + for _, tc := range []struct { + name string + opts render.Options + bundle *render.RegistryV1 + expectedResources []client.Object + }{ + { + name: "promotes permissions to clusters permissions and adds namespace policy rule when in AllNamespaces mode (target namespace is [''])", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-two", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, { + Verbs: []string{"get", "list", "watch"}, + APIGroups: []string{corev1.GroupName}, + Resources: []string{"namespaces"}, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "csv-service-account-one", + }, + }, + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-two", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, { + Verbs: []string{"get", "list", "watch"}, + APIGroups: []string{corev1.GroupName}, + Resources: []string{"namespaces"}, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-two", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-two", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "csv-service-account-two", + }, + }, + }, + }, + { + name: "generates clusterroles and clusterrolebindings for clusterpermissions", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithClusterPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-one", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-two", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-one", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-one", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-one", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "csv-service-account-one", + }, + }, + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-two", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-service-account-two", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "service-account-two", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "csv-service-account-two", + }, + }, + }, + }, + { + name: "treats empty service accounts as 'default' service account", + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace"}, + UniqueNameGenerator: fakeUniqueNameGenerator, + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithClusterPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-default", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "csv-default", + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + APIGroup: "", + Name: "default", + Namespace: "install-namespace", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "csv-default", + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleCSVClusterPermissionsGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + for i := range objs { + require.Equal(t, tc.expectedResources[i], objs[i], "failed to find expected resource at index %d", i) + } + require.Equal(t, len(tc.expectedResources), len(objs)) + }) + } +} + +func Test_BundleCSVClusterPermissionGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCSVClusterPermissionsGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleCSVServiceAccountGenerator_Succeeds(t *testing.T) { + for _, tc := range []struct { + name string + opts render.Options + bundle *render.RegistryV1 + expectedResources []client.Object + }{ + { + name: "generates unique set of clusterpermissions and permissions service accounts in the install namespace", + opts: render.Options{ + InstallNamespace: "install-namespace", + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-1", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-2", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + WithClusterPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-2", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account-3", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments"}, + Verbs: []string{"create"}, + }, + }, + }, + ), + ), + }, + expectedResources: []client.Object{ + &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "service-account-1", + Namespace: "install-namespace", + }, + }, + &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "service-account-2", + Namespace: "install-namespace", + }, + }, + &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "service-account-3", + Namespace: "install-namespace", + }, + }, + }, + }, + { + name: "treats empty service accounts as default and doesn't generate them", + opts: render.Options{ + InstallNamespace: "install-namespace", + }, + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithName("csv"), + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + ), + WithClusterPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + ), + ), + }, + expectedResources: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleCSVServiceAccountGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + slices.SortFunc(objs, func(a, b client.Object) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + for i := range objs { + require.Equal(t, tc.expectedResources[i], objs[i], "failed to find expected resource at index %d", i) + } + require.Equal(t, len(tc.expectedResources), len(objs)) + }) + } +} + +func Test_BundleCSVServiceAccountGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCSVServiceAccountGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleCRDGenerator_Succeeds(t *testing.T) { + opts := render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + } + + bundle := &render.RegistryV1{ + CRDs: []apiextensionsv1.CustomResourceDefinition{ + {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, + }, + } + + objs, err := generators.BundleCRDGenerator(bundle, opts) + require.NoError(t, err) + require.Equal(t, []client.Object{ + &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, + &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, + }, objs) +} + +func Test_BundleCRDGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCRDGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleAdditionalResourcesGenerator_Succeeds(t *testing.T) { + opts := render.Options{ + InstallNamespace: "install-namespace", + } + + bundle := &render.RegistryV1{ + Others: []unstructured.Unstructured{ + toUnstructured(t, + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bundled-service", + }, + }, + ), + toUnstructured(t, + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bundled-clusterrole", + }, + }, + ), + }, + } + + objs, err := generators.BundleAdditionalResourcesGenerator(bundle, opts) + require.NoError(t, err) + require.Len(t, objs, 2) +} + +func Test_BundleAdditionalResourcesGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleAdditionalResourcesGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func toUnstructured(t *testing.T, obj client.Object) unstructured.Unstructured { + gvk := obj.GetObjectKind().GroupVersionKind() + + var u unstructured.Unstructured + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + require.NoError(t, err) + unstructured.RemoveNestedField(uObj, "metadata", "creationTimestamp") + unstructured.RemoveNestedField(uObj, "status") + u.Object = uObj + u.SetGroupVersionKind(gvk) + return u +} diff --git a/internal/operator-controller/rukpak/render/generators/resources.go b/internal/operator-controller/rukpak/render/generators/resources.go new file mode 100644 index 000000000..53fdd1355 --- /dev/null +++ b/internal/operator-controller/rukpak/render/generators/resources.go @@ -0,0 +1,177 @@ +package generators + +import ( + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type ResourceCreatorOption = func(client.Object) +type ResourceCreatorOptions []ResourceCreatorOption + +func (r ResourceCreatorOptions) ApplyTo(obj client.Object) client.Object { + if obj == nil { + return nil + } + for _, opt := range r { + if opt != nil { + opt(obj) + } + } + return obj +} + +// WithSubjects applies rbac subjects to ClusterRoleBinding and RoleBinding resources +func WithSubjects(subjects ...rbacv1.Subject) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *rbacv1.RoleBinding: + o.Subjects = subjects + case *rbacv1.ClusterRoleBinding: + o.Subjects = subjects + } + } +} + +// WithRoleRef applies an rbac RoleRef to ClusterRoleBinding and RoleBinding resources +func WithRoleRef(roleRef rbacv1.RoleRef) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *rbacv1.RoleBinding: + o.RoleRef = roleRef + case *rbacv1.ClusterRoleBinding: + o.RoleRef = roleRef + } + } +} + +// WithRules applies rbac PolicyRules to Role and ClusterRole resources +func WithRules(rules ...rbacv1.PolicyRule) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *rbacv1.Role: + o.Rules = rules + case *rbacv1.ClusterRole: + o.Rules = rules + } + } +} + +// WithDeploymentSpec applies a DeploymentSpec to Deployment resources +func WithDeploymentSpec(depSpec appsv1.DeploymentSpec) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *appsv1.Deployment: + o.Spec = depSpec + } + } +} + +// WithLabels applies labels to the metadata of any resource +func WithLabels(labels map[string]string) func(client.Object) { + return func(obj client.Object) { + obj.SetLabels(labels) + } +} + +// CreateServiceAccountResource creates a ServiceAccount resource with name 'name', namespace 'namespace', and applying +// any ServiceAccount related options in opts +func CreateServiceAccountResource(name string, namespace string, opts ...ResourceCreatorOption) *corev1.ServiceAccount { + return ResourceCreatorOptions(opts).ApplyTo( + &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }, + ).(*corev1.ServiceAccount) +} + +// CreateRoleResource creates a Role resource with name 'name' and namespace 'namespace' and applying any +// Role related options in opts +func CreateRoleResource(name string, namespace string, opts ...ResourceCreatorOption) *rbacv1.Role { + return ResourceCreatorOptions(opts).ApplyTo( + &rbacv1.Role{ + TypeMeta: metav1.TypeMeta{ + Kind: "Role", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }, + ).(*rbacv1.Role) +} + +// CreateClusterRoleResource creates a ClusterRole resource with name 'name' and applying any +// ClusterRole related options in opts +func CreateClusterRoleResource(name string, opts ...ResourceCreatorOption) *rbacv1.ClusterRole { + return ResourceCreatorOptions(opts).ApplyTo( + &rbacv1.ClusterRole{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRole", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }, + ).(*rbacv1.ClusterRole) +} + +// CreateClusterRoleBindingResource creates a ClusterRoleBinding resource with name 'name' and applying any +// ClusterRoleBinding related options in opts +func CreateClusterRoleBindingResource(name string, opts ...ResourceCreatorOption) *rbacv1.ClusterRoleBinding { + return ResourceCreatorOptions(opts).ApplyTo( + &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }, + ).(*rbacv1.ClusterRoleBinding) +} + +// CreateRoleBindingResource creates a RoleBinding resource with name 'name', namespace 'namespace', and applying any +// RoleBinding related options in opts +func CreateRoleBindingResource(name string, namespace string, opts ...ResourceCreatorOption) *rbacv1.RoleBinding { + return ResourceCreatorOptions(opts).ApplyTo( + &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + APIVersion: rbacv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }, + ).(*rbacv1.RoleBinding) +} + +// CreateDeploymentResource creates a Deployment resource with name 'name', namespace 'namespace', and applying any +// Deployment related options in opts +func CreateDeploymentResource(name string, namespace string, opts ...ResourceCreatorOption) *appsv1.Deployment { + return ResourceCreatorOptions(opts).ApplyTo( + &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: appsv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }, + ).(*appsv1.Deployment) +} diff --git a/internal/operator-controller/rukpak/render/generators/resources_test.go b/internal/operator-controller/rukpak/render/generators/resources_test.go new file mode 100644 index 000000000..6aeed1c8f --- /dev/null +++ b/internal/operator-controller/rukpak/render/generators/resources_test.go @@ -0,0 +1,210 @@ +package generators_test + +import ( + "maps" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +func Test_OptionsApplyToExecutesIgnoresNil(t *testing.T) { + opts := []generators.ResourceCreatorOption{ + func(object client.Object) { + object.SetAnnotations(util.MergeMaps(object.GetAnnotations(), map[string]string{"h": ""})) + }, + nil, + func(object client.Object) { + object.SetAnnotations(util.MergeMaps(object.GetAnnotations(), map[string]string{"i": ""})) + }, + nil, + } + + require.Nil(t, generators.ResourceCreatorOptions(nil).ApplyTo(nil)) + require.Nil(t, generators.ResourceCreatorOptions([]generators.ResourceCreatorOption{}).ApplyTo(nil)) + + obj := generators.ResourceCreatorOptions(opts).ApplyTo(&corev1.ConfigMap{}) + require.Equal(t, "hi", strings.Join(slices.Sorted(maps.Keys(obj.GetAnnotations())), "")) +} + +func Test_CreateServiceAccount(t *testing.T) { + svc := generators.CreateServiceAccountResource("my-sa", "my-namespace") + require.NotNil(t, svc) + require.Equal(t, "my-sa", svc.Name) + require.Equal(t, "my-namespace", svc.Namespace) +} + +func Test_CreateRole(t *testing.T) { + role := generators.CreateRoleResource("my-role", "my-namespace") + require.NotNil(t, role) + require.Equal(t, "my-role", role.Name) + require.Equal(t, "my-namespace", role.Namespace) +} + +func Test_CreateRoleBinding(t *testing.T) { + roleBinding := generators.CreateRoleBindingResource("my-role-binding", "my-namespace") + require.NotNil(t, roleBinding) + require.Equal(t, "my-role-binding", roleBinding.Name) + require.Equal(t, "my-namespace", roleBinding.Namespace) +} + +func Test_CreateClusterRole(t *testing.T) { + clusterRole := generators.CreateClusterRoleResource("my-cluster-role") + require.NotNil(t, clusterRole) + require.Equal(t, "my-cluster-role", clusterRole.Name) +} + +func Test_CreateClusterRoleBinding(t *testing.T) { + clusterRoleBinding := generators.CreateClusterRoleBindingResource("my-cluster-role-binding") + require.NotNil(t, clusterRoleBinding) + require.Equal(t, "my-cluster-role-binding", clusterRoleBinding.Name) +} + +func Test_CreateDeployment(t *testing.T) { + deployment := generators.CreateDeploymentResource("my-deployment", "my-namespace") + require.NotNil(t, deployment) + require.Equal(t, "my-deployment", deployment.Name) + require.Equal(t, "my-namespace", deployment.Namespace) +} + +func Test_WithSubjects(t *testing.T) { + for _, tc := range []struct { + name string + subjects []rbacv1.Subject + }{ + { + name: "empty", + subjects: []rbacv1.Subject{}, + }, { + name: "nil", + subjects: nil, + }, { + name: "single subject", + subjects: []rbacv1.Subject{ + { + APIGroup: rbacv1.GroupName, + Kind: rbacv1.ServiceAccountKind, + Name: "my-sa", + Namespace: "my-namespace", + }, + }, + }, { + name: "multiple subjects", + subjects: []rbacv1.Subject{ + { + APIGroup: rbacv1.GroupName, + Kind: rbacv1.ServiceAccountKind, + Name: "my-sa", + Namespace: "my-namespace", + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + roleBinding := generators.CreateRoleBindingResource("my-role", "my-namespace", generators.WithSubjects(tc.subjects...)) + require.NotNil(t, roleBinding) + require.Equal(t, roleBinding.Subjects, tc.subjects) + + clusterRoleBinding := generators.CreateClusterRoleBindingResource("my-role", generators.WithSubjects(tc.subjects...)) + require.NotNil(t, clusterRoleBinding) + require.Equal(t, clusterRoleBinding.Subjects, tc.subjects) + }) + } +} + +func Test_WithRules(t *testing.T) { + for _, tc := range []struct { + name string + rules []rbacv1.PolicyRule + }{ + { + name: "empty", + rules: []rbacv1.PolicyRule{}, + }, { + name: "nil", + rules: nil, + }, { + name: "single subject", + rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + }, + }, { + name: "multiple subjects", + rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + ResourceNames: []string{"my-resource"}, + }, { + Verbs: []string{"get", "list", "watch"}, + APIGroups: []string{"appsv1"}, + Resources: []string{"deployments", "replicasets", "statefulsets"}, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + role := generators.CreateRoleResource("my-role", "my-namespace", generators.WithRules(tc.rules...)) + require.NotNil(t, role) + require.Equal(t, role.Rules, tc.rules) + + clusterRole := generators.CreateClusterRoleResource("my-role", generators.WithRules(tc.rules...)) + require.NotNil(t, clusterRole) + require.Equal(t, clusterRole.Rules, tc.rules) + }) + } +} + +func Test_WithRoleRef(t *testing.T) { + roleRef := rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: "my-role", + } + + roleBinding := generators.CreateRoleBindingResource("my-role-binding", "my-namespace", generators.WithRoleRef(roleRef)) + require.NotNil(t, roleBinding) + require.Equal(t, roleRef, roleBinding.RoleRef) + + clusterRoleBinding := generators.CreateClusterRoleBindingResource("my-cluster-role-binding", generators.WithRoleRef(roleRef)) + require.NotNil(t, clusterRoleBinding) + require.Equal(t, roleRef, clusterRoleBinding.RoleRef) +} + +func Test_WithLabels(t *testing.T) { + for _, tc := range []struct { + name string + labels map[string]string + }{ + { + name: "empty", + labels: map[string]string{}, + }, { + name: "nil", + labels: nil, + }, { + name: "not empty", + labels: map[string]string{ + "foo": "bar", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + dep := generators.CreateDeploymentResource("my-deployment", "my-namespace", generators.WithLabels(tc.labels)) + require.NotNil(t, dep) + require.Equal(t, tc.labels, dep.Labels) + }) + } +} diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go new file mode 100644 index 000000000..af904aec0 --- /dev/null +++ b/internal/operator-controller/rukpak/render/render.go @@ -0,0 +1,111 @@ +package render + +import ( + "errors" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +type RegistryV1 struct { + PackageName string + CSV v1alpha1.ClusterServiceVersion + CRDs []apiextensionsv1.CustomResourceDefinition + Others []unstructured.Unstructured +} + +// BundleValidator validates a RegistryV1 bundle by executing a series of +// checks on it and collecting any errors that were found +type BundleValidator []func(v1 *RegistryV1) []error + +func (v BundleValidator) Validate(rv1 *RegistryV1) error { + var errs []error + for _, validator := range v { + errs = append(errs, validator(rv1)...) + } + return errors.Join(errs...) +} + +// ResourceGenerator generates resources given a registry+v1 bundle and options +type ResourceGenerator func(rv1 *RegistryV1, opts Options) ([]client.Object, error) + +func (g ResourceGenerator) GenerateResources(rv1 *RegistryV1, opts Options) ([]client.Object, error) { + return g(rv1, opts) +} + +// ResourceGenerators aggregates generators. Its GenerateResource method will call all of its generators and return +// generated resources. +type ResourceGenerators []ResourceGenerator + +func (r ResourceGenerators) GenerateResources(rv1 *RegistryV1, opts Options) ([]client.Object, error) { + //nolint:prealloc + var renderedObjects []client.Object + for _, generator := range r { + objs, err := generator.GenerateResources(rv1, opts) + if err != nil { + return nil, err + } + renderedObjects = append(renderedObjects, objs...) + } + return renderedObjects, nil +} + +func (r ResourceGenerators) ResourceGenerator() ResourceGenerator { + return r.GenerateResources +} + +type UniqueNameGenerator func(string, interface{}) (string, error) + +type Options struct { + InstallNamespace string + TargetNamespaces []string + UniqueNameGenerator UniqueNameGenerator +} + +func (o *Options) apply(opts ...Option) *Options { + for _, opt := range opts { + opt(o) + } + return o +} + +type Option func(*Options) + +type BundleRenderer struct { + BundleValidator BundleValidator + ResourceGenerators []ResourceGenerator +} + +func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, watchNamespaces []string, opts ...Option) ([]client.Object, error) { + // validate bundle + if err := r.BundleValidator.Validate(&rv1); err != nil { + return nil, err + } + + genOpts := (&Options{ + InstallNamespace: installNamespace, + TargetNamespaces: watchNamespaces, + UniqueNameGenerator: DefaultUniqueNameGenerator, + }).apply(opts...) + + // generate bundle objects + objs, err := ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) + if err != nil { + return nil, err + } + + return objs, nil +} + +func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) { + hashStr, err := util.DeepHashObject(o) + if err != nil { + return "", err + } + return util.ObjectNameForBaseAndSuffix(base, hashStr), nil +} diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go new file mode 100644 index 000000000..ede57ff69 --- /dev/null +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -0,0 +1,104 @@ +package render_test + +import ( + "errors" + "fmt" + "reflect" + "testing" + + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" +) + +func Test_BundleRenderer_NoConfig(t *testing.T) { + renderer := render.BundleRenderer{} + objs, err := renderer.Render(render.RegistryV1{}, "", nil) + require.NoError(t, err) + require.Empty(t, objs) +} + +func Test_BundleRenderer_ValidatesBundle(t *testing.T) { + renderer := render.BundleRenderer{ + BundleValidator: render.BundleValidator{ + func(v1 *render.RegistryV1) []error { + return []error{errors.New("this bundle is invalid")} + }, + }, + } + objs, err := renderer.Render(render.RegistryV1{}, "", nil) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "this bundle is invalid") +} + +func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { + expectedInstallNamespace := "install-namespace" + expectedTargetNamespaces := []string{"ns-one", "ns-two"} + expectedUniqueNameGenerator := render.DefaultUniqueNameGenerator + + renderer := render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + require.Equal(t, expectedInstallNamespace, opts.InstallNamespace) + require.Equal(t, expectedTargetNamespaces, opts.TargetNamespaces) + require.Equal(t, reflect.ValueOf(expectedUniqueNameGenerator).Pointer(), reflect.ValueOf(render.DefaultUniqueNameGenerator).Pointer(), "options has unexpected default unique name generator") + return nil, nil + }, + }, + } + + _, _ = renderer.Render(render.RegistryV1{}, expectedInstallNamespace, expectedTargetNamespaces) +} + +func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { + renderer := render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&corev1.Namespace{}, &corev1.Service{}}, nil + }, + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&appsv1.Deployment{}}, nil + }, + }, + } + objs, err := renderer.Render(render.RegistryV1{}, "", nil) + require.NoError(t, err) + require.Equal(t, []client.Object{&corev1.Namespace{}, &corev1.Service{}, &appsv1.Deployment{}}, objs) +} + +func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { + renderer := render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return []client.Object{&corev1.Namespace{}, &corev1.Service{}}, nil + }, + func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + return nil, fmt.Errorf("generator error") + }, + }, + } + objs, err := renderer.Render(render.RegistryV1{}, "", nil) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "generator error") +} + +func Test_BundleValidatorCallsAllValidationFnsInOrder(t *testing.T) { + actual := "" + val := render.BundleValidator{ + func(v1 *render.RegistryV1) []error { + actual += "h" + return nil + }, + func(v1 *render.RegistryV1) []error { + actual += "i" + return nil + }, + } + require.NoError(t, val.Validate(nil)) + require.Equal(t, "hi", actual) +} diff --git a/internal/operator-controller/rukpak/convert/validator.go b/internal/operator-controller/rukpak/render/validators/validator.go similarity index 82% rename from internal/operator-controller/rukpak/convert/validator.go rename to internal/operator-controller/rukpak/render/validators/validator.go index e0e8135eb..d2ed950e5 100644 --- a/internal/operator-controller/rukpak/convert/validator.go +++ b/internal/operator-controller/rukpak/render/validators/validator.go @@ -1,4 +1,4 @@ -package convert +package validators import ( "errors" @@ -6,19 +6,12 @@ import ( "slices" "k8s.io/apimachinery/pkg/util/sets" -) - -type BundleValidator []func(v1 *RegistryV1) []error -func (v BundleValidator) Validate(rv1 *RegistryV1) error { - var errs []error - for _, validator := range v { - errs = append(errs, validator(rv1)...) - } - return errors.Join(errs...) -} + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" +) -var RegistryV1BundleValidator = BundleValidator{ +// RegistryV1BundleValidator validates RegistryV1 bundles +var RegistryV1BundleValidator = render.BundleValidator{ // NOTE: if you update this list, Test_BundleValidatorHasAllValidationFns will fail until // you bring the same changes over to that test. This helps ensure all validation rules are executed // while giving us the flexibility to test each validation function individually @@ -30,7 +23,7 @@ var RegistryV1BundleValidator = BundleValidator{ // CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name. // Errors are sorted by deployment name. -func CheckDeploymentSpecUniqueness(rv1 *RegistryV1) []error { +func CheckDeploymentSpecUniqueness(rv1 *render.RegistryV1) []error { deploymentNameSet := sets.Set[string]{} duplicateDeploymentNames := sets.Set[string]{} for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { @@ -48,7 +41,7 @@ func CheckDeploymentSpecUniqueness(rv1 *RegistryV1) []error { } // CheckOwnedCRDExistence checks bundle owned custom resource definitions declared in the csv exist in the bundle -func CheckOwnedCRDExistence(rv1 *RegistryV1) []error { +func CheckOwnedCRDExistence(rv1 *render.RegistryV1) []error { crdsNames := sets.Set[string]{} for _, crd := range rv1.CRDs { crdsNames.Insert(crd.Name) @@ -69,7 +62,7 @@ func CheckOwnedCRDExistence(rv1 *RegistryV1) []error { } // CheckCRDResourceUniqueness checks that the bundle CRD names are unique -func CheckCRDResourceUniqueness(rv1 *RegistryV1) []error { +func CheckCRDResourceUniqueness(rv1 *render.RegistryV1) []error { crdsNames := sets.Set[string]{} duplicateCRDNames := sets.Set[string]{} for _, crd := range rv1.CRDs { @@ -87,7 +80,7 @@ func CheckCRDResourceUniqueness(rv1 *RegistryV1) []error { } // CheckPackageNameNotEmpty checks that PackageName is not empty -func CheckPackageNameNotEmpty(rv1 *RegistryV1) []error { +func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error { if rv1.PackageName == "" { return []error{errors.New("package name is empty")} } diff --git a/internal/operator-controller/rukpak/convert/validator_test.go b/internal/operator-controller/rukpak/render/validators/validator_test.go similarity index 70% rename from internal/operator-controller/rukpak/convert/validator_test.go rename to internal/operator-controller/rukpak/render/validators/validator_test.go index 99c78f40f..17da3e640 100644 --- a/internal/operator-controller/rukpak/convert/validator_test.go +++ b/internal/operator-controller/rukpak/render/validators/validator_test.go @@ -1,4 +1,4 @@ -package convert_test +package validators_test import ( "errors" @@ -11,17 +11,19 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) func Test_BundleValidatorHasAllValidationFns(t *testing.T) { - expectedValidationFns := []func(v1 *convert.RegistryV1) []error{ - convert.CheckDeploymentSpecUniqueness, - convert.CheckCRDResourceUniqueness, - convert.CheckOwnedCRDExistence, - convert.CheckPackageNameNotEmpty, + expectedValidationFns := []func(v1 *render.RegistryV1) []error{ + validators.CheckDeploymentSpecUniqueness, + validators.CheckCRDResourceUniqueness, + validators.CheckOwnedCRDExistence, + validators.CheckPackageNameNotEmpty, } - actualValidationFns := convert.RegistryV1BundleValidator + actualValidationFns := validators.RegistryV1BundleValidator require.Equal(t, len(expectedValidationFns), len(actualValidationFns)) for i := range expectedValidationFns { @@ -29,33 +31,17 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) { } } -func Test_BundleValidatorCallsAllValidationFnsInOrder(t *testing.T) { - actual := "" - validator := convert.BundleValidator{ - func(v1 *convert.RegistryV1) []error { - actual += "h" - return nil - }, - func(v1 *convert.RegistryV1) []error { - actual += "i" - return nil - }, - } - require.NoError(t, validator.Validate(nil)) - require.Equal(t, "hi", actual) -} - func Test_CheckDeploymentSpecUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *convert.RegistryV1 + bundle *render.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with unique deployment strategy spec names", - bundle: &convert.RegistryV1{ - CSV: makeCSV( - withStrategyDeploymentSpecs( + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-two"}, ), @@ -64,9 +50,9 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with duplicate deployment strategy spec names", - bundle: &convert.RegistryV1{ - CSV: makeCSV( - withStrategyDeploymentSpecs( + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-two"}, v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -78,9 +64,9 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { }, }, { name: "errors are ordered by deployment strategy spec name", - bundle: &convert.RegistryV1{ - CSV: makeCSV( - withStrategyDeploymentSpecs( + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-a"}, v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-b"}, v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-c"}, @@ -96,7 +82,7 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - errs := convert.CheckDeploymentSpecUniqueness(tc.bundle) + errs := validators.CheckDeploymentSpecUniqueness(tc.bundle) require.Equal(t, tc.expectedErrs, errs) }) } @@ -105,12 +91,12 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { func Test_CRDResourceUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *convert.RegistryV1 + bundle *render.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with unique custom resource definition resources", - bundle: &convert.RegistryV1{ + bundle: &render.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b.crd.something"}}, @@ -119,7 +105,7 @@ func Test_CRDResourceUniqueness(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with duplicate custom resource definition resources", - bundle: &convert.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ + bundle: &render.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, }}, @@ -128,7 +114,7 @@ func Test_CRDResourceUniqueness(t *testing.T) { }, }, { name: "errors are ordered by custom resource definition name", - bundle: &convert.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ + bundle: &render.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "c.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "c.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, @@ -141,7 +127,7 @@ func Test_CRDResourceUniqueness(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - err := convert.CheckCRDResourceUniqueness(tc.bundle) + err := validators.CheckCRDResourceUniqueness(tc.bundle) require.Equal(t, tc.expectedErrs, err) }) } @@ -150,18 +136,18 @@ func Test_CRDResourceUniqueness(t *testing.T) { func Test_CheckOwnedCRDExistence(t *testing.T) { for _, tc := range []struct { name string - bundle *convert.RegistryV1 + bundle *render.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with existing owned custom resource definition resources", - bundle: &convert.RegistryV1{ + bundle: &render.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b.crd.something"}}, }, - CSV: makeCSV( - withOwnedCRDs( + CSV: MakeCSV( + WithOwnedCRDs( v1alpha1.CRDDescription{Name: "a.crd.something"}, v1alpha1.CRDDescription{Name: "b.crd.something"}, ), @@ -170,10 +156,10 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with missing owned custom resource definition resources", - bundle: &convert.RegistryV1{ + bundle: &render.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{}, - CSV: makeCSV( - withOwnedCRDs(v1alpha1.CRDDescription{Name: "a.crd.something"}), + CSV: MakeCSV( + WithOwnedCRDs(v1alpha1.CRDDescription{Name: "a.crd.something"}), ), }, expectedErrs: []error{ @@ -181,10 +167,10 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { }, }, { name: "errors are ordered by owned custom resource definition name", - bundle: &convert.RegistryV1{ + bundle: &render.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{}, - CSV: makeCSV( - withOwnedCRDs( + CSV: MakeCSV( + WithOwnedCRDs( v1alpha1.CRDDescription{Name: "a.crd.something"}, v1alpha1.CRDDescription{Name: "c.crd.something"}, v1alpha1.CRDDescription{Name: "b.crd.something"}, @@ -199,7 +185,7 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - errs := convert.CheckOwnedCRDExistence(tc.bundle) + errs := validators.CheckOwnedCRDExistence(tc.bundle) require.Equal(t, tc.expectedErrs, errs) }) } @@ -208,47 +194,25 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { func Test_CheckPackageNameNotEmpty(t *testing.T) { for _, tc := range []struct { name string - bundle *convert.RegistryV1 + bundle *render.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with non-empty package name", - bundle: &convert.RegistryV1{ + bundle: &render.RegistryV1{ PackageName: "not-empty", }, }, { name: "rejects bundles with empty package name", - bundle: &convert.RegistryV1{}, + bundle: &render.RegistryV1{}, expectedErrs: []error{ errors.New("package name is empty"), }, }, } { t.Run(tc.name, func(t *testing.T) { - errs := convert.CheckPackageNameNotEmpty(tc.bundle) + errs := validators.CheckPackageNameNotEmpty(tc.bundle) require.Equal(t, tc.expectedErrs, errs) }) } } - -type csvOption func(version *v1alpha1.ClusterServiceVersion) - -func withStrategyDeploymentSpecs(strategyDeploymentSpecs ...v1alpha1.StrategyDeploymentSpec) csvOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = strategyDeploymentSpecs - } -} - -func withOwnedCRDs(crdDesc ...v1alpha1.CRDDescription) csvOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.CustomResourceDefinitions.Owned = crdDesc - } -} - -func makeCSV(opts ...csvOption) v1alpha1.ClusterServiceVersion { - csv := v1alpha1.ClusterServiceVersion{} - for _, opt := range opts { - opt(&csv) - } - return csv -} diff --git a/internal/operator-controller/rukpak/util/testing.go b/internal/operator-controller/rukpak/util/testing.go new file mode 100644 index 000000000..4dfc12976 --- /dev/null +++ b/internal/operator-controller/rukpak/util/testing.go @@ -0,0 +1,59 @@ +package util + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +type CSVOption func(version *v1alpha1.ClusterServiceVersion) + +//nolint:unparam +func WithName(name string) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Name = name + } +} + +func WithStrategyDeploymentSpecs(strategyDeploymentSpecs ...v1alpha1.StrategyDeploymentSpec) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = strategyDeploymentSpecs + } +} + +func WithAnnotations(annotations map[string]string) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Annotations = annotations + } +} + +func WithPermissions(permissions ...v1alpha1.StrategyDeploymentPermissions) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.InstallStrategy.StrategySpec.Permissions = permissions + } +} + +func WithClusterPermissions(permissions ...v1alpha1.StrategyDeploymentPermissions) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.InstallStrategy.StrategySpec.ClusterPermissions = permissions + } +} + +func WithOwnedCRDs(crdDesc ...v1alpha1.CRDDescription) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.CustomResourceDefinitions.Owned = crdDesc + } +} + +func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion { + csv := v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: "ClusterServiceVersion", + }, + } + for _, opt := range opts { + opt(&csv) + } + return csv +} diff --git a/internal/operator-controller/rukpak/util/testing_test.go b/internal/operator-controller/rukpak/util/testing_test.go new file mode 100644 index 000000000..17ca328f8 --- /dev/null +++ b/internal/operator-controller/rukpak/util/testing_test.go @@ -0,0 +1,188 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/require" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +func Test_MakeCSV(t *testing.T) { + csv := MakeCSV() + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + }, csv) +} + +func Test_MakeCSV_WithName(t *testing.T) { + csv := MakeCSV(WithName("some-name")) + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + }, + }, csv) +} + +func Test_MakeCSV_WithStrategyDeploymentSpecs(t *testing.T) { + csv := MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "spec-one", + }, + v1alpha1.StrategyDeploymentSpec{ + Name: "spec-two", + }, + ), + ) + + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + Spec: v1alpha1.ClusterServiceVersionSpec{ + InstallStrategy: v1alpha1.NamedInstallStrategy{ + StrategySpec: v1alpha1.StrategyDetailsDeployment{ + DeploymentSpecs: []v1alpha1.StrategyDeploymentSpec{ + { + Name: "spec-one", + }, + { + Name: "spec-two", + }, + }, + }, + }, + }, + }, csv) +} + +func Test_MakeCSV_WithPermissions(t *testing.T) { + csv := MakeCSV( + WithPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + }, + ), + ) + + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + Spec: v1alpha1.ClusterServiceVersionSpec{ + InstallStrategy: v1alpha1.NamedInstallStrategy{ + StrategySpec: v1alpha1.StrategyDetailsDeployment{ + Permissions: []v1alpha1.StrategyDeploymentPermissions{ + { + ServiceAccountName: "service-account", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"list", "watch"}, + }, + }, + }, + { + ServiceAccountName: "", + }, + }, + }, + }, + }, + }, csv) +} + +func Test_MakeCSV_WithClusterPermissions(t *testing.T) { + csv := MakeCSV( + WithClusterPermissions( + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "service-account", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"list", "watch"}, + }, + }, + }, + v1alpha1.StrategyDeploymentPermissions{ + ServiceAccountName: "", + }, + ), + ) + + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + Spec: v1alpha1.ClusterServiceVersionSpec{ + InstallStrategy: v1alpha1.NamedInstallStrategy{ + StrategySpec: v1alpha1.StrategyDetailsDeployment{ + ClusterPermissions: []v1alpha1.StrategyDeploymentPermissions{ + { + ServiceAccountName: "service-account", + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"list", "watch"}, + }, + }, + }, + { + ServiceAccountName: "", + }, + }, + }, + }, + }, + }, csv) +} + +func Test_MakeCSV_WithOwnedCRDs(t *testing.T) { + csv := MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "a.crd.something"}, + v1alpha1.CRDDescription{Name: "b.crd.something"}, + ), + ) + + require.Equal(t, v1alpha1.ClusterServiceVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterServiceVersion", + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + Spec: v1alpha1.ClusterServiceVersionSpec{ + CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ + Owned: []v1alpha1.CRDDescription{ + {Name: "a.crd.something"}, + {Name: "b.crd.something"}, + }, + }, + }, + }, csv) +} diff --git a/internal/operator-controller/rukpak/util/util.go b/internal/operator-controller/rukpak/util/util.go index 8bcfa194e..b6f64d20b 100644 --- a/internal/operator-controller/rukpak/util/util.go +++ b/internal/operator-controller/rukpak/util/util.go @@ -1,12 +1,22 @@ package util import ( + "fmt" "io" "k8s.io/cli-runtime/pkg/resource" "sigs.k8s.io/controller-runtime/pkg/client" ) +const maxNameLength = 63 + +func ObjectNameForBaseAndSuffix(base string, suffix string) string { + if len(base)+len(suffix) > maxNameLength { + base = base[:maxNameLength-len(suffix)-1] + } + return fmt.Sprintf("%s-%s", base, suffix) +} + func MergeMaps(maps ...map[string]string) map[string]string { out := map[string]string{} for _, m := range maps { diff --git a/internal/operator-controller/rukpak/util/util_test.go b/internal/operator-controller/rukpak/util/util_test.go index 8ccb4b74e..f5048abf1 100644 --- a/internal/operator-controller/rukpak/util/util_test.go +++ b/internal/operator-controller/rukpak/util/util_test.go @@ -16,6 +16,12 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) +func Test_ObjectNameForBaseAndSuffix(t *testing.T) { + name := util.ObjectNameForBaseAndSuffix("my.object.thing.has.a.really.really.really.really.really.long.name", "suffix") + require.Len(t, name, 63) + require.Equal(t, "my.object.thing.has.a.really.really.really.really.really-suffix", name) +} + func TestMergeMaps(t *testing.T) { tests := []struct { name string