generate: make CSV generator for Go GVKs package-aware

changelog/fragments/csv-gen-package-aware.yaml

@@ -0,0 +1,6 @@
+entries:
+  - description: >
+      For Go-based projects, `generate <bundle|packagemanifests>` subcommands now consider package and type names
+      when parsing Go API types files to generate a CSV's `owned.customresourcedefinitions`, such that types in
+      different packages and files will not overwrite each other.
+    kind: bugfix

internal/generate/clusterserviceversion/bases/definitions/ast.go

@@ -18,15 +18,90 @@ import (
 	"errors"
 	"fmt"
 	"go/ast"
+	"strconv"
 	"strings"
 
+	"sigs.k8s.io/controller-tools/pkg/crd"
+	"sigs.k8s.io/controller-tools/pkg/loader"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 )
 
-// getMarkedChildrenOfField collects all marked fields from type declarations starting at root in depth-first order.
-func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string][]*fieldInfo, error) {
+// importIdents maps import identifiers to a list of corresponding path and file containing that path.
+// For example, consider the set of 2 files, one containing 'import foo "my/foo"' and the other 'import foo "your/foo"'.
+// Then the map would be: map["foo"][]struct{{f: (file 1), path: "my/foo"},{f: (file 2), path: "your/foo"}}.
+type importIdents map[string][]struct {
+	f    *ast.File
+	path string
+}
+
+// newImportIdents creates an importIdents from all imports in pkg.
+func newImportIdents(pkg *loader.Package) (importIdents, error) {
+	importIDs := make(map[string][]struct {
+		f    *ast.File
+		path string
+	})
+	for _, file := range pkg.Syntax {
+		for _, impSpec := range file.Imports {
+			val, err := strconv.Unquote(impSpec.Path.Value)
+			if err != nil {
+				return nil, err
+			}
+			// Most imports are not locally named, so the real package name should be used.
+			var impName string
+			if imp, hasImp := pkg.Imports()[val]; hasImp {
+				impName = imp.Name
+			}
+			// impSpec.Name will not be empty for locally named imports
+			if impSpec.Name != nil {
+				impName = impSpec.Name.Name
+			}
+			importIDs[impName] = append(importIDs[impName], struct {
+				f    *ast.File
+				path string
+			}{file, val})
+		}
+	}
+	return importIDs, nil
+}
+
+// findPackagePathForSelExpr returns the package path corresponding to the package name used in expr if it exists in im.
+func (im importIdents) findPackagePathForSelExpr(expr *ast.SelectorExpr) (pkgPath string) {
+	// X contains the name being selected from.
+	xIdent, isIdent := expr.X.(*ast.Ident)
+	if !isIdent {
+		return ""
+	}
+	// Imports for all import statements where local import name == name being selected from.
+	imports, hasImports := im[xIdent.String()]
+	if !hasImports {
+		return ""
+	}
+
+	// Short-circuit if only one import.
+	if len(imports) == 1 {
+		return imports[0].path
+	}
+
+	// If multiple files contain the same local import name, check to see which file contains the selector expression.
+	for _, imp := range imports {
+		if imp.f.Pos() <= expr.Pos() && imp.f.End() >= expr.End() {
+			return imp.path
+		}
+	}
+	return ""
+}
+
+// getMarkedChildrenOfField collects all marked fields from type declarations starting at rootField in depth-first order.
+func (g generator) getMarkedChildrenOfField(rootPkg *loader.Package, rootField markers.FieldInfo) (map[string][]*fieldInfo, error) {
+	// Gather all types and imports needed to build the BFS tree.
+	rootPkg.NeedTypesInfo()
+	importIDs, err := newImportIdents(rootPkg)
+	if err != nil {
+		return nil, err
+	}
+
 	// ast.Inspect will not traverse into fields, so iteratively collect them and to check for markers.
-	nextFields := []*fieldInfo{{FieldInfo: root}}
+	nextFields := []*fieldInfo{{FieldInfo: rootField}}
 	markedFields := map[string][]*fieldInfo{}
 	for len(nextFields) > 0 {
 		fields := []*fieldInfo{}
@@ -36,49 +111,65 @@ func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string]
 				if n == nil {
 					return true
 				}
-				switch expr := n.(type) {
+
+				var info *markers.TypeInfo
+				var hasInfo bool
+				switch nt := n.(type) {
+				case *ast.SelectorExpr:
+					// Case of a type definition in an imported package.
+
+					pkgPath := importIDs.findPackagePathForSelExpr(nt)
+					if pkgPath == "" {
+						// Found no reference to pkgPath in any file.
+						return true
+					}
+					if pkg, hasImport := rootPkg.Imports()[loader.NonVendorPath(pkgPath)]; hasImport {
+						// Check if the field's type exists in the known types.
+						info, hasInfo = g.types[crd.TypeIdent{Package: pkg, Name: nt.Sel.Name}]
+					}
 				case *ast.Ident:
+					// Case of a local type definition.
+
 					// Only look at type names.
-					if expr.Obj == nil || expr.Obj.Kind != ast.Typ {
-						return true
+					if nt.Obj != nil && nt.Obj.Kind == ast.Typ {
+						// Check if the field's type exists in the known types.
+						info, hasInfo = g.types[crd.TypeIdent{Package: rootPkg, Name: nt.Name}]
 					}
-					// Check if the field's type exists in the known types.
-					info, hasInfo := g.types[expr.Name]
-					if !hasInfo {
+				}
+				if !hasInfo {
+					return true
+				}
+
+				// Add all child fields to the list to search next.
+				for _, finfo := range info.Fields {
+					segment, err := getPathSegmentForField(finfo)
+					if err != nil {
+						errs = append(errs, fmt.Errorf("error getting path from type %s field %s: %v", info.Name, finfo.Name, err))
 						return true
 					}
-					// Add all child fields to the list to search next.
-					for _, finfo := range info.Fields {
-						segment, err := getPathSegmentForField(finfo)
-						if err != nil {
-							errs = append(errs, fmt.Errorf("error getting path from type %s field %s: %v",
-								info.Name, finfo.Name, err),
-							)
-							return true
-						}
-						// Add extra information to the segment if it comes from a certain field type.
-						switch finfo.RawField.Type.(type) {
-						case (*ast.ArrayType):
-							// arrayFieldGroup case.
-							if segment != ignoredTag && segment != inlinedTag {
-								segment += "[0]"
-							}
-						}
-						// Create a new set of path segments using the parent's segments
-						// and add the field to the next fields to search.
-						parentSegments := make([]string, len(field.pathSegments), len(field.pathSegments)+1)
-						copy(parentSegments, field.pathSegments)
-						f := &fieldInfo{
-							FieldInfo:    finfo,
-							pathSegments: append(parentSegments, segment),
-						}
-						fields = append(fields, f)
-						// Marked fields get collected for the caller to parse.
-						if len(finfo.Markers) != 0 {
-							markedFields[info.Name] = append(markedFields[info.Name], f)
+					// Add extra information to the segment if it comes from a certain field type.
+					switch finfo.RawField.Type.(type) {
+					case *ast.ArrayType:
+						// arrayFieldGroup case.
+						if segment != ignoredTag && segment != inlinedTag {
+							segment += "[0]"
 						}
 					}
+					// Create a new set of path segments using the parent's segments
+					// and add the field to the next fields to search.
+					parentSegments := make([]string, len(field.pathSegments), len(field.pathSegments)+1)
+					copy(parentSegments, field.pathSegments)
+					f := &fieldInfo{
+						FieldInfo:    finfo,
+						pathSegments: append(parentSegments, segment),
+					}
+					fields = append(fields, f)
+					// Marked fields get collected for the caller to parse.
+					if len(finfo.Markers) != 0 {
+						markedFields[info.Name] = append(markedFields[info.Name], f)
+					}
 				}
+
 				return true
 			})
 			if err := fmtParseErrors(errs); err != nil {

internal/generate/clusterserviceversion/bases/definitions/crd.go

@@ -26,7 +26,9 @@ import (
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/version"
+	"sigs.k8s.io/controller-tools/pkg/crd"
 	crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
+	"sigs.k8s.io/controller-tools/pkg/loader"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 
 	"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
@@ -54,7 +56,7 @@ func getHalfBySep(s, sep string, half uint) string {
 
 // buildCRDDescriptionFromType builds a crdDescription for the Go API defined
 // by key from markers and type information in g.types.
-func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, int, error) {
+func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, typeIdent crd.TypeIdent, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, int, error) {
 
 	// Initialize the description.
 	description := v1alpha1.CRDDescription{
@@ -96,15 +98,15 @@ func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kind
 	}
 	sortResources(description.Resources)
 
-	specDescriptors, err := g.getTypedDescriptors(kindType, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
+	specDescriptors, err := g.getTypedDescriptors(typeIdent.Package, kindType, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
 	if err != nil {
 		return v1alpha1.CRDDescription{}, 0, err
 	}
 	for _, d := range specDescriptors {
 		description.SpecDescriptors = append(description.SpecDescriptors, d.(v1alpha1.SpecDescriptor))
 	}
 
-	statusDescriptors, err := g.getTypedDescriptors(kindType, reflect.TypeOf(v1alpha1.StatusDescriptor{}), status)
+	statusDescriptors, err := g.getTypedDescriptors(typeIdent.Package, kindType, reflect.TypeOf(v1alpha1.StatusDescriptor{}), status)
 	if err != nil {
 		return v1alpha1.CRDDescription{}, 0, err
 	}
@@ -115,15 +117,15 @@ func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kind
 	return description, descriptionOrder, nil
 }
 
-func (g generator) getTypedDescriptors(kindType *markers.TypeInfo, t reflect.Type, descType string) ([]interface{}, error) {
+func (g generator) getTypedDescriptors(pkg *loader.Package, kindType *markers.TypeInfo, t reflect.Type, descType string) ([]interface{}, error) {
 	// Find child in the kind type.
 	child, err := findChildForDescType(kindType, descType)
 	if err != nil {
 		return nil, err
 	}
 
 	// Find annotated fields of child and parse them into descriptors.
-	markedFields, err := g.getMarkedChildrenOfField(child)
+	markedFields, err := g.getMarkedChildrenOfField(pkg, child)
 	if err != nil {
 		return nil, err
 	}