Check if webhook config references a service before throwing an error

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:
     - ""

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"
 )

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,17 +88,16 @@ 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
 		}
+	}
 	for _, lbr := range selector.MatchExpressions {
 		if !match(labels, lbr) {
 			return false

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,11 +177,29 @@ 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",
 								},
 							},
 						},
 					},
 				},
+			},
+		},
+	}
+	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
 }