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.

Sign up for GitHub

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

Jump to bottom

Support Revoking Vault Token on Shutdown #67

Merged
merged 11 commits into from
Feb 25, 2020
Merged

Support Revoking Vault Token on Shutdown #67

merged 11 commits into from
Feb 25, 2020

Conversation

lawliet89
Copy link
Contributor

@lawliet89 lawliet89 commented Feb 4, 2020 •
edited
Loading

Fixes #65

Built on top of #66

$ 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]

jasonodonnell
jasonodonnell reviewed Feb 4, 2020
View reviewed changes
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.

This is a cool feature, thanks for doing this @lawliet89! Code looks good but I think this is a good candidate to also make an environment variable so all injections have the revoke lifecycle added. Thoughts?

@lawliet89
Copy link
Contributor Author

@jasonodonnell I have added a flag to allow users to opt in to having the revoke annotation set automatically on all pods.

@lawliet89
Copy link
Contributor Author

lawliet89 commented Feb 7, 2020 •
edited
Loading

I've tested this on my Kubernetes cluster and the lifecycle hooks are indeed added.

The Vault tokens are also revoked when I look at the audit logs from Vault server.

EDIT: I added some code to add additional flags to the revoke command to add the Vault address and CA related fields.

@jasonodonnell jasonodonnell self-requested a review February 20, 2020 16:24
jasonodonnell
jasonodonnell reviewed Feb 20, 2020
View reviewed changes
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.

Some conflicts need to be resolved, but this looks good now! I will merge once resolved. Thanks @lawliet89 !

@lawliet89
Copy link
Contributor Author

@jasonodonnell I've fixed the conflicts.

@lawliet89 lawliet89 mentioned this pull request Feb 21, 2020
Merged
@lawliet89
Copy link
Contributor Author

lawliet89 commented Feb 25, 2020 •
edited
Loading

@jasonodonnell I've fixed conflicts again. Could this be merged in sooner than later for fewer conflicts?

@jasonodonnell jasonodonnell self-requested a review February 25, 2020 12:14
jasonodonnell
jasonodonnell approved these changes Feb 25, 2020
View reviewed changes
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 @lawliet89

@jasonodonnell jasonodonnell merged commit ed146a2 into hashicorp:master Feb 25, 2020
@jasonodonnell jasonodonnell mentioned this pull request Mar 4, 2020
Merged
RemcoBuddelmeijer pushed a commit to RemcoBuddelmeijer/vault-k8s that referenced this pull request Feb 22, 2022
@lawliet89
Support Revoking Vault Token on Shutdown (hashicorp#67)
93cad11 
* Write Vault Token to the usual location

As documented by the default token helper: https://www.vaultproject.io/docs/commands/token-helper/

* Add annotation

* Add Lifecycle to container

* Fix gofmt

* Add option to automatically revoke tokens for all pods

* Add flags to revoke command

* Make flags only inside if clause
@lawliet89 lawliet89 mentioned this pull request Apr 21, 2022
Open
@lawliet89 lawliet89 deleted the revoke-on-shutdown branch March 26, 2024 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonodonnell jasonodonnell jasonodonnell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Revoking Vault Token on Pod Termination
2 participants
@lawliet89 @jasonodonnell