Support both v1beta1 and v1 admission control webhooks (#124)

We have a number of checks that operate on admission control webhook 
configuration. Older clusters support only v1beta1 of admission control, while 
newer clusters support v1. Currently clusterlint fails to run on these older 
clusters because we can't fetch v1 admission control objects from them. 

This change covers the following modifications:

- When listing objects, ignore "not found" errors, which mean the cluster 
  doesn't support the resource we're trying to list.
- Duplicate our existing admission control webhook checks for v1beta1, so that 
  older clusters get the same checks as newer clusters. 
- Enhance the errors we return when listing objects fails so that we can tell 
  which resource we failed to list.
- Remove extraneous empty import: client auth plugins are already loaded in 
  objects.go, so no need for the import in object_filter.go. 
- Ensure all object lists are non-nil after fetching objects. (Since we now 
  ignore not found errors, it's possible for some object lists to be nil.) 
- Skip v1beta1 admission control tests when v1 objects exist.

Co-authored-by: Timo Reimann <treimann@digitalocean.com>

checks/basic/admission_controller_webhook_v1beta1.go

@@ -0,0 +1,120 @@
+/*
+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 basic
+
+import (
+	"fmt"
+
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+)
+
+func init() {
+	checks.Register(&betaWebhookCheck{})
+}
+
+type betaWebhookCheck struct{}
+
+// Name returns a unique name for this check.
+func (w *betaWebhookCheck) Name() string {
+	return "admission-controller-webhook-v1beta1"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (w *betaWebhookCheck) Groups() []string {
+	return []string{"basic"}
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (w *betaWebhookCheck) Description() string {
+	return "Check for admission control webhooks"
+}
+
+// Run runs this check on a set of Kubernetes objects.
+func (w *betaWebhookCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) {
+	if len(objects.ValidatingWebhookConfigurations.Items) > 0 ||
+		len(objects.MutatingWebhookConfigurations.Items) > 0 {
+		// Skip this check if there are v1 webhook configurations. On clusters
+		// that support both v1beta1 and v1 admission control, the same webhook
+		// configurations will be returned for both versions.
+		return nil, nil
+	}
+
+	const apiserverServiceName = "kubernetes"
+
+	var diagnostics []checks.Diagnostic
+
+	for _, config := range objects.ValidatingWebhookConfigurationsBeta.Items {
+		for _, wh := range config.Webhooks {
+			if wh.ClientConfig.Service != nil {
+				// Ensure that the service (and its namespace) that is configure actually exists.
+
+				if !namespaceExists(objects.Namespaces, wh.ClientConfig.Service.Namespace) {
+					diagnostics = append(diagnostics, checks.Diagnostic{
+						Severity: checks.Error,
+						Message:  fmt.Sprintf("Validating webhook %s is configured against a service in a namespace that does not exist.", wh.Name),
+						Kind:     checks.ValidatingWebhookConfiguration,
+						Object:   &config.ObjectMeta,
+						Owners:   config.ObjectMeta.GetOwnerReferences(),
+					})
+					continue
+				}
+
+				if !serviceExists(objects.Services, wh.ClientConfig.Service.Name, wh.ClientConfig.Service.Namespace) {
+					diagnostics = append(diagnostics, checks.Diagnostic{
+						Severity: checks.Error,
+						Message:  fmt.Sprintf("Validating webhook %s is configured against a service that does not exist.", wh.Name),
+						Kind:     checks.ValidatingWebhookConfiguration,
+						Object:   &config.ObjectMeta,
+						Owners:   config.ObjectMeta.GetOwnerReferences(),
+					})
+				}
+			}
+		}
+	}
+
+	for _, config := range objects.MutatingWebhookConfigurationsBeta.Items {
+		for _, wh := range config.Webhooks {
+			if wh.ClientConfig.Service != nil {
+				// Ensure that the service (and its namespace) that is configure actually exists.
+
+				if !namespaceExists(objects.Namespaces, wh.ClientConfig.Service.Namespace) {
+					diagnostics = append(diagnostics, checks.Diagnostic{
+						Severity: checks.Error,
+						Message:  fmt.Sprintf("Mutating webhook %s is configured against a service in a namespace that does not exist.", wh.Name),
+						Kind:     checks.MutatingWebhookConfiguration,
+						Object:   &config.ObjectMeta,
+						Owners:   config.ObjectMeta.GetOwnerReferences(),
+					})
+					continue
+				}
+
+				if !serviceExists(objects.Services, wh.ClientConfig.Service.Name, wh.ClientConfig.Service.Namespace) {
+					diagnostics = append(diagnostics, checks.Diagnostic{
+						Severity: checks.Error,
+						Message:  fmt.Sprintf("Mutating webhook %s is configured against a service that does not exist.", wh.Name),
+						Kind:     checks.MutatingWebhookConfiguration,
+						Object:   &config.ObjectMeta,
+						Owners:   config.ObjectMeta.GetOwnerReferences(),
+					})
+				}
+			}
+		}
+	}
+	return diagnostics, nil
+}

checks/basic/admission_controller_webhook_v1beta1_test.go

@@ -0,0 +1,348 @@
+/*
+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 basic
+
+import (
+	"testing"
+
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	"github.com/stretchr/testify/assert"
+	arv1 "k8s.io/api/admissionregistration/v1"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestBetaWebhookCheckMeta(t *testing.T) {
+	betaWebhookCheck := betaWebhookCheck{}
+	assert.Equal(t, "admission-controller-webhook-v1beta1", betaWebhookCheck.Name())
+	assert.Equal(t, []string{"basic"}, betaWebhookCheck.Groups())
+	assert.NotEmpty(t, betaWebhookCheck.Description())
+}
+
+func TestBetaWebhookCheckRegistration(t *testing.T) {
+	betaWebhookCheck := &betaWebhookCheck{}
+	check, err := checks.Get("admission-controller-webhook-v1beta1")
+	assert.NoError(t, err)
+	assert.Equal(t, check, betaWebhookCheck)
+}
+
+func TestBetaWebHookRun(t *testing.T) {
+	emptyNamespaceList := &corev1.NamespaceList{
+		Items: []corev1.Namespace{},
+	}
+	emptyServiceList := &corev1.ServiceList{
+		Items: []corev1.Service{},
+	}
+
+	baseMWC := ar.MutatingWebhookConfiguration{
+		TypeMeta: metav1.TypeMeta{Kind: "MutatingWebhookConfiguration", APIVersion: "v1beta1"},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "mwc_foo",
+		},
+		Webhooks: []ar.MutatingWebhook{},
+	}
+	baseMW := ar.MutatingWebhook{
+		Name: "mw_foo",
+	}
+
+	baseMWCv1 := arv1.MutatingWebhookConfiguration{
+		TypeMeta: metav1.TypeMeta{Kind: "MutatingWebhookConfiguration", APIVersion: "v1"},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "mwc_foo",
+		},
+		Webhooks: []arv1.MutatingWebhook{},
+	}
+	baseMWv1 := arv1.MutatingWebhook{
+		Name: "mw_foo",
+	}
+
+	baseVWC := ar.ValidatingWebhookConfiguration{
+		TypeMeta: metav1.TypeMeta{Kind: "ValidatingWebhookConfiguration", APIVersion: "v1beta1"},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "vwc_foo",
+		},
+		Webhooks: []ar.ValidatingWebhook{},
+	}
+	baseVW := ar.ValidatingWebhook{
+		Name: "vw_foo",
+	}
+
+	tests := []struct {
+		name     string
+		objs     *kube.Objects
+		expected []checks.Diagnostic
+	}{
+		{
+			name: "no webhook configuration",
+			objs: &kube.Objects{
+				MutatingWebhookConfigurations:       &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations:     &arv1.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurationsBeta:   &ar.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{},
+				SystemNamespace:                     &corev1.Namespace{},
+			},
+			expected: nil,
+		},
+		{
+			name: "direct url webhooks",
+			objs: &kube.Objects{
+				Namespaces:                      emptyNamespaceList,
+				MutatingWebhookConfigurations:   &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurationsBeta: &ar.MutatingWebhookConfigurationList{
+					Items: []ar.MutatingWebhookConfiguration{
+						func() ar.MutatingWebhookConfiguration {
+							mwc := baseMWC
+							mw := baseMW
+							mw.ClientConfig = ar.WebhookClientConfig{
+								URL: strPtr("http://webhook.com"),
+							}
+							mwc.Webhooks = append(mwc.Webhooks, mw)
+							return mwc
+						}(),
+					},
+				},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{
+					Items: []ar.ValidatingWebhookConfiguration{
+						func() ar.ValidatingWebhookConfiguration {
+							vwc := baseVWC
+							vw := baseVW
+							vw.ClientConfig = ar.WebhookClientConfig{
+								URL: strPtr("http://webhook.com"),
+							}
+							vwc.Webhooks = append(vwc.Webhooks, vw)
+							return vwc
+						}(),
+					},
+				},
+				SystemNamespace: &corev1.Namespace{},
+			},
+			expected: nil,
+		},
+		{
+			name: "namespace does not exist",
+			objs: &kube.Objects{
+				Namespaces: emptyNamespaceList,
+				Services: &corev1.ServiceList{
+					Items: []corev1.Service{
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "service",
+							},
+						},
+					},
+				},
+				MutatingWebhookConfigurations:   &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurationsBeta: &ar.MutatingWebhookConfigurationList{
+					Items: []ar.MutatingWebhookConfiguration{
+						func() ar.MutatingWebhookConfiguration {
+							mwc := baseMWC
+							mw := baseMW
+							mw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "missing",
+									Name:      "service",
+								},
+							}
+							mwc.Webhooks = append(mwc.Webhooks, mw)
+							return mwc
+						}(),
+					},
+				},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{
+					Items: []ar.ValidatingWebhookConfiguration{
+						func() ar.ValidatingWebhookConfiguration {
+							vwc := baseVWC
+							vw := baseVW
+							vw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "missing",
+									Name:      "service",
+								},
+							}
+							vwc.Webhooks = append(vwc.Webhooks, vw)
+							return vwc
+						}(),
+					},
+				},
+				SystemNamespace: &corev1.Namespace{},
+			},
+			expected: []checks.Diagnostic{
+				{
+					Severity: checks.Error,
+					Message:  "Validating webhook vw_foo is configured against a service in a namespace that does not exist.",
+					Kind:     checks.ValidatingWebhookConfiguration,
+				},
+				{
+					Severity: checks.Error,
+					Message:  "Mutating webhook mw_foo is configured against a service in a namespace that does not exist.",
+					Kind:     checks.MutatingWebhookConfiguration,
+				},
+			},
+		},
+		{
+			name: "service does not exist",
+			objs: &kube.Objects{
+				Namespaces: &corev1.NamespaceList{
+					Items: []corev1.Namespace{
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "webhook",
+							},
+						},
+					},
+				},
+				Services:                        emptyServiceList,
+				MutatingWebhookConfigurations:   &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurationsBeta: &ar.MutatingWebhookConfigurationList{
+					Items: []ar.MutatingWebhookConfiguration{
+						func() ar.MutatingWebhookConfiguration {
+							mwc := baseMWC
+							mw := baseMW
+							mw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "webhook",
+									Name:      "service",
+								},
+							}
+							mwc.Webhooks = append(mwc.Webhooks, mw)
+							return mwc
+						}(),
+					},
+				},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{
+					Items: []ar.ValidatingWebhookConfiguration{
+						func() ar.ValidatingWebhookConfiguration {
+							vwc := baseVWC
+							vw := baseVW
+							vw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "webhook",
+									Name:      "service",
+								},
+							}
+							vwc.Webhooks = append(vwc.Webhooks, vw)
+							return vwc
+						}(),
+					},
+				},
+				SystemNamespace: &corev1.Namespace{},
+			},
+			expected: []checks.Diagnostic{
+				{
+					Severity: checks.Error,
+					Message:  "Validating webhook vw_foo is configured against a service that does not exist.",
+					Kind:     checks.ValidatingWebhookConfiguration,
+				},
+				{
+					Severity: checks.Error,
+					Message:  "Mutating webhook mw_foo is configured against a service that does not exist.",
+					Kind:     checks.MutatingWebhookConfiguration,
+				},
+			},
+		},
+		{
+			name: "check skipped when v1beta1 and v1 coexist",
+			objs: &kube.Objects{
+				Namespaces: &corev1.NamespaceList{
+					Items: []corev1.Namespace{
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "webhook",
+							},
+						},
+					},
+				},
+				Services:                        emptyServiceList,
+				ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurations: &arv1.MutatingWebhookConfigurationList{
+					Items: []arv1.MutatingWebhookConfiguration{
+						func() arv1.MutatingWebhookConfiguration {
+							mwc := baseMWCv1
+							mw := baseMWv1
+							mw.ClientConfig = arv1.WebhookClientConfig{
+								Service: &arv1.ServiceReference{
+									Namespace: "webhook",
+									Name:      "service",
+								},
+							}
+							mwc.Webhooks = append(mwc.Webhooks, mw)
+							return mwc
+						}(),
+					},
+				},
+				MutatingWebhookConfigurationsBeta: &ar.MutatingWebhookConfigurationList{
+					Items: []ar.MutatingWebhookConfiguration{
+						func() ar.MutatingWebhookConfiguration {
+							mwc := baseMWC
+							mw := baseMW
+							mw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "webhook",
+									Name:      "service",
+								},
+							}
+							mwc.Webhooks = append(mwc.Webhooks, mw)
+							return mwc
+						}(),
+					},
+				},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{
+					Items: []ar.ValidatingWebhookConfiguration{
+						func() ar.ValidatingWebhookConfiguration {
+							vwc := baseVWC
+							vw := baseVW
+							vw.ClientConfig = ar.WebhookClientConfig{
+								Service: &ar.ServiceReference{
+									Namespace: "webhook",
+									Name:      "service",
+								},
+							}
+							vwc.Webhooks = append(vwc.Webhooks, vw)
+							return vwc
+						}(),
+					},
+				},
+				SystemNamespace: &corev1.Namespace{},
+			},
+			expected: []checks.Diagnostic{},
+		},
+	}
+
+	betaWebhookCheck := betaWebhookCheck{}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			diagnostics, err := betaWebhookCheck.Run(test.objs)
+			assert.NoError(t, err)
+
+			// skip checking object and owner since for this checker it just uses the object being checked.
+			var strippedDiagnostics []checks.Diagnostic
+			for _, d := range diagnostics {
+				d.Object = nil
+				d.Owners = nil
+				strippedDiagnostics = append(strippedDiagnostics, d)
+			}
+
+			assert.ElementsMatch(t, test.expected, strippedDiagnostics)
+		})
+	}
+}

checks/doks/admission_controller_webhook_replacement_v1beta1.go

@@ -0,0 +1,193 @@
+/*
+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 (
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func init() {
+	checks.Register(&betaWebhookReplacementCheck{})
+}
+
+type betaWebhookReplacementCheck struct{}
+
+// Name returns a unique name for this check.
+func (w *betaWebhookReplacementCheck) Name() string {
+	return "admission-controller-webhook-replacement-v1beta1"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (w *betaWebhookReplacementCheck) Groups() []string {
+	return []string{"doks"}
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (w *betaWebhookReplacementCheck) Description() string {
+	return "Check for admission control webhooks that could cause problems during upgrades or node replacement"
+}
+
+// Run runs this check on a set of Kubernetes objects.
+func (w *betaWebhookReplacementCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) {
+	if len(objects.ValidatingWebhookConfigurations.Items) > 0 ||
+		len(objects.MutatingWebhookConfigurations.Items) > 0 {
+		// Skip this check if there are v1 webhook configurations. On clusters
+		// that support both v1beta1 and v1 admission control, the same webhook
+		// configurations will be returned for both versions.
+		return nil, nil
+	}
+
+	const apiserverServiceName = "kubernetes"
+
+	var diagnostics []checks.Diagnostic
+
+	for _, config := range objects.ValidatingWebhookConfigurationsBeta.Items {
+		config := config
+		for _, wh := range config.Webhooks {
+			wh := wh
+			if !applicableBeta(wh.Rules) {
+				// Webhooks that do not apply to core/v1, apps/v1, apps/v1beta1, apps/v1beta2 resources are fine
+				continue
+			}
+			if *wh.FailurePolicy == ar.Ignore {
+				// Webhooks with failurePolicy: Ignore are fine.
+				continue
+			}
+			if wh.ClientConfig.Service == nil {
+				// Webhooks whose targets are external to the cluster are fine.
+				continue
+			}
+			if wh.ClientConfig.Service.Namespace == metav1.NamespaceDefault &&
+				wh.ClientConfig.Service.Name == apiserverServiceName {
+				// Webhooks that target the kube-apiserver are fine.
+				continue
+			}
+			if !selectorMatchesNamespace(wh.NamespaceSelector, objects.SystemNamespace) {
+				// Webhooks that don't apply to kube-system are fine.
+				continue
+			}
+			var svcNamespace *v1.Namespace
+			for _, ns := range objects.Namespaces.Items {
+				ns := ns
+				if ns.Name == wh.ClientConfig.Service.Namespace {
+					svcNamespace = &ns
+				}
+			}
+			if svcNamespace != nil &&
+				!selectorMatchesNamespace(wh.NamespaceSelector, svcNamespace) &&
+				len(objects.Nodes.Items) > 1 {
+				// Webhooks that don't apply to their own namespace are fine, as
+				// long as there's more than one node in the cluster.
+				continue
+			}
+
+			d := checks.Diagnostic{
+				Severity: checks.Error,
+				Message:  "Validating webhook is configured in such a way that it may be problematic during upgrades.",
+				Kind:     checks.ValidatingWebhookConfiguration,
+				Object:   &config.ObjectMeta,
+				Owners:   config.ObjectMeta.GetOwnerReferences(),
+			}
+			diagnostics = append(diagnostics, d)
+
+			// We don't want to produce diagnostics for multiple webhooks in the
+			// same webhook configuration, so break out of the inner loop if we
+			// get here.
+			break
+		}
+	}
+
+	for _, config := range objects.MutatingWebhookConfigurationsBeta.Items {
+		config := config
+		for _, wh := range config.Webhooks {
+			wh := wh
+			if !applicableBeta(wh.Rules) {
+				// Webhooks that do not apply to core/v1, apps/v1, apps/v1beta1, apps/v1beta2 resources are fine
+				continue
+			}
+			if *wh.FailurePolicy == ar.Ignore {
+				// Webhooks with failurePolicy: Ignore are fine.
+				continue
+			}
+			if wh.ClientConfig.Service == nil {
+				// Webhooks whose targets are external to the cluster are fine.
+				continue
+			}
+			if wh.ClientConfig.Service.Namespace == metav1.NamespaceDefault &&
+				wh.ClientConfig.Service.Name == apiserverServiceName {
+				// Webhooks that target the kube-apiserver are fine.
+				continue
+			}
+			if !selectorMatchesNamespace(wh.NamespaceSelector, objects.SystemNamespace) {
+				// Webhooks that don't apply to kube-system are fine.
+				continue
+			}
+			var svcNamespace *v1.Namespace
+			for _, ns := range objects.Namespaces.Items {
+				ns := ns
+				if ns.Name == wh.ClientConfig.Service.Namespace {
+					svcNamespace = &ns
+				}
+			}
+			if svcNamespace != nil &&
+				!selectorMatchesNamespace(wh.NamespaceSelector, svcNamespace) &&
+				len(objects.Nodes.Items) > 1 {
+				// Webhooks that don't apply to their own namespace are fine, as
+				// long as there's more than one node in the cluster.
+				continue
+			}
+
+			d := checks.Diagnostic{
+				Severity: checks.Error,
+				Message:  "Mutating webhook is configured in such a way that it may be problematic during upgrades.",
+				Kind:     checks.MutatingWebhookConfiguration,
+				Object:   &config.ObjectMeta,
+				Owners:   config.ObjectMeta.GetOwnerReferences(),
+			}
+			diagnostics = append(diagnostics, d)
+
+			// We don't want to produce diagnostics for multiple webhooks in the
+			// same webhook configuration, so break out of the inner loop if we
+			// get here.
+			break
+		}
+	}
+	return diagnostics, nil
+}
+
+func applicableBeta(rules []ar.RuleWithOperations) bool {
+	for _, r := range rules {
+		if apiVersions(r.APIVersions) {
+			// applies to "apiVersions: v1"
+			if len(r.APIGroups) == 0 {
+				return true
+			}
+			// applies to "apiVersion: v1", "apiVersion: apps/v1", "apiVersion: apps/v1beta1", "apiVersion: apps/v1beta2"
+			for _, g := range r.APIGroups {
+				if g == "" || g == "*" || g == "apps" {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}

checks/doks/admission_controller_webhook_replacement_v1beta1_test.go

@@ -0,0 +1,535 @@
+/*
+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 (
+	"testing"
+
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	"github.com/stretchr/testify/assert"
+	arv1 "k8s.io/api/admissionregistration/v1"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestBetaWebhookCheckMeta(t *testing.T) {
+	webhookCheck := betaWebhookReplacementCheck{}
+	assert.Equal(t, "admission-controller-webhook-replacement-v1beta1", webhookCheck.Name())
+	assert.Equal(t, []string{"doks"}, webhookCheck.Groups())
+	assert.NotEmpty(t, webhookCheck.Description())
+}
+
+func TestBetaWebhookCheckRegistration(t *testing.T) {
+	webhookCheck := &betaWebhookReplacementCheck{}
+	check, err := checks.Get("admission-controller-webhook-replacement-v1beta1")
+	assert.NoError(t, err)
+	assert.Equal(t, check, webhookCheck)
+}
+
+func TestBetaWebhookSkipWhenV1Exists(t *testing.T) {
+	v1Objs := webhookTestObjects(
+		arv1.Fail,
+		&metav1.LabelSelector{},
+		arv1.WebhookClientConfig{
+			Service: &arv1.ServiceReference{
+				Namespace: "webhook",
+				Name:      "webhook-service",
+			},
+		},
+		2,
+		[]string{"*"},
+		[]string{"*"},
+	)
+	betaObjs := webhookTestObjectsBeta(
+		ar.Fail,
+		&metav1.LabelSelector{},
+		ar.WebhookClientConfig{
+			Service: &ar.ServiceReference{
+				Namespace: "webhook",
+				Name:      "webhook-service",
+			},
+		},
+		2,
+		[]string{"*"},
+		[]string{"*"},
+	)
+
+	objs := &kube.Objects{
+		MutatingWebhookConfigurations:       v1Objs.MutatingWebhookConfigurations,
+		ValidatingWebhookConfigurations:     v1Objs.ValidatingWebhookConfigurations,
+		MutatingWebhookConfigurationsBeta:   betaObjs.MutatingWebhookConfigurationsBeta,
+		ValidatingWebhookConfigurationsBeta: betaObjs.ValidatingWebhookConfigurationsBeta,
+	}
+
+	webhookCheck := betaWebhookReplacementCheck{}
+	d, err := webhookCheck.Run(objs)
+	assert.NoError(t, err)
+	assert.Empty(t, d)
+}
+
+func TestBetaWebhookError(t *testing.T) {
+	tests := []struct {
+		name     string
+		objs     *kube.Objects
+		expected []checks.Diagnostic
+	}{
+		{
+			name: "no webhook configurations",
+			objs: &kube.Objects{
+				MutatingWebhookConfigurationsBeta:   &ar.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurations:       &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations:     &arv1.ValidatingWebhookConfigurationList{},
+				SystemNamespace:                     &corev1.Namespace{},
+			},
+			expected: nil,
+		},
+		{
+			name: "failure policy is ignore",
+			objs: webhookTestObjectsBeta(
+				ar.Ignore,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "webook does not use service",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					URL: &webhookURL,
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "webook service is apiserver",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "default",
+						Name:      "kubernetes",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace label selector does not match kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchLabels: map[string]string{"non-existent-label-on-namespace": "bar"},
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace OpExists expression selector does not match kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchExpressions: expr("non-existent", []string{}, metav1.LabelSelectorOpExists),
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace OpDoesNotExist expression selector does not match kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchExpressions: expr("doks_key", []string{}, metav1.LabelSelectorOpDoesNotExist),
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace OpIn expression selector does not match kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchExpressions: expr("doks_key", []string{"non-existent"}, metav1.LabelSelectorOpIn),
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace OpNotIn expression selector does not match kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchExpressions: expr("doks_key", []string{"bar"}, metav1.LabelSelectorOpNotIn),
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "namespace label selector does not match own namespace",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchLabels: map[string]string{"doks_key": "bar"},
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+		{
+			name: "single-node cluster",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{
+					MatchLabels: map[string]string{"doks_key": "bar"},
+				},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				1,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "webhook applies to its own namespace and kube-system",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "error: webhook applies to core/v1 group",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{""},
+				[]string{"v1"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "error: webhook applies to apps/v1 group",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"apps"},
+				[]string{"v1"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "error: webhook applies to apps/v1beta1 group",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"apps"},
+				[]string{"v1beta1"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "error: webhook applies to apps/v1beta2 group",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"apps"},
+				[]string{"v1beta2"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "error: webhook applies to *",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"*"},
+				[]string{"*"},
+			),
+			expected: webhookErrorsBeta(),
+		},
+		{
+			name: "no error: webhook applies to batch/v1",
+			objs: webhookTestObjectsBeta(
+				ar.Fail,
+				&metav1.LabelSelector{},
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				2,
+				[]string{"batch"},
+				[]string{"*"},
+			),
+			expected: nil,
+		},
+	}
+
+	webhookCheck := betaWebhookReplacementCheck{}
+
+	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 webhookTestObjectsBeta(
+	failurePolicyType ar.FailurePolicyType,
+	nsSelector *metav1.LabelSelector,
+	clientConfig ar.WebhookClientConfig,
+	numNodes int,
+	groups []string,
+	versions []string,
+) *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"},
+			},
+		},
+		Namespaces: &corev1.NamespaceList{
+			Items: []corev1.Namespace{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:   "kube-system",
+						Labels: map[string]string{"doks_key": "bar"},
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:   "webhook",
+						Labels: map[string]string{"doks_key": "xyzzy"},
+					},
+				},
+			},
+		},
+		MutatingWebhookConfigurations:   &arv1.MutatingWebhookConfigurationList{},
+		ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+		MutatingWebhookConfigurationsBeta: &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: nsSelector,
+							ClientConfig:      clientConfig,
+							Rules: []ar.RuleWithOperations{
+								{
+									Rule: ar.Rule{
+										APIGroups:   groups,
+										APIVersions: versions,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		ValidatingWebhookConfigurationsBeta: &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: nsSelector,
+							ClientConfig:      clientConfig,
+							Rules: []ar.RuleWithOperations{
+								{
+									Rule: ar.Rule{
+										APIGroups:   groups,
+										APIVersions: versions,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	objs.Nodes = &corev1.NodeList{}
+	for i := 0; i < numNodes; i++ {
+		objs.Nodes.Items = append(objs.Nodes.Items, corev1.Node{})
+	}
+	return objs
+}
+
+func webhookErrorsBeta() []checks.Diagnostic {
+	objs := webhookTestObjectsBeta(ar.Fail, nil, ar.WebhookClientConfig{}, 0, []string{"*"}, []string{"*"})
+	validatingConfig := objs.ValidatingWebhookConfigurationsBeta.Items[0]
+	mutatingConfig := objs.MutatingWebhookConfigurationsBeta.Items[0]
+
+	diagnostics := []checks.Diagnostic{
+		{
+			Severity: checks.Error,
+			Message:  "Validating webhook is configured in such a way that it may be problematic during upgrades.",
+			Kind:     checks.ValidatingWebhookConfiguration,
+			Object:   &validatingConfig.ObjectMeta,
+			Owners:   validatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+		{
+			Severity: checks.Error,
+			Message:  "Mutating webhook is configured in such a way that it may be problematic during upgrades.",
+			Kind:     checks.MutatingWebhookConfiguration,
+			Object:   &mutatingConfig.ObjectMeta,
+			Owners:   mutatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+	}
+	return diagnostics
+}

checks/doks/admission_controller_webhook_timeout_v1beta1.go

@@ -0,0 +1,106 @@
+/*
+Copyright 2020 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 (
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+)
+
+func init() {
+	checks.Register(&betaWebhookTimeoutCheck{})
+}
+
+type betaWebhookTimeoutCheck struct{}
+
+// Name returns a unique name for this check.
+func (w *betaWebhookTimeoutCheck) Name() string {
+	return "admission-controller-webhook-timeout-v1beta1"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (w *betaWebhookTimeoutCheck) Groups() []string {
+	return []string{"doks"}
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (w *betaWebhookTimeoutCheck) Description() string {
+	return "Check for admission control webhooks that have exceeded a timeout of 30 seconds."
+}
+
+// Run runs this check on a set of Kubernetes objects.
+func (w *betaWebhookTimeoutCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) {
+	if len(objects.ValidatingWebhookConfigurations.Items) > 0 ||
+		len(objects.MutatingWebhookConfigurations.Items) > 0 {
+		// Skip this check if there are v1 webhook configurations. On clusters
+		// that support both v1beta1 and v1 admission control, the same webhook
+		// configurations will be returned for both versions.
+		return nil, nil
+	}
+
+	var diagnostics []checks.Diagnostic
+
+	for _, config := range objects.ValidatingWebhookConfigurationsBeta.Items {
+		config := config
+		for _, wh := range config.Webhooks {
+			wh := wh
+			if wh.TimeoutSeconds == nil {
+				// TimeoutSeconds value should be set to a non-nil value (greater than or equal to 1 and less than 30).
+				// If the TimeoutSeconds value is set to nil and the cluster version is 1.13.*, users are
+				// unable to configure the TimeoutSeconds value and this value will stay at nil, breaking
+				// upgrades. It's only for versions >= 1.14 that the value will default to 30 seconds.
+				continue
+			} else if *wh.TimeoutSeconds < int32(1) || *wh.TimeoutSeconds >= int32(30) {
+				// Webhooks with TimeoutSeconds set: less than 1 or greater than or equal to 30 is bad.
+				d := checks.Diagnostic{
+					Severity: checks.Error,
+					Message:  "Validating webhook with a TimeoutSeconds value greater than 29 seconds will block upgrades.",
+					Kind:     checks.ValidatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
+			}
+		}
+	}
+
+	for _, config := range objects.MutatingWebhookConfigurationsBeta.Items {
+		config := config
+		for _, wh := range config.Webhooks {
+			wh := wh
+			if wh.TimeoutSeconds == nil {
+				// TimeoutSeconds value should be set to a non-nil value (greater than or equal to 1 and less than 30).
+				// If the TimeoutSeconds value is set to nil and the cluster version is 1.13.*, users are
+				// unable to configure the TimeoutSeconds value and this value will stay at nil, breaking
+				// upgrades. It's only for versions >= 1.14 that the value will default to 30 seconds.
+				continue
+			} else if *wh.TimeoutSeconds < int32(1) || *wh.TimeoutSeconds >= int32(30) {
+				// Webhooks with TimeoutSeconds set: less than 1 or greater than or equal to 30 is bad.
+				d := checks.Diagnostic{
+					Severity: checks.Error,
+					Message:  "Mutating webhook with a TimeoutSeconds value greater than 29 seconds will block upgrades.",
+					Kind:     checks.MutatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
+			}
+		}
+	}
+	return diagnostics, nil
+}

checks/doks/admission_controller_webhook_timeout_v1beta1_test.go

@@ -0,0 +1,277 @@
+/*
+Copyright 2020 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 (
+	"testing"
+
+	"github.com/digitalocean/clusterlint/checks"
+	"github.com/digitalocean/clusterlint/kube"
+	"github.com/stretchr/testify/assert"
+	arv1 "k8s.io/api/admissionregistration/v1"
+	ar "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestBetaWebhookTimeoutCheckMeta(t *testing.T) {
+	webhookCheck := betaWebhookTimeoutCheck{}
+	assert.Equal(t, "admission-controller-webhook-timeout-v1beta1", webhookCheck.Name())
+	assert.Equal(t, []string{"doks"}, webhookCheck.Groups())
+	assert.NotEmpty(t, webhookCheck.Description())
+}
+
+func TestBetaWebhookTimeoutRegistration(t *testing.T) {
+	webhookCheck := &betaWebhookTimeoutCheck{}
+	check, err := checks.Get("admission-controller-webhook-timeout-v1beta1")
+	assert.NoError(t, err)
+	assert.Equal(t, check, webhookCheck)
+}
+
+func TestBetaWebhookTimeoutSkipWhenV1Exists(t *testing.T) {
+	v1Objs := webhookTimeoutTestObjects(
+		arv1.WebhookClientConfig{
+			Service: &arv1.ServiceReference{
+				Namespace: "webhook",
+				Name:      "webhook-service",
+			},
+		},
+		toIntP(31),
+		2,
+	)
+	betaObjs := webhookTimeoutTestObjectsBeta(
+		ar.WebhookClientConfig{
+			Service: &ar.ServiceReference{
+				Namespace: "webhook",
+				Name:      "webhook-service",
+			},
+		},
+		toIntP(31),
+		2,
+	)
+
+	objs := &kube.Objects{
+		MutatingWebhookConfigurations:       v1Objs.MutatingWebhookConfigurations,
+		ValidatingWebhookConfigurations:     v1Objs.ValidatingWebhookConfigurations,
+		MutatingWebhookConfigurationsBeta:   betaObjs.MutatingWebhookConfigurationsBeta,
+		ValidatingWebhookConfigurationsBeta: betaObjs.ValidatingWebhookConfigurationsBeta,
+	}
+
+	webhookCheck := betaWebhookTimeoutCheck{}
+	d, err := webhookCheck.Run(objs)
+	assert.NoError(t, err)
+	assert.Empty(t, d)
+}
+
+func TestBetaWebhookTimeoutError(t *testing.T) {
+	tests := []struct {
+		name     string
+		objs     *kube.Objects
+		expected []checks.Diagnostic
+	}{
+		{
+			name: "no webhook configurations",
+			objs: &kube.Objects{
+				MutatingWebhookConfigurationsBeta:   &ar.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{},
+				MutatingWebhookConfigurations:       &arv1.MutatingWebhookConfigurationList{},
+				ValidatingWebhookConfigurations:     &arv1.ValidatingWebhookConfigurationList{},
+			},
+			expected: nil,
+		},
+		{
+			name: "TimeoutSeconds value is set to 10 seconds",
+			objs: webhookTimeoutTestObjectsBeta(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				toIntP(10),
+				2,
+			),
+			expected: nil,
+		},
+		{
+			name: "TimeoutSeconds value is set to 29 seconds",
+			objs: webhookTimeoutTestObjectsBeta(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				toIntP(29),
+				2,
+			),
+			expected: nil,
+		},
+		{
+			name: "TimeoutSeconds value is set to 30 seconds",
+			objs: webhookTimeoutTestObjectsBeta(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				toIntP(30),
+				2,
+			),
+			expected: webhookTimeoutErrors(),
+		},
+		{
+			name: "TimeoutSeconds value is set to 31 seconds",
+			objs: webhookTimeoutTestObjectsBeta(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				toIntP(31),
+				2,
+			),
+			expected: webhookTimeoutErrors(),
+		},
+		{
+			name: "TimeoutSeconds value is set to nil",
+			objs: webhookTimeoutTestObjectsBeta(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				nil,
+				2,
+			),
+			expected: nil,
+		},
+	}
+
+	webhookCheck := betaWebhookTimeoutCheck{}
+
+	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 webhookTimeoutTestObjectsBeta(
+	clientConfig ar.WebhookClientConfig,
+	timeoutSeconds *int32,
+	numNodes int,
+) *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_test_key": "bar"},
+			},
+		},
+		Namespaces: &corev1.NamespaceList{
+			Items: []corev1.Namespace{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:   "kube-system",
+						Labels: map[string]string{"doks_test_key": "bar"},
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:   "webhook",
+						Labels: map[string]string{"doks_test_key": "xyzzy"},
+					},
+				},
+			},
+		},
+		MutatingWebhookConfigurations:   &arv1.MutatingWebhookConfigurationList{},
+		ValidatingWebhookConfigurations: &arv1.ValidatingWebhookConfigurationList{},
+		MutatingWebhookConfigurationsBeta: &ar.MutatingWebhookConfigurationList{
+			Items: []ar.MutatingWebhookConfiguration{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "MutatingWebhookConfiguration", APIVersion: "v1beta1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "mwc_foo",
+					},
+					Webhooks: []ar.MutatingWebhook{
+						{
+							Name:           "mw_foo",
+							ClientConfig:   clientConfig,
+							TimeoutSeconds: timeoutSeconds,
+						},
+					},
+				},
+			},
+		},
+		ValidatingWebhookConfigurationsBeta: &ar.ValidatingWebhookConfigurationList{
+			Items: []ar.ValidatingWebhookConfiguration{
+				{
+					TypeMeta: metav1.TypeMeta{Kind: "ValidatingWebhookConfiguration", APIVersion: "v1beta1"},
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "vwc_foo",
+					},
+					Webhooks: []ar.ValidatingWebhook{
+						{
+							Name:           "vw_foo",
+							ClientConfig:   clientConfig,
+							TimeoutSeconds: timeoutSeconds,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	objs.Nodes = &corev1.NodeList{}
+	for i := 0; i < numNodes; i++ {
+		objs.Nodes.Items = append(objs.Nodes.Items, corev1.Node{})
+	}
+	return objs
+}
+
+func webhookTimeoutErrorsBeta() []checks.Diagnostic {
+	objs := webhookTimeoutTestObjectsBeta(ar.WebhookClientConfig{}, nil, 0)
+	validatingConfig := objs.ValidatingWebhookConfigurationsBeta.Items[0]
+	mutatingConfig := objs.MutatingWebhookConfigurationsBeta.Items[0]
+
+	diagnostics := []checks.Diagnostic{
+		{
+			Severity: checks.Error,
+			Message:  "Validating webhook with a TimeoutSeconds value greater than 29 seconds will block upgrades.",
+			Kind:     checks.ValidatingWebhookConfiguration,
+			Object:   &validatingConfig.ObjectMeta,
+			Owners:   validatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+		{
+			Severity: checks.Error,
+			Message:  "Mutating webhook with a TimeoutSeconds value greater than 29 seconds will block upgrades.",
+			Kind:     checks.MutatingWebhookConfiguration,
+			Object:   &mutatingConfig.ObjectMeta,
+			Owners:   mutatingConfig.ObjectMeta.GetOwnerReferences(),
+		},
+	}
+	return diagnostics
+}

kube/object_filter.go

@@ -17,11 +17,10 @@ limitations under the License.
 package kube
 
 import (
-	// Load client-go authentication plugins
 	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
 // ObjectFilter stores k8s object's fields that needs to be included or excluded while running checks

kube/objects.go

@@ -18,12 +18,16 @@ package kube
 
 import (
 	"context"
+	"fmt"
 
 	"golang.org/x/sync/errgroup"
-	ar "k8s.io/api/admissionregistration/v1"
+	arv1 "k8s.io/api/admissionregistration/v1"
+	arv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	batchv1beta1 "k8s.io/api/batch/v1beta1"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	st "k8s.io/api/storage/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -55,8 +59,10 @@ type Objects struct {
 	LimitRanges                         *corev1.LimitRangeList
 	StorageClasses                      *st.StorageClassList
 	DefaultStorageClass                 *st.StorageClass
-	MutatingWebhookConfigurations   *ar.MutatingWebhookConfigurationList
-	ValidatingWebhookConfigurations *ar.ValidatingWebhookConfigurationList
+	MutatingWebhookConfigurations       *arv1.MutatingWebhookConfigurationList
+	ValidatingWebhookConfigurations     *arv1.ValidatingWebhookConfigurationList
+	MutatingWebhookConfigurationsBeta   *arv1beta1.MutatingWebhookConfigurationList
+	ValidatingWebhookConfigurationsBeta *arv1beta1.ValidatingWebhookConfigurationList
 	Namespaces                          *corev1.NamespaceList
 	CronJobs                            *batchv1beta1.CronJobList
 }
@@ -71,6 +77,7 @@ type Client struct {
 func (c *Client) FetchObjects(ctx context.Context, filter ObjectFilter) (*Objects, error) {
 	client := c.KubeClient.CoreV1()
 	admissionControllerClient := c.KubeClient.AdmissionregistrationV1()
+	admissionControllerClientBeta := c.KubeClient.AdmissionregistrationV1beta1()
 	batchClient := c.KubeClient.BatchV1beta1()
 	storageClient := c.KubeClient.StorageV1()
 	opts := metav1.ListOptions{}
@@ -95,62 +102,89 @@ func (c *Client) FetchObjects(ctx context.Context, filter ObjectFilter) (*Object
 	})
 	g.Go(func() (err error) {
 		objects.PersistentVolumes, err = client.PersistentVolumes().List(gCtx, opts)
+		err = annotateFetchError("PersistentVolumes", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.Pods, err = client.Pods(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("Pods", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.PodTemplates, err = client.PodTemplates(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("PodTemplates", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.PersistentVolumeClaims, err = client.PersistentVolumeClaims(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("PersistentVolumeClaims", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.ConfigMaps, err = client.ConfigMaps(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("ConfigMaps", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.Secrets, err = client.Secrets(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("Secrets", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.Services, err = client.Services(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("Services", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.ServiceAccounts, err = client.ServiceAccounts(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("ServiceAccounts", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.ResourceQuotas, err = client.ResourceQuotas(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("ResourceQuotas", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.LimitRanges, err = client.LimitRanges(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("LimitRanges", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.SystemNamespace, err = client.Namespaces().Get(gCtx, metav1.NamespaceSystem, metav1.GetOptions{})
+		if err != nil {
+			err = fmt.Errorf("failed to fetch namespace %q: %s", metav1.NamespaceSystem, err)
+		}
 		return
 	})
 	g.Go(func() (err error) {
 		objects.MutatingWebhookConfigurations, err = admissionControllerClient.MutatingWebhookConfigurations().List(gCtx, opts)
+		err = annotateFetchError("MutatingWebhookConfigurations (v1)", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.ValidatingWebhookConfigurations, err = admissionControllerClient.ValidatingWebhookConfigurations().List(gCtx, opts)
+		err = annotateFetchError("ValidatingWebhookConfigurations (v1)", err)
+		return
+	})
+	g.Go(func() (err error) {
+		objects.MutatingWebhookConfigurationsBeta, err = admissionControllerClientBeta.MutatingWebhookConfigurations().List(gCtx, opts)
+		err = annotateFetchError("MutatingWebhookConfigurations (v1beta1)", err)
+		return
+	})
+	g.Go(func() (err error) {
+		objects.ValidatingWebhookConfigurationsBeta, err = admissionControllerClientBeta.ValidatingWebhookConfigurations().List(gCtx, opts)
+		err = annotateFetchError("ValidatingWebhookConfigurations (v1beta1)", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.Namespaces, err = client.Namespaces().List(gCtx, opts)
+		err = annotateFetchError("Namespaces", err)
 		return
 	})
 	g.Go(func() (err error) {
 		objects.CronJobs, err = batchClient.CronJobs(corev1.NamespaceAll).List(gCtx, filter.NamespaceOptions(opts))
+		err = annotateFetchError("CronJobs", err)
 		return
 	})
 
@@ -159,7 +193,79 @@ func (c *Client) FetchObjects(ctx context.Context, filter ObjectFilter) (*Object
 		return nil, err
 	}
 
-	return objects, nil
+	return objectsWithoutNils(objects), nil
+}
+
+func annotateFetchError(kind string, err error) error {
+	if err == nil {
+		return nil
+	}
+	if kerrors.IsNotFound(err) {
+		// Resource doesn't exist in this cluster's version, so there aren't any
+		// objects to list and check.
+		return nil
+	}
+
+	return fmt.Errorf("failed to fetch %s: %s", kind, err)
+}
+
+func objectsWithoutNils(objects *Objects) *Objects {
+	if objects.Nodes == nil {
+		objects.Nodes = &v1.NodeList{}
+	}
+	if objects.PersistentVolumes == nil {
+		objects.PersistentVolumes = &v1.PersistentVolumeList{}
+	}
+	if objects.Pods == nil {
+		objects.Pods = &v1.PodList{}
+	}
+	if objects.PodTemplates == nil {
+		objects.PodTemplates = &v1.PodTemplateList{}
+	}
+	if objects.PersistentVolumeClaims == nil {
+		objects.PersistentVolumeClaims = &v1.PersistentVolumeClaimList{}
+	}
+	if objects.ConfigMaps == nil {
+		objects.ConfigMaps = &v1.ConfigMapList{}
+	}
+	if objects.Services == nil {
+		objects.Services = &v1.ServiceList{}
+	}
+	if objects.Secrets == nil {
+		objects.Secrets = &v1.SecretList{}
+	}
+	if objects.ServiceAccounts == nil {
+		objects.ServiceAccounts = &v1.ServiceAccountList{}
+	}
+	if objects.ResourceQuotas == nil {
+		objects.ResourceQuotas = &v1.ResourceQuotaList{}
+	}
+	if objects.LimitRanges == nil {
+		objects.LimitRanges = &v1.LimitRangeList{}
+	}
+	if objects.StorageClasses == nil {
+		objects.StorageClasses = &st.StorageClassList{}
+	}
+	if objects.MutatingWebhookConfigurations == nil {
+		objects.MutatingWebhookConfigurations = &arv1.MutatingWebhookConfigurationList{}
+	}
+	if objects.ValidatingWebhookConfigurations == nil {
+		objects.ValidatingWebhookConfigurations = &arv1.ValidatingWebhookConfigurationList{}
+	}
+	if objects.MutatingWebhookConfigurationsBeta == nil {
+		objects.MutatingWebhookConfigurationsBeta = &arv1beta1.MutatingWebhookConfigurationList{}
+	}
+	if objects.ValidatingWebhookConfigurationsBeta == nil {
+		objects.ValidatingWebhookConfigurationsBeta = &arv1beta1.ValidatingWebhookConfigurationList{}
+	}
+	if objects.Namespaces == nil {
+		objects.Namespaces = &v1.NamespaceList{}
+	}
+	if objects.CronJobs == nil {
+		objects.CronJobs = &batchv1beta1.CronJobList{}
+	}
+
+	return objects
 }
 
 // NewClient builds a kubernetes client to interact with the live cluster.

kube/objects_test.go

@@ -19,18 +19,52 @@ package kube
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
+	ktesting "k8s.io/client-go/testing"
 )
 
 func TestFetchObjects(t *testing.T) {
+	tests := []struct {
+		name        string
+		fakeMutator func(cs *fake.Clientset)
+	}{
+		{
+			name: "happy path",
+		},
+		{
+			name: "resources not found",
+			fakeMutator: func(cs *fake.Clientset) {
+				notFoundReactionFunc := func(action ktesting.Action) (bool, runtime.Object, error) {
+					return true, nil, &kerrors.StatusError{
+						ErrStatus: metav1.Status{
+							Reason:  metav1.StatusReasonNotFound,
+							Message: fmt.Sprintf("%s not found", action.GetResource().Resource),
+						},
+					}
+				}
+				cs.PrependReactor("list", "mutatingwebhookconfigurations", notFoundReactionFunc)
+				cs.PrependReactor("list", "validatingwebhookconfigurations", notFoundReactionFunc)
+			},
+		},
+	}
+
+	for _, test := range tests {
+		cs := fake.NewSimpleClientset()
+		if test.fakeMutator != nil {
+			test.fakeMutator(cs)
+		}
+
 		api := &Client{
-		KubeClient: fake.NewSimpleClientset(),
+			KubeClient: cs,
 		}
 
 		api.KubeClient.CoreV1().Namespaces().Create(context.Background(), &corev1.Namespace{
@@ -57,6 +91,9 @@ func TestFetchObjects(t *testing.T) {
 		assert.NotNil(t, actual.ValidatingWebhookConfigurations)
 		assert.NotNil(t, actual.MutatingWebhookConfigurations)
 		assert.NotNil(t, actual.SystemNamespace)
+		assert.NotNil(t, actual.CronJobs)
+	}
+
 }
 
 func TestNewClientErrors(t *testing.T) {
@@ -104,3 +141,37 @@ users:
 	}, metav1.CreateOptions{})
 	assert.Contains(t, err.Error(), "fail")
 }
+
+func TestAnnotateFetchError(t *testing.T) {
+	kindName := "kind"
+
+	tests := []struct {
+		name    string
+		inErr   error
+		wantErr error
+	}{
+		{
+			name:    "no error",
+			inErr:   nil,
+			wantErr: nil,
+		},
+		{
+			name: "not found error",
+			inErr: &kerrors.StatusError{
+				ErrStatus: metav1.Status{
+					Reason: metav1.StatusReasonNotFound,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name:    "other error",
+			inErr:   errors.New("other error"),
+			wantErr: fmt.Errorf("failed to fetch %s: other error", kindName),
+		},
+	}
+
+	for _, test := range tests {
+		assert.Equal(t, test.wantErr, annotateFetchError(kindName, test.inErr))
+	}
+}