-
Notifications
You must be signed in to change notification settings - Fork 416
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
feat: allow webhook configuration via helm values #3832
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.
@ndegory Thanks so much for the pr. Left a comment! Also, would you be able to address another request related to it that a user asked on slack?
When using gotemplate in the config, we need to have the template file in the container. I was thinking about having a specific configmap for it, if the user pass a template we can apply it on the cluster, and then mount it on the daemonset. WDYT?
yes, that would be useful. So far I've been using a post renderer patch to do that, but I'd prefer to have this available in the chart. I'll add it to the PR. |
@ndegory can you remove the bump version commit? Maybe it is best to merge this one, than you can create the configmap in a separate PR if it makes sense for you. |
903fb11
to
cd71dad
Compare
As still in testing, pushed it to next milestone. |
@ndegory Sorry for the delay, it was holidays in Brazil. Do you have an example of how you would use this option? Because currently we allow two options: passing a full config file:
or passing specific options:
And when using single options, I was trying to not support multiple values as it make the helm syntax complex, for example, webhook now could be configured with
Though I like your change, as webhook should be under the config options, not outside by itself, maybe I'm wondering if perhaps we should simplify the syntax to allow one webhook to be configured, and if someone wants more options they can use the whole file option, but not sure, WDYT? |
don't you also support passing a values file with
Works for me, I'll update the PR. |
Signed-off-by: Nicolas Degory <nicolas.degory@gmail.com>
cd71dad
to
828b873
Compare
I cannot update the PR description, the optional webhook configuration in the config section is not an array anymore:
I tested it successfully with a values file, but also with the --set flag:
|
1. Explain what the PR does
903fb11 chore: bump chart version to 0.19.1
32a892a feat: allow webhook configuration via helm values
the webhook section of the tracee config map can not be configured anymore since the refactoring in version 0.19.0 of the Helm chart. This PR allows to configure the webhook. Choice has been made to map the webhook keys (instead of just dumping the values as is in the config map), similarly to what already exists for other portions of this configmap's values file content.
2. Explain how to test it
output doesn't include the webhook configuration (no reg).
output contains this webhook configuration:
3. Other comments
webhook content example in config samples: https://github.com/aquasecurity/tracee/blob/main/examples/config/global_config.yaml