Skip to content

Commit 9900866

Browse files
jaypipesjaypipes
committed
update pkg/resource templates for new Delta code
Adds plumbing to the `pkg/resource/depscriptor.go.tpl` template that implements the `Delta()` method on the `github.com/aws-controllers-k8s/runtime/pkg/types.AWSResourceDescriptor` interface-implementing `resource` struct. Also adds a test for the compare field ignore functionality.
1 parent 425cab5 commit 9900866

File tree

13 files changed

+70
-79
lines changed

13 files changed

+70
-79
lines changed

‎pkg/generate/ack/controller.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ var (
9393
"GoCodeSetDeleteInput": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int) string {
9494
return code.SetSDK(r.Config(), r, ackmodel.OpTypeDelete, sourceVarName, targetVarName, indentLevel)
9595
},
96+
"GoCodeCompare": func(r *ackmodel.CRD, deltaVarName string, sourceVarName string, targetVarName string, indentLevel int) string {
97+
return code.CompareResource(r.Config(), r, deltaVarName, sourceVarName, targetVarName, indentLevel)
98+
},
9699
"Empty": func(subject string) bool {
97100
return strings.TrimSpace(subject) == ""
98101
},
@@ -130,6 +133,7 @@ func Controller(
130133

131134
// First add all the CRD pkg/resource templates
132135
targets := []string{
136+
"delta.go.tpl",
133137
"descriptor.go.tpl",
134138
"identifiers.go.tpl",
135139
"manager.go.tpl",

‎pkg/generate/code/compare.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,11 @@ func CompareResource(
107107
)
108108

109109
if nilCode != "" {
110-
// else {
111-
out += nilCode + " else {\n"
110+
// else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil {
111+
out += fmt.Sprintf(
112+
"%s else if %s != nil && %s != nil {\n",
113+
nilCode, firstResAdaptedVarName, secondResAdaptedVarName,
114+
)
112115
indentLevel++
113116
} else {
114117
out += "\n"
@@ -518,8 +521,11 @@ func compareStruct(
518521
)
519522

520523
if nilCode != "" {
521-
// else {
522-
out += nilCode + " else {\n"
524+
// else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil {
525+
out += fmt.Sprintf(
526+
"%s else if %s != nil && %s != nil {\n",
527+
nilCode, firstResAdaptedVarName, secondResAdaptedVarName,
528+
)
523529
indentLevel++
524530
} else {
525531
out += "\n"

‎pkg/generate/code/compare_test.go

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,70 +32,65 @@ func TestCompareResource_S3_Bucket(t *testing.T) {
3232
crd := testutil.GetCRDByName(t, g, "Bucket")
3333
require.NotNil(crd)
3434

35+
// The ACL field is ignored in the S3 generator config and therefore should
36+
// not appear in this output.
3537
expected := `
36-
if ackcompare.HasNilDifference(a.ko.Spec.ACL, b.ko.Spec.ACL) {
37-
delta.Add("Spec.ACL", a.ko.Spec.ACL, b.ko.Spec.ACL)
38-
} else {
39-
if *a.ko.Spec.ACL != *b.ko.Spec.ACL {
40-
delta.Add("Spec.ACL", a.ko.Spec.ACL, b.ko.Spec.ACL)
41-
}
42-
}
4338
if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration) {
4439
delta.Add("Spec.CreateBucketConfiguration", a.ko.Spec.CreateBucketConfiguration, b.ko.Spec.CreateBucketConfiguration)
45-
} else {
40+
} else if a.ko.Spec.CreateBucketConfiguration != nil && b.ko.Spec.CreateBucketConfiguration != nil {
4641
if ackcompare.HasNilDifference(a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint) {
4742
delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
48-
} else {
43+
} else if a.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil && b.ko.Spec.CreateBucketConfiguration.LocationConstraint != nil {
4944
if *a.ko.Spec.CreateBucketConfiguration.LocationConstraint != *b.ko.Spec.CreateBucketConfiguration.LocationConstraint {
5045
delta.Add("Spec.CreateBucketConfiguration.LocationConstraint", a.ko.Spec.CreateBucketConfiguration.LocationConstraint, b.ko.Spec.CreateBucketConfiguration.LocationConstraint)
5146
}
5247
}
5348
}
5449
if ackcompare.HasNilDifference(a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl) {
5550
delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
56-
} else {
51+
} else if a.ko.Spec.GrantFullControl != nil && b.ko.Spec.GrantFullControl != nil {
5752
if *a.ko.Spec.GrantFullControl != *b.ko.Spec.GrantFullControl {
5853
delta.Add("Spec.GrantFullControl", a.ko.Spec.GrantFullControl, b.ko.Spec.GrantFullControl)
5954
}
6055
}
6156
if ackcompare.HasNilDifference(a.ko.Spec.GrantRead, b.ko.Spec.GrantRead) {
6257
delta.Add("Spec.GrantRead", a.ko.Spec.GrantRead, b.ko.Spec.GrantRead)
63-
} else {
58+
} else if a.ko.Spec.GrantRead != nil && b.ko.Spec.GrantRead != nil {
6459
if *a.ko.Spec.GrantRead != *b.ko.Spec.GrantRead {
6560
delta.Add("Spec.GrantRead", a.ko.Spec.GrantRead, b.ko.Spec.GrantRead)
6661
}
6762
}
6863
if ackcompare.HasNilDifference(a.ko.Spec.GrantReadACP, b.ko.Spec.GrantReadACP) {
6964
delta.Add("Spec.GrantReadACP", a.ko.Spec.GrantReadACP, b.ko.Spec.GrantReadACP)
70-
} else {
65+
} else if a.ko.Spec.GrantReadACP != nil && b.ko.Spec.GrantReadACP != nil {
7166
if *a.ko.Spec.GrantReadACP != *b.ko.Spec.GrantReadACP {
7267
delta.Add("Spec.GrantReadACP", a.ko.Spec.GrantReadACP, b.ko.Spec.GrantReadACP)
7368
}
7469
}
7570
if ackcompare.HasNilDifference(a.ko.Spec.GrantWrite, b.ko.Spec.GrantWrite) {
7671
delta.Add("Spec.GrantWrite", a.ko.Spec.GrantWrite, b.ko.Spec.GrantWrite)
77-
} else {
72+
} else if a.ko.Spec.GrantWrite != nil && b.ko.Spec.GrantWrite != nil {
7873
if *a.ko.Spec.GrantWrite != *b.ko.Spec.GrantWrite {
7974
delta.Add("Spec.GrantWrite", a.ko.Spec.GrantWrite, b.ko.Spec.GrantWrite)
8075
}
8176
}
8277
if ackcompare.HasNilDifference(a.ko.Spec.GrantWriteACP, b.ko.Spec.GrantWriteACP) {
8378
delta.Add("Spec.GrantWriteACP", a.ko.Spec.GrantWriteACP, b.ko.Spec.GrantWriteACP)
84-
} else {
79+
} else if a.ko.Spec.GrantWriteACP != nil && b.ko.Spec.GrantWriteACP != nil {
8580
if *a.ko.Spec.GrantWriteACP != *b.ko.Spec.GrantWriteACP {
8681
delta.Add("Spec.GrantWriteACP", a.ko.Spec.GrantWriteACP, b.ko.Spec.GrantWriteACP)
8782
}
8883
}
8984
if ackcompare.HasNilDifference(a.ko.Spec.Name, b.ko.Spec.Name) {
9085
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
91-
} else {
86+
} else if a.ko.Spec.Name != nil && b.ko.Spec.Name != nil {
9287
if *a.ko.Spec.Name != *b.ko.Spec.Name {
9388
delta.Add("Spec.Name", a.ko.Spec.Name, b.ko.Spec.Name)
9489
}
9590
}
9691
if ackcompare.HasNilDifference(a.ko.Spec.ObjectLockEnabledForBucket, b.ko.Spec.ObjectLockEnabledForBucket) {
9792
delta.Add("Spec.ObjectLockEnabledForBucket", a.ko.Spec.ObjectLockEnabledForBucket, b.ko.Spec.ObjectLockEnabledForBucket)
98-
} else {
93+
} else if a.ko.Spec.ObjectLockEnabledForBucket != nil && b.ko.Spec.ObjectLockEnabledForBucket != nil {
9994
if *a.ko.Spec.ObjectLockEnabledForBucket != *b.ko.Spec.ObjectLockEnabledForBucket {
10095
delta.Add("Spec.ObjectLockEnabledForBucket", a.ko.Spec.ObjectLockEnabledForBucket, b.ko.Spec.ObjectLockEnabledForBucket)
10196
}

‎pkg/generate/config/field.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ type CompareFieldConfig struct {
111111
// IsIgnored indicates the field should be ignored when comparing a
112112
// resource
113113
IsIgnored bool `json:"is_ignored"`
114-
// NilEqualsEmptyString indicates a nil string pointer and empty string
115-
// should be considered equal for the purposes of comparison
116-
NilEqualsEmptyString bool `json:"nil_equals_empty_string"`
114+
// NilEqualsZeroValue indicates a nil pointer and zero-value pointed-to
115+
// value should be considered equal for the purposes of comparison
116+
NilEqualsZeroValue bool `json:"nil_equals_zero_value"`
117117
}
118118

119119
// FieldConfig contains instructions to the code generator about how

‎pkg/generate/testdata/models/apis/s3/0000-00-00/generator.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,9 @@ resources:
1818
list_operation:
1919
match_fields:
2020
- Name
21+
fields:
22+
ACL:
23+
# This is to test the ackcompare field ignore functionality. This
24+
# should NOT be in a production generator.yaml...
25+
compare:
26+
is_ignored: true

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{{ template "boilerplate" }}
2+
3+
package {{ .CRD.Names.Snake }}
4+
5+
import (
6+
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
7+
)
8+
9+
// newResourceDelta returns a new `ackcompare.Delta` used to compare two
10+
// resources
11+
func newResourceDelta(
12+
a *resource,
13+
b *resource,
14+
) *ackcompare.Delta {
15+
delta := ackcompare.NewDelta()
16+
if ((a == nil && b != nil) ||
17+
(a != nil && b == nil)) {
18+
delta.Add("", a, b)
19+
return delta
20+
}
21+
{{ GoCodeCompare .CRD "delta" "a.ko" "b.ko" 1}}
22+
return delta
23+
}

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

Lines changed: 4 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ package {{ .CRD.Names.Snake }}
55
import (
66
acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
77
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
8-
"github.com/google/go-cmp/cmp"
9-
"github.com/google/go-cmp/cmp/cmpopts"
108
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
119
k8sapirt "k8s.io/apimachinery/pkg/runtime"
1210
k8sctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -52,51 +50,10 @@ func (d *resourceDescriptor) ResourceFromRuntimeObject(
5250
}
5351
}
5452

55-
// Equal returns true if the two supplied AWSResources have the same content.
56-
// The underlying types of the two supplied AWSResources should be the same. In
57-
// other words, the Equal() method should be called with the same concrete
58-
// implementing AWSResource type
59-
func (d *resourceDescriptor) Equal(
60-
a acktypes.AWSResource,
61-
b acktypes.AWSResource,
62-
) bool {
63-
ac := a.(*resource)
64-
bc := b.(*resource)
65-
opts := []cmp.Option{cmpopts.EquateEmpty()}
66-
{{- if .CRD.CompareIgnoredFields }}
67-
opts = append(opts, cmpopts.IgnoreFields(*ac.ko,
68-
{{- range $fieldPath := .CRD.CompareIgnoredFields }}
69-
{{ printf "%q" $fieldPath }},
70-
{{- end }}
71-
))
72-
{{- end }}
73-
return cmp.Equal(ac.ko, bc.ko, opts...)
74-
}
75-
76-
// Diff returns a Reporter which provides the difference between two supplied
77-
// AWSResources. The underlying types of the two supplied AWSResources should
78-
// be the same. In other words, the Diff() method should be called with the
79-
// same concrete implementing AWSResource type
80-
func (d *resourceDescriptor) Diff(
81-
a acktypes.AWSResource,
82-
b acktypes.AWSResource,
83-
) *ackcompare.Reporter {
84-
ac := a.(*resource)
85-
bc := b.(*resource)
86-
var diffReporter ackcompare.Reporter
87-
opts := []cmp.Option{
88-
cmp.Reporter(&diffReporter),
89-
cmp.AllowUnexported(svcapitypes.{{ .CRD.Kind }}{}),
90-
}
91-
{{- if .CRD.CompareIgnoredFields }}
92-
opts = append(opts, cmpopts.IgnoreFields(*ac.ko,
93-
{{- range $fieldPath := .CRD.CompareIgnoredFields }}
94-
{{ printf "%q" $fieldPath }},
95-
{{- end }}
96-
))
97-
{{- end }}
98-
cmp.Equal(ac.ko, bc.ko, opts...)
99-
return &diffReporter
53+
// Delta returns an `ackcompare.Delta` object containing the difference between
54+
// one `AWSResource` and another.
55+
func (d *resourceDescriptor) Delta(a, b acktypes.AWSResource) *ackcompare.Delta {
56+
return newResourceDelta(a.(*resource), b.(*resource))
10057
}
10158

10259
// UpdateCRStatus accepts an AWSResource object and changes the Status

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,15 @@ func (rm *resourceManager) Update(
112112
ctx context.Context,
113113
resDesired acktypes.AWSResource,
114114
resLatest acktypes.AWSResource,
115-
diffReporter *ackcompare.Reporter,
115+
delta *ackcompare.Delta,
116116
) (acktypes.AWSResource, error) {
117117
desired := rm.concreteResource(resDesired)
118118
latest := rm.concreteResource(resLatest)
119119
if desired.ko == nil || latest.ko == nil {
120120
// Should never happen... if it does, it's buggy code.
121121
panic("resource manager's Update() method received resource with nil CR object")
122122
}
123-
updated, err := rm.sdkUpdate(ctx, desired, latest, diffReporter)
123+
updated, err := rm.sdkUpdate(ctx, desired, latest, delta)
124124
if err != nil {
125125
return rm.onError(latest, err)
126126
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
svcapitypes "github.com/aws-controllers-k8s/{{ .ServiceIDClean }}-controller/apis/{{ .APIVersion}}"
1212
)
1313

14-
// resource implements the `aws-service-operator-k8s/pkg/types.AWSResource`
14+
// resource implements the `aws-controller-k8s/runtime/pkg/types.AWSResource`
1515
// interface
1616
type resource struct {
1717
// The Kubernetes-native CR representing the resource

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ func (rm *resourceManager) sdkUpdate(
33
ctx context.Context,
44
desired *resource,
55
latest *resource,
6-
diffReporter *ackcompare.Reporter,
6+
delta *ackcompare.Delta,
77
) (*resource, error) {
88
{{ $customMethod := .CRD.GetCustomImplementation .CRD.Ops.Update }}
99
{{ if $customMethod }}
10-
customResp, customRespErr := rm.{{ $customMethod }}(ctx, desired, latest, diffReporter)
10+
customResp, customRespErr := rm.{{ $customMethod }}(ctx, desired, latest, delta)
1111
if customResp != nil || customRespErr != nil {
1212
return customResp, customRespErr
1313
}

0 commit comments

Comments
 (0)