Update timeout check to account for nil TimeoutSeconds value

digitalocean · Jun 30, 2020 · b3915f2 · b3915f2
1 parent c571ab5
commit b3915f2
Showing 2 changed files with 80 additions and 22 deletions.
diff --git a/checks/doks/admission_controller_webhook_timeout.go b/checks/doks/admission_controller_webhook_timeout.go
@@ -49,35 +49,55 @@ func (w *webhookTimeoutCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, e
 
 	for _, config := range objects.ValidatingWebhookConfigurations.Items {
 		for _, wh := range config.Webhooks {
-			if *wh.TimeoutSeconds >= int32(1) && *wh.TimeoutSeconds < int32(30) {
-				// Webhooks with TimeoutSeconds set: between 1 and 30 is fine.
+			if wh.TimeoutSeconds == nil {
+				// TimeoutSeconds value should be set to a non-nil value (greater than or equal to 1 and less than 30).
+				d := checks.Diagnostic{
+					Severity: checks.Error,
+					Message:  "Validating webhook with the default TimeoutSeconds value of 30 will block upgrades.",
+					Kind:     checks.ValidatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
 				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)
 			}
-			d := checks.Diagnostic{
-				Severity: checks.Error,
-				Message:  "Validating webhook with a TimeoutSeconds value greater than 30 seconds will block upgrades.",
-				Kind:     checks.ValidatingWebhookConfiguration,
-				Object:   &config.ObjectMeta,
-				Owners:   config.ObjectMeta.GetOwnerReferences(),
-			}
-			diagnostics = append(diagnostics, d)
 		}
 	}
 
 	for _, config := range objects.MutatingWebhookConfigurations.Items {
 		for _, wh := range config.Webhooks {
-			if *wh.TimeoutSeconds >= int32(1) && *wh.TimeoutSeconds < int32(30) {
-				// Webhooks with TimeoutSeconds set: between 1 and 30 is fine.
+			if wh.TimeoutSeconds == nil {
+				// TimeoutSeconds value should be set to a non-nil value (greater than or equal to 1 and less than 30).
+				d := checks.Diagnostic{
+					Severity: checks.Error,
+					Message:  "Mutating webhook with the default TimeoutSeconds value of 30 will block upgrades.",
+					Kind:     checks.MutatingWebhookConfiguration,
+					Object:   &config.ObjectMeta,
+					Owners:   config.ObjectMeta.GetOwnerReferences(),
+				}
+				diagnostics = append(diagnostics, d)
 				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)
 			}
-			d := checks.Diagnostic{
-				Severity: checks.Error,
-				Message:  "Mutating webhook with a TimeoutSeconds value greater than 30 seconds will block upgrades.",
-				Kind:     checks.MutatingWebhookConfiguration,
-				Object:   &config.ObjectMeta,
-				Owners:   config.ObjectMeta.GetOwnerReferences(),
-			}
-			diagnostics = append(diagnostics, d)
 		}
 	}
 	return diagnostics, nil

diff --git a/checks/doks/admission_controller_webhook_timeout_test.go b/checks/doks/admission_controller_webhook_timeout_test.go
@@ -111,6 +111,20 @@ func TestWebhookTimeoutError(t *testing.T) {
 			),
 			expected: webhookTimeoutErrors(),
 		},
+		{
+			name: "TimeoutSeconds value is set to nil",
+			objs: webhookTimeoutTestObjects(
+				ar.WebhookClientConfig{
+					Service: &ar.ServiceReference{
+						Namespace: "webhook",
+						Name:      "webhook-service",
+					},
+				},
+				nil,
+				2,
+			),
+			expected: webhookNilTimeoutErrors(),
+		},
 	}
 
 	webhookCheck := webhookTimeoutCheck{}
@@ -206,14 +220,38 @@ func webhookTimeoutErrors() []checks.Diagnostic {
 	diagnostics := []checks.Diagnostic{
 		{
 			Severity: checks.Error,
-			Message:  "Validating webhook with a TimeoutSeconds value greater than 30 seconds will block upgrades.",
+			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
+}
+
+func webhookNilTimeoutErrors() []checks.Diagnostic {
+	objs := webhookTimeoutTestObjects(ar.WebhookClientConfig{}, nil, 0)
+	validatingConfig := objs.ValidatingWebhookConfigurations.Items[0]
+	mutatingConfig := objs.MutatingWebhookConfigurations.Items[0]
+
+	diagnostics := []checks.Diagnostic{
+		{
+			Severity: checks.Error,
+			Message:  "Validating webhook with the default TimeoutSeconds value of 30 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 30 seconds will block upgrades.",
+			Message:  "Mutating webhook with the default TimeoutSeconds value of 30 will block upgrades.",
 			Kind:     checks.MutatingWebhookConfiguration,
 			Object:   &mutatingConfig.ObjectMeta,
 			Owners:   mutatingConfig.ObjectMeta.GetOwnerReferences(),