ConfigMap with missing vault section should default to env vars #353
Conversation
When using a ConfigMap to hold the agent config, it is natural to omit the `vault` section, since the config may be hard-coded and won't know necessarily know the correct address for the Vault server. In that case, we should set any known information about the Vault server (address, cert, etc.) in environment variables passed to the agent so that it can connect to the correct Vault server. Fixes #300
| Name: "VAULT_TLS_SERVER_NAME", | ||
| Value: a.Vault.TLSServerName, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Should we also include VAULT_NAMESPACE?
There was a problem hiding this comment.
Good question. Namespace isn't part of the vault config block, only auto_auth. And if we set VAULT_NAMESPACE, it could end up overriding a namespace the user sets in their auto_auth config: https://github.com/hashicorp/vault/blob/9334ae5f338c90f9a6d65e749794593e50acd48d/command/agent.go#L367-L369. However, if they've explicitly set the namespace annotation, it does seem like a reasonable thing to add. Overall it's a difficult one to call, but it maybe feels safer to exclude it for now, to maintain the current behaviour?
There was a problem hiding this comment.
Exactly. I only targeted the things that are in the vault config block.
| if a.Vault.Address != "" { | ||
| envs = append(envs, corev1.EnvVar{ | ||
| Name: "VAULT_ADDR", | ||
| Value: a.Vault.Address, | ||
| }) | ||
| } |
There was a problem hiding this comment.
(Just making sure I understand) It looks like all these can be overridden by the user with annotations from https://www.vaultproject.io/docs/platform/k8s/injector/annotations#vault-annotations? So like for the vault address, the order of precedence would be:
vaultstanza in configmap- This setting of VAULT_ADDR
- The value of
vault.hashicorp.com/service
We may want to add a note about this new behavior in or adjacent to https://www.vaultproject.io/docs/platform/k8s/injector/examples
There was a problem hiding this comment.
Yes. I believe that is correct. I'll double-check it before merging, and add a ticket to update those docs.
There was a problem hiding this comment.
Yes, I confirmed that vault.hashicorp.com/service will override the agent container VAULT_ADDR settings.
|
I noticed this also fixes another use case: even with a helm-deployed Vault, this will now set the VAULT_ADDR as expected if the |
|
Thanks! |
When using a ConfigMap to hold the agent config, it is natural to omit
the
vaultsection, since the config may be hard-coded and won't knownecessarily know the correct address for the Vault server.
In that case, we should set any known information about the Vault server
(address, cert, etc.) in environment variables passed to the agent so
that it can connect to the correct Vault server.
Fixes #300