This commit adds handling for extra RBAC objects present in

`generate <bundle|packagemanifests>` input. These objects
will be written to the resulting bundle. For now, only Roles,
RoleBindings, their Cluster equivalents, and ServiceAccounts
are written.

internal/cmd/operator-sdk/generate: write RBAC objects to stdout
or files named with object.Name + GVK

internal/generate/{collector/clusterserviceversion}: consider
(cluster) role bindings so CSV generator can assign the correct
service account names to roles

changelog/fragments/fix-csv-role-names.yaml

@@ -0,0 +1,9 @@
+entries:
+  - description: >
+      `generate <bundle|packagemanifests>` will write RBAC objects not bound to CSV deployment service accounts
+      to the resulting manifests directory.
+    kind: addition
+  - description: >
+      Fixed incorrect (cluster) role name assignments in generated CSVs
+      [#3600](https://github.com/operator-framework/operator-sdk/issues/3600).
+    kind: bugfix

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

@@ -197,13 +197,7 @@ func (c bundleCmd) runManifests(cfg *config.Config) (err error) {
 		return fmt.Errorf("error generating ClusterServiceVersion: %v", err)
 	}
 
-	var objs []interface{}
-	for _, crd := range col.V1CustomResourceDefinitions {
-		objs = append(objs, crd)
-	}
-	for _, crd := range col.V1beta1CustomResourceDefinitions {
-		objs = append(objs, crd)
-	}
+	objs := col.GetNonCSVObjects()
 	if c.stdout {
 		if err := genutil.WriteObjects(stdout, objs...); err != nil {
 			return err
@@ -331,7 +325,7 @@ func updateMetadata(cfg *config.Config, bundleRoot string) error {
 // writeDockerfileCOPYScorecardConfig checks if bundle.Dockerfile and scorecard config exists in
 // the operator project. If it does, it injects the scorecard configuration into bundle image.
 func writeDockerfileCOPYScorecardConfig(dockerfileName, localConfigDir string) error {
-	if isExist(bundle.DockerFile) && isExist(localConfigDir) {
+	if genutil.IsExist(bundle.DockerFile) && genutil.IsExist(localConfigDir) {
 		scorecardFileContent := fmt.Sprintf("COPY %s %s\n", localConfigDir, "/"+scorecard.DefaultConfigDir)
 		return projutil.RewriteFileContents(dockerfileName, "COPY", scorecardFileContent)
 	}
@@ -391,9 +385,3 @@ func rewriteAnnotations(bundleRoot string, kvs map[string]string) error {
 	}
 	return ioutil.WriteFile(annotationsPath, b, mode)
 }
-
-// isExist returns true if path exists.
-func isExist(path string) bool {
-	_, err := os.Stat(path)
-	return err == nil || os.IsExist(err)
-}

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

@@ -23,11 +23,11 @@ import (
 	"path/filepath"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/util/validation"
-
 	"github.com/blang/semver"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/apimachinery/pkg/util/validation"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/kubebuilder/pkg/model/config"
 	"sigs.k8s.io/yaml"
 )
@@ -57,7 +57,7 @@ func IsPipeReader() bool {
 }
 
 // WriteObjects writes each object in objs to w.
-func WriteObjects(w io.Writer, objs ...interface{}) error {
+func WriteObjects(w io.Writer, objs ...controllerutil.Object) error {
 	for _, obj := range objs {
 		if err := writeObject(w, obj); err != nil {
 			return err
@@ -67,7 +67,7 @@ func WriteObjects(w io.Writer, objs ...interface{}) error {
 }
 
 // WriteObjectsToFiles creates dir then writes each object in objs to a file in dir.
-func WriteObjectsToFiles(dir string, objs ...interface{}) error {
+func WriteObjectsToFiles(dir string, objs ...controllerutil.Object) error {
 	if err := os.MkdirAll(dir, 0755); err != nil {
 		return err
 	}
@@ -76,12 +76,12 @@ func WriteObjectsToFiles(dir string, objs ...interface{}) error {
 	for _, obj := range objs {
 		var fileName string
 		switch t := obj.(type) {
-		case apiextv1.CustomResourceDefinition:
+		case *apiextv1.CustomResourceDefinition:
 			fileName = makeCRDFileName(t.Spec.Group, t.Spec.Names.Plural)
-		case apiextv1beta1.CustomResourceDefinition:
+		case *apiextv1beta1.CustomResourceDefinition:
 			fileName = makeCRDFileName(t.Spec.Group, t.Spec.Names.Plural)
 		default:
-			return fmt.Errorf("unknown object type: %T", t)
+			fileName = makeObjectFileName(t)
 		}
 
 		if _, hasFile := seenFiles[fileName]; hasFile {
@@ -99,6 +99,14 @@ func makeCRDFileName(group, resource string) string {
 	return fmt.Sprintf("%s_%s.yaml", group, resource)
 }
 
+func makeObjectFileName(obj controllerutil.Object) string {
+	gvk := obj.GetObjectKind().GroupVersionKind()
+	if gvk.Group == "" {
+		return fmt.Sprintf("%s_%s_%s.yaml", obj.GetName(), gvk.Version, strings.ToLower(gvk.Kind))
+	}
+	return fmt.Sprintf("%s_%s_%s_%s.yaml", obj.GetName(), gvk.Group, gvk.Version, strings.ToLower(gvk.Kind))
+}
+
 // writeObjectToFile marshals crd to bytes and writes them to dir in file.
 func writeObjectToFile(dir string, obj interface{}, fileName string) error {
 	f, err := os.Create(filepath.Join(dir, fileName))
@@ -144,6 +152,15 @@ func IsNotExist(path string) bool {
 	return err != nil && errors.Is(err, os.ErrNotExist)
 }
 
+// IsExist returns true if path exists on disk.
+func IsExist(path string) bool {
+	if path == "" {
+		return false
+	}
+	_, err := os.Stat(path)
+	return err == nil || errors.Is(err, os.ErrExist)
+}
+
 // GetOperatorName returns the name of the operator which is by default the projectName attribute of the PROJECT file
 // However, the Go projects built with the plugin version v2 has not this attribute and then, for this case
 // the operatorName will be the current directory.

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

@@ -28,17 +28,17 @@ import (
 //nolint:maligned
 type packagemanifestsCmd struct {
 	// Common options.
-	projectName  string
-	version      string
-	fromVersion  string
-	inputDir     string
-	outputDir    string
-	kustomizeDir string
-	deployDir    string
-	crdsDir      string
-	updateCRDs   bool
-	stdout       bool
-	quiet        bool
+	projectName   string
+	version       string
+	fromVersion   string
+	inputDir      string
+	outputDir     string
+	kustomizeDir  string
+	deployDir     string
+	crdsDir       string
+	updateObjects bool
+	stdout        bool
+	quiet         bool
 
 	// Package manifest options.
 	channelName      string
@@ -97,7 +97,8 @@ func (c *packagemanifestsCmd) addFlagsTo(fs *pflag.FlagSet) {
 	fs.StringVar(&c.channelName, "channel", "", "Channel name for the generated package")
 	fs.BoolVar(&c.isDefaultChannel, "default-channel", false, "Use the channel passed to --channel "+
 		"as the package manifest file's default channel")
-	fs.BoolVar(&c.updateCRDs, "update-crds", true, "Update CustomResoureDefinition manifests in this package")
+	fs.BoolVar(&c.updateObjects, "update-objects", true, "Update non-CSV objects in this package, "+
+		"ex. CustomResoureDefinitions, Roles")
 	fs.BoolVarP(&c.quiet, "quiet", "q", false, "Run in quiet mode")
 	fs.BoolVar(&c.stdout, "stdout", false, "Write package to stdout")
 }

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

@@ -188,14 +188,8 @@ func (c packagemanifestsCmd) run(cfg *config.Config) error {
 		return fmt.Errorf("error generating ClusterServiceVersion: %v", err)
 	}
 
-	if c.updateCRDs {
-		var objs []interface{}
-		for _, crd := range col.V1CustomResourceDefinitions {
-			objs = append(objs, crd)
-		}
-		for _, crd := range col.V1beta1CustomResourceDefinitions {
-			objs = append(objs, crd)
-		}
+	if c.updateObjects {
+		objs := col.GetNonCSVObjects()
 		if c.stdout {
 			if err := genutil.WriteObjects(stdout, objs...); err != nil {
 				return err

internal/generate/clusterserviceversion/clusterserviceversion_test.go

@@ -16,7 +16,6 @@ package clusterserviceversion
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -31,7 +30,6 @@ import (
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-registry/pkg/lib/bundle"
 	appsv1 "k8s.io/api/apps/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/kubebuilder/pkg/model/config"
 	"sigs.k8s.io/yaml"
 
@@ -45,16 +43,12 @@ var (
 	testDataDir           = filepath.Join("..", "testdata")
 	csvDir                = filepath.Join(testDataDir, "clusterserviceversions")
 	csvBasesDir           = filepath.Join(csvDir, "bases")
-	csvNewLayoutBundleDir = filepath.Join(csvDir, "newlayout", "manifests")
-
-	// TODO: create a new testdata dir (top level?) that has both a "config"
-	// dir and a "deploy" dir that contains `kustomize build config/default`
-	// output to simulate actual manifest collection behavior. Using "config"
-	// directly is not standard behavior.
-	goTestDataDir = filepath.Join(testDataDir, "non-standard-layout")
-	goAPIsDir     = filepath.Join(goTestDataDir, "api")
-	goConfigDir   = filepath.Join(goTestDataDir, "config")
-	goCRDsDir     = filepath.Join(goConfigDir, "crds")
+	csvNewLayoutBundleDir = filepath.Join(csvDir, "output")
+
+	goTestDataDir       = filepath.Join(testDataDir, "go")
+	goAPIsDir           = filepath.Join(goTestDataDir, "api")
+	goStaticDir         = filepath.Join(goTestDataDir, "static")
+	goBasicOperatorPath = filepath.Join(goStaticDir, "basic.operatory.yaml")
 )
 
 var (
@@ -74,7 +68,7 @@ var (
 
 var _ = BeforeSuite(func() {
 	col = &collector.Manifests{}
-	Expect(col.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred())
+	collectManifestsFromFileHelper(col, goBasicOperatorPath)
 
 	cfg = readConfigHelper(goTestDataDir)
 
@@ -97,7 +91,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
 		buf = &bytes.Buffer{}
 	})
 
-	Describe("for the new Go project layout", func() {
+	Describe("for a Go project", func() {
 
 		Context("with correct Options", func() {
 
@@ -281,7 +275,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
 					getBase:      makeBaseGetter(newCSV),
 				}
 				// Update the input's and expected CSV's Deployment image.
-				Expect(g.Collector.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred())
+				collectManifestsFromFileHelper(g.Collector, goBasicOperatorPath)
 				Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1))
 				imageTag := "controller:v" + g.Version
 				modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag)
@@ -311,42 +305,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
 				Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version)))
 			})
 		})
-
-		Context("generate ClusterServiceVersion", func() {
-			It("should handle CRDs with core type name", func() {
-				g = Generator{
-					OperatorName: operatorName,
-					OperatorType: operatorType,
-					Version:      version,
-					Collector:    &collector.Manifests{},
-					config:       cfg,
-					getBase:      makeBaseGetter(newCSV),
-				}
-				err := filepath.Walk(goConfigDir, func(path string, info os.FileInfo, err error) error {
-					if err != nil || info.IsDir() {
-						return err
-					}
-					file, err := os.OpenFile(path, os.O_RDONLY, 0)
-					if err != nil {
-						return err
-					}
-					defer file.Close()
-					return g.Collector.UpdateFromReader(file)
-				})
-				Expect(err).ShouldNot(HaveOccurred(), "failed to read manifests")
-				Expect(len(g.Collector.V1beta1CustomResourceDefinitions)).Should(BeEquivalentTo(2))
-				Expect(len(g.Collector.CustomResources)).Should(BeEquivalentTo(2))
-
-				csv, err := g.generate()
-				Expect(err).ToNot(HaveOccurred())
-				Expect(csv.Annotations["alm-examples"]).ShouldNot(BeEquivalentTo("[]"))
-
-				crs := []unstructured.Unstructured{}
-				err = json.Unmarshal([]byte(csv.Annotations["alm-examples"]), &crs)
-				Expect(err).ShouldNot(HaveOccurred(), "failed to parse 'alm-examples' annotations")
-				Expect(crs).Should(ConsistOf(g.Collector.CustomResources), "custom resources shall match with CSV annotations")
-			})
-		})
 	})
 
 })
@@ -388,6 +346,13 @@ var _ = Describe("Generation requires interaction", func() {
 	})
 })
 
+func collectManifestsFromFileHelper(col *collector.Manifests, path string) {
+	f, err := os.Open(path)
+	ExpectWithOffset(1, err).ToNot(HaveOccurred())
+	ExpectWithOffset(1, col.UpdateFromReader(f)).ToNot(HaveOccurred())
+	ExpectWithOffset(1, f.Close()).Should(Succeed())
+}
+
 func readConfigHelper(dir string) *config.Config {
 	wd, err := os.Getwd()
 	ExpectWithOffset(1, err).ToNot(HaveOccurred())