Skip to content

Commit 141cb9d

Browse files
brycahtabrycahta
authored
Refactor LateInitialize using FieldConfig Getters (#328)
Issue #, if available: aws-controllers-k8s/community#1184 Description of changes: * Relocate `FieldConfig` Getters to `field.go`; clarify names * Add `LateInitializeConfig` Getters * Update `late_initalize.go` to replace direct config access with Getters By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 302c31f commit 141cb9d

File tree

10 files changed

+126
-115
lines changed

10 files changed

+126
-115
lines changed

‎pkg/config/field.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
package config
1515

16+
import "strings"
17+
1618
// SourceFieldConfig instructs the code generator how to handle a field in the
1719
// Resource's SpecFields/StatusFields collection that takes its value from an
1820
// abnormal source -- in other words, not the Create operation's Input or
@@ -383,3 +385,59 @@ type FieldConfig struct {
383385
// into this type override...
384386
Type *string `json:"type,omitempty"`
385387
}
388+
389+
// GetFieldConfigs returns all FieldConfigs for a given resource as a map.
390+
// The map is keyed by the resource's field names after applying renames, if applicable.
391+
func (c *Config) GetFieldConfigs(resourceName string) map[string]*FieldConfig {
392+
if c == nil {
393+
return map[string]*FieldConfig{}
394+
}
395+
resourceConfig, ok := c.Resources[resourceName]
396+
if !ok {
397+
return map[string]*FieldConfig{}
398+
}
399+
return resourceConfig.Fields
400+
}
401+
402+
// GetFieldConfigByPath returns the FieldConfig provided a resource and path to associated field.
403+
func (c *Config) GetFieldConfigByPath(resourceName string, fieldPath string) *FieldConfig {
404+
if c == nil {
405+
return nil
406+
}
407+
for fPath, fConfig := range c.GetFieldConfigs(resourceName) {
408+
if strings.EqualFold(fPath, fieldPath) {
409+
return fConfig
410+
}
411+
}
412+
return nil
413+
}
414+
415+
// GetLateInitConfigs returns all LateInitializeConfigs for a given resource as a map.
416+
// The map is keyed by the resource's field names after applying renames, if applicable.
417+
func (c *Config) GetLateInitConfigs(resourceName string) map[string]*LateInitializeConfig {
418+
if c == nil {
419+
return nil
420+
}
421+
fieldNameToConfig := c.GetFieldConfigs(resourceName)
422+
fieldNameToLateInitConfig := make(map[string]*LateInitializeConfig)
423+
for fieldName := range fieldNameToConfig {
424+
lateInitConfig := c.GetLateInitConfigByPath(resourceName, fieldName)
425+
if lateInitConfig != nil {
426+
fieldNameToLateInitConfig[fieldName] = lateInitConfig
427+
}
428+
}
429+
return fieldNameToLateInitConfig
430+
}
431+
432+
// GetLateInitConfigByPath returns the LateInitializeConfig provided a resource and path to associated field.
433+
func (c *Config) GetLateInitConfigByPath(resourceName string, fieldPath string) *LateInitializeConfig {
434+
if c == nil {
435+
return nil
436+
}
437+
for fPath, fConfig := range c.GetFieldConfigs(resourceName) {
438+
if strings.EqualFold(fPath, fieldPath) {
439+
return fConfig.LateInitialize
440+
}
441+
}
442+
return nil
443+
}

‎pkg/config/resource.go‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
package config
1515

1616
import (
17-
"strings"
18-
1917
awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
2018

2119
"github.com/aws-controllers-k8s/code-generator/pkg/util"
@@ -417,44 +415,6 @@ func (c *Config) GetResourceConfig(resourceName string) *ResourceConfig {
417415
return &rc
418416
}
419417

420-
// GetResourceFields returns a map, keyed by target/renamed field name, of
421-
// FieldConfig struct pointers that instruct the code generator how to handle
422-
// the interpretation of special Resource fields (both Spec and Status)
423-
func (c *Config) GetResourceFields(resourceName string) map[string]*FieldConfig {
424-
if c == nil {
425-
return map[string]*FieldConfig{}
426-
}
427-
resourceConfig, ok := c.Resources[resourceName]
428-
if !ok {
429-
return map[string]*FieldConfig{}
430-
}
431-
return resourceConfig.Fields
432-
}
433-
434-
// GetResourceFieldByPath returns the FieldConfig for a field from
435-
// "resourceName" crd, where field.Path matches the passed "fieldPath" parameter.
436-
// This method performs the case-insensitive resource and fieldPath lookup.
437-
func (c *Config) GetResourceFieldByPath(resourceName string, fieldPath string) *FieldConfig {
438-
var resourceConfig ResourceConfig
439-
if c == nil {
440-
return nil
441-
}
442-
443-
for resName, resConfig := range c.Resources {
444-
if strings.EqualFold(resName, resourceName) {
445-
resourceConfig = resConfig
446-
break
447-
}
448-
}
449-
450-
for fPath, fConfig := range resourceConfig.Fields {
451-
if strings.EqualFold(fPath, fieldPath) {
452-
return fConfig
453-
}
454-
}
455-
return nil
456-
}
457-
458418
// GetCompareIgnoredFieldPaths returns the list of field paths to ignore when
459419
// comparing two different objects
460420
func (c *Config) GetCompareIgnoredFieldPaths(resourceName string) []string {

‎pkg/generate/code/compare.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func CompareResource(
9494
) string {
9595
out := "\n"
9696

97-
fieldConfigs := cfg.GetResourceFields(r.Names.Original)
97+
fieldConfigs := cfg.GetFieldConfigs(r.Names.Original)
9898

9999
// We need a deterministic order to traverse our top-level fields...
100100
specFieldNames := []string{}
@@ -544,7 +544,7 @@ func CompareStruct(
544544
) string {
545545
out := ""
546546

547-
fieldConfigs := cfg.GetResourceFields(r.Names.Original)
547+
fieldConfigs := cfg.GetFieldConfigs(r.Names.Original)
548548

549549
for _, memberName := range shape.MemberNames() {
550550
memberShapeRef := shape.MemberRefs[memberName]

‎pkg/generate/code/late_initialize.go‎

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -37,38 +37,22 @@ func FindLateInitializedFieldNames(
3737
) string {
3838
out := ""
3939
indent := strings.Repeat("\t", indentLevel)
40-
sortedFieldNames, _ := getSortedLateInitFieldsAndConfig(cfg, r)
41-
if len(sortedFieldNames) > 0 {
42-
out += fmt.Sprintf("%svar %s = []string{", indent, resVarName)
43-
for _, fName := range sortedFieldNames {
44-
out += fmt.Sprintf("%q,", fName)
45-
}
46-
out += "}\n"
47-
} else {
48-
out += fmt.Sprintf("%svar %s = []string{}\n", indent, resVarName)
40+
var lateInitFieldNames []string
41+
lateInitConfigs := cfg.GetLateInitConfigs(r.Names.Original)
42+
for fieldName := range lateInitConfigs {
43+
lateInitFieldNames = append(lateInitFieldNames, fieldName)
4944
}
50-
return out
51-
}
52-
53-
// getSortedLateInitFieldsAndConfig returns the field names in alphabetically sorted order which have LateInitialization
54-
// configuration inside generator config and also a map from fieldName to LateInitializationConfig.
55-
func getSortedLateInitFieldsAndConfig(
56-
cfg *ackgenconfig.Config,
57-
r *model.CRD,
58-
) ([]string, map[string]*ackgenconfig.LateInitializeConfig) {
59-
fieldNameToConfig := cfg.GetResourceFields(r.Names.Original)
60-
fieldNameToLateInitConfig := make(map[string]*ackgenconfig.LateInitializeConfig)
61-
sortedLateInitFieldNames := make([]string, 0)
62-
if len(fieldNameToConfig) > 0 {
63-
for fName, fConfig := range fieldNameToConfig {
64-
if fConfig != nil && fConfig.LateInitialize != nil {
65-
fieldNameToLateInitConfig[fName] = fConfig.LateInitialize
66-
sortedLateInitFieldNames = append(sortedLateInitFieldNames, fName)
67-
}
68-
}
69-
sort.Strings(sortedLateInitFieldNames)
45+
if len(lateInitFieldNames) == 0 {
46+
return fmt.Sprintf("%svar %s = []string{}\n", indent, resVarName)
47+
}
48+
// sort the slice to help with short circuiting AWSResourceManager.LateInitialize()
49+
sort.Strings(lateInitFieldNames)
50+
out += fmt.Sprintf("%svar %s = []string{", indent, resVarName)
51+
for _, fName := range lateInitFieldNames {
52+
out += fmt.Sprintf("%q,", fName)
7053
}
71-
return sortedLateInitFieldNames, fieldNameToLateInitConfig
54+
out += "}\n"
55+
return out
7256
}
7357

7458
// LateInitializeFromReadOne returns the gocode to set LateInitialization fields from the ReadOne output
@@ -147,14 +131,19 @@ func LateInitializeFromReadOne(
147131
) string {
148132
out := ""
149133
indent := strings.Repeat("\t", indentLevel)
150-
lateInitializedFieldNames, _ := getSortedLateInitFieldsAndConfig(cfg, r)
151-
if len(lateInitializedFieldNames) == 0 {
134+
var lateInitFieldNames []string
135+
lateInitConfigs := cfg.GetLateInitConfigs(r.Names.Original)
136+
for fieldName := range lateInitConfigs {
137+
lateInitFieldNames = append(lateInitFieldNames, fieldName)
138+
}
139+
if len(lateInitFieldNames) == 0 {
152140
return fmt.Sprintf("%sreturn %s", indent, targetResVarName)
153141
}
142+
sort.Strings(lateInitFieldNames)
154143
out += fmt.Sprintf("%sobservedKo := rm.concreteResource(%s).ko.DeepCopy()\n", indent, sourceResVarName)
155144
out += fmt.Sprintf("%slatestKo := rm.concreteResource(%s).ko.DeepCopy()\n", indent, targetResVarName)
156145
// TODO(vijat@): Add validation for correct field path in lateInitializedFieldNames
157-
for _, fName := range lateInitializedFieldNames {
146+
for _, fName := range lateInitFieldNames {
158147
// split the field name by period
159148
// each substring represents a field.
160149
fNameParts := strings.Split(fName, ".")
@@ -283,13 +272,17 @@ func IncompleteLateInitialization(
283272
) string {
284273
out := ""
285274
indent := strings.Repeat("\t", indentLevel)
286-
sortedLateInitFieldNames, _ := getSortedLateInitFieldsAndConfig(cfg, r)
287-
if len(sortedLateInitFieldNames) == 0 {
288-
out += fmt.Sprintf("%sreturn false", indent)
289-
return out
275+
var lateInitFieldNames []string
276+
lateInitConfigs := cfg.GetLateInitConfigs(r.Names.Original)
277+
for fieldName := range lateInitConfigs {
278+
lateInitFieldNames = append(lateInitFieldNames, fieldName)
279+
}
280+
if len(lateInitFieldNames) == 0 {
281+
return fmt.Sprintf("%sreturn false", indent)
290282
}
283+
sort.Strings(lateInitFieldNames)
291284
out += fmt.Sprintf("%sko := rm.concreteResource(%s).ko.DeepCopy()\n", indent, resVarName)
292-
for _, fName := range sortedLateInitFieldNames {
285+
for _, fName := range lateInitFieldNames {
293286
// split the field name by period
294287
// each substring represents a field.
295288
fNameParts := strings.Split(fName, ".")

‎pkg/generate/code/late_initialize_test.go‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func Test_FindLateInitializedFieldNames_EmptyFieldConfig(t *testing.T) {
3131
crd := testutil.GetCRDByName(t, g, "Repository")
3232
require.NotNil(crd)
3333
// NO fieldConfig
34-
assert.Empty(crd.Config().GetResourceFields(crd.Names.Original))
34+
assert.Empty(crd.Config().GetFieldConfigs(crd.Names.Original))
3535
expected :=
3636
` var lateInitializeFieldNames = []string{}
3737
`
@@ -47,8 +47,8 @@ func Test_FindLateInitializedFieldNames_NoLateInitializations(t *testing.T) {
4747
crd := testutil.GetCRDByName(t, g, "Repository")
4848
require.NotNil(crd)
4949
// FieldConfig without lateInitialize
50-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["Name"])
51-
assert.Nil(crd.Config().GetResourceFields(crd.Names.Original)["Name"].LateInitialize)
50+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"])
51+
assert.Nil(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"].LateInitialize)
5252
expected :=
5353
` var lateInitializeFieldNames = []string{}
5454
`
@@ -63,10 +63,10 @@ func Test_FindLateInitializedFieldNames(t *testing.T) {
6363

6464
crd := testutil.GetCRDByName(t, g, "Repository")
6565
require.NotNil(crd)
66-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["Name"])
67-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["ImageTagMutability"])
68-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["Name"].LateInitialize)
69-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["ImageTagMutability"].LateInitialize)
66+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"])
67+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageTagMutability"])
68+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"].LateInitialize)
69+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageTagMutability"].LateInitialize)
7070
expected :=
7171
` var lateInitializeFieldNames = []string{"ImageTagMutability","Name",}
7272
`
@@ -82,7 +82,7 @@ func Test_LateInitializeFromReadOne_NoFieldsToLateInitialize(t *testing.T) {
8282
crd := testutil.GetCRDByName(t, g, "Repository")
8383
require.NotNil(crd)
8484
// NO fieldConfig
85-
assert.Empty(crd.Config().GetResourceFields(crd.Names.Original))
85+
assert.Empty(crd.Config().GetFieldConfigs(crd.Names.Original))
8686
expected := " return latest"
8787
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "latest", 1))
8888
}
@@ -95,10 +95,10 @@ func Test_LateInitializeFromReadOne_NonNestedPath(t *testing.T) {
9595

9696
crd := testutil.GetCRDByName(t, g, "Repository")
9797
require.NotNil(crd)
98-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["Name"])
99-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["ImageTagMutability"])
100-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["Name"].LateInitialize)
101-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["ImageTagMutability"].LateInitialize)
98+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"])
99+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageTagMutability"])
100+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"].LateInitialize)
101+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageTagMutability"].LateInitialize)
102102
expected :=
103103
` observedKo := rm.concreteResource(observed).ko.DeepCopy()
104104
latestKo := rm.concreteResource(latest).ko.DeepCopy()
@@ -120,10 +120,10 @@ func Test_LateInitializeFromReadOne_NestedPath(t *testing.T) {
120120

121121
crd := testutil.GetCRDByName(t, g, "Repository")
122122
require.NotNil(crd)
123-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["Name"])
124-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"])
125-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["Name"].LateInitialize)
126-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"].LateInitialize)
123+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"])
124+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"])
125+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"].LateInitialize)
126+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"].LateInitialize)
127127
expected :=
128128
` observedKo := rm.concreteResource(observed).ko.DeepCopy()
129129
latestKo := rm.concreteResource(latest).ko.DeepCopy()
@@ -188,10 +188,10 @@ func Test_IncompleteLateInitialization(t *testing.T) {
188188

189189
crd := testutil.GetCRDByName(t, g, "Repository")
190190
require.NotNil(crd)
191-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["Name"])
192-
assert.NotEmpty(crd.Config().GetResourceFields(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"])
193-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["Name"].LateInitialize)
194-
assert.NotNil(crd.Config().GetResourceFields(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"].LateInitialize)
191+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"])
192+
assert.NotEmpty(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"])
193+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["Name"].LateInitialize)
194+
assert.NotNil(crd.Config().GetFieldConfigs(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"].LateInitialize)
195195
expected :=
196196
` ko := rm.concreteResource(latest).ko.DeepCopy()
197197
if ko.Spec.ImageScanningConfiguration != nil {

‎pkg/generate/code/set_resource.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ func SetResourceGetAttributes(
758758

759759
// did we output an ACKResourceMetadata guard and constructor snippet?
760760
mdGuardOut := false
761-
fieldConfigs := cfg.GetResourceFields(r.Names.Original)
761+
fieldConfigs := cfg.GetFieldConfigs(r.Names.Original)
762762
sortedAttrFieldNames := []string{}
763763
for fName, fConfig := range fieldConfigs {
764764
if fConfig.IsAttribute {

‎pkg/generate/code/set_sdk.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func SetSDK(
136136
// attrMap["KmsMasterKeyId"] = r.ko.Spec.KMSMasterKeyID
137137
// attrMap["Policy"] = r.ko.Spec.Policy
138138
// res.SetAttributes(attrMap)
139-
fieldConfigs := cfg.GetResourceFields(r.Names.Original)
139+
fieldConfigs := cfg.GetFieldConfigs(r.Names.Original)
140140
out += fmt.Sprintf("%sattrMap := map[string]*string{}\n", indent)
141141
sortedAttrFieldNames := []string{}
142142
for fName, fConfig := range fieldConfigs {
@@ -647,7 +647,7 @@ func SetSDKSetAttributes(
647647
// attrMap["Policy"] = r.ko.Spec.Policy
648648
// }
649649
// res.SetAttributes(attrMap)
650-
fieldConfigs := cfg.GetResourceFields(r.Names.Original)
650+
fieldConfigs := cfg.GetFieldConfigs(r.Names.Original)
651651
out += fmt.Sprintf("%sattrMap := map[string]*string{}\n", indent)
652652
sortedAttrFieldNames := []string{}
653653
for fName, fConfig := range fieldConfigs {

0 commit comments

Comments
 (0)