-
Notifications
You must be signed in to change notification settings - Fork 288
Allow using service account token for Prometheus/Alertmanager auth (e… #1968
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
Conversation
…ven not in openshift)
WalkthroughThis PR introduces support for using pod service account tokens for Prometheus/Alertmanager authentication. A new environment variable flag Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/robusta/integrations/openshift/token.py (1)
5-6: Consider updating the comment for clarity.The comment currently mentions "if openshift is enabled" but the token is now used for non-OpenShift clusters as well when
PROMETHEUS_CLUSTER_TOKEN_AUTHis set.Consider updating the comment to reflect the broader usage:
-# NOTE: This one will be mounted if openshift is enabled in values.yaml +# NOTE: This is the standard Kubernetes service account token path, used when IS_OPENSHIFT or PROMETHEUS_CLUSTER_TOKEN_AUTH is enabled TOKEN_LOCATION = '/var/run/secrets/kubernetes.io/serviceaccount/token'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/configuration/metric-providers-external.rst(1 hunks)docs/configuration/metric-providers-in-cluster.rst(1 hunks)src/robusta/core/model/env_vars.py(1 hunks)src/robusta/integrations/openshift/token.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.7)
src/robusta/integrations/openshift/token.py
6-6: Possible hardcoded password assigned to: "TOKEN_LOCATION"
(S105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run_tests
- GitHub Check: run_tests
- GitHub Check: Deploy docs
🔇 Additional comments (5)
src/robusta/core/model/env_vars.py (1)
117-118: LGTM! Clean implementation of the new configuration flag.The new environment variable follows the established pattern and is appropriately placed alongside the related
IS_OPENSHIFTflag. The default value ofFalsemaintains backward compatibility.src/robusta/integrations/openshift/token.py (2)
3-3: Import looks good.The addition of
PROMETHEUS_CLUSTER_TOKEN_AUTHto the imports is clean and necessary for the updated logic.
10-11: Logic correctly extends token loading to non-OpenShift environments.The condition now properly checks both flags, allowing service account token authentication when either
IS_OPENSHIFTorPROMETHEUS_CLUSTER_TOKEN_AUTHis enabled. The parentheses ensure correct evaluation order.The implementation uses the standard Kubernetes service account token path (
/var/run/secrets/kubernetes.io/serviceaccount/token), which is mounted by default in all Kubernetes distributions, making this approach universally compatible. Theload_token()function is used for both AlertManager and Prometheus authentication contexts, with callers properly checking forNonereturn values.docs/configuration/metric-providers-in-cluster.rst (1)
95-103: Documentation is clear and well-placed.The new authentication option is properly documented with a clear example. The placement under the "Authentication" section is logical, and the YAML configuration example correctly shows the environment variable as a string value.
docs/configuration/metric-providers-external.rst (1)
58-67: Documentation is consistent and well-structured.The authentication documentation correctly mirrors the example in
metric-providers-in-cluster.rst, ensuring consistency across different Prometheus deployment scenarios. The placement after the Bearer Token example provides good context for users.
…ven not in openshift)