Skip to content

Support Name parameter for TLS auth and real LIST HTTP Methods#201

Open
iascan wants to merge 6 commits intohashicorp:masterfrom
wayfair-archive:wf_master
Open

Support Name parameter for TLS auth and real LIST HTTP Methods#201
iascan wants to merge 6 commits intohashicorp:masterfrom
wayfair-archive:wf_master

Conversation

@iascan
Copy link
Contributor

@iascan iascan commented Jun 6, 2019

The name parameter, corresponding to the name of the certificate role for the TLS backend is pretty critial and did not previously exist.

Also, I found it was not possible to LIST kv2 endpoints as the GET request with the list query parameter never returned any results (the same is true for the vault cli utility).

This resolves those issues for our needs.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@juice928
Copy link

juice928 commented Feb 3, 2026

👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 2 points that might be worth attention (could be false positives, please use your judgment):

  1. The tls method signature appears to conflict with its usage in tests

    • Location: lib/vault/api/auth.rb:L290-L294
    • Impact: Positional arguments may incorrectly ingest keyword hashes, potentially leading to empty payloads or type mismatches.
    • Suggestion: Consider updating the method to support keyword arguments or adding logic to handle Hash inputs for more flexible parameter extraction.
  2. The generic request fallback currently restricts the use of request bodies

    • Location: lib/vault/client.rb:L243
    • Impact: Custom HTTP verbs might fail to send necessary payloads, which could impact compatibility for certain custom operations.
    • Suggestion: Consider setting the request_body_permitted argument to true to allow payloads for generic or unknown verbs.

If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future.

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.

3 participants