-
Notifications
You must be signed in to change notification settings - Fork 884
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
support extraLabels for vault-agent-injector #428
Conversation
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.
Hi @logileifs, thanks for the contribution!
This looks good but will require unit tests to make sure the configurable works and has coverage. Would you mind adding some? You can find the bats
injector unit tests here: https://github.com/hashicorp/vault-helm/blob/master/test/unit/injector-deployment.bats.
Here's a test that's equivalent: https://github.com/hashicorp/vault-helm/blob/master/test/unit/server-statefulset.bats#L891-L901.
@logileifs Also the CLA will need to be signed 😄 |
Hey @jasonodonnell thanks for the tips, I added a test for the extraLabels on the injector and I have already signed the CLA, I don't know why it's not updating 😕 |
I figured out what the problem was, everything should be in order now :) |
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.
One last change, this needs to be added to the values.yaml
file. You can set it to blank just we like we do for server
:
injector:
...
extraLabels: {}
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.
LGTM, thanks for the contribution!
Thanks for the quick response @jasonodonnell and just out of curiosity, when can I expect to see this change in a release? |
We release monthly (if there's things to be released) on the third Tuesday, so good chance you'll see it then. |
fixes issue #427