Skip to content

Commit

Permalink
Added webhook-annotations flag to VPA admission-controller
Browse files Browse the repository at this point in the history
Signed-off-by: Willi Carlsen <[email protected]>
  • Loading branch information
wcarlsen committed Nov 7, 2024
1 parent 9c91496 commit 3cabd17
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 36 deletions.
1 change: 1 addition & 0 deletions vertical-pod-autoscaler/pkg/admission-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 18 additions & 11 deletions vertical-pod-autoscaler/pkg/admission-controller/config.go
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
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
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

0 comments on commit 3cabd17

Please sign in to comment.