Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ReadSecretPathsAsync to also support GET Method #328

Open
greg-signi opened this issue Sep 19, 2023 · 7 comments · May be fixed by #329
Open

ReadSecretPathsAsync to also support GET Method #328

greg-signi opened this issue Sep 19, 2023 · 7 comments · May be fixed by #329

Comments

@greg-signi
Copy link

Describe the feature requesInt or question
Having problems with the LIST method using istio-proxy, which currently doesn't allow the custom http method LIST, more specifically in this request

        public async Task<Secret<ListInfo>> ReadSecretPathsAsync(string path, string mountPoint = null, string wrapTimeToLive = null)
        {
            Checker.NotNull(path, "path");

            return await _polymath.MakeVaultApiRequest<Secret<ListInfo>>(mountPoint ?? _polymath.VaultClientSettings.SecretsEngineMountPoints.KeyValueV2, "/metadata/" + path.Trim('/'), _polymath.ListHttpMethod, wrapTimeToLive: wrapTimeToLive).ConfigureAwait(_polymath.VaultClientSettings.ContinueAsyncTasksOnCapturedContext);
        }

Additional context

Vault supports both LIST & GET, can you add an option or something for us to be able to make the http request with the GET method ?
@greg-signi greg-signi changed the title ReadSecretAsync support GET Method ReadSecretPathsAsync to also support GET Method Sep 19, 2023
@greg-signi
Copy link
Author

@rajanadar do you have any thoughts on this ?

@konidev20
Copy link
Contributor

@greg-signi , let me see what I can do

@konidev20
Copy link
Contributor

Found the documentation for reference:

The API documentation uses LIST as the HTTP verb, but you can still use GET with the ?list=true query string.

Ref: https://developer.hashicorp.com/vault/api-docs#api-operations

@konidev20 konidev20 linked a pull request Oct 1, 2023 that will close this issue
Open
@konidev20
Copy link
Contributor

Hey @rajanadar,

Should we do this for other endpoints as well? I see that since LIST is not a standard HTTP method, it might cause issues for other folks as well.

I remember, @hoerup advising me not to use HTTP LIST in a previous PR that I had raised (#246 (comment)).

@greg-signi
Copy link
Author

greg-signi commented Oct 2, 2023 •
edited

@konidev20 thank you thank you 💯

@konidev20
Copy link
Contributor

Hey @greg-signi,
You can take a build from the branch and test if it works.

@greg-signi
Copy link
Author

Hey @konidev20
Yep appears to be working fine 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: update HTTP method from LIST to GET for ReadSecretPathsAsync konidev20/VaultSharp
2 participants
@konidev20 @greg-signi