Skip to content

fix: Rename module & update references mumoshu/vals -> variantdev/vals#3

Merged
mumoshu merged 1 commit into
helmfile:masterfrom
klebediev:hotfix/mod-rename
Oct 15, 2019
Merged

fix: Rename module & update references mumoshu/vals -> variantdev/vals#3
mumoshu merged 1 commit into
helmfile:masterfrom
klebediev:hotfix/mod-rename

Conversation

@klebediev

@klebediev klebediev commented Oct 11, 2019

Copy link
Copy Markdown
Contributor

As the repo was renamed in this PR module & references are renamed

@mumoshu mumoshu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot for your continuous support!

@mumoshu mumoshu merged commit 14a12b6 into helmfile:master Oct 15, 2019
digiserg added a commit to digitalis-io/vals that referenced this pull request Jun 30, 2026
Address PR review (helmfile#1213):

- Replace the never-evicting secretMapCache with singleflight dedup of
  concurrent reads, so a burst of parallel callers (vals-operator) costs
  one Vault read/token-use instead of N, without ever serving stale
  secrets (#1, helmfile#2, helmfile#3).
- Return a per-caller copy of the map since singleflight shares one
  result by reference (helmfile#2).
- Key the KV-version cache by mount path so sibling secrets under the
  same mount share a single preflight (helmfile#4).
- Serialize lazy client creation/auth in ensureClient to remove the
  first-use race (helmfile#6).
- Add concurrency tests asserting preflight/read call counts and map
  isolation against a stubbed Vault server (helmfile#5).
- Drop the hand-wavy fieldalignment nolint; order fields cleanly (helmfile#7).

Signed-off-by: Sergio Rua <[email protected]>
yxxhero pushed a commit that referenced this pull request Jun 30, 2026
* feat(vault): cache KV version preflight and secret map reads

Without caching, each secret ref in creds.yaml costs two Vault API
calls: one preflight to /v1/sys/internal/ui/mounts/<path> to detect
KV v1/v2, and one ReadWithData for the secret itself. This exhausts
low use-limit tokens (e.g. -use-limit=1) immediately.

With both caches:
- kvVersionCache: one preflight per unique mount path per run
- secretMapCache: one read per unique secret path per run

For N secrets across M unique mounts under P unique paths, token
use-limit needed drops from 2N to M+P (typically P+1 for single mount).

Claude-Session: https://claude.ai/code/session_01RDWpvGwQQzRfWUYhohzhNk
Signed-off-by: Sergio Rua <[email protected]>

* fix(vault): remove secretMapCache, add mutex-protected kvVersionCache

secretMapCache is redundant with the runtime-level LRU cache in vals.go
(docCache/strCache) which is already mutex-protected and bounded. Keeping
it at the provider level caused two problems:
1. Stale data in long-running processes (vals-operator) — no eviction
2. Data race under concurrent access — plain map, no synchronisation

kvVersionCache is kept because it caches mount-level KV version preflight
calls, which the runtime cache does not cover. Mount KV versions are
immutable at runtime so the cache never needs clearing. A sync.Mutex
protects it for concurrent callers.

Claude-Session: https://claude.ai/code/session_01RDWpvGwQQzRfWUYhohzhNk
Signed-off-by: Sergio Rua <[email protected]>

* fix(vault): restore secretMapCache with mutex protection

The runtime-level docCache in vals.go does not prevent duplicate
GetStringMap calls at the provider level — e.g. when multiple keys
reference the same secret path. Without secretMapCache each such path
costs one Vault read per key, exhausting low use-limit tokens.

Both caches are now protected by a single sync.Mutex making them safe
for concurrent callers (e.g. vals-operator).

Claude-Session: https://claude.ai/code/session_01RDWpvGwQQzRfWUYhohzhNk
Signed-off-by: Sergio Rua <[email protected]>

* fix(vault): dedup concurrent reads via singleflight, drop secret cache

Address PR review (#1213):

- Replace the never-evicting secretMapCache with singleflight dedup of
  concurrent reads, so a burst of parallel callers (vals-operator) costs
  one Vault read/token-use instead of N, without ever serving stale
  secrets (#1, #2, #3).
- Return a per-caller copy of the map since singleflight shares one
  result by reference (#2).
- Key the KV-version cache by mount path so sibling secrets under the
  same mount share a single preflight (#4).
- Serialize lazy client creation/auth in ensureClient to remove the
  first-use race (#6).
- Add concurrency tests asserting preflight/read call counts and map
  isolation against a stubbed Vault server (#5).
- Drop the hand-wavy fieldalignment nolint; order fields cleanly (#7).

Signed-off-by: Sergio Rua <[email protected]>

* fix(vault): make mount-prefix cache lookup segment-safe

Normalize the kvVersionCache key to a trailing slash on insert and match
siblings with HasPrefix(key+"/", mount) so a cached mount can never
match a textually-prefixed but distinct mount (e.g. "secret/" vs
"secretv2/x"). Empty-mount fallbacks stay full secret paths and are
only ever matched exactly, never by prefix.

Also: drop the redundant inner map copy in readSecretMap (GetStringMap
owns the per-caller copy), note the per-caller copy is shallow, and
document that sfVersion does not dedupe a concurrent first-burst of
distinct sibling paths.

Add tests for the segment-safe prefix match and for mount-key
normalization when Vault reports a mount without a trailing slash.

Signed-off-by: Sergio Rua <[email protected]>

* test(vault): fix fieldalignment in lookup test cases struct

Signed-off-by: Sergio Rua <[email protected]>

---------

Signed-off-by: Sergio Rua <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants