Reference output_wrapper_field_path when generating CRD fields as part of the Create* request in model.go (#210)

Issue #, if available:
* `model.go` **does not** take into account any `output_wrapper_field_path` overrides when processing Status fields for a CRD

Description of changes:
* Refactored `GetOutputShape` api:
  * always return an *output* shape
  * make `getWrapperOutputShape` private
  * updated tests
* Added `GetOutputShape` invocation to `model.go` so that overrides are checked before creating the CRD

Local Testing:
* *generator.yaml*
```
operations:
  CreateVpcEndpoint:
    output_wrapper_field_path: VpcEndpoint
```

* `make build-controller SERVICE=ec2` 
* **Result:** `VPCEndpointStatus` contains all unpacked fields. Assigning from aws sdk response (`vpc_endpoint/sdk.go`) includes `resp.VpcEndpoint` as prefix

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/code/set_resource.go

@@ -110,23 +110,14 @@ func SetResource(
 	if op == nil {
 		return ""
 	}
-	outputShape := op.OutputRef.Shape
+	outputShape, _ := r.GetOutputShape(op)
 	if outputShape == nil {
 		return ""
 	}
 
-	var err error
-	// We might be in a "wrapper" shape. Unwrap it to find the real object
-	// representation for the CRD's createOp/DescribeOP.
-
 	// Use the wrapper field path if it's given in the ack-generate config file.
 	wrapperFieldPath := r.GetOutputWrapperFieldPath(op)
 	if wrapperFieldPath != nil {
-		outputShape, err = r.GetWrapperOutputShape(outputShape, *wrapperFieldPath)
-		if err != nil {
-			msg := fmt.Sprintf("Unable to unwrap the output shape: %v", err)
-			panic(msg)
-		}
 		sourceVarName += "." + *wrapperFieldPath
 	} else {
 		// If the wrapper field path is not specified in the config file and if

pkg/generate/code/set_resource_test.go

@@ -14,7 +14,6 @@
 package code_test
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -23,7 +22,6 @@ import (
 	"github.com/aws-controllers-k8s/code-generator/pkg/generate/code"
 	"github.com/aws-controllers-k8s/code-generator/pkg/model"
 	"github.com/aws-controllers-k8s/code-generator/pkg/testutil"
-	awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
 )
 
 func TestSetResource_APIGWv2_Route_Create(t *testing.T) {
@@ -2940,7 +2938,23 @@ func TestSetResource_RDS_DBSubnetGroup_ReadMany(t *testing.T) {
 	)
 }
 
-func TestGetWrapperOutputShape(t *testing.T) {
+func TestGetOutputShape_VPC_No_Override(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForService(t, "ec2")
+
+	crd := testutil.GetCRDByName(t, g, "Vpc")
+	require.NotNil(crd)
+
+	expectedShape := crd.Ops.ReadMany.OutputRef.Shape
+	outputShape, _ := crd.GetOutputShape(crd.Ops.ReadMany)
+	assert.Equal(
+		expectedShape,
+		outputShape)
+}
+
+func TestGetOutputShape_DynamoDB_Override(t *testing.T) {
 	assert := assert.New(t)
 	require := require.New(t)
 
@@ -2949,54 +2963,25 @@ func TestGetWrapperOutputShape(t *testing.T) {
 	crd := testutil.GetCRDByName(t, g, "Backup")
 	require.NotNil(crd)
 
-	op := crd.Ops.ReadOne.OutputRef.Shape
-
-	type args struct {
-		outputShape *awssdkmodel.Shape
-		fieldPath   string
-	}
-	tests := []struct {
-		name          string
-		args          args
-		wantErr       bool
-		wantShapeName string
-	}{
-		{
-			name: "incorrect field path: element not found",
-			args: args{
-				outputShape: op,
-				fieldPath:   "BackupDescription.Something",
-			},
-			wantErr: true,
-		},
-		{
-			name: "incorrect field path: element not of type structure",
-			args: args{
-				outputShape: op,
-				fieldPath:   "BackupDescription.BackupArn",
-			},
-			wantErr: true,
-		},
-		{
-			name: "correct field path",
-			args: args{
-				outputShape: op,
-				fieldPath:   "BackupDescription.BackupDetails",
-			},
-			wantErr:       false,
-			wantShapeName: "BackupDetails",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			outputShape, err := crd.GetWrapperOutputShape(tt.args.outputShape, tt.args.fieldPath)
-			if (err != nil) != tt.wantErr {
-				assert.Fail(fmt.Sprintf("GetWrapperOutputShape() error = %v, wantErr %v", err, tt.wantErr))
-			} else if !tt.wantErr {
-				assert.Equal(tt.wantShapeName, outputShape.ShapeName)
-			}
-		})
-	}
+	outputShape, _ := crd.GetOutputShape(crd.Ops.ReadOne)
+	assert.Equal(
+		"BackupDetails",
+		outputShape.ShapeName)
+}
+
+func TestGetOutputShape_VPCEndpoint_Override(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForService(t, "ec2")
+
+	crd := testutil.GetCRDByName(t, g, "VpcEndpoint")
+	require.NotNil(crd)
+
+	outputShape, _ := crd.GetOutputShape(crd.Ops.Create)
+	assert.Equal(
+		"VpcEndpoint",
+		outputShape.ShapeName)
 }
 
 func TestSetResource_MQ_Broker_SetResourceIdentifiers(t *testing.T) {

pkg/model/crd.go

@@ -452,9 +452,9 @@ func (r *CRD) GetOutputWrapperFieldPath(
 	return &opConfig.OutputWrapperFieldPath
 }
 
-// GetOutputShape returns the Output shape for given operation.
+// GetOutputShape returns the Output shape for a given operation and applies
+// wrapper field path overrides, if specified in generator config.
 func (r *CRD) GetOutputShape(
-	// The operation to look for the Output shape
 	op *awssdkmodel.Operation,
 ) (*awssdkmodel.Shape, error) {
 	if op == nil {
@@ -466,37 +466,27 @@ func (r *CRD) GetOutputShape(
 		return nil, errors.New("output shape not found")
 	}
 
-	// We might be in a "wrapper" shape. Unwrap it to find the real object
-	// representation for the CRD's createOp/DescribeOP.
-
-	// Use the wrapper field path if it's given in the ack-generate config file.
+	// Check for wrapper field path overrides
 	wrapperFieldPath := r.GetOutputWrapperFieldPath(op)
 	if wrapperFieldPath != nil {
-		wrapperOutputShape, err := r.GetWrapperOutputShape(outputShape, *wrapperFieldPath)
+		wrapperOutputShape, err := r.getWrapperOutputShape(outputShape,
+			*wrapperFieldPath)
 		if err != nil {
-			return nil, fmt.Errorf("unable to unwrap the output shape: %v", err)
+			msg := fmt.Sprintf("Unable to unwrap the output shape: %s " +
+				"with field path override: %s. error: %v",
+				outputShape.OrigShapeName, *wrapperFieldPath, err)
+			panic(msg)
 		}
 		outputShape = wrapperOutputShape
-	} else {
-		// If the wrapper field path is not specified in the config file and if
-		// there is a single member shape and that member shape is a structure,
-		// unwrap it.
-		if outputShape.UsedAsOutput && len(outputShape.MemberRefs) == 1 {
-			for _, memberRef := range outputShape.MemberRefs {
-				if memberRef.Shape.Type == "structure" {
-					outputShape = memberRef.Shape
-				}
-			}
-		}
 	}
 	return outputShape, nil
 }
 
-// GetWrapperOutputShape returns the shape of the last element of a given field
-// Path. It carefully unwraps the output shape and verifies that every element
-// of the field path exists in their correspanding parent shape and that they are
+// getWrapperOutputShape returns the shape of the last element of a given field
+// Path. It unwraps the output shape and verifies that every element of the
+// field path exists in their corresponding parent shape and that they are
 // structures.
-func (r *CRD) GetWrapperOutputShape(
+func (r *CRD) getWrapperOutputShape(
 	shape *awssdkmodel.Shape,
 	fieldPath string,
 ) (*awssdkmodel.Shape, error) {
@@ -509,21 +499,19 @@ func (r *CRD) GetWrapperOutputShape(
 	memberRef, ok := shape.MemberRefs[wrapperField]
 	if !ok {
 		return nil, fmt.Errorf(
-			"Incorrect SetOutput.WrapperFieldPath. Could not find %s in Shape %s",
-			wrapperField, shape.ShapeName,
-		)
+			"could not find wrapper override field %s in Shape %s",
+			wrapperField, shape.ShapeName)
 	}
 
+	// wrapper field must be structure; otherwise cannot unpack
 	if memberRef.Shape.Type != "structure" {
-		// All the mentioned shapes must be structure
 		return nil, fmt.Errorf(
-			"Expected SetOutput.WrapperFieldPath to only contain fields of type 'structure'."+
-				" Found %s of type '%s'",
-			wrapperField, memberRef.Shape.Type,
-		)
+			"output wrapper overrides can only contain fields of type" +
+				" 'structure'. Found wrapper override field %s of type '%s'",
+			wrapperField, memberRef.Shape.Type)
 	}
 	remainPath := strings.Join(fieldPathParts[1:], ".")
-	return r.GetWrapperOutputShape(memberRef.Shape, remainPath)
+	return r.getWrapperOutputShape(memberRef.Shape, remainPath)
 }
 
 // GetCustomImplementation returns custom implementation method name for the

pkg/model/model.go

@@ -161,7 +161,10 @@ func (m *Model) GetCRDs() ([]*CRD, error) {
 		// Now process the fields that will go into the Status struct. We want
 		// fields that are in the Create operation's Output Shape but that are
 		// not in the Input Shape.
-		outputShape := createOp.OutputRef.Shape
+		outputShape, err := crd.GetOutputShape(createOp)
+		if err != nil {
+			return nil, err
+		}
 		if outputShape.UsedAsOutput && len(outputShape.MemberRefs) == 1 {
 			// We might be in a "wrapper" shape. Unwrap it to find the real object
 			// representation for the CRD's createOp. If there is a single member

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

@@ -19,7 +19,7 @@ ignore:
     - InternetGateway
     - KeyPair
     - LaunchTemplateVersion
-    # - LaunchTemplate
+    #- LaunchTemplate
     - LocalGatewayRouteTableVpcAssociation
     - LocalGatewayRoute
     - ManagedPrefixList
@@ -50,13 +50,17 @@ ignore:
     - TransitGatewayRoute
     - TransitGatewayVpcAttachment
     - TransitGateway
-    # - Volume
+    #- Volume
     - VpcEndpointConnectionNotification
     - VpcEndpointServiceConfiguration
-    - VpcEndpoint
-    # - Vpc
+    #- VpcEndpoint
+    #- Vpc
     - VpcCidrBlock
     - VpcPeeringConnection
     - VpnConnectionRoute
     - VpnConnection
     - VpnGateway
+
+operations:
+  CreateVpcEndpoint:
+    output_wrapper_field_path: VpcEndpoint