From ddfdc7a213e3a4fadb3ad6fa05456e3b5a383b8e Mon Sep 17 00:00:00 2001 From: yomna Date: Sun, 30 Aug 2026 15:17:24 -0400 Subject: [PATCH] Avoid custom domain prompt for same Worker --- .../deploy-helpers/src/triggers/deploy.ts | 1 + .../src/triggers/publish-routes.ts | 24 ++-- .../tests/publish-custom-domains.test.ts | 135 ++++++++++++++++++ .../src/__tests__/deploy/routes.test.ts | 63 ++++++++ 4 files changed, 214 insertions(+), 9 deletions(-) create mode 100644 packages/deploy-helpers/tests/publish-custom-domains.test.ts diff --git a/packages/deploy-helpers/src/triggers/deploy.ts b/packages/deploy-helpers/src/triggers/deploy.ts index 724220fe7e0..56d2e6aac1f 100644 --- a/packages/deploy-helpers/src/triggers/deploy.ts +++ b/packages/deploy-helpers/src/triggers/deploy.ts @@ -217,6 +217,7 @@ export async function triggersDeploy( config, workerUrl, accountId, + scriptName, customDomainsOnly ).then( (result) => ({ ...result, category: "Custom domains" }), diff --git a/packages/deploy-helpers/src/triggers/publish-routes.ts b/packages/deploy-helpers/src/triggers/publish-routes.ts index 8910a6f085d..f9c959e17b5 100644 --- a/packages/deploy-helpers/src/triggers/publish-routes.ts +++ b/packages/deploy-helpers/src/triggers/publish-routes.ts @@ -254,6 +254,7 @@ export async function publishCustomDomains( complianceConfig: ComplianceConfig, workerUrl: string, accountId: string, + scriptName: string, domains: Array ): Promise { const options = { @@ -314,17 +315,22 @@ export async function publishCustomDomains( ) ) ); - const existingRendered = existing - .map( - (domain) => - `\t• ${domain.hostname} (used as a domain for "${domain.service}")` - ) - .join("\n"); - const message = `Custom Domains already exist for these domains: + const existingForOtherWorkers = existing.filter( + (domain) => domain.service !== scriptName + ); + if (existingForOtherWorkers.length > 0) { + const existingRendered = existingForOtherWorkers + .map( + (domain) => + `\t• ${domain.hostname} (used as a domain for "${domain.service}")` + ) + .join("\n"); + const message = `Custom Domains already exist for these domains: ${existingRendered} Update them to point to this script instead?`; - if (!(await confirm(message))) { - return fail(); + if (!(await confirm(message))) { + return fail(); + } } options.override_existing_origin = true; } diff --git a/packages/deploy-helpers/tests/publish-custom-domains.test.ts b/packages/deploy-helpers/tests/publish-custom-domains.test.ts new file mode 100644 index 00000000000..7c7a508a983 --- /dev/null +++ b/packages/deploy-helpers/tests/publish-custom-domains.test.ts @@ -0,0 +1,135 @@ +import { afterEach, beforeEach, describe, it } from "vitest"; +import { initDeployHelpersContext } from "../src/shared/context"; +import { publishCustomDomains } from "../src/triggers/publish-routes"; +import type { CustomDomainChangeset } from "../src/triggers/publish-routes"; +import type { ComplianceConfig } from "@cloudflare/workers-utils"; + +const ACCOUNT_ID = "some-account-id"; +const SCRIPT_NAME = "test-name"; +const WORKER_URL = `/accounts/${ACCOUNT_ID}/workers/scripts/${SCRIPT_NAME}`; + +describe("publishCustomDomains", () => { + const originalStdoutIsTTY = process.stdout.isTTY; + let confirmRequests: number; + let publishedBody: unknown; + + beforeEach(() => { + confirmRequests = 0; + publishedBody = undefined; + Object.defineProperty(process.stdout, "isTTY", { + value: true, + configurable: true, + }); + + initDeployHelpersContext({ + logger: { + debug() {}, + error() {}, + info() {}, + log() {}, + warn() {}, + }, + fetchResult: fetchResult as never, + fetchListResult: (() => {}) as never, + fetchPagedListResult: (() => {}) as never, + fetchKVGetValue: (() => {}) as never, + confirm: async () => { + confirmRequests++; + return true; + }, + prompt: (() => {}) as never, + select: (() => {}) as never, + }); + }); + + afterEach(() => { + Object.defineProperty(process.stdout, "isTTY", { + value: originalStdoutIsTTY, + configurable: true, + }); + }); + + async function fetchResult( + _config: ComplianceConfig, + path: string, + init?: RequestInit + ): Promise { + const body = + typeof init?.body === "string" ? JSON.parse(init.body) : undefined; + + if (path === `${WORKER_URL}/domains/changeset?replace_state=true`) { + return { + added: [], + removed: [], + updated: [ + { + id: "101", + zone_id: "", + zone_name: "", + hostname: "api.example.com", + service: SCRIPT_NAME, + environment: "", + enabled: true, + previews_enabled: false, + modified: true, + }, + ], + conflicting: [], + } satisfies CustomDomainChangeset; + } + + if (path === `/accounts/${ACCOUNT_ID}/workers/domains/records/101`) { + return { + id: "101", + zone_id: "", + zone_name: "", + hostname: "api.example.com", + service: SCRIPT_NAME, + environment: "", + enabled: true, + previews_enabled: false, + }; + } + + if (path === `${WORKER_URL}/domains/records`) { + publishedBody = body; + return null; + } + + throw new Error(`Unexpected request: ${init?.method ?? "GET"} ${path}`); + } + + it("updates a domain already attached to this Worker without prompting", async ({ + expect, + }) => { + const result = await publishCustomDomains( + {} as ComplianceConfig, + WORKER_URL, + ACCOUNT_ID, + SCRIPT_NAME, + [ + { + pattern: "api.example.com", + custom_domain: true, + previews_enabled: true, + }, + ] + ); + + expect(confirmRequests).toBe(0); + expect(publishedBody).toEqual({ + override_scope: true, + override_existing_origin: true, + override_existing_dns_record: false, + origins: [ + { + hostname: "api.example.com", + previews_enabled: true, + }, + ], + }); + expect(result.targets).toEqual([ + "api.example.com (custom domain) [previews: enabled]", + ]); + }); +}); diff --git a/packages/wrangler/src/__tests__/deploy/routes.test.ts b/packages/wrangler/src/__tests__/deploy/routes.test.ts index 91f99cbe735..a1b7ab0b249 100644 --- a/packages/wrangler/src/__tests__/deploy/routes.test.ts +++ b/packages/wrangler/src/__tests__/deploy/routes.test.ts @@ -561,6 +561,69 @@ Update them to point to this script instead?`, expect(std.out).toContain("api.example.com (custom domain)"); }); + it("should not confirm override if custom domain already belongs to this Worker", async ({ + expect, + }) => { + writeWranglerConfig({ + routes: [ + { + pattern: "api.example.com", + custom_domain: true, + previews_enabled: true, + }, + ], + }); + writeWorkerSource(); + mockUpdateWorkerSubdomain({ enabled: false }); + mockUploadWorkerRequest({ expectedType: "esm" }); + mockGetZones(expect, "api.example.com", [{ id: "api-example-com-id" }]); + mockGetZoneWorkerRoutes(expect, "api-example-com-id", []); + mockCustomDomainsChangesetRequest({ + originConflicts: [ + { + id: "101", + zone_id: "", + zone_name: "", + hostname: "api.example.com", + service: "test-name", + environment: "", + enabled: true, + previews_enabled: false, + }, + ], + }); + mockCustomDomainLookup({ + id: "101", + zone_id: "", + zone_name: "", + hostname: "api.example.com", + service: "test-name", + environment: "", + enabled: true, + previews_enabled: false, + }); + mockPublishCustomDomainsRequest({ + publishFlags: { + override_scope: true, + override_existing_origin: true, + override_existing_dns_record: false, + }, + domains: [ + { + hostname: "api.example.com", + previews_enabled: true, + }, + ], + }); + + await runWrangler("deploy ./index"); + + expect(std.out).toContain( + "api.example.com (custom domain) [previews: enabled]" + ); + expect(std.out).not.toContain("Custom Domains already exist"); + }); + it("should confirm override if custom domain deploy contains a conflicting DNS record", async ({ expect, }) => {