[ROB-2024] assume role support #468
Conversation
WalkthroughAdds an optional EKS assume-role parameter across CLI, config model, and Prometheus AWS config construction; updates prometrix dependency to 0.2.3 in pyproject.toml and requirements.txt. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (run_strategy)
participant Config as Config Model
participant Utils as prometheus_utils.generate_prometheus_config
participant AWS as AWSPrometheusConfig
User->>CLI: run with --eks-assume-role=<ARN>
CLI->>Config: Construct Config(eks_assume_role=ARN, ...)
CLI->>Utils: generate_prometheus_config(settings=Config)
alt EKS managed Prometheus
Utils->>AWS: new AWSPrometheusConfig(..., assume_role_arn=Config.eks_assume_role)
AWS-->>Utils: AWS config instance
else other providers/branches
Utils-->>CLI: Other config instances (unchanged)
end
Utils-->>CLI: Prometheus config
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
robusta_krr/core/integrations/prometheus/prometheus_utils.py (1)
37-51: Handle missing AWS credentials gracefully (pre-existing)session.get_credentials() can return None. Calling get_frozen_credentials() will then crash. Add an explicit guard and a clearer error.
- else: - # we need at least one parameter from credentials, but we should use whatever we can from settings (this has higher precedence) - credentials = credentials.get_frozen_credentials() + else: + # we need at least one parameter from credentials, but we should use whatever we can from settings (this has higher precedence) + if credentials is None: + raise Exception( + "No AWS credentials found for EKS Managed Prometheus. " + "Provide --eks-profile-name or --eks-access-key/--eks-secret-key, " + "or configure credentials via environment/IRSA." + ) + credentials = credentials.get_frozen_credentials()
🧹 Nitpick comments (3)
robusta_krr/core/models/config.py (1)
49-49: Validate eks_assume_role as an IAM Role ARNAdd a light validator to catch typos early (supports aws, aws-cn, aws-us-gov partitions).
Additional code (outside this hunk):
@pd.validator("eks_assume_role") def validate_eks_assume_role(cls, v: Optional[str]) -> Optional[str]: if v is None: return v import re pat = r"^arn:(aws(-[a-z]+)?):iam::\d{12}:role\/.+$" if not re.match(pat, v): raise ValueError("--eks-assume-role must be a valid IAM role ARN, e.g. arn:aws:iam::123456789012:role/metrics-readonly") return vrobusta_krr/core/integrations/prometheus/prometheus_utils.py (1)
53-55: Simplify service_name selectionMinor: prefer a straightforward defaulting.
- service_name = settings.eks_service_name if settings.eks_secret_key else "aps" + service_name = settings.eks_service_name or "aps"robusta_krr/main.py (1)
186-191: Clarify help text with ARN exampleAdd a concrete ARN example to reduce user error.
- help="Sets the assumed role for eks prometheus connection. (for cross-account role assumption)", + help="IAM role ARN to assume for AWS Managed Prometheus (cross-account supported), e.g. arn:aws:iam::123456789012:role/metrics-readonly",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
pyproject.toml(1 hunks)requirements.txt(1 hunks)robusta_krr/core/integrations/prometheus/prometheus_utils.py(1 hunks)robusta_krr/core/models/config.py(1 hunks)robusta_krr/main.py(2 hunks)
🔇 Additional comments (4)
requirements.txt (1)
30-30: Prometrix bump to 0.2.3 — assume_role_arn supported
Confirmed that AWSPrometheusConfig in prometrix v0.2.3 defines anassume_role_arnfield.pyproject.toml (1)
33-33: Sync with requirements — LGTMVersion pin matches requirements.txt. Remember to refresh the lockfile (poetry lock) before merging.
robusta_krr/core/integrations/prometheus/prometheus_utils.py (1)
63-65: Pass-through of assume_role_arn — LGTMForwarding settings.eks_assume_role into AWSPrometheusConfig is correct.
robusta_krr/main.py (1)
352-352: Value is correctly plumbed into Configeks_assume_role is passed through to Config as expected.
No description provided.