Skip to content

Commit

Permalink
Refactor reference resolution into Go code (#417)
Browse files Browse the repository at this point in the history
Continuation of #401

Refactors the resolveReferenceFor<Field> Go template code into direct Go code. This way we can be more intelligent about nested field references and support unit testing its output.

Breaking change:
The previous Go template produced valid, but ultimately faulty, code for references within lists of structs. These are not supported (see aws-controllers-k8s/community#1291). As such, this update produces a panic when it detects those types of references.

Co-authored-by: Amine <hilalyamine@gmail.com>
  • Loading branch information
RedbackThomson and a-hilaly authored Feb 16, 2023
1 parent e0eac49 commit 3c89a32
Show file tree
Hide file tree
Showing 7 changed files with 410 additions and 112 deletions.
13 changes: 13 additions & 0 deletions pkg/generate/ack/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ var (
"Dereference": func(s *string) string {
return *s
},
"AddToMap": func(m map[string]interface{}, k string, v interface{}) map[string]interface{} {
if len(m) == 0 {
m = make(map[string]interface{})
}
m[k] = v
return m
},
"Nil": func() interface{} {
return nil
},
"ResourceExceptionCode": func(r *ackmodel.CRD, httpStatusCode int) string {
return r.ExceptionCode(httpStatusCode)
},
Expand Down Expand Up @@ -183,6 +193,9 @@ var (
return code.InitializeNestedStructField(r, sourceVarName, f,
apiPkgImportName, indentLevel)
},
"GoCodeResolveReference": func(f *ackmodel.Field, sourceVarName string, indentLevel int) string {
return code.ResolveReferencesForField(f, sourceVarName, indentLevel)
},
}
)

Expand Down
120 changes: 118 additions & 2 deletions pkg/generate/code/resource_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func ReferenceFieldsValidation(
out += fmt.Sprintf("%sif %s.%s != nil"+
" && %s.%s != nil {\n", fIndent, pathVarPrefix, field.GetReferenceFieldName().Camel, pathVarPrefix, field.Names.Camel)
out += fmt.Sprintf("%s\treturn "+
"ackerr.ResourceReferenceAndIDNotSupportedFor(\"%s\", \"%s\")\n",
"ackerr.ResourceReferenceAndIDNotSupportedFor(%q, %q)\n",
fIndent, field.Path, field.ReferenceFieldPath())

// Close out all the curly braces with proper indentation
Expand All @@ -117,7 +117,7 @@ func ReferenceFieldsValidation(
" %s.%s == nil {\n", fIndent, pathVarPrefix,
field.ReferenceFieldPath(), pathVarPrefix, field.Path)
out += fmt.Sprintf("%s\treturn "+
"ackerr.ResourceReferenceOrIDRequiredFor(\"%s\", \"%s\")\n",
"ackerr.ResourceReferenceOrIDRequiredFor(%q, %q)\n",
fIndent, field.Path, field.ReferenceFieldPath())
out += fmt.Sprintf("%s}\n", fIndent)
}
Expand Down Expand Up @@ -185,6 +185,122 @@ func ReferenceFieldsPresent(
return iteratorsOut + returnOut
}

// ResolveReferencesForField produces Go code for accessing all references that
// are related to the given concrete field, determining whether its in a valid
// condition and updating the concrete field with the referenced value.
// Sample code (resolving a nested singular reference):
//
// ```
//
// if ko.Spec.APIRef != nil && ko.Spec.APIRef.From != nil {
// arr := ko.Spec.APIRef.From
// if arr == nil || arr.Name == nil || *arr.Name == "" {
// return fmt.Errorf("provided resource reference is nil or empty: APIRef")
// }
// obj := &svcapitypes.API{}
// if err := getReferencedResourceState_API(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
// return err
// }
// ko.Spec.APIID = (*string)(obj.Status.APIID)
// }
//
// ```
func ResolveReferencesForField(field *model.Field, sourceVarName string, indentLevel int) string {
r := field.CRD
fp := fieldpath.FromString(field.Path)

outPrefix := ""
outSuffix := ""

fieldAccessPrefix := fmt.Sprintf("%s%s", sourceVarName, r.Config().PrefixConfig.SpecField)
targetVarName := fmt.Sprintf("%s.%s", fieldAccessPrefix, field.Path)

for idx := 0; idx < fp.Size(); idx++ {
curFP := fp.CopyAt(idx).String()
cur, ok := r.Fields[curFP]
if !ok {
panic(fmt.Sprintf("unable to find field with path %q. crd: %q", curFP, r.Kind))
}

ref := cur.ShapeRef

indent := strings.Repeat("\t", indentLevel+idx)

switch ref.Shape.Type {
case ("structure"):
fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, fp.At(idx))

outPrefix += fmt.Sprintf("%sif %s != nil {\n", indent, fieldAccessPrefix)
outSuffix = fmt.Sprintf("%s}\n%s", indent, outSuffix)
case ("list"):
if (fp.Size() - idx) > 1 {
// TODO(nithomso): add support for structs nested within lists
// The logic for structs nested within lists needs to not only
// be added here, but also in a custom patching solution since
// it isn't supported by `StrategicMergePatch`
// see https://github.com/aws-controllers-k8s/community/issues/1291
panic(fmt.Errorf("references within lists inside lists aren't supported. crd: %q, path: %q", r.Kind, field.Path))
}
fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, cur.GetReferenceFieldName().Camel)

iterVarName := fmt.Sprintf("iter%d", idx)
aggRefName := fmt.Sprintf("resolved%d", idx)

// base case for references in a list
outPrefix += fmt.Sprintf("%sif len(%s) > 0 {\n", indent, fieldAccessPrefix)
outPrefix += fmt.Sprintf("%s\t%s := %s{}\n", indent, aggRefName, field.GoType)
outPrefix += fmt.Sprintf("%s\tfor _, %s := range %s {\n", indent, iterVarName, fieldAccessPrefix)

fieldAccessPrefix = iterVarName
outPrefix += fmt.Sprintf("%s\t\tarr := %s.From\n", indent, fieldAccessPrefix)
outPrefix += fmt.Sprintf("%s\t\tif arr == nil || arr.Name == nil || *arr.Name == \"\" {\n", indent)
outPrefix += fmt.Sprintf("%s\t\t\treturn fmt.Errorf(\"provided resource reference is nil or empty: %s\")\n", indent, field.ReferenceFieldPath())
outPrefix += fmt.Sprintf("%s\t\t}\n", indent)

outPrefix += getReferencedStateForField(field, indentLevel+idx+1)

outPrefix += fmt.Sprintf("%s\t\t%s = append(%s, (%s)(obj.%s))\n", indent, aggRefName, aggRefName, field.ShapeRef.Shape.MemberRef.GoType(), field.FieldConfig.References.Path)
outPrefix += fmt.Sprintf("%s\t}\n", indent)
outPrefix += fmt.Sprintf("%s\t%s = %s\n", indent, targetVarName, aggRefName)
outPrefix += fmt.Sprintf("%s}\n", indent)
case ("map"):
panic("references cannot be within a map")
default:
// base case for single references
fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, cur.GetReferenceFieldName().Camel)

outPrefix += fmt.Sprintf("%sif %s != nil && %s.From != nil {\n", indent, fieldAccessPrefix, fieldAccessPrefix)
outPrefix += fmt.Sprintf("%s\tarr := %s.From\n", indent, fieldAccessPrefix)
outPrefix += fmt.Sprintf("%s\tif arr == nil || arr.Name == nil || *arr.Name == \"\" {\n", indent)
outPrefix += fmt.Sprintf("%s\t\treturn fmt.Errorf(\"provided resource reference is nil or empty: %s\")\n", indent, field.ReferenceFieldPath())
outPrefix += fmt.Sprintf("%s\t}\n", indent)

outPrefix += getReferencedStateForField(field, indentLevel+idx)

outPrefix += fmt.Sprintf("%s\t%s = (%s)(obj.%s)\n", indent, targetVarName, field.GoType, field.FieldConfig.References.Path)
outPrefix += fmt.Sprintf("%s}\n", indent)
}
}

return outPrefix + outSuffix
}

func getReferencedStateForField(field *model.Field, indentLevel int) string {
out := ""
indent := strings.Repeat("\t", indentLevel)

if field.FieldConfig.References.ServiceName == "" {
out += fmt.Sprintf("%s\tobj := &svcapitypes.%s{}\n", indent, field.FieldConfig.References.Resource)
} else {
out += fmt.Sprintf("%s\tobj := &%sapitypes.%s{}\n", indent, field.ReferencedServiceName(), field.FieldConfig.References.Resource)
}
out += fmt.Sprintf("%s\tif err := getReferencedResourceState_%s(ctx, apiReader, obj, *arr.Name, namespace); err != nil {\n", indent, field.FieldConfig.References.Resource)
out += fmt.Sprintf("%s\t\treturn err\n", indent)
out += fmt.Sprintf("%s\t}\n", indent)

return out
}

func nestedStructNilCheck(path fieldpath.Path, fieldAccessPrefix string) string {
out := ""
fieldNamePrefix := ""
Expand Down
157 changes: 157 additions & 0 deletions pkg/generate/code/resource_reference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,160 @@ if ko.Spec.Routes != nil {
return false || (ko.Spec.VPCRef != nil)`
assert.Equal(expected, code.ReferenceFieldsPresent(crd, "ko"))
}

func Test_ResolveReferencesForField_SingleReference(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewModelForServiceWithOptions(t, "apigatewayv2",
&testutil.TestingModelOptions{
GeneratorConfigFile: "generator-with-reference.yaml",
})

crd := testutil.GetCRDByName(t, g, "Integration")
require.NotNil(crd)
expected :=
` if ko.Spec.APIRef != nil && ko.Spec.APIRef.From != nil {
arr := ko.Spec.APIRef.From
if arr == nil || arr.Name == nil || *arr.Name == "" {
return fmt.Errorf("provided resource reference is nil or empty: APIRef")
}
obj := &svcapitypes.API{}
if err := getReferencedResourceState_API(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
return err
}
ko.Spec.APIID = (*string)(obj.Status.APIID)
}
`

field := crd.Fields["APIID"]
assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
}

func Test_ResolveReferencesForField_ReferencingARN(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewModelForServiceWithOptions(t, "iam",
&testutil.TestingModelOptions{
GeneratorConfigFile: "generator.yaml",
})

crd := testutil.GetCRDByName(t, g, "User")
require.NotNil(crd)
expected :=
` if ko.Spec.PermissionsBoundaryRef != nil && ko.Spec.PermissionsBoundaryRef.From != nil {
arr := ko.Spec.PermissionsBoundaryRef.From
if arr == nil || arr.Name == nil || *arr.Name == "" {
return fmt.Errorf("provided resource reference is nil or empty: PermissionsBoundaryRef")
}
obj := &svcapitypes.Policy{}
if err := getReferencedResourceState_Policy(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
return err
}
ko.Spec.PermissionsBoundary = (*string)(obj.Status.ACKResourceMetadata.ARN)
}
`

field := crd.Fields["PermissionsBoundary"]
assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
}

func Test_ResolveReferencesForField_SliceOfReferences(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewModelForServiceWithOptions(t, "apigatewayv2",
&testutil.TestingModelOptions{
GeneratorConfigFile: "generator-with-reference.yaml",
})

crd := testutil.GetCRDByName(t, g, "VpcLink")
require.NotNil(crd)
expected :=
` if len(ko.Spec.SecurityGroupRefs) > 0 {
resolved0 := []*string{}
for _, iter0 := range ko.Spec.SecurityGroupRefs {
arr := iter0.From
if arr == nil || arr.Name == nil || *arr.Name == "" {
return fmt.Errorf("provided resource reference is nil or empty: SecurityGroupRefs")
}
obj := &ec2apitypes.SecurityGroup{}
if err := getReferencedResourceState_SecurityGroup(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
return err
}
resolved0 = append(resolved0, (*string)(obj.Status.ID))
}
ko.Spec.SecurityGroupIDs = resolved0
}
`

field := crd.Fields["SecurityGroupIDs"]
assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
}

func Test_ResolveReferencesForField_NestedSingleReference(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewModelForServiceWithOptions(t, "apigatewayv2",
&testutil.TestingModelOptions{
GeneratorConfigFile: "generator-with-nested-reference.yaml",
})

crd := testutil.GetCRDByName(t, g, "Authorizer")
require.NotNil(crd)
expected :=
` if ko.Spec.JWTConfiguration != nil {
if ko.Spec.JWTConfiguration.IssuerRef != nil && ko.Spec.JWTConfiguration.IssuerRef.From != nil {
arr := ko.Spec.JWTConfiguration.IssuerRef.From
if arr == nil || arr.Name == nil || *arr.Name == "" {
return fmt.Errorf("provided resource reference is nil or empty: JWTConfiguration.IssuerRef")
}
obj := &svcapitypes.API{}
if err := getReferencedResourceState_API(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
return err
}
ko.Spec.JWTConfiguration.Issuer = (*string)(obj.Status.APIID)
}
}
`

field := crd.Fields["JWTConfiguration.Issuer"]
assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
}

func Test_ResolveReferencesForField_SingleReference_DeeplyNested(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewModelForServiceWithOptions(t, "s3",
&testutil.TestingModelOptions{
GeneratorConfigFile: "generator-with-nested-references.yaml",
})

crd := testutil.GetCRDByName(t, g, "Bucket")
require.NotNil(crd)

// the Go template has the appropriate nil checks to ensure the parent path exists
expected :=
` if ko.Spec.Logging != nil {
if ko.Spec.Logging.LoggingEnabled != nil {
if ko.Spec.Logging.LoggingEnabled.TargetBucketRef != nil && ko.Spec.Logging.LoggingEnabled.TargetBucketRef.From != nil {
arr := ko.Spec.Logging.LoggingEnabled.TargetBucketRef.From
if arr == nil || arr.Name == nil || *arr.Name == "" {
return fmt.Errorf("provided resource reference is nil or empty: Logging.LoggingEnabled.TargetBucketRef")
}
obj := &svcapitypes.Bucket{}
if err := getReferencedResourceState_Bucket(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
return err
}
ko.Spec.Logging.LoggingEnabled.TargetBucket = (*string)(obj.Spec.Name)
}
}
}
`

field := crd.Fields["Logging.LoggingEnabled.TargetBucket"]
assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
}
27 changes: 26 additions & 1 deletion pkg/testdata/models/apis/iam/0000-00-00/generator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ignore:
- SAMLProvider
- ServiceLinkedRole
- ServiceSpecificCredential
- User
#- User
- VirtualMFADevice
resources:
Role:
Expand Down Expand Up @@ -43,3 +43,28 @@ resources:
type: "map[string]*bool"
MyCustomInteger:
type: "*int64"
User:
renames:
operations:
CreateUser:
input_fields:
UserName: Name
fields:
PermissionsBoundary:
references:
resource: Policy
path: Status.ACKResourceMetadata.ARN
set:
# The input and output shapes are different...
- from: PermissionsBoundary.PermissionsBoundaryArn
# In order to support attaching zero or more policies to a user, we use
# custom update code path code that uses the Attach/DetachUserPolicy API
# calls to manage the set of PolicyARNs attached to this User.
Policies:
type: "[]*string"
references:
resource: Policy
path: Status.ACKResourceMetadata.ARN
Tags:
compare:
is_ignored: true
Loading

0 comments on commit 3c89a32

Please sign in to comment.