-
Notifications
You must be signed in to change notification settings - Fork 268
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
Allow users to configure webhook message content with a template file #253
Conversation
Codecov Report
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
- Coverage 81.60% 80.56% -1.04%
==========================================
Files 10 10
Lines 799 818 +19
==========================================
+ Hits 652 659 +7
- Misses 131 141 +10
- Partials 16 18 +2
Continue to review full report at Codecov.
|
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.
Rather than making the user mount the file, can you instead use a ConfigMap similar to the way the webhookURLSecretName
works.
@bwagner5 I have update the Helm chart by adding 2 new variables in order to set the configmap name and the template file name that will be mounted into the pod. |
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.
This looks good! I missed one thing during my first pass review.
Can you update the ValidateWebhookConfig()
function as well? The validate function is called on startup so you know if your template is invalid upfront rather than waiting for a termination event for the error to be thrown.
https://github.com/aws/aws-node-termination-handler/blob/master/pkg/webhook/webhook.go#L103
…emplate content message as debug + typo nthConfig
I updated the ValidateWebhookConfig function as requested. I also fixed a typo on the nthConfig variable which was written in 2 different ways. Looks like the travis test has failed because of an issue with ec2Mock not responding, but I don't know how to re trigger the job. |
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!
Description of changes:
When the destination of the webhook requires a more complex message content, it can be easier to provide a json template file which will be mounted into the pod, rather than having the webhook message content set in an inline variable.
This will allow users to set a WEBHOOK_TEMPLATE_FILE environment variable (--webhook-template-file argument) to set the template file path which will be used to send the webhook message. This takes precedence over the --webhook-template argument.
Template file will have to exist (be mounted into the pod as volume by users) otherwise it will throw an error.
Example:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.