Skip to content

Commit 02435d4

Browse files
vijtrip2vijtrip2
committed
Fixing unused variable problems in generate late initialization code.
1 parent 1833e09 commit 02435d4

File tree

3 files changed

+119
-67
lines changed

3 files changed

+119
-67
lines changed

‎pkg/generate/code/late_initialize.go‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ func FindLateInitializedFieldNames(
4444
out += fmt.Sprintf("%q,", fName)
4545
}
4646
out += "}\n"
47+
} else {
48+
out += fmt.Sprintf("%svar %s = []string{}\n", indent, resVarName)
4749
}
4850
return out
4951
}
@@ -96,43 +98,61 @@ func getSortedLateInitFieldsAndConfig(
9698
// min_backoff_seconds: 20
9799
//
98100
// Sample output:
99-
//if observed.Spec.ImageScanningConfiguration != nil && koWithDefaults.Spec.ImageScanningConfiguration != nil {
100-
// if observed.Spec.ImageScanningConfiguration.ScanOnPush != nil && koWithDefaults.Spec.ImageScanningConfiguration.ScanOnPush == nil {
101-
// koWithDefaults.Spec.ImageScanningConfiguration.ScanOnPush = observed.Spec.ImageScanningConfiguration.ScanOnPush
101+
// observedKo := observed.ko
102+
// latestKo := latest.ko
103+
// if observedKo.Spec.ImageScanningConfiguration != nil && latestKo.Spec.ImageScanningConfiguration != nil {
104+
// if observedKo.Spec.ImageScanningConfiguration.ScanOnPush != nil && latestKo.Spec.ImageScanningConfiguration.ScanOnPush == nil {
105+
// latestKo.Spec.ImageScanningConfiguration.ScanOnPush = observedKo.Spec.ImageScanningConfiguration.ScanOnPush
106+
// }
102107
// }
103-
//}
104-
//if observed.Spec.Name != nil && koWithDefaults.Spec.Name == nil {
105-
// koWithDefaults.Spec.Name = observed.Spec.Name
106-
//}
107-
//if observed.Spec.another != nil && koWithDefaults.Spec.another != nil {
108-
// if observed.Spec.another.map != nil && koWithDefaults.Spec.another.map != nil {
109-
// if observed.Spec.another.map[lastfield] != nil && koWithDefaults.Spec.another.map[lastfield] == nil {
110-
// koWithDefaults.Spec.another.map[lastfield] = observed.Spec.another.map[lastfield]
108+
// if observedKo.Spec.Name != nil && latestKo.Spec.Name == nil {
109+
// latestKo.Spec.Name = observedKo.Spec.Name
111110
// }
111+
// if observedKo.Spec.another != nil && latestKo.Spec.another != nil {
112+
// if observedKo.Spec.another.map != nil && latestKo.Spec.another.map != nil {
113+
// if observedKo.Spec.another.map["lastfield"] != nil && latestKo.Spec.another.map["lastfield"] == nil {
114+
// latestKo.Spec.another.map["lastfield"] = observedKo.Spec.another.map["lastfield"]
115+
// }
116+
// }
112117
// }
113-
//}
114-
//if observed.Spec.map != nil && koWithDefaults.Spec.map != nil {
115-
// if observed.Spec.map[subfield] != nil && koWithDefaults.Spec.map[subfield] != nil {
116-
// if observed.Spec.map[subfield].x != nil && koWithDefaults.Spec.map[subfield].x == nil {
117-
// koWithDefaults.Spec.map[subfield].x = observed.Spec.map[subfield].x
118-
//}
119-
//}
120-
//}
121-
//if observed.Spec.some != nil && koWithDefaults.Spec.some != nil {
122-
// if observed.Spec.some.list != nil && koWithDefaults.Spec.some.list == nil {
123-
// koWithDefaults.Spec.some.list = observed.Spec.some.list
118+
// if observedKo.Spec.map != nil && latestKo.Spec.map != nil {
119+
// if observedKo.Spec.map["subfield"] != nil && latestKo.Spec.map["subfield"] != nil {
120+
// if observedKo.Spec.map["subfield"].x != nil && latestKo.Spec.map["subfield"].x == nil {
121+
// latestKo.Spec.map["subfield"].x = observedKo.Spec.map["subfield"].x
122+
// }
123+
// }
124+
// }
125+
// if observedKo.Spec.some != nil && latestKo.Spec.some != nil {
126+
// if observedKo.Spec.some.list != nil && latestKo.Spec.some.list == nil {
127+
// latestKo.Spec.some.list = observedKo.Spec.some.list
128+
// }
124129
// }
125-
//}
130+
// if observedKo.Spec.structA != nil && latestKo.Spec.structA != nil {
131+
// if observedKo.Spec.structA.mapB != nil && latestKo.Spec.structA.mapB != nil {
132+
// if observedKo.Spec.structA.mapB["structC"] != nil && latestKo.Spec.structA.mapB["structC"] != nil {
133+
// if observedKo.Spec.structA.mapB["structC"].valueD != nil && latestKo.Spec.structA.mapB["structC"].valueD == nil {
134+
// latestKo.Spec.structA.mapB["structC"].valueD = observedKo.Spec.structA.mapB["structC"].valueD
135+
// }
136+
// }
137+
// }
138+
// }
139+
// return latest
126140
func LateInitializeFromReadOne(
127141
cfg *ackgenconfig.Config,
128142
r *model.CRD,
129-
sourceKoVarName string,
130-
targetKoVarName string,
143+
sourceResVarName string,
144+
targetResVarName string,
131145
// Number of levels of indentation to use
132146
indentLevel int,
133147
) string {
134148
out := ""
149+
indent := strings.Repeat("\t", indentLevel)
135150
lateInitializedFieldNames, _ := getSortedLateInitFieldsAndConfig(cfg, r)
151+
if len(lateInitializedFieldNames) == 0 {
152+
return fmt.Sprintf("%sreturn %s", indent, targetResVarName)
153+
}
154+
out += fmt.Sprintf("%sobservedKo := %s.ko\n", indent, sourceResVarName)
155+
out += fmt.Sprintf("%slatestKo := %s.ko\n", indent, targetResVarName)
136156
// TODO(vijat@): Add validation for correct field path in lateInitializedFieldNames
137157
for _, fName := range lateInitializedFieldNames {
138158
// split the field name by period
@@ -158,7 +178,7 @@ func LateInitializeFromReadOne(
158178
}
159179
// Handling for all parts except last one
160180
if i != len(fNameParts)-1 {
161-
out += fmt.Sprintf("%sif %s.%s != nil && %s.%s != nil {\n", indent, sourceKoVarName, fNamePartAccesor, targetKoVarName, fNamePartAccesor)
181+
out += fmt.Sprintf("%sif observedKo.%s != nil && latestKo.%s != nil {\n", indent, fNamePartAccesor, fNamePartAccesor)
162182
// update fParentPath and fNameIndentLevel for next iteration
163183
if mapShapedParent {
164184
fParentPath = fmt.Sprintf("%s[%q]", fParentPath, fNamePart)
@@ -171,10 +191,10 @@ func LateInitializeFromReadOne(
171191
// handle last part here
172192
// for last part, set the lateInitialized field if user did not specify field value and readOne has server side defaulted value.
173193
// i.e. field is not nil in sourceKoVarName but is nil in targetkoVarName
174-
out += fmt.Sprintf("%sif %s.%s != nil && %s.%s == nil {\n", indent, sourceKoVarName, fNamePartAccesor, targetKoVarName, fNamePartAccesor)
194+
out += fmt.Sprintf("%sif observedKo.%s != nil && latestKo.%s == nil {\n", indent, fNamePartAccesor, fNamePartAccesor)
175195
fNameIndentLevel = fNameIndentLevel + 1
176196
indent = strings.Repeat("\t", fNameIndentLevel)
177-
out += fmt.Sprintf("%s%s.%s = %s.%s\n", indent, targetKoVarName, fNamePartAccesor, sourceKoVarName, fNamePartAccesor)
197+
out += fmt.Sprintf("%slatestKo.%s = observedKo.%s\n", indent, fNamePartAccesor, fNamePartAccesor)
178198
}
179199
}
180200
// Close all if blocks with proper indentation
@@ -184,6 +204,7 @@ func LateInitializeFromReadOne(
184204
fNameIndentLevel = fNameIndentLevel - 1
185205
}
186206
}
207+
out += fmt.Sprintf("%sreturn %s", indent, targetResVarName)
187208
return out
188209
}
189210

@@ -264,7 +285,7 @@ func IncompleteLateInitialization(
264285
indent := strings.Repeat("\t", indentLevel)
265286
sortedLateInitFieldNames, _ := getSortedLateInitFieldsAndConfig(cfg, r)
266287
if len(sortedLateInitFieldNames) == 0 {
267-
out += fmt.Sprintf("%sreturn false\n", indent)
288+
out += fmt.Sprintf("%sreturn false", indent)
268289
return out
269290
}
270291
out += fmt.Sprintf("%sko := %s.ko\n", indent, resVarName)
@@ -316,6 +337,6 @@ func IncompleteLateInitialization(
316337
fNameIndentLevel = fNameIndentLevel - 1
317338
}
318339
}
319-
out += fmt.Sprintf("%sreturn false\n", indent)
340+
out += fmt.Sprintf("%sreturn false", indent)
320341
return out
321342
}

‎pkg/generate/code/late_initialize_test.go‎

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ func Test_FindLateInitializedFieldNames_EmptyFieldConfig(t *testing.T) {
3232
require.NotNil(crd)
3333
// NO fieldConfig
3434
assert.Empty(crd.Config().ResourceFields(crd.Names.Original))
35-
assert.Empty(code.FindLateInitializedFieldNames(crd.Config(), crd, "lateInitializeFieldNames", 1))
35+
expected :=
36+
` var lateInitializeFieldNames = []string{}
37+
`
38+
assert.Equal(expected, code.FindLateInitializedFieldNames(crd.Config(), crd, "lateInitializeFieldNames", 1))
3639
}
3740

3841
func Test_FindLateInitializedFieldNames_NoLateInitializations(t *testing.T) {
@@ -46,7 +49,10 @@ func Test_FindLateInitializedFieldNames_NoLateInitializations(t *testing.T) {
4649
// FieldConfig without lateInitialize
4750
assert.NotEmpty(crd.Config().ResourceFields(crd.Names.Original)["Name"])
4851
assert.Nil(crd.Config().ResourceFields(crd.Names.Original)["Name"].LateInitialize)
49-
assert.Empty(code.FindLateInitializedFieldNames(crd.Config(), crd, "lateInitializeFieldNames", 1))
52+
expected :=
53+
` var lateInitializeFieldNames = []string{}
54+
`
55+
assert.Equal(expected, code.FindLateInitializedFieldNames(crd.Config(), crd, "lateInitializeFieldNames", 1))
5056
}
5157

5258
func Test_FindLateInitializedFieldNames(t *testing.T) {
@@ -77,7 +83,8 @@ func Test_LateInitializeFromReadOne_NoFieldsToLateInitialize(t *testing.T) {
7783
require.NotNil(crd)
7884
// NO fieldConfig
7985
assert.Empty(crd.Config().ResourceFields(crd.Names.Original))
80-
assert.Empty(code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "koWithDefaults", 1))
86+
expected := " return latest"
87+
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "latest", 1))
8188
}
8289

8390
func Test_LateInitializeFromReadOne_NonNestedPath(t *testing.T) {
@@ -93,14 +100,16 @@ func Test_LateInitializeFromReadOne_NonNestedPath(t *testing.T) {
93100
assert.NotNil(crd.Config().ResourceFields(crd.Names.Original)["Name"].LateInitialize)
94101
assert.NotNil(crd.Config().ResourceFields(crd.Names.Original)["ImageTagMutability"].LateInitialize)
95102
expected :=
96-
` if observed.Spec.ImageTagMutability != nil && koWithDefaults.Spec.ImageTagMutability == nil {
97-
koWithDefaults.Spec.ImageTagMutability = observed.Spec.ImageTagMutability
103+
` observedKo := observed.ko
104+
latestKo := latest.ko
105+
if observedKo.Spec.ImageTagMutability != nil && latestKo.Spec.ImageTagMutability == nil {
106+
latestKo.Spec.ImageTagMutability = observedKo.Spec.ImageTagMutability
98107
}
99-
if observed.Spec.Name != nil && koWithDefaults.Spec.Name == nil {
100-
koWithDefaults.Spec.Name = observed.Spec.Name
108+
if observedKo.Spec.Name != nil && latestKo.Spec.Name == nil {
109+
latestKo.Spec.Name = observedKo.Spec.Name
101110
}
102-
`
103-
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "koWithDefaults", 1))
111+
return latest`
112+
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "latest", 1))
104113
}
105114

106115
func Test_LateInitializeFromReadOne_NestedPath(t *testing.T) {
@@ -116,44 +125,59 @@ func Test_LateInitializeFromReadOne_NestedPath(t *testing.T) {
116125
assert.NotNil(crd.Config().ResourceFields(crd.Names.Original)["Name"].LateInitialize)
117126
assert.NotNil(crd.Config().ResourceFields(crd.Names.Original)["ImageScanningConfiguration.ScanOnPush"].LateInitialize)
118127
expected :=
119-
` if observed.Spec.ImageScanningConfiguration != nil && koWithDefaults.Spec.ImageScanningConfiguration != nil {
120-
if observed.Spec.ImageScanningConfiguration.ScanOnPush != nil && koWithDefaults.Spec.ImageScanningConfiguration.ScanOnPush == nil {
121-
koWithDefaults.Spec.ImageScanningConfiguration.ScanOnPush = observed.Spec.ImageScanningConfiguration.ScanOnPush
128+
` observedKo := observed.ko
129+
latestKo := latest.ko
130+
if observedKo.Spec.ImageScanningConfiguration != nil && latestKo.Spec.ImageScanningConfiguration != nil {
131+
if observedKo.Spec.ImageScanningConfiguration.ScanOnPush != nil && latestKo.Spec.ImageScanningConfiguration.ScanOnPush == nil {
132+
latestKo.Spec.ImageScanningConfiguration.ScanOnPush = observedKo.Spec.ImageScanningConfiguration.ScanOnPush
122133
}
123134
}
124-
if observed.Spec.Name != nil && koWithDefaults.Spec.Name == nil {
125-
koWithDefaults.Spec.Name = observed.Spec.Name
135+
if observedKo.Spec.Name != nil && latestKo.Spec.Name == nil {
136+
latestKo.Spec.Name = observedKo.Spec.Name
126137
}
127-
if observed.Spec.another != nil && koWithDefaults.Spec.another != nil {
128-
if observed.Spec.another.map != nil && koWithDefaults.Spec.another.map != nil {
129-
if observed.Spec.another.map["lastfield"] != nil && koWithDefaults.Spec.another.map["lastfield"] == nil {
130-
koWithDefaults.Spec.another.map["lastfield"] = observed.Spec.another.map["lastfield"]
138+
if observedKo.Spec.another != nil && latestKo.Spec.another != nil {
139+
if observedKo.Spec.another.map != nil && latestKo.Spec.another.map != nil {
140+
if observedKo.Spec.another.map["lastfield"] != nil && latestKo.Spec.another.map["lastfield"] == nil {
141+
latestKo.Spec.another.map["lastfield"] = observedKo.Spec.another.map["lastfield"]
131142
}
132143
}
133144
}
134-
if observed.Spec.map != nil && koWithDefaults.Spec.map != nil {
135-
if observed.Spec.map["subfield"] != nil && koWithDefaults.Spec.map["subfield"] != nil {
136-
if observed.Spec.map["subfield"].x != nil && koWithDefaults.Spec.map["subfield"].x == nil {
137-
koWithDefaults.Spec.map["subfield"].x = observed.Spec.map["subfield"].x
145+
if observedKo.Spec.map != nil && latestKo.Spec.map != nil {
146+
if observedKo.Spec.map["subfield"] != nil && latestKo.Spec.map["subfield"] != nil {
147+
if observedKo.Spec.map["subfield"].x != nil && latestKo.Spec.map["subfield"].x == nil {
148+
latestKo.Spec.map["subfield"].x = observedKo.Spec.map["subfield"].x
138149
}
139150
}
140151
}
141-
if observed.Spec.some != nil && koWithDefaults.Spec.some != nil {
142-
if observed.Spec.some.list != nil && koWithDefaults.Spec.some.list == nil {
143-
koWithDefaults.Spec.some.list = observed.Spec.some.list
152+
if observedKo.Spec.some != nil && latestKo.Spec.some != nil {
153+
if observedKo.Spec.some.list != nil && latestKo.Spec.some.list == nil {
154+
latestKo.Spec.some.list = observedKo.Spec.some.list
144155
}
145156
}
146-
if observed.Spec.structA != nil && koWithDefaults.Spec.structA != nil {
147-
if observed.Spec.structA.mapB != nil && koWithDefaults.Spec.structA.mapB != nil {
148-
if observed.Spec.structA.mapB["structC"] != nil && koWithDefaults.Spec.structA.mapB["structC"] != nil {
149-
if observed.Spec.structA.mapB["structC"].valueD != nil && koWithDefaults.Spec.structA.mapB["structC"].valueD == nil {
150-
koWithDefaults.Spec.structA.mapB["structC"].valueD = observed.Spec.structA.mapB["structC"].valueD
157+
if observedKo.Spec.structA != nil && latestKo.Spec.structA != nil {
158+
if observedKo.Spec.structA.mapB != nil && latestKo.Spec.structA.mapB != nil {
159+
if observedKo.Spec.structA.mapB["structC"] != nil && latestKo.Spec.structA.mapB["structC"] != nil {
160+
if observedKo.Spec.structA.mapB["structC"].valueD != nil && latestKo.Spec.structA.mapB["structC"].valueD == nil {
161+
latestKo.Spec.structA.mapB["structC"].valueD = observedKo.Spec.structA.mapB["structC"].valueD
151162
}
152163
}
153164
}
154165
}
155-
`
156-
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "koWithDefaults", 1))
166+
return latest`
167+
assert.Equal(expected, code.LateInitializeFromReadOne(crd.Config(), crd, "observed", "latest", 1))
168+
}
169+
170+
func Test_IncompleteLateInitialization_NoFieldsToLateInitialization(t *testing.T) {
171+
assert := assert.New(t)
172+
require := require.New(t)
173+
174+
g := testutil.NewModelForServiceWithOptions(t, "ecr", &testutil.TestingModelOptions{GeneratorConfigFile: "generator-with-field-config.yaml"})
175+
176+
crd := testutil.GetCRDByName(t, g, "Repository")
177+
require.NotNil(crd)
178+
expected :=
179+
` return false`
180+
assert.Equal(expected, code.IncompleteLateInitialization(crd.Config(), crd, "latestWithDefaults", 1))
157181
}
158182

159183
func Test_IncompleteLateInitialization(t *testing.T) {
@@ -206,7 +230,6 @@ func Test_IncompleteLateInitialization(t *testing.T) {
206230
}
207231
}
208232
}
209-
return false
210-
`
233+
return false`
211234
assert.Equal(expected, code.IncompleteLateInitialization(crd.Config(), crd, "latestWithDefaults", 1))
212235
}

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,7 @@ func (rm *resourceManager) LateInitialize(
197197
ackcondition.SetLateInitialized(latestWithDefaults, corev1.ConditionFalse, &lateInitConditionMessage, &lateInitConditionReason)
198198
return latestWithDefaults, ackrequeue.NeededAfter(err, time.Duration(0)*time.Second)
199199
}
200-
observedKo := observed.ko
201-
{{ GoCodeLateInitializeFromReadOne .CRD "observedKo" "koWithDefaults" 1 }}
200+
latestWithDefaults = rm.lateInitializeFromReadOneOutput(observed, latestWithDefaults)
202201
incompleteInitialization := rm.incompleteLateInitialization(latestWithDefaults)
203202
if incompleteInitialization {
204203
// Add the condition with LateInitialized=False
@@ -209,7 +208,7 @@ func (rm *resourceManager) LateInitialize(
209208
}
210209
// Set LateIntialized condition to True
211210
lateInitConditionMessage = "Late initialization successful"
212-
lateInitConditionReason = "Late initialization successful"
211+
lateInitConditionReason = "Late initialization successful"
213212
ackcondition.SetLateInitialized(latestWithDefaults, corev1.ConditionTrue, &lateInitConditionMessage, &lateInitConditionReason)
214213
{{- if $hookCode := Hook .CRD "late_initialize_post_read_one" }}
215214
{{ $hookCode }}
@@ -225,6 +224,15 @@ func (rm *resourceManager) incompleteLateInitialization(
225224
{{ GoCodeIncompleteLateInitialization .CRD "latestWithDefaults" 1 }}
226225
}
227226

227+
// lateInitializeFromReadOneOutput late initializes the 'latest' resource from the 'observed'
228+
// resource and returns 'latest' resource
229+
func (rm *resourceManager) lateInitializeFromReadOneOutput(
230+
observed *resource,
231+
latest *resource,
232+
) *resource {
233+
{{ GoCodeLateInitializeFromReadOne .CRD "observed" "latest" 1 }}
234+
}
235+
228236
// newResourceManager returns a new struct implementing
229237
// acktypes.AWSResourceManager
230238
func newResourceManager(

0 commit comments

Comments
 (0)