Use staging token directly for production image copy source#757
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe deployment workflow's image staging-to-production copy step is refactored to use temporary Docker configuration directory management with cleanup, authenticate via ChangesImage Copy Authentication and Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| copy_status=1 | ||
| for attempt in $(seq 1 "${copy_image_attempts}"); do | ||
| if CPLN_PROFILE="${upstream_profile}" docker manifest inspect "${source_image_ref}" >/dev/null && | ||
| if CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null && |
There was a problem hiding this comment.
Re-running docker-login on every retry is safe (and refreshes credentials for long retry sequences), but it means copy_status=$? can capture a docker-login exit code rather than a copy exit code when login itself fails. The warning message will still say "Image copy attempt … failed" even if the real problem was authentication. Consider logging which step failed for easier diagnosis:
| if CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null && | |
| if CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null 2>&1 \ | |
| || { echo "::warning::docker-login to staging failed on attempt ${attempt}"; false; }; then | |
| if docker manifest inspect "${source_image_ref}" >/dev/null && |
Or at minimum, keep the current structure but drop the >/dev/null on docker-login stderr so auth errors surface directly in the log.
| cleanup_upstream_profile | ||
| CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln profile create "${upstream_profile}" >/dev/null | ||
| CPLN_PROFILE="${upstream_profile}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null | ||
| export DOCKER_CONFIG="${docker_config_dir}" |
There was a problem hiding this comment.
DOCKER_CONFIG is exported for the whole step, so any Docker commands that run after the loop (e.g. in the same run: block if code is added later) will also use the temp dir — which will be cleaned up on EXIT. This is intentional and correct for isolation; just worth a comment so future editors don't wonder why the env var is set here instead of inline on each docker call.
Code ReviewOverviewThis PR correctly fixes the root cause of the staging→production promotion failure. The previous approach of calling What's Good
Concerns1. Observability:
2. Minor: 3. Cosmetic: no comment explaining why SecurityNo regressions. VerdictApprove with minor suggestions. The fix is correct and the approach is cleaner than the profile-based method it replaces. The main concern worth addressing before merge: remove the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a cross-org image copy failure introduced in #756 by dropping the intermediate
Confidence Score: 4/5Safe to merge; the change correctly routes staging credentials directly to the commands that need them and cleans up after itself. The fix is narrowly scoped: three commands that previously relied on a temporary named cpln profile now receive the staging token directly. The cleanup path (trap + rm -rf) is standard and correct. The one design assumption worth verifying is that cpln image copy honours --to-profile default for destination auth even when CPLN_TOKEN is set in the environment — if the env var wins globally, the destination push would use the staging token and likely fail. .github/workflows/cpflow-promote-staging-to-production.yml — the cpln image copy invocation with both CPLN_TOKEN env var and --to-profile default is the key assumption to watch in the next production run. Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant CPLN_S as cpln (staging token)
participant Docker as docker CLI
participant CPLN_P as cpln (production profile)
GH->>GH: mktemp -d → docker_config_dir, export DOCKER_CONFIG
loop up to copy_image_attempts
GH->>CPLN_S: "CPLN_TOKEN=STAGING cpln image docker-login --org staging"
CPLN_S-->>Docker: Write staging credentials to DOCKER_CONFIG
GH->>Docker: docker manifest inspect staging-registry/image
Docker-->>GH: manifest found
GH->>CPLN_S: "CPLN_TOKEN=STAGING cpln image copy --org staging --to-profile default --to-org production"
CPLN_S-->>CPLN_P: Pull from staging registry, push via production profile
CPLN_P-->>GH: "copy_status=0, break"
end
GH->>GH: trap EXIT → rm -rf docker_config_dir
Reviews (1): Last reviewed commit: "Use staging token directly for productio..." | Re-trigger Greptile |
✅ Review App DeletedReview app for PR #757 is deleted |
1 participant
Summary
cpln profile createpath from the production copy stepcpln image copywithCPLN_TOKEN_STAGINGscoped to those commands--to-profile defaultso the destination copy still uses the production profile created by setupWhy
The fresh post-#756 production run failed with:
/org/shakacode-open-source-examples-production/serviceaccount/github-actions-production is not granted [view] on /org/shakacode-open-source-examples-stagingThat shows
CPLN_TOKEN=... cpln profile createdid not store the staging token in the temporary profile. The copy source needs the staging token directly, while the destination should keep using the default production profile.Failed run: https://github.com/shakacode/react-webpack-rails-tutorial/actions/runs/26849170364
Verification
git diff --check -- .github/workflows/cpflow-promote-staging-to-production.ymlbin/conductor-exec bin/test-cpflow-github-flowNote
Medium Risk
Changes production promotion credentials and cross-org image copy; failure would block releases but does not alter runtime app code.
Overview
Fixes staging→production image promotion in cpflow-promote-staging-to-production when a temporary Control Plane profile did not actually hold the staging token (production runs failed with missing view on the staging org).
The Copy image from staging step drops
cpln profile create/--profileupstream and instead runs stagingdocker-login,docker manifest inspect, and the source side ofcpln image copywithCPLN_TOKEN_STAGINGon each command. Credentials go in an isolatedDOCKER_CONFIGtemp dir with cleanup on exit. Destination copy still uses--to-profile default(production profile from setup).Reviewed by Cursor Bugbot for commit 937cbfc. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit