fix(ci): remove explicit AWS credential passing from docker builds#8310
fix(ci): remove explicit AWS credential passing from docker builds#8310sara4dev wants to merge 3 commits into
Conversation
BuildKit pods on aws-ci already have IAM access via IRSA (builder-v1-service-account), so sccache discovers credentials automatically through the AWS SDK credential chain. Passing AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY as step-level env vars combined with set -x caused credentials to leak into build logs via bash trace expansion. Remove --mount=type=secret for AWS creds from all Dockerfile RUN steps, remove aws_access_key_id/aws_secret_access_key inputs from actions, and remove secret forwarding from all calling workflows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sara4dev
force-pushed
the
fix/remove-aws-secret-from-docker-build
branch
from
27a9f0d to
4d400ee
Compare
WalkthroughThis change systematically removes AWS credential inputs ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
container/templates/wheel_builder.Dockerfile (1)
570-599: LGTM — kvbm wheel build step.Cache mounts retained, secret mounts dropped; auditwheel repair path unchanged.
One cross-file follow-up worth noting:
container/use-sccache.sh(lines 97–99) still documents that S3 credentials are expected to be mounted via--mount=type=secret. Now that all Dockerfile RUN steps rely on IRSA/AWS SDK credential chain instead, consider updating that comment in a follow-up so the script's documentation matches the new CI model and future maintainers aren't sent looking for secret mounts that no longer exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@container/templates/wheel_builder.Dockerfile` around lines 570 - 599, The comment in the use-sccache.sh script (use-sccache.sh around the documented block at lines that mention S3 credential mounting) still instructs users to provide S3 credentials via --mount=type=secret; update that comment to reflect the new CI model by removing the secret-mount guidance and instead state that the script relies on the AWS SDK/IRSA credential chain (environment, instance role, or IRSA) for S3 access, and optionally note any fallback behavior or env var names the script checks so future maintainers know where to look.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@container/templates/wheel_builder.Dockerfile`:
- Around line 570-599: The comment in the use-sccache.sh script (use-sccache.sh
around the documented block at lines that mention S3 credential mounting) still
instructs users to provide S3 credentials via --mount=type=secret; update that
comment to reflect the new CI model by removing the secret-mount guidance and
instead state that the script relies on the AWS SDK/IRSA credential chain
(environment, instance role, or IRSA) for S3 access, and optionally note any
fallback behavior or env var names the script checks so future maintainers know
where to look.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4bf4ba12-82c0-4379-a7f8-9a1e5dd96632
📒 Files selected for processing (10)
.github/actions/build-flavor/action.yml.github/actions/docker-build/action.yml.github/actions/docker-remote-build/action.yml.github/workflows/build-flavor.yml.github/workflows/build-frontend-image.yaml.github/workflows/build-test-distribute-flavor-matrix.yml.github/workflows/build-test-distribute-flavor.yml.github/workflows/container-validation-dynamo.yml.github/workflows/shared-build-image.ymlcontainer/templates/wheel_builder.Dockerfile
💤 Files with no reviewable changes (8)
- .github/workflows/shared-build-image.yml
- .github/workflows/build-flavor.yml
- .github/workflows/build-test-distribute-flavor.yml
- .github/workflows/build-frontend-image.yaml
- .github/workflows/container-validation-dynamo.yml
- .github/workflows/build-test-distribute-flavor-matrix.yml
- .github/actions/docker-build/action.yml
- .github/actions/build-flavor/action.yml
Replace static AWS credential passing with dynamic temporary credentials derived from the runner pod's IRSA role. Credentials are resolved via `aws configure export-credentials`, written to temp files (never as env vars), and passed to BuildKit via file-based --secret flags. Changes: - Dockerfile: restore --mount=type=secret on all sccache RUN steps, add aws-session-token for STS temp creds, rename aws-secret-id to aws-secret-key for clarity - docker-remote-build: derive temp creds from IRSA with set +x guard, write to temp files, use --secret id=...,src=<file> - No static AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY in env vars or GitHub secrets needed for docker builds Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # Clean up credential files | ||
| if [ -n "${SECRET_DIR:-}" ]; then rm -rf "$SECRET_DIR"; fi | ||
|
|
||
| BUILD_EXIT_CODE=${PIPESTATUS[0]} |
There was a problem hiding this comment.
🔴 PIPESTATUS overwritten by credential cleanup command, silently masking build failures
The credential cleanup command if [ -n "${SECRET_DIR:-}" ]; then rm -rf "$SECRET_DIR"; fi (line 187) is inserted between the docker buildx build ... | tee pipeline (line 184) and BUILD_EXIT_CODE=${PIPESTATUS[0]} (line 189). In bash, PIPESTATUS is reset after every command, so by the time line 189 executes, PIPESTATUS[0] reflects the exit status of the if/rm compound command (almost always 0), not the docker build. This means if the docker build fails, BUILD_EXIT_CODE will still be 0, exit ${BUILD_EXIT_CODE} will succeed, and the CI step will incorrectly report success.
The old code (before this PR) correctly placed BUILD_EXIT_CODE=${PIPESTATUS[0]} immediately after the pipeline with no intervening commands.
| # Clean up credential files | |
| if [ -n "${SECRET_DIR:-}" ]; then rm -rf "$SECRET_DIR"; fi | |
| BUILD_EXIT_CODE=${PIPESTATUS[0]} | |
| BUILD_EXIT_CODE=${PIPESTATUS[0]} | |
| # Clean up credential files | |
| if [ -n "${SECRET_DIR:-}" ]; then rm -rf "$SECRET_DIR"; fi |
Was this helpful? React with 👍 or 👎 to provide feedback.
aws configure export-credentials failed silently on runners, causing empty credentials to be passed to sccache. Switch to explicit aws sts assume-role-with-web-identity using the IRSA-injected AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN env vars. Also strip trailing newlines from credential files with end=''. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
closing in favor of #8324 |
2 participants
Summary
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEYpassing through CI actions and Dockerfile secret mountsbuilder-v1-service-account), so sccache discovers credentials automatically through the AWS SDK credential chainset -xin the build script, caused credentials to leak into build logs via bash trace expansionChanges
container/templates/wheel_builder.Dockerfile: Removed all 8--mount=type=secret,id=aws-key-id/aws-secret-idfrom RUN steps.github/actions/docker-remote-build/action.yml: Removedaws_access_key_id/aws_secret_access_keyinputs, env vars, andSECRET_ARGSblock.github/actions/build-flavor/action.yml: Removed credential inputs and forwarding.github/actions/docker-build/action.yml: Removed credential inputs and env varsTest plan
AKIA*patterns or AWS secret key valuesbuilder-v1-service-accountIRSA annotation exists on aws-ci cluster🤖 Generated with Claude Code
Summary by CodeRabbit