From 8d2ae78027e527bee4995948b0e26f7d236593bc Mon Sep 17 00:00:00 2001 From: umagnus Date: Wed, 16 Oct 2024 08:29:26 +0000 Subject: [PATCH] add webhook labels flag to add labels in webhook --- .../pkg/admission-controller/README.md | 1 + .../pkg/admission-controller/config.go | 36 ++++- .../pkg/admission-controller/config_test.go | 141 +++++++++++++++++- .../pkg/admission-controller/main.go | 3 +- 4 files changed, 171 insertions(+), 10 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/README.md b/vertical-pod-autoscaler/pkg/admission-controller/README.md index 9e8b67c5b938..c7eb50000325 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/README.md +++ b/vertical-pod-autoscaler/pkg/admission-controller/README.md @@ -34,6 +34,7 @@ up the changes: ```sudo systemctl restart kubelet.service``` by setting `--register-by-url=true` and passing `--webhook-address` and `--webhook-port`. 1. You can specify a minimum TLS version with `--min-tls-version` with acceptable values being `tls1_2` (default), or `tls1_3`. 1. You can also specify a comma or colon separated list of ciphers for the server to use with `--tls-ciphers` if `--min-tls-version` is set to `tls1_2`. +1. You can specify a comma separated list to set webhook labels with `--webhook-labels`, example format: key1:value1,key2:value2. ## Implementation diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config.go b/vertical-pod-autoscaler/pkg/admission-controller/config.go index 1540ecd5830f..7f24cca4f6d9 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/config.go @@ -90,7 +90,7 @@ func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struc // register this webhook admission controller with the kube-apiserver // by creating MutatingWebhookConfiguration. -func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDelay time.Duration, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespace string, ignoredNamespaces []string, webHookFailurePolicy bool) { +func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDelay time.Duration, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespace string, ignoredNamespaces []string, webHookFailurePolicy bool, webHookLabels string) { time.Sleep(webHookDelay) client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations() _, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -141,9 +141,15 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela }, } } + webhookLabelsMap, err := convertLabelsToMap(webHookLabels) + if err != nil { + klog.Warningf("Unable to parse webhook labels. Creating webhook without labels: %v\n", err) + webhookLabelsMap = map[string]string{} + } webhookConfig := &admissionregistration.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ - Name: webhookConfigName, + Name: webhookConfigName, + Labels: webhookLabelsMap, }, Webhooks: []admissionregistration.MutatingWebhook{ { @@ -181,3 +187,29 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela klog.V(3).Info("Self registration as MutatingWebhook succeeded.") } } + +// convertLabelsToMap convert the labels from string to map +// the valid labels format is "key1:value1,key2:value2", which could be converted to +// {"key1": "value1", "key2": "value2"} +func convertLabelsToMap(labels string) (map[string]string, error) { + m := make(map[string]string) + if labels == "" { + return m, nil + } + labels = strings.Trim(labels, "\"") + s := strings.Split(labels, ",") + for _, tag := range s { + kv := strings.SplitN(tag, ":", 2) + if len(kv) != 2 { + return map[string]string{}, fmt.Errorf("labels '%s' are invalid, the format should be: 'key1:value1,key2:value2'", labels) + } + key := strings.TrimSpace(kv[0]) + if key == "" { + return map[string]string{}, fmt.Errorf("labels '%s' are invalid, the format should be: 'key1:value1,key2:value2'", labels) + } + value := strings.TrimSpace(kv[1]) + m[key] = value + } + + return m, nil +} diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go index be1ed99b6b1f..fc4da6098eb2 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go @@ -40,13 +40,14 @@ func TestSelfRegistrationBase(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "key1:value1,key2:value2") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) assert.NoError(t, err, "expected no error fetching webhook configuration") assert.Equal(t, webhookConfigName, webhookConfig.Name, "expected webhook configuration name to match") + assert.Equal(t, webhookConfig.Labels, map[string]string{"key1": "value1", "key2": "value2"}, "expected webhook configuration labels to match") assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration") webhook := webhookConfig.Webhooks[0] @@ -83,7 +84,7 @@ func TestSelfRegistrationWithURL(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -111,7 +112,7 @@ func TestSelfRegistrationWithOutURL(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -141,7 +142,7 @@ func TestSelfRegistrationWithIgnoredNamespaces(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{"test"} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -172,7 +173,7 @@ func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) { selectedNamespace := "test" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -204,7 +205,7 @@ func TestSelfRegistrationWithFailurePolicy(t *testing.T) { selectedNamespace := "test" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, true) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, true, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -231,7 +232,7 @@ func TestSelfRegistrationWithOutFailurePolicy(t *testing.T) { selectedNamespace := "test" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false) + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -244,3 +245,129 @@ func TestSelfRegistrationWithOutFailurePolicy(t *testing.T) { assert.NotNil(t, *webhook.FailurePolicy, "expected namespace selector not to be nil") assert.Equal(t, *webhook.FailurePolicy, admissionregistration.Ignore, "expected failurePolicy to be Ignore") } + +func TestSelfRegistrationWithInvalidLabels(t *testing.T) { + + testClientSet := fake.NewSimpleClientset() + caCert := []byte("fake") + webHookDelay := 0 * time.Second + namespace := "default" + serviceName := "vpa-service" + url := "http://example.com/" + registerByURL := true + timeoutSeconds := int32(32) + selectedNamespace := "" + ignoredNamespaces := []string{} + + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "foo,bar") + + webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() + webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) + + assert.NoError(t, err, "expected invalid labels error fetching webhook configuration") + assert.Equal(t, webhookConfigName, webhookConfig.Name, "expected webhook configuration name to match") + assert.Equal(t, webhookConfig.Labels, map[string]string{}, "expected invalid webhook configuration labels to match") + + assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration") + webhook := webhookConfig.Webhooks[0] + assert.Equal(t, "vpa.k8s.io", webhook.Name, "expected webhook name to match") + + PodRule := webhook.Rules[0] + assert.Equal(t, []admissionregistration.OperationType{admissionregistration.Create}, PodRule.Operations, "expected operations to match") + assert.Equal(t, []string{""}, PodRule.APIGroups, "expected API groups to match") + assert.Equal(t, []string{"v1"}, PodRule.APIVersions, "expected API versions to match") + assert.Equal(t, []string{"pods"}, PodRule.Resources, "expected resources to match") + + VPARule := webhook.Rules[1] + assert.Equal(t, []admissionregistration.OperationType{admissionregistration.Create, admissionregistration.Update}, VPARule.Operations, "expected operations to match") + assert.Equal(t, []string{"autoscaling.k8s.io"}, VPARule.APIGroups, "expected API groups to match") + assert.Equal(t, []string{"*"}, VPARule.APIVersions, "ehook.Rulxpected API versions to match") + assert.Equal(t, []string{"verticalpodautoscalers"}, VPARule.Resources, "expected resources to match") + + assert.Equal(t, admissionregistration.SideEffectClassNone, *webhook.SideEffects, "expected side effects to match") + assert.Equal(t, admissionregistration.Ignore, *webhook.FailurePolicy, "expected failure policy to match") + assert.Equal(t, caCert, webhook.ClientConfig.CABundle, "expected CA bundle to match") + assert.Equal(t, timeoutSeconds, *webhook.TimeoutSeconds, "expected timeout seconds to match") +} + +func TestConvertLabelsToMap(t *testing.T) { + testCases := []struct { + desc string + labels string + expectedOutput map[string]string + expectedError bool + }{ + { + desc: "should return empty map when tag is empty", + labels: "", + expectedOutput: map[string]string{}, + expectedError: false, + }, + { + desc: "single valid tag should be converted", + labels: "key:value", + expectedOutput: map[string]string{ + "key": "value", + }, + expectedError: false, + }, + { + desc: "multiple valid labels should be converted", + labels: "key1:value1,key2:value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + desc: "whitespaces should be trimmed", + labels: "key1:value1, key2:value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + desc: "whitespaces between keys and values should be trimmed", + labels: "key1 : value1,key2 : value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + desc: "should return error for invalid format", + labels: "foo,bar", + expectedOutput: nil, + expectedError: true, + }, + { + desc: "should return error for when key is missed", + labels: "key1:value1,:bar", + expectedOutput: nil, + expectedError: true, + }, + { + desc: "should strip additional quotes", + labels: "\"key1:value1,key2:value2\"", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + } + + for i, c := range testCases { + m, err := convertLabelsToMap(c.labels) + if c.expectedError { + assert.NotNil(t, err, "TestCase[%d]: %s", i, c.desc) + } else { + assert.Nil(t, err, "TestCase[%d]: %s", i, c.desc) + assert.Equal(t, m, c.expectedOutput, "expected labels map") + } + } +} diff --git a/vertical-pod-autoscaler/pkg/admission-controller/main.go b/vertical-pod-autoscaler/pkg/admission-controller/main.go index f9d4d824eb14..2d8b89521578 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/main.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/main.go @@ -78,6 +78,7 @@ var ( webhookPort = flag.String("webhook-port", "", "Server Port for Webhook") webhookTimeout = flag.Int("webhook-timeout-seconds", 30, "Timeout in seconds that the API server should wait for this webhook to respond before failing.") webHookFailurePolicy = flag.Bool("webhook-failure-policy-fail", false, "If set to true, will configure the admission webhook failurePolicy to \"Fail\". Use with caution.") + webhookLabels = flag.String("webhook-labels", "", "Comma separated list of labels to add to the webhook object. Format: key1:value1,key2:value2") registerWebhook = flag.Bool("register-webhook", true, "If set to true, admission webhook object will be created on start up to register with the API server.") registerByURL = flag.Bool("register-by-url", false, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name") vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used. Must not be used if ignored-vpa-object-namespaces is set.") @@ -149,7 +150,7 @@ func main() { ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",") go func() { if *registerWebhook { - selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, ignoredNamespaces, *webHookFailurePolicy) + selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, ignoredNamespaces, *webHookFailurePolicy, *webhookLabels) } // Start status updates after the webhook is initialized. statusUpdater.Run(stopCh)