Skip to content

Commit

Permalink
Write env variable for single key, if not also written to file
Browse files Browse the repository at this point in the history
This fixes a bug where `SECRET_ENV=true` only worked if `DAYTONA_SECRET_DESTINATION_THING` is also set, even though that is not expected.

Fix cruise-automation#99
  • Loading branch information
jonnylangefeld committed Apr 6, 2023
1 parent f9f7dce commit 8d086b8
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 10 deletions.
43 changes: 33 additions & 10 deletions pkg/secrets/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type SecretDefinition struct {
paths []string
secrets map[string]string
plural bool
secretEnv bool
}

// SecretFetcher inspects the environment for variables that
Expand Down Expand Up @@ -84,6 +85,7 @@ func SecretFetcher(client *api.Client, config cfg.Config) {
envkey: envKey,
secretApex: apex,
secrets: make(map[string]string),
secretEnv: config.SecretEnv,
}

switch {
Expand Down Expand Up @@ -285,20 +287,23 @@ func (sd *SecretDefinition) addSecrets(secretResult *SecretResult) error {
return fmt.Errorf("vault listed a secret %s %s, but failed trying to read it; likely the rate-limiting retry attempts were exceeded", keyName, keyPath)
}

envKey := os.Getenv(secretValueKeyPrefix + sd.secretID)
if envKey != "" {
log.Info().Str("key", secretValueKeyPrefix+sd.secretID).Msg("Found an explicit vault value key, will read this value key instead of using the default")
}

if !sd.plural && sd.secretEnv && envKey != "" {
if err := sd.copyValue(secretData, envKey); err != nil {
return err
}
}

if !sd.plural && sd.outputDestination != "" {
singleValueKey := defaultKeyName
if envKey := os.Getenv(secretValueKeyPrefix + sd.secretID); envKey != "" {
log.Info().Str("key", secretValueKeyPrefix+sd.secretID).Str("value", singleValueKey).Msg("Found an explicit vault value key, will read this value key instead of using the default")
if envKey != "" {
singleValueKey = envKey
}
v, ok := secretData[singleValueKey]
if ok {
secretValue, err := valueConverter(v)
if err == nil {
sd.Lock()
sd.secrets[singleValueKey] = secretValue
sd.Unlock()
}
if err := sd.copyValue(secretData, singleValueKey); err != nil {
return err
}
}
Expand All @@ -321,6 +326,24 @@ func (sd *SecretDefinition) addSecrets(secretResult *SecretResult) error {
return nil
}

// copyValues copies a value from the secretData object returned by vault and writes it into the secrets map of the
// SecretDefintion
func (sd *SecretDefinition) copyValue(secretData map[string]interface{}, key string) error {
v, ok := secretData[key]
if ok {
secretValue, err := valueConverter(v)
if err == nil {
sd.Lock()
sd.secrets[key] = secretValue
sd.Unlock()
return nil
} else {
return err
}
}
return nil
}

// Walk walks a SecretDefintions SecretApex. This is used for iteration
// of the provided apex path
func (sd *SecretDefinition) Walk(client *api.Client) error {
Expand Down
88 changes: 88 additions & 0 deletions pkg/secrets/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,3 +804,91 @@ func TestSecretSingularDestinationKeyOverride(t *testing.T) {

assert.Equal(t, "nonstandard", string(data))
}

func TestSecretSingularEnvKeyOverride(t *testing.T) {
var config cfg.Config

ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, `
{
"auth": null,
"data": {
"username": "alice",
"password": "p@ssw0rd"
},
"lease_duration": 3600,
"lease_id": "",
"renewable": false
}
`)
}))
defer ts.Close()
client, err := testhelpers.GetTestClient(ts.URL)
if err != nil {
t.Fatal(err)
}

os.Setenv("VAULT_SECRET_APPLICATIONA", "secret/applicationA")
os.Setenv("VAULT_VALUE_KEY_APPLICATIONA", "password")

defer os.Unsetenv("VAULT_SECRET_APPLICATIONA")
defer os.Unsetenv("VAULT_VALUE_KEY_APPLICATIONA")
defer os.Unsetenv("password")

config.Workers = 3
config.SecretEnv = true
SecretFetcher(client, config)

assert.Equal(t, "p@ssw0rd", os.Getenv("password"))
}

func TestCopyValue(t *testing.T) {
tests := []struct {
name string
secretData map[string]interface{}
key string
expectedSecrets map[string]string
expectedError error
}{
{
name: "copy value",
secretData: map[string]interface{}{
"foo": "bar",
},
key: "foo",
expectedSecrets: map[string]string{
"foo": "bar",
},
expectedError: nil,
},
{
name: "value not found",
secretData: map[string]interface{}{
"foo": "bar",
},
key: "baz",
expectedSecrets: map[string]string{},
expectedError: nil,
},
{
name: "value conversion error",
secretData: map[string]interface{}{
"foo": 42,
},
key: "foo",
expectedSecrets: map[string]string{},
expectedError: fmt.Errorf("unsupported value type retrieved from vault: int"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sd := &SecretDefinition{secrets: map[string]string{}}

err := sd.copyValue(tt.secretData, tt.key)

assert.Equal(t, tt.expectedSecrets, sd.secrets)
assert.Equal(t, tt.expectedError, err)
})
}
}

0 comments on commit 8d086b8

Please sign in to comment.