Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added webhook-annotations flag to VPA admission-controller #7472

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+47 −36
Open

Added webhook-annotations flag to VPA admission-controller #7472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions vertical-pod-autoscaler/docs/components.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,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 of the Admission Controller

Expand Down
29 changes: 18 additions & 11 deletions vertical-pod-autoscaler/pkg/admission-controller/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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
Expand Down
50 changes: 26 additions & 24 deletions vertical-pod-autoscaler/pkg/admission-controller/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,15 @@ 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{})

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]
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand All @@ -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{})
Expand All @@ -259,14 +260,15 @@ 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{})

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]
Expand All @@ -290,48 +292,48 @@ 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",
},
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",
},
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",
Expand All @@ -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",
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion vertical-pod-autoscaler/pkg/admission-controller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)

Expand Down Expand Up @@ -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)
Expand Down
Loading