Skip to content

Commit ca61a72

Browse files
authored
chore: use k8s equality package to compare structs instead of reflect (aws-controllers-k8s#653)
Description of changes: Replace `reflect.DeepEqual` with `equality.Semantic.DeepEqual` from `k8s.io/apimachinery/pkg/api/equality` for comparing Kubernetes objects. Why this change: 1. Nil vs Empty equivalence: Semantic equality treats nil slices/maps as equal to empty slices/maps. This prevents false negatives when comparing API objects where nil and empty are semantically identical. 2. Proper time.Time comparison: Uses time.Time.Equal() internally, which correctly compares timestamps across different timezone representations and monotonic clock readings. Example of the problem: - `reflect.DeepEqual([]string{}, nil)` => false - `equality.Semantic.DeepEqual([]string{}, nil)` => true Both serialize to the same JSON/YAML, so they should be considered equal when comparing Kubernetes resources. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 43713d4 commit ca61a72

3 files changed

Lines changed: 8 additions & 9 deletions

File tree

pkg/generate/code/compare.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,11 @@ func CompareResource(
140140
cfg.PrefixConfig.SpecField+"."+specField.Names.Camel, ".",
141141
)
142142

143-
// Use reflect.DeepEqual for comparing Reference fields because
143+
// Use equality.Semantic.Equalities.DeepEqual for comparing Reference fields because
144144
// some of reference fields are list of pointer to structs and
145145
// DeepEqual is easy way to compare them
146146
if specField.IsReference() {
147-
out += fmt.Sprintf("%sif !reflect.DeepEqual(%s, %s) {\n",
147+
out += fmt.Sprintf("%sif !equality.Semantic.Equalities.DeepEqual(%s, %s) {\n",
148148
indent, firstResAdaptedVarName, secondResAdaptedVarName)
149149
out += fmt.Sprintf("%s\t%s.Add(\"%s\", %s, %s)\n", indent,
150150
deltaVarName, fieldPath, firstResAdaptedVarName,
@@ -425,7 +425,7 @@ func compareMap(
425425
// building up the fieldPath appropriately and calling into a
426426
// struct-specific comparator function...
427427
out += fmt.Sprintf(
428-
"%sif !reflect.DeepEqual(%s, %s) {\n",
428+
"%sif !equality.Semantic.Equalities.DeepEqual(%s, %s) {\n",
429429
indent, firstResVarName, secondResVarName,
430430
)
431431
}
@@ -497,7 +497,7 @@ func compareSlice(
497497
// comparator function...the tricky part of this is figuring out how to
498498
// sort the slice of structs...
499499
out += fmt.Sprintf(
500-
"%sif !reflect.DeepEqual(%s, %s) {\n",
500+
"%sif !equality.Semantic.Equalities.DeepEqual(%s, %s) {\n",
501501
indent, firstResVarName, secondResVarName,
502502
)
503503
default:

pkg/generate/code/compare_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestCompareResource_S3_Bucket(t *testing.T) {
9797
if len(a.ko.Spec.Logging.LoggingEnabled.TargetGrants) != len(b.ko.Spec.Logging.LoggingEnabled.TargetGrants) {
9898
delta.Add("Spec.Logging.LoggingEnabled.TargetGrants", a.ko.Spec.Logging.LoggingEnabled.TargetGrants, b.ko.Spec.Logging.LoggingEnabled.TargetGrants)
9999
} else if len(a.ko.Spec.Logging.LoggingEnabled.TargetGrants) > 0 {
100-
if !reflect.DeepEqual(a.ko.Spec.Logging.LoggingEnabled.TargetGrants, b.ko.Spec.Logging.LoggingEnabled.TargetGrants) {
100+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.Logging.LoggingEnabled.TargetGrants, b.ko.Spec.Logging.LoggingEnabled.TargetGrants) {
101101
delta.Add("Spec.Logging.LoggingEnabled.TargetGrants", a.ko.Spec.Logging.LoggingEnabled.TargetGrants, b.ko.Spec.Logging.LoggingEnabled.TargetGrants)
102102
}
103103
}
@@ -274,7 +274,7 @@ func TestCompareResource_Lambda_Function(t *testing.T) {
274274
if len(a.ko.Spec.FileSystemConfigs) != len(b.ko.Spec.FileSystemConfigs) {
275275
delta.Add("Spec.FileSystemConfigs", a.ko.Spec.FileSystemConfigs, b.ko.Spec.FileSystemConfigs)
276276
} else if len(a.ko.Spec.FileSystemConfigs) > 0 {
277-
if !reflect.DeepEqual(a.ko.Spec.FileSystemConfigs, b.ko.Spec.FileSystemConfigs) {
277+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.FileSystemConfigs, b.ko.Spec.FileSystemConfigs) {
278278
delta.Add("Spec.FileSystemConfigs", a.ko.Spec.FileSystemConfigs, b.ko.Spec.FileSystemConfigs)
279279
}
280280
}
@@ -485,7 +485,7 @@ func TestCompareResource_APIGatewayv2_Route(t *testing.T) {
485485
if len(a.ko.Spec.RequestParameters) != len(b.ko.Spec.RequestParameters) {
486486
delta.Add("Spec.RequestParameters", a.ko.Spec.RequestParameters, b.ko.Spec.RequestParameters)
487487
} else if len(a.ko.Spec.RequestParameters) > 0 {
488-
if !reflect.DeepEqual(a.ko.Spec.RequestParameters, b.ko.Spec.RequestParameters) {
488+
if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.RequestParameters, b.ko.Spec.RequestParameters) {
489489
delta.Add("Spec.RequestParameters", a.ko.Spec.RequestParameters, b.ko.Spec.RequestParameters)
490490
}
491491
}

templates/pkg/resource/delta.go.tpl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,15 @@ package {{ .CRD.Names.Snake }}
44

55
import (
66
"bytes"
7-
"reflect"
87

8+
"k8s.io/apimachinery/pkg/api/equality"
99
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
1010
acktags "github.com/aws-controllers-k8s/runtime/pkg/tags"
1111
)
1212

1313
// Hack to avoid import errors during build...
1414
var (
1515
_ = &bytes.Buffer{}
16-
_ = &reflect.Method{}
1716
_ = &acktags.Tags{}
1817
)
1918

0 commit comments

Comments
 (0)