From d84ea0b9f4b6deb8ea48707e9bef016e65d269da Mon Sep 17 00:00:00 2001 From: Nicholas Thomson Date: Sat, 4 Feb 2023 01:29:20 +0000 Subject: [PATCH] Fix nested struct detection --- pkg/generate/ack/controller.go | 2 +- pkg/generate/code/resource_reference.go | 117 +++++++++--------- pkg/generate/code/resource_reference_test.go | 23 ++-- templates/pkg/resource/references.go.tpl | 63 +--------- ...references_read_referenced_resource.go.tpl | 54 -------- 5 files changed, 74 insertions(+), 185 deletions(-) delete mode 100644 templates/pkg/resource/references_read_referenced_resource.go.tpl diff --git a/pkg/generate/ack/controller.go b/pkg/generate/ack/controller.go index c4443b85..ff19a68b 100644 --- a/pkg/generate/ack/controller.go +++ b/pkg/generate/ack/controller.go @@ -183,7 +183,7 @@ var ( return code.InitializeNestedStructField(r, sourceVarName, f, apiPkgImportName, indentLevel) }, - "GoCodeReferenceForField": func(r *ackmodel.CRD, f *ackmodel.Field, sourceVarName string, indentLevel int) string { + "GoCodeResolveReference": func(r *ackmodel.CRD, f *ackmodel.Field, sourceVarName string, indentLevel int) string { return code.ResolveReferencesForField(r, f, sourceVarName, indentLevel) }, } diff --git a/pkg/generate/code/resource_reference.go b/pkg/generate/code/resource_reference.go index 7c72f564..cb275173 100644 --- a/pkg/generate/code/resource_reference.go +++ b/pkg/generate/code/resource_reference.go @@ -190,55 +190,55 @@ func ReferenceFieldsPresent( // condition and updating the concrete field with the referenced value. // Sample code: // -// if ko.Spec.SecurityGroupRefs != nil && -// len(ko.Spec.SecurityGroupRefs) > 0 { -// resolvedReferences := []*string{} -// for _, arrw := range ko.Spec.SecurityGroupRefs { -// arr := arrw.From -// if arr == nil || arr.Name == nil || *arr.Name == "" { -// return fmt.Errorf("provided resource reference is nil or empty") -// } -// namespacedName := types.NamespacedName{ -// Namespace: namespace, -// Name: *arr.Name, -// } -// obj := ec2apitypes.SecurityGroup{} -// err := apiReader.Get(ctx, namespacedName, &obj) -// if err != nil { -// return err -// } -// var refResourceSynced, refResourceTerminal bool -// for _, cond := range obj.Status.Conditions { -// if cond.Type == ackv1alpha1.ConditionTypeResourceSynced && -// cond.Status == corev1.ConditionTrue { -// refResourceSynced = true -// } -// if cond.Type == ackv1alpha1.ConditionTypeTerminal && -// cond.Status == corev1.ConditionTrue { -// refResourceTerminal = true -// } -// } -// if refResourceTerminal { -// return ackerr.ResourceReferenceTerminalFor( -// "SecurityGroup", -// namespace, *arr.Name) -// } -// if !refResourceSynced { -// return ackerr.ResourceReferenceNotSyncedFor( -// "SecurityGroup", -// namespace, *arr.Name) +// ``` +// refVal := "" +// +// if ko.Spec.TargetRef != nil && ko.Spec.TargetRef.From != nil { +// arr := ko.Spec.TargetRef.From +// if arr == nil || arr.Name == nil || *arr.Name == "" { +// return fmt.Errorf("provided resource reference is nil or empty") +// } +// namespacedName := types.NamespacedName{ +// Namespace: namespace, +// Name: *arr.Name, +// } +// obj := svcapitypes.Integration{} +// err := apiReader.Get(ctx, namespacedName, &obj) +// if err != nil { +// return err +// } +// var refResourceSynced, refResourceTerminal bool +// for _, cond := range obj.Status.Conditions { +// if cond.Type == ackv1alpha1.ConditionTypeResourceSynced && +// cond.Status == corev1.ConditionTrue { +// refResourceSynced = true // } -// if obj.Status.ID == nil { -// return ackerr.ResourceReferenceMissingTargetFieldFor( -// "SecurityGroup", -// namespace, *arr.Name, -// "Status.ID") +// if cond.Type == ackv1alpha1.ConditionTypeTerminal && +// cond.Status == corev1.ConditionTrue { +// refResourceTerminal = true // } -// referencedValue := string(*obj.Status.ID) -// resolvedReferences = append(resolvedReferences, &referencedValue) // } -// ko.Spec.SecurityGroupIDs = resolvedReferences +// if refResourceTerminal { +// return ackerr.ResourceReferenceTerminalFor( +// "Integration", +// namespace, *arr.Name) +// } +// if !refResourceSynced { +// return ackerr.ResourceReferenceNotSyncedFor( +// "Integration", +// namespace, *arr.Name) +// } +// if obj.Status.IntegrationID == nil { +// return ackerr.ResourceReferenceMissingTargetFieldFor( +// "Integration", +// namespace, *arr.Name, +// "Status.IntegrationID") +// } +// refVal = string(*obj.Status.IntegrationID) // } +// +// ko.Spec.Target = &refVal +// ``` func ResolveReferencesForField(r *model.CRD, field *model.Field, sourceVarName string, indentLevel int) string { fp := fieldpath.FromString(field.Path) @@ -248,7 +248,6 @@ func ResolveReferencesForField(r *model.CRD, field *model.Field, sourceVarName s fieldAccessPrefix := fmt.Sprintf("%s%s", sourceVarName, r.Config().PrefixConfig.SpecField) targetVarName := fmt.Sprintf("%s%s.%s", sourceVarName, r.Config().PrefixConfig.SpecField, field.Path) - hasSeenList := false for idx := 0; idx < fp.Size(); idx++ { curFP := fp.CopyAt(idx).String() cur, ok := r.Fields[curFP] @@ -259,26 +258,23 @@ func ResolveReferencesForField(r *model.CRD, field *model.Field, sourceVarName s ref := cur.ShapeRef indent := strings.Repeat("\t", indentLevel+idx) - fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, fp.At(idx)) if ref.Shape.Type == "structure" { - if hasSeenList { - // 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 community#1291 for more details - panic("references within structs inside lists aren't supported") - } + 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) } else if ref.Shape.Type == "list" { - if hasSeenList { + 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("references within lists inside lists aren't supported") } + fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, fp.At(idx)) - hasSeenList = true iterVarName := fmt.Sprintf("iter%d", idx) refsTarget := "refVals" @@ -295,6 +291,9 @@ func ResolveReferencesForField(r *model.CRD, field *model.Field, sourceVarName s } else { // base case for single references refTarget := "refVal" + fieldAccessPrefix = fmt.Sprintf("%s.%s", fieldAccessPrefix, cur.GetReferenceFieldName().Camel) + + outPrefix += fmt.Sprintf("%s%s := \"\"\n", indent, refTarget) outPrefix += resolveSingleReference(field, fieldAccessPrefix, refTarget, false, indentLevel+idx) outPrefix += fmt.Sprintf("%s%s = &%s\n", indent, targetVarName, refTarget) } @@ -358,9 +357,9 @@ func resolveSingleReference(field *model.Field, sourceVarName string, targetVarN } if shouldAppendTarget { - out += fmt.Sprintf("%s\t%s = append(%s, string(*obj.%s))\n", indent, targetVarName, targetVarName, field.FieldConfig.References.Path) + out += fmt.Sprintf("%s\t%s = append(%s, obj.%s)\n", indent, targetVarName, targetVarName, field.FieldConfig.References.Path) } else { - out += fmt.Sprintf("%s\t%s := string(*obj.%s)\n", indent, targetVarName, field.FieldConfig.References.Path) + out += fmt.Sprintf("%s\t%s = string(*obj.%s)\n", indent, targetVarName, field.FieldConfig.References.Path) } out += fmt.Sprintf("%s}\n", indent) diff --git a/pkg/generate/code/resource_reference_test.go b/pkg/generate/code/resource_reference_test.go index 98d07011..a9e678ac 100644 --- a/pkg/generate/code/resource_reference_test.go +++ b/pkg/generate/code/resource_reference_test.go @@ -211,8 +211,9 @@ func Test_ReferenceForField_SingleReference(t *testing.T) { crd := testutil.GetCRDByName(t, g, "Integration") require.NotNil(crd) expected := - ` if ko.Spec.APIID != nil && ko.Spec.APIID.From != nil { - arr := ko.Spec.APIID.From + ` refVal := "" + 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") } @@ -252,7 +253,7 @@ func Test_ReferenceForField_SingleReference(t *testing.T) { namespace, *arr.Name, "Status.APIID") } - refVal := string(*obj.Status.APIID) + refVal = string(*obj.Status.APIID) } ko.Spec.APIID = &refVal ` @@ -316,7 +317,7 @@ func Test_ReferenceForField_SliceOfReferences(t *testing.T) { namespace, *arr.Name, "Status.ID") } - refVals = append(refVals, string(*obj.Status.ID)) + refVals = append(refVals, obj.Status.ID) } } ko.Spec.SecurityGroupIDs = refVals @@ -339,8 +340,9 @@ func Test_ReferenceForField_NestedSingleReference(t *testing.T) { require.NotNil(crd) expected := ` if ko.Spec.JWTConfiguration != nil { - if ko.Spec.JWTConfiguration.Issuer != nil && ko.Spec.JWTConfiguration.Issuer.From != nil { - arr := ko.Spec.JWTConfiguration.Issuer.From + refVal := "" + 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") } @@ -380,7 +382,7 @@ func Test_ReferenceForField_NestedSingleReference(t *testing.T) { namespace, *arr.Name, "Status.APIID") } - refVal := string(*obj.Status.APIID) + refVal = string(*obj.Status.APIID) } ko.Spec.JWTConfiguration.Issuer = &refVal } @@ -406,8 +408,9 @@ func Test_ReferenceForField_SingleReference_DeeplyNested(t *testing.T) { expected := ` if ko.Spec.Logging != nil { if ko.Spec.Logging.LoggingEnabled != nil { - if ko.Spec.Logging.LoggingEnabled.TargetBucket != nil && ko.Spec.Logging.LoggingEnabled.TargetBucket.From != nil { - arr := ko.Spec.Logging.LoggingEnabled.TargetBucket.From + refVal := "" + 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") } @@ -447,7 +450,7 @@ func Test_ReferenceForField_SingleReference_DeeplyNested(t *testing.T) { namespace, *arr.Name, "Spec.Name") } - refVal := string(*obj.Spec.Name) + refVal = string(*obj.Spec.Name) } ko.Spec.Logging.LoggingEnabled.TargetBucket = &refVal } diff --git a/templates/pkg/resource/references.go.tpl b/templates/pkg/resource/references.go.tpl index d398d690..c55eb71d 100644 --- a/templates/pkg/resource/references.go.tpl +++ b/templates/pkg/resource/references.go.tpl @@ -115,67 +115,8 @@ func resolveReferenceFor{{ $field.FieldPathWithUnderscore }}( } {{ end -}} -{{- $fp := ConstructFieldPath $field.Path -}} -{{ $_ := $fp.Pop -}} -{{ $isNested := gt $fp.Size 0 -}} -{{ $isList := eq $field.ShapeRef.Shape.Type "list" -}} -{{ if and (not $isList) (not $isNested) -}} - if ko.Spec.{{ $field.ReferenceFieldPath }} != nil && - ko.Spec.{{ $field.ReferenceFieldPath }}.From != nil { - arr := ko.Spec.{{ $field.ReferenceFieldPath }}.From -{{ template "read_referenced_resource_and_validate" $field }} - referencedValue := string(*obj.{{ $field.FieldConfig.References.Path }}) - ko.Spec.{{ $field.Path }} = &referencedValue - } -{{ else if not $isNested -}} - if ko.Spec.{{ $field.ReferenceFieldPath }} != nil && - len(ko.Spec.{{ $field.ReferenceFieldPath }}) > 0 { - resolvedReferences := []*string{} - for _, arrw := range ko.Spec.{{ $field.ReferenceFieldPath }} { - arr := arrw.From -{{ template "read_referenced_resource_and_validate" $field }} - referencedValue := string(*obj.{{ $field.FieldConfig.References.Path }}) - resolvedReferences = append(resolvedReferences, &referencedValue) - } - ko.Spec.{{ $field.Path }} = resolvedReferences - } - -{{ else }} -{{ $parentField := index .CRD.Fields $fp.String }} -{{ if eq $parentField.ShapeRef.Shape.Type "list" -}} - if len(ko.Spec.{{ $parentField.Path }}) > 0 { - for _, elem := range ko.Spec.{{ $parentField.Path }} { - arrw := elem.{{ $field.GetReferenceFieldName.Camel }} - - if arrw == nil || arrw.From == nil { - continue - } - - arr := arrw.From - if arr.Name == nil || *arr.Name == "" { - return fmt.Errorf("provided resource reference is nil or empty") - } - -{{ template "read_referenced_resource_and_validate" $field }} - referencedValue := string(*obj.{{ $field.FieldConfig.References.Path }}) - elem.{{ $field.Names.Camel }} = &referencedValue - } - } -{{ else -}} - if ko.Spec.{{ $field.ReferenceFieldPath }} != nil && - len(ko.Spec.{{ $field.ReferenceFieldPath }}) > 0 { - resolvedReferences := []*string{} - for _, arrw := range ko.Spec.{{ $field.ReferenceFieldPath }} { - arr := arrw.From -{{ template "read_referenced_resource_and_validate" $field }} - referencedValue := string(*obj.{{ $field.FieldConfig.References.Path }}) - resolvedReferences = append(resolvedReferences, &referencedValue) - } - ko.Spec.{{ $field.Path }} = resolvedReferences - } -{{ end -}} +{{ GoCodeResolveReference .CRD $field "ko" 1 }} return nil } {{ end -}} -{{ end -}} -{{ end -}} +{{ end -}} \ No newline at end of file diff --git a/templates/pkg/resource/references_read_referenced_resource.go.tpl b/templates/pkg/resource/references_read_referenced_resource.go.tpl deleted file mode 100644 index 72349b5d..00000000 --- a/templates/pkg/resource/references_read_referenced_resource.go.tpl +++ /dev/null @@ -1,54 +0,0 @@ -{{/* -read_referenced_resource_and_validate" template should be invoked with a field as -parameter -Ex: {{ template "read_referenced_resource_and_validate" $field }} -Where field is of type 'Field' from aws-controllers-k8s/code-generator/pkg/model - */}} -{{- define "read_referenced_resource_and_validate" -}} - if arr == nil || arr.Name == nil || *arr.Name == "" { - return fmt.Errorf("provided resource reference is nil or empty") - } - namespacedName := types.NamespacedName{ - Namespace: namespace, - Name: *arr.Name, - } - {{ if eq .FieldConfig.References.ServiceName "" -}} - obj := svcapitypes.{{ .FieldConfig.References.Resource }}{} - {{ else -}} - obj := {{ .ReferencedServiceName }}apitypes.{{ .FieldConfig.References.Resource }}{} - {{ end -}} - err := apiReader.Get(ctx, namespacedName, &obj) - if err != nil { - return err - } - var refResourceSynced, refResourceTerminal bool - for _, cond := range obj.Status.Conditions { - if cond.Type == ackv1alpha1.ConditionTypeResourceSynced && - cond.Status == corev1.ConditionTrue { - refResourceSynced = true - } - if cond.Type == ackv1alpha1.ConditionTypeTerminal && - cond.Status == corev1.ConditionTrue { - refResourceTerminal = true - } - } - if refResourceTerminal { - return ackerr.ResourceReferenceTerminalFor( - "{{ .FieldConfig.References.Resource }}", - namespace, *arr.Name) - } - if !refResourceSynced { - return ackerr.ResourceReferenceNotSyncedFor( - "{{ .FieldConfig.References.Resource }}", - namespace, *arr.Name) - } - {{ $nilCheck := CheckNilReferencesPath . "obj" -}} - {{ if not (eq $nilCheck "") -}} - if {{ $nilCheck }} { - return ackerr.ResourceReferenceMissingTargetFieldFor( - "{{ .FieldConfig.References.Resource }}", - namespace, *arr.Name, - "{{ .FieldConfig.References.Path }}") - } - {{- end -}} -{{- end -}}