From 3cabd170feb3d2ca7c112bb9b62551f810a4e933 Mon Sep 17 00:00:00 2001 From: Willi Carlsen Date: Thu, 7 Nov 2024 09:27:10 +0100 Subject: [PATCH] Added webhook-annotations flag to VPA admission-controller Signed-off-by: Willi Carlsen --- .../pkg/admission-controller/README.md | 1 + .../pkg/admission-controller/config.go | 29 +++++++---- .../pkg/admission-controller/config_test.go | 50 ++++++++++--------- .../pkg/admission-controller/main.go | 3 +- 4 files changed, 47 insertions(+), 36 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/README.md b/vertical-pod-autoscaler/pkg/admission-controller/README.md index c7eb50000325..07b51bb8e087 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/README.md +++ b/vertical-pod-autoscaler/pkg/admission-controller/README.md @@ -35,6 +35,7 @@ up the changes: ```sudo systemctl restart kubelet.service``` 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. +1. You can specify a comma separated list to set webhook annotations with `--webhook-annotations`, 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 e0d8ac0805f1..a5936089a3ff 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, webHookLabels string) { +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, webhookAnnotations string) { time.Sleep(webHookDelay) client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations() _, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -141,15 +141,22 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela }, } } - webhookLabelsMap, err := convertLabelsToMap(webHookLabels) + webhookLabelsMap, err := convertLabelsOrAnnotationsToMap(webHookLabels) if err != nil { klog.ErrorS(err, "Unable to parse webhook labels") webhookLabelsMap = map[string]string{} } + + webhookAnnotationsMap, err := convertLabelsOrAnnotationsToMap(webhookAnnotations) + if err != nil { + klog.ErrorS(err, "Unable to parse webhook annotations") + webhookLabelsMap = map[string]string{} + } webhookConfig := &admissionregistration.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ - Name: webhookConfigName, - Labels: webhookLabelsMap, + Name: webhookConfigName, + Labels: webhookLabelsMap, + Annotations: webhookAnnotationsMap, }, Webhooks: []admissionregistration.MutatingWebhook{ { @@ -188,24 +195,24 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela } } -// convertLabelsToMap convert the labels from string to map +// convertLabelsOrAnnotationsToMap convert the labels or annotations 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) { +func convertLabelsOrAnnotationsToMap(kv string) (map[string]string, error) { m := make(map[string]string) - if labels == "" { + if kv == "" { return m, nil } - labels = strings.Trim(labels, "\"") - s := strings.Split(labels, ",") + kv = strings.Trim(kv, "\"") + s := strings.Split(kv, ",") 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) + return map[string]string{}, fmt.Errorf("labels or annotations '%s' are invalid, the format should be: 'key1:value1,key2:value2'", kv) } 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) + return map[string]string{}, fmt.Errorf("labels or annotations '%s' are invalid, the format should be: 'key1:value1,key2:value2'", kv) } value := strings.TrimSpace(kv[1]) m[key] = value diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go index b7b3ef396abf..4cc7c3049792 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go @@ -40,7 +40,7 @@ func TestSelfRegistrationBase(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "key1:value1,key2:value2") + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "key1:value1,key2:value2", "key3:value3,key4:value4") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -48,6 +48,7 @@ func TestSelfRegistrationBase(t *testing.T) { 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.Equal(t, webhookConfig.Annotations, map[string]string{"key3": "value3", "key4": "value4"}, "expected webhook configuration annotations to match") assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration") webhook := webhookConfig.Webhooks[0] @@ -84,7 +85,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{}) @@ -112,7 +113,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{}) @@ -142,7 +143,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{}) @@ -173,7 +174,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{}) @@ -205,7 +206,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{}) @@ -232,7 +233,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{}) @@ -259,7 +260,7 @@ func TestSelfRegistrationWithInvalidLabels(t *testing.T) { selectedNamespace := "" ignoredNamespaces := []string{} - selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "foo,bar") + selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false, "foo,bar", "bar,foo") webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -267,6 +268,7 @@ func TestSelfRegistrationWithInvalidLabels(t *testing.T) { 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{}, "expected invalid webhook configuration labels to match") + assert.Equal(t, webhookConfig.Annotations, map[string]string{}, "expected invalid webhook configuration annotations to match") assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration") webhook := webhookConfig.Webhooks[0] @@ -290,30 +292,30 @@ func TestSelfRegistrationWithInvalidLabels(t *testing.T) { assert.Equal(t, timeoutSeconds, *webhook.TimeoutSeconds, "expected timeout seconds to match") } -func TestConvertLabelsToMap(t *testing.T) { +func TestConvertLabelsOrAnnotationsToMap(t *testing.T) { testCases := []struct { desc string - labels string + kv string expectedOutput map[string]string expectedError bool }{ { desc: "should return empty map when tag is empty", - labels: "", + kv: "", expectedOutput: map[string]string{}, expectedError: false, }, { - desc: "single valid tag should be converted", - labels: "key:value", + desc: "single valid tag should be converted", + kv: "key:value", expectedOutput: map[string]string{ "key": "value", }, expectedError: false, }, { - desc: "multiple valid labels should be converted", - labels: "key1:value1,key2:value2", + desc: "multiple valid labels or annotations should be converted", + kv: "key1:value1,key2:value2", expectedOutput: map[string]string{ "key1": "value1", "key2": "value2", @@ -321,8 +323,8 @@ func TestConvertLabelsToMap(t *testing.T) { expectedError: false, }, { - desc: "whitespaces should be trimmed", - labels: "key1:value1, key2:value2", + desc: "whitespaces should be trimmed", + kv: "key1:value1, key2:value2", expectedOutput: map[string]string{ "key1": "value1", "key2": "value2", @@ -330,8 +332,8 @@ func TestConvertLabelsToMap(t *testing.T) { expectedError: false, }, { - desc: "whitespaces between keys and values should be trimmed", - labels: "key1 : value1,key2 : value2", + desc: "whitespaces between keys and values should be trimmed", + kv: "key1 : value1,key2 : value2", expectedOutput: map[string]string{ "key1": "value1", "key2": "value2", @@ -340,19 +342,19 @@ func TestConvertLabelsToMap(t *testing.T) { }, { desc: "should return error for invalid format", - labels: "foo,bar", + kv: "foo,bar", expectedOutput: nil, expectedError: true, }, { desc: "should return error for when key is missed", - labels: "key1:value1,:bar", + kv: "key1:value1,:bar", expectedOutput: nil, expectedError: true, }, { - desc: "should strip additional quotes", - labels: "\"key1:value1,key2:value2\"", + desc: "should strip additional quotes", + kv: "\"key1:value1,key2:value2\"", expectedOutput: map[string]string{ "key1": "value1", "key2": "value2", @@ -362,7 +364,7 @@ func TestConvertLabelsToMap(t *testing.T) { } for i, c := range testCases { - m, err := convertLabelsToMap(c.labels) + m, err := convertLabelsOrAnnotationsToMap(c.kv) if c.expectedError { assert.NotNil(t, err, "TestCase[%d]: %s", i, c.desc) } else { diff --git a/vertical-pod-autoscaler/pkg/admission-controller/main.go b/vertical-pod-autoscaler/pkg/admission-controller/main.go index 08aca96f0882..e8fb246219d4 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/main.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/main.go @@ -74,6 +74,7 @@ var ( webHookFailurePolicy = flag.Bool("webhook-failure-policy-fail", false, "If set to true, will configure the admission webhook failurePolicy to \"Fail\". Use with caution.") 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.") webhookLabels = flag.String("webhook-labels", "", "Comma separated list of labels to add to the webhook object. Format: key1:value1,key2:value2") + webhookAnnotations = flag.String("webhook-annotations", "", "Comma separated list of annotations to add to the webhook object. Format: key1:value1,key2:value2") 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") ) @@ -144,7 +145,7 @@ func main() { ignoredNamespaces := strings.Split(commonFlags.IgnoredVpaObjectNamespaces, ",") go func() { if *registerWebhook { - selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), commonFlags.VpaObjectNamespace, ignoredNamespaces, *webHookFailurePolicy, *webhookLabels) + selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), commonFlags.VpaObjectNamespace, ignoredNamespaces, *webHookFailurePolicy, *webhookLabels, *webhookAnnotations) } // Start status updates after the webhook is initialized. statusUpdater.Run(stopCh)