allow "nested fields" to be configured

When designing the `pkg/generate/config.FieldConfig` object, we really
only thought about (and tested for) "top-level fields". In other words,
fields that were on the CR's `Spec` or `Status` objects.

However, as we've added more configurability to the code-generator
regarding fields -- including marking fields as printable or secrets --
we're now limited substantially by our initial implementation that only
allows referring to top-level fields.

For example, we cannot mark a field as being a secret field if that
field is a subfield of one of these top-level fields. Concretely, this
means that with the existing generator configuration, we cannot support
configuring AmazonMQ's `Broker` CRD's `Spec.Users.Password` field as a
SecretKeyReference because `Password` is not a top-level field. It is
instead a field *of* a top-level field -- a "nested field" -- and
therefore our simplistic configuration system isn't able to tell the
code generator to replace the `string` Go type with
`*ackv1alpha1.SecretKeyReference` like it [does][secref-inject] for
top-level fields.

[secref-inject]: https://github.com/aws-controllers-k8s/code-generator/blob/94186d92e778792ccba11b5db3478e037256b36b/pkg/model/field.go#L61-L64

The solution to this dilemma is contained in this PR and involves
reworking our generator model in a couple ways:

1) The `pkg/model.CRD` struct now has a `Fields` attribute to go along
   with the existing `SpecFields` and `StatusFields` attributes.
   `CRD.Fields` is a `map[string]*Field` like `SpecFields` and
   `StatusFields`, but unlike those attributes, the `Fields` map keys
   are field *paths* and not the original aws-sdk-go Shape member names.
   When inspecting the AWS service API model as we do in
   `pkg/generate.Generator:GetCRDs()`, we now construct a field path for
   each `pkg/model.Field` object that corresponds to "where" in the CR the
   field is. For example, `Spec.Name` or `Status.BrokerInstances`

2) The `pkg/generate.Generator` has been updated with a
   `processNestedFields()` method to populate the new field-path-keyed
   `CRD.Fields` map with nested fields, making it possible to refer in our
   generator config, by field path, to fields that are not directly in the
   Status or Spec object.

A following patch will update the `Generator.GetTypeDefs()` method to
look up field configuration by this new field-path-based map and replace
`string` with `*ackv1alpha1.SecretKeyReference` when the field is a
secret field (Issue aws-controllers-k8s/community#743).

Issue aws-controllers-k8s/community#748

go.mod

@@ -17,4 +17,5 @@ require (
 	github.com/stretchr/testify v1.7.0
 	golang.org/x/mod v0.4.1
 	k8s.io/apimachinery v0.20.1
+	k8s.io/utils v0.0.0-20201110183641-67b214c5f920
 )

pkg/generate/generator.go

@@ -226,6 +226,10 @@ func (g *Generator) GetCRDs() ([]*ackmodel.CRD, error) {
 	sort.Slice(crds, func(i, j int) bool {
 		return crds[i].Names.Camel < crds[j].Names.Camel
 	})
+	// This is the place that we build out the CRD.Fields map with
+	// `pkg/model.Field` objects that represent the non-top-level Spec and
+	// Status fields.
+	g.processNestedFields(crds)
 	g.crds = crds
 	return crds, nil
 }
@@ -401,6 +405,145 @@ func (g *Generator) GetTypeDefs() ([]*ackmodel.TypeDef, map[string]string, error
 	return tdefs, timports, nil
 }
 
+// processNestedFields is responsible for walking all of the CRDs' Spec and
+// Status fields' Shape objects and adding `pkg/model.Field` objects for all
+// nested fields along with that `Field`'s `Config` object that allows us to
+// determine if the TypeDef associated with that nested field should have its
+// data type overridden (e.g. for SecretKeyReferences)
+func (g *Generator) processNestedFields(crds []*ackmodel.CRD) {
+	for _, crd := range crds {
+		for _, field := range crd.SpecFields {
+			g.processNestedField(crd, field)
+		}
+		for _, field := range crd.StatusFields {
+			g.processNestedField(crd, field)
+		}
+	}
+}
+
+// processNestedField processes any nested fields (non-scalar fields associated
+// with the Spec and Status objects)
+func (g *Generator) processNestedField(
+	crd *ackmodel.CRD,
+	field *ackmodel.Field,
+) {
+	if field.ShapeRef == nil && (field.FieldConfig == nil || !field.FieldConfig.IsAttribute) {
+		fmt.Printf(
+			"WARNING: Field %s:%s has nil ShapeRef and is not defined as an Attribute-based Field!\n",
+			crd.Names.Original,
+			field.Names.Original,
+		)
+		return
+	}
+	if field.ShapeRef != nil {
+		fieldShape := field.ShapeRef.Shape
+		fieldType := fieldShape.Type
+		switch fieldType {
+		case "structure":
+			g.processNestedStructField(crd, field.Path+".", field)
+		case "list":
+			g.processNestedListField(crd, field.Path+"..", field)
+		case "map":
+			g.processNestedMapField(crd, field.Path+"..", field)
+		}
+	}
+	// TODO(jaypipes): Handle Attribute-based fields...
+}
+
+// processNestedStructField recurses through the members of a nested field that
+// is a struct type and adds any Field objects to the supplied CRD.
+func (g *Generator) processNestedStructField(
+	crd *ackmodel.CRD,
+	baseFieldPath string,
+	baseField *ackmodel.Field,
+) {
+	fieldConfigs := crd.Config().ResourceFields(crd.Names.Original)
+	baseFieldShape := baseField.ShapeRef.Shape
+	for memberName, memberRef := range baseFieldShape.MemberRefs {
+		memberNames := names.New(memberName)
+		memberShape := memberRef.Shape
+		memberShapeType := memberShape.Type
+		fieldPath := baseFieldPath + memberNames.Camel
+		fieldConfig := fieldConfigs[fieldPath]
+		field := ackmodel.NewField(crd, fieldPath, memberNames, memberRef, fieldConfig)
+		switch memberShapeType {
+		case "structure":
+			g.processNestedStructField(crd, fieldPath+".", field)
+		case "list":
+			g.processNestedListField(crd, fieldPath+"..", field)
+		case "map":
+			g.processNestedMapField(crd, fieldPath+"..", field)
+		}
+		crd.Fields[fieldPath] = field
+	}
+}
+
+// processNestedListField recurses through the members of a nested field that
+// is a list type that has a struct element type and adds any Field objects to
+// the supplied CRD.
+func (g *Generator) processNestedListField(
+	crd *ackmodel.CRD,
+	baseFieldPath string,
+	baseField *ackmodel.Field,
+) {
+	baseFieldShape := baseField.ShapeRef.Shape
+	elementFieldShape := baseFieldShape.MemberRef.Shape
+	if elementFieldShape.Type != "structure" {
+		return
+	}
+	fieldConfigs := crd.Config().ResourceFields(crd.Names.Original)
+	for memberName, memberRef := range elementFieldShape.MemberRefs {
+		memberNames := names.New(memberName)
+		memberShape := memberRef.Shape
+		memberShapeType := memberShape.Type
+		fieldPath := baseFieldPath + memberNames.Camel
+		fieldConfig := fieldConfigs[fieldPath]
+		field := ackmodel.NewField(crd, fieldPath, memberNames, memberRef, fieldConfig)
+		switch memberShapeType {
+		case "structure":
+			g.processNestedStructField(crd, fieldPath+".", field)
+		case "list":
+			g.processNestedListField(crd, fieldPath+"..", field)
+		case "map":
+			g.processNestedMapField(crd, fieldPath+"..", field)
+		}
+		crd.Fields[fieldPath] = field
+	}
+}
+
+// processNestedMapField recurses through the members of a nested field that
+// is a map type that has a struct value type and adds any Field objects to
+// the supplied CRD.
+func (g *Generator) processNestedMapField(
+	crd *ackmodel.CRD,
+	baseFieldPath string,
+	baseField *ackmodel.Field,
+) {
+	baseFieldShape := baseField.ShapeRef.Shape
+	valueFieldShape := baseFieldShape.ValueRef.Shape
+	if valueFieldShape.Type != "structure" {
+		return
+	}
+	fieldConfigs := crd.Config().ResourceFields(crd.Names.Original)
+	for memberName, memberRef := range valueFieldShape.MemberRefs {
+		memberNames := names.New(memberName)
+		memberShape := memberRef.Shape
+		memberShapeType := memberShape.Type
+		fieldPath := baseFieldPath + memberNames.Camel
+		fieldConfig := fieldConfigs[fieldPath]
+		field := ackmodel.NewField(crd, fieldPath, memberNames, memberRef, fieldConfig)
+		switch memberShapeType {
+		case "structure":
+			g.processNestedStructField(crd, fieldPath+".", field)
+		case "list":
+			g.processNestedListField(crd, fieldPath+"..", field)
+		case "map":
+			g.processNestedMapField(crd, fieldPath+"..", field)
+		}
+		crd.Fields[fieldPath] = field
+	}
+}
+
 // GetEnumDefs returns a slice of pointers to `ackmodel.EnumDef` structs which
 // represent string fields whose value is constrained to one or more specific
 // string values.

pkg/generate/mq_test.go

@@ -0,0 +1,46 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	 http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file 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 generate_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/aws-controllers-k8s/code-generator/pkg/testutil"
+)
+
+func TestMQ_Broker(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewGeneratorForService(t, "mq")
+
+	crds, err := g.GetCRDs()
+	require.Nil(err)
+
+	crd := getCRDByName("Broker", crds)
+	require.NotNil(crd)
+
+	// We want to verify that the `Password` field of the `Spec.Users` field
+	// (which is a `[]*User` type) is findable in the CRD's Fields collection
+	// by the path `Spec.Users..Password` and that the FieldConfig associated
+	// with this Field is marked as a SecretKeyReference.
+	passFieldPath := "Users..Password"
+	passField, found := crd.Fields[passFieldPath]
+	require.True(found)
+	require.NotNil(passField.FieldConfig)
+	assert.True(passField.FieldConfig.IsSecret)
+}

pkg/generate/testdata/models/apis/mq/0000-00-00/generator.yaml

@@ -9,3 +9,6 @@ resources:
         code: if err := rm.requeueIfNotRunning(latest); err != nil { return nil, err }
       sdk_delete_pre_build_request:
         template_path: sdk_delete_pre_build_request.go.tpl
+    fields:
+      Users..Password:
+        is_secret: true

pkg/model/crd.go

@@ -78,6 +78,9 @@ type CRD struct {
 	// field. Note that there are no fields in StatusFields that are also in
 	// SpecFields.
 	StatusFields map[string]*Field
+	// Fields is a map, keyed by the **renamed/normalized field path**, of
+	// Field objects representing a field in the CRD's Spec or Status objects.
+	Fields map[string]*Field
 	// TypeImports is a map, keyed by an import string, with the map value
 	// being the import alias
 	TypeImports map[string]string
@@ -169,13 +172,15 @@ func (r *CRD) AddSpecField(
 	memberNames names.Names,
 	shapeRef *awssdkmodel.ShapeRef,
 ) {
+	fPath := memberNames.Camel
 	fConfigs := r.cfg.ResourceFields(r.Names.Original)
 	fConfig := fConfigs[memberNames.Original]
-	f := newField(r, memberNames, shapeRef, fConfig)
+	f := NewField(r, fPath, memberNames, shapeRef, fConfig)
 	if fConfig != nil && fConfig.IsPrintable {
 		r.addSpecPrintableColumn(f)
 	}
 	r.SpecFields[memberNames.Original] = f
+	r.Fields[fPath] = f
 }
 
 // AddStatusField adds a new Field of a given name and shape into the Status
@@ -184,13 +189,15 @@ func (r *CRD) AddStatusField(
 	memberNames names.Names,
 	shapeRef *awssdkmodel.ShapeRef,
 ) {
+	fPath := memberNames.Camel
 	fConfigs := r.cfg.ResourceFields(r.Names.Original)
 	fConfig := fConfigs[memberNames.Original]
-	f := newField(r, memberNames, shapeRef, fConfig)
+	f := NewField(r, fPath, memberNames, shapeRef, fConfig)
 	if fConfig != nil && fConfig.IsPrintable {
 		r.addStatusPrintableColumn(f)
 	}
 	r.StatusFields[memberNames.Original] = f
+	r.Fields[fPath] = f
 }
 
 // AddTypeImport adds an entry in the CRD's TypeImports map for an import line
@@ -254,12 +261,20 @@ func (r *CRD) UnpackAttributes() {
 			continue
 		}
 		fieldNames := names.New(fieldName)
-		f := newField(r, fieldNames, nil, fieldConfig)
+		var fPath string
+		if !fieldConfig.IsReadOnly {
+			fPath = fieldNames.Camel
+		} else {
+			fPath = fieldNames.Camel
+		}
+
+		f := NewField(r, fPath, fieldNames, nil, fieldConfig)
 		if !fieldConfig.IsReadOnly {
 			r.SpecFields[fieldName] = f
 		} else {
 			r.StatusFields[fieldName] = f
 		}
+		r.Fields[fPath] = f
 	}
 }
 
@@ -414,6 +429,7 @@ func NewCRD(
 		additionalPrinterColumns: make([]*PrinterColumn, 0),
 		SpecFields:               map[string]*Field{},
 		StatusFields:             map[string]*Field{},
+		Fields:                   map[string]*Field{},
 		ShortNames:               cfg.ResourceShortNames(kind),
 	}
 }

pkg/model/field.go

@@ -20,11 +20,27 @@ import (
 	awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
 )
 
-// Field represents a single field in the CRD's Spec or Status objects
+// Field represents a single field in the CRD's Spec or Status objects. The
+// field may be a direct field of the Spec or Status object or may be a field
+// of a list or struct-type field of the Spec or Status object. We call these
+// latter fields "nested fields" and they are identified by the Field.Path
+// attribute.
 type Field struct {
-	CRD               *CRD
-	Names             names.Names
-	GoType            string
+	// CRD is the a pointer to the top-level custom resource definition
+	// descriptor for the field or field's parent (if a nested field)
+	CRD *CRD
+	// Names is a set of normalized names for the field
+	Names names.Names
+	// Path is a "field path" that indicates where the field is within the CRD.
+	// For example "Spec.Name" or "Status.BrokerInstances..Endpoint". Note for
+	// the latter example, the field path indicates that the field `Endpoint`
+	// is an attribute of the `Status.BrokerInstances` top-level field and the
+	// double dot (`..` indicates that BrokerInstances is a list type).
+	Path string
+	// GoType is a string containing the Go data type for the field
+	GoType string
+	// GoTypeElem indicates the Go data type for the type of list element if
+	// the field is a list type
 	GoTypeElem        string
 	GoTypeWithPkgName string
 	ShapeRef          *awssdkmodel.ShapeRef
@@ -45,9 +61,10 @@ func (f *Field) IsRequired() bool {
 	return util.InStrings(f.Names.ModelOriginal, f.CRD.Ops.Create.InputRef.Shape.Required)
 }
 
-// newField returns a pointer to a new Field object
-func newField(
+// NewField returns a pointer to a new Field object
+func NewField(
 	crd *CRD,
+	path string,
 	fieldNames names.Names,
 	shapeRef *awssdkmodel.ShapeRef,
 	cfg *ackgenconfig.FieldConfig,
@@ -72,6 +89,7 @@ func newField(
 	return &Field{
 		CRD:               crd,
 		Names:             fieldNames,
+		Path:              path,
 		ShapeRef:          shapeRef,
 		GoType:            gt,
 		GoTypeElem:        gte,