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.

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

feat: add webhook labels flag to add labels in VPA admission controller webhook #7402

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/pkg/admission-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 34 additions & 2 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) {
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{})
Expand Down Expand Up @@ -141,9 +141,15 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela
},
}
}
webhookLabelsMap, err := convertLabelsToMap(webHookLabels)
if err != nil {
klog.ErrorS(err, "Unable to parse webhook labels")
webhookLabelsMap = map[string]string{}
}
webhookConfig := &admissionregistration.MutatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: webhookConfigName,
Name: webhookConfigName,
Labels: webhookLabelsMap,
},
Webhooks: []admissionregistration.MutatingWebhook{
{
Expand Down Expand Up @@ -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
}
141 changes: 134 additions & 7 deletions vertical-pod-autoscaler/pkg/admission-controller/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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{})
Expand All @@ -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{})
Expand All @@ -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 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.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",
umagnus marked this conversation as resolved.
Show resolved Hide resolved
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,
},
umagnus marked this conversation as resolved.
Show resolved Hide resolved
{
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")
}
}
}
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 @@ -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.")
Expand Down Expand Up @@ -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)
Expand Down
Loading