Skip to content

Commit 18f7bef

Browse files
committed
fix(webapp): reconcile ECR default policy on existing repos
Mirrors how the existing-repo branch already reconciles cache settings. SetRepositoryPolicy is idempotent, so applying it on every deploy is safe and covers two recovery cases that the previous version didn't: 1. A previous repo creation succeeded but SetRepositoryPolicy failed mid-flight, leaving the repo without a policy. Without reconciliation, the existing-repo branch would just return the repo and runners would keep getting 403 Forbidden forever — manual intervention required. 2. The operator updates DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY to grant pull to additional accounts/principals. Existing repos need to pick up the new value, not just freshly created ones. The factored `applyEcrRepositoryPolicy` helper is shared between the create and reconcile call sites, keeping behavior identical.
1 parent 2492a30 commit 18f7bef

1 file changed

Lines changed: 59 additions & 7 deletions

File tree

apps/webapp/app/v3/getDeploymentImageRef.server.ts

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,19 +270,46 @@ async function createEcrRepository({
270270
// self-hosters whose ECR account is separate from the account running the
271271
// EKS workers — without this the workers get 403 Forbidden when pulling the
272272
// task image (default ECR policy only grants access to the registry owner).
273+
// The existing-repo branch of `ensureEcrRepositoryExists` reconciles this
274+
// same policy on every call, so a partial-create that fails here is
275+
// self-healing on the next deploy.
273276
if (defaultRepositoryPolicy) {
274-
await ecr.send(
275-
new SetRepositoryPolicyCommand({
276-
repositoryName: result.repository.repositoryName,
277-
registryId: result.repository.registryId,
278-
policyText: defaultRepositoryPolicy,
279-
})
280-
);
277+
await applyEcrRepositoryPolicy({
278+
repositoryName: result.repository.repositoryName!,
279+
region,
280+
accountId: result.repository.registryId ?? accountId,
281+
assumeRole,
282+
defaultRepositoryPolicy,
283+
});
281284
}
282285

283286
return result.repository;
284287
}
285288

289+
async function applyEcrRepositoryPolicy({
290+
repositoryName,
291+
region,
292+
accountId,
293+
assumeRole,
294+
defaultRepositoryPolicy,
295+
}: {
296+
repositoryName: string;
297+
region: string;
298+
accountId?: string;
299+
assumeRole?: AssumeRoleConfig;
300+
defaultRepositoryPolicy: string;
301+
}): Promise<void> {
302+
const ecr = await createEcrClient({ region, assumeRole });
303+
304+
await ecr.send(
305+
new SetRepositoryPolicyCommand({
306+
repositoryName,
307+
registryId: accountId,
308+
policyText: defaultRepositoryPolicy,
309+
})
310+
);
311+
}
312+
286313
async function updateEcrRepositoryCacheSettings({
287314
repositoryName,
288315
region,
@@ -441,6 +468,31 @@ async function ensureEcrRepositoryExists({
441468
}
442469
}
443470

471+
// Reconcile the default repository policy on every call. Idempotent, and
472+
// covers two recovery cases: (1) a previous create succeeded but the
473+
// SetRepositoryPolicy call failed mid-flight, leaving the repo without a
474+
// policy; (2) the operator updated DEPLOY_REGISTRY_ECR_DEFAULT_REPOSITORY_POLICY
475+
// and existing repos need to pick up the new value.
476+
if (defaultRepositoryPolicy) {
477+
const [policyError] = await tryCatch(
478+
applyEcrRepositoryPolicy({
479+
repositoryName,
480+
region,
481+
accountId,
482+
assumeRole,
483+
defaultRepositoryPolicy,
484+
})
485+
);
486+
487+
if (policyError) {
488+
logger.error("Failed to reconcile ECR repository policy on existing repo", {
489+
repositoryName,
490+
region,
491+
policyError,
492+
});
493+
}
494+
}
495+
444496
return {
445497
repo: existingRepo,
446498
repoCreated: false,

0 commit comments

Comments
 (0)