split CSV and non-CSV RBAC using collector.Manifests method

internal/cmd/operator-sdk/generate/bundle/bundle.go

@@ -197,7 +197,7 @@ func (c bundleCmd) runManifests(cfg *config.Config) (err error) {
 		return fmt.Errorf("error generating ClusterServiceVersion: %v", err)
 	}
 
-	objs := col.GetNonCSVObjects()
+	objs := genutil.GetManifestObjects(col)
 	if c.stdout {
 		if err := genutil.WriteObjects(stdout, objs...); err != nil {
 			return err

internal/cmd/operator-sdk/generate/internal/manifests.go

@@ -0,0 +1,45 @@
+// Copyright 2020 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package genutil
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	"github.com/operator-framework/operator-sdk/internal/generate/collector"
+)
+
+// GetManifestObjects returns all objects to be written to a manifests directory from collector.Manifests.
+func GetManifestObjects(c *collector.Manifests) (objs []controllerutil.Object) {
+	// All CRDs passed in should be written.
+	for i := range c.V1CustomResourceDefinitions {
+		objs = append(objs, &c.V1CustomResourceDefinitions[i])
+	}
+	for i := range c.V1beta1CustomResourceDefinitions {
+		objs = append(objs, &c.V1beta1CustomResourceDefinitions[i])
+	}
+
+	// All ServiceAccounts passed in should be written.
+	for i := range c.ServiceAccounts {
+		objs = append(objs, &c.ServiceAccounts[i])
+	}
+
+	// RBAC objects that are not a part of the CSV should be written.
+	_, roleObjs := c.SplitCSVPermissionsObjects()
+	objs = append(objs, roleObjs...)
+	_, clusterRoleObjs := c.SplitCSVClusterPermissionsObjects()
+	objs = append(objs, clusterRoleObjs...)
+
+	return objs
+}

internal/cmd/operator-sdk/generate/packagemanifests/packagemanifests.go

@@ -189,7 +189,7 @@ func (c packagemanifestsCmd) run(cfg *config.Config) error {
 	}
 
 	if c.updateObjects {
-		objs := col.GetNonCSVObjects()
+		objs := genutil.GetManifestObjects(col)
 		if c.stdout {
 			if err := genutil.WriteObjects(stdout, objs...); err != nil {
 				return err

internal/generate/clusterserviceversion/clusterserviceversion_test.go

@@ -48,7 +48,7 @@ var (
 	goTestDataDir       = filepath.Join(testDataDir, "go")
 	goAPIsDir           = filepath.Join(goTestDataDir, "api")
 	goStaticDir         = filepath.Join(goTestDataDir, "static")
-	goBasicOperatorPath = filepath.Join(goStaticDir, "basic.operatory.yaml")
+	goBasicOperatorPath = filepath.Join(goStaticDir, "basic.operator.yaml")
 )
 
 var (

internal/generate/clusterserviceversion/clusterserviceversion_updaters.go

@@ -86,91 +86,85 @@ const defaultServiceAccountName = "default"
 // applyRoles applies Roles to strategy's permissions field by combining Roles bound to ServiceAccounts
 // into one set of permissions.
 func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl
-	// Create a set of all service accounts for deployments. These are the only CSV-relevant service accounts.
-	saNamesToRoleNames := make(map[string]map[string]struct{})
+	objs, _ := c.SplitCSVPermissionsObjects()
+	roleSet := make(map[string]*rbacv1.Role)
+	for i := range objs {
+		switch t := objs[i].(type) {
+		case *rbacv1.Role:
+			roleSet[t.GetName()] = t
+		}
+	}
+
+	saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions)
 	for _, dep := range c.Deployments {
 		saName := dep.Spec.Template.Spec.ServiceAccountName
 		if saName == "" {
 			saName = defaultServiceAccountName
 		}
-		saNamesToRoleNames[saName] = make(map[string]struct{})
+		saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
 	}
 
 	// Collect all role names by their corresponding service accounts via bindings. This lets us
 	// look up all service accounts a role is bound to and create one set of permissions per service account.
 	for _, binding := range c.RoleBindings {
-		roleRef := binding.RoleRef
-		if roleRef.Kind == "Role" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
-			for _, name := range getSubjectServiceAccountNames(binding.Subjects) {
-				if _, hasName := saNamesToRoleNames[name]; hasName {
-					saNamesToRoleNames[name][roleRef.Name] = struct{}{}
+		if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole {
+			for _, subject := range binding.Subjects {
+				if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" {
+					perm.Rules = append(perm.Rules, role.Rules...)
+					saToPermissions[subject.Name] = perm
 				}
 			}
 		}
 	}
 
 	// Apply relevant roles to each service account.
 	perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
-	for saName, roleNames := range saNamesToRoleNames {
-		p := operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
-		for _, role := range c.Roles {
-			if _, ok := roleNames[role.GetName()]; ok {
-				p.Rules = append(p.Rules, role.Rules...)
-			}
-		}
-		perms = append(perms, p)
+	for _, perm := range saToPermissions {
+		perms = append(perms, perm)
 	}
 	strategy.Permissions = perms
 }
 
 // applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field by combining ClusterRoles
 // bound to ServiceAccounts into one set of clusterPermissions.
 func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) { //nolint:dupl
-	// Create a set of all service accounts for deployments. These are the only CSV-relevant service accounts.
-	saNamesToClusterRoleNames := make(map[string]map[string]struct{})
+	objs, _ := c.SplitCSVClusterPermissionsObjects()
+	roleSet := make(map[string]*rbacv1.ClusterRole)
+	for i := range objs {
+		switch t := objs[i].(type) {
+		case *rbacv1.ClusterRole:
+			roleSet[t.GetName()] = t
+		}
+	}
+
+	saToPermissions := make(map[string]operatorsv1alpha1.StrategyDeploymentPermissions)
 	for _, dep := range c.Deployments {
 		saName := dep.Spec.Template.Spec.ServiceAccountName
 		if saName == "" {
 			saName = defaultServiceAccountName
 		}
-		saNamesToClusterRoleNames[saName] = make(map[string]struct{})
+		saToPermissions[saName] = operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
 	}
 
-	// Collect all cluster role names by their corresponding service accounts via bindings. This lets us
-	// look up all service accounts a cluster role is bound to and create one set of permissions per service account.
+	// Collect all role names by their corresponding service accounts via bindings. This lets us
+	// look up all service accounts a role is bound to and create one set of permissions per service account.
 	for _, binding := range c.ClusterRoleBindings {
-		roleRef := binding.RoleRef
-		if roleRef.Kind == "ClusterRole" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
-			for _, name := range getSubjectServiceAccountNames(binding.Subjects) {
-				if _, hasName := saNamesToClusterRoleNames[name]; hasName {
-					saNamesToClusterRoleNames[name][roleRef.Name] = struct{}{}
+		if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole {
+			for _, subject := range binding.Subjects {
+				if perm, hasSA := saToPermissions[subject.Name]; hasSA && subject.Kind == "ServiceAccount" {
+					perm.Rules = append(perm.Rules, role.Rules...)
+					saToPermissions[subject.Name] = perm
 				}
 			}
 		}
 	}
 
-	// Apply relevant cluster roles to each service account.
-	clusterPerms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
-	for saName, clusterRoleNames := range saNamesToClusterRoleNames {
-		p := operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
-		for _, clusterRole := range c.ClusterRoles {
-			if _, ok := clusterRoleNames[clusterRole.GetName()]; ok {
-				p.Rules = append(p.Rules, clusterRole.Rules...)
-			}
-		}
-		clusterPerms = append(clusterPerms, p)
-	}
-	strategy.ClusterPermissions = clusterPerms
-}
-
-// getSubjectServiceAccountNames returns a list of all ServiceAccount subject names.
-func getSubjectServiceAccountNames(subjects []rbacv1.Subject) (saNames []string) {
-	for _, subject := range subjects {
-		if subject.Kind == "ServiceAccount" {
-			saNames = append(saNames, subject.Name)
-		}
+	// Apply relevant roles to each service account.
+	perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
+	for _, perm := range saToPermissions {
+		perms = append(perms, perm)
 	}
-	return saNames
+	strategy.ClusterPermissions = perms
 }
 
 // applyDeployments updates strategy's deployments with the Deployments