Add webhookAccessors smart reloads unit tests

This patch adds few unit tests to assert that the webhook accessors are
only recreate when they are update in the api-server.

In order to test this feature we had to make few changes to wb manager
that allows us to mock `NewValidatingWebhookAccessor` external function.

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

@@ -32,12 +32,19 @@ import (
 	"k8s.io/client-go/tools/cache/synctrack"
 )
 
+type webhookAccessorCreator 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]
-	configurationsCache sync.Map // NOTE maybe use simple map and a sync.Mutex
+	configurationsCache sync.Map
+
+	// createValidatingWebhookAccessor is used to instantiate webhook accessors.
+	// This function is defined as field instead of a struct method to allow us
+	// to replace it with a mock during unit tests.
+	createValidatingWebhookAccessor webhookAccessorCreator
 }
 
 var _ generic.Source = &validatingWebhookConfigurationManager{}
@@ -45,7 +52,8 @@ 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
 
@@ -115,12 +123,13 @@ cfgLoop:
 			n := c.Webhooks[i].Name
 			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
 			names[n]++
-			configurationAccessors = append(configurationAccessors, webhook.NewValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
+			configurationAccessors = append(configurationAccessors, v.createValidatingWebhookAccessor(uid, c.Name, &c.Webhooks[i]))
 			// Cache the new accessors
 			v.configurationsCache.Store(c.Name, configurationAccessors)
 			accessors = append(accessors, 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,176 @@ 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 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 pulled a webhook configuration
+		// from the cache without having to create new ones from scratch.
+		numberOfRefreshes int
+	}{
+		{
+			name: "no creations and no updates",
+			args: args{
+				nil,
+				nil,
+			},
+			numberOfCreations: 0,
+			numberOfRefreshes: 0,
+		},
+		{
+			name: "create configurations and no updates",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook1"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook1.1"}},
+					},
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook2"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook2.1"}},
+					},
+				},
+				nil,
+			},
+			numberOfCreations: 2,
+			numberOfRefreshes: 0,
+		},
+		{
+			name: "create configuration and update some of them",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook3"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook3.1"}},
+					},
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook4"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook4.1"}},
+					},
+				},
+				[]*v1.ValidatingWebhookConfiguration{
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook3"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook3.1-updated"}},
+					},
+				},
+			},
+			numberOfCreations: 2,
+			numberOfRefreshes: 1,
+		},
+		{
+			name: "create configuration and update moar of them",
+			args: args{
+				[]*v1.ValidatingWebhookConfiguration{
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook5"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook5.1"}, {Name: "webhook5.2"}},
+					},
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook6"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook6.1"}},
+					},
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook7.1"}, {Name: "webhook7.1"}},
+					},
+				},
+				[]*v1.ValidatingWebhookConfiguration{
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook5"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook5.1-updated"}},
+					},
+					&v1.ValidatingWebhookConfiguration{
+						ObjectMeta: metav1.ObjectMeta{Name: "webhook7"},
+						Webhooks:   []v1.ValidatingWebhook{{Name: "webhook7.1-updated"}, {Name: "webhook7.2-updated"}},
+					},
+				},
+			},
+			numberOfCreations: 5,
+			numberOfRefreshes: 3,
+		},
+	}
+
+	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)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// 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)
+			_ = manager.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)
+			_ = manager.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/webhook_cache.go

@@ -0,0 +1 @@
+package configuration