diff --git a/kv/internal/resolve/refs.go b/kv/internal/resolve/refs.go index 92f14b89..02ff34fa 100644 --- a/kv/internal/resolve/refs.go +++ b/kv/internal/resolve/refs.go @@ -12,6 +12,13 @@ type refKey struct { fragment string } +// ParseWholeValue is the exported boundary over parseWholeValue for callers +// outside the resolve engine that must parse a kv:// reference without resolving it. +func ParseWholeValue(input string) (store, path, fragment string, ok bool, err error) { + rk, ok, err := parseWholeValue(input) + return rk.store, rk.path, rk.fragment, ok, err +} + // parseWholeValue parses a whole-value reference of the form // "kv://store/path#frag". // diff --git a/kv/internal/store/store.go b/kv/internal/store/store.go index e1b9e256..649622cf 100644 --- a/kv/internal/store/store.go +++ b/kv/internal/store/store.go @@ -12,8 +12,6 @@ import ( "golang.org/x/sync/singleflight" ) -const defaultProviderTimeout = 5 * time.Second - // SecretStore is an internal decorator that adds caching and singleflight to a Provider. type SecretStore struct { name string @@ -169,7 +167,7 @@ func NewSecretStore( cache: cache, sf: &singleflight.Group{}, sfRefresh: &singleflight.Group{}, - timeout: defaultProviderTimeout, + timeout: kv.DefaultOperationTimeout, } for _, opt := range opts { diff --git a/kv/internal/store/store_test.go b/kv/internal/store/store_test.go index a86e3945..b9dbd20a 100644 --- a/kv/internal/store/store_test.go +++ b/kv/internal/store/store_test.go @@ -82,7 +82,7 @@ func TestNewSecretStore(t *testing.T) { }) require.NoError(t, err) require.NotNil(t, store) - require.Equal(t, defaultProviderTimeout, store.timeout) + require.Equal(t, kv.DefaultOperationTimeout, store.timeout) }) t.Run("cache disabled", func(t *testing.T) { diff --git a/kv/provider.go b/kv/provider.go index 8fe09c83..ee181617 100644 --- a/kv/provider.go +++ b/kv/provider.go @@ -42,6 +42,10 @@ const ( Conjur ProviderType = "cyberark_conjur" ) +// DefaultOperationTimeout bounds a single provider Get/Set when neither the store +// config nor the SecretStore wrapper supplies one. +const DefaultOperationTimeout = 5 * time.Second + // IsLocal reports whether this provider type resolves secrets from resources // available to the local process — environment variables, inline config data, // or the filesystem — requiring no network and a literal, reference-free config. @@ -101,11 +105,17 @@ type Initializer interface { Init(ctx context.Context) error } +// Setter is an optional interface for providers that support writing values +// back to their backend. +type Setter interface { + Set(ctx context.Context, key, value string) error +} + // Lister is an optional interface for providers that support enumerating -// keys by prefix. This enables dynamic discovery of available secrets +// keys & values by prefix. This enables dynamic discovery of available secrets // and operational tooling. type Lister interface { - List(ctx context.Context, prefix string) ([]string, error) + List(ctx context.Context, prefix string) (map[string]string, error) } // Closer is an optional interface for providers that need graceful shutdown @@ -126,6 +136,12 @@ type Timeouter interface { Timeout() time.Duration } +// AsSetter attempts to extract a Setter from a Provider, +// automatically unwrapping decorators. +func AsSetter(p Provider) (Setter, bool) { + return As[Setter](p) +} + // AsLister attempts to extract a Lister from a Provider, // automatically unwrapping decorators. func AsLister(p Provider) (Lister, bool) { @@ -179,3 +195,13 @@ func As[T any](p Provider) (T, bool) { return zero, false } + +// EffectiveTimeout resolves a configured timeout to the value actually used: +// the configured value when positive, else the default. +func EffectiveTimeout(configured time.Duration) time.Duration { + if configured > 0 { + return configured + } + + return DefaultOperationTimeout +} diff --git a/kv/providers/consul/consul.go b/kv/providers/consul/consul.go index a0d26624..0c885ee1 100644 --- a/kv/providers/consul/consul.go +++ b/kv/providers/consul/consul.go @@ -10,11 +10,13 @@ package consul import ( "context" "encoding/json" + "errors" "fmt" + "strings" "time" "github.com/TykTechnologies/storage/kv" - "github.com/hashicorp/consul/api" + consulsdk "github.com/hashicorp/consul/api" ) // Config is the JSON "config" block of a consul store. @@ -74,7 +76,7 @@ func NewFactory() kv.ProviderFactory { } } - clientCfg := api.DefaultConfig() + clientCfg := consulsdk.DefaultConfig() if conf.Address != "" { clientCfg.Address = conf.Address @@ -89,7 +91,7 @@ func NewFactory() kv.ProviderFactory { } if conf.HttpAuth.Username != "" || conf.HttpAuth.Password != "" { - clientCfg.HttpAuth = &api.HttpBasicAuth{ + clientCfg.HttpAuth = &consulsdk.HttpBasicAuth{ Username: conf.HttpAuth.Username, Password: conf.HttpAuth.Password, } @@ -112,7 +114,7 @@ func NewFactory() kv.ProviderFactory { applyTLSConfig(clientCfg, &conf) - client, err := api.NewClient(clientCfg) + client, err := consulsdk.NewClient(clientCfg) if err != nil { return nil, fmt.Errorf("consul: create client: %w", err) } @@ -121,7 +123,7 @@ func NewFactory() kv.ProviderFactory { } } -func applyTLSConfig(clientCfg *api.Config, conf *Config) { +func applyTLSConfig(clientCfg *consulsdk.Config, conf *Config) { tls := conf.TLSConfig if tls.Address != "" { @@ -153,7 +155,7 @@ func applyTLSConfig(clientCfg *api.Config, conf *Config) { type consulProvider struct { // kvClient is consul's KV endpoint, with the resolved Config already baked // into the underlying client. - kvClient *api.KV + kvClient *consulsdk.KV } // Get reads the value at key and returns it verbatim: no trimming, no key @@ -164,7 +166,7 @@ type consulProvider struct { // distinguishes. ctx bounds the request via QueryOptions, so the SecretStore's // per-operation deadline is honored. func (cp *consulProvider) Get(ctx context.Context, key string) (string, error) { - pair, _, err := cp.kvClient.Get(key, (&api.QueryOptions{}).WithContext(ctx)) + pair, _, err := cp.kvClient.Get(key, (&consulsdk.QueryOptions{}).WithContext(ctx)) if err != nil { return "", &kv.StoreUnavailableError{KeyPath: key, Err: err} } @@ -175,3 +177,56 @@ func (cp *consulProvider) Get(ctx context.Context, key string) (string, error) { return string(pair.Value), nil } + +// Set writes value verbatim as the raw bytes at key (PUT /v1/kv/), with no +// key transformation or interpretation. +// A transport or backend failure returns *kv.StoreUnavailableError. +func (cp *consulProvider) Set(ctx context.Context, key, value string) error { + pair := &consulsdk.KVPair{ + Key: key, + Value: []byte(value), + } + + ctx, cancel := context.WithTimeout(ctx, kv.DefaultOperationTimeout) + defer cancel() + + _, err := cp.kvClient.Put(pair, (&consulsdk.WriteOptions{}).WithContext(ctx)) + if err != nil { + return &kv.StoreUnavailableError{KeyPath: key, Err: err} + } + + return nil +} + +// List returns every key/value pair under prefix, keyed by the FULL consul key +// (the caller strips the prefix if it wants relative keys). Consul directory +// markers — keys ending in "/" — are skipped; they are not real entries. +// +// An empty prefix is rejected: consul would treat it as "list the entire KV +// store", which is never what a reference resolver wants and is an easy footgun. +// A prefix that matches nothing is not an error — it returns an empty map. +func (cp *consulProvider) List(ctx context.Context, prefix string) (map[string]string, error) { + if prefix == "" { + return nil, errors.New("consul: list requires a non-empty prefix") + } + + ctx, cancel := context.WithTimeout(ctx, kv.DefaultOperationTimeout) + defer cancel() + + pairs, _, err := cp.kvClient.List(prefix, (&consulsdk.QueryOptions{}).WithContext(ctx)) + if err != nil { + return nil, &kv.StoreUnavailableError{KeyPath: prefix, Err: err} + } + + out := make(map[string]string, len(pairs)) + + for _, p := range pairs { + if strings.HasSuffix(p.Key, "/") { + continue + } + + out[p.Key] = string(p.Value) + } + + return out, nil +} diff --git a/kv/providers/consul/consul_test.go b/kv/providers/consul/consul_test.go index 0628be89..f26cca87 100644 --- a/kv/providers/consul/consul_test.go +++ b/kv/providers/consul/consul_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "io" "net/http" "net/http/httptest" "sync" @@ -45,10 +46,11 @@ func clearConsulEnv(t *testing.T) { // consulStub is an httptest server that records the requests it receives and // delegates response construction to a per-test handler. type consulStub struct { - url string - mu sync.Mutex - got []string - auth []string + url string + mu sync.Mutex + got []string + auth []string + bodies []string } // requests returns a copy of the recorded "METHOD /path" entries. @@ -71,14 +73,32 @@ func (s *consulStub) lastAuth() string { return s.auth[len(s.auth)-1] } +// lastBody returns the raw request body of the most recent request. It is +// recorded centrally (under the mutex) so write tests can assert on it without +// racing the server goroutine. +func (s *consulStub) lastBody() string { + s.mu.Lock() + defer s.mu.Unlock() + + if len(s.bodies) == 0 { + return "" + } + + return s.bodies[len(s.bodies)-1] +} + func newConsulStub(t *testing.T, handler http.HandlerFunc) *consulStub { t.Helper() s := &consulStub{} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + s.mu.Lock() s.got = append(s.got, r.Method+" "+r.URL.Path) s.auth = append(s.auth, r.Header.Get("Authorization")) + s.bodies = append(s.bodies, string(body)) s.mu.Unlock() handler(w, r) @@ -137,6 +157,41 @@ func mustJSON(t *testing.T, v any) json.RawMessage { return b } +func writeConsulPairs(w http.ResponseWriter, pairs []struct{ Key, Value string }) { + w.Header().Set("Content-Type", "application/json") + + type kvp struct { + Key string `json:"Key"` + Value []byte `json:"Value"` + } + + arr := make([]kvp, 0, len(pairs)) + for _, p := range pairs { + arr = append(arr, kvp{Key: p.Key, Value: []byte(p.Value)}) + } + + //nolint:errcheck + _ = json.NewEncoder(w).Encode(arr) +} + +func lister(t *testing.T, p kv.Provider) kv.Lister { + t.Helper() + + l, ok := kv.AsLister(p) + require.True(t, ok, "consul provider must implement kv.Lister") + + return l +} + +func setter(t *testing.T, p kv.Provider) kv.Setter { + t.Helper() + + s, ok := kv.AsSetter(p) + require.True(t, ok, "consul provider must implement kv.Setter") + + return s +} + func TestNewFactory(t *testing.T) { tests := []struct { name string @@ -399,3 +454,188 @@ func TestBackwardCompatParity_ConsulGet(t *testing.T) { require.ErrorAs(t, err, ¬Found) }) } + +func TestList_ReturnsPairsUnderPrefix(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + writeConsulPairs(w, []struct{ Key, Value string }{ + {Key: "tyk-apis/", Value: ""}, // directory marker — must be skipped + {Key: "tyk-apis/c2_value", Value: "http://up/"}, + {Key: "tyk-apis/auth_header", Value: "X-From-Consul"}, + }) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + got, err := lister(t, p).List(t.Context(), "tyk-apis") + require.NoError(t, err) + + require.Equal(t, map[string]string{ + "tyk-apis/c2_value": "http://up/", + "tyk-apis/auth_header": "X-From-Consul", + }, got, "returns full keys, directory marker skipped") + + assert.Contains(t, stub.requests(), "GET /v1/kv/tyk-apis") +} + +func TestList_EmptyPrefixErrorsWithoutRequest(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + t.Error("List must not hit the backend for an empty prefix") + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + _, err := lister(t, p).List(t.Context(), "") + require.Error(t, err, "empty prefix must be rejected to avoid a whole-store scan") + require.Empty(t, stub.requests()) +} + +func TestList_EmptyResultIsNotError(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + writeConsulPairs(w, nil) // 200 with an empty array + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + got, err := lister(t, p).List(t.Context(), "tyk-apis") + require.NoError(t, err, "a prefix that matches nothing is not an error") + require.Empty(t, got) +} + +func TestList_BackendErrorReturnsStoreUnavailable(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + _, err := lister(t, p).List(t.Context(), "tyk-apis") + + var unavailable *kv.StoreUnavailableError + require.ErrorAs(t, err, &unavailable, + "a backend failure must map to *kv.StoreUnavailableError, like Get") +} + +func TestProvider_ImplementsSetter(t *testing.T) { + p := newConsulProvider(t, &consul.Config{}) + + _, ok := kv.AsSetter(p) + require.True(t, ok, "consul must implement kv.Setter for the write-back path") +} + +func TestSet_WritesValueVerbatim(t *testing.T) { + tests := []struct { + name string + key string + value string + wantPath string + }{ + { + name: "single-segment key", + key: "mykey", + value: "myvalue", + wantPath: "PUT /v1/kv/mykey", + }, + { + name: "multi-segment key preserved verbatim (no transform)", + key: "tyk-apis/rotated_key", + value: "abc123", + wantPath: "PUT /v1/kv/tyk-apis/rotated_key", + }, + { + name: "value written byte-exact with no trailing-newline trim", + key: "raw", + value: "line\n", + wantPath: "PUT /v1/kv/raw", + }, + { + name: "binary-safe value (embedded NUL survives)", + key: "bin", + value: "a\x00b", + wantPath: "PUT /v1/kv/bin", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + // Consul answers a successful PUT with 200 and the literal "true". + _, err := w.Write([]byte("true")) + require.NoError(t, err) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + err := setter(t, p).Set(t.Context(), tt.key, tt.value) + require.NoError(t, err) + + assert.Equal(t, []string{tt.wantPath}, stub.requests()) + assert.Equal(t, tt.value, stub.lastBody(), + "value must be written to consul verbatim, no transform") + }) + } +} + +func TestSet_BackendErrorReturnsStoreUnavailable(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + err := setter(t, p).Set(t.Context(), "tyk-apis/key", "v") + + var unavailable *kv.StoreUnavailableError + require.ErrorAs(t, err, &unavailable, + "a backend write failure must map to *kv.StoreUnavailableError") + require.Equal(t, "tyk-apis/key", unavailable.KeyPath) +} + +func TestSet_PropagatesContextCancellation(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + _, err := w.Write([]byte("true")) + require.NoError(t, err) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + err := setter(t, p).Set(ctx, "k", "v") + require.Error(t, err, + "a cancelled context must abort the write (WriteOptions.WithContext)") + require.ErrorIs(t, err, context.Canceled) +} + +func TestSet_UsesBasicAuth(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + _, err := w.Write([]byte("true")) + require.NoError(t, err) + }) + + var cfg consul.Config + cfg.Address = addrOf(stub.url) + cfg.HttpAuth.Username = "user" + cfg.HttpAuth.Password = "pass" + + p := newConsulProvider(t, &cfg) + + require.NoError(t, setter(t, p).Set(t.Context(), "k", "v")) + + want := "Basic " + base64.StdEncoding.EncodeToString([]byte("user:pass")) + require.Equal(t, want, stub.lastAuth()) +} + +func TestBackwardCompatParity_ConsulPut(t *testing.T) { + stub := newConsulStub(t, func(w http.ResponseWriter, _ *http.Request) { + _, err := w.Write([]byte("true")) + require.NoError(t, err) + }) + + p := newConsulProvider(t, &consul.Config{Address: addrOf(stub.url)}) + + require.NoError(t, setter(t, p).Set(t.Context(), "tyk-apis/edge_api_key", "rotated-secret")) + + require.Equal(t, []string{"PUT /v1/kv/tyk-apis/edge_api_key"}, stub.requests()) + require.Equal(t, "rotated-secret", stub.lastBody()) +} diff --git a/kv/providers/vault/vault.go b/kv/providers/vault/vault.go index 3be81195..4be82319 100644 --- a/kv/providers/vault/vault.go +++ b/kv/providers/vault/vault.go @@ -18,7 +18,7 @@ import ( "time" "github.com/TykTechnologies/storage/kv" - "github.com/hashicorp/vault/api" + vaultsdk "github.com/hashicorp/vault/api" ) // Config is the JSON "config" block of a vault store. @@ -98,7 +98,7 @@ func NewFactory() kv.ProviderFactory { return nil, errors.New("vault: token is required") } - defaultCfg := api.DefaultConfig() + defaultCfg := vaultsdk.DefaultConfig() if conf.Address != "" { defaultCfg.Address = conf.Address @@ -131,7 +131,7 @@ func NewFactory() kv.ProviderFactory { defaultCfg.Timeout = timeout } - client, err := api.NewClient(defaultCfg) + client, err := vaultsdk.NewClient(defaultCfg) if err != nil { return nil, fmt.Errorf("vault: create client: %w", err) } @@ -156,7 +156,7 @@ func NewFactory() kv.ProviderFactory { type vaultProvider struct { // client is the Vault API client. The resolved Config (address, token, // retries, timeout) is already baked into it at construction. - client *api.Client + client *vaultsdk.Client // timeout is the parsed Config.Timeout, surfaced via Timeout() so the // SecretStore wrapper can bound each Get with it. 0 means "unset", letting @@ -216,6 +216,45 @@ func (vp *vaultProvider) Get(ctx context.Context, key string) (string, error) { return string(b), nil } +// Set writes value as the secret's data at key. For KV v2 the map is wrapped in the +// "data" envelope and "/data" is injected into the path; for KV v1 the map +// is written as-is. When mount_path is set, key must be the full logical path under that mount. +// +// A value that is not a JSON object is rejected before any request; a backend or +// transport failure returns *kv.StoreUnavailableError. +func (vp *vaultProvider) Set(ctx context.Context, key, value string) error { + apiPath, err := vp.physicalPath(key) + if err != nil { + return err + } + + var fields map[string]any + + err = json.Unmarshal([]byte(value), &fields) + if err != nil { + return fmt.Errorf("vault: value must be a JSON object: %w", err) + } + + if fields == nil { + return errors.New("vault: value must be a JSON object") + } + + data := fields + if vp.kvv2 { + data = map[string]any{"data": fields} + } + + ctx, cancel := context.WithTimeout(ctx, kv.EffectiveTimeout(vp.timeout)) + defer cancel() + + _, err = vp.client.Logical().WriteWithContext(ctx, apiPath, data) + if err != nil { + return &kv.StoreUnavailableError{KeyPath: key, Err: err} + } + + return nil +} + func (vp *vaultProvider) Timeout() time.Duration { return vp.timeout } diff --git a/kv/providers/vault/vault_test.go b/kv/providers/vault/vault_test.go index ffde76ba..4b92465e 100644 --- a/kv/providers/vault/vault_test.go +++ b/kv/providers/vault/vault_test.go @@ -3,6 +3,7 @@ package vault_test import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "sync" @@ -52,9 +53,10 @@ func clearVaultEnv(t *testing.T) { // vaultStub is an httptest server that records the requests it receives and // delegates response construction to a per-test handler. type vaultStub struct { - url string - mu sync.Mutex - got []string + url string + mu sync.Mutex + got []string + bodies []string } // requests returns a copy of the recorded "METHOD /path" entries. @@ -65,13 +67,29 @@ func (s *vaultStub) requests() []string { return append([]string(nil), s.got...) } +// lastBody returns the raw request body of the most recent request. +func (s *vaultStub) lastBody() string { + s.mu.Lock() + defer s.mu.Unlock() + + if len(s.bodies) == 0 { + return "" + } + + return s.bodies[len(s.bodies)-1] +} + func newVaultStub(t *testing.T, handler http.HandlerFunc) *vaultStub { t.Helper() s := &vaultStub{} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + s.mu.Lock() s.got = append(s.got, r.Method+" "+r.URL.Path) + s.bodies = append(s.bodies, string(body)) s.mu.Unlock() handler(w, r) @@ -136,6 +154,15 @@ func mustJSON(t *testing.T, v any) json.RawMessage { return b } +func setter(t *testing.T, p kv.Provider) kv.Setter { + t.Helper() + + s, ok := kv.AsSetter(p) + require.True(t, ok, "vault provider must implement kv.Setter") + + return s +} + func TestNewFactory(t *testing.T) { tests := []struct { name string @@ -538,3 +565,246 @@ func TestResolver_ExtractsFieldFromVaultSecret(t *testing.T) { require.NoError(t, err) require.Equal(t, "abc123", got) } + +func TestProvider_ImplementsSetter(t *testing.T) { + p := newVaultProvider(t, &vault.Config{Token: "root"}) + + _, ok := kv.AsSetter(p) + require.True(t, ok, "vault must implement kv.Setter for the write-back path") +} + +func TestSet_WritesDataMap(t *testing.T) { + tests := []struct { + name string + key string + kvVersion int + value string + wantPath string + wantBody string + }{ + { + name: "kv2 multi-segment path injects /data and wraps in data envelope", + key: "secret/tyk-apis", + kvVersion: 2, + value: `{"api_key":"NEW"}`, + wantPath: "PUT /v1/secret/data/tyk-apis", + wantBody: `{"data":{"api_key":"NEW"}}`, + }, + { + name: "kv2 default version (0) wraps in data envelope", + key: "secret/tyk-apis", + kvVersion: 0, + value: `{"api_key":"NEW"}`, + wantPath: "PUT /v1/secret/data/tyk-apis", + wantBody: `{"data":{"api_key":"NEW"}}`, + }, + { + name: "kv2 single-segment path injects /data after the mount", + key: "mysecret", + kvVersion: 2, + value: `{"api_key":"NEW"}`, + wantPath: "PUT /v1/mysecret/data", + wantBody: `{"data":{"api_key":"NEW"}}`, + }, + { + name: "kv2 writes the whole data map faithfully (symmetric with Get: a real C2 secret has many fields)", + key: "secret/tyk-apis", + kvVersion: 2, + value: `{"api_key":"NEW","username":"bob"}`, + wantPath: "PUT /v1/secret/data/tyk-apis", + wantBody: `{"data":{"api_key":"NEW","username":"bob"}}`, + }, + { + name: "kv1 writes the data map as-is with no data envelope", + key: "secret/tyk-apis", + kvVersion: 1, + value: `{"api_key":"NEW"}`, + wantPath: "PUT /v1/secret/tyk-apis", + wantBody: `{"api_key":"NEW"}`, + }, + { + name: "kv2 non-string field values (number, bool) are written faithfully", + key: "secret/tyk-apis", + kvVersion: 2, + value: `{"api_key":"NEW","port":8080,"enabled":true}`, + wantPath: "PUT /v1/secret/data/tyk-apis", + wantBody: `{"data":{"api_key":"NEW","port":8080,"enabled":true}}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stub := newVaultStub(t, func(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{}) + }) + + p := newVaultProvider(t, &vault.Config{ + Address: stub.url, + Token: "root", + KVVersion: tt.kvVersion, + }) + + err := setter(t, p).Set(t.Context(), tt.key, tt.value) + require.NoError(t, err) + + require.Equal(t, []string{tt.wantPath}, stub.requests()) + require.JSONEq(t, tt.wantBody, stub.lastBody()) + }) + } +} + +func TestSet_MountPath(t *testing.T) { + tests := []struct { + name string + mountPath string + key string + wantPath string + wantErr bool + }{ + { + name: "kv2 nested mount injects /data after the configured mount", + mountPath: "tenants/a/kv", + key: "tenants/a/kv/tyk-apis", + wantPath: "PUT /v1/tenants/a/kv/data/tyk-apis", + }, + { + name: "kv2 key outside the mount is rejected before any request", + mountPath: "tenants/a/kv", + key: "other/secret", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stub := newVaultStub(t, func(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{}) + }) + + p := newVaultProvider(t, &vault.Config{ + Address: stub.url, + Token: "root", + KVVersion: 2, + MountPath: tt.mountPath, + }) + + err := setter(t, p).Set(t.Context(), tt.key, `{"api_key":"NEW"}`) + + if tt.wantErr { + require.Error(t, err) + require.Empty(t, stub.requests(), + "a key outside mount_path must be rejected before any Vault request") + + return + } + + require.NoError(t, err) + require.Equal(t, []string{tt.wantPath}, stub.requests()) + require.JSONEq(t, `{"data":{"api_key":"NEW"}}`, stub.lastBody()) + }) + } +} + +func TestSet_InvalidValueRejectedBeforeRequest(t *testing.T) { + tests := []struct { + name string + value string + }{ + {name: "not json at all", value: "just a string"}, + {name: "json string scalar is not an object", value: `"api_key"`}, + {name: "json number is not an object", value: "42"}, + {name: "json array is not an object", value: `["a","b"]`}, + {name: "json null is not an object", value: "null"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stub := newVaultStub(t, func(_ http.ResponseWriter, _ *http.Request) { + t.Error("Set must not reach Vault when the value is not a JSON object") + }) + + p := newVaultProvider(t, &vault.Config{Address: stub.url, Token: "root", KVVersion: 2}) + + err := setter(t, p).Set(t.Context(), "secret/tyk-apis", tt.value) + require.Error(t, err, "value must be a JSON object (the secret data map)") + require.Empty(t, stub.requests()) + }) + } +} + +func TestSet_BackendErrorReturnsStoreUnavailable(t *testing.T) { + stub := newVaultStub(t, func(w http.ResponseWriter, _ *http.Request) { + // A 200 with an unparseable body fails fast (no 5xx retry backoff) and + // deterministically exercises the write error branch. + w.WriteHeader(http.StatusOK) + + _, err := w.Write([]byte("{ this is not valid vault json")) + if err != nil { + t.Error(err) + } + }) + + p := newVaultProvider(t, &vault.Config{Address: stub.url, Token: "root", KVVersion: 2}) + + err := setter(t, p).Set(t.Context(), "secret/tyk-apis", `{"api_key":"NEW"}`) + + var unavailable *kv.StoreUnavailableError + require.ErrorAs(t, err, &unavailable, + "a backend write failure must map to *kv.StoreUnavailableError") +} + +func TestSet_PropagatesContextCancellation(t *testing.T) { + stub := newVaultStub(t, func(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{}) + }) + + p := newVaultProvider(t, &vault.Config{Address: stub.url, Token: "root", KVVersion: 2}) + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + err := setter(t, p).Set(ctx, "secret/tyk-apis", `{"api_key":"NEW"}`) + require.ErrorIs(t, err, context.Canceled, + "a cancelled context must abort the write (WriteWithContext)") +} + +func TestSet_RoundTripsWithGet(t *testing.T) { + var ( + mu sync.Mutex + stored map[string]any + ) + + var stub *vaultStub + stub = newVaultStub(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + mu.Lock() + data := stored + mu.Unlock() + + writeJSON(w, http.StatusOK, kvv2Envelope(data)) + + return + } + + var put struct { + Data map[string]any `json:"data"` + } + err := json.Unmarshal([]byte(stub.lastBody()), &put) + require.NoError(t, err) + + mu.Lock() + stored = put.Data + mu.Unlock() + + writeJSON(w, http.StatusOK, map[string]any{}) + }) + + p := newVaultProvider(t, &vault.Config{Address: stub.url, Token: "root", KVVersion: 2}) + + require.NoError(t, setter(t, p).Set(t.Context(), "secret/tyk-apis", `{"api_key":"NEW","username":"bob"}`)) + + got, err := p.Get(t.Context(), "secret/tyk-apis") + require.NoError(t, err) + require.JSONEq(t, `{"api_key":"NEW","username":"bob"}`, got, + "Set then Get must round-trip the data map") +} diff --git a/kv/resolver/reference.go b/kv/resolver/reference.go new file mode 100644 index 00000000..737e4638 --- /dev/null +++ b/kv/resolver/reference.go @@ -0,0 +1,46 @@ +package resolver + +import ( + "github.com/TykTechnologies/storage/kv/internal/resolve" +) + +// Reference is a parsed kv:// whole-value reference of the form +// "kv:///[#]". +type Reference struct { + // Store is the registry store name + Store string + + // Path is the provider-specific key or path within the store. + Path string + + // Field is the optional "#field" fragment; empty when absent. + Field string +} + +// ParseReference parses a kv:// whole-value reference into its parts without +// resolving it — for callers (e.g. a write-back path) that must route to a store +// by name rather than read a value. +// +// ok reports whether s is a kv:// whole-value reference at all. For a string that +// is not one — a legacy scheme (vault://, consul://), a plain literal, or an +// inline "$kv{...}" token — ok is false and err is nil. For a kv:// reference that +// is malformed (missing path separator, empty store or path), ok is true and err +// wraps ErrMalformedReference. +func ParseReference(s string) (Reference, bool, error) { + store, path, fragment, ok, err := resolve.ParseWholeValue(s) + if err != nil { + return Reference{}, ok, err + } + + if !ok { + return Reference{}, false, nil + } + + ref := Reference{ + Store: store, + Path: path, + Field: fragment, + } + + return ref, true, nil +} diff --git a/kv/resolver/resolver_test.go b/kv/resolver/resolver_test.go index 4f4e8b84..18a2b609 100644 --- a/kv/resolver/resolver_test.go +++ b/kv/resolver/resolver_test.go @@ -136,3 +136,129 @@ func TestProviderErrorsPropagateThroughFacade(t *testing.T) { require.ErrorAs(t, err, &got) require.Equal(t, "vault", got.StoreName) } + +func TestParseReference(t *testing.T) { + tests := []struct { + name string + input string + wantOK bool + wantErr bool + want resolver.Reference + }{ + // --- valid kv:// references (ok=true, err=nil) --- + { + name: "store and multi-segment path, no fragment", + input: "kv://vault/secret/tyk-apis", + wantOK: true, + want: resolver.Reference{Store: "vault", Path: "secret/tyk-apis"}, + }, + { + name: "store, path and fragment", + input: "kv://vault/secret/tyk-apis#api_key", + wantOK: true, + want: resolver.Reference{Store: "vault", Path: "secret/tyk-apis", Field: "api_key"}, + }, + { + name: "single-segment path", + input: "kv://env/MY_VAR", + wantOK: true, + want: resolver.Reference{Store: "env", Path: "MY_VAR"}, + }, + { + name: "consul-style multi-segment path, no fragment", + input: "kv://consul/tyk-apis/edge/key", + wantOK: true, + want: resolver.Reference{Store: "consul", Path: "tyk-apis/edge/key"}, + }, + { + name: "fragment carrying a JSON pointer is preserved verbatim", + input: "kv://vault/db/creds#data/password", + wantOK: true, + want: resolver.Reference{Store: "vault", Path: "db/creds", Field: "data/password"}, + }, + { + name: "only the first hash splits path from fragment", + input: "kv://s/p#a#b", + wantOK: true, + want: resolver.Reference{Store: "s", Path: "p", Field: "a#b"}, + }, + { + name: "trailing hash yields an empty fragment (valid, no field)", + input: "kv://vault/secret/app#", + wantOK: true, + want: resolver.Reference{Store: "vault", Path: "secret/app"}, + }, + + // --- not a kv:// whole-value reference (ok=false, err=nil) --- + { + name: "legacy vault scheme is not a kv:// reference", + input: "vault://secret/tyk-apis.api_key", + }, + { + name: "legacy consul scheme is not a kv:// reference", + input: "consul://tyk-apis/edge_key", + }, + { + name: "plain literal is not a reference", + input: "just-a-literal-value", + }, + { + name: "inline token is not a whole-value reference", + input: "$kv{vault:secret/tyk-apis#api_key}", + }, + { + name: "empty string is not a reference", + input: "", + }, + + // --- malformed kv:// references (ok=true, err wraps ErrMalformedReference) --- + { + name: "missing path separator", + input: "kv://vaultonly", + wantOK: true, + wantErr: true, + }, + { + name: "prefix only", + input: "kv://", + wantOK: true, + wantErr: true, + }, + { + name: "empty store name", + input: "kv:///secret/x", + wantOK: true, + wantErr: true, + }, + { + name: "empty path", + input: "kv://vault/", + wantOK: true, + wantErr: true, + }, + { + name: "empty store and path", + input: "kv:///", + wantOK: true, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok, err := resolver.ParseReference(tt.input) + + require.Equal(t, tt.wantOK, ok) + + if tt.wantErr { + require.ErrorIs(t, err, resolver.ErrMalformedReference, + "a malformed kv:// reference must wrap ErrMalformedReference") + + return + } + + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +}