From 408cf7b500439d30a7438d1bbea70c345dc41c6b Mon Sep 17 00:00:00 2001 From: Amine Date: Wed, 12 Jul 2023 16:20:14 +0100 Subject: [PATCH] Improve naming and code comments Kubernetes-commit: 0695853a3061ece0f602c1f267c82ced3f8c880d --- .../configuration/mutating_webhook_manager.go | 11 ++++++++--- .../configuration/mutating_webhook_manager_test.go | 4 ++-- .../configuration/validating_webhook_manager.go | 9 +++++++-- .../configuration/validating_webhook_manager_test.go | 4 ++-- pkg/admission/plugin/webhook/accessors.go | 1 - 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pkg/admission/configuration/mutating_webhook_manager.go b/pkg/admission/configuration/mutating_webhook_manager.go index 851490c66..a1b84780f 100644 --- a/pkg/admission/configuration/mutating_webhook_manager.go +++ b/pkg/admission/configuration/mutating_webhook_manager.go @@ -21,7 +21,7 @@ import ( "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" @@ -106,10 +106,15 @@ func (m *mutatingWebhookConfigurationManager) getConfiguration() ([]webhook.Webh if err != nil { return []webhook.WebhookAccessor{}, err } - return m.smartReloadMutatingWebhookConfigurations(configurations), nil + return m.getMutatingWebhookConfigurations(configurations), nil } -func (m *mutatingWebhookConfigurationManager) smartReloadMutatingWebhookConfigurations(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. diff --git a/pkg/admission/configuration/mutating_webhook_manager_test.go b/pkg/admission/configuration/mutating_webhook_manager_test.go index 8f9553f78..801c4ecaa 100644 --- a/pkg/admission/configuration/mutating_webhook_manager_test.go +++ b/pkg/admission/configuration/mutating_webhook_manager_test.go @@ -118,8 +118,8 @@ func TestGetMutatingWebhookConfigSmartReload(t *testing.T) { 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. + // number of refreshes are number of times we recrated a webhook accessor + // instead of pulling from the cache. numberOfRefreshes int finalNumberOfWebhookAccessors int }{ diff --git a/pkg/admission/configuration/validating_webhook_manager.go b/pkg/admission/configuration/validating_webhook_manager.go index d57ce7121..d9c11a975 100644 --- a/pkg/admission/configuration/validating_webhook_manager.go +++ b/pkg/admission/configuration/validating_webhook_manager.go @@ -106,10 +106,15 @@ func (v *validatingWebhookConfigurationManager) getConfiguration() ([]webhook.We if err != nil { return []webhook.WebhookAccessor{}, err } - return v.smartReloadValidatingWebhookConfigurations(configurations), nil + return v.getValidatingWebhookConfigurations(configurations), nil } -func (v *validatingWebhookConfigurationManager) smartReloadValidatingWebhookConfigurations(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{} for _, c := range configurations { diff --git a/pkg/admission/configuration/validating_webhook_manager_test.go b/pkg/admission/configuration/validating_webhook_manager_test.go index 18493a54f..2f18f386b 100644 --- a/pkg/admission/configuration/validating_webhook_manager_test.go +++ b/pkg/admission/configuration/validating_webhook_manager_test.go @@ -118,8 +118,8 @@ func TestGetValidatingWebhookConfigSmartReload(t *testing.T) { 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. + // number of refreshes are number of times we recrated a webhook accessor + // instead of pulling from the cache. numberOfRefreshes int finalNumberOfWebhookAccessors int }{ diff --git a/pkg/admission/plugin/webhook/accessors.go b/pkg/admission/plugin/webhook/accessors.go index 6c48418de..88d8120f5 100644 --- a/pkg/admission/plugin/webhook/accessors.go +++ b/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))