Skip to content

Authenticate container image pull for private image_ref - #36

Closed
shaur-k wants to merge 2 commits into
mainfrom
claude/integration-test-private-image-credentials
Closed

Authenticate container image pull for private image_ref#36
shaur-k wants to merge 2 commits into
mainfrom
claude/integration-test-private-image-credentials

Conversation

@shaur-k

@shaur-k shaur-k commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Motivation

A consuming repo's integration run (e.g. moveit_pro_example_ws PR stacked on a moveit_pro PR via the needs: token) failed at Initialize containers with 403 when image_ref pointed at a private image. The container: block set image: to the (possibly private) image_ref but carried no credentials, so the runner pulled anonymously — which 403s on a private registry. The image_ref docstring already advertised private-image support the workflow couldn't actually authenticate.

Brief description

Add optional image_registry_username / image_registry_password secrets and a container.credentials block fed by them. When the caller omits them (every current caller, pulling the public Docker Hub default), the username is empty and the runner skips docker login — anonymous pull, unchanged behavior. When supplied (private image_ref, e.g. a GitHub App token with read:packages), the pull authenticates.

How it was tested

  • python3 -c "yaml.safe_load(...)" — YAML valid.
  • actionlint 1.7.12 on the workflow — 0 findings.
  • Behavior preservation reasoned from the runner's conditional-login: empty username ⇒ no docker login ⇒ anonymous pull (existing public-image path).

Companion change required (other repo)

This unblocks authentication but is not sufficient alone. The caller must (a) pass these two secrets, and (b) point image_ref at the registry where the per-PR image actually exists. moveit_pro_example_ws/.github/workflows/ci.yaml currently builds a GHCR image_ref while moveit_pro publishes the customer image to Docker Hub — that ref must be reconciled (and the matching credentials passed) for the stacked-PR integration run to go green.

The reusable workflow's container.image can be a private image_ref (e.g. a
PR-specific image dispatched from a sister repo), but the container block had
no credentials, so the pull was always anonymous and 403'd on a private
registry. Add optional image_registry_username / image_registry_password
secrets and a container credentials block. When unset the runner skips docker
login and pulls anonymously, so the default public Docker Hub image and every
existing caller are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The workspace_integration_test workflow adds optional image_registry_username and image_registry_password secrets, a validate-registry-credentials job that enforces providing both secrets as a pair, and wires those secrets to the integration job's container credentials to enable authenticated private image pulls.

Changes

Private Container Registry Authentication

Layer / File(s) Summary
Registry secrets contract
.github/workflows/workspace_integration_test.yaml
Adds two optional workflow_call secrets: image_registry_username and image_registry_password with descriptions for private registry authentication.
Validate registry credentials job
.github/workflows/workspace_integration_test.yaml
Introduces validate-registry-credentials job that fails fast when only one of the two secrets is set.
Container credentials configuration
.github/workflows/workspace_integration_test.yaml
Wires the integration job's container.credentials.username and container.credentials.password to the new secrets so the runner can authenticate private image pulls.

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Human Review Check ❌ Error PR adds secrets for authentication (image_registry_username/password) and modifies CI/CD workflow configuration, which fall under the check's FAIL criteria. This PR requires review by a requested human reviewer. After review, a non-author requested reviewer should override this pre-merge check.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly explains the motivation (private image pull failures), the solution (adding registry credentials), and how it was tested. The description is directly related to the workflow changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@shaur-k
shaur-k marked this pull request as ready for review June 9, 2026 22:59
@shaur-k shaur-k self-assigned this Jun 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/workspace_integration_test.yaml:
- Around line 122-137: Add a no-container preflight job that validates the pair
image_registry_username and image_registry_password are either both set or both
unset, and fail fast with a clear error message if only one is provided; name it
something like validate-registry-credentials and have
integration-test-in-studio-container depend on it so the runner will not attempt
image pulls with half-configured credentials. Specifically, implement the check
as a steps-only job (runs-on: ubuntu-latest, no container) that reads the
workflow inputs image_registry_username and image_registry_password and exits
non‑zero with a descriptive message when exactly one is present, then add a
needs: validate-registry-credentials to the integration-test-in-studio-container
job so integration-test-in-studio-container.container.credentials is only used
after the preflight passes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 34e0b1ed-be37-496e-86de-e2e7b70143de

📥 Commits

Reviewing files that changed from the base of the PR and between b119d40 and 3db02fd.

📒 Files selected for processing (1)
  • .github/workflows/workspace_integration_test.yaml

Comment thread .github/workflows/workspace_integration_test.yaml
Add a steps-only validate-registry-credentials preflight that fails fast when
exactly one of image_registry_username / image_registry_password is provided,
and gate integration-test-in-studio-container on it. Prevents a half-configured
pair from silently falling back to an anonymous pull and 403-ing in container
init. Addresses the review on PR #36.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shaur-k
shaur-k requested a review from JWhitleyWork June 10, 2026 13:25
@shaur-k shaur-k closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant