Merge pull request #414 from A-Hilaly/revert-references

Revert resource reference resolution

Author: jljaco

pkg/generate/ack/controller.go

@@ -183,9 +183,6 @@ 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)
-		},
 	}
 )
 

pkg/generate/code/resource_reference.go

@@ -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(%q, %q)\n",
+				"ackerr.ResourceReferenceAndIDNotSupportedFor(\"%s\", \"%s\")\n",
 				fIndent, field.Path, field.ReferenceFieldPath())
 
 			// Close out all the curly braces with proper indentation
@@ -117,7 +117,7 @@ func ReferenceFieldsValidation(
 					" %s.%s == nil {\n", fIndent, pathVarPrefix,
 					field.ReferenceFieldPath(), pathVarPrefix, field.Path)
 				out += fmt.Sprintf("%s\treturn "+
-					"ackerr.ResourceReferenceOrIDRequiredFor(%q, %q)\n",
+					"ackerr.ResourceReferenceOrIDRequiredFor(\"%s\", \"%s\")\n",
 					fIndent, field.Path, field.ReferenceFieldPath())
 				out += fmt.Sprintf("%s}\n", fIndent)
 			}
@@ -185,111 +185,6 @@ 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.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 = 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, fp.At(idx))
-
-			iterVarName := fmt.Sprintf("iter%d", idx)
-
-			// base case for references in a list
-			outPrefix += fmt.Sprintf("%s%s = %s{}\n", indent, targetVarName, field.GoType)
-			outPrefix += fmt.Sprintf("%sfor _, %s := range %s {\n", indent, iterVarName, fieldAccessPrefix)
-
-			fieldAccessPrefix = iterVarName
-			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: \\%q\\\")\n", indent, field.ReferenceFieldPath())
-			outPrefix += fmt.Sprintf("%s\t}\n", indent)
-
-			outPrefix += fmt.Sprintf("%s\tif err := getReferencedResourceState_%s(ctx, apiReader, obj, *arr.Name, namespace); err != nil {\n", indent, field.FieldConfig.References.Resource)
-			outPrefix += fmt.Sprintf("%s\t\treturn err\n", indent)
-			outPrefix += fmt.Sprintf("%s\t}\n", indent)
-			outPrefix += fmt.Sprintf("%s\t%s = append(%s, obj.%s)\n", indent, targetVarName, targetVarName, field.FieldConfig.References.Path)
-			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: \\%q\\\")\n", indent, field.ReferenceFieldPath())
-			outPrefix += fmt.Sprintf("%s\t}\n", indent)
-
-			if field.FieldConfig.References.ServiceName == "" {
-				outPrefix += fmt.Sprintf("%s\tobj := &svcapitypes.%s{}\n", indent, field.FieldConfig.References.Resource)
-			} else {
-				outPrefix += fmt.Sprintf("%s\tobj := &%sapitypes.%s{}\n", indent, field.ReferencedServiceName(), field.FieldConfig.References.Resource)
-			}
-			outPrefix += fmt.Sprintf("%s\tif err := getReferencedResourceState_%s(ctx, apiReader, obj, *arr.Name, namespace); err != nil {\n", indent, field.FieldConfig.References.Resource)
-			outPrefix += fmt.Sprintf("%s\t\treturn err\n", indent)
-			outPrefix += fmt.Sprintf("%s\t}\n", indent)
-			outPrefix += fmt.Sprintf("%s\t%s = obj.%s\n", indent, targetVarName, field.FieldConfig.References.Path)
-			outPrefix += fmt.Sprintf("%s}\n", indent)
-		}
-	}
-
-	return outPrefix + outSuffix
-}
-
 func nestedStructNilCheck(path fieldpath.Path, fieldAccessPrefix string) string {
 	out := ""
 	fieldNamePrefix := ""

pkg/generate/code/resource_reference_test.go

@@ -198,127 +198,3 @@ 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 = obj.Status.APIID
-	}
-`
-
-	field := crd.Fields["APIID"]
-	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 :=
-		`	ko.Spec.SecurityGroupIDs = []*string{}
-	for _, iter0 := range ko.Spec.SecurityGroupIDs {
-		arr := iter0.From
-		if arr == nil || arr.Name == nil || *arr.Name == "" {
-			return fmt.Errorf("provided resource reference is nil or empty: \"SecurityGroupRefs"\")
-		}
-		if err := getReferencedResourceState_SecurityGroup(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
-			return err
-		}
-		ko.Spec.SecurityGroupIDs = append(ko.Spec.SecurityGroupIDs, obj.Status.ID)
-	}
-`
-
-	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 = 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 = obj.Spec.Name
-			}
-		}
-	}
-`
-
-	field := crd.Fields["Logging.LoggingEnabled.TargetBucket"]
-	assert.Equal(expected, code.ResolveReferencesForField(field, "ko", 1))
-}

pkg/testdata/models/apis/s3/0000-00-00/generator-with-nested-references.yaml

@@ -115,12 +115,72 @@ func resolveReferenceFor{{ $field.FieldPathWithUnderscore }}(
     }
 {{ end -}}
 
-{{ GoCodeResolveReference $field "ko" 1 }}
+{{- $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
+	}
 	return nil
 }
-
+{{ 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
+	}
+	return nil
+}
+{{ 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
+		}
+	}
+	return nil
+}
+{{ 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
+	}
+	return nil
+}
+{{ end -}}
+{{ end -}}
 {{ end -}}
 {{ end -}}
-