Skip to content

Commit 2eb18c5

Browse files
vijtrip2vijtrip2
authored
Reference field name updates (#278)
Description of changes: * Refactor adding ReferenceField in CRD inside AddSpecField method * Create new "Field.GetReferenceFieldName" method to generate ReferenceFieldName from corresponding Field * Change the ReferenceFieldName from just adding "Ref" suffix to "Singular(FieldName - IdentifierSuffix) + Ref(s)" * Uncomment the wait for ResourceSynced condition before referencing a resource * Update comments and docs to use "APIRef" instead of "APIIDRef" By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent c7d9f6b commit 2eb18c5

File tree

11 files changed

+142
-56
lines changed

11 files changed

+142
-56
lines changed

‎pkg/generate/ack/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ var (
152152
"GoCodeIncompleteLateInitialization": func(r *ackmodel.CRD, resVarName string, indentLevel int) string {
153153
return code.IncompleteLateInitialization(r.Config(), r, resVarName, indentLevel)
154154
},
155-
"GoCodeReferencesValidation": func(r *ackmodel.CRD, sourceVarName string, referenceFieldSuffix string, indentLevel int) string {
156-
return code.ReferenceFieldsValidation(r, sourceVarName, referenceFieldSuffix, indentLevel)
155+
"GoCodeReferencesValidation": func(r *ackmodel.CRD, sourceVarName string, indentLevel int) string {
156+
return code.ReferenceFieldsValidation(r, sourceVarName, indentLevel)
157157
},
158158
"GoCodeContainsReferences": func(r *ackmodel.CRD, sourceVarName string) string {
159159
return code.ReferenceFieldsPresent(r, sourceVarName)

‎pkg/generate/code/resource_reference.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,15 @@ import (
2323
// ReferenceFieldsValidation produces the go code to validate reference field and
2424
// corresponding identifier field.
2525
// Sample code:
26-
// if ko.Spec.APIIDRef != nil && ko.Spec.APIID != nil {
27-
// return ackerr.ResourceReferenceAndIDNotSupportedFor("APIID", "APIIDRef")
26+
// if ko.Spec.APIRef != nil && ko.Spec.APIID != nil {
27+
// return ackerr.ResourceReferenceAndIDNotSupportedFor("APIID", "APIRef")
2828
// }
29-
// if ko.Spec.APIIDRef == nil && ko.Spec.APIID == nil {
30-
// return ackerr.ResourceReferenceOrIDRequiredFor("APIID", "APIIDRef")
29+
// if ko.Spec.APIRef == nil && ko.Spec.APIID == nil {
30+
// return ackerr.ResourceReferenceOrIDRequiredFor("APIID", "APIRef")
3131
// }
3232
func ReferenceFieldsValidation(
3333
crd *model.CRD,
3434
sourceVarName string,
35-
referenceFieldSuffix string,
3635
indentLevel int,
3736
) string {
3837
out := ""
@@ -41,25 +40,24 @@ func ReferenceFieldsValidation(
4140
indent := strings.Repeat("\t", indentLevel)
4241
// Validation to make sure both target field and reference are
4342
// not present at the same time in desired resource
44-
out += fmt.Sprintf("%sif %s.Spec.%s%s != nil"+
45-
" && %s.Spec.%s != nil {\n", indent, sourceVarName, field.Names.Camel,
46-
referenceFieldSuffix, sourceVarName, field.Names.Camel)
43+
out += fmt.Sprintf("%sif %s.Spec.%s != nil"+
44+
" && %s.Spec.%s != nil {\n", indent, sourceVarName,
45+
field.GetReferenceFieldName().Camel, sourceVarName, field.Names.Camel)
4746
out += fmt.Sprintf("%s\treturn "+
48-
"ackerr.ResourceReferenceAndIDNotSupportedFor(\"%s\", \"%s%s\")\n",
49-
indent,field.Names.Camel, field.Names.Camel, referenceFieldSuffix)
47+
"ackerr.ResourceReferenceAndIDNotSupportedFor(\"%s\", \"%s\")\n",
48+
indent, field.Names.Camel, field.GetReferenceFieldName().Camel)
5049
out += fmt.Sprintf("%s}\n", indent)
5150

5251
// If the field is required, make sure either Ref or original
5352
// field is present in the resource
5453
if field.IsRequired() {
55-
out += fmt.Sprintf("%sif %s.Spec.%s%s == nil &&"+
54+
out += fmt.Sprintf("%sif %s.Spec.%s == nil &&"+
5655
" %s.Spec.%s == nil {\n", indent, sourceVarName,
57-
field.Names.Camel, referenceFieldSuffix, sourceVarName,
56+
field.GetReferenceFieldName().Camel, sourceVarName,
5857
field.Names.Camel)
5958
out += fmt.Sprintf("%s\treturn "+
60-
"ackerr.ResourceReferenceOrIDRequiredFor(\"%s\", \"%s%s\")\n",
61-
indent, field.Names.Camel, field.Names.Camel,
62-
referenceFieldSuffix)
59+
"ackerr.ResourceReferenceOrIDRequiredFor(\"%s\", \"%s\")\n",
60+
indent, field.Names.Camel, field.GetReferenceFieldName().Camel)
6361
out += fmt.Sprintf("%s}\n", indent)
6462
}
6563
}
@@ -71,7 +69,7 @@ func ReferenceFieldsValidation(
7169
// a non-nil reference field is present in a resource. This checks helps in deciding
7270
// whether ACK.ReferencesResolved condition should be added to resource status
7371
// Sample Code:
74-
// return false || ko.Spec.APIIDRef != nil
72+
// return false || ko.Spec.APIRef != nil
7573
func ReferenceFieldsPresent(
7674
crd *model.CRD,
7775
sourceVarName string,

‎pkg/generate/code/resource_reference_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ func Test_ReferenceFieldsValidation_NoReferenceConfig(t *testing.T) {
2828

2929
g := testutil.NewModelForServiceWithOptions(t, "apigatewayv2",
3030
&testutil.TestingModelOptions{
31-
GeneratorConfigFile: "generator-with-reference.yaml",
32-
})
31+
GeneratorConfigFile: "generator-with-reference.yaml",
32+
})
3333

3434
crd := testutil.GetCRDByName(t, g, "Api")
3535
require.NotNil(crd)
3636
expected := ""
37-
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", "Ref", 1))
37+
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", 1))
3838
}
3939

4040
func Test_ReferenceFieldsValidation_SingleReference(t *testing.T) {
@@ -49,14 +49,14 @@ func Test_ReferenceFieldsValidation_SingleReference(t *testing.T) {
4949
crd := testutil.GetCRDByName(t, g, "Integration")
5050
require.NotNil(crd)
5151
expected :=
52-
` if ko.Spec.APIIDRef != nil && ko.Spec.APIID != nil {
53-
return ackerr.ResourceReferenceAndIDNotSupportedFor("APIID", "APIIDRef")
52+
` if ko.Spec.APIRef != nil && ko.Spec.APIID != nil {
53+
return ackerr.ResourceReferenceAndIDNotSupportedFor("APIID", "APIRef")
5454
}
55-
if ko.Spec.APIIDRef == nil && ko.Spec.APIID == nil {
56-
return ackerr.ResourceReferenceOrIDRequiredFor("APIID", "APIIDRef")
55+
if ko.Spec.APIRef == nil && ko.Spec.APIID == nil {
56+
return ackerr.ResourceReferenceOrIDRequiredFor("APIID", "APIRef")
5757
}
5858
`
59-
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", "Ref", 1))
59+
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", 1))
6060
}
6161

6262
func Test_ReferenceFieldsValidation_SliceOfReferences(t *testing.T) {
@@ -73,11 +73,11 @@ func Test_ReferenceFieldsValidation_SliceOfReferences(t *testing.T) {
7373
crd := testutil.GetCRDByName(t, g, "VpcLink")
7474
require.NotNil(crd)
7575
expected :=
76-
` if ko.Spec.SecurityGroupIDsRef != nil && ko.Spec.SecurityGroupIDs != nil {
77-
return ackerr.ResourceReferenceAndIDNotSupportedFor("SecurityGroupIDs", "SecurityGroupIDsRef")
76+
` if ko.Spec.SecurityGroupRefs != nil && ko.Spec.SecurityGroupIDs != nil {
77+
return ackerr.ResourceReferenceAndIDNotSupportedFor("SecurityGroupIDs", "SecurityGroupRefs")
7878
}
7979
`
80-
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", "Ref", 1))
80+
assert.Equal(expected, code.ReferenceFieldsValidation(crd, "ko", 1))
8181
}
8282

8383
func Test_ReferenceFieldsPresent_NoReferenceConfig(t *testing.T) {
@@ -106,7 +106,7 @@ func Test_ReferenceFieldsPresent_SingleReference(t *testing.T) {
106106

107107
crd := testutil.GetCRDByName(t, g, "Integration")
108108
require.NotNil(crd)
109-
expected := "false || ko.Spec.APIIDRef != nil"
109+
expected := "false || ko.Spec.APIRef != nil"
110110
assert.Equal(expected, code.ReferenceFieldsPresent(crd, "ko"))
111111
}
112112

@@ -123,6 +123,6 @@ func Test_ReferenceFieldsPresent_SliceOfReferences(t *testing.T) {
123123
// just to test code generation for slices of reference
124124
crd := testutil.GetCRDByName(t, g, "VpcLink")
125125
require.NotNil(crd)
126-
expected := "false || ko.Spec.SecurityGroupIDsRef != nil"
126+
expected := "false || ko.Spec.SecurityGroupRefs != nil"
127127
assert.Equal(expected, code.ReferenceFieldsPresent(crd, "ko"))
128-
}
128+
}

‎pkg/generate/config/field.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,9 @@ type LateInitializeConfig struct {
278278
// resource: API
279279
// path: Status.APIID
280280
//```
281-
// The above configuration will result in generation of a new field 'APIIDRef'
281+
// The above configuration will result in generation of a new field 'APIRef'
282282
// of type 'AWSResourceReference' for ApiGatewayv2-Integration crd.
283-
// When 'APIIDRef' field is present in custom resource manifest, reconciler will
283+
// When 'APIRef' field is present in custom resource manifest, reconciler will
284284
// read the referred 'API' resource and copy the value from 'Status.APIID' in
285285
// 'Integration' resource's 'APIID' field
286286
type ReferencesConfig struct {

‎pkg/model/crd.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,15 @@ func (r *CRD) AddSpecField(
207207
}
208208
r.SpecFields[memberNames.Original] = f
209209
r.Fields[fPath] = f
210+
211+
// If this field has a ReferencesConfig, Add the new
212+
// Reference field inside Spec as well
213+
if fConfig != nil && fConfig.References != nil {
214+
referenceFieldNames := f.GetReferenceFieldName()
215+
rf := NewReferenceField(r, referenceFieldNames, shapeRef)
216+
r.SpecFields[referenceFieldNames.Original] = rf
217+
r.Fields[referenceFieldNames.Camel] = rf
218+
}
210219
}
211220

212221
// AddStatusField adds a new Field of a given name and shape into the Status

‎pkg/model/field.go

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414
package model
1515

1616
import (
17+
"fmt"
1718
"strings"
1819

20+
"github.com/gertd/go-pluralize"
21+
1922
ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
2023
"github.com/aws-controllers-k8s/code-generator/pkg/names"
2124
"github.com/aws-controllers-k8s/code-generator/pkg/util"
@@ -96,7 +99,7 @@ func (f *Field) GetSetterConfig(opType OpType) *ackgenconfig.SetFieldConfig {
9699
// path: Status.APIID
97100
// ```
98101
// For the above configuration, 'HasReference' for 'ApiId'(Original name) field
99-
// will return true because a corresponding 'ApiIdRef' field will be generated
102+
// will return true because a corresponding 'APIRef' field will be generated
100103
// by ACK code-generator
101104
func (f *Field) HasReference() bool {
102105
return f.FieldConfig != nil && f.FieldConfig.References != nil
@@ -111,6 +114,41 @@ func (f *Field) IsReference() bool {
111114
return trimmedGoType == "*ackv1alpha1.AWSResourceReferenceWrapper"
112115
}
113116

117+
// GetReferenceFieldName returns the corresponding Reference field name
118+
// Reference field name is generated by removing the identifier suffix like
119+
// 'Id', 'Arn', 'Name' etc and adding 'Ref(s)' as suffix.
120+
func (f *Field) GetReferenceFieldName() names.Names {
121+
if f.Names.Original == "" {
122+
panic(fmt.Sprintf("field with empty name inside crd %s", f.CRD.Names.Original))
123+
}
124+
refNamePrefix := f.Names.Original
125+
identifierSuffixes := []string{
126+
"id", "ids", "Id", "Ids", "ID", "IDs", "IDS",
127+
"Name", "Names", "NAME", "NAMEs", "NAMES",
128+
"Arn", "Arns", "ARN", "ARNs", "ARNS",
129+
}
130+
for _, suffix := range identifierSuffixes {
131+
if strings.HasSuffix(f.Names.Original, suffix) {
132+
refNamePrefix = strings.TrimSuffix(refNamePrefix, suffix)
133+
break
134+
}
135+
}
136+
if refNamePrefix == "" {
137+
panic("The corresponding field name for a reference field cannot" +
138+
" be just 'id(s)' or 'arn(s)' or 'name(s)'. Current value: " +
139+
f.Names.Original)
140+
}
141+
refName := refNamePrefix
142+
// If the shape of corresponding field is a list, singularize the refNamePrefix
143+
// and add Refs at the end
144+
if f.ShapeRef != nil && f.ShapeRef.Shape != nil && f.ShapeRef.Shape.Type == "list" {
145+
refName = fmt.Sprintf("%sRefs", pluralize.NewClient().Singular(refNamePrefix))
146+
} else {
147+
refName = fmt.Sprintf("%sRef", refNamePrefix)
148+
}
149+
return names.New(refName)
150+
}
151+
114152
// NewReferenceField returns a pointer to a new Field object.
115153
// The go-type of field is either slice of '*AWSResourceReferenceWrapper' or
116154
// '*AWSResourceReferenceWrapper' depending on whether 'shapeRef' parameter

‎pkg/model/field_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
package model_test
22

33
import (
4+
"fmt"
45
"testing"
56

7+
"github.com/aws-controllers-k8s/code-generator/pkg/names"
8+
9+
"github.com/aws/aws-sdk-go/private/model/api"
10+
611
"github.com/stretchr/testify/assert"
712
"github.com/stretchr/testify/require"
813

14+
"github.com/aws-controllers-k8s/code-generator/pkg/model"
915
"github.com/aws-controllers-k8s/code-generator/pkg/testutil"
1016
)
1117

@@ -142,3 +148,50 @@ func TestCustomFieldType(t *testing.T) {
142148
assert.Equal("*int64", myIntField.GoType)
143149
require.NotNil(myIntField.ShapeRef)
144150
}
151+
152+
func TestGetReferenceFieldName(t *testing.T) {
153+
assert := assert.New(t)
154+
155+
stringShape := api.ShapeRef{
156+
Shape: &api.Shape{
157+
Type: "string",
158+
},
159+
}
160+
161+
listShape := api.ShapeRef{
162+
Shape: &api.Shape{
163+
Type: "list",
164+
},
165+
}
166+
167+
testCases := []struct {
168+
fieldName string
169+
expectedReferenceFieldName string
170+
shapeRef *api.ShapeRef
171+
}{
172+
{"ClusterName", "ClusterRef", &stringShape},
173+
{"ClusterNames", "ClusterRefs", &listShape},
174+
{"ClusterARN", "ClusterRef", &stringShape},
175+
{"ClusterARNs", "ClusterRefs", &listShape},
176+
{"ClusterID", "ClusterRef", &stringShape},
177+
{"ClusterId", "ClusterRef", &stringShape},
178+
{"ClusterIds", "ClusterRefs", &listShape},
179+
{"ClusterIDs", "ClusterRefs", &listShape},
180+
{"Cluster", "ClusterRef", &stringShape},
181+
{"Clusters", "ClusterRefs", &listShape},
182+
// When the resource name indicates plural but it is singular. Ex: DHCPOptions
183+
{"Clusters", "ClustersRef", &stringShape},
184+
{"BlueDeploymentId", "BlueDeploymentRef", &stringShape},
185+
{"GreenDeploymentId", "GreenDeploymentRef", &stringShape},
186+
}
187+
188+
for _, tc := range testCases {
189+
f := model.Field{}
190+
f.ShapeRef = tc.shapeRef
191+
f.Names = names.New(tc.fieldName)
192+
referenceFieldName := f.GetReferenceFieldName().Camel
193+
msg := fmt.Sprintf("for %s, expected reference field name of %s but got %s",
194+
tc.fieldName, tc.expectedReferenceFieldName, referenceFieldName)
195+
assert.Equal(tc.expectedReferenceFieldName, referenceFieldName, msg)
196+
}
197+
}

‎pkg/model/model.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,6 @@ func (m *Model) GetCRDs() ([]*CRD, error) {
132132
continue
133133
}
134134
crd.AddSpecField(memberNames, memberShapeRef)
135-
136-
// If this field has ReferencesConfig, add corresponding
137-
// resource reference field in spec as well
138-
fConfig := m.cfg.ResourceFields(crdName)[fieldName]
139-
if fConfig != nil && fConfig.References != nil {
140-
referenceFieldNames := names.New(fieldName + "Ref")
141-
rf := NewReferenceField(crd, referenceFieldNames, memberShapeRef)
142-
crd.SpecFields[referenceFieldNames.Original] = rf
143-
crd.Fields[referenceFieldNames.Camel] = rf
144-
}
145135
}
146136

147137
// Now any additional Spec fields that are required from other API

‎pkg/model/model_apigwv2_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ func TestAPIGatewayV2_WithReference(t *testing.T) {
196196
assert.Equal("integration", crd.Names.Snake)
197197

198198
assert.NotNil(crd.SpecFields["ApiId"])
199-
assert.NotNil(crd.SpecFields["ApiIdRef"])
200-
assert.Equal("*ackv1alpha1.AWSResourceReferenceWrapper", crd.SpecFields["ApiIdRef"].GoType)
199+
assert.NotNil(crd.SpecFields["ApiRef"])
200+
assert.Equal("*ackv1alpha1.AWSResourceReferenceWrapper", crd.SpecFields["ApiRef"].GoType)
201201

202202
// List of References
203203
crd = getCRDByName("VpcLink", crds)
@@ -208,6 +208,6 @@ func TestAPIGatewayV2_WithReference(t *testing.T) {
208208
assert.Equal("vpc_link", crd.Names.Snake)
209209

210210
assert.NotNil(crd.SpecFields["SecurityGroupIds"])
211-
assert.NotNil(crd.SpecFields["SecurityGroupIdsRef"])
212-
assert.Equal("[]*ackv1alpha1.AWSResourceReferenceWrapper", crd.SpecFields["SecurityGroupIdsRef"].GoType)
211+
assert.NotNil(crd.SpecFields["SecurityGroupRefs"])
212+
assert.Equal("[]*ackv1alpha1.AWSResourceReferenceWrapper", crd.SpecFields["SecurityGroupRefs"].GoType)
213213
}

‎templates/pkg/resource/references.go.tpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (rm *resourceManager) ResolveReferences(
5757
// validateReferenceFields validates the reference field and corresponding
5858
// identifier field.
5959
func validateReferenceFields(ko *svcapitypes.{{ .CRD.Names.Camel }}) error {
60-
{{ GoCodeReferencesValidation .CRD "ko" "Ref" 1 -}}
60+
{{ GoCodeReferencesValidation .CRD "ko" 1 -}}
6161
return nil
6262
}
6363

@@ -69,7 +69,7 @@ func hasNonNilReferences(ko *svcapitypes.{{ .CRD.Names.Camel }}) bool {
6969

7070
{{ range $fieldName, $field := .CRD.Fields }}
7171
{{ if $field.HasReference }}
72-
{{ $refField := index .CRD.Fields (print $field.Names.Camel "Ref") }}
72+
{{ $refField := index .CRD.Fields $field.GetReferenceFieldName.Camel }}
7373
// resolveReferenceFor{{ $field.Names.Camel }} reads the resource referenced
7474
// from {{ $refField.Names.Camel }} field and sets the {{ $field.Names.Camel }}
7575
// from referenced resource

0 commit comments

Comments
 (0)