Add cycle detection with panic (#346)

Fixes aws-controllers-k8s/community#1327

Description of changes:
The majority of our code-generator templates and methods rely on our custom resource properties being represented as DAGs. That is, they can recurse through their children until they reach primitive values at the base. Without a major refactor, it is not possible to support a cyclic field reference. This pull request adds logic to detect a cycle as we are iterating over new fields and will panic to tell the user about the limitation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: RedbackThomson

pkg/model/field.go

@@ -229,6 +229,23 @@ func NewField(
 	fieldNames names.Names,
 	shapeRef *awssdkmodel.ShapeRef,
 	cfg *ackgenconfig.FieldConfig,
+) *Field {
+	return newFieldRecurse(crd, path, make(map[string]struct{}, 0), fieldNames, shapeRef, cfg)
+}
+
+// newFieldRecurse recursively calls itself with protection against infinite
+// cycles
+func newFieldRecurse(
+	crd *CRD,
+	path string,
+	// A map of shape name to Fields. Keeps track of every shape that has been
+	// recursively discovered so that we can detect cycles. For example, the
+	// emrcontainers `Configuration` object contains a property of type
+	// `Configuration`.
+	parentFields map[string]struct{},
+	fieldNames names.Names,
+	shapeRef *awssdkmodel.ShapeRef,
+	cfg *ackgenconfig.FieldConfig,
 ) *Field {
 	memberFields := map[string]*Field{}
 	var gte, gt, gtwp string
@@ -269,16 +286,36 @@ func NewField(
 			}
 			break
 		}
+
+		// Copy the parent fields map so that we can use it recursively without
+		// modifying it for other call stacks
+		nestedParentFields := make(map[string]struct{}, len(parentFields))
+		for k, v := range parentFields {
+			nestedParentFields[k] = v
+		}
+		nestedParentFields[shapeRef.ShapeName] = struct{}{}
+
 		if containerShape.Type == "structure" {
 			// "unpack" the member fields composing this struct field...
 			for _, memberName := range containerShape.MemberNames() {
 				cleanMemberNames := names.New(memberName)
 				memberPath := path + "." + cleanMemberNames.Camel
 				memberShape := containerShape.MemberRefs[memberName]
+
+				// Check to see if we have seen this shape before in the stack
+				// and panic. Cyclic references are not supported
+				if _, ok := nestedParentFields[memberShape.ShapeName]; ok {
+					panic(fmt.Sprintf("Detected a cyclic type reference in %s"+
+						". Add the corresponding path to `ignore.field_paths`"+
+						" in the generator config to continue.",
+						containerShape.ShapeName))
+				}
+
 				fConfigs := crd.cfg.GetFieldConfigs(crd.Names.Original)
-				memberField := NewField(
-					crd, memberPath, cleanMemberNames, memberShape, fConfigs[memberPath],
+				memberField := newFieldRecurse(
+					crd, memberPath, nestedParentFields, cleanMemberNames, memberShape, fConfigs[memberPath],
 				)
+
 				memberFields[cleanMemberNames.Camel] = memberField
 			}
 		}
@@ -287,6 +324,7 @@ func NewField(
 		gt = "*string"
 		gtwp = "*string"
 	}
+
 	return &Field{
 		CRD:               crd,
 		Names:             fieldNames,

pkg/model/model_emrcontainers_test.go

@@ -0,0 +1,32 @@
+// 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 model_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/aws-controllers-k8s/code-generator/pkg/testutil"
+)
+
+func TestEMRContainers_JobRun(t *testing.T) {
+	assert := assert.New(t)
+
+	g := testutil.NewModelForServiceWithOptions(t, "emrcontainers", &testutil.TestingModelOptions{
+		GeneratorConfigFile: "generator-with-cycle.yaml",
+	})
+
+	assert.Panics(func() { g.GetCRDs() })
+}