address review comments

kubernetes-sigs · May 12, 2022 · 2dbe5e5 · 2dbe5e5
1 parent 3a65c56
commit 2dbe5e5
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 12 deletions.
diff --git a/internal/webhooks/runtime/extensionconfig_webhook.go b/internal/webhooks/runtime/extensionconfig_webhook.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -140,7 +141,7 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
 			allErrs = append(allErrs, field.Invalid(
 				specPath.Child("clientConfig", "url"),
 				*e.Spec.ClientConfig.URL,
-				"must be a valid URL, e.g. https://example.com",
+				fmt.Sprintf("must be a valid URL, e.g. https://example.com: %v", err),
 			))
 		} else if uri.Scheme != "http" && uri.Scheme != "https" {
 			allErrs = append(allErrs, field.Invalid(
@@ -161,11 +162,11 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
 			))
 		}
 
-		if errs := validation.IsDNS1035Label(e.Spec.ClientConfig.Service.Name); len(errs) != 0 {
+		for _, msg := range validation.IsDNS1035Label(e.Spec.ClientConfig.Service.Name) {
 			allErrs = append(allErrs, field.Invalid(
 				specPath.Child("clientConfig", "service", "name"),
 				e.Spec.ClientConfig.Service.Name,
-				"invalid name",
+				msg,
 			))
 		}
 
@@ -176,11 +177,11 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
 			))
 		}
 
-		if errs := validation.IsDNS1123Label(e.Spec.ClientConfig.Service.Namespace); len(errs) != 0 {
+		for _, msg := range validation.IsDNS1123Label(e.Spec.ClientConfig.Service.Namespace) {
 			allErrs = append(allErrs, field.Invalid(
-				specPath.Child("clientConfig", "service", "name"),
+				specPath.Child("clientConfig", "service", "namespace"),
 				e.Spec.ClientConfig.Service.Name,
-				"invalid namespace",
+				msg,
 			))
 		}
 
@@ -190,23 +191,23 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
 				allErrs = append(allErrs, field.Invalid(
 					specPath.Child("clientConfig", "service", "path"),
 					path,
-					"must be a valid URL path e.g. /path/to/hook",
+					fmt.Sprintf("must be a valid URL path e.g. /path/to/hook: %v", err),
 				))
 			}
-			if path[0:1] != "/" {
+			if !strings.HasPrefix(path, "/") {
 				allErrs = append(allErrs, field.Invalid(
 					specPath.Child("clientConfig", "service", "path"),
 					path,
-					"must be a valid URL path e.g. /path/to/hook",
+					"must start with \"/\" to be a valid URL path",
 				))
 			}
 		}
 		if e.Spec.ClientConfig.Service.Port != nil {
-			if errs := validation.IsValidPortNum(int(*e.Spec.ClientConfig.Service.Port)); len(errs) != 0 {
+			for _, msg := range validation.IsValidPortNum(int(*e.Spec.ClientConfig.Service.Port)) {
 				allErrs = append(allErrs, field.Invalid(
 					specPath.Child("clientConfig", "service", "port"),
 					*e.Spec.ClientConfig.Service.Port,
-					"must be in range 1-65535 inclusive",
+					msg,
 				))
 			}
 		}

diff --git a/internal/webhooks/runtime/extensionconfig_webhook_test.go b/internal/webhooks/runtime/extensionconfig_webhook_test.go
@@ -231,7 +231,7 @@ func TestExtensionConfigValidate(t *testing.T) {
 			expectErr:   true,
 		},
 		{
-			name:        "creation should fail if no Service Namespace is defined",
+			name:        "creation should fail if Service Namespace violates Kubernetes naming rules",
 			in:          extensionWithBadServiceNamespace,
 			featureGate: true,
 			expectErr:   true,