fix(webapp): reject deploy images with runtime-incompatible zstd layers (#4184)
Container runtimes (cri-o / containerd / podman) can't pull zstd-compressed layers carried in a Docker v2s2 manifest (`application/vnd.docker.image.rootfs.diff.tar.zstd`). A deploy built with an outdated CLI can produce exactly that combination - and today it's promoted to current and then fails every run at image-pull time. This extends the pre-promotion image check (#4049) to also inspect the manifest's layer media types. If any layer uses the unpullable zstd/v2s2 media type, the deploy is rejected at finalize with a clear message to upgrade the CLI and re-deploy, instead of silently shipping a version that can't start. The manifest is already returned by the existing ECR `BatchGetImage` call, so there's no extra registry request for single-arch images. Parsing is a lenient Zod schema and **fails open** - a manifest we can't read never blocks a deploy. Manifest lists / OCI indexes (no top-level `layers[]`) and OCI zstd (`...tar+zstd`, which runtimes support) pass unaffected. Also clarifies in the contributor docs that changesets and `.server-changes/` notes are user-facing and should be written for users, not maintainers. refs TRI-11702
This commit is contained in:
@@ -42,10 +42,16 @@ The body text (below the frontmatter) is a one-line description of the change. K
|
||||
|
||||
These entries are public-facing - they ship verbatim in user-visible release notes. A few rules to keep them clean:
|
||||
|
||||
- **Write for the user, not the reviewer.** Lead with what the user notices or has to do. If a reader who doesn't know the codebase can't tell what changed for them, rewrite it.
|
||||
- **One sentence is usually enough.** The body is the bullet in the changelog. If you need a paragraph, you're probably describing the implementation rather than the change.
|
||||
- **Describe behavior, not implementation.** Skip internal scopes, middleware names, library specifics, framework internals. Users care about what's different for them, not how it's wired.
|
||||
- **Never name internal tools or infra.** Observability stacks, internal services, infra components, monitoring backends, CI surfaces, AWS specifics - none of these belong in user-facing notes.
|
||||
|
||||
Before / after:
|
||||
|
||||
- ❌ _"The image verification step now parses the manifest's layer media types and returns a new result the finalizer rejects."_ (describes the wiring; a user can't act on it)
|
||||
- ✅ _"Deploying with an outdated CLI could produce an image that fails to start on every run. These deploys are now stopped before going live, with a message asking you to upgrade the CLI and re-deploy."_ (what the user sees and does)
|
||||
|
||||
## Lifecycle
|
||||
|
||||
1. Engineer adds a `.server-changes/` file in their PR
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: fix
|
||||
---
|
||||
|
||||
Deploying with an outdated CLI could produce an image that fails to start on every run. These deploys are now stopped before going live, with a message asking you to upgrade the CLI and re-deploy.
|
||||
@@ -104,6 +104,8 @@ pnpm run changeset:add
|
||||
|
||||
When modifying only server components (`apps/webapp/`, `apps/supervisor/`, etc.) with no package changes, add a `.server-changes/` file instead. See `.server-changes/README.md` for format and documentation.
|
||||
|
||||
**Write the description for users, not maintainers.** Both changesets and `.server-changes/` notes ship verbatim in user-visible release notes. Lead with what changed *for the user* - one plain sentence describing behavior, not implementation, and never naming internal tools or infra. The full writing guidance in `.server-changes/README.md` applies to changesets too.
|
||||
|
||||
## Dependency Pinning
|
||||
|
||||
Zod is pinned to a single version across the entire monorepo (currently `3.25.76`). When adding zod to a new or existing package, use the **exact same version** as the rest of the repo - never a different version or a range. Mismatched zod versions cause runtime type incompatibilities (e.g., schemas from one package can't be used as body validators in another).
|
||||
|
||||
@@ -186,6 +186,12 @@ export class FinalizeDeploymentV2Service extends BaseService {
|
||||
);
|
||||
}
|
||||
|
||||
if (result === "nonconformant") {
|
||||
throw new ServiceValidationError(
|
||||
"Deployment image is not runnable: it contains zstd-compressed layers inside a Docker (v2s2) manifest, which the container runtime cannot pull. This typically comes from an outdated CLI version. Please upgrade to the latest trigger.dev CLI and re-deploy."
|
||||
);
|
||||
}
|
||||
|
||||
// Fail closed: if we can't confirm the image is present, don't promote a version
|
||||
// that might not start. Set DEPLOY_IMAGE_VERIFICATION_ENABLED=0 for out-of-band pushes.
|
||||
if (result === "unknown") {
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
} from "@aws-sdk/client-ecr";
|
||||
import { tryCatch } from "@trigger.dev/core";
|
||||
import pRetry, { AbortError } from "p-retry";
|
||||
import { z } from "zod";
|
||||
import { logger } from "~/services/logger.server";
|
||||
import {
|
||||
type AssumeRoleConfig,
|
||||
@@ -16,7 +17,85 @@ import { type RegistryConfig } from "../registryConfig.server";
|
||||
|
||||
const SHA256_DIGEST = /^sha256:[a-f0-9]{64}$/;
|
||||
|
||||
export type ImageLookupResult = "found" | "missing" | "unknown";
|
||||
export type ImageLookupResult = "found" | "missing" | "unknown" | "nonconformant";
|
||||
|
||||
// A zstd layer carried in a Docker (v2s2) manifest rather than an OCI manifest is
|
||||
// unpullable by cri-o/containerd/podman. OCI zstd (...tar+zstd) is fine - only this
|
||||
// Docker media type is rejected. An outdated CLI that predates OCI-media-type output
|
||||
// can emit it when reusing zstd layers from a prior build or the registry cache.
|
||||
const UNPULLABLE_LAYER_MEDIA_TYPE = "application/vnd.docker.image.rootfs.diff.tar.zstd";
|
||||
|
||||
// Nested indexes are exotic (index -> per-platform image manifests is depth 1); cap the
|
||||
// walk so a pathological manifest can't fan out unbounded.
|
||||
const MAX_MANIFEST_DEPTH = 3;
|
||||
|
||||
// Lenient: we only read what we need. An image manifest carries layers[]; a manifest list
|
||||
// / OCI index carries manifests[] (per-platform child pointers). Both optional so either
|
||||
// shape parses cleanly.
|
||||
const ManifestSchema = z.object({
|
||||
layers: z.array(z.object({ mediaType: z.string().optional() })).optional(),
|
||||
manifests: z.array(z.object({ digest: z.string() })).optional(),
|
||||
});
|
||||
|
||||
// Inspect one raw manifest: does it directly carry an unpullable layer, and (if it's an
|
||||
// index) which child manifests should be walked. Fails open to empty on anything we can't
|
||||
// parse, so an unreadable manifest never blocks a deploy.
|
||||
export function inspectManifest(rawManifest: string | undefined): {
|
||||
hasUnpullableLayer: boolean;
|
||||
childDigests: string[];
|
||||
} {
|
||||
const empty = { hasUnpullableLayer: false, childDigests: [] };
|
||||
|
||||
if (!rawManifest) {
|
||||
return empty;
|
||||
}
|
||||
|
||||
let json: unknown;
|
||||
try {
|
||||
json = JSON.parse(rawManifest);
|
||||
} catch {
|
||||
return empty;
|
||||
}
|
||||
|
||||
const parsed = ManifestSchema.safeParse(json);
|
||||
if (!parsed.success) {
|
||||
return empty;
|
||||
}
|
||||
|
||||
return {
|
||||
hasUnpullableLayer:
|
||||
parsed.data.layers?.some((layer) => layer.mediaType === UNPULLABLE_LAYER_MEDIA_TYPE) ?? false,
|
||||
childDigests: parsed.data.manifests?.map((child) => child.digest) ?? [],
|
||||
};
|
||||
}
|
||||
|
||||
// Walk a manifest and, for a multi-arch index, its child manifests (fetched by digest).
|
||||
// Returns true if any layer anywhere in the tree uses the unpullable media type. A child
|
||||
// that can't be fetched (undefined) is treated as conformant - fail open.
|
||||
export async function treeHasUnpullableLayer(
|
||||
rawManifest: string | undefined,
|
||||
fetchManifest: (digest: string) => Promise<string | undefined>,
|
||||
depth = 0
|
||||
): Promise<boolean> {
|
||||
const { hasUnpullableLayer, childDigests } = inspectManifest(rawManifest);
|
||||
|
||||
if (hasUnpullableLayer) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (depth >= MAX_MANIFEST_DEPTH) {
|
||||
return false;
|
||||
}
|
||||
|
||||
for (const digest of childDigests) {
|
||||
const childManifest = await fetchManifest(digest);
|
||||
if (await treeHasUnpullableLayer(childManifest, fetchManifest, depth + 1)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Split a stored ECR image reference into repository + tag.
|
||||
@@ -86,8 +165,10 @@ const sendBatchGetImage: BatchGetImageSender = async ({
|
||||
imageIds,
|
||||
}) => {
|
||||
const ecr = await createEcrClient({ region, assumeRole });
|
||||
// No acceptedMediaTypes: only the single-manifest types are valid enum values, and
|
||||
// we only care whether the image exists, not its manifest format.
|
||||
// Intentionally no acceptedMediaTypes: ECR returns the manifest as stored - which we
|
||||
// rely on for the layer-media-type check - and omitting it avoids a multi-arch index
|
||||
// being reported as a failure (i.e. misread as missing). BatchGetImage populates
|
||||
// imageManifest by default when the image exists; the check fails open if it's absent.
|
||||
return ecr.send(new BatchGetImageCommand({ repositoryName, registryId, imageIds }));
|
||||
};
|
||||
|
||||
@@ -183,5 +264,46 @@ export async function ecrImageExists(
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
return interpretBatchGetImageResponse(response);
|
||||
const result = interpretBatchGetImageResponse(response);
|
||||
|
||||
if (result !== "found") {
|
||||
return result;
|
||||
}
|
||||
|
||||
// Image exists - now confirm the runtime can actually pull it. Follow index children by
|
||||
// digest so multi-arch deploys are covered, not just single image manifests.
|
||||
const fetchManifest = async (digest: string): Promise<string | undefined> => {
|
||||
const [fetchError, childResponse] = await tryCatch(
|
||||
_send({
|
||||
region,
|
||||
assumeRole,
|
||||
registryId: accountId,
|
||||
repositoryName: parsed.repositoryName,
|
||||
imageIds: [{ imageDigest: digest }],
|
||||
})
|
||||
);
|
||||
|
||||
if (fetchError) {
|
||||
logger.warn("Could not fetch child manifest for conformance check", {
|
||||
imageReference,
|
||||
digest,
|
||||
error: fetchError.message,
|
||||
});
|
||||
return undefined; // fail open on this child
|
||||
}
|
||||
|
||||
return childResponse.images?.[0]?.imageManifest;
|
||||
};
|
||||
|
||||
const topManifest = response.images?.[0]?.imageManifest;
|
||||
|
||||
if (await treeHasUnpullableLayer(topManifest, fetchManifest)) {
|
||||
logger.error("Deployment image has a runtime-incompatible layer media type", {
|
||||
imageReference,
|
||||
repositoryName: parsed.repositoryName,
|
||||
});
|
||||
return "nonconformant";
|
||||
}
|
||||
|
||||
return "found";
|
||||
}
|
||||
|
||||
@@ -2,14 +2,37 @@ import { RepositoryNotFoundException } from "@aws-sdk/client-ecr";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
ecrImageExists,
|
||||
inspectManifest,
|
||||
interpretBatchGetImageResponse,
|
||||
parseEcrImageReference,
|
||||
treeHasUnpullableLayer,
|
||||
} from "~/v3/services/verifyDeploymentImage.server";
|
||||
import { type RegistryConfig } from "~/v3/registryConfig.server";
|
||||
|
||||
const ECR_HOST = "123456789012.dkr.ecr.us-east-1.amazonaws.com";
|
||||
const ecrConfig: RegistryConfig = { host: ECR_HOST, namespace: "deployments-test" };
|
||||
|
||||
const DIGEST_A = `sha256:${"a".repeat(64)}`;
|
||||
const DIGEST_B = `sha256:${"b".repeat(64)}`;
|
||||
const ZSTD_DOCKER = "application/vnd.docker.image.rootfs.diff.tar.zstd";
|
||||
|
||||
const imageManifest = (layerMediaTypes: string[]) =>
|
||||
JSON.stringify({
|
||||
schemaVersion: 2,
|
||||
mediaType: "application/vnd.docker.distribution.manifest.v2+json",
|
||||
layers: layerMediaTypes.map((mediaType) => ({ mediaType, digest: DIGEST_A })),
|
||||
});
|
||||
|
||||
const indexManifest = (childDigests: string[]) =>
|
||||
JSON.stringify({
|
||||
schemaVersion: 2,
|
||||
mediaType: "application/vnd.oci.image.index.v1+json",
|
||||
manifests: childDigests.map((digest) => ({
|
||||
digest,
|
||||
mediaType: "application/vnd.oci.image.manifest.v1+json",
|
||||
})),
|
||||
});
|
||||
|
||||
describe("parseEcrImageReference", () => {
|
||||
it("splits repository and tag for a ref under the configured host", () => {
|
||||
const ref = `${ECR_HOST}/deployments-test/proj_abc:20240101.1.prod.a1b2c3d4`;
|
||||
@@ -61,6 +84,73 @@ describe("interpretBatchGetImageResponse", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("inspectManifest", () => {
|
||||
it("flags an unpullable zstd layer in an image manifest", () => {
|
||||
const result = inspectManifest(
|
||||
imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip", ZSTD_DOCKER])
|
||||
);
|
||||
expect(result.hasUnpullableLayer).toBe(true);
|
||||
expect(result.childDigests).toEqual([]);
|
||||
});
|
||||
|
||||
it("passes OCI zstd and gzip layers (runtime-supported media types)", () => {
|
||||
const result = inspectManifest(
|
||||
imageManifest([
|
||||
"application/vnd.oci.image.layer.v1.tar+gzip",
|
||||
"application/vnd.oci.image.layer.v1.tar+zstd",
|
||||
])
|
||||
);
|
||||
expect(result.hasUnpullableLayer).toBe(false);
|
||||
});
|
||||
|
||||
it("returns child digests for an index and does not flag it directly", () => {
|
||||
const result = inspectManifest(indexManifest([DIGEST_A, DIGEST_B]));
|
||||
expect(result.hasUnpullableLayer).toBe(false);
|
||||
expect(result.childDigests).toEqual([DIGEST_A, DIGEST_B]);
|
||||
});
|
||||
|
||||
it("fails open (empty) when the manifest is absent or unparseable", () => {
|
||||
expect(inspectManifest(undefined)).toEqual({ hasUnpullableLayer: false, childDigests: [] });
|
||||
expect(inspectManifest("not json")).toEqual({ hasUnpullableLayer: false, childDigests: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe("treeHasUnpullableLayer", () => {
|
||||
const neverFetch = async () => undefined;
|
||||
|
||||
it("detects an unpullable layer in a flat image manifest", async () => {
|
||||
expect(await treeHasUnpullableLayer(imageManifest([ZSTD_DOCKER]), neverFetch)).toBe(true);
|
||||
});
|
||||
|
||||
it("follows an index and detects an unpullable layer in a child", async () => {
|
||||
const children: Record<string, string> = {
|
||||
[DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
|
||||
[DIGEST_B]: imageManifest([ZSTD_DOCKER]),
|
||||
};
|
||||
const result = await treeHasUnpullableLayer(
|
||||
indexManifest([DIGEST_A, DIGEST_B]),
|
||||
async (digest) => children[digest]
|
||||
);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for an index whose children are all conformant", async () => {
|
||||
const children: Record<string, string> = {
|
||||
[DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
|
||||
[DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
|
||||
};
|
||||
const result = await treeHasUnpullableLayer(
|
||||
indexManifest([DIGEST_A, DIGEST_B]),
|
||||
async (digest) => children[digest]
|
||||
);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("fails open when a child manifest can't be fetched", async () => {
|
||||
expect(await treeHasUnpullableLayer(indexManifest([DIGEST_A]), neverFetch)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ecrImageExists", () => {
|
||||
it("returns unknown for a non-ECR registry without calling the registry", async () => {
|
||||
let called = false;
|
||||
@@ -174,4 +264,54 @@ describe("ecrImageExists", () => {
|
||||
);
|
||||
expect(seen.imageIds).toEqual([{ imageTag: "v1.prod.a1b2c3d4" }]);
|
||||
});
|
||||
|
||||
// Resolve a manifest by tag or digest against a fixture map, mimicking BatchGetImage.
|
||||
const sendFrom =
|
||||
(byRef: Record<string, string>) =>
|
||||
async (input: any): Promise<any> => {
|
||||
const id = input.imageIds[0];
|
||||
const manifest = byRef[id.imageDigest ?? id.imageTag];
|
||||
return manifest ? { images: [{ imageManifest: manifest }] } : { images: [{}] };
|
||||
};
|
||||
|
||||
it("returns nonconformant for a single-arch image with an unpullable zstd layer", async () => {
|
||||
const result = await ecrImageExists(
|
||||
{
|
||||
imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
|
||||
registryConfig: ecrConfig,
|
||||
},
|
||||
sendFrom({ "v1.prod.a1b2c3d4": imageManifest([ZSTD_DOCKER]) })
|
||||
);
|
||||
expect(result).toBe("nonconformant");
|
||||
});
|
||||
|
||||
it("returns nonconformant when a multi-arch index has an unpullable child", async () => {
|
||||
const result = await ecrImageExists(
|
||||
{
|
||||
imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
|
||||
registryConfig: ecrConfig,
|
||||
},
|
||||
sendFrom({
|
||||
"v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
|
||||
[DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
|
||||
[DIGEST_B]: imageManifest([ZSTD_DOCKER]),
|
||||
})
|
||||
);
|
||||
expect(result).toBe("nonconformant");
|
||||
});
|
||||
|
||||
it("returns found when a multi-arch index's children are all conformant", async () => {
|
||||
const result = await ecrImageExists(
|
||||
{
|
||||
imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
|
||||
registryConfig: ecrConfig,
|
||||
},
|
||||
sendFrom({
|
||||
"v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
|
||||
[DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
|
||||
[DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
|
||||
})
|
||||
);
|
||||
expect(result).toBe("found");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user