Skip to content

Commit 1134a58

Browse files
zoumozoumo
authored
feat(client): add maps utils and refactor writer interface (#102)
* feat(client): add maps utils and refactor writer interface - Add new maps util functions for labels and annotations - Rename util.go to writer.go and reorganize patch logic * fix lint
1 parent 9f5d345 commit 1134a58

5 files changed

Lines changed: 247 additions & 37 deletions

File tree

‎client/maps.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
Copyright 2025 The KusionStack Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package client
18+
19+
import "sigs.k8s.io/controller-runtime/pkg/client"
20+
21+
func MutateLabels(obj client.Object, mutateFn func(map[string]string)) {
22+
if mutateFn == nil {
23+
return
24+
}
25+
labels := obj.GetLabels()
26+
labels = mutateMapStrings(labels, mutateFn)
27+
obj.SetLabels(labels)
28+
}
29+
30+
func MutateAnnotations(obj client.Object, mutateFn func(map[string]string)) {
31+
if mutateFn == nil {
32+
return
33+
}
34+
35+
annotations := obj.GetAnnotations()
36+
annotations = mutateMapStrings(annotations, mutateFn)
37+
obj.SetAnnotations(annotations)
38+
}
39+
40+
func mutateMapStrings(in map[string]string, mutateFn func(map[string]string)) map[string]string {
41+
var mapIsNil bool
42+
if in == nil {
43+
in = make(map[string]string)
44+
mapIsNil = true
45+
}
46+
mutateFn(in)
47+
if mapIsNil && len(in) == 0 {
48+
// keep map nil if it's empty
49+
in = nil
50+
}
51+
52+
return in
53+
}
54+
55+
func GetMapValueByDefault[K comparable, V any](m map[K]V, key K, defaultValue V) V {
56+
v, ok := m[key]
57+
if !ok {
58+
return defaultValue
59+
}
60+
return v
61+
}

‎client/maps_test.go‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/*
2+
Copyright 2025 The KusionStack Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package client
18+
19+
import (
20+
"github.com/stretchr/testify/suite"
21+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
22+
"sigs.k8s.io/controller-runtime/pkg/client"
23+
)
24+
25+
type mapsTestSuite struct {
26+
suite.Suite
27+
}
28+
29+
func (s *mapsTestSuite) TestMutateLabelsAndAnnotations() {
30+
type testCase struct {
31+
name string
32+
setupFunc func() client.Object
33+
mutateLabels func(labels map[string]string)
34+
expectedLabels map[string]string
35+
mutateAnnotations func(annotations map[string]string)
36+
expectedAnnotations map[string]string
37+
}
38+
39+
testCases := []testCase{
40+
{
41+
name: "mutate pod labels and annotations",
42+
setupFunc: func() client.Object {
43+
return newTestRandomPod()
44+
},
45+
mutateLabels: func(labels map[string]string) {
46+
labels["mutate-labels"] = "test"
47+
},
48+
expectedLabels: map[string]string{
49+
"version": "v1",
50+
"mutate-labels": "test",
51+
},
52+
mutateAnnotations: func(annotations map[string]string) {
53+
annotations["mutate-annotations"] = "test"
54+
},
55+
expectedAnnotations: map[string]string{
56+
"mutate-annotations": "test",
57+
},
58+
},
59+
{
60+
name: "mutate unstructured labels and annotations",
61+
setupFunc: func() client.Object {
62+
return &unstructured.Unstructured{Object: map[string]any{}}
63+
},
64+
mutateLabels: func(labels map[string]string) {
65+
labels["mutate-labels"] = "test"
66+
},
67+
expectedLabels: map[string]string{
68+
"mutate-labels": "test",
69+
},
70+
mutateAnnotations: func(annotations map[string]string) {
71+
annotations["mutate-annotations"] = "test"
72+
},
73+
expectedAnnotations: map[string]string{
74+
"mutate-annotations": "test",
75+
},
76+
},
77+
{
78+
name: "keep nil annotation",
79+
setupFunc: func() client.Object {
80+
return newTestRandomPod()
81+
},
82+
mutateAnnotations: func(annotations map[string]string) {
83+
},
84+
expectedAnnotations: nil,
85+
},
86+
{
87+
name: "keep empty labels",
88+
setupFunc: func() client.Object {
89+
return newTestRandomPod()
90+
},
91+
mutateLabels: func(labels map[string]string) {
92+
delete(labels, "version")
93+
},
94+
expectedLabels: map[string]string{},
95+
},
96+
}
97+
for i := range testCases {
98+
tc := testCases[i]
99+
s.Run(tc.name, func() {
100+
obj := tc.setupFunc()
101+
102+
if tc.mutateLabels != nil {
103+
MutateLabels(obj, tc.mutateLabels)
104+
s.Equal(tc.expectedLabels, obj.GetLabels())
105+
}
106+
107+
if tc.mutateAnnotations != nil {
108+
MutateAnnotations(obj, tc.mutateAnnotations)
109+
s.Equal(tc.expectedAnnotations, obj.GetAnnotations())
110+
}
111+
})
112+
}
113+
}
114+
115+
func (s *mapsTestSuite) TestGetMapValueByDefault() {
116+
type testCase struct {
117+
name string
118+
m map[string]string
119+
key string
120+
defValue string
121+
expected string
122+
}
123+
124+
testCases := []testCase{
125+
{
126+
name: "get value by key",
127+
m: map[string]string{"key": "value"},
128+
key: "key",
129+
expected: "value",
130+
},
131+
{
132+
name: "get value by default",
133+
m: map[string]string{},
134+
key: "key",
135+
defValue: "default",
136+
expected: "default",
137+
},
138+
{
139+
name: "get value from nil map",
140+
m: nil,
141+
key: "key",
142+
defValue: "default",
143+
expected: "default",
144+
},
145+
}
146+
for i := range testCases {
147+
tc := testCases[i]
148+
s.Run(tc.name, func() {
149+
s.Equal(tc.expected, GetMapValueByDefault(tc.m, tc.key, tc.defValue))
150+
})
151+
}
152+
}

‎client/suit_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,7 @@ import (
2727
func TestClientTestSuite(t *testing.T) {
2828
suite.Run(t, new(clientTestSuite))
2929
}
30+
31+
func TestMapsTestSuite(t *testing.T) {
32+
suite.Run(t, new(mapsTestSuite))
33+
}

client/util.go renamed to client/writer.go

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ type UpdateWriter interface {
3737
Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error
3838
}
3939

40+
type PatchWriter interface {
41+
// Patch patches the given obj in the Kubernetes cluster. obj must be a
42+
// struct pointer so that obj can be updated with the content returned by the Server.
43+
Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error
44+
}
45+
4046
// UpdateOnConflict attempts to update a resource while avoiding conflicts that may arise
4147
// from concurrent modifications. It utilizes the mutateFn function to apply changes to the
4248
// input obj and then attempts an update using the writer, which can be either client.Writer
@@ -89,17 +95,6 @@ func UpdateOnConflict[T client.Object](
8995
return changed, err
9096
}
9197

92-
// mutate wraps a MutateFn and applies validation to its result.
93-
func mutate[T client.Object](f MutateFn[T], key client.ObjectKey, obj T) error {
94-
if err := f(obj); err != nil {
95-
return err
96-
}
97-
if newKey := client.ObjectKeyFromObject(obj); key != newKey {
98-
return fmt.Errorf("MutateFn cannot mutate object name and/or object namespace")
99-
}
100-
return nil
101-
}
102-
10398
// CreateOrUpdateOnConflict creates or updates the given object in the Kubernetes
10499
// cluster. The object's desired state must be reconciled with the existing
105100
// state inside the passed in callback MutateFn.
@@ -138,12 +133,6 @@ func CreateOrUpdateOnConflict[T client.Object](
138133
return controllerutil.OperationResultUpdated, nil
139134
}
140135

141-
type PatchWriter interface {
142-
// Patch patches the given obj in the Kubernetes cluster. obj must be a
143-
// struct pointer so that obj can be updated with the content returned by the Server.
144-
Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error
145-
}
146-
147136
// Patch applies changes to a Kubernetes object using the merge patch strategy.
148137
// It creates a merge patch from the original object, applies the mutateFn to modify the object,
149138
// and then applies the patch if there are actual changes.
@@ -154,27 +143,22 @@ func Patch[T client.Object](
154143
inputObj T,
155144
mutateFn MutateFn[T],
156145
) (changed bool, err error) {
157-
original := inputObj.DeepCopyObject().(client.Object)
146+
key := client.ObjectKeyFromObject(inputObj)
158147

159-
mergePatch := client.MergeFrom(original)
148+
original := inputObj.DeepCopyObject().(client.Object)
160149

161150
// modify inputObj object
162-
err = mutateFn(inputObj)
163-
if err != nil {
164-
return false, err
165-
}
166-
167-
// calculate patch data
168-
patchData, err := mergePatch.Data(inputObj)
151+
err = mutate(mutateFn, key, inputObj)
169152
if err != nil {
170153
return false, err
171154
}
172155

173-
if len(patchData) == 0 || string(patchData) == "{}" {
156+
if equality.Semantic.DeepEqual(original, inputObj) {
174157
// nothing changed, skip update
175158
return false, nil
176159
}
177160

161+
mergePatch := client.MergeFrom(original)
178162
err = writer.Patch(ctx, inputObj, mergePatch)
179163
if err != nil {
180164
return false, err
@@ -218,19 +202,11 @@ func PatchOnConflict[T client.Object](
218202
original := inputObj.DeepCopyObject().(client.Object)
219203

220204
// modify inputObj object
221-
if innerErr := mutateFn(inputObj); innerErr != nil {
205+
if innerErr := mutate(mutateFn, key, inputObj); innerErr != nil {
222206
return innerErr
223207
}
224208

225-
mergePatch := client.MergeFrom(original)
226-
227-
// calculate patch data
228-
patchData, err := mergePatch.Data(inputObj)
229-
if err != nil {
230-
return err
231-
}
232-
233-
if len(patchData) == 0 || string(patchData) == "{}" {
209+
if equality.Semantic.DeepEqual(original, inputObj) {
234210
// nothing changed, skip update
235211
return nil
236212
}
@@ -247,3 +223,14 @@ func PatchOnConflict[T client.Object](
247223

248224
return changed, err
249225
}
226+
227+
// mutate wraps a MutateFn and applies validation to its result.
228+
func mutate[T client.Object](f MutateFn[T], key client.ObjectKey, obj T) error {
229+
if err := f(obj); err != nil {
230+
return err
231+
}
232+
if newKey := client.ObjectKeyFromObject(obj); key != newKey {
233+
return fmt.Errorf("MutateFn cannot mutate object name and/or object namespace")
234+
}
235+
return nil
236+
}

client/util_test.go renamed to client/writer_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ func (s *clientTestSuite) TestConflict() {
163163
mergePatch := client.MergeFromWithOptions(oldPod, client.MergeFromWithOptimisticLock{})
164164
err = s.cli.Patch(context.Background(), modified, mergePatch)
165165
s.True(errors.IsConflict(err), "patch should fail on conflict")
166+
167+
modified = oldPod.DeepCopy()
168+
modified.Labels["version"] = "v5"
169+
mergePatch = client.MergeFrom(oldPod)
170+
err = s.cli.Patch(context.Background(), modified, mergePatch)
171+
s.Require().NoError(err, "patch will succeed without optimistic lock")
166172
}
167173

168174
func (s *clientTestSuite) TestUpdateOnConflict() {

0 commit comments

Comments
 (0)