-
Notifications
You must be signed in to change notification settings - Fork 168
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 namespace and log level annotations #82
Conversation
7cd4f04
to
41ed03e
Compare
agent-inject/agent/container_env.go
Outdated
@@ -10,6 +10,11 @@ import ( | |||
func (a *Agent) ContainerEnvVars(init bool) ([]corev1.EnvVar, error) { | |||
var envs []corev1.EnvVar | |||
|
|||
envs = append(envs, corev1.EnvVar{ | |||
Name: "VAULT_TOKEN", | |||
Value: "/home/vault/.vault-token", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use the TokenFile
const?
vault-k8s/agent-inject/agent/config.go
Line 14 in 41ed03e
TokenFile = "/home/vault/.vault-token" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this environment variable hold the contents of the token instead? cf. https://www.vaultproject.io/docs/commands/#vault_token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bug with Kube auth login and namespaces that we haven't been able to track down, so setting this was the only way to get it to work. Auto-auth populates file cache which takes priority. This could be any value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can put a comment in about this being a work around until we find the bug in the auth method.
There was a problem hiding this comment.
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 might affect the auto-revoke on shutdown that I implemented in #67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will test the two together and let you know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lawliet89 I found a less intrusive solution. This shouldn't cause issues for you!
Really looking forward to this PR going in 😄 Great work! |
* Add namespace and log level annotations * Remove env, add blank token file * Fix tests * gofmt
This resolves #20 by adding Vault namespace support for enterprise installations. Additionally when debugging namespace features, I needed to increase log level. This adds another annotation to change the Vault Agent log level (default to
info
).