Repository navigation
fix(stack): pin catalog images that ECR Public mirrors and report every mirror failure #6804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,16 @@ | |
| * `apps/cli/src/shared/services/Dockerfile`. Native archive URLs are derived | ||
| * from service + version. An entry that already carries a digest keeps one: | ||
| * the published `ghcr.io/supabase/cli/<service>:<version>` manifest digest. | ||
| * A pin is refused until ECR Public serves the same digest, so the stack's | ||
| * image fallback can pull it. | ||
| * | ||
| * Run: `bun .github/scripts/sync-artifacts-catalog.ts <dockerfile> <catalog> [base-dockerfile]` | ||
| */ | ||
|
|
||
| import { parseDockerfileServiceImages } from "../../apps/cli/src/shared/services/parse-dockerfile-service-images.ts"; | ||
| import { isOrioleImage, slimCatalogPin } from "../../apps/cli/src/shared/services/slim-images.ts"; | ||
| import { | ||
| DEST_REGISTRY, | ||
| DIGEST_PATTERN, | ||
| InvalidPayloadError, | ||
| SOURCE_REGISTRY, | ||
|
|
@@ -71,7 +74,7 @@ export interface CatalogPlan { | |
|
|
||
| export type ReleasePublication = | ||
| | { readonly status: "published"; readonly digest?: string } | ||
| | { readonly status: "missing" | "lookup-failed" }; | ||
| | { readonly status: "missing" | "unmirrored" | "lookup-failed" }; | ||
|
|
||
| /** `definition("<service>", "<version>", "<image>"`. */ | ||
| function defaultEntryPattern(service: string): RegExp { | ||
|
|
@@ -267,7 +270,9 @@ export async function planArtifactCatalogUpdate(input: { | |
| reason: | ||
| release.status === "missing" | ||
| ? `${pin.service}:${pin.version} has no published slim image and native release.` | ||
| : `${pin.service}:${pin.version} publication check failed.`, | ||
| : release.status === "unmirrored" | ||
| ? `${pin.service}:${pin.version} on ${DEST_REGISTRY} does not match the GHCR digest.` | ||
|
Comment on lines
+273
to
+274
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ NIT · A missing ECR tag is reported as a digest mismatch, obscuring that the image has not been mirrored. Evidence: .github/scripts/sync-artifacts-catalog.ts:324 returns a missing mirror with no digest; lines 365 and 273-274 turn that into the message that ECR does not match the GHCR digest. Suggested fix: Distinguish a missing ECR tag from an existing tag with a different digest in the blocking reason.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded the blocking reason to "is not on public.ecr.aws/supabase/cli under the GHCR digest", which is accurate for both a missing tag and a mismatched digest. 🤖 Addressed by Claude Code |
||
| : `${pin.service}:${pin.version} publication check failed.`, | ||
| blocking: true, | ||
| }); | ||
| continue; | ||
|
|
@@ -347,15 +352,17 @@ async function probeNativeRelease(service: string, version: string): Promise<Pro | |
| } | ||
|
|
||
| async function lookupPublication(service: string, version: string): Promise<ReleasePublication> { | ||
| const reference = `${SLIM_IMAGE_PREFIX}${service}:${version}`; | ||
| const [manifest, native] = await Promise.all([ | ||
| probeManifest(reference), | ||
| const [manifest, mirror, native] = await Promise.all([ | ||
| probeManifest(`${SLIM_IMAGE_PREFIX}${service}:${version}`), | ||
| probeManifest(`${DEST_REGISTRY}/${service}:${version}`), | ||
| probeNativeRelease(service, version), | ||
| ]); | ||
| if (manifest.probe === "lookup-failed" || native === "lookup-failed") { | ||
| return { status: "lookup-failed" }; | ||
| } | ||
| if (manifest.probe === "missing" || native === "missing") return { status: "missing" }; | ||
| if (mirror.probe === "lookup-failed") return { status: "lookup-failed" }; | ||
| if (mirror.digest !== manifest.digest) return { status: "unmirrored" }; | ||
| return { status: "published", digest: manifest.digest }; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 MINOR ·
test-coverage· source: codexThe added test does not exercise the new ECR registry probe or digest comparison.
Evidence: .github/scripts/sync-artifacts-catalog.test.ts:95-111 supplies a publication callback that directly returns unmirrored. The probe and comparison are in .github/scripts/sync-artifacts-catalog.ts:354-366, outside the tested planner.
Suggested fix: Add a focused test of the publication lookup with controlled GHCR and ECR probe responses, covering missing and mismatched mirrors.