Use SetConfig in SetResourceForStruct when target shape has no members (#292)

Issue #, if available: aws-controllers-k8s/community#1146

This is a follow-up to #289. A `targetShape` without members in `SetResourceForStruct` will not be processed today; however, this is **not** desired behavior for all APIs such as EC2's DHCPOptions resource. 

This patch will look up and consume `SetConfig` in `SetResourceForStruct` when `targetShape` has no members like when DHCPOptions' field, `DHCPConfigurations.Values []*string`, aligns (by name) with aws-sdk's output shape, `DhcpConfigurations.Values []*AttributeValue`. Instead of skipping the processing of `Values []*string` field, it will check if a `SetConfig` exists and if it's valid (referencing a *real* shape in sourceShape), then use that updated field to source the target field's value resulting in using `DhcpConfigurations.Values.Value *string`  to populate the resource's `DHCPConfigurations.Values []*string`.






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

Author: brycahta

pkg/generate/ack/controller.go

@@ -114,7 +114,7 @@ var (
 			if setCfg != nil && setCfg.Ignore {
 				return ""
 			}
-			return code.SetResourceForStruct(r.Config(), r, targetFieldName, targetVarName, targetShapeRef, setCfg, sourceVarName, sourceShapeRef, indentLevel)
+			return code.SetResourceForStruct(r.Config(), r, targetFieldName, targetVarName, targetShapeRef, setCfg, sourceVarName, sourceShapeRef, "", model.OpTypeList, indentLevel)
 		},
 		"GoCodeCompare": func(r *ackmodel.CRD, deltaVarName string, sourceVarName string, targetVarName string, indentLevel int) string {
 			return code.CompareResource(r.Config(), r, deltaVarName, sourceVarName, targetVarName, indentLevel)

pkg/generate/code/set_resource.go

@@ -308,6 +308,8 @@ func SetResource(
 					setCfg,
 					sourceAdaptedVarName,
 					sourceMemberShapeRef,
+					f.Names.Camel,
+					opType,
 					indentLevel+1,
 				)
 				out += setResourceForScalar(
@@ -578,6 +580,8 @@ func setResourceReadMany(
 					setCfg,
 					sourceAdaptedVarName,
 					sourceMemberShapeRef,
+					f.Names.Camel,
+					model.OpTypeList,
 					indentLevel+2,
 				)
 				out += setResourceForScalar(
@@ -1156,6 +1160,9 @@ func setResourceForContainer(
 	sourceVarName string,
 	// ShapeRef of the source struct field
 	sourceShapeRef *awssdkmodel.ShapeRef,
+	// targetFieldPath is the field path for the target containing field
+	targetFieldPath string,
+	op model.OpType,
 	indentLevel int,
 ) string {
 	switch sourceShapeRef.Shape.Type {
@@ -1168,6 +1175,8 @@ func setResourceForContainer(
 			targetSetCfg,
 			sourceVarName,
 			sourceShapeRef,
+			targetFieldPath,
+			op,
 			indentLevel,
 		)
 	case "list":
@@ -1179,6 +1188,8 @@ func setResourceForContainer(
 			targetSetCfg,
 			sourceVarName,
 			sourceShapeRef,
+			targetFieldPath,
+			op,
 			indentLevel,
 		)
 	case "map":
@@ -1190,6 +1201,8 @@ func setResourceForContainer(
 			targetSetCfg,
 			sourceVarName,
 			sourceShapeRef,
+			targetFieldPath,
+			op,
 			indentLevel,
 		)
 	default:
@@ -1219,15 +1232,21 @@ func SetResourceForStruct(
 	sourceVarName string,
 	// ShapeRef of the source struct field
 	sourceShapeRef *awssdkmodel.ShapeRef,
+	// targetFieldPath is the field path to targetFieldName
+	targetFieldPath string,
+	op model.OpType,
 	indentLevel int,
 ) string {
 	out := ""
 	indent := strings.Repeat("\t", indentLevel)
 	sourceShape := sourceShapeRef.Shape
 	targetShape := targetShapeRef.Shape
 
+	var sourceMemberShapeRef *awssdkmodel.ShapeRef
+	var sourceAdaptedVarName, qualifiedTargetVar string
+
 	for _, targetMemberName := range targetShape.MemberNames() {
-		sourceMemberShapeRef := sourceShape.MemberRefs[targetMemberName]
+		sourceMemberShapeRef = sourceShape.MemberRefs[targetMemberName]
 		if sourceMemberShapeRef == nil {
 			continue
 		}
@@ -1244,13 +1263,14 @@ func SetResourceForStruct(
 		indexedVarName := fmt.Sprintf("%sf%d", targetVarName, sourceMemberIndex)
 		sourceMemberShape := sourceMemberShapeRef.Shape
 		targetMemberCleanNames := names.New(targetMemberName)
-		sourceAdaptedVarName := sourceVarName + "." + targetMemberName
+		sourceAdaptedVarName = sourceVarName + "." + targetMemberName
 		out += fmt.Sprintf(
 			"%sif %s != nil {\n", indent, sourceAdaptedVarName,
 		)
-		qualifiedTargetVar := fmt.Sprintf(
+		qualifiedTargetVar = fmt.Sprintf(
 			"%s.%s", targetVarName, targetMemberCleanNames.Camel,
 		)
+		updatedTargetFieldPath := targetFieldPath + "." + targetMemberCleanNames.Camel
 
 		switch sourceMemberShape.Type {
 		case "list", "structure", "map":
@@ -1269,6 +1289,8 @@ func SetResourceForStruct(
 					nil,
 					sourceAdaptedVarName,
 					sourceMemberShapeRef,
+					updatedTargetFieldPath,
+					op,
 					indentLevel+1,
 				)
 				out += setResourceForScalar(
@@ -1290,6 +1312,52 @@ func SetResourceForStruct(
 			"%s}\n", indent,
 		)
 	}
+	if len(targetShape.MemberNames()) == 0 {
+		// This scenario can occur when the targetShape is a primitive, but
+		// the sourceShape is a struct. For example, EC2 resource DHCPOptions
+		// has a field NewDhcpConfiguration.Values(targetShape = string) whose name
+		// aligns with DhcpConfiguration.Values(sourceShape = AttributeValue).
+		// Although the names correspond, the shapes/types are different and the intent
+		// is to set NewDhcpConfiguration.Values using DhcpConfiguration.Values.Value
+		// (AttributeValue.Value) shape instead. This behavior can be configured using
+		// SetConfig.
+
+		// Check if target field has a SetConfig, validate SetConfig.From points
+		// to a shape within sourceShape, and generate Go code using
+		// said shape. Using the example above, SetConfig is set
+		// for NewDhcpConfiguration.Values and Setconfig.From points
+		// to AttributeValue.Value (string), which leads to generating Go
+		// code referencing DhcpConfiguration.Values.Value instead of 'Values'.
+
+		if targetField, ok := r.Fields[targetFieldPath]; ok {
+			setCfg := targetField.GetSetterConfig(op)
+			if setCfg != nil && setCfg.From != nil {
+				fp := fieldpath.FromString(*setCfg.From)
+				sourceMemberShapeRef = fp.ShapeRef(sourceShapeRef)
+				if sourceMemberShapeRef != nil && sourceMemberShapeRef.Shape != nil {
+					names := names.New(sourceMemberShapeRef.LocationName)
+					sourceAdaptedVarName = sourceVarName + "." + names.Camel
+					out += fmt.Sprintf(
+						"%sif %s != nil {\n", indent, sourceAdaptedVarName,
+					)
+					qualifiedTargetVar = targetVarName
+
+					// Use setResourceForScalar and dereference sourceAdaptedVarName
+					// because primitives are being set.
+					sourceAdaptedVarName = "*" + sourceAdaptedVarName
+					out += setResourceForScalar(
+						qualifiedTargetVar,
+						sourceAdaptedVarName,
+						sourceMemberShapeRef,
+						indentLevel+1,
+					)
+					out += fmt.Sprintf(
+						"%s}\n", indent,
+					)
+				}
+			}
+		}
+	}
 	return out
 }
 
@@ -1310,6 +1378,9 @@ func setResourceForSlice(
 	sourceVarName string,
 	// ShapeRef of the source slice field
 	sourceShapeRef *awssdkmodel.ShapeRef,
+	// targetFieldPath is the field path to targetFieldName
+	targetFieldPath string,
+	op model.OpType,
 	indentLevel int,
 ) string {
 	out := ""
@@ -1388,6 +1459,8 @@ func setResourceForSlice(
 			targetSetCfg,
 			iterVarName,
 			&sourceShape.MemberRef,
+			targetFieldPath,
+			op,
 			indentLevel+1,
 		)
 	}
@@ -1421,6 +1494,9 @@ func setResourceForMap(
 	sourceVarName string,
 	// ShapeRef of the source map field
 	sourceShapeRef *awssdkmodel.ShapeRef,
+	// targetFieldPath is the field path to targetFieldName
+	targetFieldPath string,
+	op model.OpType,
 	indentLevel int,
 ) string {
 	out := ""
@@ -1453,6 +1529,8 @@ func setResourceForMap(
 		nil,
 		valIterVarName,
 		&sourceShape.ValueRef,
+		targetFieldPath,
+		op,
 		indentLevel+1,
 	)
 	addressOfVar := ""

pkg/generate/code/set_resource_test.go

@@ -3169,3 +3169,75 @@ func TestSetResource_IAM_Role_NestedSetConfig(t *testing.T) {
 		code.SetResource(crd.Config(), crd, model.OpTypeGet, "resp", "ko", 1),
 	)
 }
+
+func TestSetResource_EC2_DHCPOptions_NestedSetConfig(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForService(t, "ec2")
+
+	crd := testutil.GetCRDByName(t, g, "DhcpOptions")
+	require.NotNil(crd)
+
+	// The input and output shapes for the DhcpConfigurations.Values are different
+	// and we have a custom SetConfig for this field in our generator.yaml file
+	// to configure the SetResource function to set the value of the resource's
+	// (nested) Spec.DHCPConfigurations.Values to the value of the (nested)
+	// GetDhcpOptionsResponse.DhcpOptions.DhcpConfigurations.Values.Value field
+	expected := `
+	if resp.DhcpOptions.DhcpConfigurations != nil {
+		f0 := []*svcapitypes.NewDHCPConfiguration{}
+		for _, f0iter := range resp.DhcpOptions.DhcpConfigurations {
+			f0elem := &svcapitypes.NewDHCPConfiguration{}
+			if f0iter.Key != nil {
+				f0elem.Key = f0iter.Key
+			}
+			if f0iter.Values != nil {
+				f0elemf1 := []*string{}
+				for _, f0elemf1iter := range f0iter.Values {
+					var f0elemf1elem string
+					if f0elemf1iter.Value != nil {
+						f0elemf1elem = *f0elemf1iter.Value
+					}
+					f0elemf1 = append(f0elemf1, &f0elemf1elem)
+				}
+				f0elem.Values = f0elemf1
+			}
+			f0 = append(f0, f0elem)
+		}
+		ko.Spec.DHCPConfigurations = f0
+	} else {
+		ko.Spec.DHCPConfigurations = nil
+	}
+	if resp.DhcpOptions.DhcpOptionsId != nil {
+		ko.Status.DHCPOptionsID = resp.DhcpOptions.DhcpOptionsId
+	} else {
+		ko.Status.DHCPOptionsID = nil
+	}
+	if resp.DhcpOptions.OwnerId != nil {
+		ko.Status.OwnerID = resp.DhcpOptions.OwnerId
+	} else {
+		ko.Status.OwnerID = nil
+	}
+	if resp.DhcpOptions.Tags != nil {
+		f3 := []*svcapitypes.Tag{}
+		for _, f3iter := range resp.DhcpOptions.Tags {
+			f3elem := &svcapitypes.Tag{}
+			if f3iter.Key != nil {
+				f3elem.Key = f3iter.Key
+			}
+			if f3iter.Value != nil {
+				f3elem.Value = f3iter.Value
+			}
+			f3 = append(f3, f3elem)
+		}
+		ko.Status.Tags = f3
+	} else {
+		ko.Status.Tags = nil
+	}
+`
+	assert.Equal(
+		expected,
+		code.SetResource(crd.Config(), crd, model.OpTypeCreate, "resp", "ko", 1),
+	)
+}

pkg/testdata/models/apis/ec2/0000-00-00/generator.yaml

@@ -68,8 +68,11 @@ operations:
     output_wrapper_field_path: VpcEndpoint
 
 resources:
-  DHCPOptions:
-
+  DhcpOptions:
+    fields:
+      DHCPConfigurations.Values:
+        set:
+          - from: AttributeValue.Value
   SecurityGroup:
     renames:
       operations: