Repository navigation
feat(aws-lambda): push a prebuilt image to ECR without Docker - #6063
Conversation
Edit accuracy: accurate 271 (base branch 271), smooth 196 of thoseThe gate passes. Quarantined, measured but not gated (0) |
58562b8 to
a452395
Compare
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 2 nits · head 8e7f3c27
- nit:
pushImageToEcr.ts:230— a 0-byte blob skips the part loop and callsCompleteLayerUploadwith no parts, which ECR rejects (EmptyUploadException). Real image layouts don't contain 0-byte blobs, so only worth a clear error if you touch this again. - nit: the PR body's
bun.locknote (refreshed workspace versions, droppeddate-fns) no longer matches the diff, which is +3/−0 inbun.lock.
Scope
Read pushImageToEcr.ts end to end, plus the build.mjs external, package.json, sdk/index.ts exports and the lockfile entry; skimmed the test fake. Checked: the one-manifest gate, digest/size verification before any ECR call, de-duplicated layer digests, the 100-digest batching, inclusive part byte ranges, the file handle closed in finally, idempotent PutImage (a moved tag on an immutable repository still throws ImageTagAlreadyExists), and refusing to adopt a repository whose tags don't match. All 8 Actions workflows on this head passed. The PR adds an SDK primitive only; nothing in the release workflow calls it yet.
— Genesis
8e7f3c2 to
39c003e
Compare
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · 8e7f3c27 → 39c003e7
Re-review after the rebase. Commits 1–5 have the same patches as the head I approved before (git range-diff). The main commits they now sit on don't touch build.mjs, sdk/index.ts or the dependency list, and they leave sha256File the same. The budget commit sets @hyperframes/aws-lambda to the measured size, the same way #6055 did, and verify:packed-manifests passes at this head. The two nits from the earlier review still apply and are optional.
Adds pushImageToEcr and ensureEcrRepository to the SDK subpath. The push reads a single-platform OCI image layout, verifies every blob against its descriptor before any ECR call, uploads only the blobs ECR lacks through the layer-upload API, and puts the manifest under a tag, returning the digest-pinned image URI.
Like the other AWS SDK clients, @aws-sdk/client-ecr is a dependency the consumer resolves. Bundled into the ESM SDK it hits a dynamic require of node:https and the packed package fails to import.
…s the caller's tags
pushImageToEcr ships in dist (unminified chunk plus its .d.ts); no source maps, tests or release-only files are involved. Built dist, npm pack --dry-run, main versus main plus this branch: +2486 B packed, +8846 B unpacked. The merge-group run measured 73369 packed and 266805 unpacked, which are the new budgets (were 70874 and 258166).
39c003e to
e9d3753
Compare
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · 39c003e7 → e9d37534
Rebase only. git range-diff shows commits 2–5 are identical. Commit 1 differs only in the sdk/index.ts export context: the prebuiltHandler exports from #6070 and the new pushImageToEcr exports are both kept. Commit 6 changes only its message. The net diff against the new base matches the approved head, and CI passes on e9d37534, including the package size check.
1 participant
What
Adds two functions to
@hyperframes/aws-lambda/sdk:ensureEcrRepository({ ecr, repositoryName, tags? })creates the ECR repository, or returns the existing one's URI when it already exists.pushImageToEcr({ ecr, layoutDir, repositoryName, repositoryUri, tag, log? })pushes a prebuilt single-platform OCI image layout (a directory) into that repository and returns{ imageUri, digest, architecture, os }, whereimageUriis<repositoryUri>@sha256:<manifest digest>.Adds the
@aws-sdk/client-ecrdependency on the same^3.1146.0line as the other AWS SDK clients. No CLI wiring yet; callers come in a follow-up.Why
The arm64 video function will ship as a container image, and Lambda only runs images from the account's own private ECR. Users of the released CLI should not need Docker to get it there. The release will publish the image as an OCI layout; this function moves that layout into the user's ECR with the ECR API alone (no Docker, no registry HTTP calls).
Related work
First piece of the container-image deploy path. Nothing else touches ECR yet.
How
index.jsonmust list exactly one image manifest (OCI or Docker v2 schema 2). An image index, attestation manifests, or zero or several manifests throw an error naming what was found.architectureandos; both are returned so the caller can assert the platform it expects.BatchCheckLayerAvailabilityin batches of at most 100 digests; only blobs notAVAILABLEare uploaded:InitiateLayerUpload, parts of exactlypartSizebytes read from disk one part at a time (inclusivepartFirstByte/partLastByte), thenCompleteLayerUpload.LayerAlreadyExistsExceptioncounts as uploaded.PutImagesends the manifest's exact bytes, its media type, the tag and the digest.ImageAlreadyExistsExceptioncounts as success, so a redeploy of the same image is a no-op.bun.lock: besides the new entry,bun installrefreshed the stale workspace version fields and dropped an unuseddate-fnsentry.Test plan
pushImageToEcr.test.tsruns the real code path against an in-memory ECR that behaves like ECR where it matters. It tracks uploads per upload id, rejects out-of-order or non-contiguous parts, a wrongpartLastByte, or a short non-final part, re-hashes the assembled bytes onCompleteLayerUpload, rejectsPutImagewhen a referenced blob is missing, and caps availability checks at 100 digests. The fixture layout is built in a temp dir: one config, a 21-byte layer and a 17-byte layer, with a 7-byte part size.bun test src/sdk/pushImageToEcr.test.ts: exit 0, 11 pass / 0 fail, three runs in a row.bun test src/handler.test.ts: exit 0, 31 pass / 0 fail.tsc --noEmitinpackages/aws-lambda: exit 0. oxlint: 0 warnings, 0 errors. oxfmt--check: clean.partLastByteoff by one: the 5 tests that upload went red.LayerAlreadyExists,ImageAlreadyExistsorRepositoryAlreadyExists: in each case only the matching test went red.