Remove kube-rbac-proxy sidecar from metrics exporter DaemonSet#1060
Remove kube-rbac-proxy sidecar from metrics exporter DaemonSet#1060zeeke wants to merge 1 commit intok8snetworkplumbingwg:masterfrom
Conversation
The metrics exporter now handles TLS and authentication/authorization natively, eliminating the need for the kube-rbac-proxy sidecar container. Update the metrics exporter DaemonSet to have a single container with TLS flags, authn/authz, and cert volume mount. Remove METRICS_EXPORTER_KUBE_RBAC_PROXY_IMAGE from controller, Helm chart, deploy manifests, env.sh, and tests Resolves: k8snetworkplumbingwg#811 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
|
Thanks for your PR,
To skip the vendors CIs, Maintainers can use one of:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the metrics exporter component by integrating TLS and authentication/authorization capabilities directly into the exporter itself. This change eliminates the dependency on the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly removes the kube-rbac-proxy sidecar from the metrics exporter DaemonSet, integrating TLS and authentication/authorization directly into the exporter. The changes are consistently applied across manifests, controller code, tests, and documentation. The overall change improves the architecture by simplifying the deployment. I have one suggestion to improve maintainability.
| - --web.listen-address=[$(HOST_IP)]:{{.MetricsExporterPort}} | ||
| - --tls-cert-file=/etc/metrics/tls.crt | ||
| - --tls-private-key-file=/etc/metrics/tls.key | ||
| - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 |
There was a problem hiding this comment.
For better maintainability, consider defining this long list of TLS cipher suites as a constant in a Go file and passing it to the template. Hardcoding it here makes it difficult to read and manage.
For example, you could:
- Define a constant in
pkg/consts/constants.go. - Pass it to the template data in
controllers/sriovoperatorconfig_controller.go. - Use the variable in this template as shown in the suggestion.
- --tls-cipher-suites={{.TLSCipherSuites}}
2 participants
The metrics exporter now handles TLS and authentication/authorization natively, eliminating the need for the kube-rbac-proxy sidecar container.
Depends on:
Resolves: