hashicorp / vault-k8s

First-class support for Vault and Kubernetes.
Mozilla Public License 2.0
784 stars 171 forks source link

Allow patching the Agent's configuration #621

Open busser opened 5 months ago

busser commented 5 months ago

Hi πŸ‘‹

Is your feature request related to a problem? Please describe.

We use the agent injector to add the agent's init-container and sidecar to our pods. The agent reads a dynamic secret and writes it to a file that our service reads on startup and then watches for changes. The agent renews the secret's lease and re-reads the secret when the lease is about to expire. This works perfectly. Thank you!

By default the sidecar re-reads the dynamic secret when it starts up, and then edits the file our service watches. This is unnecessary, because the secret fetched by the init-container is still valid. To fix this, we enabled persistent caching with the vault.hashicorp.com/agent-cache-enable annotation. Now the init-container persists its lease in a file which the sidecar reads when it starts up. Having the lease, the sidecar no longer re-reads the dynamic secret immediately. This works as planned.

The issue is that the annotation also enables the Vault Agent's API Proxy. In our setup, we don't want the API Proxy enabled, because of this bug: https://github.com/hashicorp/vault/issues/19684. While we work on fixing this bug, we'd also like to disable the API Proxy entirely while keeping the persistent cache between the init-container and sidecar.

Describe the solution you'd like

We would like to add annotations that allow users to patch the Agent configuration generated by the Agent Injector.

Analogous to the existing agent-json-patch and agent-init-json-patch annotations, that allow patching the injected containers, we could add agent-config-json-patch and agent-init-config-json-patch annotations that allow users to patch the generated configuration.

In our case, these annotations would allow us to remove the fields that enable the API Proxy. I also believe that such generic annotations could prove useful in many other situations where users want to adjust the Agent's configuration.

Describe alternatives you've considered

I considered adding an annotation that enables the persistent cache between the containers but does not enable the API Proxy. Something like agent-persistent-cache-enable. However I see two issues with this approach:

  1. The semantics of this annotation and how it relates to the existing agent-cache-enable annotation would likely be very confusing to users.
  2. The Agent Injector's current code makes no distinction between the "API Proxy" and "persistent cache" features. Splitting the logic in two would require significant work.

I also considered removing the need for persistence between the init-container and sidecar entirely. We could achieve this with Kubernetes native sidecars. However the Agent Injector does not yet support injecting the Agent as a native sidecar. I have created issue #620 to track this feature.

Additional context

https://github.com/hashicorp/vault/issues/19436#issuecomment-1613735592 discusses how the Agent's configuration can be adjusted to enable persistence between the two containers without enabling the API Proxy.

I am more than happy to help implement this feature.

VioletHynes commented 5 months ago

Hey there! I'm less familiar with the inner workings of the Agent Injector annotations, and I'd agree that you should be able to turn on the cache without the API proxy for Agent. In the meantime, I wanted to point out that we do support giving a configmap for Agent config: https://developer.hashicorp.com/vault/docs/platform/k8s/injector/annotations#vault-hashicorp-com-agent-configmap

This means you could configure Agent's cache without a listener via HCL. It's just a workaround, but I wanted to point it out in case it'd help.

tvoran commented 5 months ago

Yeah, a listener hasn't been a requirement for the agent cache since Vault 1.9 I believe, so we should be able to remove it from the injected configuration.

The exception to this is when the quit endpoint is enabled, which would still require a listener. (And we'll probably need some other way to enable the agent's API proxy mode.)

As for general agent-config-json-patch and agent-init-config-json-patch options, I think I'm generally in favor πŸ‘. And like Violet mentioned the configmap option also allows for full control over the injected configurations.

busser commented 5 months ago

Hi @VioletHynes, thanks for responding! While the agent-configmap annotation is definitely useful, it is incompatible with other annotations like agent-inject-template. Ideally we'd like to patch the generated configuration rather than replace it entirely.

Hey @tvoran, I'm glad you like the idea of *-config-json-patch annotations! I'm willing to get started on an implementation, if that's OK with you. If so, is there anything I should be mindful of while working on this feature?