Merge pull request #118051 from A-Hilaly/api-server/webhooks/smart-reload

support `WebhookAccessors` smart reload

staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_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"
@@ -29,13 +30,22 @@ import (
 	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/cache/synctrack"
+	"k8s.io/klog/v2"
 )
 
+// Type for test injection.
+type mutatingWebhookAccessorCreator func(uid string, configurationName string, h *v1.MutatingWebhook) webhook.WebhookAccessor
+
 // mutatingWebhookConfigurationManager collects the mutating webhook objects so that they can be called.
 type mutatingWebhookConfigurationManager struct {
-	lister    admissionregistrationlisters.MutatingWebhookConfigurationLister
-	hasSynced func() bool
-	lazy      synctrack.Lazy[[]webhook.WebhookAccessor]
+	lister              admissionregistrationlisters.MutatingWebhookConfigurationLister
+	hasSynced           func() bool
+	lazy                synctrack.Lazy[[]webhook.WebhookAccessor]
+	configurationsCache sync.Map
+	// createMutatingWebhookAccessor is used to instantiate webhook accessors.
+	// This function is defined as field instead of a struct method to allow injection
+	// during tests
+	createMutatingWebhookAccessor mutatingWebhookAccessorCreator
 }
 
 var _ generic.Source = &mutatingWebhookConfigurationManager{}
@@ -43,14 +53,35 @@ var _ generic.Source = &mutatingWebhookConfigurationManager{}
 func NewMutatingWebhookConfigurationManager(f informers.SharedInformerFactory) generic.Source {
 	informer := f.Admissionregistration().V1().MutatingWebhookConfigurations()
 	manager := &mutatingWebhookConfigurationManager{
-		lister: informer.Lister(),
+		lister:                        informer.Lister(),
+		createMutatingWebhookAccessor: webhook.NewMutatingWebhookAccessor,
 	}
 	manager.lazy.Evaluate = manager.getConfiguration
 
 	handle, _ := informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		AddFunc:    func(_ interface{}) { manager.lazy.Notify() },
-		UpdateFunc: func(_, _ interface{}) { manager.lazy.Notify() },
-		DeleteFunc: func(_ interface{}) { manager.lazy.Notify() },
+		AddFunc: func(_ interface{}) { manager.lazy.Notify() },
+		UpdateFunc: func(old, new interface{}) {
+			obj := new.(*v1.MutatingWebhookConfiguration)
+			manager.configurationsCache.Delete(obj.GetName())
+			manager.lazy.Notify()
+		},
+		DeleteFunc: func(obj interface{}) {
+			vwc, ok := obj.(*v1.MutatingWebhookConfiguration)
+			if !ok {
+				tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+				if !ok {
+					klog.V(2).Infof("Couldn't get object from tombstone %#v", obj)
+					return
+				}
+				vwc, ok = tombstone.Obj.(*v1.MutatingWebhookConfiguration)
+				if !ok {
+					klog.V(2).Infof("Tombstone contained object that is not expected %#v", obj)
+					return
+				}
+			}
+			manager.configurationsCache.Delete(vwc.Name)
+			manager.lazy.Notify()
+		},
 	})
 	manager.hasSynced = handle.HasSynced
 
@@ -75,25 +106,46 @@ func (m *mutatingWebhookConfigurationManager) getConfiguration() ([]webhook.Webh
 	if err != nil {
 		return []webhook.WebhookAccessor{}, err
 	}
-	return mergeMutatingWebhookConfigurations(configurations), nil
+	return m.getMutatingWebhookConfigurations(configurations), nil
 }
 
-func mergeMutatingWebhookConfigurations(configurations []*v1.MutatingWebhookConfiguration) []webhook.WebhookAccessor {
+// getMutatingWebhookConfigurations returns the webhook accessors for a given list of
+// mutating webhook configurations.
+//
+// This function will, first, try to load the webhook accessors from the cache and avoid
+// recreating them, which can be expessive (requiring CEL expression recompilation).
+func (m *mutatingWebhookConfigurationManager) getMutatingWebhookConfigurations(configurations []*v1.MutatingWebhookConfiguration) []webhook.WebhookAccessor {
 	// The internal order of webhooks for each configuration is provided by the user
 	// but configurations themselves can be in any order. As we are going to run these
 	// webhooks in serial, they are sorted here to have a deterministic order.
 	sort.SliceStable(configurations, MutatingWebhookConfigurationSorter(configurations).ByName)
-	accessors := []webhook.WebhookAccessor{}
+	size := 0
+	for _, cfg := range configurations {
+		size += len(cfg.Webhooks)
+	}
+	accessors := make([]webhook.WebhookAccessor, 0, size)
+
 	for _, c := range configurations {
+		cachedConfigurationAccessors, ok := m.configurationsCache.Load(c.Name)
+		if ok {
+			// Pick an already cached webhookAccessor
+			accessors = append(accessors, cachedConfigurationAccessors.([]webhook.WebhookAccessor)...)
+			continue
+		}
+
 		// webhook names are not validated for uniqueness, so we check for duplicates and
 		// add a int suffix to distinguish between them
 		names := map[string]int{}
+		configurationAccessors := make([]webhook.WebhookAccessor, 0, len(c.Webhooks))
 		for i := range c.Webhooks {
 			n := c.Webhooks[i].Name
 			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
 			names[n]++
-			accessors = append(accessors, webhook.NewMutatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
+			configurationAccessor := m.createMutatingWebhookAccessor(uid, c.Name, &c.Webhooks[i])
+			configurationAccessors = append(configurationAccessors, configurationAccessor)
 		}
+		accessors = append(accessors, configurationAccessors...)
+		m.configurationsCache.Store(c.Name, configurationAccessors)
 	}
 	return accessors
 }

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

@@ -24,6 +24,7 @@ import (
 
 	"k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 )
@@ -81,3 +82,202 @@ func TestGetMutatingWebhookConfig(t *testing.T) {
 		}
 	}
 }
+
+// mockCreateMutatingWebhookAccessor is a struct used to compute how many times
+// the function webhook.NewMutatingWebhookAccessor is being called when refreshing
+// webhookAccessors.
+//
+// NOTE: Maybe there some testing help that we can import and reuse instead.
+type mockCreateMutatingWebhookAccessor struct {
+	numberOfCalls int
+}
+
+func (mock *mockCreateMutatingWebhookAccessor) calledNTimes() int { return mock.numberOfCalls }
+func (mock *mockCreateMutatingWebhookAccessor) resetCounter()     { mock.numberOfCalls = 0 }
+func (mock *mockCreateMutatingWebhookAccessor) incrementCounter() { mock.numberOfCalls++ }
+
+func (mock *mockCreateMutatingWebhookAccessor) fn(uid string, configurationName string, h *v1.MutatingWebhook) webhook.WebhookAccessor {
+	mock.incrementCounter()
+	return webhook.NewMutatingWebhookAccessor(uid, configurationName, h)
+}
+
+func mutatingConfigurationTotalWebhooks(configurations []*v1.MutatingWebhookConfiguration) int {
+	total := 0
+	for _, configuration := range configurations {
+		total += len(configuration.Webhooks)
+	}
+	return total
+}
+
+func TestGetMutatingWebhookConfigSmartReload(t *testing.T) {
+	type args struct {
+		createWebhookConfigurations []*v1.MutatingWebhookConfiguration
+		updateWebhookConfigurations []*v1.MutatingWebhookConfiguration
+	}
+	tests := []struct {
+		name              string
+		args              args
+		numberOfCreations int
+		// number of refreshes are number of times we recrated a webhook accessor
+		// instead of pulling from the cache.
+		numberOfRefreshes             int
+		finalNumberOfWebhookAccessors int
+	}{
+		{
+			name: "no creations and no updates",
+			args: args{
+				nil,
+				nil,
+			},
+			numberOfCreations:             0,
+			numberOfRefreshes:             0,
+			finalNumberOfWebhookAccessors: 0,
+		},
+		{
+			name: "create configurations and no updates",
+			args: args{
+				[]*v1.MutatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook1"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook1.1"}, {Name: "webhook1.2"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook2"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook2.1"}, {Name: "webhook2.2"}},
+					},
+				},
+				nil,
+			},
+			numberOfCreations:             4,
+			numberOfRefreshes:             0,
+			finalNumberOfWebhookAccessors: 4,
+		},
+		{
+			name: "create configurations and update some of them",
+			args: args{
+				[]*v1.MutatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook3"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook3.1"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook4"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook4.1"}},
+					},
+				},
+				[]*v1.MutatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook4"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook4.1-updated"}, {Name: "webhook4.2"}},
+					},
+				},
+			},
+			numberOfCreations:             2,
+			numberOfRefreshes:             2,
+			finalNumberOfWebhookAccessors: 3,
+		},
+		{
+			name: "create configuration and update moar of them",
+			args: args{
+				[]*v1.MutatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook5"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook5.1"}, {Name: "webhook5.2"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook6"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook6.1"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook7.1"}},
+					},
+				},
+				[]*v1.MutatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook6"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook6.1-updated"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.MutatingWebhook{{Name: "webhook7.1-updated"}, {Name: "webhook7.2"}},
+					},
+				},
+			},
+			numberOfCreations:             4,
+			numberOfRefreshes:             3,
+			finalNumberOfWebhookAccessors: 5,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fake.NewSimpleClientset()
+			informerFactory := informers.NewSharedInformerFactory(client, 0)
+			stop := make(chan struct{})
+			defer close(stop)
+			manager := NewMutatingWebhookConfigurationManager(informerFactory)
+			managerStructPtr := manager.(*mutatingWebhookConfigurationManager)
+			fakeWebhookAccessorCreator := &mockCreateMutatingWebhookAccessor{}
+			managerStructPtr.createMutatingWebhookAccessor = fakeWebhookAccessorCreator.fn
+			informerFactory.Start(stop)
+			informerFactory.WaitForCacheSync(stop)
+
+			// Create webhooks
+			for _, configurations := range tt.args.createWebhookConfigurations {
+				client.
+					AdmissionregistrationV1().
+					MutatingWebhookConfigurations().
+					Create(context.TODO(), configurations, metav1.CreateOptions{})
+			}
+			// TODO use channels to wait for manager.createMutatingWebhookAccessor
+			// to be called instead of using time.Sleep
+			time.Sleep(1 * time.Second)
+			webhooks := manager.Webhooks()
+			if mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
+				t.Errorf("Expected number of webhooks %d received %d",
+					mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations),
+					len(webhooks),
+				)
+			}
+			// assert creations
+			if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
+				t.Errorf(
+					"Expected number of creations %d received %d",
+					tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
+				)
+			}
+
+			// reset mock counter
+			fakeWebhookAccessorCreator.resetCounter()
+
+			// Update webhooks
+			for _, configurations := range tt.args.updateWebhookConfigurations {
+				client.
+					AdmissionregistrationV1().
+					MutatingWebhookConfigurations().
+					Update(context.TODO(), configurations, metav1.UpdateOptions{})
+			}
+			// TODO use channels to wait for manager.createMutatingWebhookAccessor
+			// to be called instead of using time.Sleep
+			time.Sleep(1 * time.Second)
+			webhooks = manager.Webhooks()
+			if tt.finalNumberOfWebhookAccessors != len(webhooks) {
+				t.Errorf("Expected final number of webhooks %d received %d",
+					tt.finalNumberOfWebhookAccessors,
+					len(webhooks),
+				)
+			}
+
+			// assert updates
+			if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
+				t.Errorf(
+					"Expected number of refreshes %d received %d",
+					tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
+				)
+			}
+			// reset mock counter for the next test cases
+			fakeWebhookAccessorCreator.resetCounter()
+		})
+	}
+}

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"
@@ -29,13 +30,22 @@ import (
 	admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/cache/synctrack"
+	"k8s.io/klog/v2"
 )
 
+// Type for test injection.
+type validatingWebhookAccessorCreator func(uid string, configurationName string, h *v1.ValidatingWebhook) webhook.WebhookAccessor
+
 // validatingWebhookConfigurationManager collects the validating webhook objects so that they can be called.
 type validatingWebhookConfigurationManager struct {
-	lister    admissionregistrationlisters.ValidatingWebhookConfigurationLister
-	hasSynced func() bool
-	lazy      synctrack.Lazy[[]webhook.WebhookAccessor]
+	lister              admissionregistrationlisters.ValidatingWebhookConfigurationLister
+	hasSynced           func() bool
+	lazy                synctrack.Lazy[[]webhook.WebhookAccessor]
+	configurationsCache sync.Map
+	// createValidatingWebhookAccessor is used to instantiate webhook accessors.
+	// This function is defined as field instead of a struct method to allow injection
+	// during tests
+	createValidatingWebhookAccessor validatingWebhookAccessorCreator
 }
 
 var _ generic.Source = &validatingWebhookConfigurationManager{}
@@ -43,14 +53,35 @@ var _ generic.Source = &validatingWebhookConfigurationManager{}
 func NewValidatingWebhookConfigurationManager(f informers.SharedInformerFactory) generic.Source {
 	informer := f.Admissionregistration().V1().ValidatingWebhookConfigurations()
 	manager := &validatingWebhookConfigurationManager{
-		lister: informer.Lister(),
+		lister:                          informer.Lister(),
+		createValidatingWebhookAccessor: webhook.NewValidatingWebhookAccessor,
 	}
 	manager.lazy.Evaluate = manager.getConfiguration
 
 	handle, _ := informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		AddFunc:    func(_ interface{}) { manager.lazy.Notify() },
-		UpdateFunc: func(_, _ interface{}) { manager.lazy.Notify() },
-		DeleteFunc: func(_ interface{}) { manager.lazy.Notify() },
+		AddFunc: func(_ interface{}) { manager.lazy.Notify() },
+		UpdateFunc: func(old, new interface{}) {
+			obj := new.(*v1.ValidatingWebhookConfiguration)
+			manager.configurationsCache.Delete(obj.GetName())
+			manager.lazy.Notify()
+		},
+		DeleteFunc: func(obj interface{}) {
+			vwc, ok := obj.(*v1.ValidatingWebhookConfiguration)
+			if !ok {
+				tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+				if !ok {
+					klog.V(2).Infof("Couldn't get object from tombstone %#v", obj)
+					return
+				}
+				vwc, ok = tombstone.Obj.(*v1.ValidatingWebhookConfiguration)
+				if !ok {
+					klog.V(2).Infof("Tombstone contained object that is not expected %#v", obj)
+					return
+				}
+			}
+			manager.configurationsCache.Delete(vwc.Name)
+			manager.lazy.Notify()
+		},
 	})
 	manager.hasSynced = handle.HasSynced
 
@@ -66,7 +97,7 @@ func (v *validatingWebhookConfigurationManager) Webhooks() []webhook.WebhookAcce
 	return out
 }
 
-// HasSynced returns true if the initial set of mutating webhook configurations
+// HasSynced returns true if the initial set of validating webhook configurations
 // has been loaded.
 func (v *validatingWebhookConfigurationManager) HasSynced() bool { return v.hasSynced() }
 
@@ -75,23 +106,45 @@ func (v *validatingWebhookConfigurationManager) getConfiguration() ([]webhook.We
 	if err != nil {
 		return []webhook.WebhookAccessor{}, err
 	}
-	return mergeValidatingWebhookConfigurations(configurations), nil
+	return v.getValidatingWebhookConfigurations(configurations), nil
 }
 
-func mergeValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor {
+// getMutatingWebhookConfigurations returns the webhook accessors for a given list of
+// mutating webhook configurations.
+//
+// This function will, first, try to load the webhook accessors from the cache and avoid
+// recreating them, which can be expessive (requiring CEL expression recompilation).
+func (v *validatingWebhookConfigurationManager) getValidatingWebhookConfigurations(configurations []*v1.ValidatingWebhookConfiguration) []webhook.WebhookAccessor {
 	sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName)
-	accessors := []webhook.WebhookAccessor{}
+	size := 0
+	for _, cfg := range configurations {
+		size += len(cfg.Webhooks)
+	}
+	accessors := make([]webhook.WebhookAccessor, 0, size)
+
 	for _, c := range configurations {
+		cachedConfigurationAccessors, ok := v.configurationsCache.Load(c.Name)
+		if ok {
+			// Pick an already cached webhookAccessor
+			accessors = append(accessors, cachedConfigurationAccessors.([]webhook.WebhookAccessor)...)
+			continue
+		}
+
 		// webhook names are not validated for uniqueness, so we check for duplicates and
 		// add a int suffix to distinguish between them
 		names := map[string]int{}
+		configurationAccessors := make([]webhook.WebhookAccessor, 0, len(c.Webhooks))
 		for i := range c.Webhooks {
 			n := c.Webhooks[i].Name
 			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
 			names[n]++
-			accessors = append(accessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
+			configurationAccessor := v.createValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i])
+			configurationAccessors = append(configurationAccessors, configurationAccessor)
 		}
+		accessors = append(accessors, configurationAccessors...)
+		v.configurationsCache.Store(c.Name, configurationAccessors)
 	}
+
 	return accessors
 }
 

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

@@ -24,6 +24,7 @@ import (
 
 	"k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 )
@@ -81,3 +82,202 @@ func TestGetValidatingWebhookConfig(t *testing.T) {
 		}
 	}
 }
+
+// mockCreateValidatingWebhookAccessor is a struct used to compute how many times
+// the function webhook.NewValidatingWebhookAccessor is being called when refreshing
+// webhookAccessors.
+//
+// NOTE: Maybe there some testing help that we can import and reuse instead.
+type mockCreateValidatingWebhookAccessor struct {
+	numberOfCalls int
+}
+
+func (mock *mockCreateValidatingWebhookAccessor) calledNTimes() int { return mock.numberOfCalls }
+func (mock *mockCreateValidatingWebhookAccessor) resetCounter()     { mock.numberOfCalls = 0 }
+func (mock *mockCreateValidatingWebhookAccessor) incrementCounter() { mock.numberOfCalls++ }
+
+func (mock *mockCreateValidatingWebhookAccessor) fn(uid string, configurationName string, h *v1.ValidatingWebhook) webhook.WebhookAccessor {
+	mock.incrementCounter()
+	return webhook.NewValidatingWebhookAccessor(uid, configurationName, h)
+}
+
+func configurationTotalWebhooks(configurations []*v1.ValidatingWebhookConfiguration) int {
+	total := 0
+	for _, configuration := range configurations {
+		total += len(configuration.Webhooks)
+	}
+	return total
+}
+
+func TestGetValidatingWebhookConfigSmartReload(t *testing.T) {
+	type args struct {
+		createWebhookConfigurations []*v1.ValidatingWebhookConfiguration
+		updateWebhookConfigurations []*v1.ValidatingWebhookConfiguration
+	}
+	tests := []struct {
+		name              string
+		args              args
+		numberOfCreations int
+		// number of refreshes are number of times we recrated a webhook accessor
+		// instead of pulling from the cache.
+		numberOfRefreshes             int
+		finalNumberOfWebhookAccessors int
+	}{
+		{
+			name: "no creations and no updates",
+			args: args{
+				nil,
+				nil,
+			},
+			numberOfCreations:             0,
+			numberOfRefreshes:             0,
+			finalNumberOfWebhookAccessors: 0,
+		},
+		{
+			name: "create configurations and no updates",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook1"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook1.1"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook2"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook2.1"}},
+					},
+				},
+				nil,
+			},
+			numberOfCreations:             2,
+			numberOfRefreshes:             0,
+			finalNumberOfWebhookAccessors: 2,
+		},
+		{
+			name: "create configurations and update some of them",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook3"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook3.1"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook4"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook4.1"}},
+					},
+				},
+				[]*v1.ValidatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook3"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook3.1-updated"}},
+					},
+				},
+			},
+			numberOfCreations:             2,
+			numberOfRefreshes:             1,
+			finalNumberOfWebhookAccessors: 2,
+		},
+		{
+			name: "create configuration and update moar of them",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook5"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook5.1"}, {Name: "webhook5.2"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook6"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook6.1"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook7.1"}, {Name: "webhook7.1"}},
+					},
+				},
+				[]*v1.ValidatingWebhookConfiguration{
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook5"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook5.1-updated"}},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook7.1-updated"}, {Name: "webhook7.2-updated"}, {Name: "webhook7.3"}},
+					},
+				},
+			},
+			numberOfCreations:             5,
+			numberOfRefreshes:             4,
+			finalNumberOfWebhookAccessors: 5,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fake.NewSimpleClientset()
+			informerFactory := informers.NewSharedInformerFactory(client, 0)
+			stop := make(chan struct{})
+			defer close(stop)
+			manager := NewValidatingWebhookConfigurationManager(informerFactory)
+			managerStructPtr := manager.(*validatingWebhookConfigurationManager)
+			fakeWebhookAccessorCreator := &mockCreateValidatingWebhookAccessor{}
+			managerStructPtr.createValidatingWebhookAccessor = fakeWebhookAccessorCreator.fn
+			informerFactory.Start(stop)
+			informerFactory.WaitForCacheSync(stop)
+
+			// Create webhooks
+			for _, configurations := range tt.args.createWebhookConfigurations {
+				client.
+					AdmissionregistrationV1().
+					ValidatingWebhookConfigurations().
+					Create(context.TODO(), configurations, metav1.CreateOptions{})
+			}
+			// TODO use channels to wait for manager.createValidatingWebhookAccessor
+			// to be called instead of using time.Sleep
+			time.Sleep(1 * time.Second)
+			webhooks := manager.Webhooks()
+			if configurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
+				t.Errorf("Expected number of webhooks %d received %d",
+					configurationTotalWebhooks(tt.args.createWebhookConfigurations),
+					len(webhooks),
+				)
+			}
+			// assert creations
+			if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
+				t.Errorf(
+					"Expected number of creations %d received %d",
+					tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
+				)
+			}
+
+			// reset mock counter
+			fakeWebhookAccessorCreator.resetCounter()
+
+			// Update webhooks
+			for _, configurations := range tt.args.updateWebhookConfigurations {
+				client.
+					AdmissionregistrationV1().
+					ValidatingWebhookConfigurations().
+					Update(context.TODO(), configurations, metav1.UpdateOptions{})
+			}
+			// TODO use channels to wait for manager.createValidatingWebhookAccessor
+			// to be called instead of using time.Sleep
+			time.Sleep(1 * time.Second)
+			webhooks = manager.Webhooks()
+			if tt.finalNumberOfWebhookAccessors != len(webhooks) {
+				t.Errorf("Expected final number of webhooks %d received %d",
+					tt.finalNumberOfWebhookAccessors,
+					len(webhooks),
+				)
+			}
+
+			// assert updates
+			if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
+				t.Errorf(
+					"Expected number of refreshes %d received %d",
+					tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
+				)
+			}
+			// reset mock counter for the next test cases
+			fakeWebhookAccessorCreator.resetCounter()
+		})
+	}
+}

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

@@ -123,7 +123,6 @@ func (m *mutatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.Clien
 	return m.client, m.clientErr
 }
 
-// TODO: graduation to beta: resolve the fact that we rebuild ALL items whenever ANY config changes in NewMutatingWebhookConfigurationManager and NewValidatingWebhookConfigurationManager ... now that we're doing CEL compilation, we probably want to avoid that
 func (m *mutatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompiler) matchconditions.Matcher {
 	m.compileMatcher.Do(func() {
 		expressions := make([]cel.ExpressionAccessor, len(m.MutatingWebhook.MatchConditions))