diff --git a/apps/webapp/app/v3/getDeploymentImageRef.server.ts b/apps/webapp/app/v3/getDeploymentImageRef.server.ts index e01f340a26..a456791e97 100644 --- a/apps/webapp/app/v3/getDeploymentImageRef.server.ts +++ b/apps/webapp/app/v3/getDeploymentImageRef.server.ts @@ -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 = { @@ -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 { diff --git a/apps/webapp/app/v3/services/initializeDeployment.server.ts b/apps/webapp/app/v3/services/initializeDeployment.server.ts index cdd174ed92..234cfa1270 100644 --- a/apps/webapp/app/v3/services/initializeDeployment.server.ts +++ b/apps/webapp/app/v3/services/initializeDeployment.server.ts @@ -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, }) ); @@ -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, diff --git a/apps/webapp/test/getDeploymentImageRef.test.ts b/apps/webapp/test/getDeploymentImageRef.test.ts index 2810c14435..a05236596f 100644 --- a/apps/webapp/test/getDeploymentImageRef.test.ts +++ b/apps/webapp/test/getDeploymentImageRef.test.ts @@ -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"; @@ -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; } @@ -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", @@ -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, @@ -87,20 +139,26 @@ 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", @@ -108,23 +166,15 @@ describe.skipIf(process.env.RUN_REGISTRY_TESTS !== "1")("getDeploymentImageRef", 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", @@ -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"; @@ -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 () => {