Merge pull request #127 from hashicorp/vm-run-collision

[HVS] Warn on run collision
hashicorp · Jul 8, 2024 · 8d6ff8b · 8d6ff8b
2 parents 7039c52 + f9193ee
commit 8d6ff8b
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 19 deletions.
diff --git a/.changelog/127.txt b/.changelog/127.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+vault-secrets: issue an error if formatted secret names collide during a run command
+```
diff --git a/internal/commands/vaultsecrets/run/run.go b/internal/commands/vaultsecrets/run/run.go
@@ -13,7 +13,9 @@ import (
 
 	"github.com/hashicorp/go-hclog"
 	preview_secret_service "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/client/secret_service"
+	"github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/models"
 	"github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/stable/2023-06-13/client/secret_service"
+
 	"github.com/hashicorp/hcp/internal/commands/vaultsecrets/apps/helper"
 	"github.com/hashicorp/hcp/internal/commands/vaultsecrets/secrets/appname"
 	"github.com/hashicorp/hcp/internal/pkg/cmd"
@@ -148,23 +150,51 @@ func getAllSecretsForEnv(opts *RunOpts) ([]string, error) {
 	}
 
 	result := os.Environ()
+	collisions := make(map[string][]*models.Secrets20231128OpenSecret, 0)
 
 	for _, secret := range res.Payload.Secrets {
 		// we need to append results in case of duplicates we want secrets to override
 		switch {
 		case secret.RotatingVersion != nil:
 			for name, value := range secret.RotatingVersion.Values {
-				result = append(result, fmt.Sprintf("%v_%v=%v", strings.ToUpper(secret.Name), strings.ToUpper(name), value))
+				fmtName := fmt.Sprintf("%v_%v", strings.ToUpper(secret.Name), strings.ToUpper(name))
+				collisions[fmtName] = append(collisions[fmtName], secret)
+				result = append(result, fmt.Sprintf("%v=%v", fmtName, value))
 			}
 		case secret.DynamicInstance != nil:
 			for name, value := range secret.DynamicInstance.Values {
-				result = append(result, fmt.Sprintf("%v_%v=%v", strings.ToUpper(secret.Name), strings.ToUpper(name), value))
+				fmtName := fmt.Sprintf("%v_%v", strings.ToUpper(secret.Name), strings.ToUpper(name))
+				collisions[fmtName] = append(collisions[fmtName], secret)
+				result = append(result, fmt.Sprintf("%v=%v", fmtName, value))
 			}
 		case secret.StaticVersion != nil:
-			result = append(result, fmt.Sprintf("%v=%v", strings.ToUpper(secret.Name), secret.StaticVersion.Value))
+			fmtName := strings.ToUpper(secret.Name)
+			collisions[fmtName] = append(collisions[fmtName], secret)
+			result = append(result, fmt.Sprintf("%v=%v", fmtName, secret.StaticVersion.Value))
+		}
+	}
+
+	// check collisions and emit an error for each
+	hasCollisions := false
+	for fmtName, uses := range collisions {
+		if len(uses) > 1 {
+			hasCollisions = true
+			var offenders []string
+			for _, use := range uses {
+				offenders = append(offenders, fmt.Sprintf("\"%s\" [%s]", use.Name, use.Type))
+			}
+			_, err = fmt.Fprintf(opts.IO.Err(), "%s %s map to the same environment variable \"%s\"\n",
+				opts.IO.ColorScheme().ErrorLabel(), strings.Join(offenders, ", "), fmtName)
+			if err != nil {
+				return nil, err
+			}
 		}
 	}
 
+	if hasCollisions {
+		return nil, fmt.Errorf("multiple secrets map to the same environment variable")
+	}
+
 	return result, nil
 }
 

diff --git a/internal/commands/vaultsecrets/run/run_test.go b/internal/commands/vaultsecrets/run/run_test.go
@@ -17,6 +17,7 @@ import (
 
 	preview_secret_service "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/client/secret_service"
 	preview_models "github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/models"
+
 	mock_preview_secret_service "github.com/hashicorp/hcp/internal/pkg/api/mocks/github.com/hashicorp/hcp-sdk-go/clients/cloud-vault-secrets/preview/2023-11-28/client/secret_service"
 	"github.com/hashicorp/hcp/internal/pkg/cmd"
 	"github.com/hashicorp/hcp/internal/pkg/format"
@@ -98,21 +99,67 @@ func TestRunRun(t *testing.T) {
 	}
 
 	cases := []struct {
-		Name       string
-		RespErr    bool
-		ErrMsg     string
-		MockCalled bool
+		Name            string
+		Secrets         []*preview_models.Secrets20231128OpenSecret
+		RespErr         bool
+		ErrMsg          string
+		IOErrorContains string
+		MockCalled      bool
 	}{
 		{
 			Name:       "Failed: Secret not found",
 			RespErr:    true,
+			Secrets:    nil,
 			ErrMsg:     "[GET /secrets/2023-11-28/organizations/{organization_id}/projects/{project_id}/apps/{app_name}/secrets:open][403]",
 			MockCalled: true,
 		},
 		{
 			Name:       "Success",
 			RespErr:    false,
 			MockCalled: true,
+			Secrets: []*preview_models.Secrets20231128OpenSecret{
+				{
+					Name:          "static",
+					StaticVersion: &preview_models.Secrets20231128OpenSecretStaticVersion{},
+				},
+				{
+					Name: "rotating",
+					RotatingVersion: &preview_models.Secrets20231128OpenSecretRotatingVersion{
+						Values: map[string]string{"sub_key": "value"},
+					},
+				},
+				{
+					Name: "dynamic",
+					DynamicInstance: &preview_models.Secrets20231128OpenSecretDynamicInstance{
+						Values: map[string]string{"sub_key": "value"},
+					},
+				},
+			},
+		},
+		{
+			Name:            "Collide",
+			RespErr:         false,
+			MockCalled:      true,
+			ErrMsg:          "multiple secrets map to the same environment variable",
+			IOErrorContains: "ERROR: \"static_collision\" [static], \"static\" [rotating] map to the same environment variable \"STATIC_COLLISION\"",
+			Secrets: []*preview_models.Secrets20231128OpenSecret{
+				{
+					Name:          "static_collision",
+					Type:          "static",
+					LatestVersion: 1,
+					CreatedAt:     strfmt.DateTime(time.Now()),
+					StaticVersion: &preview_models.Secrets20231128OpenSecretStaticVersion{},
+				},
+				{
+					Name:          "static",
+					Type:          "rotating",
+					LatestVersion: 1,
+					CreatedAt:     strfmt.DateTime(time.Now()),
+					RotatingVersion: &preview_models.Secrets20231128OpenSecretRotatingVersion{
+						Values: map[string]string{"collision": ""},
+					},
+				},
+			},
 		},
 	}
 
@@ -146,18 +193,7 @@ func TestRunRun(t *testing.T) {
 						Context:        opts.Ctx,
 					}, nil).Return(&preview_secret_service.OpenAppSecretsOK{
 						Payload: &preview_models.Secrets20231128OpenAppSecretsResponse{
-							Secrets: []*preview_models.Secrets20231128OpenSecret{
-								{
-									Name:          "secret_1",
-									LatestVersion: 2,
-									CreatedAt:     strfmt.DateTime(time.Now()),
-								},
-								{
-									Name:          "secret_2",
-									LatestVersion: 2,
-									CreatedAt:     strfmt.DateTime(time.Now()),
-								},
-							},
+							Secrets: c.Secrets,
 						},
 					}, nil).Once()
 				}
@@ -166,6 +202,9 @@ func TestRunRun(t *testing.T) {
 			// Run the command
 			err := runRun(opts)
 			if c.ErrMsg != "" {
+				// Check for additional error messages
+				r.Contains(io.Error.String(), c.IOErrorContains)
+
 				r.Contains(err.Error(), c.ErrMsg)
 				return
 			}