Sanitize namespace flag consistently (#220)

Co-authored-by: Tony Li <[email protected]>

Author: hoffm

main.go

@@ -381,13 +381,22 @@ func checkParams(c *cli.Context) error {
 		return fmt.Errorf("Missing required param: cluster")
 	}
 
+	namespace := c.String("namespace")
+	c.Set("namespace", sanitizeNamespace(namespace))
+
 	if err := validateKubectlVersion(c, extraKubectlVersions); err != nil {
 		return err
 	}
 
 	return nil
 }
 
+func sanitizeNamespace(namespace string) string {
+	namespace = strings.ToLower(namespace)
+	namespace = invalidNameRegex.ReplaceAllString(namespace, "-")
+	return namespace
+}
+
 // validateKubectlVersion tests whether a given version is valid within the current environment
 func validateKubectlVersion(c *cli.Context, availableVersions []string) error {
 	kubectlVersionParam := c.String("kubectl-version")
@@ -729,10 +738,6 @@ func setNamespace(c *cli.Context, project string, runner Runner) error {
 		return nil
 	}
 
-	//replace invalid char in namespace
-	namespace = strings.ToLower(namespace)
-	namespace = invalidNameRegex.ReplaceAllString(namespace, "-")
-
 	// Set the execution namespace.
 	log("Configuring kubectl to the %s namespace\n", namespace)
 

main_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"strconv"
 	"strings"
@@ -73,6 +72,17 @@ func TestCheckParams(t *testing.T) {
 	set.String("cluster", "cluster-0", "")
 	err = checkParams(c)
 	assert.NoError(t, err)
+
+	// Sanitizes namespace
+	set = flag.NewFlagSet("namespace-sanitize", 0)
+	c = cli.NewContext(nil, set, nil)
+	set.String("token", "{}", "")
+	set.String("region", "us-west1", "")
+	set.String("cluster", "cluster-0", "")
+	set.String("namespace", "feature/1892-TEST-NS", "")
+	err = checkParams(c)
+	assert.NoError(t, err)
+	assert.Equal(t, "feature-1892-test-ns", c.String("namespace"))
 }
 
 func TestValidateKubectlVersion(t *testing.T) {
@@ -197,7 +207,8 @@ func TestFetchCredentials(t *testing.T) {
 	assert.NoError(t, regionalErr)
 
 	// Verify token file
-	buf, err := ioutil.ReadFile("/tmp/gcloud.json")
+	buf, err := os.ReadFile("/tmp/gcloud.json")
+	assert.NoError(t, err)
 	assert.Equal(t, "{\"key\", \"val\"}", string(buf))
 
 	// Run() error
@@ -259,13 +270,13 @@ func TestTemplateData(t *testing.T) {
 
 	// Variable overrides existing ones
 	vars = map[string]interface{}{"zone": "us-east4-b"}
-	tmplData, secretsData, secretsDataRedacted, err = templateData(c, "us-east1-b", vars, secrets)
+	_, _, _, err = templateData(c, "us-east1-b", vars, secrets)
 	assert.Error(t, err)
 
 	// Secret overrides variable
 	vars = map[string]interface{}{"SECRET_TEST": "val0"}
 	secrets = map[string]string{"SECRET_TEST": "test_val"}
-	tmplData, secretsData, secretsDataRedacted, err = templateData(c, "us-east1-b", vars, secrets)
+	_, _, _, err = templateData(c, "us-east1-b", vars, secrets)
 	assert.Error(t, err)
 }
 
@@ -324,19 +335,20 @@ func TestTemplateDataExpandingVars(t *testing.T) {
 
 	// Variable overrides existing ones
 	vars = map[string]interface{}{"zone": "us-east4-b"}
-	tmplData, secretsData, secretsDataRedacted, err = templateData(c, "us-east1-b", vars, secrets)
+	_, _, _, err = templateData(c, "us-east1-b", vars, secrets)
 	assert.Error(t, err)
 
 	// Secret overrides variable
 	vars = map[string]interface{}{"SECRET_TEST": "val0"}
 	secrets = map[string]string{"SECRET_TEST": "test_val"}
-	tmplData, secretsData, secretsDataRedacted, err = templateData(c, "us-east1-b", vars, secrets)
+	_, _, _, err = templateData(c, "us-east1-b", vars, secrets)
 	assert.Error(t, err)
 }
 
 func TestRenderTemplates(t *testing.T) {
 	// Mkdir for testing template files
-	os.MkdirAll("/tmp/drone-gke-tests/", os.ModePerm)
+	err := os.MkdirAll("/tmp/drone-gke-tests/", os.ModePerm)
+	assert.NoError(t, err)
 	kubeTemplatePath := "/tmp/drone-gke-tests/.kube.yml"
 	secretTemplatePath := "/tmp/drone-gke-tests/.kube.sec.yml"
 
@@ -359,32 +371,35 @@ func TestRenderTemplates(t *testing.T) {
 	// No template file, should error
 	os.Remove(kubeTemplatePath)
 	os.Remove(secretTemplatePath)
-	_, err := renderTemplates(c, tmplData, secretsData)
+	_, err = renderTemplates(c, tmplData, secretsData)
 	assert.Error(t, err)
 
 	// Normal
 	// Create test template files
 	tmplBuf := []byte("{{.COMMIT}}-{{.key0}}")
-	err = ioutil.WriteFile(kubeTemplatePath, tmplBuf, 0600)
+	err = os.WriteFile(kubeTemplatePath, tmplBuf, 0600)
 	assert.NoError(t, err)
 	tmplBuf = []byte("{{.COMMIT}}-{{.SECRET_TEST}}")
-	err = ioutil.WriteFile(secretTemplatePath, tmplBuf, 0600)
+	err = os.WriteFile(secretTemplatePath, tmplBuf, 0600)
 	assert.NoError(t, err)
 
 	// Render
 	manifestPaths, err := renderTemplates(c, tmplData, secretsData)
 	assert.NoError(t, err)
 
 	// Verify token files
-	buf, err := ioutil.ReadFile(manifestPaths[kubeTemplatePath])
+	buf, err := os.ReadFile(manifestPaths[kubeTemplatePath])
+	assert.NoError(t, err)
 	assert.Equal(t, "e0f21b90a-val0", string(buf))
 
-	buf, err = ioutil.ReadFile(manifestPaths[secretTemplatePath])
+	buf, err = os.ReadFile(manifestPaths[secretTemplatePath])
+	assert.NoError(t, err)
 	assert.Equal(t, "e0f21b90a-test_sec_val", string(buf))
 
 	// Secret variables shouldn't be available in kube template
 	tmplBuf = []byte("{{.SECRET_TEST}}")
-	err = ioutil.WriteFile(kubeTemplatePath, tmplBuf, 0600)
+	err = os.WriteFile(kubeTemplatePath, tmplBuf, 0600)
+	assert.NoError(t, err)
 	_, err = renderTemplates(c, tmplData, secretsData)
 	assert.Error(t, err)
 }
@@ -475,14 +490,15 @@ func TestSetNamespace(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Verify written file
-	buf, err := ioutil.ReadFile("/tmp/namespace.json")
+	buf, err := os.ReadFile("/tmp/namespace.json")
+	assert.NoError(t, err)
 	assert.Equal(t, "\n---\napiVersion: v1\nkind: Namespace\nmetadata:\n  name: test-ns\n", string(buf))
 
 	// Dry-run
 	set = flag.NewFlagSet("test-set", 0)
 	set.String("zone", "us-east1-b", "")
 	set.String("cluster", "cluster-0", "")
-	set.String("namespace", "Feature/1892-TEST-NS", "")
+	set.String("namespace", "feature-1892-test-ns", "") // the namespace is already sanitized by this point
 	set.Bool("dry-run", true, "")
 	set.Bool("create-namespace", true, "")
 	c = cli.NewContext(nil, set, nil)
@@ -498,7 +514,7 @@ func TestSetNamespace(t *testing.T) {
 	set = flag.NewFlagSet("no-create-namespace-set", 0)
 	set.String("zone", "us-east1-b", "")
 	set.String("cluster", "cluster-0", "")
-	set.String("namespace", "Feature/1892-TEST-NS", "")
+	set.String("namespace", "feature-1892-test-ns", "") // the namespace is already sanitized by this point
 	set.Bool("dry-run", false, "")
 	set.Bool("create-namespace", false, "")
 	c = cli.NewContext(nil, set, nil)
@@ -891,6 +907,7 @@ func TestSetDryRunFlag(t *testing.T) {
 			expectedFlag:    clientSideDryRunFlagDefault,
 		},
 	}
+
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			os.Clearenv()
@@ -918,7 +935,8 @@ func TestSetDryRunFlag(t *testing.T) {
 					}
 
 					// Run
-					setDryRunFlag(testRunner, buf, ctx)
+					err := setDryRunFlag(testRunner, buf, ctx)
+					assert.NoError(t, err)
 
 					// Check
 					if dryRunFlag != test.expectedFlag {
@@ -927,7 +945,6 @@ func TestSetDryRunFlag(t *testing.T) {
 					return nil
 				},
 			}).Run([]string{"run"})
-
 			if err != nil {
 				t.Fatalf("unepected err: %v", err)
 			}
@@ -988,3 +1005,49 @@ func Test_decodeToken(t *testing.T) {
 		})
 	}
 }
+
+func TestSanitizeNamespace(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "already sanitized",
+			input:    "test-ns",
+			expected: "test-ns",
+		},
+		{
+			name:     "uppercase characters",
+			input:    "TEST-NS",
+			expected: "test-ns",
+		},
+		{
+			name:     "mixed case",
+			input:    "Test-Ns",
+			expected: "test-ns",
+		},
+		{
+			name:     "special-chars",
+			input:    "feature/1892_test_ns",
+			expected: "feature-1892-test-ns",
+		},
+		{
+			name:     "uppercase and special chars",
+			input:    "feature/1892-TEST-NS",
+			expected: "feature-1892-test-ns",
+		},
+		{
+			name:     "empty",
+			input:    "",
+			expected: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			output := sanitizeNamespace(tt.input)
+			assert.Equal(t, tt.expected, output)
+		})
+	}
+}