perf: reduce remaining hot-path overhead (#4661)

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
Co-authored-by: Jaydip Gabani <gabanijaydip@gmail.com>

Author: sozercan

main.go

@@ -432,6 +432,7 @@ func setupControllers(ctx context.Context, mgr ctrl.Manager, tracker *readiness.
 		case *externaldataProviderResponseCacheTTL > 0:
 			providerResponseCache := frameworksexternaldata.NewProviderResponseCache(ctx, *externaldataProviderResponseCacheTTL)
 			args = append(args, rego.AddExternalDataProviderResponseCache(providerResponseCache))
+			mutationOpts.ProviderResponseCache = providerResponseCache
 		case *externaldataProviderResponseCacheTTL == 0:
 			setupLog.Info("external data provider response cache is disabled")
 		default:

pkg/audit/manager.go

@@ -493,6 +493,9 @@ func (am *Manager) auditResources(
 	for gv, gvKinds := range clusterAPIResources {
 	kindsLoop:
 		for kind := range gvKinds {
+			if !shouldAuditKind(matchedKinds, kind) {
+				continue
+			}
 			am.log.V(logging.DebugLevel).Info("Listing objects for GVK", "group", gv.Group, "version", gv.Version, "kind", kind)
 			// delete all existing folders from cache dir before starting next kind
 			err := am.removeAllFromDir(*apiCacheDir, *auditChunkSize)
@@ -502,10 +505,6 @@ func (am *Manager) auditResources(
 			}
 			// tracking number of folders created for this kind
 			folderCount := 0
-			_, matchAll := matchedKinds["*"]
-			if _, found := matchedKinds[kind]; !found && !matchAll {
-				continue
-			}
 
 			objList := &unstructured.UnstructuredList{}
 			opts := &client.ListOptions{
@@ -585,6 +584,13 @@ func (am *Manager) auditResources(
 	return nil
 }
 
+func shouldAuditKind(matchedKinds map[string]bool, kind string) bool {
+	if matchedKinds["*"] {
+		return true
+	}
+	return matchedKinds[kind]
+}
+
 func (am *Manager) auditFromCache(ctx context.Context) ([]Result, []error) {
 	objs, err := am.auditCache.ListObjects(ctx)
 	if err != nil {

pkg/audit/manager_benchmark_test.go

@@ -148,3 +148,71 @@ func writeBenchmarkPerObjectFiles(tb testing.TB, directory string, objects []uns
 		require.NoError(tb, os.WriteFile(path.Join(directory, strconv.Itoa(i)), jsonBytes, 0o600))
 	}
 }
+
+func TestShouldAuditKind(t *testing.T) {
+	for _, tc := range []struct {
+		name         string
+		matchedKinds map[string]bool
+		kind         string
+		want         bool
+	}{
+		{name: "explicit match", matchedKinds: map[string]bool{"Pod": true}, kind: "Pod", want: true},
+		{name: "explicit mismatch", matchedKinds: map[string]bool{"Pod": true}, kind: "ConfigMap"},
+		{name: "wildcard", matchedKinds: map[string]bool{"*": true}, kind: "ConfigMap", want: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := shouldAuditKind(tc.matchedKinds, tc.kind); got != tc.want {
+				t.Fatalf("shouldAuditKind(%v, %q) = %t, want %t", tc.matchedKinds, tc.kind, got, tc.want)
+			}
+		})
+	}
+}
+
+func BenchmarkAuditSkippedKindAvoidsCacheCleanup(b *testing.B) {
+	const staleDirs = 100
+	am := benchmarkAuditManager(b)
+	matchedKinds := map[string]bool{"Pod": true}
+
+	b.Run("old_order_cleanup_before_skip", func(b *testing.B) {
+		root := b.TempDir()
+		b.ReportAllocs()
+		for i := 0; i < b.N; i++ {
+			b.StopTimer()
+			populateAuditCacheDirs(b, root, staleDirs)
+			b.StartTimer()
+			if err := am.removeAllFromDir(root, *auditChunkSize); err != nil {
+				b.Fatal(err)
+			}
+			if shouldAuditKind(matchedKinds, "ConfigMap") {
+				b.Fatal("expected ConfigMap to be skipped")
+			}
+		}
+	})
+
+	b.Run("new_order_skip_before_cleanup", func(b *testing.B) {
+		root := b.TempDir()
+		populateAuditCacheDirs(b, root, staleDirs)
+		b.ReportAllocs()
+		for i := 0; i < b.N; i++ {
+			if !shouldAuditKind(matchedKinds, "ConfigMap") {
+				continue
+			}
+			if err := am.removeAllFromDir(root, *auditChunkSize); err != nil {
+				b.Fatal(err)
+			}
+		}
+	})
+}
+
+func populateAuditCacheDirs(tb testing.TB, root string, count int) {
+	tb.Helper()
+	for i := 0; i < count; i++ {
+		dir := path.Join(root, "stale-"+strconv.Itoa(i))
+		if err := os.MkdirAll(dir, 0o750); err != nil {
+			tb.Fatal(err)
+		}
+		if err := os.WriteFile(path.Join(dir, auditObjectsFile), []byte("[]"), 0o600); err != nil {
+			tb.Fatal(err)
+		}
+	}
+}

pkg/audit/manager_test.go

@@ -3,12 +3,16 @@ package audit
 import (
 	"container/heap"
 	"context"
+	"encoding/json"
 	"flag"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path"
 	"reflect"
 	"strconv"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -37,8 +41,10 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	controllerruntimemanager "sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 func Test_SVQueue(t *testing.T) {
@@ -608,6 +614,130 @@ func Test_removeAllFromDir(t *testing.T) {
 	})
 }
 
+type auditResourcesTestManager struct {
+	controllerruntimemanager.Manager
+	config *rest.Config
+}
+
+func (m *auditResourcesTestManager) GetConfig() *rest.Config {
+	return m.config
+}
+
+type auditResourcesTestClient struct {
+	client.Client
+	cacheDir            string
+	constraintListCalls int
+	resourceListCalls   int
+}
+
+func (c *auditResourcesTestClient) List(_ context.Context, list client.ObjectList, _ ...client.ListOption) error {
+	switch list.GetObjectKind().GroupVersionKind().Kind {
+	case "ConfigMapList":
+		c.resourceListCalls++
+		return nil
+	case "K8sRequiredLabelsList":
+		c.constraintListCalls++
+	default:
+		return fmt.Errorf("unexpected list GVK %s", list.GetObjectKind().GroupVersionKind())
+	}
+
+	constraintList, ok := list.(*unstructured.UnstructuredList)
+	if !ok {
+		return fmt.Errorf("unexpected list type %T", list)
+	}
+	constraintList.Items = []unstructured.Unstructured{{Object: map[string]interface{}{
+		"spec": map[string]interface{}{
+			"match": map[string]interface{}{
+				"kinds": []interface{}{
+					map[string]interface{}{"kinds": []interface{}{"Pod"}},
+				},
+			},
+		},
+	}}}
+
+	sentinelDir := path.Join(c.cacheDir, "sentinel")
+	if err := os.Mkdir(sentinelDir, 0o750); err != nil {
+		return err
+	}
+	return os.WriteFile(path.Join(sentinelDir, auditObjectsFile), []byte("[]"), 0o600)
+}
+
+func TestAuditResourcesSkipsUnmatchedKindBeforeCleanup(t *testing.T) {
+	oldAPICacheDir := *apiCacheDir
+	oldAuditMatchKindOnly := *auditMatchKindOnly
+	t.Cleanup(func() {
+		*apiCacheDir = oldAPICacheDir
+		*auditMatchKindOnly = oldAuditMatchKindOnly
+	})
+
+	cacheDir := t.TempDir()
+	*apiCacheDir = cacheDir
+	*auditMatchKindOnly = true
+
+	var coreResourcesDiscovered atomic.Bool
+	discoveryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		var response interface{}
+		switch r.URL.Path {
+		case "/api":
+			response = metav1.APIVersions{
+				TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "APIVersions"},
+				Versions: []string{"v1"},
+			}
+		case "/api/v1":
+			coreResourcesDiscovered.Store(true)
+			response = metav1.APIResourceList{
+				GroupVersion: "v1",
+				APIResources: []metav1.APIResource{{
+					Name:       "configmaps",
+					Namespaced: true,
+					Kind:       "ConfigMap",
+					Verbs:      metav1.Verbs{"list"},
+				}},
+			}
+		case "/apis":
+			response = metav1.APIGroupList{
+				TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "APIGroupList"},
+			}
+		default:
+			http.NotFound(w, r)
+			return
+		}
+		if err := json.NewEncoder(w).Encode(response); err != nil {
+			t.Errorf("encoding discovery response: %v", err)
+		}
+	}))
+	t.Cleanup(discoveryServer.Close)
+
+	testClient := &auditResourcesTestClient{
+		Client:   fake.NewClientBuilder().Build(),
+		cacheDir: cacheDir,
+	}
+	am := &Manager{
+		client: testClient,
+		mgr: &auditResourcesTestManager{
+			config: &rest.Config{Host: discoveryServer.URL},
+		},
+		log: logr.Discard(),
+	}
+
+	err := am.auditResources(
+		context.Background(),
+		[]schema.GroupVersionKind{{Group: "constraints.gatekeeper.sh", Version: "v1beta1", Kind: "K8sRequiredLabelsList"}},
+		map[util.KindVersionName]*LimitQueue{},
+		map[util.KindVersionName]int64{},
+		map[util.EnforcementAction]int64{},
+		"test-timestamp",
+		&auditExportPublishingState{Errors: map[string]error{}},
+	)
+	require.NoError(t, err)
+	require.True(t, coreResourcesDiscovered.Load(), "core resources were not discovered")
+	require.Equal(t, 1, testClient.constraintListCalls)
+	require.Zero(t, testClient.resourceListCalls, "resource List called for unmatched ConfigMap")
+	_, err = os.Stat(path.Join(cacheDir, "sentinel", auditObjectsFile))
+	require.NoError(t, err, "cache cleanup ran for unmatched ConfigMap")
+}
+
 func Test_readUnstructured(t *testing.T) {
 	am := Manager{}