Skip to content

Commit 9c178b7

Browse files
openshift-cherrypick-robotestroz
openshift-cherrypick-robot
and
estroz
authored
[v1.3.x] generate: make CSV generator for Go GVKs package-aware (#4480)
Signed-off-by: Eric Stroczynski <[email protected]> Co-authored-by: Eric Stroczynski <[email protected]>
1 parent 9f7e16d commit 9c178b7

File tree

12 files changed

+349
-218
lines changed

12 files changed

+349
-218
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
entries:
2+
- description: >
3+
For Go-based projects, `generate <bundle|packagemanifests>` subcommands now consider package and type names
4+
when parsing Go API types files to generate a CSV's `owned.customresourcedefinitions`, such that types in
5+
different packages and files will not overwrite each other.
6+
kind: bugfix

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

Lines changed: 129 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,90 @@ import (
1818
"errors"
1919
"fmt"
2020
"go/ast"
21+
"strconv"
2122
"strings"
2223

24+
"sigs.k8s.io/controller-tools/pkg/crd"
25+
"sigs.k8s.io/controller-tools/pkg/loader"
2326
"sigs.k8s.io/controller-tools/pkg/markers"
2427
)
2528

26-
// getMarkedChildrenOfField collects all marked fields from type declarations starting at root in depth-first order.
27-
func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string][]*fieldInfo, error) {
29+
// importIdents maps import identifiers to a list of corresponding path and file containing that path.
30+
// For example, consider the set of 2 files, one containing 'import foo "my/foo"' and the other 'import foo "your/foo"'.
31+
// Then the map would be: map["foo"][]struct{{f: (file 1), path: "my/foo"},{f: (file 2), path: "your/foo"}}.
32+
type importIdents map[string][]struct {
33+
f *ast.File
34+
path string
35+
}
36+
37+
// newImportIdents creates an importIdents from all imports in pkg.
38+
func newImportIdents(pkg *loader.Package) (importIdents, error) {
39+
importIDs := make(map[string][]struct {
40+
f *ast.File
41+
path string
42+
})
43+
for _, file := range pkg.Syntax {
44+
for _, impSpec := range file.Imports {
45+
val, err := strconv.Unquote(impSpec.Path.Value)
46+
if err != nil {
47+
return nil, err
48+
}
49+
// Most imports are not locally named, so the real package name should be used.
50+
var impName string
51+
if imp, hasImp := pkg.Imports()[val]; hasImp {
52+
impName = imp.Name
53+
}
54+
// impSpec.Name will not be empty for locally named imports
55+
if impSpec.Name != nil {
56+
impName = impSpec.Name.Name
57+
}
58+
importIDs[impName] = append(importIDs[impName], struct {
59+
f *ast.File
60+
path string
61+
}{file, val})
62+
}
63+
}
64+
return importIDs, nil
65+
}
66+
67+
// findPackagePathForSelExpr returns the package path corresponding to the package name used in expr if it exists in im.
68+
func (im importIdents) findPackagePathForSelExpr(expr *ast.SelectorExpr) (pkgPath string) {
69+
// X contains the name being selected from.
70+
xIdent, isIdent := expr.X.(*ast.Ident)
71+
if !isIdent {
72+
return ""
73+
}
74+
// Imports for all import statements where local import name == name being selected from.
75+
imports, hasImports := im[xIdent.String()]
76+
if !hasImports {
77+
return ""
78+
}
79+
80+
// Short-circuit if only one import.
81+
if len(imports) == 1 {
82+
return imports[0].path
83+
}
84+
85+
// If multiple files contain the same local import name, check to see which file contains the selector expression.
86+
for _, imp := range imports {
87+
if imp.f.Pos() <= expr.Pos() && imp.f.End() >= expr.End() {
88+
return imp.path
89+
}
90+
}
91+
return ""
92+
}
93+
94+
// getMarkedChildrenOfField collects all marked fields from type declarations starting at rootField in depth-first order.
95+
func (g generator) getMarkedChildrenOfField(rootPkg *loader.Package, rootField markers.FieldInfo) (map[string][]*fieldInfo, error) {
96+
// Gather all types and imports needed to build the BFS tree.
97+
rootPkg.NeedTypesInfo()
98+
importIDs, err := newImportIdents(rootPkg)
99+
if err != nil {
100+
return nil, err
101+
}
102+
28103
// ast.Inspect will not traverse into fields, so iteratively collect them and to check for markers.
29-
nextFields := []*fieldInfo{{FieldInfo: root}}
104+
nextFields := []*fieldInfo{{FieldInfo: rootField}}
30105
markedFields := map[string][]*fieldInfo{}
31106
for len(nextFields) > 0 {
32107
fields := []*fieldInfo{}
@@ -36,49 +111,65 @@ func (g generator) getMarkedChildrenOfField(root markers.FieldInfo) (map[string]
36111
if n == nil {
37112
return true
38113
}
39-
switch expr := n.(type) {
114+
115+
var info *markers.TypeInfo
116+
var hasInfo bool
117+
switch nt := n.(type) {
118+
case *ast.SelectorExpr:
119+
// Case of a type definition in an imported package.
120+
121+
pkgPath := importIDs.findPackagePathForSelExpr(nt)
122+
if pkgPath == "" {
123+
// Found no reference to pkgPath in any file.
124+
return true
125+
}
126+
if pkg, hasImport := rootPkg.Imports()[loader.NonVendorPath(pkgPath)]; hasImport {
127+
// Check if the field's type exists in the known types.
128+
info, hasInfo = g.types[crd.TypeIdent{Package: pkg, Name: nt.Sel.Name}]
129+
}
40130
case *ast.Ident:
131+
// Case of a local type definition.
132+
41133
// Only look at type names.
42-
if expr.Obj == nil || expr.Obj.Kind != ast.Typ {
43-
return true
134+
if nt.Obj != nil && nt.Obj.Kind == ast.Typ {
135+
// Check if the field's type exists in the known types.
136+
info, hasInfo = g.types[crd.TypeIdent{Package: rootPkg, Name: nt.Name}]
44137
}
45-
// Check if the field's type exists in the known types.
46-
info, hasInfo := g.types[expr.Name]
47-
if !hasInfo {
138+
}
139+
if !hasInfo {
140+
return true
141+
}
142+
143+
// Add all child fields to the list to search next.
144+
for _, finfo := range info.Fields {
145+
segment, err := getPathSegmentForField(finfo)
146+
if err != nil {
147+
errs = append(errs, fmt.Errorf("error getting path from type %s field %s: %v", info.Name, finfo.Name, err))
48148
return true
49149
}
50-
// Add all child fields to the list to search next.
51-
for _, finfo := range info.Fields {
52-
segment, err := getPathSegmentForField(finfo)
53-
if err != nil {
54-
errs = append(errs, fmt.Errorf("error getting path from type %s field %s: %v",
55-
info.Name, finfo.Name, err),
56-
)
57-
return true
58-
}
59-
// Add extra information to the segment if it comes from a certain field type.
60-
switch finfo.RawField.Type.(type) {
61-
case (*ast.ArrayType):
62-
// arrayFieldGroup case.
63-
if segment != ignoredTag && segment != inlinedTag {
64-
segment += "[0]"
65-
}
66-
}
67-
// Create a new set of path segments using the parent's segments
68-
// and add the field to the next fields to search.
69-
parentSegments := make([]string, len(field.pathSegments), len(field.pathSegments)+1)
70-
copy(parentSegments, field.pathSegments)
71-
f := &fieldInfo{
72-
FieldInfo: finfo,
73-
pathSegments: append(parentSegments, segment),
74-
}
75-
fields = append(fields, f)
76-
// Marked fields get collected for the caller to parse.
77-
if len(finfo.Markers) != 0 {
78-
markedFields[info.Name] = append(markedFields[info.Name], f)
150+
// Add extra information to the segment if it comes from a certain field type.
151+
switch finfo.RawField.Type.(type) {
152+
case *ast.ArrayType:
153+
// arrayFieldGroup case.
154+
if segment != ignoredTag && segment != inlinedTag {
155+
segment += "[0]"
79156
}
80157
}
158+
// Create a new set of path segments using the parent's segments
159+
// and add the field to the next fields to search.
160+
parentSegments := make([]string, len(field.pathSegments), len(field.pathSegments)+1)
161+
copy(parentSegments, field.pathSegments)
162+
f := &fieldInfo{
163+
FieldInfo: finfo,
164+
pathSegments: append(parentSegments, segment),
165+
}
166+
fields = append(fields, f)
167+
// Marked fields get collected for the caller to parse.
168+
if len(finfo.Markers) != 0 {
169+
markedFields[info.Name] = append(markedFields[info.Name], f)
170+
}
81171
}
172+
82173
return true
83174
})
84175
if err := fmtParseErrors(errs); err != nil {

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ import (
2626
"github.com/operator-framework/api/pkg/operators/v1alpha1"
2727
"k8s.io/apimachinery/pkg/runtime/schema"
2828
"k8s.io/apimachinery/pkg/version"
29+
"sigs.k8s.io/controller-tools/pkg/crd"
2930
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
31+
"sigs.k8s.io/controller-tools/pkg/loader"
3032
"sigs.k8s.io/controller-tools/pkg/markers"
3133

3234
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
@@ -54,7 +56,7 @@ func getHalfBySep(s, sep string, half uint) string {
5456

5557
// buildCRDDescriptionFromType builds a crdDescription for the Go API defined
5658
// by key from markers and type information in g.types.
57-
func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, int, error) {
59+
func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, typeIdent crd.TypeIdent, kindType *markers.TypeInfo) (v1alpha1.CRDDescription, int, error) {
5860

5961
// Initialize the description.
6062
description := v1alpha1.CRDDescription{
@@ -96,15 +98,15 @@ func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kind
9698
}
9799
sortResources(description.Resources)
98100

99-
specDescriptors, err := g.getTypedDescriptors(kindType, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
101+
specDescriptors, err := g.getTypedDescriptors(typeIdent.Package, kindType, reflect.TypeOf(v1alpha1.SpecDescriptor{}), spec)
100102
if err != nil {
101103
return v1alpha1.CRDDescription{}, 0, err
102104
}
103105
for _, d := range specDescriptors {
104106
description.SpecDescriptors = append(description.SpecDescriptors, d.(v1alpha1.SpecDescriptor))
105107
}
106108

107-
statusDescriptors, err := g.getTypedDescriptors(kindType, reflect.TypeOf(v1alpha1.StatusDescriptor{}), status)
109+
statusDescriptors, err := g.getTypedDescriptors(typeIdent.Package, kindType, reflect.TypeOf(v1alpha1.StatusDescriptor{}), status)
108110
if err != nil {
109111
return v1alpha1.CRDDescription{}, 0, err
110112
}
@@ -115,15 +117,15 @@ func (g generator) buildCRDDescriptionFromType(gvk schema.GroupVersionKind, kind
115117
return description, descriptionOrder, nil
116118
}
117119

118-
func (g generator) getTypedDescriptors(kindType *markers.TypeInfo, t reflect.Type, descType string) ([]interface{}, error) {
120+
func (g generator) getTypedDescriptors(pkg *loader.Package, kindType *markers.TypeInfo, t reflect.Type, descType string) ([]interface{}, error) {
119121
// Find child in the kind type.
120122
child, err := findChildForDescType(kindType, descType)
121123
if err != nil {
122124
return nil, err
123125
}
124126

125127
// Find annotated fields of child and parse them into descriptors.
126-
markedFields, err := g.getMarkedChildrenOfField(child)
128+
markedFields, err := g.getMarkedChildrenOfField(pkg, child)
127129
if err != nil {
128130
return nil, err
129131
}

0 commit comments

Comments
 (0)