Skip to content

Commit 82c294c

Browse files
authored
allow SetFieldConfig.From to refer to nested fields (#255)
Often, the Go type of a Create Input shape field (which goes in the resource's Spec struct) is different from the Go type of the same-named field in an Output shape. This causes problems for the code generator, which doesn't know how to set the value of the Spec field due to these different source and target Go types. The `pkg/generate/config.SetConfig` struct was designed to solve this particular issue, however the original solution only worked properly when the field in the Output shape that we wanted to set the Spec field *from* was at the same "level" as the field we wanted to set the value *to*. Take for example the case of the IAM Role resource, which has a Create Input shape with a string `PermissionsBoundary` field. When creating the Role, the user specifies the PermissionsBoundary as an ARN field. In the Create and Get Output shapes, however, the `PermissionsBoundary` field is a struct containing a `PermissionsBoundaryArn` string field. We want to set the value of the Role resource's `Spec.PermissionsBoundary` field to the string value of this nested `PermissionsBoundary.PermissionsBoundaryArn` field. This patch facilitates that by modifying the `pkg/generate/code.SetResource` function to look up the field in the Output shape via a field path instead of a single field name map lookup. Issue aws-controllers-k8s/community#1065 Signed-off-by: Jay Pipes <[email protected]> By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent d56efd4 commit 82c294c

File tree

5 files changed

+10026
-17
lines changed

5 files changed

+10026
-17
lines changed

pkg/generate/code/set_resource.go

+6-16
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
2222

23+
"github.com/aws-controllers-k8s/code-generator/pkg/fieldpath"
2324
ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
2425
"github.com/aws-controllers-k8s/code-generator/pkg/model"
2526
"github.com/aws-controllers-k8s/code-generator/pkg/names"
@@ -236,11 +237,11 @@ func SetResource(
236237
// And this would indicate to the code generator that the Spec.Name
237238
// field should be set from the value of the Output shape's Bucket
238239
// field.
239-
var found bool
240240
if setCfg != nil && setCfg.From != nil {
241-
sourceMemberShapeRef, found = outputShape.MemberRefs[*setCfg.From]
241+
fp := fieldpath.FromString(*setCfg.From)
242+
sourceMemberShapeRef = fp.ShapeRef(sourceMemberShapeRef)
242243
}
243-
if !found || sourceMemberShapeRef.Shape == nil {
244+
if sourceMemberShapeRef == nil || sourceMemberShapeRef.Shape == nil {
244245
// Technically this should not happen, so let's bail here if it
245246
// does...
246247
msg := fmt.Sprintf(
@@ -251,7 +252,7 @@ func SetResource(
251252
}
252253
}
253254

254-
sourceMemberShape := sourceMemberShapeRef.Shape
255+
targetMemberShape := targetMemberShapeRef.Shape
255256

256257
// fieldVarName is the name of the variable that is used for temporary
257258
// storage of complex member field values
@@ -289,7 +290,7 @@ func SetResource(
289290
"%s.%s", targetAdaptedVarName, f.Names.Camel,
290291
)
291292

292-
switch sourceMemberShape.Type {
293+
switch targetMemberShape.Type {
293294
case "list", "structure", "map":
294295
{
295296
memberVarName := fmt.Sprintf("f%d", memberIndex)
@@ -321,17 +322,6 @@ func SetResource(
321322
// We have some special instructions to pull the value from a
322323
// different field or member...
323324
sourceAdaptedVarName = sourceVarName + "." + *setCfg.From
324-
var found bool
325-
sourceMemberShapeRef, found = outputShape.MemberRefs[*setCfg.From]
326-
if !found {
327-
// This is likely a typo in the generator.yaml file...
328-
msg := fmt.Sprintf(
329-
"expected to find a member shape of %s called %s",
330-
outputShape.ShapeName,
331-
*setCfg.From,
332-
)
333-
panic(msg)
334-
}
335325
}
336326
out += setResourceForScalar(
337327
qualifiedTargetVar,

pkg/generate/code/set_resource_test.go

+98-1
Original file line numberDiff line numberDiff line change
@@ -3071,4 +3071,101 @@ func TestSetResource_EC2_SecurityGroups_SetResourceIdentifiers(t *testing.T) {
30713071
expected,
30723072
code.SetResource(crd.Config(), crd, model.OpTypeCreate, "resp", "ko", 1),
30733073
)
3074-
}
3074+
}
3075+
3076+
func TestSetResource_IAM_Role_NestedSetConfig(t *testing.T) {
3077+
assert := assert.New(t)
3078+
require := require.New(t)
3079+
3080+
g := testutil.NewModelForService(t, "iam")
3081+
3082+
crd := testutil.GetCRDByName(t, g, "Role")
3083+
require.NotNil(crd)
3084+
3085+
// The input and output shapes for the PermissionsBoundary are different
3086+
// and we have a custom SetConfig for this field in our generator.yaml file
3087+
// to configure the SetResource function to set the value of the resource's
3088+
// Spec.PermissionsBoundary to the value of the (nested)
3089+
// GetRoleResponse.Role.PermissionsBoundary.PermissionsBoundaryArn field
3090+
expected := `
3091+
if ko.Status.ACKResourceMetadata == nil {
3092+
ko.Status.ACKResourceMetadata = &ackv1alpha1.ResourceMetadata{}
3093+
}
3094+
if resp.Role.Arn != nil {
3095+
arn := ackv1alpha1.AWSResourceName(*resp.Role.Arn)
3096+
ko.Status.ACKResourceMetadata.ARN = &arn
3097+
}
3098+
if resp.Role.AssumeRolePolicyDocument != nil {
3099+
ko.Spec.AssumeRolePolicyDocument = resp.Role.AssumeRolePolicyDocument
3100+
} else {
3101+
ko.Spec.AssumeRolePolicyDocument = nil
3102+
}
3103+
if resp.Role.CreateDate != nil {
3104+
ko.Status.CreateDate = &metav1.Time{*resp.Role.CreateDate}
3105+
} else {
3106+
ko.Status.CreateDate = nil
3107+
}
3108+
if resp.Role.Description != nil {
3109+
ko.Spec.Description = resp.Role.Description
3110+
} else {
3111+
ko.Spec.Description = nil
3112+
}
3113+
if resp.Role.MaxSessionDuration != nil {
3114+
ko.Spec.MaxSessionDuration = resp.Role.MaxSessionDuration
3115+
} else {
3116+
ko.Spec.MaxSessionDuration = nil
3117+
}
3118+
if resp.Role.Path != nil {
3119+
ko.Spec.Path = resp.Role.Path
3120+
} else {
3121+
ko.Spec.Path = nil
3122+
}
3123+
if resp.Role.PermissionsBoundary != nil {
3124+
ko.Spec.PermissionsBoundary = resp.Role.PermissionsBoundary.PermissionsBoundaryArn
3125+
} else {
3126+
ko.Spec.PermissionsBoundary = nil
3127+
}
3128+
if resp.Role.RoleId != nil {
3129+
ko.Status.RoleID = resp.Role.RoleId
3130+
} else {
3131+
ko.Status.RoleID = nil
3132+
}
3133+
if resp.Role.RoleLastUsed != nil {
3134+
f8 := &svcapitypes.RoleLastUsed{}
3135+
if resp.Role.RoleLastUsed.LastUsedDate != nil {
3136+
f8.LastUsedDate = &metav1.Time{*resp.Role.RoleLastUsed.LastUsedDate}
3137+
}
3138+
if resp.Role.RoleLastUsed.Region != nil {
3139+
f8.Region = resp.Role.RoleLastUsed.Region
3140+
}
3141+
ko.Status.RoleLastUsed = f8
3142+
} else {
3143+
ko.Status.RoleLastUsed = nil
3144+
}
3145+
if resp.Role.RoleName != nil {
3146+
ko.Spec.Name = resp.Role.RoleName
3147+
} else {
3148+
ko.Spec.Name = nil
3149+
}
3150+
if resp.Role.Tags != nil {
3151+
f10 := []*svcapitypes.Tag{}
3152+
for _, f10iter := range resp.Role.Tags {
3153+
f10elem := &svcapitypes.Tag{}
3154+
if f10iter.Key != nil {
3155+
f10elem.Key = f10iter.Key
3156+
}
3157+
if f10iter.Value != nil {
3158+
f10elem.Value = f10iter.Value
3159+
}
3160+
f10 = append(f10, f10elem)
3161+
}
3162+
ko.Spec.Tags = f10
3163+
} else {
3164+
ko.Spec.Tags = nil
3165+
}
3166+
`
3167+
assert.Equal(
3168+
expected,
3169+
code.SetResource(crd.Config(), crd, model.OpTypeGet, "resp", "ko", 1),
3170+
)
3171+
}

0 commit comments

Comments
 (0)