ci: add gate, preflight and verification to testing image release - #718
ci: add gate, preflight and verification to testing image release#718nvasiu wants to merge 2 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
f48e551 to
44d8ed0
Compare
nvasiu
changed the title
| if aws ecr-public describe-images \ | ||
| --region "${{ env.aws_region }}" \ | ||
| --repository-name "$repo_name" \ | ||
| --image-ids imageTag="v${VERSION}" >/dev/null 2>&1; then | ||
| echo "exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "exists=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ilnzykk7t6nru3bdky4dlegmro
Every describe-images failure—not just a missing tag—is converted to exists=false. Missing permissions, throttling, or service errors can therefore proceed to overwrite an existing mutable version tag or fail only after building. Treat only ImageNotFoundException as absent and fail preflight for all other errors.
| --repository-name "$repo_name" \ | ||
| --image-ids imageTag="latest" \ | ||
| --query 'imageDetails[0].imageDigest' --output text 2>/dev/null || true)" | ||
| if [[ "$latest_digest" != "$version_digest" ]]; then |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_fdnghwehki3bd75myykb7emh2e
OCI index digests include descriptor order. The version manifest is created x86_64 then arm64, while latest is created arm64 then x86_64, so their top-level digests differ despite containing the same images. This marks successful publishes as failed. Create both tags from one identically ordered manifest or compare their child platform digests.
| latest_digest="$(aws ecr-public describe-images \ | ||
| --region "${{ env.aws_region }}" \ | ||
| --repository-name "$repo_name" \ | ||
| --image-ids imageTag="latest" \ | ||
| --query 'imageDetails[0].imageDigest' --output text 2>/dev/null || true)" |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_vbr4zrftdaxr75tg7x2l4btewl
The version tag is retried, but latest, which is pushed afterward, is queried only once. ECR can expose the version before the latest update, causing intermittent failures with an old or empty digest. Poll both tags within the same deadline before failing.
| _PATTERN = re.compile(r"testing-v[0-9]+\.[0-9]+\.[0-9]+([^,]*)") | ||
|
|
||
|
|
||
| def parse_testing_version(release_tag: str) -> str: | ||
| """Return the testing version named by the release tag, or empty if none.""" | ||
| match = _PATTERN.search(release_tag) |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_4yrwtgbfg6esv2mdlgeyix4gi5
The unanchored pattern and search() treat tags such as not-testing-v1.2.1 or sdk-v2.0.0,mytesting-v1.2.1 as testing releases. If that version matches source, an unrelated malformed release publishes the image. Split on commas and full-match a testing-v... component, with regression tests for invalid prefixes.
Codex AI reviewFound four release-automation defects affecting publication safety and verification reliability. Reviewed commit |
There was a problem hiding this comment.
The preflight and verification confirm that an image tag exists. They do not confirm the image contains what the release names. The Dockerfile installs aws-durable-execution-sdk-python>=1.0.0, resolved at build time and unbounded, so two builds of one testing version can produce different images, and skip-if-exists assumes a version identifies one artifact.
(The other three points are on the relevant lines.)
| if aws ecr-public describe-images \ | ||
| --region "${{ env.aws_region }}" \ | ||
| --repository-name "$repo_name" \ | ||
| --image-ids imageTag="v${VERSION}" >/dev/null 2>&1; then | ||
| echo "exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "exists=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Every nonzero exit becomes exists=false, which means publish. A missing IAM permission, a throttle, a network error, and a genuinely absent tag all result in publish here?
|
|
||
| # Matches a testing-v<x.y.z> entry in the release tag, stopping at a comma | ||
| # so combined tags like "sdk-v2.0.0,testing-v2.0.0" resolve to the testing part. | ||
| _PATTERN = re.compile(r"testing-v[0-9]+\.[0-9]+\.[0-9]+([^,]*)") |
There was a problem hiding this comment.
resolve_layer_sdk_version.py already parses release tags, using split(",") plus startswith("sdk-v"). parse_testing_version.py uses an unanchored regex with a ([^,]*) suffix capture. The logic is also not equivalent.
| latest_digest="$(aws ecr-public describe-images \ | ||
| --region "${{ env.aws_region }}" \ | ||
| --repository-name "$repo_name" \ | ||
| --image-ids imageTag="latest" \ | ||
| --query 'imageDetails[0].imageDigest' --output text 2>/dev/null || true)" | ||
| if [[ "$latest_digest" != "$version_digest" ]]; then | ||
| echo "::error::latest on public ECR points at $latest_digest but v$VERSION is $version_digest." |
There was a problem hiding this comment.
verify-publish only confirms latest points at the version just pushed, not that it points at the newest version, so a backport release like testing-v1.1.3 cut after 1.2.1 would move latest backwards and still pass.
2 participants
Issue #, if available:
Related to #716
Description of changes:
The
ecr-release.ymlaction would previously run for every monorepo release, regardless if the release included a new version for the testing package or not. So it was possible for this action to publish the latest changes of the testing package before they were released. This PR updates the action to be safer..github/workflows/ecr-release.yml.github/scripts/parse_testing_version.py.github/scripts/tests/test_parse_testing_version.py.github/workflows/test-parser.ymlBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.