diff --git a/Makefile b/Makefile index 55f84a43006..aae8017e771 100644 --- a/Makefile +++ b/Makefile @@ -110,7 +110,7 @@ container: clean-e2e: kubectl delete crds --all kubectl delete apiservices.apiregistration.k8s.io v1alpha1.packages.apps.redhat.com - kubectl delete -f test/e2e/resources/0000_30_00-namespace.yaml + kubectl delete -f test/e2e/resources/0000_50_00-namespace.yaml clean: @rm -rf cover.out diff --git a/pkg/controller/registry/resolver/rbac.go b/pkg/controller/registry/resolver/rbac.go index 607e119d8ab..d257d177eb8 100644 --- a/pkg/controller/registry/resolver/rbac.go +++ b/pkg/controller/registry/resolver/rbac.go @@ -6,12 +6,17 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/storage/names" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" ) +var generateName = func(base string) string { + return names.SimpleNameGenerator.GenerateName(base + "-") +} + type OperatorPermissions struct { ServiceAccount *corev1.ServiceAccount Roles []*rbacv1.Role @@ -63,7 +68,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri } // Resolve Permissions - for i, permission := range strategyDetailsDeployment.Permissions { + for _, permission := range strategyDetailsDeployment.Permissions { // Create ServiceAccount if necessary if _, ok := permissions[permission.ServiceAccountName]; !ok { serviceAccount := &corev1.ServiceAccount{} @@ -76,7 +81,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri // Create Role role := &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", csv.GetName(), i), + Name: generateName(csv.GetName()), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, Labels: ownerutil.OwnerLabel(csv), @@ -88,7 +93,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri // Create RoleBinding roleBinding := &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName), + Name: generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, Labels: ownerutil.OwnerLabel(csv), @@ -107,7 +112,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri } // Resolve ClusterPermissions as StepResources - for i, permission := range strategyDetailsDeployment.ClusterPermissions { + for _, permission := range strategyDetailsDeployment.ClusterPermissions { // Create ServiceAccount if necessary if _, ok := permissions[permission.ServiceAccountName]; !ok { serviceAccount := &corev1.ServiceAccount{} @@ -120,7 +125,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri // Create ClusterRole role := &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%d", csv.GetName(), i), + Name: generateName(csv.GetName()), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, Labels: ownerutil.OwnerLabel(csv), }, @@ -131,7 +136,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri // Create ClusterRoleBinding roleBinding := &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName), + Name: generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)), Namespace: csv.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)}, Labels: ownerutil.OwnerLabel(csv), diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 98882bbe669..d4da42dc8b1 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -348,6 +348,10 @@ func TestNamespaceResolver(t *testing.T) { } func TestNamespaceResolverRBAC(t *testing.T) { + generateName = func(base string) string { + return "a" + } + namespace := "catsrc-namespace" catalog := CatalogKey{"catsrc", namespace} @@ -363,7 +367,7 @@ func TestNamespaceResolverRBAC(t *testing.T) { }, }, } - + bundle := bundleWithPermissions("a.v1", "a", "alpha", "", nil, nil, nil, nil, simplePermissions, simplePermissions) type out struct { steps [][]*v1alpha1.Step subs []*v1alpha1.Subscription @@ -380,12 +384,10 @@ func TestNamespaceResolverRBAC(t *testing.T) { clusterState: []runtime.Object{ newSub(namespace, "a", "alpha", catalog), }, - bundlesInCatalog: []*opregistry.Bundle{ - bundleWithPermissions("a.v1", "a", "alpha", "", nil, nil, nil, nil, simplePermissions, simplePermissions), - }, + bundlesInCatalog: []*opregistry.Bundle{bundle}, out: out{ steps: [][]*v1alpha1.Step{ - bundleSteps(bundleWithPermissions("a.v1", "a", "alpha", "", nil, nil, nil, nil, simplePermissions, simplePermissions), namespace, catalog), + bundleSteps(bundle, namespace, catalog), }, subs: []*v1alpha1.Subscription{ updatedSub(namespace, "a.v1", "a", "alpha", catalog), diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go index c00f2230e01..55aaaa145f6 100644 --- a/test/e2e/installplan_e2e_test.go +++ b/test/e2e/installplan_e2e_test.go @@ -687,13 +687,13 @@ func TestCreateInstallPlanWithPermissions(t *testing.T) { // Expect correct RBAC resources to be resolved and created expectedSteps := map[registry.ResourceKey]struct{}{ - registry.ResourceKey{Name: crd.Name, Kind: "CustomResourceDefinition"}: {}, - registry.ResourceKey{Name: stableCSVName, Kind: "ClusterServiceVersion"}: {}, - registry.ResourceKey{Name: serviceAccountName, Kind: "ServiceAccount"}: {}, - registry.ResourceKey{Name: fmt.Sprintf("%s-0", stableCSVName), Kind: "Role"}: {}, - registry.ResourceKey{Name: fmt.Sprintf("%s-0-%s", stableCSVName, serviceAccountName), Kind: "RoleBinding"}: {}, - registry.ResourceKey{Name: fmt.Sprintf("%s-0", stableCSVName), Kind: "ClusterRole"}: {}, - registry.ResourceKey{Name: fmt.Sprintf("%s-0-%s", stableCSVName, serviceAccountName), Kind: "ClusterRoleBinding"}: {}, + registry.ResourceKey{Name: crd.Name, Kind: "CustomResourceDefinition"}: {}, + registry.ResourceKey{Name: stableCSVName, Kind: "ClusterServiceVersion"}: {}, + registry.ResourceKey{Name: serviceAccountName, Kind: "ServiceAccount"}: {}, + registry.ResourceKey{Name: stableCSVName, Kind: "Role"}: {}, + registry.ResourceKey{Name: stableCSVName, Kind: "RoleBinding"}: {}, + registry.ResourceKey{Name: stableCSVName, Kind: "ClusterRole"}: {}, + registry.ResourceKey{Name: stableCSVName, Kind: "ClusterRoleBinding"}: {}, } require.Equal(t, len(expectedSteps), len(fetchedInstallPlan.Status.Plan), "number of expected steps does not match installed") @@ -703,15 +703,19 @@ func TestCreateInstallPlanWithPermissions(t *testing.T) { Name: step.Resource.Name, Kind: step.Resource.Kind, } - _, ok := expectedSteps[key] - require.True(t, ok) - - // Remove the entry from the expected steps set (to ensure no duplicates in resolved plan) - delete(expectedSteps, key) + for expected := range expectedSteps { + if expected == key { + delete(expectedSteps, expected) + } else if strings.HasPrefix(key.Name, expected.Name) && key.Kind == expected.Kind { + delete(expectedSteps, expected) + } else { + t.Logf("%v, %v: %v && %v", key, expected, strings.HasPrefix(key.Name, expected.Name), key.Kind == expected.Kind) + } + } } // Should have removed every matching step - require.Equal(t, 0, len(expectedSteps), "Actual resource steps do not match expected") + require.Equal(t, 0, len(expectedSteps), "Actual resource steps do not match expected: %#v", expectedSteps) }