Merge pull request #271 from cloudflare/maximo/fixup-235

Unreverts #235 and don't automatically install wrangler when checking if it present

.changeset/brown-spies-grab.md

@@ -0,0 +1,5 @@
+---
+"wrangler-action": minor
+---
+
+This unreverts #235 ensuring wrangler-action will re-use existing wrangler installations, thanks @AdiRishi! and ensures we don't automatically install wrangler when checking if it present

.github/workflows/deploy.yml

@@ -29,7 +29,7 @@ jobs:
       - name: Only build app
         uses: ./
         with:
-          workingDirectory: "./test/base"
+          workingDirectory: "./test/only-build"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           command: deploy --dry-run
@@ -38,12 +38,11 @@ jobs:
         uses: ./
         with:
           quiet: true
-          workingDirectory: "./test/base"
+          workingDirectory: "./test/build-quiet"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           command: deploy --dry-run
 
-      # START Setup and teardown of Worker Environment Tests
       - name: Environment support
         uses: ./
         with:
@@ -52,6 +51,7 @@ jobs:
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           environment: dev
           preCommands: npx wrangler deploy --env dev # https://github.com/cloudflare/wrangler-action/issues/162
+          postCommands: npx wrangler delete --name wrangler-action-dev-environment-test --force
           secrets: |
             SECRET1
             SECRET2
@@ -59,21 +59,12 @@ jobs:
           SECRET1: ${{ secrets.SECRET1 }}
           SECRET2: ${{ secrets.SECRET2 }}
 
-      - name: Clean up Deployed Environment Worker
-        uses: ./
-        with:
-          workingDirectory: "./test/base"
-          apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
-          accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
-          command: delete --name wrangler-action-dev-environment-test --force
-
-      # END Setup and teardown of Worker Environment Tests
       # START Setup and teardown of Workers w/ Secrets Tests
       - name: Deploy app secrets w/ hardcoded Wrangler v2
         uses: ./
         with:
           wranglerVersion: "2.20.0"
-          workingDirectory: "./test/base"
+          workingDirectory: "./test/secrets-v2"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           secrets: |
@@ -84,13 +75,13 @@ jobs:
           SECRET2: ${{ secrets.SECRET2 }}
 
       - name: Health Check Deployed Worker
-        run: node .github/workflows/workerHealthCheck.cjs
+        run: node .github/workflows/workerHealthCheck.cjs wrangler-action-test-secrets-v2
         shell: bash
 
       - name: Deploy app secrets w/ default version
         uses: ./
         with:
-          workingDirectory: "./test/base"
+          workingDirectory: "./test/secrets-default"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           secrets: |
@@ -101,22 +92,23 @@ jobs:
           SECRET2: ${{ secrets.SECRET2 }}
 
       - name: Health Check Deployed Worker
-        run: node .github/workflows/workerHealthCheck.cjs
+        run: node .github/workflows/workerHealthCheck.cjs wrangler-action-test-secrets-default
         shell: bash
 
       - name: Clean Up Deployed Workers
         uses: ./
         with:
-          workingDirectory: "./test/base"
+          workingDirectory: "./test/secrets-default"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
-          command: delete --name wrangler-action-test --force
+          command: delete --name wrangler-action-test-secrets-v2 --force
+          postCommands: npx wrangler delete --name wrangler-action-test-secrets-default --force
       # END Setup and teardown of Workers w/ Secrets Tests
 
       - name: Support packageManager variable
         uses: ./
         with:
-          workingDirectory: "./test/empty"
+          workingDirectory: "./test/specify-package-manager"
           packageManager: "npm"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
@@ -125,7 +117,7 @@ jobs:
       - name: Support unspecified packageManager with no lockfile
         uses: ./
         with:
-          workingDirectory: "./test/empty"
+          workingDirectory: "./test/unspecified-package-manager"
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           command: deploy --dry-run
@@ -159,3 +151,14 @@ jobs:
           apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           command: deploy --dry-run
+
+      - name: Change directory to pre-installed-wrangler and install dependencies
+        run: |
+          cd ./test/pre-installed-wrangler
+          npm install
+
+      - name: Support pre-installed wrangler
+        uses: ./
+        with:
+          workingDirectory: "./test/pre-installed-wrangler"
+          command: action-test

.github/workflows/workerHealthCheck.cjs

@@ -1,8 +1,7 @@
 const { execSync } = require("child_process");
 
-function workerHealthCheck() {
+function workerHealthCheck(workerName) {
-	const url =
+	const url = `https://${workerName}.devprod-testing7928.workers.dev/secret-health-check`;
-		"https://wrangler-action-test.devprod-testing7928.workers.dev/secret-health-check";
 
 	const buffer = execSync(`curl ${url}`);
 
@@ -17,4 +16,13 @@ function workerHealthCheck() {
 	return response;
 }
 
-workerHealthCheck();
+const args = Array.from(process.argv);
+const workerName = args.pop();
+
+if (!workerName) {
+	throw new Error(
+		"Please provide the worker name as an argument when calling this program.",
+	);
+}
+
+workerHealthCheck(workerName);

package-lock.json

@@ -1,12 +1,12 @@
 {
 	"name": "wrangler-action",
-	"version": "3.5.0",
+	"version": "3.6.1",
 	"lockfileVersion": 3,
 	"requires": true,
 	"packages": {
 		"": {
 			"name": "wrangler-action",
-			"version": "3.5.0",
+			"version": "3.6.1",
 			"license": "MIT OR Apache-2.0",
 			"dependencies": {
 				"@actions/core": "^1.10.1",

src/index.ts

@@ -10,6 +10,8 @@ import {
 	setFailed,
 	setOutput,
 } from "@actions/core";
+import { getExecOutput } from "@actions/exec";
+import semverEq from "semver/functions/eq";
 import { exec, execShell } from "./exec";
 import { checkWorkingDirectory, semverCompare } from "./utils";
 import { getPackageManager } from "./packageManagers";
@@ -21,6 +23,7 @@ const DEFAULT_WRANGLER_VERSION = "3.13.2";
  */
 const config = {
 	WRANGLER_VERSION: getInput("wranglerVersion") || DEFAULT_WRANGLER_VERSION,
+	didUserProvideWranglerVersion: Boolean(getInput("wranglerVersion")),
 	secrets: getMultilineInput("secrets"),
 	workingDirectory: checkWorkingDirectory(getInput("workingDirectory")),
 	CLOUDFLARE_API_TOKEN: getInput("apiToken"),
@@ -82,6 +85,65 @@ async function installWrangler() {
 		);
 	}
 
+	startGroup("🔍 Checking for existing Wrangler installation");
+	let installedVersion = "";
+	let installedVersionSatisfiesRequirement = false;
+	try {
+		const { stdout } = await getExecOutput(
+			// We want to simply invoke wrangler to check if it's installed, but don't want to auto-install it at this stage
+			packageManager.execNoInstall,
+			["wrangler", "--version"],
+			{
+				cwd: config["workingDirectory"],
+				silent: config.QUIET_MODE,
+			},
+		);
+		// There are two possible outputs from `wrangler --version`:
+		// ` ⛅️ wrangler 3.48.0 (update available 3.53.1)`
+		// and
+		// `3.48.0`
+		const versionMatch =
+			stdout.match(/wrangler (\d+\.\d+\.\d+)/) ??
+			stdout.match(/^(\d+\.\d+\.\d+)/);
+		if (versionMatch) {
+			installedVersion = versionMatch[1];
+		}
+		if (config.didUserProvideWranglerVersion) {
+			installedVersionSatisfiesRequirement = semverEq(
+				installedVersion,
+				config["WRANGLER_VERSION"],
+			);
+		}
+		if (!config.didUserProvideWranglerVersion && installedVersion) {
+			info(
+				`✅ No wrangler version specified, using pre-installed wrangler version ${installedVersion}`,
+				true,
+			);
+			endGroup();
+			return;
+		}
+		if (
+			config.didUserProvideWranglerVersion &&
+			installedVersionSatisfiesRequirement
+		) {
+			info(`✅ Using Wrangler ${installedVersion}`, true);
+			endGroup();
+			return;
+		}
+		info(
+			"⚠️ Wrangler not found or version is incompatible. Installing...",
+			true,
+		);
+	} catch (error) {
+		debug(`Error checking Wrangler version: ${error}`);
+		info(
+			"⚠️ Wrangler not found or version is incompatible. Installing...",
+			true,
+		);
+	} finally {
+		endGroup();
+	}
+
 	startGroup("📥 Installing Wrangler");
 	try {
 		await exec(

src/packageManagers.test.ts

@@ -7,22 +7,25 @@ describe("getPackageManager", () => {
 			.toMatchInlineSnapshot(`
 				{
 				  "exec": "npx",
+				  "execNoInstall": "npx --no-install",
 				  "install": "npm i",
 				}
 			`);
 
 		expect(getPackageManager("yarn", { workingDirectory: "test/npm" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "yarn",
+				  "exec": "yarn",
-			  "install": "yarn add",
+				  "execNoInstall": "yarn",
-			}
+				  "install": "yarn add",
-		`);
+				}
+			`);
 
 		expect(getPackageManager("pnpm", { workingDirectory: "test/npm" }))
 			.toMatchInlineSnapshot(`
 				{
 				  "exec": "pnpm exec",
+				  "execNoInstall": "pnpm exec",
 				  "install": "pnpm add",
 				}
 			`);
@@ -31,6 +34,7 @@ describe("getPackageManager", () => {
 			.toMatchInlineSnapshot(`
 				{
 				  "exec": "bunx",
+				  "execNoInstall": "bun run",
 				  "install": "bun i",
 				}
 			`);
@@ -39,51 +43,56 @@ describe("getPackageManager", () => {
 	test("should use npm if no value provided and package-lock.json exists", () => {
 		expect(getPackageManager("", { workingDirectory: "test/npm" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "npx",
+				  "exec": "npx",
-			  "install": "npm i",
+				  "execNoInstall": "npx --no-install",
-			}
+				  "install": "npm i",
-		`);
+				}
+			`);
 	});
 
 	test("should use yarn if no value provided and yarn.lock exists", () => {
 		expect(getPackageManager("", { workingDirectory: "test/yarn" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "yarn",
+				  "exec": "yarn",
-			  "install": "yarn add",
+				  "execNoInstall": "yarn",
-			}
+				  "install": "yarn add",
-		`);
+				}
+			`);
 	});
 
 	test("should use pnpm if no value provided and pnpm-lock.yaml exists", () => {
 		expect(getPackageManager("", { workingDirectory: "test/pnpm" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "pnpm exec",
+				  "exec": "pnpm exec",
-			  "install": "pnpm add",
+				  "execNoInstall": "pnpm exec",
-			}
+				  "install": "pnpm add",
-		`);
+				}
+			`);
 	});
 
 	test("should use bun if no value provided and bun.lockb exists", () => {
 		expect(getPackageManager("", { workingDirectory: "test/bun" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "bunx",
+				  "exec": "bunx",
-			  "install": "bun i",
+				  "execNoInstall": "bun run",
-			}
+				  "install": "bun i",
+				}
 			`);
 	});
 
 	test("should use npm if no value provided and no lockfile is present", () => {
 		expect(getPackageManager("", { workingDirectory: "test/empty" }))
 			.toMatchInlineSnapshot(`
-			{
+				{
-			  "exec": "npx",
+				  "exec": "npx",
-			  "install": "npm i",
+				  "execNoInstall": "npx --no-install",
-			}
+				  "install": "npm i",
-		`);
+				}
+			`);
 	});
 
 	test("should throw if an invalid value is provided", () => {

src/packageManagers.ts

@@ -4,24 +4,29 @@ import * as path from "node:path";
 interface PackageManager {
 	install: string;
 	exec: string;
+	execNoInstall: string;
 }
 
 const PACKAGE_MANAGERS = {
 	npm: {
 		install: "npm i",
 		exec: "npx",
+		execNoInstall: "npx --no-install",
 	},
 	yarn: {
 		install: "yarn add",
 		exec: "yarn",
+		execNoInstall: "yarn",
 	},
 	pnpm: {
 		install: "pnpm add",
 		exec: "pnpm exec",
+		execNoInstall: "pnpm exec",
 	},
 	bun: {
 		install: "bun i",
 		exec: "bunx",
+		execNoInstall: "bun run",
 	},
 } as const satisfies Readonly<Record<string, PackageManager>>;
 

test/build-quiet/index.ts

@@ -17,7 +17,6 @@ export default {
 			return new Response("OK");
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),

test/build-quiet/wrangler.toml

@@ -0,0 +1,4 @@
+name = "wrangler-action-test-build-quiet"
+main = "./index.ts"
+compatibility_date = "2023-07-07"
+workers_dev = true

test/bun/index.ts

@@ -17,7 +17,6 @@ export default {
 			return new Response("OK");
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),

test/environment/index.ts

@@ -12,7 +12,6 @@ export default {
 			return new Response(`${SECRET1} ${SECRET2}`);
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),

test/npm/index.ts

@@ -17,7 +17,6 @@ export default {
 			return new Response("OK");
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),

test/only-build/index.ts

@@ -0,0 +1,25 @@
+type Env = {
+	SECRET1?: string;
+	SECRET2?: string;
+};
+
+export default {
+	fetch(request: Request, env: Env) {
+		const url = new URL(request.url);
+
+		if (url.pathname === "/secret-health-check") {
+			const { SECRET1, SECRET2 } = env;
+
+			if (SECRET1 !== "SECRET_1_VALUE" || SECRET2 !== "SECRET_2_VALUE") {
+				throw new Error("SECRET1 or SECRET2 is not defined");
+			}
+
+			return new Response("OK");
+		}
+
+		return Response.json({
+			...request,
+			headers: Object.fromEntries(request.headers),
+		});
+	},
+};

test/only-build/package-lock.json

@@ -0,0 +1,10 @@
+{
+	"name": "wrangler-action-test",
+	"lockfileVersion": 3,
+	"requires": true,
+	"packages": {
+		"": {
+			"name": "wrangler-action-test"
+		}
+	}
+}

test/only-build/package.json

@@ -0,0 +1,5 @@
+{
+	"name": "wrangler-action-test",
+	"license": "MIT",
+	"private": true
+}

test/only-build/wrangler.toml

@@ -1,4 +1,4 @@
-name = "wrangler-action-test"
+name = "wrangler-action-test-only-build"
 main = "./index.ts"
 compatibility_date = "2023-07-07"
 workers_dev = true

test/pnpm/index.ts

@@ -17,7 +17,6 @@ export default {
 			return new Response("OK");
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),

test/pre-installed-wrangler/index.ts

@@ -0,0 +1,25 @@
+type Env = {
+	SECRET1?: string;
+	SECRET2?: string;
+};
+
+export default {
+	fetch(request: Request, env: Env) {
+		const url = new URL(request.url);
+
+		if (url.pathname === "/secret-health-check") {
+			const { SECRET1, SECRET2 } = env;
+
+			if (SECRET1 !== "SECRET_1_VALUE" || SECRET2 !== "SECRET_2_VALUE") {
+				throw new Error("SECRET1 or SECRET2 is not defined");
+			}
+
+			return new Response("OK");
+		}
+
+		return Response.json({
+			...request,
+			headers: Object.fromEntries(request.headers),
+		});
+	},
+};

test/pre-installed-wrangler/mock_packages/wrangler/index.js

@@ -0,0 +1,18 @@
+#!/usr/bin/env node
+"use strict";
+
+const args = Array.from(process.argv);
+const command = args.pop();
+switch (command) {
+	case "--version":
+		console.log(`
+⛅️ wrangler 1.1.1 (update available 1.2.3)
+------------------------------------------`);
+		process.exit(0);
+	case "action-test":
+		console.log("Test successful.");
+		process.exit(0);
+	default:
+		console.error("Invalid command");
+		process.exit(1);
+}

test/pre-installed-wrangler/mock_packages/wrangler/package.json

@@ -0,0 +1,7 @@
+{
+	"private": true,
+	"name": "wrangler",
+	"version": "1.1.1",
+	"main": "index.js",
+	"bin": "index.js"
+}

test/pre-installed-wrangler/package-lock.json

@@ -0,0 +1,30 @@
+{
+	"name": "wrangler-action-pre-installed-wrangler-test",
+	"lockfileVersion": 2,
+	"requires": true,
+	"packages": {
+		"": {
+			"name": "wrangler-action-pre-installed-wrangler-test",
+			"license": "MIT",
+			"devDependencies": {
+				"wrangler": "file:mock_packages/wrangler"
+			}
+		},
+		"mock_packages/wrangler": {
+			"version": "1.1.1",
+			"dev": true,
+			"bin": {
+				"wrangler": "index.js"
+			}
+		},
+		"node_modules/wrangler": {
+			"resolved": "mock_packages/wrangler",
+			"link": true
+		}
+	},
+	"dependencies": {
+		"wrangler": {
+			"version": "file:mock_packages/wrangler"
+		}
+	}
+}

test/pre-installed-wrangler/package.json

@@ -0,0 +1,8 @@
+{
+	"name": "wrangler-action-pre-installed-wrangler-test",
+	"license": "MIT",
+	"private": true,
+	"devDependencies": {
+		"wrangler": "file:mock_packages/wrangler"
+	}
+}

test/secrets-default/index.ts

@@ -0,0 +1,25 @@
+type Env = {
+	SECRET1?: string;
+	SECRET2?: string;
+};
+
+export default {
+	fetch(request: Request, env: Env) {
+		const url = new URL(request.url);
+
+		if (url.pathname === "/secret-health-check") {
+			const { SECRET1, SECRET2 } = env;
+
+			if (SECRET1 !== "SECRET_1_VALUE" || SECRET2 !== "SECRET_2_VALUE") {
+				throw new Error("SECRET1 or SECRET2 is not defined");
+			}
+
+			return new Response("OK");
+		}
+
+		return Response.json({
+			...request,
+			headers: Object.fromEntries(request.headers),
+		});
+	},
+};

test/secrets-default/package-lock.json

@@ -0,0 +1,10 @@
+{
+	"name": "wrangler-action-test",
+	"lockfileVersion": 3,
+	"requires": true,
+	"packages": {
+		"": {
+			"name": "wrangler-action-test"
+		}
+	}
+}

test/secrets-default/package.json

@@ -0,0 +1,5 @@
+{
+	"name": "wrangler-action-test",
+	"license": "MIT",
+	"private": true
+}

test/secrets-default/wrangler.toml

@@ -0,0 +1,4 @@
+name = "wrangler-action-test-secrets-default"
+main = "./index.ts"
+compatibility_date = "2023-07-07"
+workers_dev = true

test/secrets-v2/index.ts

@@ -0,0 +1,25 @@
+type Env = {
+	SECRET1?: string;
+	SECRET2?: string;
+};
+
+export default {
+	fetch(request: Request, env: Env) {
+		const url = new URL(request.url);
+
+		if (url.pathname === "/secret-health-check") {
+			const { SECRET1, SECRET2 } = env;
+
+			if (SECRET1 !== "SECRET_1_VALUE" || SECRET2 !== "SECRET_2_VALUE") {
+				throw new Error("SECRET1 or SECRET2 is not defined");
+			}
+
+			return new Response("OK");
+		}
+
+		return Response.json({
+			...request,
+			headers: Object.fromEntries(request.headers),
+		});
+	},
+};

test/secrets-v2/package-lock.json

@@ -0,0 +1,10 @@
+{
+	"name": "wrangler-action-test",
+	"lockfileVersion": 3,
+	"requires": true,
+	"packages": {
+		"": {
+			"name": "wrangler-action-test"
+		}
+	}
+}

test/secrets-v2/package.json

@@ -0,0 +1,5 @@
+{
+	"name": "wrangler-action-test",
+	"license": "MIT",
+	"private": true
+}

test/secrets-v2/wrangler.toml

@@ -1,4 +1,4 @@
-name = "wrangler-action-test"
+name = "wrangler-action-test-secrets-v2"
 main = "./index.ts"
 compatibility_date = "2023-07-07"
 workers_dev = true

test/specify-package-manager/wrangler.toml

@@ -0,0 +1,4 @@
+name = "wrangler-action-test-specify-package-manager"
+main = "./index.ts"
+compatibility_date = "2023-07-07"
+workers_dev = true

test/unspecified-package-manager/index.ts

@@ -0,0 +1 @@
+export default {};

test/unspecified-package-manager/package.json

@@ -0,0 +1,5 @@
+{
+	"name": "wrangler-action-detect-package-manager-test",
+	"license": "MIT",
+	"private": true
+}

test/unspecified-package-manager/wrangler.toml

@@ -0,0 +1,4 @@
+name = "wrangler-action-test-unspecified-package-manager"
+main = "./index.ts"
+compatibility_date = "2023-07-07"
+workers_dev = true

test/yarn/index.ts

@@ -17,7 +17,6 @@ export default {
 			return new Response("OK");
 		}
 
-		// @ts-expect-error
 		return Response.json({
 			...request,
 			headers: Object.fromEntries(request.headers),