INFRA-3884 (#17)

* prevent helm namespace from overriding namespace flag

* update makefile to enable local tests

* fix failing unit tests

* update hardcoded vars

* update some more tests

* update deprecated pkg

* update tests

* update plugin version

* update makefile

* fmt

* Update cmd/ksec/main.go

Co-authored-by: Colin Hoglund <colinhoglund@users.noreply.github.com>

* add namespace check back

* update mocks to check for empty config namespace

* update dependent test

---------

Co-authored-by: Danny Petitclerc <danny.petitclerc@mongodb.com>
Co-authored-by: Colin Hoglund <colinhoglund@users.noreply.github.com>

Author: 3 people

Makefile

@@ -17,6 +17,11 @@ test:
 build:
 	go install ./cmd/ksec/
 
+.PHONY: install-plugin-local
+install-plugin-local: build
+	helm plugin remove $(CMD_NAME) || true
+	HELM_PLUGIN_BUILD_SOURCE=1 helm plugin install $(shell pwd)
+
 .PHONY: dist
 dist: dist-setup dist-linux dist-darwin dist-windows ## Cross compile binaries into ./dist/
 

cmd/ksec/main.go

@@ -57,7 +57,14 @@ func NewRootCmd() *cobra.Command {
 		Version: version.Get().Version,
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			var err error
-			secretsClient, err = models.NewSecretsClient(viper.GetString("namespace"))
+			namespace := viper.GetString("namespace")
+
+			// sets namespace when used as a helm plugin since helm hijacks the "--namespace" flag.
+			if namespace == "" && os.Getenv("HELM_NAMESPACE") != "" {
+				namespace = os.Getenv("HELM_NAMESPACE")
+			}
+
+			secretsClient, err = models.NewSecretsClient(namespace)
 			if err != nil {
 				log.Fatal(err.Error())
 			}

cmd/ksec/main_test.go

@@ -3,13 +3,13 @@ package main
 import (
 	"bufio"
 	"context"
-	"io/ioutil"
 	"os"
 	"strings"
 	"testing"
 
 	"github.com/kanopy-platform/ksec/pkg/models"
 	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
 )
 
 // mock rootCmd
@@ -21,113 +21,102 @@ func TestMain(m *testing.M) {
 		Short: "A tool for managing Kubernetes Secret data",
 	}
 	initRootCmd(rootCmd)
-	secretsClient = models.MockNewSecretsClient()
+	mockConfig := models.MockClientConfig()
+	secretsClient, _ = models.MockNewSecretsClient(mockConfig, "default")
 	os.Exit(m.Run())
-
-}
-
-// helpers
-func testErr(err error, t *testing.T) {
-	if err != nil {
-		t.Fatal(err)
-	}
 }
 
 func cmdExec(args []string) error {
 	rootCmd.SetArgs(args)
-	if err := rootCmd.Execute(); err != nil {
-		return err
-	}
-	return nil
+	return rootCmd.Execute()
 }
 
 // tests
 func TestCreateSecret(t *testing.T) {
 	ctx := context.Background()
 
 	err := cmdExec([]string{"create", "test"})
-	testErr(err, t)
+	assert.NoError(t, err, "Creating secret should not return an error")
 
 	err = cmdExec([]string{"set", "test", "key=value"})
-	testErr(err, t)
+	assert.NoError(t, err, "Setting secret key should not return an error")
 
 	secret, err := secretsClient.Get(ctx, "test")
-	testErr(err, t)
+	assert.NoError(t, err, "Getting secret should not return an error")
+	assert.NotNil(t, secret, "Secret should not be nil")
 
 	val, ok := secret.Data["key"]
-	if !ok {
-		t.Fatal("Key does not exist")
-	} else if string(val) != "value" {
-		t.Fatal("Key has incorrect value")
-	}
+	assert.True(t, ok, "Key should exist in secret data")
+	assert.Equal(t, "value", string(val), "Key should have the correct value")
 }
 
 func TestUnsetSecretKey(t *testing.T) {
 	ctx := context.Background()
+
 	err := cmdExec([]string{"unset", "test", "key"})
-	testErr(err, t)
+	assert.NoError(t, err, "Unsetting secret key should not return an error")
 
 	secret, err := secretsClient.Get(ctx, "test")
-	testErr(err, t)
+	assert.NoError(t, err, "Getting secret should not return an error")
+	assert.NotNil(t, secret, "Secret should not be nil")
 
-	if _, ok := secret.Data["key"]; ok {
-		t.Fatal("Key should not exist")
-	}
+	_, ok := secret.Data["key"]
+	assert.False(t, ok, "Key should not exist in secret data")
 }
 
 func TestDeleteSecret(t *testing.T) {
 	err := cmdExec([]string{"delete", "test", "--yes"})
-	testErr(err, t)
+	assert.NoError(t, err, "Deleting secret should not return an error")
 
 	err = cmdExec([]string{"get", "test"})
-	if !strings.HasSuffix(err.Error(), "not found") {
-		t.Fatal("Secret still exists")
-	}
+	assert.Error(t, err, "Getting deleted secret should return an error")
+	assert.True(t, strings.HasSuffix(err.Error(), "not found"), "Error should indicate that the secret was not found")
 }
 
 func TestPushSecret(t *testing.T) {
 	ctx := context.Background()
 	content := []byte("ENV_VAR=secret")
-	tempfile, err := ioutil.TempFile("", "ksec")
-	testErr(err, t)
+
+	tempfile, err := os.CreateTemp("", "ksec")
+	assert.NoError(t, err, "Creating temp file should not return an error")
 	defer os.Remove(tempfile.Name())
 
 	_, err = tempfile.Write(content)
-	testErr(err, t)
+	assert.NoError(t, err, "Writing to temp file should not return an error")
 
 	err = cmdExec([]string{"push", tempfile.Name(), "pushtest"})
-	testErr(err, t)
+	assert.NoError(t, err, "Pushing secret should not return an error")
 
 	secret, err := secretsClient.Get(ctx, "pushtest")
-	testErr(err, t)
+	assert.NoError(t, err, "Getting pushed secret should not return an error")
+	assert.NotNil(t, secret, "Pushed secret should not be nil")
 
 	val, ok := secret.Data["ENV_VAR"]
-	if !ok {
-		t.Fatal("Key does not exist")
-	} else if string(val) != "secret" {
-		t.Fatal("Key has incorrect value")
-	}
+	assert.True(t, ok, "ENV_VAR key should exist in pushed secret data")
+	assert.Equal(t, "secret", string(val), "ENV_VAR key should have the correct value")
 
 	err = tempfile.Close()
-	testErr(err, t)
+	assert.NoError(t, err, "Closing temp file should not return an error")
 }
 
 func TestPullSecret(t *testing.T) {
-	tempfile, err := ioutil.TempFile("", "ksec")
-	testErr(err, t)
+	tempfile, err := os.CreateTemp("", "ksec")
+	assert.NoError(t, err, "Creating temp file should not return an error")
 	defer os.Remove(tempfile.Name())
 
 	err = cmdExec([]string{"set", "pulltest", "ENV_VAR=secret"})
-	testErr(err, t)
+	assert.NoError(t, err, "Setting secret should not return an error")
 
 	err = cmdExec([]string{"pull", "pulltest", tempfile.Name()})
-	testErr(err, t)
+	assert.NoError(t, err, "Pulling secret should not return an error")
+
+	file, err := os.Open(tempfile.Name())
+	assert.NoError(t, err, "Opening temp file should not return an error")
+	defer file.Close()
 
-	reader := bufio.NewReader(tempfile)
+	reader := bufio.NewReader(file)
 	line, _, err := reader.ReadLine()
-	testErr(err, t)
+	assert.NoError(t, err, "Reading line from temp file should not return an error")
 
-	if string(line) != "ENV_VAR=secret" {
-		t.Fatal("File does not contain pulled contents")
-	}
+	assert.Equal(t, "ENV_VAR=secret", string(line), "File should contain the pulled secret contents")
 }

pkg/models/secrets_client_test.go

@@ -4,110 +4,152 @@ import (
 	"context"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var secretsClient *SecretsClient
-
-// helpers
-func testErr(err error, t *testing.T) {
-	if err != nil {
-		t.Fatal(err.Error())
-	}
+var expectedSecretName = "test-secret"
+var defaultNamespace = "default"
+var err error
+
+// setupTestClient initializes a new secretsClient before each test
+func setupTestClient(namespace string) {
+	mockConfig := MockClientConfig()
+	secretsClient, err = MockNewSecretsClient(mockConfig, namespace)
 }
 
-// unit tests
 func TestNewSecretsClient(t *testing.T) {
-	secretsClient = MockNewSecretsClient()
+	setupTestClient(defaultNamespace)
+
+	assert.NoError(t, err)
+	assert.Equal(t, defaultNamespace, secretsClient.Namespace)
+	assert.Equal(t, "testuser", secretsClient.AuthInfo)
+	assert.NotNil(t, secretsClient.secretInterface)
+}
+
+func TestNewSecretsClientUndefinedNamespace(t *testing.T) {
+	// undefined namespace, expected to fail
+	setupTestClient("")
+	assert.Error(t, err)
 }
 
 func TestCreate(t *testing.T) {
+	setupTestClient(defaultNamespace)
 	ctx := context.Background()
-	_, err := secretsClient.Create(ctx, "test-secret")
-	testErr(err, t)
+
+	secret, err := secretsClient.Create(ctx, expectedSecretName)
+	assert.NoError(t, err, "Creating secret should not return an error")
+
+	assert.NotNil(t, secret, "Created secret should not be nil")
+	assert.Equal(t, secret.Name, expectedSecretName)
 }
 
 func TestList(t *testing.T) {
+	setupTestClient(defaultNamespace)
 	ctx := context.Background()
-	secrets, err := secretsClient.List(ctx)
-	testErr(err, t)
 
-	if secrets.Items[0].Name != "test-secret" {
-		t.Fatal(err.Error())
-	}
+	_, err := secretsClient.Create(ctx, expectedSecretName)
+	assert.NoError(t, err, "Creating secret should not return an error")
+
+	secrets, err := secretsClient.List(ctx)
+	assert.NoError(t, err, "Listing secrets should not return an error")
+	assert.NotEmpty(t, secrets.Items, "Secrets list should not be empty")
+	assert.Equal(t, expectedSecretName, secrets.Items[0].Name, "First secret name should be %s", expectedSecretName)
 }
 
 func TestCreateWithData(t *testing.T) {
+	setupTestClient(defaultNamespace)
+	ctx := context.Background()
+
 	dataArgs := []string{
 		"key=value",
 		"secret-key=secret-value",
 		"ENV_VAR=~!@#$%^&*()_+",
 		"DB_URL=mongodb://host1.example.com:27017,host2.example.com:27017/prod?replicaSet=prod",
 	}
+
 	data := make(map[string][]byte)
 
 	for _, item := range dataArgs {
 		split := strings.SplitN(item, "=", 2)
-		if len(split) != 2 {
-			t.Errorf("Data is not formatted correctly: %s", item)
-		}
+		assert.Len(t, split, 2, "Data is not formatted correctly")
 		data[split[0]] = []byte(split[1])
 	}
 
-	ctx := context.Background()
-	_, err := secretsClient.CreateWithData(ctx, "test-secret-with-data", data)
-	testErr(err, t)
+	secret, err := secretsClient.CreateWithData(ctx, expectedSecretName, data)
+	assert.NoError(t, err, "Creating secret with data should not return an error")
+
+	assert.NotNil(t, secret.Data, "Created secret with data should not be nil")
+	assert.Equal(t, data, secret.Data)
 }
 
 func TestGet(t *testing.T) {
+	setupTestClient(defaultNamespace)
 	ctx := context.Background()
-	secret, err := secretsClient.Get(ctx, "test-secret-with-data")
-	testErr(err, t)
 
-	if string(secret.Data["DB_URL"]) != "mongodb://host1.example.com:27017,host2.example.com:27017/prod?replicaSet=prod" {
-		t.Fatal(err.Error())
-	}
+	_, err := secretsClient.CreateWithData(ctx, expectedSecretName, map[string][]byte{
+		"DB_URL": []byte("mongodb://host1.example.com:27017,host2.example.com:27017/prod?replicaSet=prod"),
+	})
+	assert.NoError(t, err, "Creating secret with data should not return an error")
+
+	secret, err := secretsClient.Get(ctx, expectedSecretName)
+	assert.NoError(t, err, "Getting secret should not return an error")
+	assert.Equal(t, "mongodb://host1.example.com:27017,host2.example.com:27017/prod?replicaSet=prod", string(secret.Data["DB_URL"]), "DB_URL should match the expected value")
 }
 
 func TestGetKey(t *testing.T) {
+	setupTestClient(defaultNamespace)
 	ctx := context.Background()
-	value, err := secretsClient.GetKey(ctx, "test-secret-with-data", "secret-key")
-	testErr(err, t)
 
-	if value != "secret-value" {
-		t.Fatal(err.Error())
-	}
+	_, err := secretsClient.CreateWithData(ctx, expectedSecretName, map[string][]byte{
+		"secret-key": []byte("secret-value"),
+	})
+	assert.NoError(t, err, "Creating secret with data should not return an error")
 
-	value, err = secretsClient.GetKey(ctx, "test-secret-with_data", "thiskeydoesnotexist")
-	if err == nil {
-		t.Fatal("non-existent key should have received an error")
-	}
+	value, err := secretsClient.GetKey(ctx, expectedSecretName, "secret-key")
+	assert.NoError(t, err, "Getting secret key should not return an error")
+	assert.Equal(t, "secret-value", value, "Secret key value should match the expected value")
+
+	value, err = secretsClient.GetKey(ctx, expectedSecretName, "thiskeydoesnotexist")
+	assert.Error(t, err, "Getting a non-existent key should return an error")
+	assert.Empty(t, value, "Non-existent key should return an empty value")
 }
 
 func TestUpdate(t *testing.T) {
+	setupTestClient(defaultNamespace)
 	ctx := context.Background()
-	secret, err := secretsClient.Get(ctx, "test-secret-with-data")
-	testErr(err, t)
+
+	secret, err := secretsClient.CreateWithData(ctx, expectedSecretName, map[string][]byte{
+		"key": []byte("oldvalue"),
+	})
+	assert.NoError(t, err, "Creating secret with data should not return an error")
 
 	data := map[string][]byte{
 		"key": []byte("newvalue"),
 	}
 
-	secretsClient.Update(ctx, secret, data)
-
-	if string(secret.Data["key"]) != "newvalue" {
-		t.Fatal(err.Error())
-	}
+	secret, err = secretsClient.Update(ctx, secret, data)
+	assert.NoError(t, err, "Updating secret should not return an error")
+	assert.Equal(t, "newvalue", string(secret.Data["key"]), "Key value should be updated to 'newvalue'")
 }
 
 func TestUpsert(t *testing.T) {
+	setupTestClient(defaultNamespace)
+	ctx := context.Background()
+
 	data := map[string][]byte{
 		"key": []byte("upsert"),
 	}
-	ctx := context.Background()
-	secret, err := secretsClient.Upsert(ctx, "upsert-secret", data)
-	testErr(err, t)
 
-	if string(secret.Data["key"]) != "upsert" {
-		t.Fatal(err.Error())
-	}
+	// First upsert (should create the secret)
+	secret, err := secretsClient.Upsert(ctx, expectedSecretName, data)
+	assert.NoError(t, err, "Upserting (creating) secret should not return an error")
+	assert.Equal(t, "upsert", string(secret.Data["key"]), "Key value should be 'upsert'")
+
+	// Second upsert (should update the secret)
+	data["key"] = []byte("upserted")
+	secret, err = secretsClient.Upsert(ctx, expectedSecretName, data)
+	assert.NoError(t, err, "Upserting (updating) secret should not return an error")
+	assert.Equal(t, "upserted", string(secret.Data["key"]), "Key value should be 'upserted'")
 }