From 27e8d133224c2b78347a95e2e8eee52cb153840e Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Thu, 18 May 2023 22:09:04 +0000
Subject: [PATCH 01/22] start with an inplace refactor

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go    |  24 ++--
 pkg/controller/sync/sync_controller.go        | 130 +++---------------
 pkg/syncutil/cmt/cmt.go                       |  37 +++++
 .../sync => syncutil}/opadataclient.go        |   2 +-
 .../sync => syncutil}/stats_reporter.go       |  93 ++++++++++++-
 .../sync => syncutil}/stats_reporter_test.go  |  10 +-
 6 files changed, 167 insertions(+), 129 deletions(-)
 create mode 100644 pkg/syncutil/cmt/cmt.go
 rename pkg/{controller/sync => syncutil}/opadataclient.go (99%)
 rename pkg/{controller/sync => syncutil}/stats_reporter.go (53%)
 rename pkg/{controller/sync => syncutil}/stats_reporter_test.go (95%)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index c83180ae798..9428c41936d 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -29,6 +29,8 @@ import (
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
+	syncutil "github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/target"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/watch"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -109,16 +111,16 @@ func (a *Adder) InjectWatchSet(watchSet *watch.Set) {
 // events is the channel from which sync controller will receive the events
 // regEvents is the channel registered by Registrar to put the events in
 // events and regEvents point to same event channel except for testing.
-func newReconciler(mgr manager.Manager, opa syncc.OpaDataClient, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, processExcluder *process.Excluder, events <-chan event.GenericEvent, watchSet *watch.Set, regEvents chan<- event.GenericEvent) (*ReconcileConfig, error) {
-	filteredOpa := syncc.NewFilteredOpaDataClient(opa, watchSet)
-	syncMetricsCache := syncc.NewMetricsCache()
+func newReconciler(mgr manager.Manager, opa syncutil.OpaDataClient, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, processExcluder *process.Excluder, events <-chan event.GenericEvent, watchSet *watch.Set, regEvents chan<- event.GenericEvent) (*ReconcileConfig, error) {
+	filteredOpa := syncutil.NewFilteredOpaDataClient(opa, watchSet)
+	syncMetricsCache := syncutil.NewMetricsCache()
+	cmt := cmt.NewCacheManager(opa, syncMetricsCache)
 
 	syncAdder := syncc.Adder{
-		Opa:             filteredOpa,
 		Events:          events,
-		MetricsCache:    syncMetricsCache,
 		Tracker:         tracker,
 		ProcessExcluder: processExcluder,
+		CMT: cmt,
 	}
 	// Create subordinate controller - we will feed it events dynamically via watch
 	if err := syncAdder.Add(mgr); err != nil {
@@ -176,8 +178,8 @@ type ReconcileConfig struct {
 	statusClient client.StatusClient
 
 	scheme           *runtime.Scheme
-	opa              syncc.OpaDataClient
-	syncMetricsCache *syncc.MetricsCache
+	opa              syncutil.OpaDataClient
+	syncMetricsCache *syncutil.MetricsCache
 	cs               *watch.ControllerSwitch
 	watcher          *watch.Registrar
 
@@ -327,7 +329,7 @@ func (r *ReconcileConfig) wipeCacheIfNeeded(ctx context.Context) error {
 
 		// reset sync cache before sending the metric
 		r.syncMetricsCache.ResetCache()
-		r.syncMetricsCache.ReportSync(&syncc.Reporter{})
+		r.syncMetricsCache.ReportSync(&syncutil.Reporter{}, log)
 
 		r.needsWipe = false
 	}
@@ -352,7 +354,7 @@ func (r *ReconcileConfig) replayData(ctx context.Context) error {
 			return fmt.Errorf("replaying data for %+v: %w", gvk, err)
 		}
 
-		defer r.syncMetricsCache.ReportSync(&syncc.Reporter{})
+		defer r.syncMetricsCache.ReportSync(&syncutil.Reporter{}, log)
 
 		for i := range u.Items {
 			syncKey := r.syncMetricsCache.GetSyncKey(u.Items[i].GetNamespace(), u.Items[i].GetName())
@@ -367,14 +369,14 @@ func (r *ReconcileConfig) replayData(ctx context.Context) error {
 			}
 
 			if _, err := r.opa.AddData(ctx, &u.Items[i]); err != nil {
-				r.syncMetricsCache.AddObject(syncKey, syncc.Tags{
+				r.syncMetricsCache.AddObject(syncKey, syncutil.Tags{
 					Kind:   u.Items[i].GetKind(),
 					Status: metrics.ErrorStatus,
 				})
 				return fmt.Errorf("adding data for %+v: %w", gvk, err)
 			}
 
-			r.syncMetricsCache.AddObject(syncKey, syncc.Tags{
+			r.syncMetricsCache.AddObject(syncKey, syncutil.Tags{
 				Kind:   u.Items[i].GetKind(),
 				Status: metrics.ActiveStatus,
 			})
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index e060994131f..921ab2958b7 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -17,8 +17,6 @@ package sync
 
 import (
 	"context"
-	"strings"
-	"sync"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -28,6 +26,8 @@ import (
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -44,9 +44,8 @@ import (
 var log = logf.Log.WithName("controller").WithValues("metaKind", "Sync")
 
 type Adder struct {
-	Opa             OpaDataClient
+	CMT *cmt.CacheManagerTracker
 	Events          <-chan event.GenericEvent
-	MetricsCache    *MetricsCache
 	Tracker         *readiness.Tracker
 	ProcessExcluder *process.Excluder
 }
@@ -57,34 +56,32 @@ func (a *Adder) Add(mgr manager.Manager) error {
 	if !operations.HasValidationOperations() {
 		return nil
 	}
-	reporter, err := NewStatsReporter()
+	reporter, err := syncutil.NewStatsReporter()
 	if err != nil {
 		log.Error(err, "Sync metrics reporter could not start")
 		return err
 	}
 
-	r := newReconciler(mgr, a.Opa, *reporter, a.MetricsCache, a.Tracker, a.ProcessExcluder)
+	r := newReconciler(mgr, *reporter, a.Tracker, a.ProcessExcluder, a.CMT)
 	return add(mgr, r, a.Events)
 }
 
 // newReconciler returns a new reconcile.Reconciler.
 func newReconciler(
 	mgr manager.Manager,
-	opa OpaDataClient,
-	reporter Reporter,
-	metricsCache *MetricsCache,
+	reporter syncutil.Reporter,
 	tracker *readiness.Tracker,
 	processExcluder *process.Excluder,
+	cmt *cmt.CacheManagerTracker,
 ) reconcile.Reconciler {
 	return &ReconcileSync{
 		reader:          mgr.GetCache(),
 		scheme:          mgr.GetScheme(),
-		opa:             opa,
 		log:             log,
 		reporter:        reporter,
-		metricsCache:    metricsCache,
 		tracker:         tracker,
 		processExcluder: processExcluder,
+		cmt: cmt,
 	}
 }
 
@@ -108,26 +105,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler, events <-chan event.Generi
 
 var _ reconcile.Reconciler = &ReconcileSync{}
 
-type MetricsCache struct {
-	mux        sync.RWMutex
-	Cache      map[string]Tags
-	KnownKinds map[string]bool
-}
-
-type Tags struct {
-	Kind   string
-	Status metrics.Status
-}
-
 // ReconcileSync reconciles an arbitrary object described by Kind.
 type ReconcileSync struct {
 	reader client.Reader
 
 	scheme          *runtime.Scheme
-	opa             OpaDataClient
 	log             logr.Logger
-	reporter        Reporter
-	metricsCache    *MetricsCache
+	reporter        syncutil.Reporter
+	cmt *cmt.CacheManagerTracker
 	tracker         *readiness.Tracker
 	processExcluder *process.Excluder
 }
@@ -147,17 +132,17 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-	syncKey := r.metricsCache.GetSyncKey(unpackedRequest.Namespace, unpackedRequest.Name)
+	syncKey := r.cmt.SyncMetricsCache.GetSyncKey(unpackedRequest.Namespace, unpackedRequest.Name)
 	reportMetrics := false
 	defer func() {
 		if reportMetrics {
-			if err := r.reporter.reportSyncDuration(time.Since(timeStart)); err != nil {
+			if err := r.reporter.ReportSyncDuration(time.Since(timeStart)); err != nil {
 				log.Error(err, "failed to report sync duration")
 			}
 
-			r.metricsCache.ReportSync(&r.reporter)
+			r.cmt.SyncMetricsCache.ReportSync(&r.reporter, log)
 
-			if err := r.reporter.reportLastSync(); err != nil {
+			if err := r.reporter.ReportLastSync(); err != nil {
 				log.Error(err, "failed to report last sync timestamp")
 			}
 		}
@@ -171,7 +156,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.opa.RemoveData(ctx, instance); err != nil {
+			if _, err := r.cmt.RemoveData(ctx, instance); err != nil {
 				return reconcile.Result{}, err
 			}
 
@@ -179,7 +164,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			t := r.tracker.ForData(instance.GroupVersionKind())
 			t.CancelExpect(instance)
 
-			r.metricsCache.DeleteObject(syncKey)
+			r.cmt.SyncMetricsCache.DeleteObject(syncKey)
 			reportMetrics = true
 			return reconcile.Result{}, nil
 		}
@@ -201,7 +186,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	}
 
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.opa.RemoveData(ctx, instance); err != nil {
+		if _, err := r.cmt.RemoveData(ctx, instance); err != nil {
 			return reconcile.Result{}, err
 		}
 
@@ -209,7 +194,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		t := r.tracker.ForData(instance.GroupVersionKind())
 		t.CancelExpect(instance)
 
-		r.metricsCache.DeleteObject(syncKey)
+		r.cmt.SyncMetricsCache.DeleteObject(syncKey)
 		reportMetrics = true
 		return reconcile.Result{}, nil
 	}
@@ -222,8 +207,8 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.opa.AddData(ctx, instance); err != nil {
-		r.metricsCache.AddObject(syncKey, Tags{
+	if _, err := r.cmt.AddData(ctx, instance); err != nil {
+		r.cmt.SyncMetricsCache.AddObject(syncKey, syncutil.Tags{
 			Kind:   instance.GetKind(),
 			Status: metrics.ErrorStatus,
 		})
@@ -234,12 +219,12 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	r.tracker.ForData(gvk).Observe(instance)
 	log.V(1).Info("[readiness] observed data", "gvk", gvk, "namespace", instance.GetNamespace(), "name", instance.GetName())
 
-	r.metricsCache.AddObject(syncKey, Tags{
+	r.cmt.SyncMetricsCache.AddObject(syncKey, syncutil.Tags{
 		Kind:   instance.GetKind(),
 		Status: metrics.ActiveStatus,
 	})
 
-	r.metricsCache.addKind(instance.GetKind())
+	r.cmt.SyncMetricsCache.AddKind(instance.GetKind())
 
 	reportMetrics = true
 
@@ -254,74 +239,3 @@ func (r *ReconcileSync) skipExcludedNamespace(obj *unstructured.Unstructured) (b
 
 	return isNamespaceExcluded, err
 }
-
-func NewMetricsCache() *MetricsCache {
-	return &MetricsCache{
-		Cache:      make(map[string]Tags),
-		KnownKinds: make(map[string]bool),
-	}
-}
-
-func (c *MetricsCache) GetSyncKey(namespace string, name string) string {
-	return strings.Join([]string{namespace, name}, "/")
-}
-
-// need to know encountered kinds to reset metrics for that kind
-// this is a known memory leak
-// footprint should naturally reset on Pod upgrade b/c the container restarts.
-func (c *MetricsCache) addKind(key string) {
-	c.mux.Lock()
-	defer c.mux.Unlock()
-
-	c.KnownKinds[key] = true
-}
-
-func (c *MetricsCache) ResetCache() {
-	c.mux.Lock()
-	defer c.mux.Unlock()
-
-	c.Cache = make(map[string]Tags)
-}
-
-func (c *MetricsCache) AddObject(key string, t Tags) {
-	c.mux.Lock()
-	defer c.mux.Unlock()
-
-	c.Cache[key] = Tags{
-		Kind:   t.Kind,
-		Status: t.Status,
-	}
-}
-
-func (c *MetricsCache) DeleteObject(key string) {
-	c.mux.Lock()
-	defer c.mux.Unlock()
-
-	delete(c.Cache, key)
-}
-
-func (c *MetricsCache) ReportSync(reporter *Reporter) {
-	c.mux.RLock()
-	defer c.mux.RUnlock()
-
-	totals := make(map[Tags]int)
-	for _, v := range c.Cache {
-		totals[v]++
-	}
-
-	for kind := range c.KnownKinds {
-		for _, status := range metrics.AllStatuses {
-			if err := reporter.reportSync(
-				Tags{
-					Kind:   kind,
-					Status: status,
-				},
-				int64(totals[Tags{
-					Kind:   kind,
-					Status: status,
-				}])); err != nil {
-				log.Error(err, "failed to report sync")
-			}
-		}
-	}
-}
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
new file mode 100644
index 00000000000..e55a12619c4
--- /dev/null
+++ b/pkg/syncutil/cmt/cmt.go
@@ -0,0 +1,37 @@
+package cmt
+
+import (
+	"context"
+	"sync"
+
+	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+)
+
+type CacheManagerTracker struct {
+	lock sync.RWMutex
+
+	opa              syncutil.OpaDataClient
+	SyncMetricsCache *syncutil.MetricsCache
+}
+
+func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache) *CacheManagerTracker {
+	return &CacheManagerTracker{
+		opa:              opa,
+		SyncMetricsCache: syncMetricsCache,
+	}
+}
+
+func (c *CacheManagerTracker) AddData(ctx context.Context, data interface{}) (*types.Responses, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	return c.opa.AddData(ctx, data)
+}
+
+func (c *CacheManagerTracker) RemoveData(ctx context.Context, data interface{}) (*types.Responses, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	return c.opa.RemoveData(ctx, data)
+}
diff --git a/pkg/controller/sync/opadataclient.go b/pkg/syncutil/opadataclient.go
similarity index 99%
rename from pkg/controller/sync/opadataclient.go
rename to pkg/syncutil/opadataclient.go
index ebcc9475c44..63acd1ddfac 100644
--- a/pkg/controller/sync/opadataclient.go
+++ b/pkg/syncutil/opadataclient.go
@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package sync
+package syncutil
 
 import (
 	"context"
diff --git a/pkg/controller/sync/stats_reporter.go b/pkg/syncutil/stats_reporter.go
similarity index 53%
rename from pkg/controller/sync/stats_reporter.go
rename to pkg/syncutil/stats_reporter.go
index aca35f92e94..ec0a396efa1 100644
--- a/pkg/controller/sync/stats_reporter.go
+++ b/pkg/syncutil/stats_reporter.go
@@ -1,9 +1,12 @@
-package sync
+package syncutil
 
 import (
 	"context"
+	"strings"
+	"sync"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
@@ -47,6 +50,88 @@ var (
 	}
 )
 
+type MetricsCache struct {
+	mux        sync.RWMutex // todo FRICTION, gate metrics cache under mux from cmt
+	Cache      map[string]Tags
+	KnownKinds map[string]bool
+}
+
+type Tags struct {
+	Kind   string
+	Status metrics.Status
+}
+
+func NewMetricsCache() *MetricsCache {
+	return &MetricsCache{
+		Cache:      make(map[string]Tags),
+		KnownKinds: make(map[string]bool),
+	}
+}
+
+func (c *MetricsCache) GetSyncKey(namespace string, name string) string {
+	return strings.Join([]string{namespace, name}, "/")
+}
+
+// need to know encountered kinds to reset metrics for that kind
+// this is a known memory leak
+// footprint should naturally reset on Pod upgrade b/c the container restarts.
+func (c *MetricsCache) AddKind(key string) {
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
+	c.KnownKinds[key] = true
+}
+
+func (c *MetricsCache) ResetCache() {
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
+	c.Cache = make(map[string]Tags)
+}
+
+func (c *MetricsCache) AddObject(key string, t Tags) {
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
+	c.Cache[key] = Tags{
+		Kind:   t.Kind,
+		Status: t.Status,
+	}
+}
+
+func (c *MetricsCache) DeleteObject(key string) {
+	c.mux.Lock()
+	defer c.mux.Unlock()
+
+	delete(c.Cache, key)
+}
+
+func (c *MetricsCache) ReportSync(reporter *Reporter, log logr.Logger) {
+	c.mux.RLock()
+	defer c.mux.RUnlock()
+
+	totals := make(map[Tags]int)
+	for _, v := range c.Cache {
+		totals[v]++
+	}
+
+	for kind := range c.KnownKinds {
+		for _, status := range metrics.AllStatuses {
+			if err := reporter.ReportSync(
+				Tags{
+					Kind:   kind,
+					Status: status,
+				},
+				int64(totals[Tags{
+					Kind:   kind,
+					Status: status,
+				}])); err != nil {
+				log.Error(err, "failed to report sync")
+			}
+		}
+	}
+}
+
 func init() {
 	if err := register(); err != nil {
 		panic(err)
@@ -66,17 +151,17 @@ func NewStatsReporter() (*Reporter, error) {
 	return &Reporter{now: now}, nil
 }
 
-func (r *Reporter) reportSyncDuration(d time.Duration) error {
+func (r *Reporter) ReportSyncDuration(d time.Duration) error {
 	ctx := context.Background()
 	return metrics.Record(ctx, syncDurationM.M(d.Seconds()))
 }
 
-func (r *Reporter) reportLastSync() error {
+func (r *Reporter) ReportLastSync() error {
 	ctx := context.Background()
 	return metrics.Record(ctx, lastRunSyncM.M(r.now()))
 }
 
-func (r *Reporter) reportSync(t Tags, v int64) error {
+func (r *Reporter) ReportSync(t Tags, v int64) error {
 	ctx, err := tag.New(
 		context.Background(),
 		tag.Insert(kindKey, t.Kind),
diff --git a/pkg/controller/sync/stats_reporter_test.go b/pkg/syncutil/stats_reporter_test.go
similarity index 95%
rename from pkg/controller/sync/stats_reporter_test.go
rename to pkg/syncutil/stats_reporter_test.go
index cdc091c07cd..828c5856ced 100644
--- a/pkg/controller/sync/stats_reporter_test.go
+++ b/pkg/syncutil/stats_reporter_test.go
@@ -1,4 +1,4 @@
-package sync
+package syncutil
 
 import (
 	"testing"
@@ -22,7 +22,7 @@ func TestReportSync(t *testing.T) {
 		t.Errorf("newStatsReporter() error %v", err)
 	}
 
-	err = r.reportSync(wantTags, wantValue)
+	err = r.ReportSync(wantTags, wantValue)
 	if err != nil {
 		t.Fatalf("got reportSync() error %v", err)
 	}
@@ -62,12 +62,12 @@ func TestReportSyncLatency(t *testing.T) {
 		t.Fatalf("got newStatsReporter() error %v, want nil", err)
 	}
 
-	err = r.reportSyncDuration(minLatency)
+	err = r.ReportSyncDuration(minLatency)
 	if err != nil {
 		t.Fatalf("got reportSyncDuration() error %v, want nil", err)
 	}
 
-	err = r.reportSyncDuration(maxLatency)
+	err = r.ReportSyncDuration(maxLatency)
 	if err != nil {
 		t.Fatalf("got reportSyncDuration error %v, want nil", err)
 	}
@@ -105,7 +105,7 @@ func TestLastRunSync(t *testing.T) {
 	}
 
 	r.now = fakeNow
-	err = r.reportLastSync()
+	err = r.ReportLastSync()
 	if err != nil {
 		t.Fatalf("got reportLastSync() error %v, want nil", err)
 	}

From 4a508369fa0e6d5e90de1be54747ed9283165ae0 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Thu, 18 May 2023 22:13:32 +0000
Subject: [PATCH 02/22] make pure func

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go |  4 ++--
 pkg/controller/sync/sync_controller.go     | 10 +++++-----
 pkg/syncutil/stats_reporter.go             |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index 9428c41936d..04ae4e38d89 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -120,7 +120,7 @@ func newReconciler(mgr manager.Manager, opa syncutil.OpaDataClient, wm *watch.Ma
 		Events:          events,
 		Tracker:         tracker,
 		ProcessExcluder: processExcluder,
-		CMT: cmt,
+		CMT:             cmt,
 	}
 	// Create subordinate controller - we will feed it events dynamically via watch
 	if err := syncAdder.Add(mgr); err != nil {
@@ -357,7 +357,7 @@ func (r *ReconcileConfig) replayData(ctx context.Context) error {
 		defer r.syncMetricsCache.ReportSync(&syncutil.Reporter{}, log)
 
 		for i := range u.Items {
-			syncKey := r.syncMetricsCache.GetSyncKey(u.Items[i].GetNamespace(), u.Items[i].GetName())
+			syncKey := syncutil.GetKeyForSyncMetrics(u.Items[i].GetNamespace(), u.Items[i].GetName())
 
 			isExcludedNamespace, err := r.skipExcludedNamespace(&u.Items[i])
 			if err != nil {
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 921ab2958b7..6b697e898b0 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -25,9 +25,9 @@ import (
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -44,7 +44,7 @@ import (
 var log = logf.Log.WithName("controller").WithValues("metaKind", "Sync")
 
 type Adder struct {
-	CMT *cmt.CacheManagerTracker
+	CMT             *cmt.CacheManagerTracker
 	Events          <-chan event.GenericEvent
 	Tracker         *readiness.Tracker
 	ProcessExcluder *process.Excluder
@@ -81,7 +81,7 @@ func newReconciler(
 		reporter:        reporter,
 		tracker:         tracker,
 		processExcluder: processExcluder,
-		cmt: cmt,
+		cmt:             cmt,
 	}
 }
 
@@ -112,7 +112,7 @@ type ReconcileSync struct {
 	scheme          *runtime.Scheme
 	log             logr.Logger
 	reporter        syncutil.Reporter
-	cmt *cmt.CacheManagerTracker
+	cmt             *cmt.CacheManagerTracker
 	tracker         *readiness.Tracker
 	processExcluder *process.Excluder
 }
@@ -132,7 +132,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-	syncKey := r.cmt.SyncMetricsCache.GetSyncKey(unpackedRequest.Namespace, unpackedRequest.Name)
+	syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
 	reportMetrics := false
 	defer func() {
 		if reportMetrics {
diff --git a/pkg/syncutil/stats_reporter.go b/pkg/syncutil/stats_reporter.go
index ec0a396efa1..91f97b3b6d4 100644
--- a/pkg/syncutil/stats_reporter.go
+++ b/pkg/syncutil/stats_reporter.go
@@ -68,7 +68,7 @@ func NewMetricsCache() *MetricsCache {
 	}
 }
 
-func (c *MetricsCache) GetSyncKey(namespace string, name string) string {
+func GetKeyForSyncMetrics(namespace string, name string) string {
 	return strings.Join([]string{namespace, name}, "/")
 }
 

From a07928e34b1efd5f384ae196c8cbf73deb2bd138 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Sat, 20 May 2023 02:31:21 +0000
Subject: [PATCH 03/22] pull data ops out of sync controller

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 34 ++++++++---------
 pkg/syncutil/cmt/cmt.go                | 53 +++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 6b697e898b0..f7b27b0bb2a 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -133,14 +133,14 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	}
 
 	syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
-	reportMetrics := false
+	reportMetricsForRenconcileRun := false
 	defer func() {
-		if reportMetrics {
+		if reportMetricsForRenconcileRun {
 			if err := r.reporter.ReportSyncDuration(time.Since(timeStart)); err != nil {
 				log.Error(err, "failed to report sync duration")
 			}
 
-			r.cmt.SyncMetricsCache.ReportSync(&r.reporter, log)
+			r.cmt.ReportSyncMetrics(&r.reporter, log)
 
 			if err := r.reporter.ReportLastSync(); err != nil {
 				log.Error(err, "failed to report last sync timestamp")
@@ -156,7 +156,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.cmt.RemoveData(ctx, instance); err != nil {
+			if _, err := r.cmt.RemoveData(ctx, instance, &syncKey); err != nil {
 				return reconcile.Result{}, err
 			}
 
@@ -164,8 +164,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			t := r.tracker.ForData(instance.GroupVersionKind())
 			t.CancelExpect(instance)
 
-			r.cmt.SyncMetricsCache.DeleteObject(syncKey)
-			reportMetrics = true
+			reportMetricsForRenconcileRun = true
 			return reconcile.Result{}, nil
 		}
 		// Error reading the object - requeue the request.
@@ -186,7 +185,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	}
 
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.cmt.RemoveData(ctx, instance); err != nil {
+		if _, err := r.cmt.RemoveData(ctx, instance, &syncKey); err != nil {
 			return reconcile.Result{}, err
 		}
 
@@ -194,8 +193,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		t := r.tracker.ForData(instance.GroupVersionKind())
 		t.CancelExpect(instance)
 
-		r.cmt.SyncMetricsCache.DeleteObject(syncKey)
-		reportMetrics = true
+		reportMetricsForRenconcileRun = true
 		return reconcile.Result{}, nil
 	}
 
@@ -207,26 +205,26 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.cmt.AddData(ctx, instance); err != nil {
-		r.cmt.SyncMetricsCache.AddObject(syncKey, syncutil.Tags{
-			Kind:   instance.GetKind(),
-			Status: metrics.ErrorStatus,
-		})
-		reportMetrics = true
+	if _, err := r.cmt.AddData(ctx, instance, &syncKey); err != nil {
+		reportMetricsForRenconcileRun = true
 
 		return reconcile.Result{}, err
 	}
 	r.tracker.ForData(gvk).Observe(instance)
 	log.V(1).Info("[readiness] observed data", "gvk", gvk, "namespace", instance.GetNamespace(), "name", instance.GetName())
 
-	r.cmt.SyncMetricsCache.AddObject(syncKey, syncutil.Tags{
+	// FRICTION:
+	// it would be great to abstract away these two funcs
+	// but that will require the readiness tracker to be moved to
+	// the CMT thing. Should the
+	r.cmt.AddObjectForSyncMetrics(syncKey, syncutil.Tags{
 		Kind:   instance.GetKind(),
 		Status: metrics.ActiveStatus,
 	})
 
-	r.cmt.SyncMetricsCache.AddKind(instance.GetKind())
+	r.cmt.AddKindForSyncMetrics(instance.GetKind())
 
-	reportMetrics = true
+	reportMetricsForRenconcileRun = true
 
 	return reconcile.Result{}, nil
 }
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
index e55a12619c4..d8041481089 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cmt/cmt.go
@@ -4,34 +4,75 @@ import (
 	"context"
 	"sync"
 
+	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 type CacheManagerTracker struct {
 	lock sync.RWMutex
 
 	opa              syncutil.OpaDataClient
-	SyncMetricsCache *syncutil.MetricsCache
+	syncMetricsCache *syncutil.MetricsCache
 }
 
 func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache) *CacheManagerTracker {
 	return &CacheManagerTracker{
 		opa:              opa,
-		SyncMetricsCache: syncMetricsCache,
+		syncMetricsCache: syncMetricsCache,
 	}
 }
 
-func (c *CacheManagerTracker) AddData(ctx context.Context, data interface{}) (*types.Responses, error) {
+func (c *CacheManagerTracker) AddData(ctx context.Context, instance *unstructured.Unstructured, syncMetricKey *string) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
-	return c.opa.AddData(ctx, data)
+	resp, err := c.opa.AddData(ctx, instance)
+	if err != nil && syncMetricKey != nil {
+		c.AddObjectForSyncMetrics(*syncMetricKey, syncutil.Tags{
+			Kind:   instance.GetKind(),
+			Status: metrics.ErrorStatus,
+		})
+	}
+
+	return resp, err
+}
+
+// todo call this instance not data.
+func (c *CacheManagerTracker) RemoveData(ctx context.Context, instance *unstructured.Unstructured, syncMetricKey *string) (*types.Responses, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	resp, err := c.opa.RemoveData(ctx, instance)
+	// only delete from metrics map if the data removal was succcesful
+	if err != nil && syncMetricKey != nil {
+		c.syncMetricsCache.DeleteObject(*syncMetricKey)
+	}
+
+	return resp, err
+}
+
+func (c *CacheManagerTracker) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
+	c.syncMetricsCache.ReportSync(reporter, log)
+}
+
+// when/ if readyness tracker becomes part of CMT, then we won't need to have this func.
+func (c *CacheManagerTracker) AddObjectForSyncMetrics(syncMetricKey string, tag syncutil.Tags) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	c.syncMetricsCache.AddObject(syncMetricKey, tag)
 }
 
-func (c *CacheManagerTracker) RemoveData(ctx context.Context, data interface{}) (*types.Responses, error) {
+// when/ if readyness tracker becomes part of CMT, then we won't need to have this func.
+func (c *CacheManagerTracker) AddKindForSyncMetrics(syncMetricKind string) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
-	return c.opa.RemoveData(ctx, data)
+	c.syncMetricsCache.AddKind(syncMetricKind)
 }

From 60cd4037d76a46fcd302cfa583f15d15ae70e1ef Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Sat, 20 May 2023 03:11:56 +0000
Subject: [PATCH 04/22] inject tracker in cmt

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 38 ++++++----------
 pkg/syncutil/cmt/cmt.go                | 61 +++++++++++++++-----------
 2 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index f7b27b0bb2a..f8a0188a120 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -22,7 +22,6 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
@@ -74,6 +73,8 @@ func newReconciler(
 	processExcluder *process.Excluder,
 	cmt *cmt.CacheManagerTracker,
 ) reconcile.Reconciler {
+	cmt.WithTracker(tracker)
+
 	return &ReconcileSync{
 		reader:          mgr.GetCache(),
 		scheme:          mgr.GetScheme(),
@@ -132,7 +133,10 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-	syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
+	// todo acpana -- double check that request namespace & name match instance namespace & name
+	// syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
+
+	// todo FRICTION -- reportMetricsForRenconcileRun should not be part of the sync controller w all the new cmt changes
 	reportMetricsForRenconcileRun := false
 	defer func() {
 		if reportMetricsForRenconcileRun {
@@ -156,14 +160,10 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.cmt.RemoveData(ctx, instance, &syncKey); err != nil {
+			if _, err := r.cmt.RemoveGVKFromSync(ctx, instance); err != nil {
 				return reconcile.Result{}, err
 			}
 
-			// cancel expectations
-			t := r.tracker.ForData(instance.GroupVersionKind())
-			t.CancelExpect(instance)
-
 			reportMetricsForRenconcileRun = true
 			return reconcile.Result{}, nil
 		}
@@ -179,20 +179,18 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 
 	if isExcludedNamespace {
 		// cancel expectations
+		// todo FRICTION -- when process exclusion is moved out of sync controller
+		// then we can fully remove the tracker out of the sync controller.
 		t := r.tracker.ForData(instance.GroupVersionKind())
 		t.CancelExpect(instance)
 		return reconcile.Result{}, nil
 	}
 
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.cmt.RemoveData(ctx, instance, &syncKey); err != nil {
+		if _, err := r.cmt.RemoveGVKFromSync(ctx, instance); err != nil {
 			return reconcile.Result{}, err
 		}
 
-		// cancel expectations
-		t := r.tracker.ForData(instance.GroupVersionKind())
-		t.CancelExpect(instance)
-
 		reportMetricsForRenconcileRun = true
 		return reconcile.Result{}, nil
 	}
@@ -205,24 +203,14 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.cmt.AddData(ctx, instance, &syncKey); err != nil {
+	if _, err := r.cmt.AddGVKToSync(ctx, instance); err != nil {
 		reportMetricsForRenconcileRun = true
 
 		return reconcile.Result{}, err
 	}
-	r.tracker.ForData(gvk).Observe(instance)
-	log.V(1).Info("[readiness] observed data", "gvk", gvk, "namespace", instance.GetNamespace(), "name", instance.GetName())
 
-	// FRICTION:
-	// it would be great to abstract away these two funcs
-	// but that will require the readiness tracker to be moved to
-	// the CMT thing. Should the
-	r.cmt.AddObjectForSyncMetrics(syncKey, syncutil.Tags{
-		Kind:   instance.GetKind(),
-		Status: metrics.ActiveStatus,
-	})
-
-	r.cmt.AddKindForSyncMetrics(instance.GetKind())
+	// todo FRICTION -- this log line should move if readiness tracker is not part of sync controlle anymore
+	log.V(1).Info("[readiness] observed data", "gvk", gvk, "namespace", instance.GetNamespace(), "name", instance.GetName())
 
 	reportMetricsForRenconcileRun = true
 
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
index d8041481089..6e1c7a963b6 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cmt/cmt.go
@@ -7,6 +7,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
@@ -16,6 +17,7 @@ type CacheManagerTracker struct {
 
 	opa              syncutil.OpaDataClient
 	syncMetricsCache *syncutil.MetricsCache
+	tracker          *readiness.Tracker
 }
 
 func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache) *CacheManagerTracker {
@@ -25,32 +27,55 @@ func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.Metr
 	}
 }
 
-func (c *CacheManagerTracker) AddData(ctx context.Context, instance *unstructured.Unstructured, syncMetricKey *string) (*types.Responses, error) {
+func (c *CacheManagerTracker) WithTracker(newTracker *readiness.Tracker) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
+	c.tracker = newTracker
+}
+
+func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	syncKey := syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName())
 	resp, err := c.opa.AddData(ctx, instance)
-	if err != nil && syncMetricKey != nil {
-		c.AddObjectForSyncMetrics(*syncMetricKey, syncutil.Tags{
-			Kind:   instance.GetKind(),
-			Status: metrics.ErrorStatus,
-		})
+	if err != nil {
+		c.syncMetricsCache.AddObject(
+			syncKey,
+			syncutil.Tags{
+				Kind:   instance.GetKind(),
+				Status: metrics.ErrorStatus,
+			},
+		)
+
+		return resp, err
 	}
 
+	c.tracker.ForData(instance.GroupVersionKind()).Observe(instance)
+
+	c.syncMetricsCache.AddObject(syncKey, syncutil.Tags{
+		Kind:   instance.GetKind(),
+		Status: metrics.ActiveStatus,
+	})
+	c.syncMetricsCache.AddKind(instance.GetKind())
+
 	return resp, err
 }
 
-// todo call this instance not data.
-func (c *CacheManagerTracker) RemoveData(ctx context.Context, instance *unstructured.Unstructured, syncMetricKey *string) (*types.Responses, error) {
+func (c *CacheManagerTracker) RemoveGVKFromSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
 	resp, err := c.opa.RemoveData(ctx, instance)
 	// only delete from metrics map if the data removal was succcesful
-	if err != nil && syncMetricKey != nil {
-		c.syncMetricsCache.DeleteObject(*syncMetricKey)
+	if err != nil {
+		c.syncMetricsCache.DeleteObject(syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName()))
+
+		return resp, err
 	}
 
+	c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
 	return resp, err
 }
 
@@ -60,19 +85,3 @@ func (c *CacheManagerTracker) ReportSyncMetrics(reporter *syncutil.Reporter, log
 
 	c.syncMetricsCache.ReportSync(reporter, log)
 }
-
-// when/ if readyness tracker becomes part of CMT, then we won't need to have this func.
-func (c *CacheManagerTracker) AddObjectForSyncMetrics(syncMetricKey string, tag syncutil.Tags) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
-	c.syncMetricsCache.AddObject(syncMetricKey, tag)
-}
-
-// when/ if readyness tracker becomes part of CMT, then we won't need to have this func.
-func (c *CacheManagerTracker) AddKindForSyncMetrics(syncMetricKind string) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
-	c.syncMetricsCache.AddKind(syncMetricKind)
-}

From c11c39bf9df27602f175dcb26efdf7d33723c6f8 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Sat, 20 May 2023 03:31:54 +0000
Subject: [PATCH 05/22] inject process excluder in cmt

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 52 +++++++++-----------------
 pkg/syncutil/cmt/cmt.go                | 26 +++++++++++++
 2 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index f8a0188a120..798d8b2c8d0 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -74,15 +74,14 @@ func newReconciler(
 	cmt *cmt.CacheManagerTracker,
 ) reconcile.Reconciler {
 	cmt.WithTracker(tracker)
+	cmt.WithProcessExcluder(processExcluder)
 
 	return &ReconcileSync{
-		reader:          mgr.GetCache(),
-		scheme:          mgr.GetScheme(),
-		log:             log,
-		reporter:        reporter,
-		tracker:         tracker,
-		processExcluder: processExcluder,
-		cmt:             cmt,
+		reader:   mgr.GetCache(),
+		scheme:   mgr.GetScheme(),
+		log:      log,
+		reporter: reporter,
+		cmt:      cmt,
 	}
 }
 
@@ -110,12 +109,10 @@ var _ reconcile.Reconciler = &ReconcileSync{}
 type ReconcileSync struct {
 	reader client.Reader
 
-	scheme          *runtime.Scheme
-	log             logr.Logger
-	reporter        syncutil.Reporter
-	cmt             *cmt.CacheManagerTracker
-	tracker         *readiness.Tracker
-	processExcluder *process.Excluder
+	scheme   *runtime.Scheme
+	log      logr.Logger
+	reporter syncutil.Reporter
+	cmt      *cmt.CacheManagerTracker
 }
 
 // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete
@@ -171,21 +168,15 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, err
 	}
 
+	// todo FRICTION -- logging may be out of place now that process exclusion happens in cmt
 	// namespace is excluded from sync
-	isExcludedNamespace, err := r.skipExcludedNamespace(instance)
-	if err != nil {
-		log.Error(err, "error while excluding namespaces")
-	}
-
-	if isExcludedNamespace {
-		// cancel expectations
-		// todo FRICTION -- when process exclusion is moved out of sync controller
-		// then we can fully remove the tracker out of the sync controller.
-		t := r.tracker.ForData(instance.GroupVersionKind())
-		t.CancelExpect(instance)
-		return reconcile.Result{}, nil
-	}
+	// isExcludedNamespace, err := r.skipExcludedNamespace(instance)
+	// if err != nil {
+	// 	log.Error(err, "error while excluding namespaces")
+	// }
 
+	// todo acpana -- double check that it is okay to remove what has been
+	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
 		if _, err := r.cmt.RemoveGVKFromSync(ctx, instance); err != nil {
 			return reconcile.Result{}, err
@@ -216,12 +207,3 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 
 	return reconcile.Result{}, nil
 }
-
-func (r *ReconcileSync) skipExcludedNamespace(obj *unstructured.Unstructured) (bool, error) {
-	isNamespaceExcluded, err := r.processExcluder.IsNamespaceExcluded(process.Sync, obj)
-	if err != nil {
-		return false, err
-	}
-
-	return isNamespaceExcluded, err
-}
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
index 6e1c7a963b6..268acc3f264 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cmt/cmt.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
@@ -18,6 +19,9 @@ type CacheManagerTracker struct {
 	opa              syncutil.OpaDataClient
 	syncMetricsCache *syncutil.MetricsCache
 	tracker          *readiness.Tracker
+	processExcluder  *process.Excluder
+
+	// todo acpana -- integrate gvkaggregator
 }
 
 func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache) *CacheManagerTracker {
@@ -34,10 +38,32 @@ func (c *CacheManagerTracker) WithTracker(newTracker *readiness.Tracker) {
 	c.tracker = newTracker
 }
 
+func (c *CacheManagerTracker) WithProcessExcluder(newExcluder *process.Excluder) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	c.processExcluder = newExcluder
+}
+
 func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
+	isNamespaceExcluded, err := c.processExcluder.IsNamespaceExcluded(process.Sync, instance)
+	if err != nil { //nolint:staticcheck
+		// todo figure out logging
+	}
+
+	// bail because it means we should not be
+	// syncing this gvk
+	if isNamespaceExcluded {
+		// todo acpana -- consider actually calling RemoveGVKToSync in this case
+		// as we should not be tracking this GVK anymore
+		c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
+
+		return &types.Responses{}, nil
+	}
+
 	syncKey := syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName())
 	resp, err := c.opa.AddData(ctx, instance)
 	if err != nil {

From ba4317d4cdd63f90ced76d1b338f41c451545be4 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Sat, 20 May 2023 04:02:17 +0000
Subject: [PATCH 06/22] pass tracker, excluder from config controller

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go |  8 +++-----
 pkg/controller/sync/sync_controller.go     | 15 +++------------
 pkg/syncutil/cmt/cmt.go                    | 18 +++---------------
 3 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index 04ae4e38d89..90b4caab8f2 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -114,13 +114,11 @@ func (a *Adder) InjectWatchSet(watchSet *watch.Set) {
 func newReconciler(mgr manager.Manager, opa syncutil.OpaDataClient, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, processExcluder *process.Excluder, events <-chan event.GenericEvent, watchSet *watch.Set, regEvents chan<- event.GenericEvent) (*ReconcileConfig, error) {
 	filteredOpa := syncutil.NewFilteredOpaDataClient(opa, watchSet)
 	syncMetricsCache := syncutil.NewMetricsCache()
-	cmt := cmt.NewCacheManager(opa, syncMetricsCache)
+	cmt := cmt.NewCacheManager(opa, syncMetricsCache, tracker, processExcluder)
 
 	syncAdder := syncc.Adder{
-		Events:          events,
-		Tracker:         tracker,
-		ProcessExcluder: processExcluder,
-		CMT:             cmt,
+		Events: events,
+		CMT:    cmt,
 	}
 	// Create subordinate controller - we will feed it events dynamically via watch
 	if err := syncAdder.Add(mgr); err != nil {
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 798d8b2c8d0..decda6a8bec 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -20,10 +20,8 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
@@ -43,10 +41,8 @@ import (
 var log = logf.Log.WithName("controller").WithValues("metaKind", "Sync")
 
 type Adder struct {
-	CMT             *cmt.CacheManagerTracker
-	Events          <-chan event.GenericEvent
-	Tracker         *readiness.Tracker
-	ProcessExcluder *process.Excluder
+	CMT    *cmt.CacheManagerTracker
+	Events <-chan event.GenericEvent
 }
 
 // Add creates a new Sync Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller
@@ -61,7 +57,7 @@ func (a *Adder) Add(mgr manager.Manager) error {
 		return err
 	}
 
-	r := newReconciler(mgr, *reporter, a.Tracker, a.ProcessExcluder, a.CMT)
+	r := newReconciler(mgr, *reporter, a.CMT)
 	return add(mgr, r, a.Events)
 }
 
@@ -69,13 +65,8 @@ func (a *Adder) Add(mgr manager.Manager) error {
 func newReconciler(
 	mgr manager.Manager,
 	reporter syncutil.Reporter,
-	tracker *readiness.Tracker,
-	processExcluder *process.Excluder,
 	cmt *cmt.CacheManagerTracker,
 ) reconcile.Reconciler {
-	cmt.WithTracker(tracker)
-	cmt.WithProcessExcluder(processExcluder)
-
 	return &ReconcileSync{
 		reader:   mgr.GetCache(),
 		scheme:   mgr.GetScheme(),
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
index 268acc3f264..eca53561f28 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cmt/cmt.go
@@ -24,27 +24,15 @@ type CacheManagerTracker struct {
 	// todo acpana -- integrate gvkaggregator
 }
 
-func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache) *CacheManagerTracker {
+func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache, tracker *readiness.Tracker, processExcluder *process.Excluder) *CacheManagerTracker {
 	return &CacheManagerTracker{
 		opa:              opa,
 		syncMetricsCache: syncMetricsCache,
+		tracker:          tracker,
+		processExcluder:  processExcluder,
 	}
 }
 
-func (c *CacheManagerTracker) WithTracker(newTracker *readiness.Tracker) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
-	c.tracker = newTracker
-}
-
-func (c *CacheManagerTracker) WithProcessExcluder(newExcluder *process.Excluder) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
-	c.processExcluder = newExcluder
-}
-
 func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()

From dc78ba63a245520645f9c1767894d9cc828d2c69 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Mon, 22 May 2023 17:35:08 +0000
Subject: [PATCH 07/22] add glog to cmt

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 7 -------
 pkg/syncutil/cmt/cmt.go                | 9 +++++++--
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index decda6a8bec..e70164b3319 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -159,13 +159,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, err
 	}
 
-	// todo FRICTION -- logging may be out of place now that process exclusion happens in cmt
-	// namespace is excluded from sync
-	// isExcludedNamespace, err := r.skipExcludedNamespace(instance)
-	// if err != nil {
-	// 	log.Error(err, "error while excluding namespaces")
-	// }
-
 	// todo acpana -- double check that it is okay to remove what has been
 	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cmt/cmt.go
index eca53561f28..1c05c278fa3 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cmt/cmt.go
@@ -7,12 +7,16 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+var log = logf.Log.WithName("data-replication").WithValues("metaKind", "CacheManagerTracker")
+
 type CacheManagerTracker struct {
 	lock sync.RWMutex
 
@@ -38,8 +42,8 @@ func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstru
 	defer c.lock.Unlock()
 
 	isNamespaceExcluded, err := c.processExcluder.IsNamespaceExcluded(process.Sync, instance)
-	if err != nil { //nolint:staticcheck
-		// todo figure out logging
+	if err != nil {
+		log.Error(err, "error while excluding namespaces")
 	}
 
 	// bail because it means we should not be
@@ -74,6 +78,7 @@ func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstru
 	})
 	c.syncMetricsCache.AddKind(instance.GetKind())
 
+	log.V(logging.DebugLevel).Info("[readiness] observed data", "gvk", instance.GroupVersionKind(), "namespace", instance.GetNamespace(), "name", instance.GetName())
 	return resp, err
 }
 

From 768c2bfe74877d90138aafb245c8f8b16d1c352e Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Mon, 22 May 2023 17:56:36 +0000
Subject: [PATCH 08/22] polish

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 4 ----
 pkg/syncutil/stats_reporter.go         | 2 +-
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index e70164b3319..1e42bacc24c 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -124,7 +124,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	// todo acpana -- double check that request namespace & name match instance namespace & name
 	// syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
 
-	// todo FRICTION -- reportMetricsForRenconcileRun should not be part of the sync controller w all the new cmt changes
 	reportMetricsForRenconcileRun := false
 	defer func() {
 		if reportMetricsForRenconcileRun {
@@ -184,9 +183,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, err
 	}
 
-	// todo FRICTION -- this log line should move if readiness tracker is not part of sync controlle anymore
-	log.V(1).Info("[readiness] observed data", "gvk", gvk, "namespace", instance.GetNamespace(), "name", instance.GetName())
-
 	reportMetricsForRenconcileRun = true
 
 	return reconcile.Result{}, nil
diff --git a/pkg/syncutil/stats_reporter.go b/pkg/syncutil/stats_reporter.go
index 91f97b3b6d4..2288ae958c5 100644
--- a/pkg/syncutil/stats_reporter.go
+++ b/pkg/syncutil/stats_reporter.go
@@ -51,7 +51,7 @@ var (
 )
 
 type MetricsCache struct {
-	mux        sync.RWMutex // todo FRICTION, gate metrics cache under mux from cmt
+	mux        sync.RWMutex
 	Cache      map[string]Tags
 	KnownKinds map[string]bool
 }

From afab5d108aa4f9600559ddf624877f1fe828f9d9 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Wed, 24 May 2023 17:58:28 +0000
Subject: [PATCH 09/22] rename cmt to cachemanager

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go    |  8 +++----
 pkg/controller/sync/sync_controller.go        | 22 +++++++++----------
 .../cmt.go => cachemanager/cachemanager.go}   | 14 ++++++------
 3 files changed, 22 insertions(+), 22 deletions(-)
 rename pkg/syncutil/{cmt/cmt.go => cachemanager/cachemanager.go} (85%)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index 90b4caab8f2..5594551cec1 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -30,7 +30,7 @@ import (
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	syncutil "github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
+	cm "github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cachemanager"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/target"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/watch"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -114,11 +114,11 @@ func (a *Adder) InjectWatchSet(watchSet *watch.Set) {
 func newReconciler(mgr manager.Manager, opa syncutil.OpaDataClient, wm *watch.Manager, cs *watch.ControllerSwitch, tracker *readiness.Tracker, processExcluder *process.Excluder, events <-chan event.GenericEvent, watchSet *watch.Set, regEvents chan<- event.GenericEvent) (*ReconcileConfig, error) {
 	filteredOpa := syncutil.NewFilteredOpaDataClient(opa, watchSet)
 	syncMetricsCache := syncutil.NewMetricsCache()
-	cmt := cmt.NewCacheManager(opa, syncMetricsCache, tracker, processExcluder)
+	cm := cm.NewCacheManager(opa, syncMetricsCache, tracker, processExcluder)
 
 	syncAdder := syncc.Adder{
-		Events: events,
-		CMT:    cmt,
+		Events:       events,
+		CacheManager: cm,
 	}
 	// Create subordinate controller - we will feed it events dynamically via watch
 	if err := syncAdder.Add(mgr); err != nil {
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 1e42bacc24c..4a679e0492d 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -23,7 +23,7 @@ import (
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/operations"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cmt"
+	cm "github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil/cachemanager"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -41,8 +41,8 @@ import (
 var log = logf.Log.WithName("controller").WithValues("metaKind", "Sync")
 
 type Adder struct {
-	CMT    *cmt.CacheManagerTracker
-	Events <-chan event.GenericEvent
+	CacheManager *cm.CacheManager
+	Events       <-chan event.GenericEvent
 }
 
 // Add creates a new Sync Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller
@@ -57,7 +57,7 @@ func (a *Adder) Add(mgr manager.Manager) error {
 		return err
 	}
 
-	r := newReconciler(mgr, *reporter, a.CMT)
+	r := newReconciler(mgr, *reporter, a.CacheManager)
 	return add(mgr, r, a.Events)
 }
 
@@ -65,14 +65,14 @@ func (a *Adder) Add(mgr manager.Manager) error {
 func newReconciler(
 	mgr manager.Manager,
 	reporter syncutil.Reporter,
-	cmt *cmt.CacheManagerTracker,
+	cmt *cm.CacheManager,
 ) reconcile.Reconciler {
 	return &ReconcileSync{
 		reader:   mgr.GetCache(),
 		scheme:   mgr.GetScheme(),
 		log:      log,
 		reporter: reporter,
-		cmt:      cmt,
+		cm:      cmt,
 	}
 }
 
@@ -103,7 +103,7 @@ type ReconcileSync struct {
 	scheme   *runtime.Scheme
 	log      logr.Logger
 	reporter syncutil.Reporter
-	cmt      *cmt.CacheManagerTracker
+	cm      *cm.CacheManager
 }
 
 // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete
@@ -131,7 +131,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 				log.Error(err, "failed to report sync duration")
 			}
 
-			r.cmt.ReportSyncMetrics(&r.reporter, log)
+			r.cm.ReportSyncMetrics(&r.reporter, log)
 
 			if err := r.reporter.ReportLastSync(); err != nil {
 				log.Error(err, "failed to report last sync timestamp")
@@ -147,7 +147,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.cmt.RemoveGVKFromSync(ctx, instance); err != nil {
+			if _, err := r.cm.RemoveGVKFromSync(ctx, instance); err != nil {
 				return reconcile.Result{}, err
 			}
 
@@ -161,7 +161,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	// todo acpana -- double check that it is okay to remove what has been
 	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.cmt.RemoveGVKFromSync(ctx, instance); err != nil {
+		if _, err := r.cm.RemoveGVKFromSync(ctx, instance); err != nil {
 			return reconcile.Result{}, err
 		}
 
@@ -177,7 +177,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.cmt.AddGVKToSync(ctx, instance); err != nil {
+	if _, err := r.cm.AddGVKToSync(ctx, instance); err != nil {
 		reportMetricsForRenconcileRun = true
 
 		return reconcile.Result{}, err
diff --git a/pkg/syncutil/cmt/cmt.go b/pkg/syncutil/cachemanager/cachemanager.go
similarity index 85%
rename from pkg/syncutil/cmt/cmt.go
rename to pkg/syncutil/cachemanager/cachemanager.go
index 1c05c278fa3..64fab60e832 100644
--- a/pkg/syncutil/cmt/cmt.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -1,4 +1,4 @@
-package cmt
+package cachemanager
 
 import (
 	"context"
@@ -17,7 +17,7 @@ import (
 
 var log = logf.Log.WithName("data-replication").WithValues("metaKind", "CacheManagerTracker")
 
-type CacheManagerTracker struct {
+type CacheManager struct {
 	lock sync.RWMutex
 
 	opa              syncutil.OpaDataClient
@@ -28,8 +28,8 @@ type CacheManagerTracker struct {
 	// todo acpana -- integrate gvkaggregator
 }
 
-func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache, tracker *readiness.Tracker, processExcluder *process.Excluder) *CacheManagerTracker {
-	return &CacheManagerTracker{
+func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache, tracker *readiness.Tracker, processExcluder *process.Excluder) *CacheManager {
+	return &CacheManager{
 		opa:              opa,
 		syncMetricsCache: syncMetricsCache,
 		tracker:          tracker,
@@ -37,7 +37,7 @@ func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.Metr
 	}
 }
 
-func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
+func (c *CacheManager) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -82,7 +82,7 @@ func (c *CacheManagerTracker) AddGVKToSync(ctx context.Context, instance *unstru
 	return resp, err
 }
 
-func (c *CacheManagerTracker) RemoveGVKFromSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
+func (c *CacheManager) RemoveGVKFromSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -98,7 +98,7 @@ func (c *CacheManagerTracker) RemoveGVKFromSync(ctx context.Context, instance *u
 	return resp, err
 }
 
-func (c *CacheManagerTracker) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
+func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
 	c.lock.RLock()
 	defer c.lock.RUnlock()
 

From ffd8d01dab2cdb05a518ec6940cb9139c99f6102 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Wed, 24 May 2023 18:09:39 +0000
Subject: [PATCH 10/22] rename funcs, no locks for now now

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go    | 10 +++++-----
 pkg/syncutil/cachemanager/cachemanager.go | 16 ++--------------
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 4a679e0492d..91c49caea5f 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -72,7 +72,7 @@ func newReconciler(
 		scheme:   mgr.GetScheme(),
 		log:      log,
 		reporter: reporter,
-		cm:      cmt,
+		cm:       cmt,
 	}
 }
 
@@ -103,7 +103,7 @@ type ReconcileSync struct {
 	scheme   *runtime.Scheme
 	log      logr.Logger
 	reporter syncutil.Reporter
-	cm      *cm.CacheManager
+	cm       *cm.CacheManager
 }
 
 // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete
@@ -147,7 +147,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.cm.RemoveGVKFromSync(ctx, instance); err != nil {
+			if _, err := r.cm.RemoveObject(ctx, instance); err != nil {
 				return reconcile.Result{}, err
 			}
 
@@ -161,7 +161,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	// todo acpana -- double check that it is okay to remove what has been
 	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.cm.RemoveGVKFromSync(ctx, instance); err != nil {
+		if _, err := r.cm.RemoveObject(ctx, instance); err != nil {
 			return reconcile.Result{}, err
 		}
 
@@ -177,7 +177,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.cm.AddGVKToSync(ctx, instance); err != nil {
+	if _, err := r.cm.AddObject(ctx, instance); err != nil {
 		reportMetricsForRenconcileRun = true
 
 		return reconcile.Result{}, err
diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 64fab60e832..719d464563b 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -2,7 +2,6 @@ package cachemanager
 
 import (
 	"context"
-	"sync"
 
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
@@ -18,8 +17,6 @@ import (
 var log = logf.Log.WithName("data-replication").WithValues("metaKind", "CacheManagerTracker")
 
 type CacheManager struct {
-	lock sync.RWMutex
-
 	opa              syncutil.OpaDataClient
 	syncMetricsCache *syncutil.MetricsCache
 	tracker          *readiness.Tracker
@@ -37,10 +34,7 @@ func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.Metr
 	}
 }
 
-func (c *CacheManager) AddGVKToSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
+func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	isNamespaceExcluded, err := c.processExcluder.IsNamespaceExcluded(process.Sync, instance)
 	if err != nil {
 		log.Error(err, "error while excluding namespaces")
@@ -82,10 +76,7 @@ func (c *CacheManager) AddGVKToSync(ctx context.Context, instance *unstructured.
 	return resp, err
 }
 
-func (c *CacheManager) RemoveGVKFromSync(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-
+func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	resp, err := c.opa.RemoveData(ctx, instance)
 	// only delete from metrics map if the data removal was succcesful
 	if err != nil {
@@ -99,8 +90,5 @@ func (c *CacheManager) RemoveGVKFromSync(ctx context.Context, instance *unstruct
 }
 
 func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
-	c.lock.RLock()
-	defer c.lock.RUnlock()
-
 	c.syncMetricsCache.ReportSync(reporter, log)
 }

From a7e477a134546060e1b4777f5acd89d2c88da612 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Wed, 24 May 2023 18:12:28 +0000
Subject: [PATCH 11/22] return err on process exclusion err

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/syncutil/cachemanager/cachemanager.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 719d464563b..69ad7d9fde2 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -2,6 +2,7 @@ package cachemanager
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
@@ -37,7 +38,7 @@ func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.Metr
 func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
 	isNamespaceExcluded, err := c.processExcluder.IsNamespaceExcluded(process.Sync, instance)
 	if err != nil {
-		log.Error(err, "error while excluding namespaces")
+		return nil, fmt.Errorf("error while excluding namespaces: %w", err)
 	}
 
 	// bail because it means we should not be

From b8e7bd3736f25e9e149a548601318fe8e8af4f74 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Wed, 24 May 2023 18:15:04 +0000
Subject: [PATCH 12/22] just return err

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go    |  6 +++---
 pkg/syncutil/cachemanager/cachemanager.go | 21 ++++++++++-----------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 91c49caea5f..ca3c4ec630c 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -147,7 +147,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			// This is a deletion; remove the data
 			instance.SetNamespace(unpackedRequest.Namespace)
 			instance.SetName(unpackedRequest.Name)
-			if _, err := r.cm.RemoveObject(ctx, instance); err != nil {
+			if err := r.cm.RemoveObject(ctx, instance); err != nil {
 				return reconcile.Result{}, err
 			}
 
@@ -161,7 +161,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	// todo acpana -- double check that it is okay to remove what has been
 	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
-		if _, err := r.cm.RemoveObject(ctx, instance); err != nil {
+		if err := r.cm.RemoveObject(ctx, instance); err != nil {
 			return reconcile.Result{}, err
 		}
 
@@ -177,7 +177,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		logging.ResourceName, instance.GetName(),
 	)
 
-	if _, err := r.cm.AddObject(ctx, instance); err != nil {
+	if err := r.cm.AddObject(ctx, instance); err != nil {
 		reportMetricsForRenconcileRun = true
 
 		return reconcile.Result{}, err
diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 69ad7d9fde2..696f3559399 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	"github.com/go-logr/logr"
-	"github.com/open-policy-agent/frameworks/constraint/pkg/types"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
@@ -35,10 +34,10 @@ func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.Metr
 	}
 }
 
-func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
+func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Unstructured) error {
 	isNamespaceExcluded, err := c.processExcluder.IsNamespaceExcluded(process.Sync, instance)
 	if err != nil {
-		return nil, fmt.Errorf("error while excluding namespaces: %w", err)
+		return fmt.Errorf("error while excluding namespaces: %w", err)
 	}
 
 	// bail because it means we should not be
@@ -48,11 +47,11 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 		// as we should not be tracking this GVK anymore
 		c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
 
-		return &types.Responses{}, nil
+		return nil
 	}
 
 	syncKey := syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName())
-	resp, err := c.opa.AddData(ctx, instance)
+	_, err = c.opa.AddData(ctx, instance)
 	if err != nil {
 		c.syncMetricsCache.AddObject(
 			syncKey,
@@ -62,7 +61,7 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 			},
 		)
 
-		return resp, err
+		return err
 	}
 
 	c.tracker.ForData(instance.GroupVersionKind()).Observe(instance)
@@ -74,20 +73,20 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 	c.syncMetricsCache.AddKind(instance.GetKind())
 
 	log.V(logging.DebugLevel).Info("[readiness] observed data", "gvk", instance.GroupVersionKind(), "namespace", instance.GetNamespace(), "name", instance.GetName())
-	return resp, err
+	return err
 }
 
-func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.Unstructured) (*types.Responses, error) {
-	resp, err := c.opa.RemoveData(ctx, instance)
+func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.Unstructured) error {
+	_, err := c.opa.RemoveData(ctx, instance)
 	// only delete from metrics map if the data removal was succcesful
 	if err != nil {
 		c.syncMetricsCache.DeleteObject(syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName()))
 
-		return resp, err
+		return err
 	}
 
 	c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
-	return resp, err
+	return err
 }
 
 func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {

From b04a59fe3221050d91acb92da9d1a7b3badba7d3 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Mon, 5 Jun 2023 23:50:53 +0000
Subject: [PATCH 13/22] refactor: export FakeOpa

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 .../config/config_controller_test.go          |  27 ++--
 pkg/controller/config/fakes_test.go           | 100 --------------
 pkg/fakes/opadataclient.go                    | 123 ++++++++++++++++++
 3 files changed, 137 insertions(+), 113 deletions(-)
 create mode 100644 pkg/fakes/opadataclient.go

diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go
index 500b5c5799c..33f99d1cb14 100644
--- a/pkg/controller/config/config_controller_test.go
+++ b/pkg/controller/config/config_controller_test.go
@@ -142,6 +142,7 @@ func TestReconcile(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	// todo acpana this is how you can set up a process excluder
 	processExcluder := process.Get()
 	processExcluder.Add(instance.Spec.Match)
 	events := make(chan event.GenericEvent, 1024)
@@ -429,7 +430,7 @@ func TestConfig_CacheContents(t *testing.T) {
 	mgr, wm := setupManager(t)
 	c := testclient.NewRetryClient(mgr.GetClient())
 
-	opaClient := &fakeOpa{}
+	opaClient := &fakes.FakeOpa{}
 	cs := watch.NewSwitch()
 	tracker, err := readiness.SetupTracker(mgr, false, false, false)
 	if err != nil {
@@ -503,9 +504,9 @@ func TestConfig_CacheContents(t *testing.T) {
 		}
 	}()
 
-	expected := map[opaKey]interface{}{
-		{gvk: nsGVK, key: "default"}:                      nil,
-		{gvk: configMapGVK, key: "default/config-test-1"}: nil,
+	expected := map[fakes.OpaKey]interface{}{
+		{Gvk: nsGVK, Key: "default"}:                      nil,
+		{Gvk: configMapGVK, Key: "default/config-test-1"}: nil,
 		// kube-system namespace is being excluded, it should not be in opa cache
 	}
 	g.Eventually(func() bool {
@@ -535,20 +536,20 @@ func TestConfig_CacheContents(t *testing.T) {
 
 	// Expect our configMap to return at some point
 	// TODO: In the future it will remain instead of having to repopulate.
-	expected = map[opaKey]interface{}{
+	expected = map[fakes.OpaKey]interface{}{
 		{
-			gvk: configMapGVK,
-			key: "default/config-test-1",
+			Gvk: configMapGVK,
+			Key: "default/config-test-1",
 		}: nil,
 	}
 	g.Eventually(func() bool {
 		return opaClient.Contains(expected)
 	}, 10*time.Second).Should(gomega.BeTrue(), "waiting for ConfigMap to repopulate in cache")
 
-	expected = map[opaKey]interface{}{
+	expected = map[fakes.OpaKey]interface{}{
 		{
-			gvk: configMapGVK,
-			key: "kube-system/config-test-2",
+			Gvk: configMapGVK,
+			Key: "kube-system/config-test-2",
 		}: nil,
 	}
 	g.Eventually(func() bool {
@@ -590,7 +591,7 @@ func TestConfig_Retries(t *testing.T) {
 	mgr, wm := setupManager(t)
 	c := testclient.NewRetryClient(mgr.GetClient())
 
-	opaClient := &fakeOpa{}
+	opaClient := &fakes.FakeOpa{}
 	cs := watch.NewSwitch()
 	tracker, err := readiness.SetupTracker(mgr, false, false, false)
 	if err != nil {
@@ -665,8 +666,8 @@ func TestConfig_Retries(t *testing.T) {
 		}
 	}()
 
-	expected := map[opaKey]interface{}{
-		{gvk: configMapGVK, key: "default/config-test-1"}: nil,
+	expected := map[fakes.OpaKey]interface{}{
+		{Gvk: configMapGVK, Key: "default/config-test-1"}: nil,
 	}
 	g.Eventually(func() bool {
 		return opaClient.Contains(expected)
diff --git a/pkg/controller/config/fakes_test.go b/pkg/controller/config/fakes_test.go
index 8a9925fcaca..3c209d4e3d4 100644
--- a/pkg/controller/config/fakes_test.go
+++ b/pkg/controller/config/fakes_test.go
@@ -17,110 +17,10 @@ package config
 
 import (
 	"context"
-	"fmt"
-	gosync "sync"
 
-	constraintTypes "github.com/open-policy-agent/frameworks/constraint/pkg/types"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/target"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-type opaKey struct {
-	gvk schema.GroupVersionKind
-	key string
-}
-
-// fakeOpa is an OpaDataClient for testing.
-type fakeOpa struct {
-	mu   gosync.Mutex
-	data map[opaKey]interface{}
-}
-
-// keyFor returns an opaKey for the provided resource.
-// Returns error if the resource is not a runtime.Object w/ metadata.
-func (f *fakeOpa) keyFor(obj interface{}) (opaKey, error) {
-	o, ok := obj.(client.Object)
-	if !ok {
-		return opaKey{}, fmt.Errorf("expected runtime.Object, got: %T", obj)
-	}
-	gvk := o.GetObjectKind().GroupVersionKind()
-	ns := o.GetNamespace()
-	if ns == "" {
-		return opaKey{gvk: gvk, key: o.GetName()}, nil
-	}
-
-	return opaKey{gvk: gvk, key: fmt.Sprintf("%s/%s", ns, o.GetName())}, nil
-}
-
-func (f *fakeOpa) AddData(ctx context.Context, data interface{}) (*constraintTypes.Responses, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	key, err := f.keyFor(data)
-	if err != nil {
-		return nil, err
-	}
-
-	if f.data == nil {
-		f.data = make(map[opaKey]interface{})
-	}
-
-	f.data[key] = data
-	return &constraintTypes.Responses{}, nil
-}
-
-func (f *fakeOpa) RemoveData(ctx context.Context, data interface{}) (*constraintTypes.Responses, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	if target.IsWipeData(data) {
-		f.data = make(map[opaKey]interface{})
-		return &constraintTypes.Responses{}, nil
-	}
-
-	key, err := f.keyFor(data)
-	if err != nil {
-		return nil, err
-	}
-
-	delete(f.data, key)
-	return &constraintTypes.Responses{}, nil
-}
-
-// Contains returns true if all expected resources are in the cache.
-func (f *fakeOpa) Contains(expected map[opaKey]interface{}) bool {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	for k := range expected {
-		if _, ok := f.data[k]; !ok {
-			return false
-		}
-	}
-	return true
-}
-
-// HasGVK returns true if the cache has any data of the requested kind.
-func (f *fakeOpa) HasGVK(gvk schema.GroupVersionKind) bool {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	for k := range f.data {
-		if k.gvk == gvk {
-			return true
-		}
-	}
-	return false
-}
-
-// Len returns the number of items in the cache.
-func (f *fakeOpa) Len() int {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	return len(f.data)
-}
-
 // hookReader is a client.Reader with overrideable methods.
 type hookReader struct {
 	client.Reader
diff --git a/pkg/fakes/opadataclient.go b/pkg/fakes/opadataclient.go
new file mode 100644
index 00000000000..74291757813
--- /dev/null
+++ b/pkg/fakes/opadataclient.go
@@ -0,0 +1,123 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package fakes
+
+import (
+	"context"
+	"fmt"
+	gosync "sync"
+
+	constraintTypes "github.com/open-policy-agent/frameworks/constraint/pkg/types"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/target"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type OpaKey struct {
+	Gvk schema.GroupVersionKind
+	Key string
+}
+
+// FakeOpa is an OpaDataClient for testing.
+type FakeOpa struct {
+	mu   gosync.Mutex
+	data map[OpaKey]interface{}
+}
+
+var _ syncutil.OpaDataClient = &FakeOpa{}
+
+// keyFor returns an opaKey for the provided resource.
+// Returns error if the resource is not a runtime.Object w/ metadata.
+func (f *FakeOpa) keyFor(obj interface{}) (OpaKey, error) {
+	o, ok := obj.(client.Object)
+	if !ok {
+		return OpaKey{}, fmt.Errorf("expected runtime.Object, got: %T", obj)
+	}
+	gvk := o.GetObjectKind().GroupVersionKind()
+	ns := o.GetNamespace()
+	if ns == "" {
+		return OpaKey{Gvk: gvk, Key: o.GetName()}, nil
+	}
+
+	return OpaKey{Gvk: gvk, Key: fmt.Sprintf("%s/%s", ns, o.GetName())}, nil
+}
+
+func (f *FakeOpa) AddData(ctx context.Context, data interface{}) (*constraintTypes.Responses, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	key, err := f.keyFor(data)
+	if err != nil {
+		return nil, err
+	}
+
+	if f.data == nil {
+		f.data = make(map[OpaKey]interface{})
+	}
+
+	f.data[key] = data
+	return &constraintTypes.Responses{}, nil
+}
+
+func (f *FakeOpa) RemoveData(ctx context.Context, data interface{}) (*constraintTypes.Responses, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	if target.IsWipeData(data) {
+		f.data = make(map[OpaKey]interface{})
+		return &constraintTypes.Responses{}, nil
+	}
+
+	key, err := f.keyFor(data)
+	if err != nil {
+		return nil, err
+	}
+
+	delete(f.data, key)
+	return &constraintTypes.Responses{}, nil
+}
+
+// Contains returns true if all expected resources are in the cache.
+func (f *FakeOpa) Contains(expected map[OpaKey]interface{}) bool {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	for k := range expected {
+		if _, ok := f.data[k]; !ok {
+			return false
+		}
+	}
+	return true
+}
+
+// HasGVK returns true if the cache has any data of the requested kind.
+func (f *FakeOpa) HasGVK(gvk schema.GroupVersionKind) bool {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	for k := range f.data {
+		if k.Gvk == gvk {
+			return true
+		}
+	}
+	return false
+}
+
+// Len returns the number of items in the cache.
+func (f *FakeOpa) Len() int {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return len(f.data)
+}

From f056ec55d8aee7513a8d03968c33c5d6f3aa2807 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Tue, 6 Jun 2023 19:43:04 +0000
Subject: [PATCH 14/22] add cm tests

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/syncutil/cachemanager/cachemanager.go     |   4 +-
 .../cachemanager/cachemanager_test.go         | 113 ++++++++++++++++++
 2 files changed, 114 insertions(+), 3 deletions(-)
 create mode 100644 pkg/syncutil/cachemanager/cachemanager_test.go

diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 696f3559399..a91501e69b6 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -43,11 +43,9 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 	// bail because it means we should not be
 	// syncing this gvk
 	if isNamespaceExcluded {
-		// todo acpana -- consider actually calling RemoveGVKToSync in this case
-		// as we should not be tracking this GVK anymore
 		c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
 
-		return nil
+		return c.RemoveObject(ctx, instance)
 	}
 
 	syncKey := syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName())
diff --git a/pkg/syncutil/cachemanager/cachemanager_test.go b/pkg/syncutil/cachemanager/cachemanager_test.go
new file mode 100644
index 00000000000..90a1675a2bf
--- /dev/null
+++ b/pkg/syncutil/cachemanager/cachemanager_test.go
@@ -0,0 +1,113 @@
+package cachemanager
+
+import (
+	"context"
+	"testing"
+
+	configv1alpha1 "github.com/open-policy-agent/gatekeeper/v3/apis/config/v1alpha1"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/fakes"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/util"
+	"github.com/open-policy-agent/gatekeeper/v3/test/testutils"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/rest"
+)
+
+var cfg *rest.Config
+
+func TestMain(m *testing.M) {
+	testutils.StartControlPlane(m, &cfg, 3)
+}
+
+// TestCacheManager_AddObject_RemoveObject tests that we can add/ remove objects in the cache.
+func TestCacheManager_AddObject_RemoveObject(t *testing.T) {
+	mgr, _ := testutils.SetupManager(t, cfg)
+	opaClient := &fakes.FakeOpa{}
+
+	tracker, err := readiness.SetupTracker(mgr, false, false, false)
+	assert.NoError(t, err)
+
+	processExcluder := process.Get()
+	cm := NewCacheManager(opaClient, syncutil.NewMetricsCache(), tracker, processExcluder)
+	ctx := context.Background()
+
+	pod := fakes.Pod(
+		fakes.WithNamespace("test-ns"),
+		fakes.WithName("test-name"),
+	)
+	unstructuredPod, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod)
+	require.NoError(t, err)
+
+	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod}))
+
+	// test that pod is cache managed
+	require.True(t, opaClient.HasGVK(pod.GroupVersionKind()))
+
+	// now remove the object and verify it's removed
+	require.NoError(t, cm.RemoveObject(ctx, &unstructured.Unstructured{Object: unstructuredPod}))
+	require.False(t, opaClient.HasGVK(pod.GroupVersionKind()))
+}
+
+// TestCacheManager_processExclusion makes sure that we don't add objects that are process excluded
+// and remove any objects that were previously not process excluded but have become so now.
+func TestCacheManager_processExclusion(t *testing.T) {
+	mgr, _ := testutils.SetupManager(t, cfg)
+	opaClient := &fakes.FakeOpa{}
+
+	tracker, err := readiness.SetupTracker(mgr, false, false, false)
+	assert.NoError(t, err)
+
+	// exclude "test-ns-excluded" namespace as excluded
+	processExcluder := process.Get()
+	processExcluder.Add([]configv1alpha1.MatchEntry{
+		{
+			ExcludedNamespaces: []util.Wildcard{"test-ns-excluded"},
+			Processes:          []string{"sync"},
+		},
+	})
+
+	cm := NewCacheManager(opaClient, syncutil.NewMetricsCache(), tracker, processExcluder)
+	ctx := context.Background()
+
+	pod := fakes.Pod(
+		fakes.WithNamespace("test-ns-excluded"),
+		fakes.WithName("test-name"),
+	)
+	unstructuredPod, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod)
+	require.NoError(t, err)
+	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod}))
+
+	// test that pod from excluded namespace is not cache managed
+	require.False(t, opaClient.HasGVK(pod.GroupVersionKind()))
+
+	// now add a new pod that IS NOT name space excluded at the time
+	// of addition but becomes exlcluded later.
+	pod2 := fakes.Pod(
+		fakes.WithNamespace("test-ns-excluded-2"),
+		fakes.WithName("test-name-2"),
+	)
+	unstructuredPod2, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod2)
+	require.NoError(t, err)
+	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod2}))
+
+	// test that pod is cache managed
+	require.True(t, opaClient.HasGVK(pod2.GroupVersionKind()))
+
+	// now namespace exclude and attempt to re-add, this sequence should
+	// remove the object from the cache altogether.
+	processExcluder.Add([]configv1alpha1.MatchEntry{
+		{
+			ExcludedNamespaces: []util.Wildcard{"test-ns-excluded-2"},
+			Processes:          []string{"sync"},
+		},
+	})
+	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod2}))
+
+	// test that pod is now NOT cache managed
+	require.False(t, opaClient.HasGVK(pod2.GroupVersionKind()))
+}

From 2431c9d5070e1324e5bd53de3f386ac407bb5900 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Tue, 6 Jun 2023 19:56:35 +0000
Subject: [PATCH 15/22] add err cases

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/fakes/opadataclient.go                    | 20 +++++++++++++--
 .../cachemanager/cachemanager_test.go         | 25 +++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/pkg/fakes/opadataclient.go b/pkg/fakes/opadataclient.go
index 74291757813..76c117e3e94 100644
--- a/pkg/fakes/opadataclient.go
+++ b/pkg/fakes/opadataclient.go
@@ -32,8 +32,9 @@ type OpaKey struct {
 
 // FakeOpa is an OpaDataClient for testing.
 type FakeOpa struct {
-	mu   gosync.Mutex
-	data map[OpaKey]interface{}
+	mu           gosync.Mutex
+	data         map[OpaKey]interface{}
+	needsToError bool
 }
 
 var _ syncutil.OpaDataClient = &FakeOpa{}
@@ -58,6 +59,10 @@ func (f *FakeOpa) AddData(ctx context.Context, data interface{}) (*constraintTyp
 	f.mu.Lock()
 	defer f.mu.Unlock()
 
+	if f.needsToError {
+		return nil, fmt.Errorf("test error")
+	}
+
 	key, err := f.keyFor(data)
 	if err != nil {
 		return nil, err
@@ -75,6 +80,10 @@ func (f *FakeOpa) RemoveData(ctx context.Context, data interface{}) (*constraint
 	f.mu.Lock()
 	defer f.mu.Unlock()
 
+	if f.needsToError {
+		return nil, fmt.Errorf("test error")
+	}
+
 	if target.IsWipeData(data) {
 		f.data = make(map[OpaKey]interface{})
 		return &constraintTypes.Responses{}, nil
@@ -121,3 +130,10 @@ func (f *FakeOpa) Len() int {
 	defer f.mu.Unlock()
 	return len(f.data)
 }
+
+// SetErroring will error out on AddObject or RemoveObject.
+func (f *FakeOpa) SetErroring(enabled bool) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.needsToError = enabled
+}
diff --git a/pkg/syncutil/cachemanager/cachemanager_test.go b/pkg/syncutil/cachemanager/cachemanager_test.go
index 90a1675a2bf..ebd922264d6 100644
--- a/pkg/syncutil/cachemanager/cachemanager_test.go
+++ b/pkg/syncutil/cachemanager/cachemanager_test.go
@@ -111,3 +111,28 @@ func TestCacheManager_processExclusion(t *testing.T) {
 	// test that pod is now NOT cache managed
 	require.False(t, opaClient.HasGVK(pod2.GroupVersionKind()))
 }
+
+// TestCacheManager_errors tests that we cache manager responds to errors from the opa client.
+func TestCacheManager_errors(t *testing.T) {
+	mgr, _ := testutils.SetupManager(t, cfg)
+	opaClient := &fakes.FakeOpa{}
+	opaClient.SetErroring(true) // AddObject, RemoveObject will error out now.
+
+	tracker, err := readiness.SetupTracker(mgr, false, false, false)
+	assert.NoError(t, err)
+
+	processExcluder := process.Get()
+	cm := NewCacheManager(opaClient, syncutil.NewMetricsCache(), tracker, processExcluder)
+	ctx := context.Background()
+
+	pod := fakes.Pod(
+		fakes.WithNamespace("test-ns"),
+		fakes.WithName("test-name"),
+	)
+	unstructuredPod, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod)
+	require.NoError(t, err)
+
+	// test that cm bubbles up the errors
+	require.ErrorContains(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod}), "test error")
+	require.ErrorContains(t, cm.RemoveObject(ctx, &unstructured.Unstructured{Object: unstructuredPod}), "test error")
+}

From 28b4466b2bf3a63e83e1dda6ed5c781af5fedb9d Mon Sep 17 00:00:00 2001
From: alex <8968914+acpana@users.noreply.github.com>
Date: Tue, 6 Jun 2023 13:04:21 -0700
Subject: [PATCH 16/22] Apply suggestions from code review

removes todos

Signed-off-by: alex <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller_test.go | 1 -
 pkg/controller/sync/sync_controller.go          | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go
index 33f99d1cb14..ae768b39097 100644
--- a/pkg/controller/config/config_controller_test.go
+++ b/pkg/controller/config/config_controller_test.go
@@ -142,7 +142,6 @@ func TestReconcile(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	// todo acpana this is how you can set up a process excluder
 	processExcluder := process.Get()
 	processExcluder.Add(instance.Spec.Match)
 	events := make(chan event.GenericEvent, 1024)
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index ca3c4ec630c..3ab307e3bb0 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -121,8 +121,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-	// todo acpana -- double check that request namespace & name match instance namespace & name
-	// syncKey := syncutil.GetKeyForSyncMetrics(unpackedRequest.Namespace, unpackedRequest.Name)
 
 	reportMetricsForRenconcileRun := false
 	defer func() {
@@ -158,8 +156,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, err
 	}
 
-	// todo acpana -- double check that it is okay to remove what has been
-	// namespace excluded now (but was not namesapced excluded before)
 	if !instance.GetDeletionTimestamp().IsZero() {
 		if err := r.cm.RemoveObject(ctx, instance); err != nil {
 			return reconcile.Result{}, err

From 4858f5d8f6635e16002507d85757e51f07433867 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Tue, 6 Jun 2023 20:07:38 +0000
Subject: [PATCH 17/22] lint

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 3ab307e3bb0..73a9653f680 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -121,7 +121,6 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-
 	reportMetricsForRenconcileRun := false
 	defer func() {
 		if reportMetricsForRenconcileRun {

From b4b8bcfd75a9943112e90f61221d13f98741d22a Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Tue, 6 Jun 2023 22:50:36 +0000
Subject: [PATCH 18/22] review feedback

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/syncutil/cachemanager/cachemanager.go     | 16 ++++------
 .../cachemanager/cachemanager_test.go         | 31 ++-----------------
 2 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index a91501e69b6..2b32c9a077b 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -21,8 +21,6 @@ type CacheManager struct {
 	syncMetricsCache *syncutil.MetricsCache
 	tracker          *readiness.Tracker
 	processExcluder  *process.Excluder
-
-	// todo acpana -- integrate gvkaggregator
 }
 
 func NewCacheManager(opa syncutil.OpaDataClient, syncMetricsCache *syncutil.MetricsCache, tracker *readiness.Tracker, processExcluder *process.Excluder) *CacheManager {
@@ -44,8 +42,7 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 	// syncing this gvk
 	if isNamespaceExcluded {
 		c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
-
-		return c.RemoveObject(ctx, instance)
+		return nil
 	}
 
 	syncKey := syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName())
@@ -75,16 +72,15 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 }
 
 func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.Unstructured) error {
-	_, err := c.opa.RemoveData(ctx, instance)
-	// only delete from metrics map if the data removal was succcesful
-	if err != nil {
-		c.syncMetricsCache.DeleteObject(syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName()))
-
+	if _, err := c.opa.RemoveData(ctx, instance); err != nil {
 		return err
 	}
 
+	// only delete from metrics map if the data removal was succcesful
+	c.syncMetricsCache.DeleteObject(syncutil.GetKeyForSyncMetrics(instance.GetNamespace(), instance.GetName()))
 	c.tracker.ForData(instance.GroupVersionKind()).CancelExpect(instance)
-	return err
+
+	return nil
 }
 
 func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
diff --git a/pkg/syncutil/cachemanager/cachemanager_test.go b/pkg/syncutil/cachemanager/cachemanager_test.go
index ebd922264d6..633edac0e97 100644
--- a/pkg/syncutil/cachemanager/cachemanager_test.go
+++ b/pkg/syncutil/cachemanager/cachemanager_test.go
@@ -53,8 +53,7 @@ func TestCacheManager_AddObject_RemoveObject(t *testing.T) {
 	require.False(t, opaClient.HasGVK(pod.GroupVersionKind()))
 }
 
-// TestCacheManager_processExclusion makes sure that we don't add objects that are process excluded
-// and remove any objects that were previously not process excluded but have become so now.
+// TestCacheManager_processExclusion makes sure that we don't add objects that are process excluded.
 func TestCacheManager_processExclusion(t *testing.T) {
 	mgr, _ := testutils.SetupManager(t, cfg)
 	opaClient := &fakes.FakeOpa{}
@@ -62,7 +61,7 @@ func TestCacheManager_processExclusion(t *testing.T) {
 	tracker, err := readiness.SetupTracker(mgr, false, false, false)
 	assert.NoError(t, err)
 
-	// exclude "test-ns-excluded" namespace as excluded
+	// exclude "test-ns-excluded" namespace
 	processExcluder := process.Get()
 	processExcluder.Add([]configv1alpha1.MatchEntry{
 		{
@@ -84,32 +83,6 @@ func TestCacheManager_processExclusion(t *testing.T) {
 
 	// test that pod from excluded namespace is not cache managed
 	require.False(t, opaClient.HasGVK(pod.GroupVersionKind()))
-
-	// now add a new pod that IS NOT name space excluded at the time
-	// of addition but becomes exlcluded later.
-	pod2 := fakes.Pod(
-		fakes.WithNamespace("test-ns-excluded-2"),
-		fakes.WithName("test-name-2"),
-	)
-	unstructuredPod2, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod2)
-	require.NoError(t, err)
-	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod2}))
-
-	// test that pod is cache managed
-	require.True(t, opaClient.HasGVK(pod2.GroupVersionKind()))
-
-	// now namespace exclude and attempt to re-add, this sequence should
-	// remove the object from the cache altogether.
-	processExcluder.Add([]configv1alpha1.MatchEntry{
-		{
-			ExcludedNamespaces: []util.Wildcard{"test-ns-excluded-2"},
-			Processes:          []string{"sync"},
-		},
-	})
-	require.NoError(t, cm.AddObject(ctx, &unstructured.Unstructured{Object: unstructuredPod2}))
-
-	// test that pod is now NOT cache managed
-	require.False(t, opaClient.HasGVK(pod2.GroupVersionKind()))
 }
 
 // TestCacheManager_errors tests that we cache manager responds to errors from the opa client.

From f04af8a1b98a154c2f1c6941125b32af1c586191 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Fri, 9 Jun 2023 21:20:07 +0000
Subject: [PATCH 19/22] log in Observe()

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/constraint/constraint_controller.go           | 1 -
 .../constrainttemplate/constrainttemplate_controller.go      | 1 -
 pkg/controller/expansion/expansion_controller.go             | 1 -
 pkg/readiness/object_tracker.go                              | 3 +++
 pkg/syncutil/cachemanager/cachemanager.go                    | 5 -----
 5 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go
index 7110ab8b465..f720c4499c9 100644
--- a/pkg/controller/constraint/constraint_controller.go
+++ b/pkg/controller/constraint/constraint_controller.go
@@ -424,7 +424,6 @@ func (r *ReconcileConstraint) cacheConstraint(ctx context.Context, instance *uns
 
 	// Track for readiness
 	t.Observe(instance)
-	log.Info("[readiness] observed Constraint", "name", instance.GetName())
 
 	return nil
 }
diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go
index 19f9ed4a67b..73e696fe27d 100644
--- a/pkg/controller/constrainttemplate/constrainttemplate_controller.go
+++ b/pkg/controller/constrainttemplate/constrainttemplate_controller.go
@@ -441,7 +441,6 @@ func (r *ReconcileConstraintTemplate) handleUpdate(
 	// Mark for readiness tracking
 	t := r.tracker.For(gvkConstraintTemplate)
 	t.Observe(unversionedCT)
-	logger.Info("[readiness] observed ConstraintTemplate", "name", unversionedCT.GetName())
 
 	var newCRD *apiextensionsv1.CustomResourceDefinition
 	if currentCRD == nil {
diff --git a/pkg/controller/expansion/expansion_controller.go b/pkg/controller/expansion/expansion_controller.go
index 3d2b2577d28..a73aef17973 100644
--- a/pkg/controller/expansion/expansion_controller.go
+++ b/pkg/controller/expansion/expansion_controller.go
@@ -170,7 +170,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 
 	upsertErr := r.system.UpsertTemplate(et)
 	if upsertErr == nil {
-		log.Info("[readiness] observed ExpansionTemplate", "template name", et.GetName())
 		r.getTracker().Observe(versionedET)
 		r.registry.add(request.NamespacedName, metrics.ActiveStatus)
 	} else {
diff --git a/pkg/readiness/object_tracker.go b/pkg/readiness/object_tracker.go
index 4ec9cf39fde..994413a3455 100644
--- a/pkg/readiness/object_tracker.go
+++ b/pkg/readiness/object_tracker.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -250,6 +251,8 @@ func (t *objectTracker) Observe(o runtime.Object) {
 
 	// Track for future expectation.
 	t.seen[k] = struct{}{}
+
+	log.V(logging.DebugLevel).Info("[readiness] observed data", "gvk", o.GetObjectKind().GroupVersionKind())
 }
 
 func (t *objectTracker) Populated() bool {
diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 2b32c9a077b..516a1b6f199 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -6,16 +6,12 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
-	"github.com/open-policy-agent/gatekeeper/v3/pkg/logging"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/syncutil"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-var log = logf.Log.WithName("data-replication").WithValues("metaKind", "CacheManagerTracker")
-
 type CacheManager struct {
 	opa              syncutil.OpaDataClient
 	syncMetricsCache *syncutil.MetricsCache
@@ -67,7 +63,6 @@ func (c *CacheManager) AddObject(ctx context.Context, instance *unstructured.Uns
 	})
 	c.syncMetricsCache.AddKind(instance.GetKind())
 
-	log.V(logging.DebugLevel).Info("[readiness] observed data", "gvk", instance.GroupVersionKind(), "namespace", instance.GetNamespace(), "name", instance.GetName())
 	return err
 }
 

From 4aea7fa427cc4c287f2d56b2668dec8b393cdbdd Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Fri, 9 Jun 2023 21:39:18 +0000
Subject: [PATCH 20/22] review: don't pass in log

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go | 4 ++--
 pkg/controller/sync/sync_controller.go     | 2 +-
 pkg/syncutil/cachemanager/cachemanager.go  | 5 ++---
 pkg/syncutil/stats_reporter.go             | 6 ++++--
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index 5594551cec1..81a0e31b2e6 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -327,7 +327,7 @@ func (r *ReconcileConfig) wipeCacheIfNeeded(ctx context.Context) error {
 
 		// reset sync cache before sending the metric
 		r.syncMetricsCache.ResetCache()
-		r.syncMetricsCache.ReportSync(&syncutil.Reporter{}, log)
+		r.syncMetricsCache.ReportSync(&syncutil.Reporter{})
 
 		r.needsWipe = false
 	}
@@ -352,7 +352,7 @@ func (r *ReconcileConfig) replayData(ctx context.Context) error {
 			return fmt.Errorf("replaying data for %+v: %w", gvk, err)
 		}
 
-		defer r.syncMetricsCache.ReportSync(&syncutil.Reporter{}, log)
+		defer r.syncMetricsCache.ReportSync(&syncutil.Reporter{})
 
 		for i := range u.Items {
 			syncKey := syncutil.GetKeyForSyncMetrics(u.Items[i].GetNamespace(), u.Items[i].GetName())
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 73a9653f680..e6be6e0624b 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -128,7 +128,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 				log.Error(err, "failed to report sync duration")
 			}
 
-			r.cm.ReportSyncMetrics(&r.reporter, log)
+			r.cm.ReportSyncMetrics(&r.reporter)
 
 			if err := r.reporter.ReportLastSync(); err != nil {
 				log.Error(err, "failed to report last sync timestamp")
diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index 516a1b6f199..b1bbd737fe4 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/readiness"
@@ -78,6 +77,6 @@ func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.
 	return nil
 }
 
-func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter, log logr.Logger) {
-	c.syncMetricsCache.ReportSync(reporter, log)
+func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter) {
+	c.syncMetricsCache.ReportSync(reporter)
 }
diff --git a/pkg/syncutil/stats_reporter.go b/pkg/syncutil/stats_reporter.go
index 2288ae958c5..aaaf548b689 100644
--- a/pkg/syncutil/stats_reporter.go
+++ b/pkg/syncutil/stats_reporter.go
@@ -6,13 +6,15 @@ import (
 	"sync"
 	"time"
 
-	"github.com/go-logr/logr"
 	"github.com/open-policy-agent/gatekeeper/v3/pkg/metrics"
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+var log = logf.Log.WithName("reporter").WithValues("metaKind", "Sync")
+
 const (
 	syncMetricName         = "sync"
 	syncDurationMetricName = "sync_duration_seconds"
@@ -106,7 +108,7 @@ func (c *MetricsCache) DeleteObject(key string) {
 	delete(c.Cache, key)
 }
 
-func (c *MetricsCache) ReportSync(reporter *Reporter, log logr.Logger) {
+func (c *MetricsCache) ReportSync(reporter *Reporter) {
 	c.mux.RLock()
 	defer c.mux.RUnlock()
 

From e8161248adee9e8e77b26c76e166064ab22e9cc1 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Fri, 9 Jun 2023 21:58:39 +0000
Subject: [PATCH 21/22] review: don't pass in reporter

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/config/config_controller.go | 4 ++--
 pkg/controller/sync/sync_controller.go     | 2 +-
 pkg/syncutil/cachemanager/cachemanager.go  | 4 ++--
 pkg/syncutil/stats_reporter.go             | 8 +++++++-
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
index 81a0e31b2e6..f0c842787da 100644
--- a/pkg/controller/config/config_controller.go
+++ b/pkg/controller/config/config_controller.go
@@ -327,7 +327,7 @@ func (r *ReconcileConfig) wipeCacheIfNeeded(ctx context.Context) error {
 
 		// reset sync cache before sending the metric
 		r.syncMetricsCache.ResetCache()
-		r.syncMetricsCache.ReportSync(&syncutil.Reporter{})
+		r.syncMetricsCache.ReportSync()
 
 		r.needsWipe = false
 	}
@@ -352,7 +352,7 @@ func (r *ReconcileConfig) replayData(ctx context.Context) error {
 			return fmt.Errorf("replaying data for %+v: %w", gvk, err)
 		}
 
-		defer r.syncMetricsCache.ReportSync(&syncutil.Reporter{})
+		defer r.syncMetricsCache.ReportSync()
 
 		for i := range u.Items {
 			syncKey := syncutil.GetKeyForSyncMetrics(u.Items[i].GetNamespace(), u.Items[i].GetName())
diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index e6be6e0624b..16467ea5186 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -128,7 +128,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 				log.Error(err, "failed to report sync duration")
 			}
 
-			r.cm.ReportSyncMetrics(&r.reporter)
+			r.cm.ReportSyncMetrics()
 
 			if err := r.reporter.ReportLastSync(); err != nil {
 				log.Error(err, "failed to report last sync timestamp")
diff --git a/pkg/syncutil/cachemanager/cachemanager.go b/pkg/syncutil/cachemanager/cachemanager.go
index b1bbd737fe4..73e723860b5 100644
--- a/pkg/syncutil/cachemanager/cachemanager.go
+++ b/pkg/syncutil/cachemanager/cachemanager.go
@@ -77,6 +77,6 @@ func (c *CacheManager) RemoveObject(ctx context.Context, instance *unstructured.
 	return nil
 }
 
-func (c *CacheManager) ReportSyncMetrics(reporter *syncutil.Reporter) {
-	c.syncMetricsCache.ReportSync(reporter)
+func (c *CacheManager) ReportSyncMetrics() {
+	c.syncMetricsCache.ReportSync()
 }
diff --git a/pkg/syncutil/stats_reporter.go b/pkg/syncutil/stats_reporter.go
index aaaf548b689..adb5cf27cae 100644
--- a/pkg/syncutil/stats_reporter.go
+++ b/pkg/syncutil/stats_reporter.go
@@ -108,10 +108,16 @@ func (c *MetricsCache) DeleteObject(key string) {
 	delete(c.Cache, key)
 }
 
-func (c *MetricsCache) ReportSync(reporter *Reporter) {
+func (c *MetricsCache) ReportSync() {
 	c.mux.RLock()
 	defer c.mux.RUnlock()
 
+	reporter, err := NewStatsReporter()
+	if err != nil {
+		log.Error(err, "failed to initialize reporter")
+		return
+	}
+
 	totals := make(map[Tags]int)
 	for _, v := range c.Cache {
 		totals[v]++

From 0ff4b54ceac59685227ad121d0a62cd8fb755ef0 Mon Sep 17 00:00:00 2001
From: Alex Pana <8968914+acpana@users.noreply.github.com>
Date: Fri, 9 Jun 2023 22:03:43 +0000
Subject: [PATCH 22/22] review: rename reportMetrics

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
---
 pkg/controller/sync/sync_controller.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pkg/controller/sync/sync_controller.go b/pkg/controller/sync/sync_controller.go
index 16467ea5186..7dd5630bead 100644
--- a/pkg/controller/sync/sync_controller.go
+++ b/pkg/controller/sync/sync_controller.go
@@ -121,9 +121,9 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 		return reconcile.Result{}, nil
 	}
 
-	reportMetricsForRenconcileRun := false
+	reportMetrics := false
 	defer func() {
-		if reportMetricsForRenconcileRun {
+		if reportMetrics {
 			if err := r.reporter.ReportSyncDuration(time.Since(timeStart)); err != nil {
 				log.Error(err, "failed to report sync duration")
 			}
@@ -148,7 +148,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 				return reconcile.Result{}, err
 			}
 
-			reportMetricsForRenconcileRun = true
+			reportMetrics = true
 			return reconcile.Result{}, nil
 		}
 		// Error reading the object - requeue the request.
@@ -160,7 +160,7 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 			return reconcile.Result{}, err
 		}
 
-		reportMetricsForRenconcileRun = true
+		reportMetrics = true
 		return reconcile.Result{}, nil
 	}
 
@@ -173,12 +173,12 @@ func (r *ReconcileSync) Reconcile(ctx context.Context, request reconcile.Request
 	)
 
 	if err := r.cm.AddObject(ctx, instance); err != nil {
-		reportMetricsForRenconcileRun = true
+		reportMetrics = true
 
 		return reconcile.Result{}, err
 	}
 
-	reportMetricsForRenconcileRun = true
+	reportMetrics = true
 
 	return reconcile.Result{}, nil
 }