address review comments

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

Hide file tree

Showing 2 changed files with 10 additions and 17 deletions.
diff --git a/internal/webhooks/runtime/extensionconfig_webhook.go b/internal/webhooks/runtime/extensionconfig_webhook.go
@@ -140,7 +140,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 +161,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 +176,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 +190,16 @@ 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",
-				))
-			}
-			if path[0:1] != "/" {
-				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 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,