VAULT-11510 Vault Agent can start listeners without caching #18137
Conversation
|
Oh, woops, there are merge conflicts. Looking to fix those. |
| @@ -36,7 +90,7 @@ func Handler(ctx context.Context, logger hclog.Logger, proxier Proxier, inmemSin | |||
| } | |||
|
|
|||
| // Parse and reset body. | |||
| reqBody, err := ioutil.ReadAll(r.Body) | |||
| reqBody, err := io.ReadAll(r.Body) | |||
There was a problem hiding this comment.
ioutil.ReadAll is deprecated in favour of io.ReadAll
| @@ -45,7 +99,7 @@ func Handler(ctx context.Context, logger hclog.Logger, proxier Proxier, inmemSin | |||
| if r.Body != nil { | |||
| r.Body.Close() | |||
| } | |||
| r.Body = ioutil.NopCloser(bytes.NewReader(reqBody)) | |||
| r.Body = io.NopCloser(bytes.NewReader(reqBody)) | |||
There was a problem hiding this comment.
ioutil.NopCloser is deprecated in favour of io.NopCloser
There was a problem hiding this comment.
Looks good to me, I added some random 'bits' just so you know I read it cover to cover 👍🏼
I think there's only 1 typo that the suggestion probably needs accepting for, I'll approve it anyway and you can deal with that rather than 'request changes' seems a bit drawn out for a tiny thing.
|
@finnstech: #18092 can be closed once this ( |
|
After a discussion over Slack, I'm going to update this PR to rely upon a new HCL stanza, |
|
I just pushed an update to support the I'm going to write a new docs page about the API Proxy in general, as we don't actually have one. Lastly, it might be nice to refactor the code so that the Apologies that the PR is getting a little chunky. I think the only thing that's left is the new docs page. tl;dr: the PR isn't done yet, and I'll re-request review when it is. |
| } | ||
|
|
||
| // Create a muxer and add paths relevant for the lease cache layer and API proxy layer | ||
| mux := http.NewServeMux() |
There was a problem hiding this comment.
These functions have been moved over from the cache_test.go file, though the next ~25 lines of code were modified on the way.
|
Okay! I think we're good to go for review. Find the new doc page here: https://vault-hffnd61a7-hashicorp.vercel.app/vault/docs/agent/apiproxy We now support the I think I've updated everywhere that needs to be updated, but let me know if I missed anything. |
There was a problem hiding this comment.
Thanks for the PR Violet, sorry the re-review took me longer than I wanted.
I spent a while going over the docs since I'm newer to Vault Agent and wanted to make sure I understood it.
I've made a few suggestions which I think we need (typo type stuff), some that I think make the docs clearer (to me) which you can take or leave, and the occasional rambling where I was trying to figure things out.
|
Random, and not your code/docs etc. but any chance while you're in there you can fix this one thing that for some reason has been making my eye twitch:
to
|
* VAULT-11510 Vault Agent can start listeners without caching * VAULT-11510 fix order of imports * VAULT-11510 changelog * VAULT-11510 typo and better switch * VAULT-11510 update name * VAULT-11510 New api_proxy stanza to configure API proxy * VAULT-11510 First pass at API Proxy docs * VAULT-11510 nav data * VAULT-11510 typo * VAULT-11510 docs update
This enables the usage of listeners and the API proxy even without the caching block. Everything works in both cases, and requests to authenticated endpoints (like token lookup) work in both.
I made the decision to make
cache-clearreturn a 200 in the case that a cache does not exist, as I think that behaviour probably makes the most sense, but I'm welcome to being challenged on it.I also updated a lot of the docs around this area, including a new page for the API Proxy.
There were also config items that were using "caching" to refer to the requests made by the API proxy. I allowed "proxying" as an alternative to make things clearer, hopefully for both us and customers. Similarly, I factored out the configuration items in the
cachestanza that configured the API proxy to a newapi_proxystanza.