Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAws.Authenticate now logs and proceeds when default credential testing fails; NonInteractiveGitHubLogin writes a web identity token file whenever Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/Client
participant GH as GitHub (OIDC)
participant Fabric as Fabric Auth Server
participant FS as Filesystem
participant AWS as AWS SDK
CLI->>GH: Request OIDC idToken (non-interactive)
GH-->>CLI: idToken
CLI->>Fabric: Exchange idToken for Fabric access token
Fabric-->>CLI: resp.AccessToken
CLI->>FS: If AWS_WEB_IDENTITY_TOKEN_FILE unset -> ensure dir (0700), write idToken file
CLI->>CLI: set AWS_WEB_IDENTITY_TOKEN_FILE & AWS_ROLE_SESSION_NAME env
CLI->>AWS: Attempt AWS auth using web identity token (or fallback)
AWS-->>CLI: Auth result (success or failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pkg/login/login.go`:
- Around line 95-97: The token is being requested with an empty audience which
yields GitHub's default aud and will be rejected by AWS; update the call that
fetches the OIDC token (the github.GetIdToken(...) invocation) to pass the
AWS-expected audience (e.g. "sts.amazonaws.com" or the configured audience)
instead of leaving it empty, and keep writing that token via
writeWebIdentityToken when AWS_WEB_IDENTITY_TOKEN_FILE is empty so
AssumeRoleWithWebIdentity receives a matching aud; ensure the audience string is
configurable or matches the value used in your AWS trust policy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6715f826-d4d9-42f8-8ceb-a3b926d0ae4f
📒 Files selected for processing (2)
src/pkg/clouds/aws/login.gosrc/pkg/login/login.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/login/login_test.go (1)
117-121:⚠️ Potential issue | 🔴 CriticalPipeline failure:
StateDirdirectory is not created before use.The test sets
client.StateDirto a path that includes adefangsubdirectory, but this directory is never created.t.TempDir()only creates the base temp directory, not the nested path. WhenNonInteractiveGitHubLogintries to write the web identity token file to this path, it fails withno such file or directory.🐛 Proposed fix to create the directory
// use a prevStateDir dir for the token file prevStateDir := client.StateDir client.StateDir = filepath.Join(t.TempDir(), "defang") + if err := os.MkdirAll(client.StateDir, 0755); err != nil { + t.Fatalf("failed to create StateDir: %v", err) + } originalTokenStore := client.TokenStore client.TokenStore = &tokenstore.LocalDirTokenStore{Dir: t.TempDir()}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pkg/login/login_test.go` around lines 117 - 121, The test fails because client.StateDir is set to a nested path (filepath.Join(t.TempDir(), "defang")) but the "defang" directory is never created before NonInteractiveGitHubLogin writes the token file; fix by creating the directory (use os.MkdirAll) at client.StateDir after setting it and before calling NonInteractiveGitHubLogin (or adjust to use the plain t.TempDir() without nesting); also ensure tokenstore.LocalDirTokenStore{Dir: t.TempDir()} points to an existing directory or create it similarly so TokenStore writes succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/pkg/login/login_test.go`:
- Around line 117-121: The test fails because client.StateDir is set to a nested
path (filepath.Join(t.TempDir(), "defang")) but the "defang" directory is never
created before NonInteractiveGitHubLogin writes the token file; fix by creating
the directory (use os.MkdirAll) at client.StateDir after setting it and before
calling NonInteractiveGitHubLogin (or adjust to use the plain t.TempDir()
without nesting); also ensure tokenstore.LocalDirTokenStore{Dir: t.TempDir()}
points to an existing directory or create it similarly so TokenStore writes
succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b958cab6-f9eb-438b-86ba-16e4440a8f26
📒 Files selected for processing (1)
src/pkg/login/login_test.go
edwardrf
force-pushed
the
edw/fix-aws-oidc
branch
from
7d2ae8b to
a1643bb
Compare
3 participants
Description
Write AWS_WEB_IDENTITY_TOKEN_FILE with github idtoken irrespective of if AWS_ROLE_ARN is present.
Linked Issues
fixes: #2831
Checklist
Summary by CodeRabbit
Bug Fixes
Tests