feat(hashivault): enhance SignerVerifier to accept Vault options in addition to VAULT envs by dmmordvi · Pull Request #103 · deckhouse/delivery-kit-sdk

dmmordvi · 2026-07-22T17:14:31Z

Summary

Adds programmatic configuration of the HashiCorp Vault SignerVerifier via a new VaultOpts / VaultAuth options API, in addition to the existing environment-variable configuration. Callers can now select the Vault address, transit engine path, and authentication method directly in code through KeyOpts.SignerVerifierOpts.VaultOpts, without relying on VAULT_* / WERF_* env vars. Along the way it fixes two latent authentication bugs.

Motivation

Previously the Vault SignerVerifier could only be configured through environment variables (VAULT_ADDR, VAULT_TOKEN, WERF_VAULT_AUTH_, ACTIONS_, …). This makes it hard for embedders to configure Vault per-call, run with multiple Vault identities in one process, or avoid leaking host credentials. This PR introduces an explicit, typed options API while keeping the env-based behavior fully backward-compatible.

What changed

New programmatic API

VaultOpts — top-level options: Address, TransitSecretEnginePath, and Auth *VaultAuth.
VaultAuth — selects exactly one auth method: AppRole, OIDC, JWT, or Token.
Per-method structs: AppRoleAuth (RoleID/SecretID/Path), OIDCAuth (Audience/Role/Path), JWTAuth (JWT/Role/Path), TokenAuth (Token).
New entrypoint hashivault.LoadSignerVerifierWithOpts(ref, hashFunc, VaultOpts); LoadSignerVerifier is preserved and delegates with empty opts.
Threaded through KeyOpts.SignerVerifierOpts → signerVerifierFromKeyRef → the Vault loader.

Configuration semantics

Transport options (Address, TransitSecretEnginePath) always fall back to env vars / defaults when empty.
Auth source is all-or-nothing:
- Auth == nil → env-mode: unchanged legacy behavior (priority AppRole > GitHub Actions OIDC > static JWT > static token, with VAULT_TOKEN / ~/.vault-token fallback).
- Auth != nil → opts-mode: credentials come only from the selected variant; env auth vars are ignored. Exactly one method must be set — zero or multiple is a configuration error. (GitHub Actions
  OIDC still reads the runner-injected ACTIONS_ID_TOKEN_REQUEST_URL / ACTIONS_ID_TOKEN_REQUEST_TOKEN.)
Auth mount Path is configurable, defaulting to ar (AppRole) and jwt (JWT/OIDC).

Bug fixes

Host-token leak: vault.NewClient auto-loads VAULT_TOKEN; the client now calls ClearToken() immediately after construction so the host token can no longer ride in the X-Vault-Token header of the first /auth//login request. Regression test added.
Static-token Login no-op: staticAuthenticator.Login never called SetToken, so a configured TokenAuth was silently dropped (requests signed under the env token or unauthenticated). It now installs its token on every login, which also fixes the latent ~/.vault-token env path. Regression test added.

Docs & tests

README section documenting VaultOpts: field→env mapping, transport/auth separation, auth-source selection, priority, path resolution, and the no-fallback error behavior.
New tests: authenticator selection matrix (env vs opts), VaultOpts passthrough via NewSignerVerifier, token-leak regression, static-token login regression.

Minor

validReference → exported ValidReference (needed for trdl ELF signing during config validation)

Backward compatibility

Fully backward compatible: existing env-var-only callers are unaffected (Auth == nil reproduces the previous behavior). LoadSignerVerifier's signature is unchanged.

…ddition to VAULT envs Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Add table-driven tests for newAuthSettings/newAuthenticator across the env and opts sources: method priority (AppRole > OIDC > static JWT > token), transport-only opts preserving env auth, opts credentials ignoring env, incomplete opts returning an error without env/file token fallback, and auth-path resolution (opts over WERF_VAULT_AUTH_PATH, ar/jwt defaults). Env is isolated per test by unsetting Vault variables, since the getters chain WERF_* over VAULT_* via os.LookupEnv where an empty-but-present value would mask the fallback. Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Add Ginkgo specs proving KeyOpts.SignerVerifierOpts.VaultOpts is threaded to the Vault loader for hashivault:// refs (incomplete opts surface the "incomplete Vault auth options" error before any network call), that non-Vault refs take a different path, and that an empty KeyRef is rejected up front. Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Add a README section covering KeyOpts.SignerVerifierOpts.VaultOpts: the field-to-env mapping, the transport/auth separation, the all-or-nothing auth source selection, the auth-method priority and auth-path resolution, the incomplete-opts error (no env/file token fallback), and that OIDC still reads the GitHub-injected ACTIONS_ID_TOKEN_REQUEST_* variables. Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

staticAuthenticator.Login was a no-op that never called client.SetToken, so in opts-mode a configured TokenAuth was silently dropped: the Vault SDK auto-loads VAULT_TOKEN at client construction, so requests were signed under the environment token (wrong identity) or, with VAULT_TOKEN unset, sent unauthenticated. This violated the opts-mode guarantee that credentials come only from the selected variant and never from the host environment. Login now calls client.SetToken(s.tokenID) on every invocation, which also fixes the pre-existing latent ~/.vault-token env-mode path. Add a Login-level regression test on a real *vault.Client covering both a conflicting VAULT_TOKEN and an unset VAULT_TOKEN; existing tests only asserted the constructed tokenID field and never exercised Login, so the bug was invisible to CI. Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

…leak vault.NewClient auto-loads VAULT_TOKEN from the environment. For AppRole/JWT/OIDC in opts-mode, baseAuthenticator.login writes to /auth/<path>/login before SetToken, leaking the host token in the X-Vault-Token header of the login request — violating the documented opts-mode isolation guarantee. Clear the token right after client construction; each authenticator installs its own token before signing. Add a regression test asserting the login request carries no host token, and correct the README's JWT-source note for opts-mode. Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

dmmordvi

Automated code review (via pi-pi).

The change is correct, well-tested, and mergeable as-is: programmatic VaultOpts/VaultAuth configuration is cleanly threaded through KeyOpts.SignerVerifierOpts, and two real latent bugs are fixed along the way (host-token leak in the login request header; static-token Login no-op). go build, go vet, and go test -race ./pkg/signver/... all pass. No blocking issues were found. Three minor polish items are noted as inline comments.

(generated by pi-pi)

dmmordvi · 2026-07-24T18:15:46Z

@@ -20,7 +20,7 @@ type baseAuthenticator struct {
 }

 func (b *baseAuthenticator) login(client *vault.Client, data map[string]interface{}) error {


MINOR: the auth mount path is now captured once when the authenticator is constructed and used immutably here, whereas previously it was re-read from WERF_VAULT_AUTH_PATH on every login. A change to that env var after construction but before a token refresh no longer takes effect. In practice the authenticator is built fresh per LoadSignerVerifier call, so this only matters for a long-lived instance whose environment mutates mid-lifetime — practically never. Informational; the immutable design is arguably cleaner. If per-login env re-read is a required behavior, restore a dynamic path resolver for the env path and add a refresh-after-mutation test.
(generated by pi-pi)

so this only matters for a long-lived instance whose environment mutates mid-lifetime — practically never
I think we can ignore it, it is not our case

dmmordvi · 2026-07-24T18:15:46Z

-// validReference returns a non-nil error if the reference string is invalid
-func validReference(ref string) error {
+// ValidReference returns a non-nil error if the reference string is invalid
+func ValidReference(ref string) error {


MINOR: validReference was renamed to the exported ValidReference, but its only caller is internal (client.go). Exporting it commits the function name and its error semantics to the public API surface. Consider keeping it unexported unless an external consumer actually needs it; if external use is intended, document that intent.
(generated by pi-pi)

It is needed for KeyRef validation for ELF signing trdl

dmmordvi · 2026-07-24T18:15:46Z

@@ -69,6 +69,15 @@ type SignerVerifier struct {
 // It also can verify signatures (via a remote vall to the Vault instance). The hashFunc will be
 // automatically set to crypto.Hash(0) if the key referred to by referenceStr is an ED25519 signing key.
 func LoadSignerVerifier(referenceStr string, hashFunc crypto.Hash, opts ...signature.RPCOption) (*SignerVerifier, error) {


MINOR: LoadSignerVerifier accepts opts ...signature.RPCOption but never uses it (pre-existing, not introduced here). Sitting next to the new LoadSignerVerifierWithOpts — whose options do change behavior — this is now a confusing API with two differently-typed "opts", one ignored and one honored. At minimum rename the unused parameter to _ ...signature.RPCOption; better, either wire it through or deprecate/remove it in a breaking release.
(generated by pi-pi)

Renamed arg to _

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

dmmordvi added 10 commits July 22, 2026 18:40

feat(hashivault): enhance SignerVerifier to accept Vault options in a…

27217cf

…ddition to VAULT envs Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

feat: explicit auth method selection and error handling by VaultOpts

2aa49a8

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

refactor: add default case for VaultAuth validation

33f8de1

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

refactor: public ValidReference function

ad0c19b

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

fix: format auth factory

480daa9

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

dmmordvi commented Jul 24, 2026

View reviewed changes

feat(hashivault): rename unused RPCOption arg

687005b

Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(hashivault): enhance SignerVerifier to accept Vault options in addition to VAULT envs#103

feat(hashivault): enhance SignerVerifier to accept Vault options in addition to VAULT envs#103
dmmordvi wants to merge 11 commits into
mainfrom
feat/signer-verifier-vault-options

dmmordvi commented Jul 22, 2026 •

edited

Loading

Uh oh!

dmmordvi left a comment

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

dmmordvi Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -20,7 +20,7 @@ type baseAuthenticator struct {
		}

		func (b baseAuthenticator) login(client vault.Client, data map[string]interface{}) error {

Uh oh!

Conversation

dmmordvi commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

What changed

New programmatic API

Configuration semantics

Bug fixes

Docs & tests

Minor

Backward compatibility

Uh oh!

dmmordvi left a comment

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

dmmordvi Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dmmordvi commented Jul 22, 2026 •

edited

Loading