From 91fd0f22ef0e2c437bdab89099b28f3df0225c5d Mon Sep 17 00:00:00 2001 From: Patrick Mischler Date: Fri, 6 Sep 2024 17:11:08 +0200 Subject: [PATCH] Remote vault patch 1 (#1605) * add Secret to Argument struct * read path and secret to get vault secret * update vault test to test changes with path and key * rename argument secret to key to be allign with vault terminology * use Key as specified in the arguments at vault.go * update CHANGELOG.md * update CHANGELOG.md add link to issue * update documentation to reflect changes for remote.vault * remove unused imports * change attribut `key` to be optional * add if-else to allow both methods if-else to allow working with or without `key` argument * add test cases with and without key argument * fix typo and change required to "no" for key argument * adapt client to work with both variants * update vault_test.go to use path and key to fetch secret * add code documentation --- CHANGELOG.md | 4 + .../components/remote/remote.vault.md | 5 +- internal/component/remote/vault/client.go | 35 ++++++--- internal/component/remote/vault/vault.go | 1 + internal/component/remote/vault/vault_test.go | 78 ++++++++++++++++++- 5 files changed, 110 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69976fdf9c..20a406e29f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,10 @@ Main (unreleased) for `discovery.*` is reloaded in such a way that no new targets were discovered. (@ptodev, @thampiotr) +- Fixed an issue (see https://github.com/grafana/alloy/issues/1599) where specifying both path and key in the remote.vault `path` + configuration could result in incorrect URLs. The `path` and `key` arguments have been separated to allow for clear and accurate + specification of Vault secrets. (@PatMis16) + ### Other - Renamed standard library functions. Old names are still valid but are marked deprecated. (@wildum) diff --git a/docs/sources/reference/components/remote/remote.vault.md b/docs/sources/reference/components/remote/remote.vault.md index ad6bd5804d..072627bbf7 100644 --- a/docs/sources/reference/components/remote/remote.vault.md +++ b/docs/sources/reference/components/remote/remote.vault.md @@ -23,6 +23,7 @@ labels. remote.vault "LABEL" { server = "VAULT_SERVER" path = "VAULT_PATH" + key = "VAULT_KEY" // Alternatively, use one of the other auth.* mechanisms. auth.token { @@ -40,6 +41,7 @@ Name | Type | Description `server` | `string` | The Vault server to connect to. | | yes `namespace` | `string` | The Vault namespace to connect to (Vault Enterprise only). | | no `path` | `string` | The path to retrieve a secret from. | | yes +`key` | `string` | The key to retrieve a secret from. | | no `reread_frequency` | `duration` | Rate to re-read keys. | `"0s"` | no Tokens with a lease will be automatically renewed roughly two-thirds through their lease duration. @@ -290,7 +292,8 @@ local.file "vault_token" { remote.vault "remote_write" { server = "https://prod-vault.corporate.internal" - path = "secret/prometheus/remote_write" + path = "secret" + key = "prometheus/remote_write auth.token { token = local.file.vault_token.content diff --git a/internal/component/remote/vault/client.go b/internal/component/remote/vault/client.go index 09dd566ecb..822f7c9386 100644 --- a/internal/component/remote/vault/client.go +++ b/internal/component/remote/vault/client.go @@ -19,16 +19,31 @@ type secretStore interface { type kvStore struct{ c *vault.Client } func (ks *kvStore) Read(ctx context.Context, args *Arguments) (*vault.Secret, error) { - // Split the path so we know which kv mount we want to use. - pathParts := strings.SplitN(args.Path, "/", 2) - if len(pathParts) != 2 { - return nil, fmt.Errorf("missing mount path in %q", args.Path) - } - - kv := ks.c.KVv2(pathParts[0]) - kvSecret, err := kv.Get(ctx, pathParts[1]) - if err != nil { - return nil, err + var kvSecret *vault.KVSecret + var err error + + // If a key is provided, we use that to get the secret from the KV store. + // This allows for more flexibility in how secrets are stored in vault. + // eg. long/path/kv/secret/key where long/path/kv is the path and secret/key is the key. + if args.Key != "" { + kv := ks.c.KVv2(args.Path) + kvSecret, err = kv.Get(ctx, args.Key) + if err != nil { + return nil, err + } + } else { + // for backward compatibility, we assume the path is in the format path/secret + // Split the path so we know which kv mount we want to use. + pathParts := strings.SplitN(args.Path, "/", 2) + if len(pathParts) != 2 { + return nil, fmt.Errorf("missing mount path in %q", args.Path) + } + + kv := ks.c.KVv2(pathParts[0]) + kvSecret, err = kv.Get(ctx, pathParts[1]) + if err != nil { + return nil, err + } } // kvSecret.Data contains unwrapped data. Let's assign that to the raw secret diff --git a/internal/component/remote/vault/vault.go b/internal/component/remote/vault/vault.go index deb48caf08..2ebad77bd5 100644 --- a/internal/component/remote/vault/vault.go +++ b/internal/component/remote/vault/vault.go @@ -35,6 +35,7 @@ type Arguments struct { Namespace string `alloy:"namespace,attr,optional"` Path string `alloy:"path,attr"` + Key string `alloy:"key,attr,optional"` RereadFrequency time.Duration `alloy:"reread_frequency,attr,optional"` diff --git a/internal/component/remote/vault/vault_test.go b/internal/component/remote/vault/vault_test.go index d5d60dc3de..9ac5e1b1c8 100644 --- a/internal/component/remote/vault/vault_test.go +++ b/internal/component/remote/vault/vault_test.go @@ -37,7 +37,8 @@ func Test_GetSecrets(t *testing.T) { cfg := fmt.Sprintf(` server = "%s" - path = "secret/test" + path = "secret" + secret = "test" reread_frequency = "0s" @@ -70,7 +71,7 @@ func Test_GetSecrets(t *testing.T) { require.Equal(t, expectExports, actualExports) } -func Test_PollSecrets(t *testing.T) { +func Test_PollSecretsWithoutKey(t *testing.T) { var ( ctx = componenttest.TestContext(t) l = util.TestLogger(t) @@ -142,6 +143,79 @@ func Test_PollSecrets(t *testing.T) { } } +func Test_PollSecretsWithKey(t *testing.T) { + var ( + ctx = componenttest.TestContext(t) + l = util.TestLogger(t) + ) + + cli := getTestVaultServer(t) + + // Store a secret in value to use from the component. + _, err := cli.KVv2("secret").Put(ctx, "test", map[string]any{ + "key": "value", + }) + require.NoError(t, err) + + cfg := fmt.Sprintf(` + server = "%s" + path = "" + key = "secret/test" + + reread_frequency = "100ms" + + auth.token { + token = "%s" + } + `, cli.Address(), cli.Token()) + + var args Arguments + require.NoError(t, syntax.Unmarshal([]byte(cfg), &args)) + + ctrl, err := componenttest.NewControllerFromID(l, "remote.vault") + require.NoError(t, err) + + go func() { + require.NoError(t, ctrl.Run(ctx, args)) + }() + require.NoError(t, ctrl.WaitRunning(time.Minute)) + + // Get the initial secret. + { + require.NoError(t, ctrl.WaitExports(time.Minute)) + + var ( + expectExports = Exports{ + Data: map[string]alloytypes.Secret{ + "key": alloytypes.Secret("value"), + }, + } + actualExports = ctrl.Exports().(Exports) + ) + require.Equal(t, expectExports, actualExports) + } + + // Get an updated secret. + { + _, err := cli.KVv2("secret").Put(ctx, "test", map[string]any{ + "key": "newvalue", + }) + require.NoError(t, err) + + require.NoError(t, ctrl.WaitExports(time.Minute)) + + var ( + expectExports = Exports{ + Data: map[string]alloytypes.Secret{ + "key": alloytypes.Secret("newvalue"), + }, + } + actualExports = ctrl.Exports().(Exports) + ) + require.Equal(t, expectExports, actualExports) + } +} + func getTestVaultServer(t *testing.T) *vaultapi.Client { // TODO: this is broken with go 1.20.6 // waiting on https://github.com/testcontainers/testcontainers-go/issues/1359