Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: wrangler deploy prompts warning with deployment #5992

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ import {
mswSuccessOauthHandlers,
mswSuccessUserHandlers,
} from "./helpers/msw";
import {
mswListNewDeploymentsLatestFiftyFifty,
mswListNewDeploymentsLatestFull,
} from "./helpers/msw/handlers/versions";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import { writeWorkerSource } from "./helpers/write-worker-source";
Expand Down Expand Up @@ -78,6 +82,7 @@ describe("deploy", () => {
setIsTTY(true);
mockLastDeploymentRequest();
mockDeploymentsListRequest();
msw.use(...mswListNewDeploymentsLatestFull);
logger.loggerLevel = "log";
});

Expand Down Expand Up @@ -423,6 +428,34 @@ describe("deploy", () => {
`);
});

it("should warn user when worker has deployment with multiple versions", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test that checks what happens if the user says yes?
Also what happens if the console is non-interactive? Can that be tested too?

msw.use(...mswListNewDeploymentsLatestFiftyFifty);
writeWranglerToml();
writeWorkerSource();
mockSubDomainRequest();
mockUploadWorkerRequest();
mockConfirm({
text: `"wrangler deploy" will upload a new version and deploy it globally immediately.\nAre you sure you want to continue?`,
result: false,
});

await runWrangler("deploy ./index");

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Your last deployment has multiple versions. To progress this deployment use \\"wrangler versions deploy\\" instead.

Currently deployed versions:

Version(s): (50%) test-name:version:0
Created: 2021-01-01T00:00:00.000Z

(50%) test-name:version:1
Created: 2021-01-01T00:00:00.000Z

"
`);
});

it("should warn user when additional properties are passed to a services config", async () => {
writeWranglerToml({
d1_databases: [
Expand Down
11 changes: 9 additions & 2 deletions packages/wrangler/src/__tests__/deprecated-usage-model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { mockConsoleMethods } from "./helpers/mock-console";
import { mockUploadWorkerRequest } from "./helpers/mock-upload-worker";
import { mockSubDomainRequest } from "./helpers/mock-workers-subdomain";
import { msw, mswSuccessDeploymentScriptMetadata } from "./helpers/msw";
import { mswListNewDeploymentsLatestFull } from "./helpers/msw/handlers/versions";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import { writeWorkerSource } from "./helpers/write-worker-source";
Expand All @@ -26,7 +27,10 @@ describe("deprecated-usage-model", () => {
});

it("should warn user about ignored usage model if usage_model specified", async () => {
msw.use(...mswSuccessDeploymentScriptMetadata);
msw.use(
...mswSuccessDeploymentScriptMetadata,
...mswListNewDeploymentsLatestFull
);
writeWranglerToml({ usage_model: "bundled" });
writeWorkerSource();
mockSubDomainRequest();
Expand All @@ -41,7 +45,10 @@ describe("deprecated-usage-model", () => {
`);
});
it("should not warn user about ignored usage model if usage_model not specified", async () => {
msw.use(...mswSuccessDeploymentScriptMetadata);
msw.use(
...mswSuccessDeploymentScriptMetadata,
...mswListNewDeploymentsLatestFull
);
writeWranglerToml();
writeWorkerSource();
mockSubDomainRequest();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,102 @@
import { http, HttpResponse } from "msw";
import { createFetchResult } from "../index";
import type { ApiDeployment, ApiVersion } from "../../../../versions/types";

export const mswListNewDeploymentsLatestFull = [
http.get(
"*/accounts/:accountId/workers/scripts/:scriptName/deployments",
({ params }) => {
return HttpResponse.json(
createFetchResult({
deployments: [
{
id: `deployment:${params["scriptName"]}`,
source: "api",
strategy: "percentage",
author_email: "[email protected]",
created_on: "2021-01-01T00:00:00.000000Z",
versions: [
{
version_id: `${params["scriptName"]}:version:0`,
percentage: 100.0,
},
],
},
] as Array<ApiDeployment>,
})
);
},
{ once: true }
),
http.get(
"*/accounts/:accountId/workers/scripts/:scriptName/versions/:version",
({ params }) => {
return HttpResponse.json(
createFetchResult({
id: params["version"],
number: 1,
metadata: {
created_on: "2021-01-01T00:00:00.000000Z",
modified_on: "2021-01-01T00:00:00.000000Z",
source: "api",
author_email: "[email protected]",
},
} as ApiVersion)
);
},
{ once: false }
),
];

export const mswListNewDeploymentsLatestFiftyFifty = [
http.get(
"*/accounts/:accountId/workers/scripts/:scriptName/deployments",
({ params }) => {
return HttpResponse.json(
createFetchResult({
deployments: [
{
id: `deployment:${params["scriptName"]}`,
source: "api",
strategy: "percentage",
author_email: "[email protected]",
created_on: "2021-01-01T00:00:00.000000Z",
versions: [
{
version_id: `${params["scriptName"]}:version:0`,
percentage: 50.0,
},
{
version_id: `${params["scriptName"]}:version:1`,
percentage: 50.0,
},
],
},
] as Array<ApiDeployment>,
})
);
},
{ once: true }
),
http.get(
"*/accounts/:accountId/workers/scripts/:scriptName/versions/:version",
({ params }) => {
return HttpResponse.json(
createFetchResult({
id: params["version"],
number: 1,
metadata: {
created_on: "2021-01-01T00:00:00.000000Z",
modified_on: "2021-01-01T00:00:00.000000Z",
source: "api",
author_email: "[email protected]",
},
} as ApiVersion)
);
},
{ once: false }
),
];

export const mswListNewDeployments = http.get(
"*/accounts/:accountId/workers/scripts/:workerName/deployments",
Expand Down
14 changes: 13 additions & 1 deletion packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import assert from "node:assert";
import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
import path from "node:path";
import { URLSearchParams } from "node:url";
import { cancel } from "@cloudflare/cli";
import { fetchListResult, fetchResult } from "../cfetch";
import { printBindings } from "../config";
import { bundleWorker } from "../deployment-bundle/bundle";
Expand Down Expand Up @@ -45,6 +46,7 @@ import {
} from "../sourcemap";
import triggersDeploy from "../triggers/deploy";
import { logVersionIdChange } from "../utils/deployment-id-version-id-change";
import { confirmLatestDeploymentOverwrite } from "../versions/deploy";
import { getZoneForRoute } from "../zones";
import type { Config } from "../config";
import type {
Expand Down Expand Up @@ -421,7 +423,8 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
const envName = props.env ?? "production";

const start = Date.now();
const notProd = Boolean(!props.legacyEnv && props.env);
const prod = Boolean(props.legacyEnv || !props.env);
const notProd = !prod;
const workerName = notProd ? `${scriptName} (${envName})` : scriptName;
const workerUrl = props.dispatchNamespace
? `/accounts/${accountId}/workers/dispatch/namespaces/${props.dispatchNamespace}/scripts/${scriptName}`
Expand All @@ -433,6 +436,14 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

const { format } = props.entry;

if (!props.dispatchNamespace && prod && accountId && scriptName) {
const yes = await confirmLatestDeploymentOverwrite(accountId, scriptName);
if (!yes) {
cancel("Aborting deploy...");
danielrs marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}

if (
!props.isWorkersSite &&
Boolean(props.assetPaths) &&
Expand Down Expand Up @@ -460,6 +471,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
"You cannot configure [data_blobs] with an ES module worker. Instead, import the file directly in your code, and optionally configure `[rules]` in your wrangler.toml"
);
}

try {
if (props.noBundle) {
// if we're not building, let's just copy the entry to the destination directory
Expand Down
66 changes: 66 additions & 0 deletions packages/wrangler/src/versions/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ import {
spinnerWhile,
} from "@cloudflare/cli/interactive";
import { findWranglerToml, readConfig } from "../config";
import { confirm } from "../dialogs";
import { UserError } from "../errors";
import { logger } from "../logger";
import * as metrics from "../metrics";
import { APIError } from "../parse";
import { printWranglerBanner } from "../update-check";
import { requireAuth } from "../user";
import formatLabelledValues from "../utils/render-labelled-values";
import {
createDeployment,
fetchDeployableVersions,
fetchLatestDeployment,
fetchLatestDeploymentVersions,
fetchVersions,
patchNonVersionedScriptSettings,
Expand Down Expand Up @@ -205,6 +209,68 @@ function getConfig(
return config;
}

/**
* Prompts the user for confirmation when overwriting the latest deployment, given that it's split.
*/
export async function confirmLatestDeploymentOverwrite(
accountId: string,
scriptName: string
) {
try {
const latest = await fetchLatestDeployment(accountId, scriptName);
if (latest && latest.versions.length >= 2) {
// Fetch the version details.

const versionCache: VersionCache = new Map();
const versionIds = latest.versions.map((v) => v.version_id);
await fetchVersions(accountId, scriptName, versionCache, ...versionIds);

// Format each version.

const formattedVersions = latest.versions.map((traffic) => {
const version = versionCache.get(traffic.version_id);
assert(version);
const percentage = brandColor(`(${traffic.percentage}%)`);
const details = formatLabelledValues(
{ Created: new Date(version.metadata["created_on"]).toISOString() },
{
indentationCount: 4,
labelJustification: "right",
formatLabel: (label) => gray(label + ":"),
formatValue: (value) => gray(value),
}
);
return `${percentage} ${version.id}\n${details}`;
});

// Format deployment.

const formattedDeployment = formatLabelledValues({
"Version(s)": formattedVersions.join("\n\n"),
});

// Print message and confirmation.
Copy link
Contributor

@RamIdeas RamIdeas Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this all be replaced with a call to printLatestDeployment (the function right below this one)?

And then the styling will remain consistent


logger.warn(
`Your last deployment has multiple versions. To progress this deployment use "wrangler versions deploy" instead.
Currently deployed versions:

${formattedDeployment}`
);

return await confirm(
`"wrangler deploy" will upload a new version and deploy it globally immediately.\nAre you sure you want to continue?`
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use inputPrompt instead of confirm please? There are other usages in this file you can use for reference

}
} catch (e) {
const isNotFound = e instanceof APIError && e.code == 10007;
if (!isNotFound) {
throw e;
}
}
return true;
}

export async function printLatestDeployment(
accountId: string,
workerName: string,
Expand Down
Loading