Revert "Revert "Add serve_directly option to assets configuration (#7316

)" (#…" (#7355) This reverts commit cc93b2c.
cloudflare · Nov 27, 2024 · 5928e8c · 5928e8c
1 parent 4ab073c
commit 5928e8c
Show file tree

Hide file tree

Showing 12 changed files with 143 additions and 2 deletions.
diff --git a/.changeset/six-coats-rule.md b/.changeset/six-coats-rule.md
@@ -0,0 +1,7 @@
+---
+"wrangler": minor
+---
+
+feat: add `serve_directly` option to Workers with Assets
+
+Users can now specify whether their assets are served directly against HTTP requests or whether these requests always go to the Worker, which can then respond with asset retrieved by its assets binding.
diff --git a/fixtures/asset-config/html-handling.test.ts b/fixtures/asset-config/html-handling.test.ts
@@ -64,6 +64,7 @@ describe.each(testSuites)("$title", ({ title, suite }) => {
 				return {
 					html_handling,
 					not_found_handling: "none",
+					serve_directly: true,
 				};
 			});
 		});

diff --git a/fixtures/asset-config/redirects.test.ts b/fixtures/asset-config/redirects.test.ts
@@ -20,7 +20,7 @@ const existsMock = (fileList: Set<string>) => {
 };
 const BASE_URL = "http://example.com";
 
-describe("[Asset Worker] `test url normalization`", () => {
+describe("[Asset Worker] `test location rewrite`", () => {
 	afterEach(() => {
 		vi.mocked(getAssetWithMetadataFromKV).mockRestore();
 	});
@@ -41,6 +41,7 @@ describe("[Asset Worker] `test url normalization`", () => {
 			return {
 				html_handling: "none",
 				not_found_handling: "none",
+				serve_directly: true,
 			};
 		});
 	});

diff --git a/fixtures/asset-config/url-normalization.test.ts b/fixtures/asset-config/url-normalization.test.ts
@@ -20,7 +20,7 @@ const existsMock = (fileList: Set<string>) => {
 };
 const BASE_URL = "http://example.com";
 
-describe("[Asset Worker] `test redirects`", () => {
+describe("[Asset Worker] `test slash normalization`", () => {
 	afterEach(() => {
 		vi.mocked(getAssetWithMetadataFromKV).mockRestore();
 	});
@@ -41,6 +41,7 @@ describe("[Asset Worker] `test redirects`", () => {
 			return {
 				html_handling: "none",
 				not_found_handling: "none",
+				serve_directly: true,
 			};
 		});
 	});

diff --git a/packages/workers-shared/asset-worker/src/configuration.ts b/packages/workers-shared/asset-worker/src/configuration.ts
@@ -6,5 +6,6 @@ export const applyConfigurationDefaults = (
 	return {
 		html_handling: configuration?.html_handling ?? "auto-trailing-slash",
 		not_found_handling: configuration?.not_found_handling ?? "none",
+		serve_directly: configuration?.serve_directly ?? true,
 	};
 };
diff --git a/packages/workers-shared/asset-worker/tests/handler.test.ts b/packages/workers-shared/asset-worker/tests/handler.test.ts
@@ -7,6 +7,7 @@ describe("[Asset Worker] `handleRequest`", () => {
 		const configuration: Required<AssetConfig> = {
 			html_handling: "none",
 			not_found_handling: "none",
+			serve_directly: true,
 		};
 		const eTag = "some-etag";
 		const exists = vi.fn().mockReturnValue(eTag);
@@ -29,6 +30,7 @@ describe("[Asset Worker] `handleRequest`", () => {
 		const configuration: Required<AssetConfig> = {
 			html_handling: "none",
 			not_found_handling: "none",
+			serve_directly: true,
 		};
 		const eTag = "some-etag";
 		const exists = vi.fn().mockReturnValue(eTag);
@@ -53,6 +55,7 @@ describe("[Asset Worker] `handleRequest`", () => {
 		const configuration: Required<AssetConfig> = {
 			html_handling: "none",
 			not_found_handling: "none",
+			serve_directly: true,
 		};
 		const eTag = "some-etag";
 		const exists = vi.fn().mockReturnValue(eTag);
@@ -77,6 +80,7 @@ describe("[Asset Worker] `handleRequest`", () => {
 		const configuration: Required<AssetConfig> = {
 			html_handling: "none",
 			not_found_handling: "none",
+			serve_directly: true,
 		};
 		const eTag = "some-etag";
 		const exists = vi.fn().mockReturnValue(eTag);

diff --git a/packages/workers-shared/utils/types.ts b/packages/workers-shared/utils/types.ts
@@ -17,6 +17,7 @@ export const AssetConfigSchema = z.object({
 	not_found_handling: z
 		.enum(["single-page-application", "404-page", "none"])
 		.optional(),
+	serve_directly: z.boolean().optional(),
 });
 
 export type RoutingConfig = z.infer<typeof RoutingConfigSchema>;

diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts
@@ -4948,6 +4948,118 @@ addEventListener('fetch', event => {});`
 			await runWrangler("deploy");
 		});
 
+		it("serve_directly correctly overrides default if set to false", async () => {
+			const assets = [
+				{ filePath: "file-1.txt", content: "Content of file-1" },
+				{ filePath: "boop/file-2.txt", content: "Content of file-2" },
+			];
+			writeAssets(assets);
+			writeWorkerSource({ format: "js" });
+			writeWranglerConfig({
+				main: "index.js",
+				compatibility_date: "2024-09-27",
+				compatibility_flags: ["nodejs_compat"],
+				assets: {
+					directory: "assets",
+					binding: "ASSETS",
+					html_handling: "none",
+					not_found_handling: "404-page",
+					serve_directly: false,
+				},
+			});
+			await mockAUSRequest();
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedAssets: {
+					jwt: "<<aus-completion-token>>",
+					config: {
+						html_handling: "none",
+						not_found_handling: "404-page",
+						serve_directly: false,
+					},
+				},
+				expectedBindings: [{ name: "ASSETS", type: "assets" }],
+				expectedMainModule: "index.js",
+				expectedCompatibilityDate: "2024-09-27",
+				expectedCompatibilityFlags: ["nodejs_compat"],
+			});
+			await runWrangler("deploy");
+		});
+
+		it("serve_directly omitted when not provided in config", async () => {
+			const assets = [
+				{ filePath: "file-1.txt", content: "Content of file-1" },
+				{ filePath: "boop/file-2.txt", content: "Content of file-2" },
+			];
+			writeAssets(assets);
+			writeWorkerSource({ format: "js" });
+			writeWranglerConfig({
+				main: "index.js",
+				compatibility_date: "2024-09-27",
+				compatibility_flags: ["nodejs_compat"],
+				assets: {
+					directory: "assets",
+					binding: "ASSETS",
+					html_handling: "none",
+					not_found_handling: "404-page",
+				},
+			});
+			await mockAUSRequest();
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedAssets: {
+					jwt: "<<aus-completion-token>>",
+					config: {
+						html_handling: "none",
+						not_found_handling: "404-page",
+					},
+				},
+				expectedBindings: [{ name: "ASSETS", type: "assets" }],
+				expectedMainModule: "index.js",
+				expectedCompatibilityDate: "2024-09-27",
+				expectedCompatibilityFlags: ["nodejs_compat"],
+			});
+			await runWrangler("deploy");
+		});
+
+		it("serve_directly provided if set to true", async () => {
+			const assets = [
+				{ filePath: "file-1.txt", content: "Content of file-1" },
+				{ filePath: "boop/file-2.txt", content: "Content of file-2" },
+			];
+			writeAssets(assets);
+			writeWorkerSource({ format: "js" });
+			writeWranglerConfig({
+				main: "index.js",
+				compatibility_date: "2024-09-27",
+				compatibility_flags: ["nodejs_compat"],
+				assets: {
+					directory: "assets",
+					binding: "ASSETS",
+					html_handling: "none",
+					not_found_handling: "404-page",
+					serve_directly: true,
+				},
+			});
+			await mockAUSRequest();
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedAssets: {
+					jwt: "<<aus-completion-token>>",
+					config: {
+						html_handling: "none",
+						not_found_handling: "404-page",
+						serve_directly: true,
+					},
+				},
+				expectedBindings: [{ name: "ASSETS", type: "assets" }],
+				expectedMainModule: "index.js",
+				expectedCompatibilityDate: "2024-09-27",
+				expectedCompatibilityFlags: ["nodejs_compat"],
+			});
+			await runWrangler("deploy");
+		});
+
 		it("should be able to upload an asset-only project", async () => {
 			const assets = [
 				{ filePath: "file-1.txt", content: "Content of file-1" },

diff --git a/packages/wrangler/src/assets.ts b/packages/wrangler/src/assets.ts
@@ -347,6 +347,7 @@ export function processAssetsArg(
 	const assetConfig = {
 		html_handling: config.assets?.html_handling,
 		not_found_handling: config.assets?.not_found_handling,
+		serve_directly: config.assets?.serve_directly,
 	};
 
 	return {

diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts
@@ -948,6 +948,7 @@ export type Assets = {
 		| "drop-trailing-slash"
 		| "none";
 	not_found_handling?: "single-page-application" | "404-page" | "none";
+	serve_directly?: boolean;
 };
 
 export interface Observability {

diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts
@@ -2168,12 +2168,22 @@ const validateAssetsConfig: ValidatorFn = (diagnostics, field, value) => {
 			["single-page-application", "404-page", "none"]
 		) && isValid;
 
+	isValid =
+		validateOptionalProperty(
+			diagnostics,
+			field,
+			"serve_directly",
+			(value as Assets).serve_directly,
+			"boolean"
+		) && isValid;
+
 	isValid =
 		validateAdditionalProperties(diagnostics, field, Object.keys(value), [
 			"directory",
 			"binding",
 			"html_handling",
 			"not_found_handling",
+			"serve_directly",
 		]) && isValid;
 
 	return isValid;

diff --git a/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts b/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts
@@ -194,6 +194,7 @@ export function createWorkerUploadForm(worker: CfWorkerInit): FormData {
 	const assetConfig = {
 		html_handling: assets?.assetConfig?.html_handling,
 		not_found_handling: assets?.assetConfig?.not_found_handling,
+		serve_directly: assets?.assetConfig?.serve_directly,
 	};
 
 	// short circuit if static assets upload only