From 524150c662f9c030d2caa9ad8f79d2ff9521c431 Mon Sep 17 00:00:00 2001
From: Matt Butcher <matt.butcher@microsoft.com>
Date: Fri, 1 May 2020 14:02:47 -0600
Subject: [PATCH] fix: use correct regular expression for Kubernetes names
 (#8013)

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
---
 pkg/action/action.go      | 13 +++++++------
 pkg/action/action_test.go | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/pkg/action/action.go b/pkg/action/action.go
index a8437d729..bb9ef5f71 100644
--- a/pkg/action/action.go
+++ b/pkg/action/action.go
@@ -62,16 +62,17 @@ var (
 	errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 53")
 )
 
-// ValidName is a regular expression for names.
+// ValidName is a regular expression for resource names.
 //
 // According to the Kubernetes help text, the regular expression it uses is:
 //
-//	(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?
+//	[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*
 //
-// We modified that. First, we added start and end delimiters. Second, we changed
-// the final ? to + to require that the pattern match at least once. This modification
-// prevents an empty string from matching.
-var ValidName = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$")
+// This follows the above regular expression (but requires a full string match, not partial).
+//
+// The Kubernetes documentation is here, though it is not entirely correct:
+// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+var ValidName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
 
 // Configuration injects the dependencies that all actions share.
 type Configuration struct {
diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go
index 36ef261a3..0cbdb162b 100644
--- a/pkg/action/action_test.go
+++ b/pkg/action/action_test.go
@@ -316,3 +316,40 @@ func TestGetVersionSet(t *testing.T) {
 		t.Error("Non-existent version is reported found.")
 	}
 }
+
+// TestValidName is a regression test for ValidName
+//
+// Kubernetes has strict naming conventions for resource names. This test represents
+// those conventions.
+//
+// See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+//
+// NOTE: At the time of this writing, the docs above say that names cannot begin with
+// digits. However, `kubectl`'s regular expression explicit allows this, and
+// Kubernetes (at least as of 1.18) also accepts resources whose names begin with digits.
+func TestValidName(t *testing.T) {
+	names := map[string]bool{
+		"":                          false,
+		"foo":                       true,
+		"foo.bar1234baz.seventyone": true,
+		"FOO":                       false,
+		"123baz":                    true,
+		"foo.BAR.baz":               false,
+		"one-two":                   true,
+		"-two":                      false,
+		"one_two":                   false,
+		"a..b":                      false,
+		"%^&#$%*@^*@&#^":            false,
+		"example:com":               false,
+		"example%%com":              false,
+	}
+	for input, expectPass := range names {
+		if ValidName.MatchString(input) != expectPass {
+			st := "fail"
+			if expectPass {
+				st = "succeed"
+			}
+			t.Errorf("Expected %q to %s", input, st)
+		}
+	}
+}