Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define order of vault init container #53

Closed
drpebcak opened this issue Jan 20, 2020 · 9 comments · Fixed by #91
Closed

Define order of vault init container #53

drpebcak opened this issue Jan 20, 2020 · 9 comments · Fixed by #91
Labels
enhancement New feature or request injector Area: mutating webhook service question A general question about usage
Milestone

Comments

@drpebcak
Copy link

I have an application which uses an init container to do database migrations. This init container wants to have secrets from vault in it.

The vault init container is injected, but it never starts because the db migration sidecar starts first.

Is it possible to get the vault init container to run first?

@jasonodonnell
Copy link
Contributor

Hi @drpebcak, it's currently not possible to set the order for the init container (it's always going to come last since we append it). It's should be possible to support this, though.

@jasonodonnell jasonodonnell added enhancement New feature or request question A general question about usage labels Jan 21, 2020
@drpebcak
Copy link
Author

This is a blocker for us, so hopefully it’s something that could be implemented fairly soon.

Thanks for your response!

@jasonodonnell
Copy link
Contributor

Something like this should be possible(where the default is append), exact placement would not be:

vault.hashicorp.com/agent-inject-init-order: "prepend"
vault.hashicorp.com/agent-inject-init-order: "append"

@drpebcak
Copy link
Author

Sounds good to me. I saw there was a related issue that this might solve with istio. I think this approach would work for 99% of use cases.

@tvoran tvoran added injector Area: mutating webhook service chart Area: helm chart and removed chart Area: helm chart labels Jan 22, 2020
@jjensen90
Copy link

jjensen90 commented Jan 29, 2020

I also ran into the init container order being an issue and came here to find if there was an existing issue, I had to implement a clunky workaround. I would also really appreciate this feature (just prepend/append would be perfect).

@jasonodonnell jasonodonnell added this to the 0.3.0 milestone Feb 19, 2020
@rvdh
Copy link

rvdh commented Feb 26, 2020

I also ran into the init container order being an issue and came here to find if there was an existing issue, I had to implement a clunky workaround. I would also really appreciate this feature (just prepend/append would be perfect).

@jjensen90 could you describe the workaround you implemented?

I'm having this issue with Istio. The Istio init container runs first, which sets firewall rules to redirect all traffic to the Envoy sidecar. The Vault init container runs second, but can't reach Vault, because the traffic is redirected to the Envoy sidecar, which obviously isn't running yet.. so Connection refused.

@zystem
Copy link

zystem commented Mar 1, 2020

I think there is no need for "prepend" and "append" annotations. I think "vault-agent-init" must be always first. I have no ideas about how it can create problems or can break something.
And there are no technical problems to implement it. Just create temporary array, fill the first element of it with "vault-agent-init" and append it with existing init containers. Then rewrite an array of init containers in kubernetes with this temporary array.
Additionally, I found what /vault/secrets not mounted in init containers others than "vault-agent-init" . Need to fix it too.

@jasonodonnell
Copy link
Contributor

Hi @zystem, thanks for the note about volume mounts not being present in other init containers. I have fixed that in #91 .

I think being able to configure if Vault Agent runs first or last is still desirable. For example, istio may want to run first to configure iptables prior to Vault Agent running. There could be other examples not thought of so having a configurable makes this more flexible.

@drpebcak
Copy link
Author

drpebcak commented Mar 2, 2020

Yeah I would say in general it doesn't hurt to keep the old behavior for compatibility!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request injector Area: mutating webhook service question A general question about usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants