Set namespace for template server in agent #10757
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). vault-storybook – ./ui🔍 Inspect: https://vercel.com/hashicorp/vault-storybook/m2kgvmbhz [Deployment for 8f86d61 canceled] |
|
You might also want to include the CL entry in this PR :) |
| @@ -371,6 +371,7 @@ func (c *AgentCommand) Run(args []string) int { | |||
| // Check if a default namespace has been set | |||
| mountPath := config.AutoAuth.Method.MountPath | |||
| if cns := config.AutoAuth.Method.Namespace; cns != "" { | |||
| namespace = cns | |||
There was a problem hiding this comment.
I wonder if we should incorporate the logic for EnvVaultNamespace here? It's confusing to rely on the code in consult-template that consults the env when we don't specify a namespace explicitly. Especially since we do look at the env to set mountPath.
There was a problem hiding this comment.
The EnvVaultNamespace logic here seems like it is only applicable to the auth subsystem of agent, and it seems like a sane thing to do. It is possible that we can refactor this to better manage the code, but I am feeling reluctant to touch it at this point in the release cycle.
There was a problem hiding this comment.
It looks like by setting namespace = cns, if there is an env var set, the templating code will ignore it.
|
Did you test to validate that #8534 is still fixed with this change? |
|
We could've maybe avoided this regression if we'd adopted #9777 and used it to write automated tests against this part of the code. It's not too late (well, it is for 1.6.2, but going forward...) |
Actually maybe not, it looks like the problematic area is outside the scope of what I tackled in that PR. Nonetheless, it'd be nice to see more of agent's Run method moved into more testable places. |
|
Yes, I have verified that #8534 fix isn't broken. Used the script here to do it: https://github.com/hashicorp/vault-tools/blob/master/users/vishal/agent/agent_auto_auth_namespace.sh |
|
@ncabatoff FWIW I plan on re-visiting #9777 shortly. |
* Set namespace for template server in agent * cl++
* Add list peers to DR secondaries (#10746) * Fix build (#10749) * Move the declaration to a OSS build tag file to not have it collide w… (#10750) * Move the declaration to a OSS build tag file to not have it collide with ent declarations * Add comment * Remove comment to trigger ci * Fix remove peers check (#10758) * Remove peer DR op token check only on secondaries (#10765) * Set namespace for template server in agent (#10757) * Set namespace for template server in agent * cl++
Fixes #10602