Vault Agent Cache #6220
Vault Agent Cache #6220
Conversation
vishalnayak
force-pushed
the
agent-cache
branch
from
aa77467
to
62feece
Compare
vishalnayak
force-pushed
the
agent-cache
branch
from
5d59794
to
9c3d366
Compare
|
|
vishalnayak
force-pushed
the
agent-cache
branch
from
9c3d366
to
0a9b640
Compare
vishalnayak
force-pushed
the
agent-cache
branch
from
0a9b640
to
67dc23d
Compare
| req.Request.Body.Close() | ||
| } | ||
| req.Request.Body = ioutil.NopCloser(bytes.NewBuffer(req.RequestBody)) | ||
|
|
There was a problem hiding this comment.
Won't this function not compute an equivalent index if the parameters are given in a different order?
There was a problem hiding this comment.
Yes, we settled on being acceptable for this first iteration (I forget if it was over Slack or one of the sync-ups). The expectation is that clients won't be switching back and forth on the ordering of the params that they request through agent since these are mainly going to be scripts or application services making the request.
There was a problem hiding this comment.
The problem with that argument is that you assume JSON marshaling behavior is consistent from language to language or library to library. It may be, but it may not. Go lexically orders JSON keys, but there's a performance impact to doing so, so speed demon libs or implementations may simply randomly order the parameters.
As an example, even in Go such applications may use jsoniter instead of stdlib json, and in jsoniter sorting the keys is an option that is not turned on by default.
|
@jefferai While you continue to give comments on this PR, I'll be addressing the review comments in here https://github.com/hashicorp/vault/pull/6237/files. |
| Listeners []*Listener `hcl:"listeners"` | ||
| } | ||
|
|
||
| type Listener struct { |
There was a problem hiding this comment.
Can you instead just use the same Listener that we use in Vault? I think the parsing code below is basically a straight copy so it seems like a win to factor it out somewhere (e.g. helper/listener or so).
| @@ -72,7 +72,7 @@ func listenerWrapProxy(ln net.Listener, config map[string]interface{}) (net.List | |||
| return newLn, nil | |||
| } | |||
|
|
|||
| func listenerWrapTLS( | |||
| func ListenerWrapTLS( | |||
There was a problem hiding this comment.
Same comment basically -- I think we should move the listener stuff out somewhere common.
No description provided.