Webhook Accessors Smart Recompilation

Addresses https://github.com/kubernetes/kubernetes/issues/116588

This is an WIP patch trying to avoid recompiling CELs expressions when
recreation Validating/Mutating WebhookAccessors.

Maybe we should also concider using generatic.Controller from
5f59f44983/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/internal/generic/controller.go

staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go

@@ -19,8 +19,9 @@ package configuration
 import (
 	"fmt"
 	"sort"
+	"sync"
 
-	"k8s.io/api/admissionregistration/v1"
+	v1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook"
@@ -36,6 +37,7 @@ type validatingWebhookConfigurationManager struct {
 	lister      admissionregistrationlisters.ValidatingWebhookConfigurationLister
 	hasSynced   func() bool
 	lazy        synctrack.Lazy[[]webhook.WebhookAccessor]
+	needRefresh sync.Map // NOTE maybe use simple map and a sync.Mutex
 }
 
 var _ generic.Source = &validatingWebhookConfigurationManager{}
@@ -49,14 +51,31 @@ func NewValidatingWebhookConfigurationManager(f informers.SharedInformerFactory)
 
 	handle, _ := informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(_ interface{}) { manager.lazy.Notify() },
-		UpdateFunc: func(_, _ interface{}) { manager.lazy.Notify() },
+		UpdateFunc: func(old, new interface{}) {
-		DeleteFunc: func(_ interface{}) { manager.lazy.Notify() },
+			//TODO: we can still dig deeper and figure out which ones of
+			// the webhooks have changed + whether CEL expressions changed
+			// etc... thinking about using reflect..
+			obj := new.(*v1.ValidatingWebhookConfiguration)
+			// lock R+W
+			manager.needRefresh.Store(obj.GetName(), true)
+			manager.lazy.Notify()
+		},
+		DeleteFunc: func(vwc interface{}) {
+			obj := vwc.(*v1.ValidatingWebhookConfiguration)
+			manager.needRefresh.Delete(obj.Name)
+			manager.lazy.Notify()
+		},
 	})
 	manager.hasSynced = handle.HasSynced
 
 	return manager
 }
 
+func (v *validatingWebhookConfigurationManager) configurationNeedRefresh(namespacedName string) bool {
+	_, ok := v.needRefresh.Load(namespacedName)
+	return ok
+}
+
 // Webhooks returns the merged ValidatingWebhookConfiguration.
 func (v *validatingWebhookConfigurationManager) Webhooks() []webhook.WebhookAccessor {
 	out, err := v.lazy.Get()
@@ -75,15 +94,31 @@ func (v *validatingWebhookConfigurationManager) getConfiguration() ([]webhook.We
 	if err != nil {
 		return []webhook.WebhookAccessor{}, err
 	}
-	return mergeValidatingWebhookConfigurations(configurations), nil
+	return v.smartReloadValidatingWebhookConfigurations(configurations), nil
 }
 
-func mergeValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor {
+func (v *validatingWebhookConfigurationManager) smartReloadValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor {
 	sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName)
 	accessors := []webhook.WebhookAccessor{}
+	cachedAccessors, _ := v.lazy.Get()
+cfgLoop:
 	for _, c := range configurations {
+		if !v.configurationNeedRefresh(c.Name) {
+			// Pick an already cached webhookAccessor
+			for _, ca := range cachedAccessors {
+				if ca.GetName() == c.Name {
+					accessors = append(accessors, ca)
+					v.needRefresh.Delete(c.Name)
+					continue cfgLoop
+				}
+			}
+		}
+
 		// webhook names are not validated for uniqueness, so we check for duplicates and
 		// add a int suffix to distinguish between them
+		//
+		// NOTE: In `pkg/apis/admissionregistration/validation.go` webhook names are checked
+		// for uniqueness now. Is it safe to remove this?
 		names := map[string]int{}
 		for i := range c.Webhooks {
 			n := c.Webhooks[i].Name

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go

@@ -216,6 +216,22 @@ func NewValidatingWebhookAccessor(uid, configurationName string, h *v1.Validatin
 	return &validatingWebhookAccessor{uid: uid, configurationName: configurationName, ValidatingWebhook: h}
 }
 
+// NOTE: we're thinking about adding a new NewValidatingWebhookAccessorWithOptions to allow
+// recreating webhookAccessor with existing restClient, compiled CELs, selectors etc... to
+// avoid remaking unnecessary expensive operations.
+/*
+type Options struct {
+	// rest client
+	// compiled CELs
+	// selectors...
+}
+
+// NewValidatingWebhookAccessorWithOptions creates an accessor for a ValidatingWebhook.
+func NewValidatingWebhookAccessorWithOptions(uid, configurationName string, h *v1.ValidatingWebhook, opt *Options) WebhookAccessor {
+	return &validatingWebhookAccessor{uid: uid, configurationName: configurationName, ValidatingWebhook: h}
+}
+*/
+
 type validatingWebhookAccessor struct {
 	*v1.ValidatingWebhook
 	uid               string