diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go index 34780bf1b22..cde2a577b13 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go +++ b/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 } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go index dd28805f689..e90b79ddaaf 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go +++ b/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() + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/webhook_cache.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/webhook_cache.go new file mode 100644 index 00000000000..49abac4d91d --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/webhook_cache.go @@ -0,0 +1 @@ +package configuration