Add comment on why we just continue for nil TimeoutSeconds value

digitalocean · Jul 7, 2020 · 161c7a1 · 161c7a1
1 parent b3915f2
commit 161c7a1
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 41 deletions.
diff --git a/checks/doks/admission_controller_webhook_timeout.go b/checks/doks/admission_controller_webhook_timeout.go
@@ -51,14 +51,9 @@ func (w *webhookTimeoutCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, e
 		for _, wh := range config.Webhooks {
 			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)
+				// 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.
@@ -78,14 +73,9 @@ func (w *webhookTimeoutCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, e
 		for _, wh := range config.Webhooks {
 			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)
+				// 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.

diff --git a/checks/doks/admission_controller_webhook_timeout_test.go b/checks/doks/admission_controller_webhook_timeout_test.go
@@ -123,7 +123,7 @@ func TestWebhookTimeoutError(t *testing.T) {
 				nil,
 				2,
 			),
-			expected: webhookNilTimeoutErrors(),
+			expected: nil,
 		},
 	}
 
@@ -236,30 +236,6 @@ func webhookTimeoutErrors() []checks.Diagnostic {
 	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 the default TimeoutSeconds value of 30 will block upgrades.",
-			Kind:     checks.MutatingWebhookConfiguration,
-			Object:   &mutatingConfig.ObjectMeta,
-			Owners:   mutatingConfig.ObjectMeta.GetOwnerReferences(),
-		},
-	}
-	return diagnostics
-}
-
 // converts an int to an int32 and returns a pointer
 func toIntP(i int) *int32 {
 	num := int32(i)