Skip to content

Commit e3937b3

Browse files
authored
Merge pull request kubernetes-retired#206 from erikgb/feat/readonly-client
Read-only client in webhooks only mode
2 parents 185c46b + 683a23b commit e3937b3

File tree

8 files changed

+110
-74
lines changed

8 files changed

+110
-74
lines changed

cmd/manager/main.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func createManager() ctrl.Manager {
242242
// it turns out to be harmful.
243243
cfg.Burst = int(cfg.QPS * 1.5)
244244
mgr, err := ctrl.NewManager(cfg, ctrl.Options{
245-
NewClient: config.NewCachingClient,
245+
NewClient: config.NewClient(webhooksOnly),
246246
Scheme: scheme,
247247
MetricsBindAddress: metricsAddr,
248248
HealthProbeBindAddress: probeAddr,
@@ -304,7 +304,6 @@ func startControllers(mgr ctrl.Manager, certsReady chan struct{}) {
304304
opts := setup.Options{
305305
NoWebhooks: noWebhooks,
306306
MaxReconciles: maxReconciles,
307-
ReadOnly: webhooksOnly,
308307
HRQ: enableHRQ,
309308
}
310309
setup.Create(setupLog, mgr, f, opts)

internal/anchor/reconciler.go

-19
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ type Reconciler struct {
5252
// https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches.html) that is used to
5353
// enqueue additional objects that need updating.
5454
affected chan event.GenericEvent
55-
56-
// ReadOnly disables writebacks
57-
ReadOnly bool
5855
}
5956

6057
// Reconcile sets up some basic variables and then calls the business logic. It currently
@@ -345,10 +342,6 @@ func (r *Reconciler) getInstance(ctx context.Context, pnm, nm string) (*api.Subn
345342
}
346343

347344
func (r *Reconciler) writeInstance(ctx context.Context, log logr.Logger, inst *api.SubnamespaceAnchor) error {
348-
if r.ReadOnly {
349-
return nil
350-
}
351-
352345
if inst.CreationTimestamp.IsZero() {
353346
if err := r.Create(ctx, inst); err != nil {
354347
log.Error(err, "while creating on apiserver")
@@ -367,10 +360,6 @@ func (r *Reconciler) writeInstance(ctx context.Context, log logr.Logger, inst *a
367360
// finalizers on the instance before calling this function.
368361
//lint:ignore U1000 Ignore for now, as it may be used again in the future
369362
func (r *Reconciler) deleteInstance(ctx context.Context, inst *api.SubnamespaceAnchor) error {
370-
if r.ReadOnly {
371-
return nil
372-
}
373-
374363
if err := r.Delete(ctx, inst); err != nil {
375364
return fmt.Errorf("while deleting on apiserver: %w", err)
376365
}
@@ -393,10 +382,6 @@ func (r *Reconciler) getNamespace(ctx context.Context, nm string) (*corev1.Names
393382
}
394383

395384
func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pnm string) error {
396-
if r.ReadOnly {
397-
return nil
398-
}
399-
400385
inst := &corev1.Namespace{}
401386
inst.ObjectMeta.Name = nm
402387
metadata.SetAnnotation(inst, api.SubnamespaceOf, pnm)
@@ -414,10 +399,6 @@ func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pn
414399
}
415400

416401
func (r *Reconciler) deleteNamespace(ctx context.Context, log logr.Logger, inst *corev1.Namespace) error {
417-
if r.ReadOnly {
418-
return nil
419-
}
420-
421402
if err := r.Delete(ctx, inst); err != nil {
422403
log.Error(err, "While deleting subnamespace")
423404
return err

internal/config/cache_unstructured.go

-28
This file was deleted.

internal/config/client.go

+101
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package config
2+
3+
import (
4+
"context"
5+
6+
authzv1 "k8s.io/api/authorization/v1"
7+
"k8s.io/apimachinery/pkg/api/meta"
8+
"k8s.io/apimachinery/pkg/runtime"
9+
"k8s.io/client-go/rest"
10+
"sigs.k8s.io/controller-runtime/pkg/cache"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
12+
"sigs.k8s.io/controller-runtime/pkg/cluster"
13+
)
14+
15+
// newCachingClient is an alternative implementation of controller-runtime's
16+
// default client for manager.Manager.
17+
// The only difference is that this implementation sets `CacheUnstructured` to `true` to
18+
// cache unstructured objects.
19+
func newCachingClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
20+
c, err := client.New(config, options)
21+
if err != nil {
22+
return nil, err
23+
}
24+
25+
return client.NewDelegatingClient(client.NewDelegatingClientInput{
26+
CacheReader: cache,
27+
Client: c,
28+
UncachedObjects: uncachedObjects,
29+
CacheUnstructured: true,
30+
})
31+
}
32+
33+
func NewClient(readOnly bool) cluster.NewClientFunc {
34+
return func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
35+
c, err := newCachingClient(cache, config, options, uncachedObjects...)
36+
if readOnly {
37+
c = &readOnlyClient{client: c}
38+
}
39+
return c, err
40+
}
41+
}
42+
43+
// readOnlyClient is a client decorator that ensures no write operations are possible.
44+
type readOnlyClient struct {
45+
client client.Client
46+
}
47+
48+
func (c readOnlyClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
49+
return c.client.Get(ctx, key, obj)
50+
}
51+
52+
func (c readOnlyClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
53+
return c.client.List(ctx, list, opts...)
54+
}
55+
56+
func (c readOnlyClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
57+
// Allow callers to create subject access reviews - even in read-only mode.
58+
// Creating a SAR will not modify state in etcd. Used to perform RBAC checks.
59+
if _, ok := obj.(*authzv1.SubjectAccessReview); ok {
60+
return c.client.Create(ctx, obj, opts...)
61+
}
62+
return nil
63+
}
64+
65+
func (c readOnlyClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
66+
return nil
67+
}
68+
69+
func (c readOnlyClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
70+
return nil
71+
}
72+
73+
func (c readOnlyClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
74+
return nil
75+
}
76+
77+
func (c readOnlyClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error {
78+
return nil
79+
}
80+
81+
func (c readOnlyClient) Status() client.StatusWriter {
82+
return nopStatusWriter{}
83+
}
84+
85+
func (c readOnlyClient) Scheme() *runtime.Scheme {
86+
return c.client.Scheme()
87+
}
88+
89+
func (c readOnlyClient) RESTMapper() meta.RESTMapper {
90+
return c.client.RESTMapper()
91+
}
92+
93+
type nopStatusWriter struct{}
94+
95+
func (w nopStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
96+
return nil
97+
}
98+
99+
func (w nopStatusWriter) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
100+
return nil
101+
}

internal/hierarchyconfig/reconciler.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ type Reconciler struct {
7575
// https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches.html) that is used to
7676
// enqueue additional namespaces that need updating.
7777
affected chan event.GenericEvent
78-
79-
ReadOnly bool
8078
}
8179

8280
type AnchorReconcilerType interface {
@@ -704,7 +702,7 @@ func (r *Reconciler) writeInstances(ctx context.Context, log logr.Logger, newHC
704702
func (r *Reconciler) writeHierarchy(ctx context.Context, log logr.Logger, inst *api.HierarchyConfiguration, isDeletingNS bool) error {
705703
// The inst's name will be blank if it wasn't on the apiserver and we didn't want to make any
706704
// changes to it (e.g. no children, conditions, etc).
707-
if r.ReadOnly || inst.GetName() == "" {
705+
if inst.GetName() == "" {
708706
return nil
709707
}
710708
exists := !inst.CreationTimestamp.IsZero()
@@ -732,10 +730,6 @@ func (r *Reconciler) writeHierarchy(ctx context.Context, log logr.Logger, inst *
732730
}
733731

734732
func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, inst *corev1.Namespace) error {
735-
if r.ReadOnly {
736-
return nil
737-
}
738-
739733
// NB: HCR can't create namespaces, that's only in anchor reconciler
740734
stats.WriteNamespace()
741735
log.V(1).Info("Updating namespace on apiserver")

internal/hncconfig/reconciler.go

-7
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ type Reconciler struct {
4848
// that is used to enqueue the singleton to trigger reconciliation.
4949
trigger chan event.GenericEvent
5050

51-
// ReadOnly disables writebacks
52-
ReadOnly bool
53-
5451
// RefreshDuration is the maximum amount of time between refreshes
5552
RefreshDuration time.Duration
5653

@@ -210,10 +207,6 @@ func (r *Reconciler) validateSingleton(inst *api.HNCConfiguration) {
210207
// We will write the singleton to apiserver even it is not changed because we assume this
211208
// reconciler is called very infrequently and is not performance critical.
212209
func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfiguration) error {
213-
if r.ReadOnly {
214-
return nil
215-
}
216-
217210
if inst.CreationTimestamp.IsZero() {
218211
// No point creating it if the CRD's being deleted
219212
if isDeleted, err := crd.IsDeletingCRD(ctx, api.HNCConfigSingletons); isDeleted || err != nil {

internal/integtest/setup.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func HNCBeforeSuite() {
9999

100100
By("creating manager")
101101
k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
102-
NewClient: config.NewCachingClient,
102+
NewClient: config.NewClient(false),
103103
MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes
104104
Scheme: scheme.Scheme,
105105
})

internal/setup/reconcilers.go

+6-10
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818

1919
type Options struct {
2020
MaxReconciles int
21-
ReadOnly bool
2221
UseFakeClient bool
2322
NoWebhooks bool
2423
HNCCfgRefresh time.Duration
@@ -51,10 +50,9 @@ func CreateReconcilers(mgr ctrl.Manager, f *forest.Forest, opts Options) error {
5150

5251
// Create Anchor reconciler.
5352
ar := &anchor.Reconciler{
54-
Client: mgr.GetClient(),
55-
Log: ctrl.Log.WithName("anchor").WithName("reconcile"),
56-
Forest: f,
57-
ReadOnly: opts.ReadOnly,
53+
Client: mgr.GetClient(),
54+
Log: ctrl.Log.WithName("anchor").WithName("reconcile"),
55+
Forest: f,
5856
}
5957
f.AddListener(ar)
6058

@@ -64,16 +62,14 @@ func CreateReconcilers(mgr ctrl.Manager, f *forest.Forest, opts Options) error {
6462
Log: ctrl.Log.WithName("hncconfig").WithName("reconcile"),
6563
Manager: mgr,
6664
Forest: f,
67-
ReadOnly: opts.ReadOnly,
6865
RefreshDuration: opts.HNCCfgRefresh,
6966
}
7067

7168
// Create the HC reconciler with a pointer to the Anchor reconciler.
7269
hcr := &hierarchyconfig.Reconciler{
73-
Client: mgr.GetClient(),
74-
Log: ctrl.Log.WithName("hierarchyconfig").WithName("reconcile"),
75-
Forest: f,
76-
ReadOnly: opts.ReadOnly,
70+
Client: mgr.GetClient(),
71+
Log: ctrl.Log.WithName("hierarchyconfig").WithName("reconcile"),
72+
Forest: f,
7773
}
7874

7975
if opts.HRQ {

0 commit comments

Comments
 (0)