getAccountId should also check config.account_id (#10520)

* move config.account_id check into getAccountId

* add regression test for dry-run with container image uris

* changeset

Author: emily-shen

.changeset/short-monkeys-cough.md

@@ -0,0 +1,8 @@
+---
+"wrangler": patch
+---
+
+fix: wrangler deploy dry run should not require you to be logged in
+
+Fixes a bug where if you had a container where the image was an image registry link, dry run would require you to be logged in.
+Also fixes a bug where container deployments were not respecting `account_id` set in Wrangler config.

packages/wrangler/src/__tests__/containers/deploy.test.ts

@@ -1508,25 +1508,20 @@ describe("wrangler deploy with containers dry run", () => {
 	const cliStd = mockCLIOutput();
 	beforeEach(() => {
 		clearCachedAccount();
+		expect(process.env.CLOUDFLARE_API_TOKEN).toBeUndefined();
 	});
 
 	afterEach(() => {
 		vi.unstubAllEnvs();
 	});
 
 	it("builds the image without pushing", async () => {
-		// Reduced mock chain for dry run (no delete, modified push)
+		// Reduced mock chain for dry run (no delete, push)
 		vi.mocked(spawn)
 			.mockImplementationOnce(mockDockerInfo())
 			.mockImplementationOnce(
 				mockDockerBuild("my-container", "worker", "FROM scratch", process.cwd())
-			)
-			.mockImplementationOnce(
-				mockDockerImageInspectDigests("my-container", "worker")
-			)
-			.mockImplementationOnce(mockDockerLogin("mockpassword"))
-			.mockImplementationOnce(mockDockerPush("my-container", "worker"));
-
+			);
 		vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker");
 		fs.writeFileSync("./Dockerfile", "FROM scratch");
 		fs.writeFileSync(
@@ -1550,6 +1545,30 @@ describe("wrangler deploy with containers dry run", () => {
 		`);
 		expect(cliStd.stdout).toMatchInlineSnapshot(`""`);
 	});
+
+	it("builds the image without pushing", async () => {
+		// No docker mocks at all
+
+		fs.writeFileSync(
+			"index.js",
+			`export class ExampleDurableObject {}; export default{};`
+		);
+		writeWranglerConfig({
+			...DEFAULT_DURABLE_OBJECTS,
+			containers: [DEFAULT_CONTAINER_FROM_REGISTRY],
+		});
+
+		await runWrangler("deploy --dry-run index.js");
+		expect(std.out).toMatchInlineSnapshot(`
+			"Total Upload: xx KiB / gzip: xx KiB
+			Your Worker has access to the following bindings:
+			Binding                                            Resource
+			env.EXAMPLE_DO_BINDING (ExampleDurableObject)      Durable Object
+
+			--dry-run: exiting now."
+		`);
+		expect(cliStd.stdout).toMatchInlineSnapshot(`""`);
+	});
 });
 
 // Docker mock factory

packages/wrangler/src/cloudchamber/apply.ts

@@ -356,7 +356,7 @@ export async function apply(
 					`${config.name}-${appConfigNoDefaults.class_name}`
 			];
 
-		const accountId = config.account_id || (await getAccountId(config));
+		const accountId = await getAccountId(config);
 		const appConfig = containerAppToCreateApplication(
 			accountId,
 			appConfigNoDefaults,

packages/wrangler/src/cloudchamber/build.ts

@@ -280,7 +280,7 @@ export async function pushCommand(
 ) {
 	try {
 		await dockerLoginManagedRegistry(args.pathToDocker);
-		const accountId = config.account_id || (await getAccountId(config));
+		const accountId = await getAccountId(config);
 		const newTag = getCloudflareRegistryWithAccountNamespace(
 			accountId,
 			args.TAG

packages/wrangler/src/cloudchamber/images/images.ts

@@ -89,7 +89,7 @@ async function handleDeleteImageCommand(
 
 	const digest = await promiseSpinner(
 		getCreds().then(async (creds) => {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			const url = new URL(`https://${getCloudflareContainerRegistry()}`);
 			const baseUrl = `${url.protocol}//${url.host}`;
 			const [image, tag] = args.image.split(":");
@@ -126,7 +126,7 @@ async function handleListImagesCommand(
 		getCreds().then(async (creds) => {
 			const repos = await listReposWithTags(creds);
 			const processed: Repository[] = [];
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			const accountIdPrefix = new RegExp(`^${accountId}/`);
 			const filter = new RegExp(args.filter ?? "");
 			for (const [repo, tags] of Object.entries(repos)) {

packages/wrangler/src/containers/config.ts

@@ -25,6 +25,7 @@ export const getNormalizedContainerOptions = async (
 	args: {
 		/** set by args.containersRollout */
 		containersRollout?: "gradual" | "immediate";
+		dryRun?: boolean;
 	}
 ): Promise<ContainerNormalizedConfig[]> => {
 	if (!config.containers || config.containers.length === 0) {
@@ -145,11 +146,12 @@ export const getNormalizedContainerOptions = async (
 				image_vars: container.image_vars,
 			});
 		} else {
-			const accountId = await getAccountId(config);
 			normalizedContainers.push({
 				...shared,
 				...instanceTypeOrLimits,
-				image_uri: resolveImageName(accountId, container.image), // if it is not a dockerfile, it must be an image uri or have thrown an error
+				image_uri: args.dryRun
+					? container.image
+					: resolveImageName(await getAccountId(config), container.image), // if it is not a dockerfile, it must be an image uri or have thrown an error
 			});
 		}
 	}

packages/wrangler/src/secrets-store/commands.ts

@@ -77,7 +77,7 @@ export const secretsStoreStoreCreateCommand = createCommand({
 		let store: { id: string };
 		logger.log(`🔐 Creating store... (Name: ${args.name})`);
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			store = await createStore(config, accountId, { name: args.name });
 		} else {
 			throw new UserError(
@@ -112,7 +112,7 @@ export const secretsStoreStoreDeleteCommand = createCommand({
 	async handler(args, { config }) {
 		logger.log(`🔐 Deleting store... (Name: ${args.storeId})`);
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			await deleteStore(config, accountId, args.storeId);
 		} else {
 			throw new UserError(
@@ -158,7 +158,7 @@ export const secretsStoreStoreListCommand = createCommand({
 
 		let stores: Store[];
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			stores = await listStores(config, accountId, urlParams);
 		} else {
 			throw new UserError(
@@ -235,7 +235,7 @@ export const secretsStoreSecretListCommand = createCommand({
 
 		let secrets: Secret[];
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			secrets = await listSecrets(config, accountId, args.storeId, urlParams);
 		} else {
 			secrets = (
@@ -313,7 +313,7 @@ export const secretsStoreSecretGetCommand = createCommand({
 
 		let secret: Secret;
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			secret = await getSecret(config, accountId, args.storeId, args.secretId);
 		} else {
 			const name = await usingLocalSecretsStoreSecretAPI(
@@ -421,7 +421,7 @@ export const secretsStoreSecretCreateCommand = createCommand({
 
 		let secrets: Secret[];
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			secrets = await createSecret(config, accountId, args.storeId, {
 				name: args.name,
 				value: secretValue,
@@ -548,7 +548,7 @@ export const secretsStoreSecretUpdateCommand = createCommand({
 
 		let secret: Secret;
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			secret = await updateSecret(
 				config,
 				accountId,
@@ -632,7 +632,7 @@ export const secretsStoreSecretDeleteCommand = createCommand({
 		logger.log(`🔐 Deleting secret... (ID: ${args.secretId})`);
 
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			await deleteSecret(config, accountId, args.storeId, args.secretId);
 		} else {
 			await usingLocalSecretsStoreSecretAPI(
@@ -698,7 +698,7 @@ export const secretsStoreSecretDuplicateCommand = createCommand({
 
 		let duplicatedSecret: Secret;
 		if (args.remote) {
-			const accountId = config.account_id || (await getAccountId(config));
+			const accountId = await getAccountId(config);
 			duplicatedSecret = await duplicateSecret(
 				config,
 				accountId,

packages/wrangler/src/user/user.ts

@@ -1210,16 +1210,24 @@ export function listScopes(message = "💁 Available scopes:"): void {
 	// TODO: maybe a good idea to show usage here
 }
 
+/**
+ *
+ * Returns account_id preferentially from config.account_id > CLOUDFLARE_ACCOUNT_ID env var > cache > or user selection.
+ */
 export async function getAccountId(
-	complianceConfig: ComplianceConfig
+	config: ComplianceConfig & { account_id?: string }
 ): Promise<string> {
+	// TODO: v5 we should prioritise the env var instead of the config value here, for consistency
+	if (config.account_id) {
+		return config.account_id;
+	}
 	// check if we have a cached value
 	const cachedAccount = getAccountFromCache();
 	if (cachedAccount && !getCloudflareAccountIdFromEnv()) {
 		return cachedAccount.id;
 	}
 
-	const accounts = await getAccountChoices(complianceConfig);
+	const accounts = await getAccountChoices(config);
 	if (accounts.length === 1) {
 		saveAccountToCache({ id: accounts[0].id, name: accounts[0].name });
 		return accounts[0].id;
@@ -1272,7 +1280,7 @@ export async function requireAuth(
 			throw new UserError("Did not login, quitting...");
 		}
 	}
-	const accountId = config.account_id || (await getAccountId(config));
+	const accountId = await getAccountId(config);
 	if (!accountId) {
 		throw new UserError("No account id found, quitting...");
 	}