From a57e32beced02e493ebfaff4c241390753034cbb Mon Sep 17 00:00:00 2001
From: killianmuldoon <kmuldoon@vmware.com>
Date: Tue, 14 Mar 2023 10:36:40 +0000
Subject: [PATCH] Update handler name validation rules

---
 exp/runtime/server/server.go                  |  1 +
 internal/runtime/client/client.go             |  4 ++--
 internal/runtime/client/client_test.go        | 19 ++++++++++++++++++-
 .../runtime/extensionconfig_webhook.go        |  8 ++++++++
 .../runtime/extensionconfig_webhook_test.go   |  9 +++++++++
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/exp/runtime/server/server.go b/exp/runtime/server/server.go
index bb38b5c920fd..1e2370bb7c8a 100644
--- a/exp/runtime/server/server.go
+++ b/exp/runtime/server/server.go
@@ -112,6 +112,7 @@ type ExtensionHandler struct {
 	Hook runtimecatalog.Hook
 
 	// Name is the name of the extension handler.
+	// An extension handler name must be valid in line RFC 1123 Label Names.
 	Name string
 
 	// HandlerFunc is the handler function.
diff --git a/internal/runtime/client/client.go b/internal/runtime/client/client.go
index 4a12cfed0239..a6cd941a32c8 100644
--- a/internal/runtime/client/client.go
+++ b/internal/runtime/client/client.go
@@ -586,8 +586,8 @@ func defaultAndValidateDiscoveryResponse(cat *runtimecatalog.Catalog, discovery
 		}
 		names[handler.Name] = true
 
-		// Name should match Kubernetes naming conventions - validated based on Kubernetes DNS1123 Subdomain rules.
-		if errStrings := validation.IsDNS1123Subdomain(handler.Name); len(errStrings) > 0 {
+		// Name should match Kubernetes naming conventions - validated based on DNS1123 label rules.
+		if errStrings := validation.IsDNS1123Label(handler.Name); len(errStrings) > 0 {
 			errs = append(errs, errors.Errorf("handler name %s is not valid: %s", handler.Name, errStrings))
 		}
 
diff --git a/internal/runtime/client/client_test.go b/internal/runtime/client/client_test.go
index 3949b4debeb8..27c46536cd94 100644
--- a/internal/runtime/client/client_test.go
+++ b/internal/runtime/client/client_test.go
@@ -362,7 +362,7 @@ func Test_defaultAndValidateDiscoveryResponse(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "error with name violating DNS1123",
+			name: "error if handler name has capital letters",
 			discovery: &runtimehooksv1.DiscoveryResponse{
 				TypeMeta: metav1.TypeMeta{
 					Kind:       "DiscoveryResponse",
@@ -378,6 +378,23 @@ func Test_defaultAndValidateDiscoveryResponse(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "error if handler name has full stops",
+			discovery: &runtimehooksv1.DiscoveryResponse{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "DiscoveryResponse",
+					APIVersion: runtimehooksv1.GroupVersion.String(),
+				},
+				Handlers: []runtimehooksv1.ExtensionHandler{{
+					Name: "has.full.stops",
+					RequestHook: runtimehooksv1.GroupVersionHook{
+						Hook:       "FakeHook",
+						APIVersion: fakev1alpha1.GroupVersion.String(),
+					},
+				}},
+			},
+			wantErr: true,
+		},
 		{
 			name: "error with Timeout of over 30 seconds",
 			discovery: &runtimehooksv1.DiscoveryResponse{
diff --git a/internal/webhooks/runtime/extensionconfig_webhook.go b/internal/webhooks/runtime/extensionconfig_webhook.go
index d9e49adcba4a..1f852fea09e9 100644
--- a/internal/webhooks/runtime/extensionconfig_webhook.go
+++ b/internal/webhooks/runtime/extensionconfig_webhook.go
@@ -104,6 +104,14 @@ func (webhook *ExtensionConfig) validate(_ context.Context, _, newExtensionConfi
 	}
 
 	var allErrs field.ErrorList
+
+	// Name should match Kubernetes naming conventions - validated based on DNS1123 label rules.
+	if errStrings := validation.IsDNS1123Label(newExtensionConfig.Name); len(errStrings) > 0 {
+		allErrs = append(allErrs, field.Invalid(
+			field.NewPath("metadata", "name"),
+			newExtensionConfig.Name,
+			fmt.Sprintf("ExtensionConfig name should be a valid DNS1123 label name: %s", errStrings)))
+	}
 	allErrs = append(allErrs, validateExtensionConfigSpec(newExtensionConfig)...)
 
 	if len(allErrs) > 0 {
diff --git a/internal/webhooks/runtime/extensionconfig_webhook_test.go b/internal/webhooks/runtime/extensionconfig_webhook_test.go
index d15dc311638a..1fa477cfaefc 100644
--- a/internal/webhooks/runtime/extensionconfig_webhook_test.go
+++ b/internal/webhooks/runtime/extensionconfig_webhook_test.go
@@ -161,6 +161,9 @@ func TestExtensionConfigValidate(t *testing.T) {
 	extensionWithServiceAndURL := extensionWithURL.DeepCopy()
 	extensionWithServiceAndURL.Spec.ClientConfig.Service = extensionWithService.Spec.ClientConfig.Service
 
+	extensionWithBadName := extensionWithURL.DeepCopy()
+	extensionWithBadName.Name = "bad.name"
+
 	// Valid updated Extension
 	updatedExtension := extensionWithURL.DeepCopy()
 	updatedExtension.Spec.ClientConfig.URL = pointer.String("https://a-in-extension-address.com")
@@ -238,6 +241,12 @@ func TestExtensionConfigValidate(t *testing.T) {
 			featureGate: true,
 			expectErr:   true,
 		},
+		{
+			name:        "creation should fail if extensionConfig Name violates Kubernetes naming rules",
+			in:          extensionWithBadName,
+			featureGate: true,
+			expectErr:   true,
+		},
 		{
 			name:        "creation should fail if Service Name violates Kubernetes naming rules",
 			in:          extensionWithBadServiceName,