Make sure to sanitize entity IDs before usage in labels

helm/names.go

@@ -3,15 +3,14 @@ package helm
 import (
 	"crypto/md5"
 	"fmt"
+	"regexp"
 )
 
 const (
 	NamespaceLabel       = "gruntwork.io/tiller-namespace"
 	CredentialsLabel     = "gruntwork.io/tiller-credentials"
 	CredentialsTypeLabel = "gruntwork.io/tiller-credentials-type"
-	CredentialsNameLabel = "gruntwork.io/tiller-credentials-name"
-	RoleNameLabel        = "gruntwork.io/tiller-access-role-name"
-	RoleBindingNameLabel = "gruntwork.io/tiller-access-rolebinding-name"
+	EntityIDLabel        = "gruntwork.io/tiller-entity-id"
 )
 
 // NOTE: RBAC has relaxed constraints for names compared to resource names. Specifically, RBAC names allow many more
@@ -25,12 +24,12 @@ func md5HashString(input string) string {
 	return fmt.Sprintf("%x", md5.Sum([]byte(input)))
 }
 
-func getTillerClientCertSecretLabels(entityID, namespace string) map[string]string {
+func getTillerClientCertSecretLabels(entityID string, namespace string) map[string]string {
 	return map[string]string{
 		NamespaceLabel:       namespace,
 		CredentialsLabel:     "true",
 		CredentialsTypeLabel: "client",
-		CredentialsNameLabel: getTillerClientCertSecretName(entityID),
+		EntityIDLabel:        sanitizeLabelValues(entityID),
 	}
 }
 
@@ -46,24 +45,41 @@ func getTillerCACertSecretLabels(namespace string) map[string]string {
 	}
 }
 
-func getTillerAccessRoleName(entityID, namespace string) string {
+func getTillerAccessRoleName(entityID string, namespace string) string {
 	return fmt.Sprintf("%s-%s-tiller-access", entityID, namespace)
 }
 
-func getTillerAccessRoleBindingName(entityID, roleName string) string {
+func getTillerAccessRoleBindingName(entityID string, roleName string) string {
 	return fmt.Sprintf("%s-%s-binding", entityID, roleName)
 }
 
-func getTillerRoleLabels(entityID, namespace string) map[string]string {
+// sanitizeLabelValues will sanitize the provided string so that it can be used as a kubernetes label value. Kubernetes
+// labels have the following restrictions:
+// - Must be 63 characters or less
+// - Begin and end with an alphanumeric character ([a-zA-Z0-9])
+// - Only contain dashes (-), underscores (_), dots (.), and alphanumeric characters ([a-zA-Z0-9])
+// This sanitization will handle the unsupported characters piece ONLY. Specifically, this will take all unsupported
+// characters and replace them with dashes (-). E.g if you have the value "foo@bar", this will be converted to
+// "foo-bar".
+func sanitizeLabelValues(value string) string {
+	re := regexp.MustCompile(`[^0-9A-Za-z-_.]`)
+	return re.ReplaceAllString(value, "-")
+}
+
+func getTillerRoleLabels(entityID string, namespace string) map[string]string {
+	// Here we only sanitize the role name because namespace names are already constrained with the same restrictions as
+	// node labels, and thus you can't create or reference a namespace that is not a valid label.
 	return map[string]string{
 		NamespaceLabel: namespace,
-		RoleNameLabel:  getTillerAccessRoleName(entityID, namespace),
+		EntityIDLabel:  sanitizeLabelValues(entityID),
 	}
 }
 
-func getTillerRoleBindingLabels(entityID, namespace string) map[string]string {
+func getTillerRoleBindingLabels(entityID string, namespace string) map[string]string {
+	// Here we only sanitize the role binding name because namespace names are already constrained with the same
+	// restrictions as node labels, and thus you can't create or reference a namespace that is not a valid label.
 	return map[string]string{
-		NamespaceLabel:       namespace,
-		RoleBindingNameLabel: getTillerAccessRoleBindingName(entityID, namespace),
+		NamespaceLabel: namespace,
+		EntityIDLabel:  sanitizeLabelValues(entityID),
 	}
 }

helm/names_test.go

@@ -0,0 +1,70 @@
+package helm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSanitizeLabelValues(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			"ReplaceUnsupportedChar",
+			"foo@bar",
+			"foo-bar",
+		},
+		{
+			"PreserveSupportedChars",
+			"foo-bar",
+			"foo-bar",
+		},
+		{
+			"PreserveCaps",
+			"FOO@BAR",
+			"FOO-BAR",
+		},
+		{
+			"EmptyString",
+			"",
+			"",
+		},
+		{
+			"MultipleUnsupportedChars",
+			"team@gruntwork.io$foobar",
+			"team-gruntwork.io-foobar",
+		},
+		{
+			"Numbers",
+			"1337",
+			"1337",
+		},
+	}
+
+	for _, testCase := range testCases {
+		// Capture range variable to bring it in scope of the for loop
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			assert.Equal(t, sanitizeLabelValues(testCase.input), testCase.expected)
+		})
+	}
+}
+
+func TestGetTillerRoleLabelsSanitizesValues(t *testing.T) {
+	t.Parallel()
+
+	labels := getTillerRoleLabels("foo@bar", "default")
+	assert.Equal(t, labels[EntityIDLabel], "foo-bar")
+}
+
+func TestGetTillerRoleBindingLabelsSanitizesValues(t *testing.T) {
+	t.Parallel()
+
+	labels := getTillerRoleBindingLabels("foo@bar", "default")
+	assert.Equal(t, labels[EntityIDLabel], "foo-bar")
+}