-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[kube-prometheus-stack] Do not send the bearer token to every service #6427
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
base: main
Are you sure you want to change the base?
[kube-prometheus-stack] Do not send the bearer token to every service #6427
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 @killerwhile, thanks for the contribution.
What about using sendBearerToken instead and set it true by default that still keeps it backwards compatible? I think semantically is easier to understand than a possible double negative.
|
Thanks for your comment. This would make more sense, indeed. I'll update shortly. Thanks |
…o some services Signed-off-by: Killerwhile <[email protected]>
a05f904 to
076d5dd
Compare
|
@GMartinez-Sisti I updated as you proposed and DCO signed the commit. |
| {{- if .Values.coreDns.serviceMonitor.sendBearerToken }} | ||
| bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token | ||
| {{- end }} |
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.
for consistency reasons, i preffer the pattern that we used for other properties as well.
Move bearerTokenFile to values and you can empty the value on your own.
| {{- if .Values.coreDns.serviceMonitor.sendBearerToken }} | |
| bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token | |
| {{- end }} | |
| {{- if .Values.coreDns.serviceMonitor.bearerTokenFile }} | |
| bearerTokenFile: {{ .Values.coreDns.serviceMonitor.bearerTokenFile }} | |
| {{- end }} |
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.
Thanks for the review. I'll adapt shortly.
What this PR does / why we need it
Some ServiceMonitors send bearer token where it's not needed, especially to http endpoints.
This PR do allow to configure when to set the bearer token. It keeps the actual behavior, configuration options should actively being set to change the behavior.
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged)Special notes for your reviewer
Checklist
[prometheus-couchdb-exporter])