Skip to content

Commit 1e1697f

Browse files
committed
allow different Go types for input/output shapes
Adds a new SetFieldConfig struct to the code generator config that allows teams to instruct the code generator to handle the SetResource generator specially for a particular `pkg/model.OpType`. This is useful for at least a couple use cases: 1. Different Go types in input and output shapes When the Go type of a field in the Create Input shape differs from the Go type of the same-named field in another operation's Output shape, the code generator doesn't know how to handle this situation and ends up outputting invalid Go code. An example of this behaviour is in the RDS controller's DBInstance resource. The DBSecurityGroups field in the Create Input shape is of type `[]*string`, but the same field in the Create, ReadMany and Update output shapes is of type `[]*DBSecurityGroupMembership`. The `DBSecurityGroupMembership` struct has two fields, `DBSecurityGroupName` and `Status`. The only part of that struct that is relevant to the `Spec.DBSecurityGroups` field is the `DBSecurityGroupName` field, which corresponds to the string identifier of the DB security group provided by the user in the `CreateDBInstanceRequest.DBSecurityGroups` field. The code generator ends up calling `pkg/generate/code.SetResource` for the DBInstance resource, gets to the `Spec.DBSecurityGroups` field and just doesn't know how to handle the different Go types and so ends up outputting nothing, which breaks compilation and has caused us to manually comment out or fix the generated code. operation. See: https://github.com/aws-controllers-k8s/rds-controller/blob/dc3271fa7b455fc99c3531ce372ea221c9f5b8e7/pkg/resource/db_instance/sdk.go#L853-L864 In the RDS DBInstance DBSecurityGroups case, we'd add the following to the RDS generator.yaml file to instruct the code generator how to transform the response from the DescribeDBInstances Output shape for setting the value of `Spec.DBSecurityGroups` from the set of `DBSecurityGroupMembership.DBSecurityGroupName` values: ```yaml resources: DBInstance: fields: DBSecurityGroups: set: - on: Create from: DBSecurityGroupName - on: ReadOne from: DBSecurityGroupName ``` 2. Ignoring fields in output shapes containing stale data For some service APIs, the returned value for a field in the Output shape of an Update operation contains the *originally-set* value instead of the value of the field that was set in the Update operation itself. An example of this situation is the ElastiCache ModifyReplicationGroup API call. If you pass some value for the LogDeliveryConfiguration field in the Input shape (which comes from the Spec.LogDeliveryConfiguration field of the ReplicationGroup resource), the value that is returned in the Output shape's LogDeliveryConfiguration is actually the *old* (stale) value for the field. The reason for this return of stale data is because the log delivery configuration is asynchronously mutated. For these cases, we want to be able to tell the code generator to ignore these fields when outputting code for the `pkg/generate/code.SetResource` function, but only for the Update operation. In this case, we'd add the following to the ElastiCache generator.yaml file: ```yaml resources: ReplicationGroup: fields: LogDeliveryConfiguration: set: - on: Update ignore: true ``` Signed-off-by: Jay Pipes <[email protected]>
1 parent 31204bc commit 1e1697f

File tree

9 files changed

+638
-418
lines changed

9 files changed

+638
-418
lines changed

pkg/generate/ack/controller.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/aws-controllers-k8s/code-generator/pkg/generate/code"
2222
ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
2323
"github.com/aws-controllers-k8s/code-generator/pkg/generate/templateset"
24+
"github.com/aws-controllers-k8s/code-generator/pkg/model"
2425
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
2526
awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
2627
)
@@ -102,7 +103,18 @@ var (
102103
return code.SetSDKForStruct(r.Config(), r, targetFieldName, targetVarName, targetShapeRef, sourceFieldPath, sourceVarName, indentLevel)
103104
},
104105
"GoCodeSetResourceForStruct": func(r *ackmodel.CRD, targetFieldName string, targetVarName string, targetShapeRef *awssdkmodel.ShapeRef, sourceVarName string, sourceShapeRef *awssdkmodel.ShapeRef, indentLevel int) string {
105-
return code.SetResourceForStruct(r.Config(), r, targetFieldName, targetVarName, targetShapeRef, sourceVarName, sourceShapeRef, indentLevel)
106+
f, ok := r.Fields[targetFieldName]
107+
if !ok {
108+
return ""
109+
}
110+
// We may have some special instructions for how to handle setting the
111+
// field value...
112+
setCfg := f.GetSetterConfig(model.OpTypeList)
113+
114+
if setCfg != nil && setCfg.Ignore {
115+
return ""
116+
}
117+
return code.SetResourceForStruct(r.Config(), r, targetFieldName, targetVarName, targetShapeRef, setCfg, sourceVarName, sourceShapeRef, indentLevel)
106118
},
107119
"GoCodeCompare": func(r *ackmodel.CRD, deltaVarName string, sourceVarName string, targetVarName string, indentLevel int) string {
108120
return code.CompareResource(r.Config(), r, deltaVarName, sourceVarName, targetVarName, indentLevel)

pkg/generate/code/set_resource.go

+148-29
Original file line numberDiff line numberDiff line change
@@ -181,19 +181,6 @@ func SetResource(
181181
continue
182182
}
183183

184-
sourceMemberShapeRef := outputShape.MemberRefs[memberName]
185-
if sourceMemberShapeRef.Shape == nil {
186-
// Technically this should not happen, so let's bail here if it
187-
// does...
188-
msg := fmt.Sprintf(
189-
"expected .Shape to not be nil for ShapeRef of memberName %s",
190-
memberName,
191-
)
192-
panic(msg)
193-
}
194-
195-
sourceMemberShape := sourceMemberShapeRef.Shape
196-
197184
// Determine whether the input shape's field is in the Spec or the
198185
// Status struct and set the source variable appropriately.
199186
var f *model.Field
@@ -219,6 +206,53 @@ func SetResource(
219206
}
220207

221208
targetMemberShapeRef = f.ShapeRef
209+
210+
// We may have some special instructions for how to handle setting the
211+
// field value...
212+
setCfg := f.GetSetterConfig(opType)
213+
214+
if setCfg != nil && setCfg.Ignore {
215+
continue
216+
}
217+
218+
sourceMemberShapeRef := outputShape.MemberRefs[memberName]
219+
if sourceMemberShapeRef.Shape == nil {
220+
// We may have some instructions to specially handle this field by
221+
// accessing a different Output shape member.
222+
//
223+
// As an example, let's say we're dealing with a resource called
224+
// Bucket that has a "Name" field. In the Output shape of the
225+
// ReadOne resource manager method, the field name is "Bucket",
226+
// though. There may be a generator.yaml file that looks like this:
227+
//
228+
// resources:
229+
// Bucket:
230+
// fields:
231+
// Name:
232+
// set:
233+
// - method: ReadOne
234+
// from: Bucket
235+
//
236+
// And this would indicate to the code generator that the Spec.Name
237+
// field should be set from the value of the Output shape's Bucket
238+
// field.
239+
var found bool
240+
if setCfg != nil && setCfg.From != nil {
241+
sourceMemberShapeRef, found = outputShape.MemberRefs[*setCfg.From]
242+
}
243+
if !found || sourceMemberShapeRef.Shape == nil {
244+
// Technically this should not happen, so let's bail here if it
245+
// does...
246+
msg := fmt.Sprintf(
247+
"expected .Shape to not be nil for ShapeRef of memberName %s",
248+
memberName,
249+
)
250+
panic(msg)
251+
}
252+
}
253+
254+
sourceMemberShape := sourceMemberShapeRef.Shape
255+
222256
// fieldVarName is the name of the variable that is used for temporary
223257
// storage of complex member field values
224258
//
@@ -270,6 +304,7 @@ func SetResource(
270304
f.Names.Camel,
271305
memberVarName,
272306
targetMemberShapeRef,
307+
setCfg,
273308
sourceAdaptedVarName,
274309
sourceMemberShapeRef,
275310
indentLevel+1,
@@ -282,6 +317,22 @@ func SetResource(
282317
)
283318
}
284319
default:
320+
if setCfg != nil && setCfg.From != nil {
321+
// We have some special instructions to pull the value from a
322+
// different field or member...
323+
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+
}
335+
}
285336
out += setResourceForScalar(
286337
qualifiedTargetVar,
287338
sourceAdaptedVarName,
@@ -502,6 +553,14 @@ func setResourceReadMany(
502553
continue
503554
}
504555

556+
// We may have some special instructions for how to handle setting the
557+
// field value...
558+
setCfg := f.GetSetterConfig(model.OpTypeList)
559+
560+
if setCfg != nil && setCfg.Ignore {
561+
continue
562+
}
563+
505564
targetMemberShapeRef = f.ShapeRef
506565
out += fmt.Sprintf(
507566
"%s\tif %s != nil {\n", indent, sourceAdaptedVarName,
@@ -526,6 +585,7 @@ func setResourceReadMany(
526585
f.Names.Camel,
527586
memberVarName,
528587
targetMemberShapeRef,
588+
setCfg,
529589
sourceAdaptedVarName,
530590
sourceMemberShapeRef,
531591
indentLevel+2,
@@ -1100,6 +1160,8 @@ func setResourceForContainer(
11001160
targetVarName string,
11011161
// Shape Ref of the target struct field
11021162
targetShapeRef *awssdkmodel.ShapeRef,
1163+
// SetFieldConfig of the *target* field
1164+
targetSetCfg *ackgenconfig.SetFieldConfig,
11031165
// The struct or struct field that we access our source value from
11041166
sourceVarName string,
11051167
// ShapeRef of the source struct field
@@ -1113,6 +1175,7 @@ func setResourceForContainer(
11131175
targetFieldName,
11141176
targetVarName,
11151177
targetShapeRef,
1178+
targetSetCfg,
11161179
sourceVarName,
11171180
sourceShapeRef,
11181181
indentLevel,
@@ -1123,6 +1186,7 @@ func setResourceForContainer(
11231186
targetFieldName,
11241187
targetVarName,
11251188
targetShapeRef,
1189+
targetSetCfg,
11261190
sourceVarName,
11271191
sourceShapeRef,
11281192
indentLevel,
@@ -1133,6 +1197,7 @@ func setResourceForContainer(
11331197
targetFieldName,
11341198
targetVarName,
11351199
targetShapeRef,
1200+
targetSetCfg,
11361201
sourceVarName,
11371202
sourceShapeRef,
11381203
indentLevel,
@@ -1158,6 +1223,8 @@ func SetResourceForStruct(
11581223
targetVarName string,
11591224
// Shape Ref of the target struct field
11601225
targetShapeRef *awssdkmodel.ShapeRef,
1226+
// SetFieldConfig of the *target* field
1227+
targetSetCfg *ackgenconfig.SetFieldConfig,
11611228
// The struct or struct field that we access our source value from
11621229
sourceVarName string,
11631230
// ShapeRef of the source struct field
@@ -1199,6 +1266,7 @@ func SetResourceForStruct(
11991266
cleanNames.Camel,
12001267
memberVarName,
12011268
targetMemberShapeRef,
1269+
nil,
12021270
sourceAdaptedVarName,
12031271
memberShapeRef,
12041272
indentLevel+1,
@@ -1236,6 +1304,8 @@ func setResourceForSlice(
12361304
targetVarName string,
12371305
// Shape Ref of the target slice field
12381306
targetShapeRef *awssdkmodel.ShapeRef,
1307+
// SetFieldConfig of the *target* field
1308+
targetSetCfg *ackgenconfig.SetFieldConfig,
12391309
// The struct or struct field that we access our source value from
12401310
sourceVarName string,
12411311
// ShapeRef of the source slice field
@@ -1244,6 +1314,7 @@ func setResourceForSlice(
12441314
) string {
12451315
out := ""
12461316
indent := strings.Repeat("\t", indentLevel)
1317+
12471318
sourceShape := sourceShapeRef.Shape
12481319
targetShape := targetShapeRef.Shape
12491320
iterVarName := fmt.Sprintf("%siter", targetVarName)
@@ -1257,26 +1328,71 @@ func setResourceForSlice(
12571328
targetShape.MemberRef.Shape,
12581329
indentLevel+1,
12591330
)
1260-
// f0elem0 = *f0iter0
1331+
// We may have some instructions to specially handle this field by
1332+
// accessing a different Output shape member.
12611333
//
1262-
// or
1334+
// As an example, let's say we're dealing with a resource called
1335+
// DBInstance that has a "DBSecurityGroups" field of type `[]*string`.
1336+
// In the Output shape of the ReadOne resource manager method, the
1337+
// field name is "DBSecurityGroups" of type
1338+
// `[]*DBSecurityGroupMembership`, though. This
1339+
// `DBSecurityGroupMembership` struct contains two fields,
1340+
// `DBSecurityGroupName` and `DBSecurityGroupStatus` and we wish to
1341+
// grab only the value of the `DBSecurityGroupName` fields and set the
1342+
// `Spec.DBSecurityGroups` field to the set of those field values.
12631343
//
1264-
// f0elem0.SetMyField(*f0iter0)
1265-
containerFieldName := ""
1266-
if sourceShape.MemberRef.Shape.Type == "structure" {
1267-
containerFieldName = targetFieldName
1344+
// There may be a generator.yaml file that looks like this:
1345+
//
1346+
// resources:
1347+
// DBInstance:
1348+
// fields:
1349+
// DBSecurityGroups:
1350+
// set:
1351+
// - method: ReadOne
1352+
// from: DBSecurityGroupName
1353+
//
1354+
// And this would indicate to the code generator that the
1355+
// Spec.DBSecurityGroups field should be set from the value of the
1356+
// Output shape's DBSecurityGroups..DBSecurityGroupName fields.
1357+
if targetSetCfg != nil && targetSetCfg.From != nil {
1358+
if sourceMemberShapeRef, found := sourceShape.MemberRef.Shape.MemberRefs[*targetSetCfg.From]; found {
1359+
out += setResourceForScalar(
1360+
elemVarName,
1361+
fmt.Sprintf("*%s.%s", iterVarName, *targetSetCfg.From),
1362+
sourceMemberShapeRef,
1363+
indentLevel+1,
1364+
)
1365+
} else {
1366+
// This is a bug in the code generation if this occurs...
1367+
msg := fmt.Sprintf(
1368+
"could not find targetSetCfg.From %s in struct member type.",
1369+
*targetSetCfg.From,
1370+
)
1371+
panic(msg)
1372+
}
1373+
} else {
1374+
// f0elem0 = *f0iter0
1375+
//
1376+
// or
1377+
//
1378+
// f0elem0.SetMyField(*f0iter0)
1379+
containerFieldName := ""
1380+
if sourceShape.MemberRef.Shape.Type == "structure" {
1381+
containerFieldName = targetFieldName
1382+
}
1383+
out += setResourceForContainer(
1384+
cfg, r,
1385+
containerFieldName,
1386+
elemVarName,
1387+
&targetShape.MemberRef,
1388+
targetSetCfg,
1389+
iterVarName,
1390+
&sourceShape.MemberRef,
1391+
indentLevel+1,
1392+
)
12681393
}
1269-
out += setResourceForContainer(
1270-
cfg, r,
1271-
containerFieldName,
1272-
elemVarName,
1273-
&targetShape.MemberRef,
1274-
iterVarName,
1275-
&sourceShape.MemberRef,
1276-
indentLevel+1,
1277-
)
12781394
addressOfVar := ""
1279-
switch sourceShape.MemberRef.Shape.Type {
1395+
switch targetShape.MemberRef.Shape.Type {
12801396
case "structure", "list", "map":
12811397
break
12821398
default:
@@ -1299,6 +1415,8 @@ func setResourceForMap(
12991415
targetVarName string,
13001416
// Shape Ref of the target map field
13011417
targetShapeRef *awssdkmodel.ShapeRef,
1418+
// SetFieldConfig of the *target* field
1419+
targetSetCfg *ackgenconfig.SetFieldConfig,
13021420
// The struct or struct field that we access our source value from
13031421
sourceVarName string,
13041422
// ShapeRef of the source map field
@@ -1332,6 +1450,7 @@ func setResourceForMap(
13321450
containerFieldName,
13331451
valVarName,
13341452
&targetShape.ValueRef,
1453+
nil,
13351454
valIterVarName,
13361455
&sourceShape.ValueRef,
13371456
indentLevel+1,

0 commit comments

Comments
 (0)