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

Add annotations to configure Agent Cache #132

Merged
merged 4 commits into from
Jun 4, 2020

Conversation

lawliet89
Copy link
Contributor

@lawliet89 lawliet89 commented May 6, 2020

  • TLS is always disabled because there is no way to mount arbitrary
    volumes to the agent sidecar container

Fixes #73

$ make test
go test -race ./...
?       github.com/hashicorp/vault-k8s  [no test files]
ok      github.com/hashicorp/vault-k8s/agent-inject     (cached)
ok      github.com/hashicorp/vault-k8s/agent-inject/agent       (cached)
ok      github.com/hashicorp/vault-k8s/helper/cert      (cached)
ok      github.com/hashicorp/vault-k8s/subcommand/injector      (cached)
?       github.com/hashicorp/vault-k8s/subcommand/version       [no test files]
?       github.com/hashicorp/vault-k8s/version  [no test files]

- TLS is always disabled because there is no way to mount arbitrary
volumes to the agent sidecar container
AnnotationAgentCacheUseAutoAuthToken = "vault.hashicorp.com/agent-cache-use-auth-auth-token"

// AnnotationAgentCacheListenerAddress configures the address the agent cache should listen on
AnnotationAgentCacheListenerAddress = "vault.hashicorp.com/agent-cache-listener-address"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this shouldn't be configurable but always be 127.0.0.1. This really shouldn't be listening to anything other than localhost. Especially since the use-auth-auth-token parameter is configurable, unauthenticated clients could retrieve secrets from Vault if misconfigured.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of allowing the user to change the port. Should I change this into a port configuration then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your thoughts with enabling the listener? If other containers in the pod want to use the vault agent as a proxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean by this question. Do you mean what are my thoughts on allowing the listener to listen to IP addresses other than 127.0.0.1? I think you're right that it is a bad idea.

Copy link
Contributor

@jasonodonnell jasonodonnell May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trying to understand the feature more in general.

We definitely don't want to bind to anything other than localhost, port configuration could be useful.

Copy link
Contributor Author

@lawliet89 lawliet89 May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think this is a very useful feature. Other than just proxying, the vault agent cache also takes care of renewing secrets. I have some applications that basically "outsource" the renewal duty to Vault agent. Otherwise, they would have to implement the logic themselves, which might not be trivial.

The agent cache also takes care of Vault's CA without having to require the user to somehow configure themselves to accept the Vault CA.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with it being useful. I think turning this into a port configurable makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I will make the change in a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. See f0dc686

@lawliet89
Copy link
Contributor Author

@jasonodonnell Could we get this merged in if it's ok? There's been quite a few conflicts.

Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jasonodonnell jasonodonnell merged commit d0c76c7 into hashicorp:master Jun 4, 2020
@lawliet89 lawliet89 deleted the agent-cache branch June 5, 2020 02:51
@hamishforbes
Copy link

Any chance of a release with this feature? It just barely missed the 0.4.0 release
Would really be appreciated as doing this with config maps for vault is a real pita!

@reegnz
Copy link

reegnz commented Jul 21, 2020

Please do a 0.5.0 release with this! Would be really great if I could start using this and not have to do explicit agent configs to do have listeners. :)

@lawliet89
Copy link
Contributor Author

lawliet89 commented Aug 12, 2020

@tvoran and @jasonodonnell : Could you please make a new release with this?

In any case, if you cannot wait, I've built and push a new image to lawliet89/vault-k8s:0.4.1-gh132 from commit 23879d1

The caveats are:

  • Built with make image
  • Binary was compiled on my machine
  • Binary is not signed by Hashicorp GPG key

EDIT: Might also be great to include #167 for a "pure" agent cache usage scenario.

@jasonodonnell
Copy link
Contributor

We're planning a release soon @lawliet89, there's a few more PRs we're hoping to pull in for that release. Likely next week (August 17th-21st).

@jasonodonnell
Copy link
Contributor

@lawliet89 My apologies for the delay on this release. I wanted to circle back to let you know this release is planned for August 24th. Vault is currently being released and since the Vault container is an official Docker image, we need to wait for them to publish the new images, which can take a day or two. Once the official images are available on Dockerhub, we'll update Vault K8s internal references to the latest image and get this published.

Thanks!

@reegnz
Copy link

reegnz commented Aug 24, 2020

It seems like those images got pushed 3 days ago.
Hoping for a release today! 🤞

How come this release is blocked by the new image version BTW? These releases shouldn't rely on what vault version we're running.

@jasonodonnell
Copy link
Contributor

@reegnz, it doesn't really matter, but there are references internally for default injected image:

DefaultVaultImage = "vault:1.4.2"
.

Whenever possible we try to update this to the latest image. 1.5.2 released late on Friday so Vault K8s releases today.

@jasonodonnell jasonodonnell mentioned this pull request Aug 24, 2020
RemcoBuddelmeijer pushed a commit to RemcoBuddelmeijer/vault-k8s that referenced this pull request Feb 22, 2022
* Add annotations to configure Agent Cache

- TLS is always disabled because there is no way to mount arbitrary
volumes to the agent sidecar container

* Allow configuring listener port and not address
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support running Vault Agent cache in sidecar container
4 participants