search

falcosecurity / charts

Community managed Helm charts for running Falco with Kubernetes
Apache License 2.0
246 stars 291 forks source link

fix(falcosidekick): Allow overriding the Prometheus scrape Service annotation #779

Open TiagoJMartins opened 2 weeks ago

TiagoJMartins commented 2 weeks ago

What type of PR is this?

/kind bug /kind chart-release

Any specific area of the project related to this PR?

/area falcosidekick-chart

What this PR does / why we need it:

We are working on a migration from a deprecated Prometheus setup to one based on the Prometheus Kubernetes Operator. Since metrics scraped by the operator-managed instances are targeting ServiceMonitor and PodMonitor resources, we need to be able to configure the prometheus.io/scrape Service annotation to gradually disable scraping from our deprecated instances.

Which issue(s) this PR fixes:

Because the prometheus.io/scrape annotation is hard-coded in the Service template and it's the last entry in the annotations map, there's no way to include custom values after it.

This PR simply moves the hard-coded annotation to before the user-provided values, so at least they have an escape hatch in case they need to override the value.

Special notes for your reviewer:

You can test with:

helm template falcosidekick charts/falcosidekick -s templates/service.yaml -f - <<EOF | yq '.metadata.annotations["prometheus.io/scrape"]'
service:
  annotations:
    prometheus.io/scrape: "false"
EOF

The final template will include duplicate values, but the override will take precedence when the yaml gets parsed/applied to the cluster.

apiVersion: v1
kind: Service
metadata:
  name: falcosidekick
  annotations:
    prometheus.io/scrape: "true"
    prometheus.io/scrape: "false"
  ...

The duplicated keys will be removed by the cluster.

I considered going with a different approach based on something like .Values.service.prometheusScrape, but decided against it due to it requiring changes to the values.yaml structure.

Checklist

poiana commented 2 weeks ago

Welcome @TiagoJMartins! It looks like this is your first PR to falcosecurity/charts 🎉

poiana commented 2 weeks ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: TiagoJMartins Once this PR has been reviewed and has the lgtm label, please assign issif for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/falcosecurity/charts/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
rsicart commented 2 weeks ago

And what about removing the hard-coded annotation and putting it by default in .Values.service.annotations?