diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 4fe4a6692..d94b5935b 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -242,7 +242,7 @@ func createManager() ctrl.Manager { // it turns out to be harmful. cfg.Burst = int(cfg.QPS * 1.5) mgr, err := ctrl.NewManager(cfg, ctrl.Options{ - NewClient: config.NewCachingClient, + NewClient: config.NewClient(webhooksOnly), Scheme: scheme, MetricsBindAddress: metricsAddr, HealthProbeBindAddress: probeAddr, @@ -304,7 +304,6 @@ func startControllers(mgr ctrl.Manager, certsReady chan struct{}) { opts := setup.Options{ NoWebhooks: noWebhooks, MaxReconciles: maxReconciles, - ReadOnly: webhooksOnly, HRQ: enableHRQ, } setup.Create(setupLog, mgr, f, opts) diff --git a/internal/anchor/reconciler.go b/internal/anchor/reconciler.go index 1c7b625c1..7ed9e6410 100644 --- a/internal/anchor/reconciler.go +++ b/internal/anchor/reconciler.go @@ -51,9 +51,6 @@ type Reconciler struct { // https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches.html) that is used to // enqueue additional objects that need updating. affected chan event.GenericEvent - - // ReadOnly disables writebacks - ReadOnly bool } // Reconcile sets up some basic variables and then calls the business logic. It currently @@ -348,10 +345,6 @@ func (r *Reconciler) getInstance(ctx context.Context, pnm, nm string) (*api.Subn } func (r *Reconciler) writeInstance(ctx context.Context, log logr.Logger, inst *api.SubnamespaceAnchor) error { - if r.ReadOnly { - return nil - } - if inst.CreationTimestamp.IsZero() { if err := r.Create(ctx, inst); err != nil { log.Error(err, "while creating on apiserver") @@ -370,10 +363,6 @@ func (r *Reconciler) writeInstance(ctx context.Context, log logr.Logger, inst *a // finalizers on the instance before calling this function. //lint:ignore U1000 Ignore for now, as it may be used again in the future func (r *Reconciler) deleteInstance(ctx context.Context, inst *api.SubnamespaceAnchor) error { - if r.ReadOnly { - return nil - } - if err := r.Delete(ctx, inst); err != nil { return fmt.Errorf("while deleting on apiserver: %w", err) } @@ -396,10 +385,6 @@ func (r *Reconciler) getNamespace(ctx context.Context, nm string) (*corev1.Names } func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pnm string) error { - if r.ReadOnly { - return nil - } - inst := &corev1.Namespace{} inst.ObjectMeta.Name = nm metadata.SetAnnotation(inst, api.SubnamespaceOf, pnm) @@ -417,10 +402,6 @@ func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pn } func (r *Reconciler) deleteNamespace(ctx context.Context, log logr.Logger, inst *corev1.Namespace) error { - if r.ReadOnly { - return nil - } - if err := r.Delete(ctx, inst); err != nil { log.Error(err, "While deleting subnamespace") return err diff --git a/internal/config/cache_unstructured.go b/internal/config/cache_unstructured.go deleted file mode 100644 index 8065acca7..000000000 --- a/internal/config/cache_unstructured.go +++ /dev/null @@ -1,28 +0,0 @@ -package config - -import ( - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/cache" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/cluster" -) - -// NewCachingClient is an alternative implementation of controller-runtime's -// default client for manager.Manager. -// The only difference is that this implementation sets `CacheUnstructured` to `true` to -// cache unstructured objects. -func NewCachingClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { - c, err := client.New(config, options) - if err != nil { - return nil, err - } - - return client.NewDelegatingClient(client.NewDelegatingClientInput{ - CacheReader: cache, - Client: c, - UncachedObjects: uncachedObjects, - CacheUnstructured: true, - }) -} - -var _ cluster.NewClientFunc = NewCachingClient diff --git a/internal/config/client.go b/internal/config/client.go new file mode 100644 index 000000000..162b7dc74 --- /dev/null +++ b/internal/config/client.go @@ -0,0 +1,101 @@ +package config + +import ( + "context" + + authzv1 "k8s.io/api/authorization/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" +) + +// newCachingClient is an alternative implementation of controller-runtime's +// default client for manager.Manager. +// The only difference is that this implementation sets `CacheUnstructured` to `true` to +// cache unstructured objects. +func newCachingClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { + c, err := client.New(config, options) + if err != nil { + return nil, err + } + + return client.NewDelegatingClient(client.NewDelegatingClientInput{ + CacheReader: cache, + Client: c, + UncachedObjects: uncachedObjects, + CacheUnstructured: true, + }) +} + +func NewClient(readOnly bool) cluster.NewClientFunc { + return func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { + c, err := newCachingClient(cache, config, options, uncachedObjects...) + if readOnly { + c = &readOnlyClient{client: c} + } + return c, err + } +} + +// readOnlyClient is a client decorator that ensures no write operations are possible. +type readOnlyClient struct { + client client.Client +} + +func (c readOnlyClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error { + return c.client.Get(ctx, key, obj) +} + +func (c readOnlyClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return c.client.List(ctx, list, opts...) +} + +func (c readOnlyClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + // Allow callers to create subject access reviews - even in read-only mode. + // Creating a SAR will not modify state in etcd. Used to perform RBAC checks. + if _, ok := obj.(*authzv1.SubjectAccessReview); ok { + return c.client.Create(ctx, obj, opts...) + } + return nil +} + +func (c readOnlyClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil +} + +func (c readOnlyClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil +} + +func (c readOnlyClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + return nil +} + +func (c readOnlyClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { + return nil +} + +func (c readOnlyClient) Status() client.StatusWriter { + return nopStatusWriter{} +} + +func (c readOnlyClient) Scheme() *runtime.Scheme { + return c.client.Scheme() +} + +func (c readOnlyClient) RESTMapper() meta.RESTMapper { + return c.client.RESTMapper() +} + +type nopStatusWriter struct{} + +func (w nopStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil +} + +func (w nopStatusWriter) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + return nil +} diff --git a/internal/hierarchyconfig/reconciler.go b/internal/hierarchyconfig/reconciler.go index 0fa219f63..30592a768 100644 --- a/internal/hierarchyconfig/reconciler.go +++ b/internal/hierarchyconfig/reconciler.go @@ -74,8 +74,6 @@ type Reconciler struct { // https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches.html) that is used to // enqueue additional namespaces that need updating. affected chan event.GenericEvent - - ReadOnly bool } type AnchorReconcilerType interface { @@ -703,7 +701,7 @@ func (r *Reconciler) writeInstances(ctx context.Context, log logr.Logger, newHC func (r *Reconciler) writeHierarchy(ctx context.Context, log logr.Logger, inst *api.HierarchyConfiguration, isDeletingNS bool) error { // The inst's name will be blank if it wasn't on the apiserver and we didn't want to make any // changes to it (e.g. no children, conditions, etc). - if r.ReadOnly || inst.GetName() == "" { + if inst.GetName() == "" { return nil } exists := !inst.CreationTimestamp.IsZero() @@ -731,10 +729,6 @@ func (r *Reconciler) writeHierarchy(ctx context.Context, log logr.Logger, inst * } func (r *Reconciler) writeNamespace(ctx context.Context, log logr.Logger, inst *corev1.Namespace) error { - if r.ReadOnly { - return nil - } - // NB: HCR can't create namespaces, that's only in anchor reconciler stats.WriteNamespace() log.V(1).Info("Updating namespace on apiserver") diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index ea593bf18..6354d17d4 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -48,9 +48,6 @@ type Reconciler struct { // that is used to enqueue the singleton to trigger reconciliation. trigger chan event.GenericEvent - // ReadOnly disables writebacks - ReadOnly bool - // RefreshDuration is the maximum amount of time between refreshes RefreshDuration time.Duration @@ -210,10 +207,6 @@ func (r *Reconciler) validateSingleton(inst *api.HNCConfiguration) { // We will write the singleton to apiserver even it is not changed because we assume this // reconciler is called very infrequently and is not performance critical. func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfiguration) error { - if r.ReadOnly { - return nil - } - if inst.CreationTimestamp.IsZero() { // No point creating it if the CRD's being deleted if isDeleted, err := crd.IsDeletingCRD(ctx, api.HNCConfigSingletons); isDeleted || err != nil { diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index 28995062c..68f0be275 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -99,7 +99,7 @@ func HNCBeforeSuite() { By("creating manager") k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ - NewClient: config.NewCachingClient, + NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, }) diff --git a/internal/setup/reconcilers.go b/internal/setup/reconcilers.go index f516faa0b..a60681946 100644 --- a/internal/setup/reconcilers.go +++ b/internal/setup/reconcilers.go @@ -18,7 +18,6 @@ import ( type Options struct { MaxReconciles int - ReadOnly bool UseFakeClient bool NoWebhooks bool HNCCfgRefresh time.Duration @@ -51,10 +50,9 @@ func CreateReconcilers(mgr ctrl.Manager, f *forest.Forest, opts Options) error { // Create Anchor reconciler. ar := &anchor.Reconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("anchor").WithName("reconcile"), - Forest: f, - ReadOnly: opts.ReadOnly, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("anchor").WithName("reconcile"), + Forest: f, } f.AddListener(ar) @@ -64,16 +62,14 @@ func CreateReconcilers(mgr ctrl.Manager, f *forest.Forest, opts Options) error { Log: ctrl.Log.WithName("hncconfig").WithName("reconcile"), Manager: mgr, Forest: f, - ReadOnly: opts.ReadOnly, RefreshDuration: opts.HNCCfgRefresh, } // Create the HC reconciler with a pointer to the Anchor reconciler. hcr := &hierarchyconfig.Reconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("hierarchyconfig").WithName("reconcile"), - Forest: f, - ReadOnly: opts.ReadOnly, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("hierarchyconfig").WithName("reconcile"), + Forest: f, } if opts.HRQ {