fix: fix vault-env generated env which may contain duplicates

Since https://github.com/banzaicloud/bank-vaults/pull/1697 we are experimenting issues with `vault-env`: the generated environment has all the `vault:` env vars duplicated, one version in its original version and a second one with the secret instead of the token. I have noticed that the new `preprocessTransitSecrets` is calling `inject` even if nothing has been replaced with values from the transit cache. In case you are working with "maps" (ConfigMap, Secret) this does not matter as the "old" code will still overwrite the key with the secret. But in the case of `vault-env` you will have the same env var set twice and most the the time only the first occurence (without the secret) is taken into account.
backmarket-oss · Feb 1, 2023 · 4d22cfa · 4d22cfa
1 parent fedc9f4
commit 4d22cfa
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 2 deletions.
diff --git a/internal/injector/injector.go b/internal/injector/injector.go
@@ -143,12 +143,20 @@ func (i SecretInjector) preprocessTransitSecrets(references *map[string]string,
 
 	for name, value := range *references {
 		if HasInlineVaultDelimiters(value) {
+			newValue := value
 			for _, vaultSecretReference := range FindInlineVaultDelimiters(value) {
 				if v, ok := i.transitCache[vaultSecretReference[0]]; ok {
-					value = strings.Replace(value, vaultSecretReference[0], string(v), -1)
+					newValue = strings.Replace(value, vaultSecretReference[0], string(v), -1)
 				}
 			}
-			inject(name, value)
+
+			// Only inject the value if its content has been updated using the transit cache
+			if value != newValue {
+				inject(name, value)
+
+				// Delete the key from the references to avoid a double processing by the old logic
+				delete(*references, name)
+			}
 
 			continue
 		}

diff --git a/internal/injector/injector_test.go b/internal/injector/injector_test.go
@@ -32,6 +32,11 @@ import (
 	"github.com/banzaicloud/bank-vaults/pkg/sdk/vault"
 )
 
+func assertKeyDoesNotExist(t *testing.T, m map[string]string, k string) {
+	_, hasKey := m[k]
+	assert.False(t, hasKey)
+}
+
 func TestSecretInjector(t *testing.T) {
 	t.Parallel()
 
@@ -93,6 +98,7 @@ func TestSecretInjector(t *testing.T) {
 
 		results := map[string]string{}
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}
 
@@ -128,6 +134,7 @@ func TestSecretInjector(t *testing.T) {
 
 		results := map[string]string{}
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}
 
@@ -144,6 +151,7 @@ func TestSecretInjector(t *testing.T) {
 
 		results := map[string]string{}
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}
 
@@ -189,6 +197,7 @@ func TestSecretInjectorFromPath(t *testing.T) {
 		results := map[string]string{}
 
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}
 
@@ -208,6 +217,7 @@ func TestSecretInjectorFromPath(t *testing.T) {
 		results := map[string]string{}
 
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}
 
@@ -229,6 +239,7 @@ func TestSecretInjectorFromPath(t *testing.T) {
 
 		results := map[string]string{}
 		injectFunc := func(key, value string) {
+			assertKeyDoesNotExist(t, results, key)
 			results[key] = value
 		}