From f1736595cde96d2dcc6d5a74b62e92c5e4aecd16 Mon Sep 17 00:00:00 2001 From: ThullyoCunha Date: Wed, 29 Apr 2026 11:17:23 -0300 Subject: [PATCH] feat(webapp): apply default repository policy on ECR repo creation (#3467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Self-hosters that operate the webapp's ECR account separately from the account running the EKS workers (e.g., a shared platform account that hosts the registry plus per-team accounts that host clusters) currently hit a 403 Forbidden the first time **any** project is deployed: ``` Failed to pull image ".dkr.ecr..amazonaws.com//proj_…:…": unexpected status from HEAD request to .../v2/.../manifests/sha256:…: 403 Forbidden ``` `ensureEcrRepositoryExists` in `apps/webapp/app/v3/getDeploymentImageRef.server.ts` calls `CreateRepository` and `PutLifecyclePolicy`, but never `SetRepositoryPolicy` — so the new repo inherits the AWS default (only the registry-owner account can read/pull). Workers in the cluster account get 403 every single deploy. The only workarounds today are running a one-off post-create script or pre-creating every repo by hand. ## Proposed change Add an optional env var: ``` DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY (V4 mirror: V4_DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY) ``` Raw IAM policy JSON. When set, the webapp calls `SetRepositoryPolicy` immediately after `CreateRepository` so every new repo carries that policy from creation. Operators control the principal/actions; we don't bake in any opinions about cross-account boundaries. Example value (for the typical self-host case — grant pull to the cluster account): ```json { "Version": "2012-10-17", "Statement": [{ "Sid": "AllowClusterAccountPull", "Effect": "Allow", "Principal": {"AWS": "arn:aws:iam:::root"}, "Action": [ "ecr:GetDownloadUrlForLayer", "ecr:BatchGetImage", "ecr:BatchCheckLayerAvailability" ] }] } ``` ## Why env var (not a chart-level field) - Mirrors the shape of the sibling vars (`DEPLOY_REGISTRY_ECR_TAGS`, `DEPLOY_REGISTRY_ECR_ASSUME_ROLE_ARN`, etc.) which are already operator-supplied via `webapp.extraEnvVars` in self-host setups. - Cloud is unaffected — the env var is optional, unset by default; existing behavior unchanged. - Existing repos are unaffected — only newly-created repos get the policy. - `RepositoryCreationTemplate` from the AWS provider isn't an alternative here: it only applies to repos created via pull-through-cache or replication, not to `ecr:CreateRepository` API calls. ## Implementation - `apps/webapp/app/env.server.ts` — declare `DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY` and the V4 fallback. - `apps/webapp/app/v3/registryConfig.server.ts` — propagate `ecrDefaultRepositoryPolicy` to `RegistryConfig`. - `apps/webapp/app/v3/getDeploymentImageRef.server.ts` — `createEcrRepository` accepts the policy; if set, calls `SetRepositoryPolicy` after `PutLifecyclePolicy`. - `docs/self-hosting/env/webapp.mdx` — documentation row added under **Deploy & Registry**. ## Verification Verified end-to-end against a self-hosted Trigger.dev on EKS where the ECR account is separate from the cluster account: - **Without the env var** (current `main`): the new project's first run pod stays in `ImagePullBackOff` with `403 Forbidden`. - **With the env var set** to a JSON granting `ecr:BatchGetImage`/`GetDownloadUrlForLayer`/`BatchCheckLayerAvailability` to the cluster account: a fresh `trigger.dev deploy --env prod` followed by a `hello-world` run completes in ~5s end-to-end on the first try. Manually also confirmed that existing repos are untouched (the call only fires inside `createEcrRepository`, which only runs when `DescribeRepositories` returned `RepositoryNotFoundException`). ## Out of scope - Chart values surface for this — operators already pass the existing ECR vars via `webapp.extraEnvVars`, so this follows the same pattern. Happy to add a first-class chart field in a follow-up if that's the preferred direction. - IAM-policy validation in the webapp — we forward the JSON verbatim to AWS and surface AWS's error messages on misuse, matching how `DEPLOY_REGISTRY_ECR_TAGS` is handled today. This is a draft pending CI / CodeRabbit pass — happy to iterate on direction (e.g., split into per-action env vars, or extend the chart values schema) if any of the above choices feels off. --------- Co-authored-by: nicktrn <55853254+nicktrn@users.noreply.github.com> --- .../ecr-default-repository-policy.md | 6 ++ apps/webapp/app/env.server.ts | 5 ++ .../app/v3/getDeploymentImageRef.server.ts | 81 ++++++++++++++++++- apps/webapp/app/v3/registryConfig.server.ts | 3 + docs/self-hosting/env/webapp.mdx | 1 + 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 .server-changes/ecr-default-repository-policy.md diff --git a/.server-changes/ecr-default-repository-policy.md b/.server-changes/ecr-default-repository-policy.md new file mode 100644 index 000000000..0ec2d0465 --- /dev/null +++ b/.server-changes/ecr-default-repository-policy.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: feature +--- + +Optional `DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY` env var to apply a default repository policy when the webapp creates new ECR repos diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 1807f0a54..52ca0cc77 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -300,6 +300,7 @@ const EnvironmentSchema = z DEPLOY_REGISTRY_ECR_TAGS: z.string().optional(), // csv, for example: "key1=value1,key2=value2" DEPLOY_REGISTRY_ECR_ASSUME_ROLE_ARN: z.string().optional(), DEPLOY_REGISTRY_ECR_ASSUME_ROLE_EXTERNAL_ID: z.string().optional(), + DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY: z.string().optional(), // raw IAM policy JSON applied to every repo created by the webapp // Deployment registry (v4) - falls back to v3 registry if not specified V4_DEPLOY_REGISTRY_HOST: z @@ -332,6 +333,10 @@ const EnvironmentSchema = z .string() .optional() .transform((v) => v ?? process.env.DEPLOY_REGISTRY_ECR_ASSUME_ROLE_EXTERNAL_ID), + V4_DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY: z + .string() + .optional() + .transform((v) => v ?? process.env.DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY), // Compute gateway (template creation during deploy finalize) COMPUTE_GATEWAY_URL: z.string().optional(), diff --git a/apps/webapp/app/v3/getDeploymentImageRef.server.ts b/apps/webapp/app/v3/getDeploymentImageRef.server.ts index 70e602389..d6531b838 100644 --- a/apps/webapp/app/v3/getDeploymentImageRef.server.ts +++ b/apps/webapp/app/v3/getDeploymentImageRef.server.ts @@ -8,6 +8,7 @@ import { GetAuthorizationTokenCommand, PutLifecyclePolicyCommand, PutImageTagMutabilityCommand, + SetRepositoryPolicyCommand, } from "@aws-sdk/client-ecr"; import { STSClient, AssumeRoleCommand } from "@aws-sdk/client-sts"; import { tryCatch } from "@trigger.dev/core"; @@ -138,6 +139,7 @@ export async function getDeploymentImageRef({ roleArn: registry.ecrAssumeRoleArn, externalId: registry.ecrAssumeRoleExternalId, }, + defaultRepositoryPolicy: registry.ecrDefaultRepositoryPolicy, }) ); @@ -219,12 +221,14 @@ async function createEcrRepository({ accountId, registryTags, assumeRole, + defaultRepositoryPolicy, }: { repositoryName: string; region: string; accountId?: string; registryTags?: string; assumeRole?: AssumeRoleConfig; + defaultRepositoryPolicy?: string; }): Promise { const ecr = await createEcrClient({ region, assumeRole }); @@ -262,9 +266,50 @@ async function createEcrRepository({ }) ); + // Apply an operator-provided IAM policy to the new repository. Useful for + // self-hosters whose ECR account is separate from the account running the + // EKS workers — without this the workers get 403 Forbidden when pulling the + // task image (default ECR policy only grants access to the registry owner). + // The existing-repo branch of `ensureEcrRepositoryExists` reconciles this + // same policy on every call, so a partial-create that fails here is + // self-healing on the next deploy. + if (defaultRepositoryPolicy) { + await applyEcrRepositoryPolicy({ + repositoryName: result.repository.repositoryName!, + region, + accountId: result.repository.registryId ?? accountId, + assumeRole, + defaultRepositoryPolicy, + }); + } + return result.repository; } +async function applyEcrRepositoryPolicy({ + repositoryName, + region, + accountId, + assumeRole, + defaultRepositoryPolicy, +}: { + repositoryName: string; + region: string; + accountId?: string; + assumeRole?: AssumeRoleConfig; + defaultRepositoryPolicy: string; +}): Promise { + const ecr = await createEcrClient({ region, assumeRole }); + + await ecr.send( + new SetRepositoryPolicyCommand({ + repositoryName, + registryId: accountId, + policyText: defaultRepositoryPolicy, + }) + ); +} + async function updateEcrRepositoryCacheSettings({ repositoryName, region, @@ -386,11 +431,13 @@ async function ensureEcrRepositoryExists({ registryHost, registryTags, assumeRole, + defaultRepositoryPolicy, }: { repositoryName: string; registryHost: string; registryTags?: string; assumeRole?: AssumeRoleConfig; + defaultRepositoryPolicy?: string; }): Promise<{ repo: Repository; repoCreated: boolean }> { const { region, accountId } = parseEcrRegistryDomain(registryHost); @@ -421,6 +468,31 @@ async function ensureEcrRepositoryExists({ } } + // Reconcile the default repository policy on every call. Idempotent, and + // covers two recovery cases: (1) a previous create succeeded but the + // SetRepositoryPolicy call failed mid-flight, leaving the repo without a + // policy; (2) the operator updated DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY + // and existing repos need to pick up the new value. + if (defaultRepositoryPolicy) { + const [policyError] = await tryCatch( + applyEcrRepositoryPolicy({ + repositoryName, + region, + accountId, + assumeRole, + defaultRepositoryPolicy, + }) + ); + + if (policyError) { + logger.error("Failed to reconcile ECR repository policy on existing repo", { + repositoryName, + region, + policyError, + }); + } + } + return { repo: existingRepo, repoCreated: false, @@ -428,7 +500,14 @@ async function ensureEcrRepositoryExists({ } const [createRepoError, newRepo] = await tryCatch( - createEcrRepository({ repositoryName, region, accountId, registryTags, assumeRole }) + createEcrRepository({ + repositoryName, + region, + accountId, + registryTags, + assumeRole, + defaultRepositoryPolicy, + }) ); if (createRepoError) { diff --git a/apps/webapp/app/v3/registryConfig.server.ts b/apps/webapp/app/v3/registryConfig.server.ts index 72e2abdae..eb7986edd 100644 --- a/apps/webapp/app/v3/registryConfig.server.ts +++ b/apps/webapp/app/v3/registryConfig.server.ts @@ -8,6 +8,7 @@ export type RegistryConfig = { ecrTags?: string; ecrAssumeRoleArn?: string; ecrAssumeRoleExternalId?: string; + ecrDefaultRepositoryPolicy?: string; }; export function getRegistryConfig(isV4Deployment: boolean): RegistryConfig { @@ -20,6 +21,7 @@ export function getRegistryConfig(isV4Deployment: boolean): RegistryConfig { ecrTags: env.V4_DEPLOY_REGISTRY_ECR_TAGS, ecrAssumeRoleArn: env.V4_DEPLOY_REGISTRY_ECR_ASSUME_ROLE_ARN, ecrAssumeRoleExternalId: env.V4_DEPLOY_REGISTRY_ECR_ASSUME_ROLE_EXTERNAL_ID, + ecrDefaultRepositoryPolicy: env.V4_DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY, }; } @@ -31,5 +33,6 @@ export function getRegistryConfig(isV4Deployment: boolean): RegistryConfig { ecrTags: env.DEPLOY_REGISTRY_ECR_TAGS, ecrAssumeRoleArn: env.DEPLOY_REGISTRY_ECR_ASSUME_ROLE_ARN, ecrAssumeRoleExternalId: env.DEPLOY_REGISTRY_ECR_ASSUME_ROLE_EXTERNAL_ID, + ecrDefaultRepositoryPolicy: env.DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY, }; } diff --git a/docs/self-hosting/env/webapp.mdx b/docs/self-hosting/env/webapp.mdx index 16a54e0f9..3a45bf7b0 100644 --- a/docs/self-hosting/env/webapp.mdx +++ b/docs/self-hosting/env/webapp.mdx @@ -76,6 +76,7 @@ mode: "wide" | `DEPLOY_REGISTRY_USERNAME` | No | — | Deploy registry username. | | `DEPLOY_REGISTRY_PASSWORD` | No | — | Deploy registry password. | | `DEPLOY_REGISTRY_NAMESPACE` | No | trigger | Deploy registry namespace. | +| `DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY` | No | — | Raw IAM policy JSON applied via SetRepositoryPolicy to every ECR repo created by the webapp. Use to grant cross-account pull access to EKS workers when the ECR account is separate from the cluster account. | | `DEPLOY_IMAGE_PLATFORM` | No | linux/amd64 | Deploy image platform, same values as docker `--platform` flag. | | `DEPLOY_TIMEOUT_MS` | No | 480000 (8m) | Deploy timeout (ms). | | **Object store (S3)** | | | |