hashicorp
VAULT-2012 pass CCC index from auto-auth to api proxy
There are still some parts of Agent that don't use this header, such as the LifetimeWatcher. Modifying the code so that index is used (and consistent) throughout every API call in Agent seemed like quite a large task with limited benefit. Still, if we want to go down that path in the future, the approach here should make it easier.
Can you create a test to verify the new behaviour?
I did look into this, but it wasn't particularly easy to do so given the layers of testing we have and the relative end-to-end-y nature of this. It also doesn't seem like we have the best testing infrastructure around CCC to begin with, as the initial PR that enabled this (https://github.com/hashicorp/vault/pull/10974) didn't seem to have tests around CCC either.
I'm open to suggestions if I'm missing something, though.
Can you create a test to verify the new behaviour?
I did look into this, but it wasn't particularly easy to do so given the layers of testing we have and the relative end-to-end-y nature of this. It also doesn't seem like we have the best testing infrastructure around CCC to begin with, as the initial PR that enabled this (#10974) didn't seem to have tests around CCC either.
I'm open to suggestions if I'm missing something, though.
#10974 was the OSS part of https://github.com/hashicorp/vault-enterprise/pull/1679/files. See consistency_headers_ent_test.go.
Can you create a test to verify the new behaviour?
I did look into this, but it wasn't particularly easy to do so given the layers of testing we have and the relative end-to-end-y nature of this. It also doesn't seem like we have the best testing infrastructure around CCC to begin with, as the initial PR that enabled this (#10974) didn't seem to have tests around CCC either. I'm open to suggestions if I'm missing something, though.
#10974 was the OSS part of https://github.com/hashicorp/vault-enterprise/pull/1679/files. See consistency_headers_ent_test.go.
From what I can tell, that's more testing the consistency headers feature in general, not agent or its use of them. My PR only touches agent. To rephrase - I couldn't find any tests that validate correct use of consistency headers in Agent (though, again, I could be missing something).
From what I can tell, that's more testing the consistency headers feature in general, not agent or its use of them. My PR only touches agent. To rephrase - I couldn't find any tests that validate correct use of consistency headers in Agent (though, again, I could be missing something).
Possibly there are not, I pointed you at the file because I thought there were techniques within that could be helpful in writing an agent test for ccc, e.g. to keep a node backlogged until a client does a retry so we can ensure that CCC is handling that scenario.
Everything looks good to me. I'll wait to approve, however, in case there is additional testing coming.
BTW, I did find that we have TestAgentClientConsistency to validate the proxy's use of CCC.
I spent a lot of time with @ncabatoff today and we collectively realized we no longer need this PR due to SSCT tokens. This change does work, in the sense that the X-Vault-Index header would be present now. However, this is no longer required unless you fiddle with the agent client configuration and as a result this change is extremely low value for the cost of complicating the code.
We're making the decision to not make this change, as a result. Tests that cover SSCT tokens implicitly test this functionality, too.