Use zone_name for deployment of custom hostnames
#4232
Conversation
🦋 Changeset detectedLatest commit: fe6c83c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
I have created a changeset for "minor", however, this change could also be considered a "patch". |
|
@penalosa Could you please take a look on this change when you have a chance? |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6644228754/npm-package-wrangler-4232You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6644228754/npm-package-wrangler-4232Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6644228754/npm-package-wrangler-4232 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6644228754/npm-package-cloudflare-pages-shared-4232Note that these links will no longer work once the GitHub Actions artifact expires.
| Please ensure constraints are pinned, and |
packages/wrangler/src/zones.ts
Outdated
There was a problem hiding this comment.
@romeupalos thanks for this PR! Unfortunately I don't think this is the right fix for this issue, since we don't want to split the implementations of dev and deploy. I think a simpler option would be to change the implementation of getZoneForRoute to
export async function getZoneForRoute(route: Route): Promise<Zone | undefined> {
const host = getHostFromRoute(route);
let id: string | undefined;
if (typeof route === "object" && "zone_id" in route) {
id = route.zone_id;
} else if (typeof route === "object" && "zone_name" in route) {
id = await getZoneIdFromHost(route.zone_name);
} else if (host) {
id = await getZoneIdFromHost(host);
}
return id && host ? { id, host } : undefined;
}Does that work for your use case?
There was a problem hiding this comment.
@penalosa I've incorporated your code suggestion and updated the PR.
Please, let me know if it looks good to be merged.
Codecov Report
@@ Coverage Diff @@
## main #4232 +/- ##
==========================================
- Coverage 75.41% 75.37% -0.05%
==========================================
Files 223 223
Lines 12231 12292 +61
Branches 3161 3177 +16
==========================================
+ Hits 9224 9265 +41
- Misses 3007 3027 +20
|
romeupalos
force-pushed
the
romeupalos/3662-fix-saas-domains-deploy
branch
from
b14af00
to
3378f99
Compare
| @@ -369,22 +369,6 @@ describe("wrangler dev", () => { | |||
| expect(std.err).toMatchInlineSnapshot(`""`); | |||
| }); | |||
|
|
|||
| it("should fail for non-existing zones", async () => { | |||
There was a problem hiding this comment.
@penalosa
I've removed this UT, since it does not seem to be correct when having Cloudflare for SaaS product in mind.
Please, let me know if it is ok to remove it.
Fixes #3662
What this PR solves / how to test:
use
zone_nameto determine a zone when the pattern is a custom hostnameIn Cloudflare for SaaS, custom hostnames of third party domain owners can be used in Cloudflare.
Workers are allowed to intercept these requests based on the routes configuration.
Before this change, the same logic used by
wrangler devwas used inwrangler deploy, which caused wrangler to fail with:✘ [ERROR] Could not find zone for [partner-saas-domain.com]
Example of routes definition that wouldn't work before this PR:
Author has included the following, where applicable:
Reviewer is to perform the following, as applicable: