Admission webhook check: Add a doks specific error is webhook applies to objects in kube-system namespace

checks/diagnostic.go

@@ -65,4 +65,8 @@ const (
 	ServiceAccount Kind = "service account"
 	// PersistentVolume identifies Kubernetes objects of kind `persistent volume`
 	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 Kind = "mutating webhook configuration"
 )

checks/doks/admission_controller_webhook.go

@@ -0,0 +1,123 @@
+package doks
+
+import (
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func init() {
+	checks.Register(&webhookCheck{})
+}
+
+type webhookCheck struct{}
+
+// Name returns a unique name for this check.
+func (w *webhookCheck) Name() string {
+	return "admission-controller-webhook"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (w *webhookCheck) Groups() []string {
+	return []string{"doks"}
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (w *webhookCheck) Description() string {
+	return "Check for admission controllers that could prevent managed components from starting"
+}
+
+// Run runs this check on a set of Kubernetes objects. It can return warnings
+// (low-priority problems) and errors (high-priority problems) as well as an
+// error value indicating that the check failed to run.
+func (w *webhookCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) {
+	var diagnostics []checks.Diagnostic
+
+	for _, config := range objects.ValidatingWebhookConfigurations.Items {
+		for _, validatingWebhook := range config.Webhooks {
+			if *validatingWebhook.FailurePolicy == ar.Fail && doesSelectorIncludeKubeSystem(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.",
+					Kind:     checks.ValidatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
+			}
+		}
+	}
+
+	for _, config := range objects.MutatingWebhookConfigurations.Items {
+		for _, mutatingWebhook := range config.Webhooks {
+			if *mutatingWebhook.FailurePolicy == ar.Fail && doesSelectorIncludeKubeSystem(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.",
+					Kind:     checks.MutatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
+			}
+		}
+	}
+	return diagnostics, nil
+}
+
+func doesSelectorIncludeKubeSystem(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
+		}
+		return false
+	}
+	for _, lbr := range selector.MatchExpressions {
+		if !match(labels, lbr) {
+			return false
+		}
+	}
+	return true
+}
+
+func match(labels map[string]string, lbr metav1.LabelSelectorRequirement) bool {
+	switch lbr.Operator {
+	case metav1.LabelSelectorOpExists:
+		if _, ok := labels[lbr.Key]; ok {
+			return true
+		}
+		return false
+	case metav1.LabelSelectorOpDoesNotExist:
+		if _, ok := labels[lbr.Key]; !ok {
+			return true
+		}
+		return false
+	case metav1.LabelSelectorOpIn:
+		if v, ok := labels[lbr.Key]; ok && contains(lbr.Values, v) {
+			return true
+		}
+		return false
+	case metav1.LabelSelectorOpNotIn:
+		if v, ok := labels[lbr.Key]; !ok || !contains(lbr.Values, v) {
+			return true
+		}
+		return false
+	}
+	return false
+}
+
+func contains(list []string, name string) bool {
+	for _, l := range list {
+		if l == name {
+			return true
+		}
+	}
+	return false
+}

checks/doks/admission_controller_webhook_test.go

@@ -0,0 +1,217 @@
+package doks
+
+import (
+	"testing"
+
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	"github.com/stretchr/testify/assert"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestWebhookCheckMeta(t *testing.T) {
+	webhookCheck := webhookCheck{}
+	assert.Equal(t, "admission-controller-webhook", webhookCheck.Name())
+	assert.Equal(t, []string{"doks"}, webhookCheck.Groups())
+	assert.NotEmpty(t, webhookCheck.Description())
+}
+
+func TestWebhookCheckRegistration(t *testing.T) {
+	webhookCheck := &webhookCheck{}
+	check, err := checks.Get("admission-controller-webhook")
+	assert.NoError(t, err)
+	assert.Equal(t, check, webhookCheck)
+}
+
+func TestWebhookError(t *testing.T) {
+	tests := []struct {
+		name     string
+		objs     *kube.Objects
+		expected []checks.Diagnostic
+	}{
+		{
+			name: "no webhook configurations",
+			objs: &kube.Objects{
+				MutatingWebhookConfigurations:   &ar.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations: &ar.ValidatingWebhookConfigurationList{},
+				SystemNamespace:                 &corev1.Namespace{},
+			},
+			expected: nil,
+		},
+		{
+			name:     "failure policy is ignore",
+			objs:     initObjects(ar.Ignore),
+			expected: nil,
+		},
+		{
+			name:     "namespace selector is empty",
+			objs:     initObjects(ar.Fail),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector matches label",
+			objs:     label(map[string]string{"doks_key": "bar"}),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector does not match label",
+			objs:     label(map[string]string{"non-existent-label-on-namespace": "bar"}),
+			expected: nil,
+		},
+		{
+			name:     "namespace selector matches OpExists expression",
+			objs:     expr("doks_key", []string{}, metav1.LabelSelectorOpExists),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector matches OpDoesNotExist expression",
+			objs:     expr("random", []string{}, metav1.LabelSelectorOpDoesNotExist),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector matches OpIn expression",
+			objs:     expr("doks_key", []string{"bar"}, metav1.LabelSelectorOpIn),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector matches OpNotIn expression",
+			objs:     expr("doks_key", []string{"non-existent"}, metav1.LabelSelectorOpNotIn),
+			expected: webhookErrors(),
+		},
+		{
+			name:     "namespace selector does not match OpExists expression",
+			objs:     expr("non-existent", []string{}, metav1.LabelSelectorOpExists),
+			expected: nil,
+		},
+		{
+			name:     "namespace selector does not match OpDoesNotExist expression",
+			objs:     expr("doks_key", []string{}, metav1.LabelSelectorOpDoesNotExist),
+			expected: nil,
+		},
+		{
+			name:     "namespace selector does not match OpIn expression",
+			objs:     expr("doks_key", []string{"non-existent"}, metav1.LabelSelectorOpIn),
+			expected: nil,
+		},
+		{
+			name:     "namespace selector does not match OpNotIn expression",
+			objs:     expr("doks_key", []string{"bar"}, metav1.LabelSelectorOpNotIn),
+			expected: nil,
+		},
+	}
+
+	webhookCheck := webhookCheck{}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			d, err := webhookCheck.Run(test.objs)
+			assert.NoError(t, err)
+			assert.ElementsMatch(t, test.expected, d)
+		})
+	}
+}
+
+func initObjects(failurePolicyType ar.FailurePolicyType) *kube.Objects {
+	objs := &kube.Objects{
+		SystemNamespace: &corev1.Namespace{
+			TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "kube-system",
+				Labels: map[string]string{"doks_key": "bar"}},
+		},
+		MutatingWebhookConfigurations: &ar.MutatingWebhookConfigurationList{
+			Items: []ar.MutatingWebhookConfiguration{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "MutatingWebhookConfiguration", APIVersion: "v1beta1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "mwc_foo",
+					},
+					Webhooks: []ar.MutatingWebhook{
+						{
+							Name:              "mw_foo",
+							FailurePolicy:     &failurePolicyType,
+							NamespaceSelector: &metav1.LabelSelector{},
+						},
+					},
+				},
+			},
+		},
+		ValidatingWebhookConfigurations: &ar.ValidatingWebhookConfigurationList{
+			Items: []ar.ValidatingWebhookConfiguration{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "ValidatingWebhookConfiguration", APIVersion: "v1beta1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vwc_foo",
+					},
+					Webhooks: []ar.ValidatingWebhook{
+						{
+							Name:              "vw_foo",
+							FailurePolicy:     &failurePolicyType,
+							NamespaceSelector: &metav1.LabelSelector{},
+						},
+					},
+				},
+			},
+		},
+	}
+	return objs
+}
+
+func label(label map[string]string) *kube.Objects {
+	objs := initObjects(ar.Fail)
+	objs.ValidatingWebhookConfigurations.Items[0].Webhooks[0].NamespaceSelector = &metav1.LabelSelector{
+		MatchLabels: label,
+	}
+	objs.MutatingWebhookConfigurations.Items[0].Webhooks[0].NamespaceSelector = &metav1.LabelSelector{
+		MatchLabels: label,
+	}
+	return objs
+}
+
+func expr(key string, values []string, labelOperator metav1.LabelSelectorOperator) *kube.Objects {
+	objs := initObjects(ar.Fail)
+	objs.ValidatingWebhookConfigurations.Items[0].Webhooks[0].NamespaceSelector = &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{
+			{
+				Key:      key,
+				Operator: labelOperator,
+				Values:   values,
+			},
+		},
+	}
+	objs.MutatingWebhookConfigurations.Items[0].Webhooks[0].NamespaceSelector = &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{
+			{
+				Key:      key,
+				Operator: labelOperator,
+				Values:   values,
+			},
+		},
+	}
+	return objs
+}
+
+func webhookErrors() []checks.Diagnostic {
+	objs := initObjects(ar.Fail)
+	validatingConfig := objs.ValidatingWebhookConfigurations.Items[0]
+	mutatingConfig := objs.MutatingWebhookConfigurations.Items[0]
+	diagnostics := []checks.Diagnostic{
+		{
+			Severity: checks.Error,
+			Message:  "Webhook matches objects in the kube-system namespace. This can cause problems when upgrading the cluster.",
+			Kind:     checks.ValidatingWebhookConfiguration,
+			Object:   &validatingConfig.ObjectMeta,
+			Owners:   validatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+		{
+			Severity: checks.Error,
+			Message:  "Webhook matches objects in the kube-system namespace. This can cause problems when upgrading the cluster.",
+			Kind:     checks.MutatingWebhookConfiguration,
+			Object:   &mutatingConfig.ObjectMeta,
+			Owners:   mutatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+	}
+	return diagnostics
+}

kube/objects.go

@@ -18,6 +18,7 @@ package kube
 
 import (
 	"golang.org/x/sync/errgroup"
+	ar "k8s.io/api/admissionregistration/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -36,6 +37,7 @@ type Objects struct {
 	Nodes                           *corev1.NodeList
 	PersistentVolumes               *corev1.PersistentVolumeList
 	ComponentStatuses               *corev1.ComponentStatusList
+	SystemNamespace                 *corev1.Namespace
 	Pods                            *corev1.PodList
 	PodTemplates                    *corev1.PodTemplateList
 	PersistentVolumeClaims          *corev1.PersistentVolumeClaimList
@@ -45,6 +47,8 @@ type Objects struct {
 	ServiceAccounts                 *corev1.ServiceAccountList
 	ResourceQuotas                  *corev1.ResourceQuotaList
 	LimitRanges                     *corev1.LimitRangeList
+	MutatingWebhookConfigurations   *ar.MutatingWebhookConfigurationList
+	ValidatingWebhookConfigurations *ar.ValidatingWebhookConfigurationList
 }
 
 // Client encapsulates a client for a Kubernetes cluster.
@@ -55,6 +59,7 @@ type Client struct {
 // FetchObjects returns the objects from a Kubernetes cluster.
 func (c *Client) FetchObjects() (*Objects, error) {
 	client := c.kubeClient.CoreV1()
+	admissionControllerClient := c.kubeClient.AdmissionregistrationV1beta1()
 	opts := metav1.ListOptions{}
 	objects := &Objects{}
 
@@ -108,6 +113,18 @@ func (c *Client) FetchObjects() (*Objects, error) {
 		objects.LimitRanges, err = client.LimitRanges(corev1.NamespaceAll).List(opts)
 		return
 	})
+	g.Go(func() (err error) {
+		objects.SystemNamespace, err = client.Namespaces().Get(metav1.NamespaceSystem, metav1.GetOptions{})
+		return
+	})
+	g.Go(func() (err error) {
+		objects.MutatingWebhookConfigurations, err = admissionControllerClient.MutatingWebhookConfigurations().List(opts)
+		return
+	})
+	g.Go(func() (err error) {
+		objects.ValidatingWebhookConfigurations, err = admissionControllerClient.ValidatingWebhookConfigurations().List(opts)
+		return
+	})
 
 	err := g.Wait()
 	if err != nil {