Skip to content

fix(webapp): prevent duplicate preview env image tags #2475

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 4, 2025
Merged

fix(webapp): prevent duplicate preview env image tags #2475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd07aae
fix(webapp): prevent duplicate preview env image tags
nicktrn Sep 3, 2025
0a17676
use deploy shortcode instead of new nanoid
nicktrn Sep 3, 2025
a721b30
replace regexps in tests
nicktrn Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions apps/webapp/app/v3/getDeploymentImageRef.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { STSClient, AssumeRoleCommand } from "@aws-sdk/client-sts";
import { tryCatch } from "@trigger.dev/core";
import { logger } from "~/services/logger.server";
import { type RegistryConfig } from "./registryConfig.server";
import type { EnvironmentType } from "@trigger.dev/core/v3";

// Optional configuration for cross-account access
export type AssumeRoleConfig = {
Expand Down Expand Up @@ -101,19 +102,22 @@ export async function getDeploymentImageRef({
registry,
projectRef,
nextVersion,
environmentSlug,
environmentType,
deploymentShortCode,
}: {
registry: RegistryConfig;
projectRef: string;
nextVersion: string;
environmentSlug: string;
environmentType: EnvironmentType;
deploymentShortCode: string;
}): Promise<{
imageRef: string;
isEcr: boolean;
repoCreated: boolean;
}> {
const repositoryName = `${registry.namespace}/${projectRef}`;
const imageRef = `${registry.host}/${repositoryName}:${nextVersion}.${environmentSlug}`;
const envType = environmentType.toLowerCase();
const imageRef = `${registry.host}/${repositoryName}:${nextVersion}.${envType}.${deploymentShortCode}`;

if (!isEcrRegistry(registry.host)) {
return {
Expand Down
7 changes: 5 additions & 2 deletions apps/webapp/app/v3/services/initializeDeployment.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,15 @@ export class InitializeDeploymentService extends BaseService {
const isV4Deployment = payload.type === "MANAGED";
const registryConfig = getRegistryConfig(isV4Deployment);

const deploymentShortCode = nanoid(8);

const [imageRefError, imageRefResult] = await tryCatch(
getDeploymentImageRef({
registry: registryConfig,
projectRef: environment.project.externalRef,
nextVersion,
environmentSlug: environment.slug,
environmentType: environment.type,
deploymentShortCode,
})
);

Expand Down Expand Up @@ -111,7 +114,7 @@ export class InitializeDeploymentService extends BaseService {
data: {
friendlyId: generateFriendlyId("deployment"),
contentHash: payload.contentHash,
shortCode: nanoid(8),
shortCode: deploymentShortCode,
version: nextVersion,
status: "BUILDING",
environmentId: environment.id,
Expand Down
149 changes: 92 additions & 57 deletions apps/webapp/test/getDeploymentImageRef.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "../app/v3/getDeploymentImageRef.server";
import { DeleteRepositoryCommand } from "@aws-sdk/client-ecr";

describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef", () => {
describe("getDeploymentImageRef", () => {
const testHost =
process.env.DEPLOY_REGISTRY_HOST || "123456789012.dkr.ecr.us-east-1.amazonaws.com";
const testNamespace = process.env.DEPLOY_REGISTRY_NAMESPACE || "test-namespace";
Expand All @@ -25,7 +25,7 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef",

// Clean up test repository after tests
afterAll(async () => {
if (process.env.KEEP_TEST_REPO === "1") {
if (process.env.KEEP_TEST_REPO === "1" || process.env.RUN_ECR_TESTS !== "1") {
return;
}

Expand Down Expand Up @@ -57,7 +57,7 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef",
it("should return the correct image ref for non-ECR registry", async () => {
const imageRef = await getDeploymentImageRef({
registry: {
host: "registry.digitalocean.com",
host: "registry.example.com",
namespace: testNamespace,
username: "test-user",
password: "test-pass",
Expand All @@ -67,17 +67,69 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef",
},
projectRef: testProjectRef,
nextVersion: "20250630.1",
environmentSlug: "test",
environmentType: "DEVELOPMENT",
deploymentShortCode: "test1234",
});

// Check the image ref structure and that it contains expected parts
expect(imageRef.imageRef).toBe(
`registry.digitalocean.com/${testNamespace}/${testProjectRef}:20250630.1.test`
`registry.example.com/${testNamespace}/${testProjectRef}:20250630.1.development.test1234`
);
expect(imageRef.isEcr).toBe(false);
});

it("should create ECR repository and return correct image ref", async () => {
const imageRef1 = await getDeploymentImageRef({
it.skipIf(process.env.RUN_ECR_TESTS !== "1")(
"should create ECR repository and return correct image ref",
async () => {
const imageRef1 = await getDeploymentImageRef({
registry: {
host: testHost,
namespace: testNamespace,
username: "test-user",
password: "test-pass",
ecrTags: registryTags,
ecrAssumeRoleArn: roleArn,
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef2,
nextVersion: "20250630.1",
environmentType: "DEVELOPMENT",
deploymentShortCode: "test1234",
});

expect(imageRef1.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef2}:20250630.1.development.test1234`
);
expect(imageRef1.isEcr).toBe(true);
expect(imageRef1.repoCreated).toBe(true);

const imageRef2 = await getDeploymentImageRef({
registry: {
host: testHost,
namespace: testNamespace,
username: "test-user",
password: "test-pass",
ecrTags: registryTags,
ecrAssumeRoleArn: roleArn,
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef2,
nextVersion: "20250630.2",
environmentType: "DEVELOPMENT",
deploymentShortCode: "test1234",
});

expect(imageRef2.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef2}:20250630.2.development.test1234`
);
expect(imageRef2.isEcr).toBe(true);
expect(imageRef2.repoCreated).toBe(false);
}
);

it.skipIf(process.env.RUN_ECR_TESTS !== "1")("should reuse existing ECR repository", async () => {
// This should use the repository created in the previous test
const imageRef = await getDeploymentImageRef({
registry: {
host: testHost,
namespace: testNamespace,
Expand All @@ -87,44 +139,42 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef",
ecrAssumeRoleArn: roleArn,
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef2,
nextVersion: "20250630.1",
environmentSlug: "test",
projectRef: testProjectRef,
nextVersion: "20250630.2",
environmentType: "PRODUCTION",
deploymentShortCode: "test1234",
});

expect(imageRef1.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef2}:20250630.1.test`
expect(imageRef.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef}:20250630.2.production.test1234`
);
expect(imageRef1.isEcr).toBe(true);
expect(imageRef1.repoCreated).toBe(true);
expect(imageRef.isEcr).toBe(true);
});

it("should generate unique image tags for different deployments with same environment type", async () => {
// Simulates the scenario where multiple deployments happen to the same environment type
const sameEnvironmentType = "PREVIEW";
const sameVersion = "20250630.1";

const imageRef2 = await getDeploymentImageRef({
const firstImageRef = await getDeploymentImageRef({
registry: {
host: testHost,
host: "registry.example.com",
namespace: testNamespace,
username: "test-user",
password: "test-pass",
ecrTags: registryTags,
ecrAssumeRoleArn: roleArn,
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef2,
nextVersion: "20250630.2",
environmentSlug: "test",
projectRef: testProjectRef,
nextVersion: sameVersion,
environmentType: sameEnvironmentType,
deploymentShortCode: "test1234",
});

expect(imageRef2.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef2}:20250630.2.test`
);
expect(imageRef2.isEcr).toBe(true);
expect(imageRef2.repoCreated).toBe(false);
});

it("should reuse existing ECR repository", async () => {
// This should use the repository created in the previous test
const imageRef = await getDeploymentImageRef({
const secondImageRef = await getDeploymentImageRef({
registry: {
host: testHost,
host: "registry.example.com",
namespace: testNamespace,
username: "test-user",
password: "test-pass",
Expand All @@ -133,37 +183,23 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef",
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef,
nextVersion: "20250630.2",
environmentSlug: "prod",
nextVersion: sameVersion,
environmentType: sameEnvironmentType,
deploymentShortCode: "test4321",
});

expect(imageRef.imageRef).toBe(
`${testHost}/${testNamespace}/${testProjectRef}:20250630.2.prod`
// Even with the same environment type and version, the image refs should be different due to deployment short codes
expect(firstImageRef.imageRef).toBe(
`registry.example.com/${testNamespace}/${testProjectRef}:${sameVersion}.preview.test1234`
);
expect(imageRef.isEcr).toBe(true);
});

it("should throw error for invalid ECR host", async () => {
await expect(
getDeploymentImageRef({
registry: {
host: "invalid.ecr.amazonaws.com",
namespace: testNamespace,
username: "test-user",
password: "test-pass",
ecrTags: registryTags,
ecrAssumeRoleArn: roleArn,
ecrAssumeRoleExternalId: externalId,
},
projectRef: testProjectRef,
nextVersion: "20250630.1",
environmentSlug: "test",
})
).rejects.toThrow("Invalid ECR registry host: invalid.ecr.amazonaws.com");
expect(secondImageRef.imageRef).toBe(
`registry.example.com/${testNamespace}/${testProjectRef}:${sameVersion}.preview.test4321`
);
expect(firstImageRef.imageRef).not.toBe(secondImageRef.imageRef);
});
});

describe.skipIf(process.env.RUN_REGISTRY_AUTH_TESTS !== "1")("getEcrAuthToken", () => {
describe.skipIf(process.env.RUN_ECR_TESTS !== "1")("getEcrAuthToken", () => {
const testHost =
process.env.DEPLOY_REGISTRY_HOST || "123456789012.dkr.ecr.us-east-1.amazonaws.com";

Expand All @@ -188,8 +224,7 @@ describe.skipIf(process.env.RUN_REGISTRY_AUTH_TESTS !== "1")("getEcrAuthToken",
expect(auth.password.length).toBeGreaterThan(0);

// Verify the token format (should be a base64-encoded string)
const base64Regex = /^[A-Za-z0-9+/=]+$/;
expect(base64Regex.test(auth.password)).toBe(true);
expect(auth.password).toMatch(/^[A-Za-z0-9+/=]+$/);
});

it("should throw error for invalid region", async () => {
Expand Down
Loading