Skip to content

Commit cc01bf5

Browse files
ardikabsardikabs
authored
fix: use Patch API for infra-client (#3034)
* fix(infrastructure): use Patch API instead Signed-off-by: Ardika Bagus <[email protected]> * chore: add interceptor for ApplyPatch on fake client Signed-off-by: Ardika Bagus <[email protected]> * chore: trigger make generate Signed-off-by: Ardika Bagus <[email protected]> * chore: remove update verb Signed-off-by: Ardika Bagus <[email protected]> * chore: SetUID no longer needed Signed-off-by: Ardika Bagus <[email protected]> --------- Signed-off-by: Ardika Bagus <[email protected]>
1 parent 2a38de6 commit cc01bf5

File tree

9 files changed

+91
-19
lines changed

9 files changed

+91
-19
lines changed

‎charts/gateway-helm/templates/infra-manager-rbac.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,26 @@ rules:
1414
verbs:
1515
- create
1616
- get
17-
- update
1817
- delete
18+
- patch
1919
- apiGroups:
2020
- apps
2121
resources:
2222
- deployments
2323
verbs:
2424
- create
2525
- get
26-
- update
2726
- delete
27+
- patch
2828
- apiGroups:
2929
- autoscaling
3030
resources:
3131
- horizontalpodautoscalers
3232
verbs:
3333
- create
3434
- get
35-
- update
3635
- delete
36+
- patch
3737
---
3838
apiVersion: rbac.authorization.k8s.io/v1
3939
kind: RoleBinding

‎internal/infrastructure/kubernetes/infra_client.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ func (cli *InfraClient) CreateOrUpdate(ctx context.Context, key client.ObjectKey
3838
// Since the client.Object does not have a specific Spec field to compare
3939
// just perform an update for now.
4040
if updateChecker() {
41-
specific.SetUID(current.GetUID())
42-
if err := cli.Client.Update(ctx, specific); err != nil {
41+
opts := []client.PatchOption{client.ForceOwnership, client.FieldOwner("envoy-gateway")}
42+
if err := cli.Client.Patch(ctx, specific, client.Apply, opts...); err != nil {
4343
return fmt.Errorf("for Update: %w", err)
4444
}
4545
}

‎internal/infrastructure/kubernetes/proxy_configmap_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,16 @@ func TestCreateOrUpdateProxyConfigMap(t *testing.T) {
9999
t.Run(tc.name, func(t *testing.T) {
100100
var cli client.Client
101101
if tc.current != nil {
102-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(tc.current).Build()
102+
cli = fakeclient.NewClientBuilder().
103+
WithScheme(envoygateway.GetScheme()).
104+
WithObjects(tc.current).
105+
WithInterceptorFuncs(interceptorFunc).
106+
Build()
103107
} else {
104-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
108+
cli = fakeclient.NewClientBuilder().
109+
WithScheme(envoygateway.GetScheme()).
110+
WithInterceptorFuncs(interceptorFunc).
111+
Build()
105112
}
106113
kube := NewInfra(cli, cfg)
107114
r := proxy.NewResourceRender(kube.Namespace, infra.GetProxyInfra())

‎internal/infrastructure/kubernetes/proxy_deployment_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,16 @@ func TestCreateOrUpdateProxyDeployment(t *testing.T) {
106106
t.Run(tc.name, func(t *testing.T) {
107107
var cli client.Client
108108
if tc.current != nil {
109-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(tc.current).Build()
109+
cli = fakeclient.NewClientBuilder().
110+
WithScheme(envoygateway.GetScheme()).
111+
WithObjects(tc.current).
112+
WithInterceptorFuncs(interceptorFunc).
113+
Build()
110114
} else {
111-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
115+
cli = fakeclient.NewClientBuilder().
116+
WithScheme(envoygateway.GetScheme()).
117+
WithInterceptorFuncs(interceptorFunc).
118+
Build()
112119
}
113120

114121
kube := NewInfra(cli, cfg)

‎internal/infrastructure/kubernetes/proxy_infra_test.go‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,21 @@ package kubernetes
77

88
import (
99
"context"
10+
"errors"
11+
"fmt"
1012
"reflect"
1113
"testing"
1214

1315
"github.com/stretchr/testify/assert"
1416
"github.com/stretchr/testify/require"
1517
appsv1 "k8s.io/api/apps/v1"
1618
corev1 "k8s.io/api/core/v1"
19+
kerrors "k8s.io/apimachinery/pkg/api/errors"
1720
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
"k8s.io/apimachinery/pkg/types"
1822
"sigs.k8s.io/controller-runtime/pkg/client"
1923
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
24+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
2025
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
2126

2227
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -28,10 +33,42 @@ import (
2833
)
2934

3035
func newTestInfra(t *testing.T) *Infra {
31-
cli := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
36+
cli := fakeclient.NewClientBuilder().
37+
WithScheme(envoygateway.GetScheme()).
38+
WithInterceptorFuncs(interceptorFunc).
39+
Build()
3240
return newTestInfraWithClient(t, cli)
3341
}
3442

43+
// Borrowing the interceptor from https://github.com/istio/istio/blob/2f54c6a52a5c6661d5eb9bd2277aab77304fee45/operator/pkg/helmreconciler/apply_test.go#L40
44+
// Interceptor is used for ApplyPatch as of this patch is not yet supported by the fake client, see https://github.com/kubernetes/kubernetes/issues/99953
45+
var interceptorFunc = interceptor.Funcs{Patch: func(
46+
ctx context.Context,
47+
clnt client.WithWatch,
48+
obj client.Object,
49+
patch client.Patch,
50+
opts ...client.PatchOption,
51+
) error {
52+
// Apply patches are supposed to upsert, but fake client fails if the object doesn't exist,
53+
// if an apply patch occurs for an object that doesn't yet exist, create it.
54+
if patch.Type() != types.ApplyPatchType {
55+
return clnt.Patch(ctx, obj, patch, opts...)
56+
}
57+
check, ok := obj.DeepCopyObject().(client.Object)
58+
if !ok {
59+
return errors.New("could not check for object in fake client")
60+
}
61+
if err := clnt.Get(ctx, client.ObjectKeyFromObject(obj), check); kerrors.IsNotFound(err) {
62+
if err := clnt.Create(ctx, check); err != nil {
63+
return fmt.Errorf("could not inject object creation for fake: %w", err)
64+
}
65+
} else if err != nil {
66+
return err
67+
}
68+
obj.SetResourceVersion(check.GetResourceVersion())
69+
return clnt.Update(ctx, obj)
70+
}}
71+
3572
func TestCmpBytes(t *testing.T) {
3673
m1 := map[string][]byte{}
3774
m1["a"] = []byte("aaa")

‎internal/infrastructure/kubernetes/proxy_serviceaccount_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,16 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) {
174174

175175
var cli client.Client
176176
if tc.current != nil {
177-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(tc.current).Build()
177+
cli = fakeclient.NewClientBuilder().
178+
WithScheme(envoygateway.GetScheme()).
179+
WithObjects(tc.current).
180+
WithInterceptorFuncs(interceptorFunc).
181+
Build()
178182
} else {
179-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
183+
cli = fakeclient.NewClientBuilder().
184+
WithScheme(envoygateway.GetScheme()).
185+
WithInterceptorFuncs(interceptorFunc).
186+
Build()
180187
}
181188

182189
kube := NewInfra(cli, cfg)

‎internal/infrastructure/kubernetes/ratelimit_deployment_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,16 @@ func TestCreateOrUpdateRateLimitDeployment(t *testing.T) {
6868
t.Run(tc.name, func(t *testing.T) {
6969
var cli client.Client
7070
if tc.current != nil {
71-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(tc.current).Build()
71+
cli = fakeclient.NewClientBuilder().
72+
WithScheme(envoygateway.GetScheme()).
73+
WithObjects(tc.current).
74+
WithInterceptorFuncs(interceptorFunc).
75+
Build()
7276
} else {
73-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
77+
cli = fakeclient.NewClientBuilder().
78+
WithScheme(envoygateway.GetScheme()).
79+
WithInterceptorFuncs(interceptorFunc).
80+
Build()
7481
}
7582

7683
kube := NewInfra(cli, cfg)

‎internal/infrastructure/kubernetes/ratelimit_serviceaccount_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,16 @@ func TestCreateOrUpdateRateLimitServiceAccount(t *testing.T) {
9292
t.Run(tc.name, func(t *testing.T) {
9393
var cli client.Client
9494
if tc.current != nil {
95-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects(tc.current).Build()
95+
cli = fakeclient.NewClientBuilder().
96+
WithScheme(envoygateway.GetScheme()).
97+
WithObjects(tc.current).
98+
WithInterceptorFuncs(interceptorFunc).
99+
Build()
96100
} else {
97-
cli = fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build()
101+
cli = fakeclient.NewClientBuilder().
102+
WithScheme(envoygateway.GetScheme()).
103+
WithInterceptorFuncs(interceptorFunc).
104+
Build()
98105
}
99106

100107
cfg, err := config.New()

‎test/helm/default.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,26 +186,26 @@ rules:
186186
verbs:
187187
- create
188188
- get
189-
- update
190189
- delete
190+
- patch
191191
- apiGroups:
192192
- apps
193193
resources:
194194
- deployments
195195
verbs:
196196
- create
197197
- get
198-
- update
199198
- delete
199+
- patch
200200
- apiGroups:
201201
- autoscaling
202202
resources:
203203
- horizontalpodautoscalers
204204
verbs:
205205
- create
206206
- get
207-
- update
208207
- delete
208+
- patch
209209
---
210210
# Source: gateway-helm/templates/leader-election-rbac.yaml
211211
apiVersion: rbac.authorization.k8s.io/v1

0 commit comments

Comments
 (0)