Skip to content

Commit bcebbb0

Browse files
sapphirewsapphirew
authored
feat: add CompareResourceForPreDelete and DeltaForPreDelete support (#686)
Add pre-delete delta comparison support to the code generator. Controllers can opt specific fields into pre-delete comparison via `compare.pre_delete_include: true` in generator.yaml, so that fields like `DeletionProtectionEnabled` are synced before deletion even when they use `compare.is_ignored` for normal reconciliation. Config changes: - Add `PreDeleteInclude` bool to `CompareFieldConfig` (field.go) - Add `PreDeleteSyncConfig` with `CompareAll to ResourceConfig` (resource.go) Code generation: - Add `CompareResourceForPreDelete` function that emits comparison code only for opted-in fields (or all fields when compare_all is true) - Register `GoCodeCompareForPreDelete` template function (controller.go) - Add `newResourceDeltaForPreDelete` in delta.go.tpl - Add `DeltaForPreDelete` method on `resourceDescriptor` in descriptor.go.tpl Tests: - Four tests covering no-opt-in (empty), explicit opt-in, compare_all, and empty-without-opt-in scenarios - Two new S3 test fixture generator YAMLs runtime changes: aws-controllers-k8s/runtime#234 Issue #, if available: Description of changes: By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 96dcae8 commit bcebbb0

11 files changed

Lines changed: 640 additions & 71 deletions

File tree

‎go.mod‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,6 @@ require (
103103
sigs.k8s.io/randfill v1.0.0 // indirect
104104
sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect
105105
)
106+
107+
108+

‎pkg/config/field.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,9 @@ type CompareFieldConfig struct {
270270
// NilEqualsZeroValue indicates a nil pointer and zero-value pointed-to
271271
// value should be considered equal for the purposes of comparison
272272
NilEqualsZeroValue bool `json:"nil_equals_zero_value"`
273+
// PreDeleteInclude indicates the field should be included in the
274+
// pre-delete delta comparison even if IsIgnored is true
275+
PreDeleteInclude bool `json:"pre_delete_include"`
273276
}
274277

275278
// PrintFieldConfig instructs the code generator how to handle kubebuilder:printcolumn

‎pkg/config/resource.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ type ResourceConfig struct {
9191
// Reconcile describes options for controlling the reconciliation
9292
// logic for a particular resource.
9393
Reconcile *ReconcileConfig `json:"reconcile,omitempty"`
94+
// PreDeleteSync contains instructions for the code generator about
95+
// how to handle pre-delete sync delta comparison for a resource.
96+
PreDeleteSync *PreDeleteSyncConfig `json:"pre_delete_sync,omitempty"`
9497
// UpdateConditionsCustomMethodName provides the name of the custom method on the
9598
// `resourceManager` struct that will set Conditions on a `resource` struct
9699
// depending on the status of the resource.
@@ -471,6 +474,16 @@ type ReconcileConfig struct {
471474
RequeueOnSuccessSeconds int `json:"requeue_on_success_seconds,omitempty"`
472475
}
473476

477+
// PreDeleteSyncConfig contains instructions for the code generator about
478+
// how to handle pre-delete sync delta comparison for a resource.
479+
type PreDeleteSyncConfig struct {
480+
// CompareAll indicates that ALL spec fields (including those with
481+
// compare.is_ignored: true) should be included in the pre-delete delta
482+
// comparison. When false (the default), only fields with
483+
// compare.pre_delete_include: true are included.
484+
CompareAll bool `json:"compare_all"`
485+
}
486+
474487
// ResourceIsIgnored returns true if resource name is configured to be ignored
475488
// in generator config for the AWS service
476489
func (c *Config) ResourceIsIgnored(resourceName string) bool {

‎pkg/generate/ack/controller.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,15 @@ var (
145145
"GoCodeCompare": func(r *ackmodel.CRD, deltaVarName string, sourceVarName string, targetVarName string, indentLevel int) (string, error) {
146146
return code.CompareResource(r.Config(), r, deltaVarName, sourceVarName, targetVarName, indentLevel)
147147
},
148+
"GoCodeCompareForPreDelete": func(r *ackmodel.CRD, deltaVarName string, sourceVarName string, targetVarName string, indentLevel int) (string, error) {
149+
return code.CompareResourceForPreDelete(r.Config(), r, deltaVarName, sourceVarName, targetVarName, indentLevel)
150+
},
151+
"GoCodeMergeForPreDelete": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int) (string, error) {
152+
return code.MergeResourceForPreDelete(r.Config(), r, sourceVarName, targetVarName, indentLevel)
153+
},
154+
"HasPreDeleteSync": func(r *ackmodel.CRD) bool {
155+
return code.HasPreDeleteSync(r.Config(), r)
156+
},
148157
"GoCodeIsSynced": func(r *ackmodel.CRD, resVarName string, indentLevel int) (string, error) {
149158
return code.ResourceIsSynced(r.Config(), r, resVarName, indentLevel)
150159
},

‎pkg/generate/code/compare.go‎

Lines changed: 228 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -25,75 +25,56 @@ import (
2525
"github.com/aws-controllers-k8s/code-generator/pkg/model"
2626
)
2727

28-
// CompareResource returns the Go code that traverses a set of two Resources,
29-
// adding differences between the two Resources to an `ackcompare.Delta`
30-
//
31-
// By default, we produce Go code that only looks at the fields in a resource's
32-
// Spec, since those are the fields that represent the desired state of a
33-
// resource. When we make a ReadOne/ReadMany/GetAttributes call to a backend
34-
// AWS API, we construct a Resource and set the Spec fields to values contained
35-
// in the ReadOne/ReadMany/GetAttributes Output shape. This Resource,
36-
// constructed from the Read operation, is compared to the Resource we got from
37-
// the Kubernetes API server's event bus. The code that is returned from this
38-
// function is the code that compares those two Resources.
39-
//
40-
// The Go code we return depends on the Go type of the various fields for the
41-
// resource being compared.
42-
//
43-
// For *scalar* Go types, the output Go code looks like this:
44-
//
45-
// if ackcompare.HasNilDifference(a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl) {
46-
// delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
47-
// } else if a.ko.Spec.GrantFullControl != nil && b.ko.Spec.GrantFullControl != nil {
48-
//
49-
// if *a.ko.Spec.GrantFullControl != *b.ko.Spec.GrantFullControl {
50-
// delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
51-
// }
52-
// }
53-
//
54-
// For *struct* Go types, the output Go code looks like this (note that it is a
55-
// simple recursive-descent output of all the struct's fields...):
56-
//
57-
// if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration) {
58-
// delta.Add("Spec.CreateBucketConfiguration", a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration)
59-
// } else if a.ko.Spec.CreateBucketConfiguration != nil && b.ko.Spec.CreateBucketConfiguration != nil {
60-
//
61-
// if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint) {
62-
// delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
63-
// } else if a.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil && b.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil {
64-
// if *a.ko.Spec.CreateBucketConfiguration.LocationConstraint != *b.ko.Spec.CreateBucketConfiguration.LocationConstraint {
65-
// delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
66-
// }
67-
// }
68-
// }
69-
//
70-
// For *slice of strings* Go types, the output Go code looks like this:
71-
//
72-
// if ackcompare.HasNilDifference(a.ko.Spec.AllowedPublishers, b.ko.Spec.AllowedPublishers) {
73-
// delta.Add("Spec.AllowedPublishers", a.ko.Spec.AllowedPublishers, b.ko.Spec.AllowedPublishers)
74-
// } else if a.ko.Spec.AllowedPublishers != nil && b.ko.Spec.AllowedPublishers != nil {
75-
//
76-
// if !ackcompare.SliceStringPEqual(a.ko.Spec.AllowedPublishers.SigningProfileVersionARNs, b.ko.Spec.AllowedPublishers.SigningProfileVersionARNs) {
77-
// delta.Add("Spec.AllowedPublishers.SigningProfileVersionARNs", a.ko.Spec.AllowedPublishers.SigningProfileVersionARNs, b.ko.Spec.AllowedPublishers.SigningProfileVersionARNs)
78-
// }
79-
// }
80-
func CompareResource(
28+
// HasPreDeleteSync returns true if the CRD has any pre-delete sync
29+
// configuration — either pre_delete_sync.compare_all is true, or at least
30+
// one field has compare.pre_delete_include: true.
31+
func HasPreDeleteSync(
32+
cfg *ackgenconfig.Config,
33+
r *model.CRD,
34+
) bool {
35+
resConfig := cfg.GetResourceConfig(r.Names.Camel)
36+
if resConfig == nil {
37+
return false
38+
}
39+
if resConfig.PreDeleteSync != nil && resConfig.PreDeleteSync.CompareAll {
40+
return true
41+
}
42+
for fieldName := range r.SpecFields {
43+
fieldConfig := resConfig.GetFieldConfig(fieldName)
44+
if fieldConfig != nil && fieldConfig.Compare != nil && fieldConfig.Compare.PreDeleteInclude {
45+
return true
46+
}
47+
}
48+
return false
49+
}
50+
51+
// sortedSpecFieldNames returns the spec field names for a CRD in
52+
// deterministic sorted order.
53+
func sortedSpecFieldNames(r *model.CRD) []string {
54+
fieldNames := make([]string, 0, len(r.SpecFields))
55+
for fieldName := range r.SpecFields {
56+
fieldNames = append(fieldNames, fieldName)
57+
}
58+
sort.Strings(fieldNames)
59+
return fieldNames
60+
}
61+
62+
// fieldFilter is a function that determines whether a spec field should be
63+
// included in the comparison output. It receives the field's compare config
64+
// (which may be nil) and returns true if the field should be included.
65+
type fieldFilter func(compareConfig *ackgenconfig.CompareFieldConfig) bool
66+
67+
// compareResourceFields is the shared implementation for generating Go code
68+
// that compares spec fields between two resources. The includeField callback
69+
// controls which fields are included in the output.
70+
func compareResourceFields(
8171
cfg *ackgenconfig.Config,
8272
r *model.CRD,
83-
// String representing the name of the variable that is of type
84-
// `*ackcompare.Delta`. We will generate Go code that calls the `Add()`
85-
// method of this variable when differences between fields are detected.
8673
deltaVarName string,
87-
// String representing the name of the variable that represents the first
88-
// CR under comparison. This will typically be something like "a.ko". See
89-
// `templates/pkg/resource/delta.go.tpl`.
9074
firstResVarName string,
91-
// String representing the name of the variable that represents the second
92-
// CR under comparison. This will typically be something like "b.ko". See
93-
// `templates/pkg/resource/delta.go.tpl`.
9475
secondResVarName string,
95-
// Number of levels of indentation to use
9676
indentLevel int,
77+
includeField fieldFilter,
9778
) (string, error) {
9879
out := "\n"
9980

@@ -105,13 +86,7 @@ func CompareResource(
10586
}
10687

10788
// We need a deterministic order to traverse our top-level fields...
108-
specFieldNames := []string{}
109-
for fieldName := range r.SpecFields {
110-
specFieldNames = append(specFieldNames, fieldName)
111-
}
112-
sort.Strings(specFieldNames)
113-
114-
for _, fieldName := range specFieldNames {
89+
for _, fieldName := range sortedSpecFieldNames(r) {
11590
specField := r.SpecFields[fieldName]
11691
indent := strings.Repeat("\t", indentLevel)
11792
firstResAdaptedVarName := firstResVarName + cfg.PrefixConfig.SpecField
@@ -129,7 +104,7 @@ func CompareResource(
129104
compareConfig = fieldConfig.Compare
130105
}
131106

132-
if compareConfig != nil && compareConfig.IsIgnored {
107+
if !includeField(compareConfig) {
133108
continue
134109
}
135110

@@ -271,6 +246,84 @@ func CompareResource(
271246
return out, nil
272247
}
273248

249+
// CompareResource returns the Go code that traverses a set of two Resources,
250+
// adding differences between the two Resources to an `ackcompare.Delta`
251+
//
252+
// By default, we produce Go code that only looks at the fields in a resource's
253+
// Spec, since those are the fields that represent the desired state of a
254+
// resource. When we make a ReadOne/ReadMany/GetAttributes call to a backend
255+
// AWS API, we construct a Resource and set the Spec fields to values contained
256+
// in the ReadOne/ReadMany/GetAttributes Output shape. This Resource,
257+
// constructed from the Read operation, is compared to the Resource we got from
258+
// the Kubernetes API server's event bus. The code that is returned from this
259+
// function is the code that compares those two Resources.
260+
//
261+
// The Go code we return depends on the Go type of the various fields for the
262+
// resource being compared.
263+
//
264+
// For *scalar* Go types, the output Go code looks like this:
265+
//
266+
// if ackcompare.HasNilDifference(a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl) {
267+
// delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
268+
// } else if a.ko.Spec.GrantFullControl != nil && b.ko.Spec.GrantFullControl != nil {
269+
//
270+
// if *a.ko.Spec.GrantFullControl != *b.ko.Spec.GrantFullControl {
271+
// delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
272+
// }
273+
// }
274+
//
275+
// For *struct* Go types, the output Go code looks like this (note that it is a
276+
// simple recursive-descent output of all the struct's fields...):
277+
//
278+
// if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration) {
279+
// delta.Add("Spec.CreateBucketConfiguration", a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration)
280+
// } else if a.ko.Spec.CreateBucketConfiguration != nil && b.ko.Spec.CreateBucketConfiguration != nil {
281+
//
282+
// if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint) {
283+
// delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
284+
// } else if a.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil && b.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil {
285+
// if *a.ko.Spec.CreateBucketConfiguration.LocationConstraint != *b.ko.Spec.CreateBucketConfiguration.LocationConstraint {
286+
// delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
287+
// }
288+
// }
289+
// }
290+
//
291+
// For *slice of strings* Go types, the output Go code looks like this:
292+
//
293+
// if ackcompare.HasNilDifference(a.ko.Spec.AllowedPublishers, b.ko.Spec.AllowedPublishers) {
294+
// delta.Add("Spec.AllowedPublishers", a.ko.Spec.AllowedPublishers, b.ko.Spec.AllowedPublishers)
295+
// } else if a.ko.Spec.AllowedPublishers != nil && b.ko.Spec.AllowedPublishers != nil {
296+
//
297+
// if !ackcompare.SliceStringPEqual(a.ko.Spec.AllowedPublishers.SigningProfileVersionARNs, b.ko.Spec.AllowedPublishers.SigningProfileVersionARNs) {
298+
// delta.Add("Spec.AllowedPublishers.SigningProfileVersionARNs", a.ko.Spec.AllowedPublishers.SigningProfileVersionARNs, b.ko.Spec.AllowedPublishers.SigningProfileVersionARNs)
299+
// }
300+
// }
301+
func CompareResource(
302+
cfg *ackgenconfig.Config,
303+
r *model.CRD,
304+
// String representing the name of the variable that is of type
305+
// `*ackcompare.Delta`. We will generate Go code that calls the `Add()`
306+
// method of this variable when differences between fields are detected.
307+
deltaVarName string,
308+
// String representing the name of the variable that represents the first
309+
// CR under comparison. This will typically be something like "a.ko". See
310+
// `templates/pkg/resource/delta.go.tpl`.
311+
firstResVarName string,
312+
// String representing the name of the variable that represents the second
313+
// CR under comparison. This will typically be something like "b.ko". See
314+
// `templates/pkg/resource/delta.go.tpl`.
315+
secondResVarName string,
316+
// Number of levels of indentation to use
317+
indentLevel int,
318+
) (string, error) {
319+
return compareResourceFields(cfg, r, deltaVarName, firstResVarName, secondResVarName, indentLevel,
320+
func(compareConfig *ackgenconfig.CompareFieldConfig) bool {
321+
// Skip fields marked as ignored in normal reconciliation
322+
return compareConfig == nil || !compareConfig.IsIgnored
323+
},
324+
)
325+
}
326+
274327
// compareNil outputs Go code that compares two field values for nullability
275328
// and, if there is a nil difference, adds the difference to a variable
276329
// representing the `ackcompare.Delta`
@@ -1079,3 +1132,107 @@ func fastCompareTypes(
10791132
}
10801133
return out, needToCloseBlock, nil
10811134
}
1135+
1136+
// CompareResourceForPreDelete returns the Go code that traverses a set of two
1137+
// Resources, adding differences between the two Resources to an
1138+
// `ackcompare.Delta`.
1139+
//
1140+
// When the resource has `pre_delete_sync.compare_all: true` set, ALL fields
1141+
// are included (even those with `compare.is_ignored: true`), mirroring
1142+
// CompareResource without the is_ignored skip.
1143+
//
1144+
// Otherwise (the default), only fields explicitly opted in via
1145+
// `compare.pre_delete_include: true` are included; all other fields are
1146+
// skipped. This opt-in approach reduces blast radius so controllers only get
1147+
// pre-delete sync comparison on fields they've deliberately configured.
1148+
func CompareResourceForPreDelete(
1149+
cfg *ackgenconfig.Config,
1150+
r *model.CRD,
1151+
// String representing the name of the variable that is of type
1152+
// `*ackcompare.Delta`. We will generate Go code that calls the `Add()`
1153+
// method of this variable when differences between fields are detected.
1154+
deltaVarName string,
1155+
// String representing the name of the variable that represents the first
1156+
// CR under comparison. This will typically be something like "a.ko". See
1157+
// `templates/pkg/resource/delta.go.tpl`.
1158+
firstResVarName string,
1159+
// String representing the name of the variable that represents the second
1160+
// CR under comparison. This will typically be something like "b.ko". See
1161+
// `templates/pkg/resource/delta.go.tpl`.
1162+
secondResVarName string,
1163+
// Number of levels of indentation to use
1164+
indentLevel int,
1165+
) (string, error) {
1166+
resConfig := cfg.GetResourceConfig(r.Names.Camel)
1167+
1168+
// Determine whether to include all fields in the pre-delete sync delta.
1169+
compareAll := false
1170+
if resConfig != nil && resConfig.PreDeleteSync != nil {
1171+
compareAll = resConfig.PreDeleteSync.CompareAll
1172+
}
1173+
1174+
return compareResourceFields(cfg, r, deltaVarName, firstResVarName, secondResVarName, indentLevel,
1175+
func(compareConfig *ackgenconfig.CompareFieldConfig) bool {
1176+
if compareAll {
1177+
return true
1178+
}
1179+
// Default opt-in behavior: only include fields with pre_delete_include
1180+
return compareConfig != nil && compareConfig.PreDeleteInclude
1181+
},
1182+
)
1183+
}
1184+
1185+
// MergeResourceForPreDelete outputs Go code that copies only the pre-delete sync
1186+
// configured fields from the source variable onto the target variable. This is
1187+
// used to build a merged resource that starts as a deep copy of observed and
1188+
// has only the pre-delete sync fields overwritten from desired.
1189+
//
1190+
// The generated code is a series of simple field assignments for each field
1191+
// that has `compare.pre_delete_include: true` (or all fields when
1192+
// `pre_delete_sync.compare_all: true`).
1193+
func MergeResourceForPreDelete(
1194+
cfg *ackgenconfig.Config,
1195+
r *model.CRD,
1196+
// String representing the variable to copy fields FROM (e.g. "a.ko")
1197+
sourceVarName string,
1198+
// String representing the variable to copy fields TO (e.g. "merged.ko")
1199+
targetVarName string,
1200+
// Number of levels of indentation to use
1201+
indentLevel int,
1202+
) (string, error) {
1203+
out := ""
1204+
1205+
resConfig := cfg.GetResourceConfig(r.Names.Camel)
1206+
1207+
compareAll := false
1208+
if resConfig != nil && resConfig.PreDeleteSync != nil {
1209+
compareAll = resConfig.PreDeleteSync.CompareAll
1210+
}
1211+
1212+
for _, fieldName := range sortedSpecFieldNames(r) {
1213+
specField := r.SpecFields[fieldName]
1214+
indent := strings.Repeat("\t", indentLevel)
1215+
1216+
var fieldConfig *ackgenconfig.FieldConfig
1217+
var compareConfig *ackgenconfig.CompareFieldConfig
1218+
1219+
if resConfig != nil {
1220+
fieldConfig = resConfig.GetFieldConfig(fieldName)
1221+
}
1222+
if fieldConfig != nil {
1223+
compareConfig = fieldConfig.Compare
1224+
}
1225+
1226+
if !compareAll {
1227+
if compareConfig == nil || !compareConfig.PreDeleteInclude {
1228+
continue
1229+
}
1230+
}
1231+
1232+
sourceField := sourceVarName + cfg.PrefixConfig.SpecField + "." + specField.Names.Camel
1233+
targetField := targetVarName + cfg.PrefixConfig.SpecField + "." + specField.Names.Camel
1234+
1235+
out += fmt.Sprintf("%s%s = %s\n", indent, targetField, sourceField)
1236+
}
1237+
return out, nil
1238+
}

0 commit comments

Comments
 (0)