Skip to content

Commit 119cbe3

Browse files
Merge pull request #700 from gustavodiaz7722/feat/nil-equals-zero-value
Complete implementation for NilEqualsZeroValue
2 parents 40bfad5 + 6da14fd commit 119cbe3

3 files changed

Lines changed: 127 additions & 5 deletions

File tree

pkg/generate/code/compare.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ func compareNil(
301301
out := ""
302302
indent := strings.Repeat("\t", indentLevel)
303303

304+
nilEqualsZeroValue := compareConfig != nil && compareConfig.NilEqualsZeroValue
305+
304306
switch shape.Type {
305307
case "boolean", "string", "character", "byte", "short", "integer", "long",
306308
"float", "double", "timestamp", "structure", "jsonvalue":
@@ -312,11 +314,32 @@ func compareNil(
312314
default:
313315
return "", fmt.Errorf("field %q: unsupported shape type in compareNil: %s", fieldPath, shape.Type)
314316
}
315-
// delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
316-
out += fmt.Sprintf(
317-
"%s\t%s.Add(\"%s\", %s, %s)\n",
318-
indent, deltaVarName, fieldPath, firstResVarName, secondResVarName,
319-
)
317+
318+
if nilEqualsZeroValue {
319+
// When nil_equals_zero_value is true, a nil pointer in the desired
320+
// state (first resource) is considered equal to a pointer to the
321+
// zero value in the latest state (second resource). We only add a
322+
// delta if IsNilEqualsZero returns false.
323+
out += fmt.Sprintf(
324+
"%s\tif !ackcompare.IsNilEqualsZero(%s, %s) {\n",
325+
indent, firstResVarName, secondResVarName,
326+
)
327+
// delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
328+
out += fmt.Sprintf(
329+
"%s\t\t%s.Add(\"%s\", %s, %s)\n",
330+
indent, deltaVarName, fieldPath, firstResVarName, secondResVarName,
331+
)
332+
// }
333+
out += fmt.Sprintf(
334+
"%s\t}\n", indent,
335+
)
336+
} else {
337+
// delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
338+
out += fmt.Sprintf(
339+
"%s\t%s.Add(\"%s\", %s, %s)\n",
340+
indent, deltaVarName, fieldPath, firstResVarName, secondResVarName,
341+
)
342+
}
320343
// }
321344
out += fmt.Sprintf(
322345
"%s}", indent,

pkg/generate/code/compare_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,3 +814,69 @@ func TestCompareResource_S3Files_AccessPoint(t *testing.T) {
814814
require.NoError(err)
815815
assert.Equal(expected, got)
816816
}
817+
818+
func TestCompareResource_SNS_Topic_NilEqualsZeroValue(t *testing.T) {
819+
assert := assert.New(t)
820+
require := require.New(t)
821+
822+
g := testutil.NewModelForServiceWithOptions(t, "sns", &testutil.TestingModelOptions{
823+
GeneratorConfigFile: "generator-nil-equals-zero-value.yaml",
824+
})
825+
826+
crd := testutil.GetCRDByName(t, g, "Topic")
827+
require.NotNil(crd)
828+
829+
// DisplayName has compare.nil_equals_zero_value: true, so the generated
830+
// code should only report a nil difference if the non-nil side (latest
831+
// from AWS) is not an empty string. This prevents permanent drift when
832+
// AWS returns "" for an unset optional field while the desired state has nil.
833+
expected := `
834+
if ackcompare.HasNilDifference(a.ko.Spec.DeliveryPolicy, b.ko.Spec.DeliveryPolicy) {
835+
delta.Add("Spec.DeliveryPolicy", a.ko.Spec.DeliveryPolicy, b.ko.Spec.DeliveryPolicy)
836+
} else if a.ko.Spec.DeliveryPolicy != nil && b.ko.Spec.DeliveryPolicy != nil {
837+
if *a.ko.Spec.DeliveryPolicy != *b.ko.Spec.DeliveryPolicy {
838+
delta.Add("Spec.DeliveryPolicy", a.ko.Spec.DeliveryPolicy, b.ko.Spec.DeliveryPolicy)
839+
}
840+
}
841+
if ackcompare.HasNilDifference(a.ko.Spec.DisplayName, b.ko.Spec.DisplayName) {
842+
if !ackcompare.IsNilEqualsZero(a.ko.Spec.DisplayName, b.ko.Spec.DisplayName) {
843+
delta.Add("Spec.DisplayName", a.ko.Spec.DisplayName, b.ko.Spec.DisplayName)
844+
}
845+
} else if a.ko.Spec.DisplayName != nil && b.ko.Spec.DisplayName != nil {
846+
if *a.ko.Spec.DisplayName != *b.ko.Spec.DisplayName {
847+
delta.Add("Spec.DisplayName", a.ko.Spec.DisplayName, b.ko.Spec.DisplayName)
848+
}
849+
}
850+
if ackcompare.HasNilDifference(a.ko.Spec.KMSMasterKeyID, b.ko.Spec.KMSMasterKeyID) {
851+
delta.Add("Spec.KMSMasterKeyID", a.ko.Spec.KMSMasterKeyID, b.ko.Spec.KMSMasterKeyID)
852+
} else if a.ko.Spec.KMSMasterKeyID != nil && b.ko.Spec.KMSMasterKeyID != nil {
853+
if *a.ko.Spec.KMSMasterKeyID != *b.ko.Spec.KMSMasterKeyID {
854+
delta.Add("Spec.KMSMasterKeyID", a.ko.Spec.KMSMasterKeyID, b.ko.Spec.KMSMasterKeyID)
855+
}
856+
}
857+
if ackcompare.HasNilDifference(a.ko.Spec.Name, b.ko.Spec.Name) {
858+
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
859+
} else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil {
860+
if *a.ko.Spec.Name != *b.ko.Spec.Name {
861+
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
862+
}
863+
}
864+
if ackcompare.HasNilDifference(a.ko.Spec.Policy, b.ko.Spec.Policy) {
865+
delta.Add("Spec.Policy", a.ko.Spec.Policy, b.ko.Spec.Policy)
866+
} else if a.ko.Spec.Policy != nil && b.ko.Spec.Policy != nil {
867+
if *a.ko.Spec.Policy != *b.ko.Spec.Policy {
868+
delta.Add("Spec.Policy", a.ko.Spec.Policy, b.ko.Spec.Policy)
869+
}
870+
}
871+
desiredACKTags, _ := convertToOrderedACKTags(a.ko.Spec.Tags)
872+
latestACKTags, _ := convertToOrderedACKTags(b.ko.Spec.Tags)
873+
if !ackcompare.MapStringStringEqual(desiredACKTags, latestACKTags) {
874+
delta.Add("Spec.Tags", a.ko.Spec.Tags, b.ko.Spec.Tags)
875+
}
876+
`
877+
got, err := code.CompareResource(
878+
crd.Config(), crd, "delta", "a.ko", "b.ko", 1,
879+
)
880+
require.NoError(err)
881+
assert.Equal(expected, got)
882+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
resources:
2+
Topic:
3+
is_arn_primary_key: true
4+
unpack_attributes_map:
5+
set_attributes_single_attribute: true
6+
fields:
7+
DeliveryPolicy:
8+
is_attribute: true
9+
DisplayName:
10+
is_attribute: true
11+
compare:
12+
nil_equals_zero_value: true
13+
Policy:
14+
is_attribute: true
15+
KmsMasterKeyId:
16+
is_attribute: true
17+
Owner:
18+
is_attribute: true
19+
is_read_only: true
20+
is_owner_account_id: true
21+
EffectiveDeliveryPolicy:
22+
is_attribute: true
23+
is_read_only: true
24+
TopicArn:
25+
is_attribute: true
26+
is_read_only: true
27+
ignore:
28+
resource_names:
29+
- PlatformApplication
30+
- Endpoint
31+
- Subscription
32+
field_paths:
33+
- CreateTopicInput.DataProtectionPolicy

0 commit comments

Comments
 (0)