feat: KV v2 support + generic verbs (opt-in, non-breaking)#96
Merged
Conversation
Add transparent KV v2 handling and generic backend access to the simple client. Opt-in via api.kv.autoDetect (default false) or a static api.engines map; with neither set, read/list/write are byte-for-byte unchanged and no sys/internal/ui/mounts call is made. New: - src/kvTransform.js — pure rewritePath/normalizeResponse per the KV op table - src/MountResolver.js — lazy per-mount version detection with cache + in-flight dedupe; engines override bypasses detection Changes: - VaultClient: read/list/write/delete are version-aware when enabled (passthrough otherwise); new update() (PATCH merge-patch+json), request() (raw literal path, no Lease), and v2 helpers deleteVersions/ undeleteVersions/destroyVersions/readMetadata/deleteMetadata (throw UnsupportedOperationError on non-v2 mounts) - VaultApiClient: only set the default Content-Type when the caller did not supply one (enables application/merge-patch+json) - Lease: optional metadata + getMetadata() (undefined on v1) - errors: add UnsupportedOperationError Detection only runs when autoDetect is on. With engines set but autoDetect off, listed mounts use the override and any other mount is passthrough, so engines is a working fallback for Vaults that deny the detection endpoint. Signed-off-by: Yuriy R <22548029+kurok@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Lets the simple client work against KV v2 mounts and any other Vault backend, lazily and transparently — callers don't need to know the engine version — while staying non-breaking for current users.
api.kv.autoDetect(defaultfalse) → with it andapi.enginesunset,read/list/writeare byte-for-byte the old behavior and nosys/internal/ui/mountscall is made.autoDetect:true, each mount's KV version is detected on first use viaGET sys/internal/ui/mounts/<path>(readable by least-privilege tokens), then paths are rewritten and responses unwrapped.api.enginesoverride. Static{ secret: 2, legacy: 1 }map; listed mounts skip detection. Withenginesset butautoDetect:false, unlisted mounts passthrough (no detection call) — so it's a working fallback for Vaults that deny the detection endpoint.Public API
read/list/write/deleteupdate(path, data)application/merge-patch+json+{data}envelope (KV v2)request(method, path, data)deleteVersions/undeleteVersions/destroyVersions/readMetadata/deleteMetadataUnsupportedOperationErroron v1/non-kvLease.getMetadata()undefinedon v1 (additive)Design
src/kvTransform.js(new, pure, no I/O) —rewritePath/normalizeResponseencode the full op × {v1,v2} table.src/MountResolver.js(new) — injecteddetectFn, cache by canonical mount path, longest-prefix lookup, in-flight Promise dedupe,enginesbypass, actionableVaultErroron detection failure.VaultApiClient.makeRequest— minimal change: only set the defaultContent-Typewhen the caller didn't supply one (enables merge-patch).Lease— optionalmetadata(additive);errors— addUnsupportedOperationError.How this was built & reviewed
Implemented via a multi-agent workflow (TDD build → 3-reviewer panel: architecture/consistency, correctness, acceptance → fix), then independently validated. Bugs caught and fixed before this PR:
list('secret/')hitsecretinstead ofsecret/, violating the byte-for-byte guarantee. Fixed:version!==2now uses the caller's literal path. Regression tests added.team/kvA,team/kvB) collapsed into one detection and mis-classified the second. Fixed: followers verify the resolved canonical mount actually prefixes their path; else a fresh detection runs. Regression test added.autoDetect:false+enginesset — an unlisted mount still attempted detection. Fixed: detection only runs whenautoDetectis on; engines-listed mounts resolve via override, everything else passthrough. Regression test added.request()falsy body —data || nulldropped0/''/false; nowdata === undefined ? null : data.secret/data/foo+ autoDetect) documented in the README.Tests
npm run test:unit— 237 passing (newkvTransform+MountResolversuites are exhaustive;VaultClient/conformancecover every acceptance criterion)npm run lint— cleanScope notes
Within the agreed file list, plus a 9-line addition to
test/errors.test.mjscovering the new error class. No version bump / CHANGELOG (handled at release). Out of scope: auth backends, typed transit/database helpers,index.d.ts.