Repository navigation
fix: restore generateRootStatus and repair addKubernetesRole/certLogin schemas - #312
Merged
Merged
Conversation
…n schemas
addKubernetesRole and certLogin threw "TypeError: Cannot convert undefined
or null to object" whenever they were given any argument, and have done so
since both were added in 0.10.0 (first published in 0.9.23-canary.1).
extendOptions builds the request body from schema.req.properties, and
neither schema had one: addKubernetesRole's fields sat directly under req,
and certLogin's req was a bare { type: 'object' }.
- addKubernetesRole: the schema now declares the role parameters of Vault's
Kubernetes auth API, per its OpenAPI spec: the current token_*, audience,
alias_name_source, alias_metadata and namespace-selector fields, plus the
deprecated aliases it already had (policies, ttl, max_ttl, period,
num_uses, bound_cidrs). As in Vault, list fields take an array or a
comma-separated string and durations take seconds or a duration string.
Undeclared keys are never sent, so declaring only the legacy names would
have dropped token_bound_cidrs, audience and the rest without an error.
The unused `name` field is gone: the role name is role_name in the path.
- certLogin: declare the optional `name` (the certificate role to match).
- generateRootStatus (GET /sys/generate-root/attempt): restored exactly as
it was in 0.9.22. The 0.10.0 lint merge (#225) dropped it, and 0.11.0
then removed its dangling type declaration, which is restored too.
- features.md regenerated (adds generateRootStatus).
New unit tests cover the three commands, and a guard test checks that every
command's req/query schema declares type 'object' with properties, which
would have caught both schema bugs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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 was broken
addKubernetesRoleTypeError: Cannot convert undefined or null to object, before a request is sent. No role can be created.certLoginTypeErrorfor any argument, including per-callrequestOptionswith the client certificate. Only a barecertLogin()with TLS settings in the client config works.generateRootStatusvault.generateRootStatus is not a function. The command was dropped by the 0.10.0 lint merge (#225), and 0.11.0 then removed its type declaration as dangling.Root cause of the first two:
extendOptionsbuilds the request body fromschema.req.properties, and neither schema had that key:addKubernetesRole's fields sat directly underreq.certLogin'sreqwas a bare{ type: 'object' }.Changes
addKubernetesRole: the schema now declares the role parameters from Vault's own OpenAPI spec forauth/kubernetes/role/{name}:token_*fields,audience,alias_name_source,alias_metadataandbound_service_account_namespace_selector;policies,ttl,max_ttl,period,num_uses,bound_cidrs).List fields accept an array or a comma-separated string, and durations accept seconds or a string such as
"1h", exactly as Vault does. This uses the sametype: [...]uniontokenRenewalready uses.Why not just re-nest the legacy fields: undeclared keys are never sent. A schema with only the legacy names would make calls that use
token_bound_cidrs,token_num_usesoraudiencesucceed while silently creating a role without those restrictions.nameis gone from the schema. The role name isrole_namein the path, and Vault overwrote anamein the body anyway.certLogin: declares the optionalname, which is the certificate role to match.generateRootStatus: restored exactly as in 0.9.22 (GET /sys/generate-root/attempt), together with itsindex.d.tsdeclaration.features.mdis regenerated.Tests:
requestOptionskept out of the body,generateRootStatus.req/queryschema is{ type: 'object', properties: {...} }. It would have caught both schema bugs.Verification
vault:1.13.3(the repo's image) andhashicorp/vault:2.1.1. Each command was driven from this branch and from master:addKubernetesRolewith the current fields, string forms included ('app,worker',token_ttl: '1h',token_bound_cidrs,audience, …)TypeError, nothing sent'1h'→ 3600, strings → arrays)addKubernetesRolewith the legacy payload fromexample/auth_kubernetes.jsTypeErrortoken_policies: ["app"],token_ttl: 3600)addKubernetesRoleon a second mount viamount_pointTypeErrorcertLogin({ name: 'web' })over mutual TLS (rolesweb,db)TypeErrortokenLookupSelfshows web's policies (db→ db's)certLogin({ name: 'nope' })/ self-signed client certTypeError{"name":"nope"}reached Vault)generateRootStatus()across init → status → cancel → statusstartedtrue → false with the same nonce; responses validate against the restored schemaBehaviour notes
addKubernetesRolearguments are now validated like every other command's, so a wrongly typed value (e.g.token_num_uses: 'many') rejects before any request is sent.Not in this PR (pre-existing, found while verifying)
POST/PUTcommands (e.g.mount) send all arguments as the JSON body, per-callrequestOptionsincluded, so a clientkey/passphrasereaches Vault in the body.certLoginhard-codes/auth/cert/login. Unlike the other login commands, it has nomount_point.example/auth_kubernetes.jsbuilds the config path with single quotes ('auth/${kubernetesPath}/config', a literal string)..github/workflows/write-features.yamlfilters onsrc/commands/**, which never matchessrc/commands.js, sofeatures.mdnever regenerates on its own.🤖 Generated with Claude Code