Closed
Conversation
…oyments When use-assume is set with role-arn but WITHOUT global-role-arn, the connector now performs single-hop assume role (IRSA -> target role). This supports self-hosted deployments (e.g., EKS with IRSA) that don't need an intermediate binding account. Changes: - external-id is only required for two-hop mode (when global-role-arn is set) - Dockerfile uses correct binary name (baton-aws) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
When globalRoleARN is empty but roleARN is set, assume directly into the target role without an intermediate binding account hop. This enables self-hosted deployments (e.g., EKS with IRSA) to use assume role without needing ConductorOne's binding account flow. Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe changes refactor validation logic in the config module while introducing a single-hop assume-role path in the connector. When globalRoleARN is empty and roleARN is set, the system now directly assumes the specified role via STS; otherwise, it maintains the existing two-hop flow. Validation is reordered to check Role ARN first, with ExternalID validation only performed in two-hop mode. Changes
Sequence Diagram(s)sequenceDiagram
participant Code as Application Code
participant Connector as Connector Logic
participant STS as AWS STS
participant Cache as Credentials Cache
Code->>Connector: getCallingConfig(roleARN, externalID, globalRoleARN)
alt Single-Hop Mode (globalRoleARN empty)
Connector->>STS: AssumeRole(roleARN, externalID)
STS-->>Connector: Credentials
Connector->>Cache: Store credentials
Connector-->>Code: Config with assumed credentials
else Two-Hop Mode (globalRoleARN set)
Connector->>STS: AssumeRole(globalRoleARN)
STS-->>Connector: Intermediate credentials
Connector->>STS: AssumeRole(roleARN, externalID)
STS-->>Connector: Final credentials
Connector->>Cache: Store credentials
Connector-->>Code: Config with final credentials
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/config/config.go`:
- Around line 118-129: The code currently validates RoleArn via
connector.IsValidRoleARN but doesn't validate GlobalRoleArn; update the
validation flow in the same block to call connector.IsValidRoleARN on
awsc.GetString(GlobalRoleArnField.FieldName) (similar to RoleArnField) before
running ValidateExternalId, and return the error if that call fails; keep
ValidateExternalId(awsc.GetString(ExternalIdField.FieldName)) only for the
two-hop case when globalRoleArn != "".
Comment on lines
+118
to
+129
| err := connector.IsValidRoleARN(awsc.GetString(RoleArnField.FieldName)) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = connector.IsValidRoleARN(awsc.GetString(RoleArnField.FieldName)) | ||
| if err != nil { | ||
| return err | ||
| // Only validate external-id for two-hop mode (when global-role-arn is set) | ||
| // Single-hop mode (IRSA → target role) doesn't require external-id | ||
| globalRoleArn := awsc.GetString(GlobalRoleArnField.FieldName) | ||
| if globalRoleArn != "" { | ||
| err = ValidateExternalId(awsc.GetString(ExternalIdField.FieldName)) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Validate global-role-arn when provided.
Right now an invalid global-role-arn can pass validation and only fail later during STS calls. Consider validating it alongside role-arn before external-id checks.
🔧 Proposed fix
err := connector.IsValidRoleARN(awsc.GetString(RoleArnField.FieldName))
if err != nil {
return err
}
// Only validate external-id for two-hop mode (when global-role-arn is set)
// Single-hop mode (IRSA → target role) doesn't require external-id
globalRoleArn := awsc.GetString(GlobalRoleArnField.FieldName)
if globalRoleArn != "" {
- err = ValidateExternalId(awsc.GetString(ExternalIdField.FieldName))
- if err != nil {
- return err
- }
+ if err := connector.IsValidRoleARN(globalRoleArn); err != nil {
+ return err
+ }
+ if err := ValidateExternalId(awsc.GetString(ExternalIdField.FieldName)); err != nil {
+ return err
+ }
}🤖 Prompt for AI Agents
In `@pkg/config/config.go` around lines 118 - 129, The code currently validates
RoleArn via connector.IsValidRoleARN but doesn't validate GlobalRoleArn; update
the validation flow in the same block to call connector.IsValidRoleARN on
awsc.GetString(GlobalRoleArnField.FieldName) (similar to RoleArnField) before
running ValidateExternalId, and return the error if that call fails; keep
ValidateExternalId(awsc.GetString(ExternalIdField.FieldName)) only for the
two-hop case when globalRoleArn != "".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.