Skip to content
Merged
39 changes: 37 additions & 2 deletions .github/workflows/cpflow-promote-staging-to-production.yml
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,10 @@ jobs:
echo "image=${staging_image}" >> "$GITHUB_OUTPUT"

- name: Copy image from staging
id: copy-image
env:
# Pass the upstream token via env rather than `-t` so it doesn't appear in /proc/<pid>/cmdline.
CPLN_TOKEN_STAGING: ${{ secrets.CPLN_TOKEN_STAGING }}
CPLN_UPSTREAM_TOKEN: ${{ secrets.CPLN_TOKEN_STAGING }}
PRODUCTION_APP_NAME: ${{ vars.PRODUCTION_APP_NAME }}
CPLN_ORG_STAGING: ${{ vars.CPLN_ORG_STAGING }}
CPLN_ORG_PRODUCTION: ${{ vars.CPLN_ORG_PRODUCTION }}
Expand Down Expand Up @@ -348,9 +348,38 @@ jobs:
exit 1
fi

staging_commit="${STAGING_IMAGE##*_}"
if [[ "${staging_commit}" == "${STAGING_IMAGE}" || -z "${staging_commit}" ]]; then
echo "::error::Staging image '${STAGING_IMAGE}' does not include the expected '_<commit>' suffix."
exit 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow promoting images without commit suffixes

When the staging workload points at a valid cpflow image that was built without --commit (for example my-app:7), this new guard aborts the entire production promotion even though cpflow still supports such tags: latest_image_next only appends _<commit> when a commit is present (lib/core/controlplane.rb:53-62), and the previous copy-image-from-upstream path passed a nil commit through to create the next plain production tag (lib/command/copy_image_from_upstream.rb:80-90). This regresses promotions for existing/manual staging images that do not have the optional commit suffix; generate the production name without a suffix in that case instead of exiting.

Useful? React with 👍 / 👎.

fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Digest pin drops commit suffix

Medium Severity

When the staging image reference includes a digest (@sha256:…), commit suffix extraction uses the digest segment instead of the tag before @, so _&lt;commit&gt; is not copied onto the new production tag despite documented behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6cd8225. Configure here.

Comment on lines +466 to +475

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: commit suffix is never extracted for digest-pinned images.

When staging_image is myapp:123_abc@sha256:deadbeef, the *@* branch sets staging_tag = "sha256:deadbeef" (the digest). Because a digest hash never contains _, staging_commit stays empty and the _abc suffix is silently dropped from the production tag — contradicting the documented behaviour.

Suggested change
staging_tag=""
if [[ "${staging_image}" == *@* ]]; then
staging_tag="${staging_image##*@}"
elif [[ "${staging_image}" == *:* ]]; then
staging_tag="${staging_image##*:}"
fi
staging_commit=""
if [[ "${staging_tag}" == *_* ]]; then
staging_commit="${staging_tag##*_}"
fi
staging_tag=""
if [[ "${staging_image}" == *:*@* ]]; then
staging_tag="${staging_image##*:}" # e.g. 123_abc@sha256:...
staging_tag="${staging_tag%%@*}" # strip digest -> 123_abc
elif [[ "${staging_image}" == *@* ]]; then
staging_tag="" # digest-only ref, no tag to extract
elif [[ "${staging_image}" == *:* ]]; then
staging_tag="${staging_image##*:}"
fi
staging_commit=""
if [[ "${staging_tag}" == *_* ]]; then
staging_commit="${staging_tag##*_}"
fi


latest_number="$(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a TOCTOU window between this query and the docker push below: if two promotion runs somehow bypassed the concurrency group (or the group was misconfigured), both would read the same latest_number and push to the same production tag, silently overwriting each other. The concurrency group closes this in practice, but the comment explaining the dependency is currently on the Deploy image step. Worth adding a note here so the invariant is visible at the point where it actually matters:

Suggested change
latest_number="$(
# The workflow-level concurrency group serializes access here: without it,
# two concurrent runs could derive the same latest_number and overwrite each other.
latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"

cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"
production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Race condition on sequential promotions: latest_number is computed from a live API query, so two promotions running close together (e.g. a re-run while the first is still copying) can both read the same max number and produce the same production_image tag. The second cpln image copy would overwrite the first silently.

If the Control Plane API supports it, passing --to-name without the numeric prefix and letting the registry assign it, or using a commit-hash-only tag like ${PRODUCTION_APP_NAME}:${staging_commit}, would be race-free. If the incrementing number is intentional for ordering, consider adding a uniqueness guard (e.g. abort if the computed tag already exists in the production registry before copying).

Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent-failure risk in latest_number subshell assignment

In bash, var="$(cmd)" always exits 0 — even with set -o pipefail — so if cpln image query fails (network hiccup, API error), the pipeline produces no output, jq exits 0 on empty stdin, and latest_number is silently set to "". Bash then evaluates $(("" + 1)) as 1, meaning production_image would be tagged :1_<commit> and potentially overwrite an existing image.

Suggested fix: validate the result before using it in arithmetic:

Suggested change
latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"
production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"
latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"
if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then
echo "::error::Failed to determine the latest production image number for '${PRODUCTION_APP_NAME}'; cpln image query may have failed."
exit 1
fi
production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docker pull + docker tag + docker push approach downloads every layer to the runner disk before re-uploading to the production registry. For a large production image this can exhaust the ~14 GB of disk on a GitHub-hosted ubuntu-latest runner and adds significant latency.

A registry-to-registry copy tool such as crane copy or skopeo copy --multi-arch all streams directly between registries without touching the runner disk, preserves multi-arch manifests, and is substantially faster. Worth considering if image sizes become a concern.

source_image_ref="${CPLN_ORG_STAGING}.registry.cpln.io/${STAGING_IMAGE}"

docker_config_dir="$(mktemp -d)"
cleanup_copy_credentials() {
rm -rf "${docker_config_dir}"
}
trap cleanup_copy_credentials EXIT

export DOCKER_CONFIG="${docker_config_dir}"

copy_status=1
for attempt in $(seq 1 "${copy_image_attempts}"); do
if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then
if CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null &&
docker manifest inspect "${source_image_ref}" >/dev/null &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

docker manifest inspect verifies the image exists before pulling, which avoids downloading layers for a non-existent image. This is fine as a fast pre-flight check. One subtle issue: if the manifest call succeeds but the image is deleted between this line and docker pull (very unlikely), the retry loop handles it. No action needed, just documenting the intent.

CPLN_TOKEN="${CPLN_TOKEN_STAGING}" \
cpln image copy "${STAGING_IMAGE}" \
--org "${CPLN_ORG_STAGING}" \
--to-profile default \
--to-org "${CPLN_ORG_PRODUCTION}" \
--to-name "${production_image}"; then
copy_status=0
break
else
Expand All @@ -370,6 +399,8 @@ jobs:
exit "${copy_status}"
fi

echo "image=${production_image}" >> "$GITHUB_OUTPUT"

- name: Deploy image to production
env:
PRODUCTION_APP_NAME: ${{ vars.PRODUCTION_APP_NAME }}
Expand Down Expand Up @@ -531,19 +562,23 @@ jobs:
HEALTHY: ${{ steps.health-check.outputs.healthy }}
PREVIOUS_IMAGE: ${{ steps.capture-current.outputs.current_image }}
PREVIOUS_VERSION: ${{ steps.capture-current.outputs.current_version }}
COPIED_IMAGE: ${{ steps.copy-image.outputs.image }}
shell: bash
run: |
{
echo "## Promotion Summary"
echo
if [[ "${HEALTHY}" == "true" ]]; then
echo "✅ Status: deployment successful"
deployed_image="${COPIED_IMAGE}"
else
echo "❌ Status: deployment failed"
deployed_image="${PREVIOUS_IMAGE}"
fi
echo
echo "Previous image: \`${PREVIOUS_IMAGE}\`"
echo "Previous version: ${PREVIOUS_VERSION}"
echo "Deployed image: \`${deployed_image}\`"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary mislabels failed promotion image

Medium Severity

On a failed health check, the promotion summary always prints Deployed image as the pre-promotion reference, even when rollback did not run or failed and production may still be running the newly copied image.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6cd8225. Configure here.

} >> "$GITHUB_STEP_SUMMARY"

create-github-release:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ jobs:
echo "image=${staging_image}" >> "$GITHUB_OUTPUT"

- name: Copy image from staging
id: copy-image
env:
# Pass the upstream token via env rather than `-t` so it doesn't appear in /proc/<pid>/cmdline.
CPLN_TOKEN_STAGING: ${{ secrets.CPLN_TOKEN_STAGING }}
CPLN_UPSTREAM_TOKEN: ${{ secrets.CPLN_TOKEN_STAGING }}
PRODUCTION_APP_NAME: ${{ vars.PRODUCTION_APP_NAME }}
CPLN_ORG_STAGING: ${{ vars.CPLN_ORG_STAGING }}
CPLN_ORG_PRODUCTION: ${{ vars.CPLN_ORG_PRODUCTION }}
Expand Down Expand Up @@ -367,9 +367,38 @@ jobs:
exit 1
fi

staging_commit="${STAGING_IMAGE##*_}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Strip digests before deriving promoted tag

When the staging workload was deployed with use_digest_image_ref, cpflow deploy-image writes a tagged digest reference (see lib/command/deploy_image.rb:72-75), so this workflow captures a STAGING_IMAGE like app:7_deadbeef@sha256:<digest>. This extraction keeps the digest in staging_commit, and line 381 then builds a --to-name such as prod:8_deadbeef@sha256:<digest>, which is not a valid destination image name/tag for cpln image copy; promotions from digest-pinned staging workloads will fail before deployment. Strip any @... digest from STAGING_IMAGE before extracting the commit and constructing production_image.

Useful? React with 👍 / 👎.

if [[ "${staging_commit}" == "${STAGING_IMAGE}" || -z "${staging_commit}" ]]; then
echo "::error::Staging image '${STAGING_IMAGE}' does not include the expected '_<commit>' suffix."
exit 1
fi
Comment on lines +485 to +494

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: same commit suffix extraction issue as in the reusable workflow — for myapp:123_abc@sha256:deadbeef, staging_tag receives the digest (sha256:deadbeef), not the tag, so staging_commit is always empty for digest-pinned images.

Suggested change
staging_tag=""
if [[ "${staging_image}" == *@* ]]; then
staging_tag="${staging_image##*@}"
elif [[ "${staging_image}" == *:* ]]; then
staging_tag="${staging_image##*:}"
fi
staging_commit=""
if [[ "${staging_tag}" == *_* ]]; then
staging_commit="${staging_tag##*_}"
fi
staging_tag=""
if [[ "${staging_image}" == *:*@* ]]; then
staging_tag="${staging_image##*:}" # e.g. 123_abc@sha256:...
staging_tag="${staging_tag%%@*}" # strip digest -> 123_abc
elif [[ "${staging_image}" == *@* ]]; then
staging_tag="" # digest-only ref, no tag to extract
elif [[ "${staging_image}" == *:* ]]; then
staging_tag="${staging_image##*:}"
fi
staging_commit=""
if [[ "${staging_tag}" == *_* ]]; then
staging_commit="${staging_tag##*_}"
fi


latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include all production images when choosing the next tag

In repos that have accumulated more than 50 production images, this query only considers the CLI's first page before computing latest_number; the Control Plane CLI docs for cpln image query list --max as the maximum records with default 50. If the highest existing PRODUCTION_APP_NAME:N tag is outside that page, promotion can reuse an existing number and the subsequent Docker push can overwrite/collide with an older production image. Add --max 0 or otherwise page through all results before taking the max.

Useful? React with 👍 / 👎.

production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same silent-failure risk as the companion comment in the reusable workflow: latest_number="$(subshell)" masks pipeline failures in bash, so a cpln image query error silently produces production_image="…:1_<commit>".

Apply the same guard here:

Suggested change
latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"
production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"
latest_number="$(
cpln image query --org "${CPLN_ORG_PRODUCTION}" --prop "name~${PRODUCTION_APP_NAME}:" -o json |
jq -r --arg prefix "${PRODUCTION_APP_NAME}:" \
'[.items[].name | select(startswith($prefix)) | (try capture("^[^:]+:(?<number>[0-9]+)") catch empty) | .number | tonumber] | max // 0'
)"
if ! [[ "${latest_number}" =~ ^[0-9]+$ ]]; then
echo "::error::Failed to determine the latest production image number for '${PRODUCTION_APP_NAME}'; cpln image query may have failed."
exit 1
fi
production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"

source_image_ref="${CPLN_ORG_STAGING}.registry.cpln.io/${STAGING_IMAGE}"

docker_config_dir="$(mktemp -d)"
cleanup_copy_credentials() {
rm -rf "${docker_config_dir}"
}
trap cleanup_copy_credentials EXIT

export DOCKER_CONFIG="${docker_config_dir}"

copy_status=1
for attempt in $(seq 1 "${copy_image_attempts}"); do
if cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" --org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"; then
if CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login --org "${CPLN_ORG_STAGING}" >/dev/null &&
docker manifest inspect "${source_image_ref}" >/dev/null &&
CPLN_TOKEN="${CPLN_TOKEN_STAGING}" \
cpln image copy "${STAGING_IMAGE}" \
--org "${CPLN_ORG_STAGING}" \
--to-profile default \
--to-org "${CPLN_ORG_PRODUCTION}" \
--to-name "${production_image}"; then
copy_status=0
break
else
Expand All @@ -389,6 +418,8 @@ jobs:
exit "${copy_status}"
fi

echo "image=${production_image}" >> "$GITHUB_OUTPUT"

- name: Deploy image to production
env:
PRODUCTION_APP_NAME: ${{ vars.PRODUCTION_APP_NAME }}
Expand Down Expand Up @@ -553,21 +584,23 @@ jobs:
HEALTHY: ${{ steps.health-check.outputs.healthy }}
PREVIOUS_IMAGE: ${{ steps.capture-current.outputs.current_image }}
PREVIOUS_VERSION: ${{ steps.capture-current.outputs.current_version }}
DEPLOYED_IMAGE: ${{ steps.staging-image.outputs.image }}
COPIED_IMAGE: ${{ steps.copy-image.outputs.image }}
shell: bash
run: |
{
echo "## Promotion Summary"
echo
if [[ "${HEALTHY}" == "true" ]]; then
echo "✅ Status: deployment successful"
deployed_image="${COPIED_IMAGE}"
else
echo "❌ Status: deployment failed"
deployed_image="${PREVIOUS_IMAGE}"
fi
echo
echo "Previous image: \`${PREVIOUS_IMAGE}\`"
echo "Previous version: ${PREVIOUS_VERSION}"
echo "Deployed image: \`${DEPLOYED_IMAGE}\`"
echo "Deployed image: \`${deployed_image}\`"
} >> "$GITHUB_STEP_SUMMARY"

create-github-release:
Expand Down
23 changes: 21 additions & 2 deletions spec/command/generate_github_actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -826,13 +826,32 @@ def shared_workflow_path(name)
expect(contents).to include("COPY_IMAGE_RETRY_INTERVAL must be a non-negative integer")
expect(contents).to include("copy_image_attempts=$((copy_image_retries + 1))")
expect(contents).to include("Staging image '${STAGING_IMAGE}' was not found")
expect(contents).to include("id: copy-image")
expect(contents).to include('staging_commit="${STAGING_IMAGE##*_}"')
expect(contents).to include('production_image="${PRODUCTION_APP_NAME}:$((latest_number + 1))_${staging_commit}"')
expect(contents).to include('docker_config_dir="$(mktemp -d)"')
expect(contents).to include("cleanup_copy_credentials")
expect(contents).to include('export DOCKER_CONFIG="${docker_config_dir}"')
expect(contents).to include('CPLN_TOKEN="${CPLN_TOKEN_STAGING}" cpln image docker-login')
expect(contents).to include('docker manifest inspect "${source_image_ref}"')
expect(contents).to include('for attempt in $(seq 1 "${copy_image_attempts}"); do')
expect(contents).to include('CPLN_TOKEN="${CPLN_TOKEN_STAGING}" \\')
expect(contents).to include('cpln image copy "${STAGING_IMAGE}"')
expect(contents).to include("--to-profile default")
expect(contents).to include("--to-name \"${production_image}\"")
expect(contents).to include('echo "image=${production_image}" >> "$GITHUB_OUTPUT"')
expect(contents).to include("COPIED_IMAGE: ${{ steps.copy-image.outputs.image }}")
expect(contents).to include('deployed_image="${COPIED_IMAGE}"')
expect(contents).to include('deployed_image="${PREVIOUS_IMAGE}"')
expect(contents).to include("Image copy attempt ${attempt}/${copy_image_attempts} failed")
expect(contents).to include("no attempts remain")
expect(contents).to include("workload_name: ${{ steps.workloads.outputs.primary }}")
expect(contents).not_to include("workload_name: ${{ env.PRIMARY_WORKLOAD || 'rails' }}")
expect(contents).to include('cpflow copy-image-from-upstream -a "${PRODUCTION_APP_NAME}" ' \
'--org "${CPLN_ORG_PRODUCTION}" --image "${STAGING_IMAGE}"')
expect(contents).not_to include("CPLN_UPSTREAM_TOKEN:")
expect(contents).not_to include('cpln profile create "${upstream_profile}"')
expect(contents).not_to include("--profile \"${upstream_profile}\"")
expect(contents).not_to include("cpflow copy-image-from-upstream")
expect(contents).not_to include("--cleanup")
end

it "detects release phase support from controlplane.yml instead of cpflow config text" do
Expand Down
Loading