-
Notifications
You must be signed in to change notification settings - Fork 153
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 optional annotations to secrets #1599
Conversation
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
Please see the failing build:
|
@atoulme I have added a changelog file to the PR, but I'm unsure of what to put in the issue field. I have a support case number, but I'm unsure if I should be referencing that in the changelog, or if there is supposed to be a github issue linked to this. |
you can use this PR number. I have gone ahead and added it. |
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. I have asked for a second review from the team.
Description: Add optional annotations to be passed for secrets.
My team uses a mutating webhook to inject secrets on create. We utilize annotations as part of this webhook which this chart does not currently support.
Testing: This is an optional value, and tested that no examples show any changes when rendering. I also tested rendering with the below test value file to verify my changes show up correctly. I did not think this change warranted a new example to be created.
Documentation: Updated values schema.
Note:
I went with an if conditional in the templates because I saw it was done this way in the serviceAccount.yaml file:
https://github.com/signalfx/splunk-otel-collector-chart/blob/main/helm-charts/splunk-otel-collector/templates/serviceAccount.yaml#L20-L23
Afterwards I noticed that the service.yaml uses with instead:
https://github.com/signalfx/splunk-otel-collector-chart/blob/main/helm-charts/splunk-otel-collector/templates/service.yaml#L17-L20
I don't know if there is a preference but I can change if requested.