-
Notifications
You must be signed in to change notification settings - Fork 2k
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
disable injecting unnecessary variables allowing access to k8s API #1591
disable injecting unnecessary variables allowing access to k8s API #1591
Conversation
@@ -98,6 +98,7 @@ function(params) { | |||
apiVersion: 'v1', | |||
kind: 'ServiceAccount', | |||
metadata: p._metadata, | |||
automountServiceAccountToken: false, |
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 part can be treated as a breaking change for folks who are using prometheus-operator in versions that do not have change from prometheus-operator/prometheus-operator#4514. Do you think we should wait with this until we have a new operator version and then circle back?
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.
yes sounds good
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.
Just out of curiosity, what is the next release date?
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.
Yesterday 😄
7f58166
to
2d077a0
Compare
rebased |
b9c3de4
to
648a675
Compare
648a675
to
8cb6979
Compare
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
Aside, if we had some way of generating certs and configure scraping over TLS or mTLS would it be wise to remove (optionally) kube-rbac-proxy or are there some other considerations to be had?
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.
Nice, thanks for taking care of this one!
After the kube-prometheus upgrade in #65, Grafana broke because the k8s-sidecar container that loads custom dashboards needs access to the service account token. That access was removed in prometheus-operator/kube-prometheus#1591 . A quick search did not turn up a simple way to only mount the token in one of the pod's containers (`k8s-sidecar`, not `grafana`), so we're mounting it for all containers in the pod instead. Might be possible to tighten this up further.
66: monitoring: mount service account token for grafana dashboard sidecar r=bfritz a=bfritz After the kube-prometheus upgrade in #65, Grafana broke because the k8s-sidecar container that loads custom dashboards needs access to the service account token. That access was removed in prometheus-operator/kube-prometheus#1591 . A quick search did not turn up a simple way to only mount the token in one of the pod's containers (`k8s-sidecar`, not `grafana`), so we're mounting it for all containers in the pod instead. Might be possible to tighten this up further. Co-authored-by: Brad Fritz <[email protected]>
Description
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.
This will explicitly specify which applications need SA token and which do not need it.
Due to how prometheus server is managed, this PR requires prometheus-operator/prometheus-operator#4514 to be merged first.
This is a part of #1589
Type of change
What type of changes does your code introduce to the kube-prometheus? Put an
x
in the box that apply.CHANGE
(fix or feature that would cause existing functionality to not work as expected)FEATURE
(non-breaking change which adds functionality)BUGFIX
(non-breaking change which fixes an issue)ENHANCEMENT
(non-breaking change which improves existing functionality)NONE
(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Changelog entry
Please put a one-line changelog entry below. Later this will be copied to the changelog file.