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

Enable Vault to review kube tokens when using external Vault #392

Merged

Conversation

jfroche
Copy link
Contributor

@jfroche jfroche commented Sep 29, 2020

We want Vault to perform token reviews with Kubernetes even if we are
using an external Vault.

We need to create the ServiceAccount, Secret and ClusterRoleBinding with
the system:auth-delegator role to enable delegated authentication and
authorization checks [1].

These SA and RBAC objects are created when we deploy the Vault server.
In order to enable the creation of these objects when using an external
Vault, we remove the condition on external mode.

User might want to provide a sensible name (in global.serviceAccount.name) to the service
account such as: vault-auth.

refs #376

[1] https://www.vaultproject.io/docs/auth/kubernetes#configuring-kubernetes

@jfroche jfroche force-pushed the feat/external-vault-sa-auth-delegator branch from f26d636 to 8b5d3d8 Compare September 29, 2020 21:37
@michalschott
Copy link

I'm facing the same problem here, temporary workaround is to do helm template and patch with kustomize. Upvoting.

@jasonodonnell
Copy link
Contributor

Hi @jfroche, thanks for the contribution!

I'll take a closer look at this PR today but please remove the CHANGELOG changes, we do not make those changes in PRs and will update the CHANGELOG in master after PRs are merged.

@jfroche jfroche force-pushed the feat/external-vault-sa-auth-delegator branch from 8b5d3d8 to caeaebc Compare September 30, 2020 17:02
@jasonodonnell jasonodonnell self-requested a review October 16, 2020 17:16
values.yaml Outdated
Comment on lines 28 to 37
serviceAccount:
# Specifies whether a service account should be created
create: true
# The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
name: ""
# Extra annotations for the serviceAccount definition. This can either be
# YAML or a YAML-formatted multi-line templated string map of the
# annotations to apply to the serviceAccount.
annotations: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be moved away from server. This will make the change backwards compatible and it's specific to the server, not a global config.

@jasonodonnell
Copy link
Contributor

@jfroche This looks good to me but we need to move SA back to server. This will break existing users and while I appreciate increasing the visibility of this setting, it's specific to the server and not a global config. I'll merge once this is moved 😄

We want Vault to perform token reviews with Kubernetes even if we are
using an external Vault.

We need to create the ServiceAccount, Secret and ClusterRoleBinding with
the system:auth-delegator role to enable delegated authentication and
authorization checks [1].

These SA and RBAC objects are created when we deploy the Vault server.
In order to enable the creation of these objects when using an external
Vault, we remove the condition on external mode.

User might want to provide a sensible name (in global.serviceAccount.name) to the service
account such as: vault-auth.

refs hashicorp#376

[1] https://www.vaultproject.io/docs/auth/kubernetes#configuring-kubernetes
@jfroche jfroche force-pushed the feat/external-vault-sa-auth-delegator branch from caeaebc to 4778894 Compare October 16, 2020 20:47
@jfroche
Copy link
Contributor Author

jfroche commented Oct 16, 2020

@jasonodonnell Ok, fixed it

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!

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.

3 participants