ci: Fix Dockerfile mount secrets

[dillon-cullinan]

Overview:

Changes the secrets in dockerfiles to be secret mounts. This is the recommended way of handling secrets in docker build contexts.

Summary by CodeRabbit

• Chores
  • Migrated Docker/image builds to use BuildKit secrets for AWS credentials.
  • Updated build scripts and steps to consistently use secret mounts during dependency installation.
• Security
  • Removed credential build-args; credentials are now injected securely at build time via secrets, reducing exposure.
• Reliability
  • Standardized credential handling improves stability of dependency builds (e.g., model/runtime components).

[coderabbitai]

Walkthrough

Replaces Docker build-arg–based AWS credentials with BuildKit secrets across Dockerfiles and the build script. Adds secret mounts to relevant RUN steps, updates build.sh to pass secrets instead of build args, and leaves other build logic unchanged.

Changes

Cohort / File(s)	Summary
Dockerfiles: BuildKit secrets for AWS creds container/Dockerfile, container/Dockerfile.vllm	Removed --build-arg usage for AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY. Added --mount=type=secret,id=aws-key-id,env=AWS_ACCESS_KEY_ID and --mount=type=secret,id=aws-key-id,env=AWS_SECRET_ACCESS_KEY to RUN steps (UCX cleanup, NIXL clone, wheel builds, vLLM deps).
Build script: pass secrets to build container/build.sh	Replaced --build-arg AWS_ACCESS_KEY_ID=... and --build-arg AWS_SECRET_ACCESS_KEY=... with --secret id=aws-key-id,env=AWS_ACCESS_KEY_ID and --secret id=aws-key-id,env=AWS_SECRET_ACCESS_KEY within the USE_SCCACHE path. No other flow changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer
    participant Build as build.sh
    participant Docker as Docker/BuildKit
    participant DF as Dockerfile(.vllm)
    participant Steps as RUN Steps

    Dev->>Build: Invoke container/build.sh
    Build->>Docker: docker build ... --secret id=aws-key-id,env=AWS_ACCESS_KEY_ID/SECRET
    Docker->>DF: Parse Dockerfile stages
    DF->>Steps: RUN commands with secret mounts
    note over Steps: Secrets exposed as env vars only during RUN
    Steps-->>Docker: Execute git/uv install requiring AWS creds
    Docker-->>Dev: Image built

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor: replace vllm with vllm_v1 container #1953 — Touches container/Dockerfile.vllm and container/build.sh with build configuration changes; overlaps in file scope.
• ci: OPS-724: Move to ARC runners #2904 — Adjusts AWS credential provisioning during Docker builds; similar shift in how credentials are supplied.

Poem

In burrows where the build winds flow,
I stash my keys where secrets go. 🔐
No args to spill, no creds to stray,
Just tucked in mounts along the way.
Hop, compile, and wheels now spin—
A silent waltz, securely in. 🐇✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description only contains an Overview section and omits mandatory headings defined in the repository’s template, including Details, Where should the reviewer start?, and Related Issues. These sections are required to describe the specific code changes, direct reviewers to key files, and link to any related GitHub issues. Without them, the scope and context of the changes are unclear and issue tracking is incomplete. Consequently, the current description does not satisfy the repository’s PR template requirements.	Please update the pull request description to include a Details section that outlines the specific modifications, a Where should the reviewer start? section identifying the primary files or sections to examine, and a Related Issues section linking to any relevant GitHub issues with the appropriate action keywords (Closes, Fixes, etc.). Completing these sections will ensure adherence to the PR template and provide reviewers with the necessary context and traceability.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.
Title Check	✅ Passed	The title succinctly and accurately highlights the primary change of migrating Dockerfile credentials to BuildKit secret mounts without extraneous details, making it clear to reviewers what the PR addresses.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (4)

container/Dockerfile.vllm (1)

21-25: Cleanup: remove unused AWS build-args to avoid confusion and accidental leakage paths

If we’re standardizing on BuildKit secrets, keep credentials out of ARGs entirely.

 ARG USE_SCCACHE
 ARG SCCACHE_BUCKET=""
 ARG SCCACHE_REGION=""
-ARG AWS_ACCESS_KEY_ID=""
-ARG AWS_SECRET_ACCESS_KEY=""

-ARG AWS_ACCESS_KEY_ID=""
-ARG AWS_SECRET_ACCESS_KEY=""

Also applies to: 124-126

container/build.sh (1)

370-379: Optional guard: validate AWS env vars when --use-sccache is set

Fast-fail with a clear error if required env vars are missing; avoids mysterious S3 auth failures during build.

     if [ "$USE_SCCACHE" = true ]; then
         if [ -z "$SCCACHE_BUCKET" ]; then
             error "ERROR: --sccache-bucket is required when --use-sccache is specified"
         fi
         if [ -z "$SCCACHE_REGION" ]; then
             error "ERROR: --sccache-region is required when --use-sccache is specified"
         fi
+        : "${AWS_ACCESS_KEY_ID:?ERROR: AWS_ACCESS_KEY_ID must be set in the environment when --use-sccache is specified}"
+        : "${AWS_SECRET_ACCESS_KEY:?ERROR: AWS_SECRET_ACCESS_KEY must be set in the environment when --use-sccache is specified}"
+        # Optional if using STS:
+        if [ -n "${AWS_SESSION_TOKEN}" ]; then
+            echo "Info: Using AWS_SESSION_TOKEN for STS credentials"
+        fi
     fi

container/Dockerfile (2)

59-61: Cleanup: drop leftover AWS ARGs now that secrets are used

Avoid dangling credential plumbing via build args.

-ARG AWS_ACCESS_KEY_ID
-ARG AWS_SECRET_ACCESS_KEY

-ARG AWS_ACCESS_KEY_ID
-ARG AWS_SECRET_ACCESS_KEY

Also applies to: 277-279

---

240-241: Nit: typo in comment (“devr image”)

-# Copy NIXL source from devr image
+# Copy NIXL source from dev image

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09c7b73 and c42bae0.

📒 Files selected for processing (3)

• container/Dockerfile (3 hunks)
• container/Dockerfile.vllm (1 hunks)
• container/build.sh (1 hunks)

⏰ 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). (2)

• GitHub Check: Build and Test - vllm
• GitHub Check: Build and Test - dynamo