Skip to content

Conversation

@arikalon1
Copy link
Contributor

…ven not in openshift)

@arikalon1 arikalon1 requested a review from RoiGlinik December 3, 2025 17:41
@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

This PR introduces support for using pod service account tokens for Prometheus/Alertmanager authentication. A new environment variable flag PROMETHEUS_CLUSTER_TOKEN_AUTH is added to control this behavior. The token loading logic is updated to check this flag alongside the existing OpenShift condition. Documentation is updated to explain the new configuration option.

Changes

Cohort / File(s) Summary
Documentation updates
docs/configuration/metric-providers-external.rst,
docs/configuration/metric-providers-in-cluster.rst
Added configuration examples demonstrating how to use pod service account tokens for Prometheus/Alertmanager authentication via runner.additional_env_vars.PROMETHEUS_CLUSTER_TOKEN_AUTH set to "true".
Environment variable declaration
src/robusta/core/model/env_vars.py
Introduced new public boolean variable PROMETHEUS_CLUSTER_TOKEN_AUTH initialized from environment via load_bool() with default value False.
Token loading logic
src/robusta/integrations/openshift/token.py
Updated to import PROMETHEUS_CLUSTER_TOKEN_AUTH and modified eligibility check in load_token() to return None if neither IS_OPENSHIFT nor PROMETHEUS_CLUSTER_TOKEN_AUTH is true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that adding the PROMETHEUS_CLUSTER_TOKEN_AUTH check to the token loading condition does not inadvertently change behavior for existing OpenShift deployments
  • Confirm environment variable naming aligns with existing configuration patterns

Suggested reviewers

  • moshemorad

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: enabling service account token authentication for Prometheus/Alertmanager outside OpenShift environments.
Description check ✅ Passed The description, while brief, is directly related to the changeset and aligns with the PR objectives about enabling service account token authentication for Prometheus/Alertmanager.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch prom-serviceaccount-auth

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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_AUTH is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 453fb97 and 2df2a7f.

📒 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_OPENSHIFT flag. The default value of False maintains backward compatibility.

src/robusta/integrations/openshift/token.py (2)

3-3: Import looks good.

The addition of PROMETHEUS_CLUSTER_TOKEN_AUTH to 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_OPENSHIFT or PROMETHEUS_CLUSTER_TOKEN_AUTH is 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. The load_token() function is used for both AlertManager and Prometheus authentication contexts, with callers properly checking for None return 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.

@RoiGlinik RoiGlinik merged commit f7a22f7 into master Dec 3, 2025
10 checks passed
@RoiGlinik RoiGlinik deleted the prom-serviceaccount-auth branch December 3, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants