From 279004da515ee7655a024f538a3311a56b984c05 Mon Sep 17 00:00:00 2001 From: Varsha Varadarajan Date: Tue, 2 Jul 2019 17:39:26 -0400 Subject: [PATCH] Check if webhook config references a service before throwing an error --- checks.md | 8 ++-- checks/diagnostic.go | 2 +- checks/doks/admission_controller_webhook.go | 31 ++++++++++--- .../doks/admission_controller_webhook_test.go | 45 +++++++++++++++++++ 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/checks.md b/checks.md index 4847b82..b396616 100644 --- a/checks.md +++ b/checks.md @@ -194,9 +194,9 @@ Example: apiVersion: admissionregistration.k8s.io/v1beta1 kind: ValidatingWebhookConfiguration metadata: - name: sample-webhook.adamwg.com + name: sample-webhook.example.com webhooks: -- name: sample-webhook.adamwg.com +- name: sample-webhook.example.com rules: - apiGroups: - "" @@ -226,9 +226,9 @@ How to fix: apiVersion: admissionregistration.k8s.io/v1beta1 kind: ValidatingWebhookConfiguration metadata: - name: sample-webhook.adamwg.com + name: sample-webhook.example.com webhooks: -- name: sample-webhook.adamwg.com +- name: sample-webhook.example.com rules: - apiGroups: - "" diff --git a/checks/diagnostic.go b/checks/diagnostic.go index 87951ad..b036979 100644 --- a/checks/diagnostic.go +++ b/checks/diagnostic.go @@ -67,6 +67,6 @@ const ( PersistentVolume Kind = "persistent volume" // ValidatingWebhookConfiguration identifies Kubernetes objects of kind `validating webhook configuration` ValidatingWebhookConfiguration Kind = "validating webhook configuration" - // MutatingWebhookConfiguration identifies Kubernetes objects of kind `validating webhook configuration` + // MutatingWebhookConfiguration identifies Kubernetes objects of kind `mutating webhook configuration` MutatingWebhookConfiguration Kind = "mutating webhook configuration" ) diff --git a/checks/doks/admission_controller_webhook.go b/checks/doks/admission_controller_webhook.go index cee47dc..32a1f1e 100644 --- a/checks/doks/admission_controller_webhook.go +++ b/checks/doks/admission_controller_webhook.go @@ -1,3 +1,19 @@ +/* +Copyright 2019 DigitalOcean + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package doks import ( @@ -38,7 +54,9 @@ func (w *webhookCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { for _, config := range objects.ValidatingWebhookConfigurations.Items { for _, validatingWebhook := range config.Webhooks { - if *validatingWebhook.FailurePolicy == ar.Fail && doesSelectorIncludeKubeSystem(validatingWebhook.NamespaceSelector, objects.SystemNamespace) { + if *validatingWebhook.FailurePolicy == ar.Fail && + validatingWebhook.ClientConfig.Service != nil && + selectorMatchesNamespace(validatingWebhook.NamespaceSelector, objects.SystemNamespace) { d := checks.Diagnostic{ Severity: checks.Error, Message: "Webhook matches objects in the kube-system namespace. This can cause problems when upgrading the cluster.", @@ -53,7 +71,9 @@ func (w *webhookCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { for _, config := range objects.MutatingWebhookConfigurations.Items { for _, mutatingWebhook := range config.Webhooks { - if *mutatingWebhook.FailurePolicy == ar.Fail && doesSelectorIncludeKubeSystem(mutatingWebhook.NamespaceSelector, objects.SystemNamespace) { + if *mutatingWebhook.FailurePolicy == ar.Fail && + mutatingWebhook.ClientConfig.Service != nil && + selectorMatchesNamespace(mutatingWebhook.NamespaceSelector, objects.SystemNamespace) { d := checks.Diagnostic{ Severity: checks.Error, Message: "Webhook matches objects in the kube-system namespace. This can cause problems when upgrading the cluster.", @@ -68,16 +88,15 @@ func (w *webhookCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { return diagnostics, nil } -func doesSelectorIncludeKubeSystem(selector *metav1.LabelSelector, namespace *corev1.Namespace) bool { +func selectorMatchesNamespace(selector *metav1.LabelSelector, namespace *corev1.Namespace) bool { if selector.Size() == 0 { return true } labels := namespace.GetLabels() for key, value := range selector.MatchLabels { - if v, ok := labels[key]; ok && v == value { - continue + if v, ok := labels[key]; !ok || v != value { + return false } - return false } for _, lbr := range selector.MatchExpressions { if !match(labels, lbr) { diff --git a/checks/doks/admission_controller_webhook_test.go b/checks/doks/admission_controller_webhook_test.go index bd4da0e..32cde42 100644 --- a/checks/doks/admission_controller_webhook_test.go +++ b/checks/doks/admission_controller_webhook_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2019 DigitalOcean + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package doks import ( @@ -45,6 +61,11 @@ func TestWebhookError(t *testing.T) { objs: initObjects(ar.Ignore), expected: nil, }, + { + name: "webook does not use service", + objs: webhookURL(), + expected: nil, + }, { name: "namespace selector is empty", objs: initObjects(ar.Fail), @@ -133,6 +154,12 @@ func initObjects(failurePolicyType ar.FailurePolicyType) *kube.Objects { Name: "mw_foo", FailurePolicy: &failurePolicyType, NamespaceSelector: &metav1.LabelSelector{}, + ClientConfig: ar.WebhookClientConfig{ + Service: &ar.ServiceReference{ + Name: "some-svc", + Namespace: "k8s", + }, + }, }, }, }, @@ -150,6 +177,12 @@ func initObjects(failurePolicyType ar.FailurePolicyType) *kube.Objects { Name: "vw_foo", FailurePolicy: &failurePolicyType, NamespaceSelector: &metav1.LabelSelector{}, + ClientConfig: ar.WebhookClientConfig{ + Service: &ar.ServiceReference{ + Name: "some-svc", + Namespace: "k8s", + }, + }, }, }, }, @@ -159,6 +192,18 @@ func initObjects(failurePolicyType ar.FailurePolicyType) *kube.Objects { return objs } +func webhookURL() *kube.Objects { + var url = "https://example.com/webhook/action" + objs := initObjects(ar.Fail) + objs.ValidatingWebhookConfigurations.Items[0].Webhooks[0].ClientConfig = ar.WebhookClientConfig{ + URL: &url, + } + objs.MutatingWebhookConfigurations.Items[0].Webhooks[0].ClientConfig = ar.WebhookClientConfig{ + URL: &url, + } + return objs +} + func label(label map[string]string) *kube.Objects { objs := initObjects(ar.Fail) objs.ValidatingWebhookConfigurations.Items[0].Webhooks[0].NamespaceSelector = &metav1.LabelSelector{