Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/silver-pears-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Fix D1 remote database resolution to fall back to `database_name` when a binding in config does not have a `database_id`. This allows commands like `wrangler d1 migrations apply --remote` to work after [deploy-time auto provisioning](https://developers.cloudflare.com/workers/wrangler/configuration/#automatic-provisioning).
75 changes: 75 additions & 0 deletions packages/wrangler/src/__tests__/d1/migrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,81 @@ describe("migrate", () => {
describe("apply", () => {
mockAccountId({ accountId: null });
mockApiToken();

it("should apply remote migrations when config has database_name but no database_id", async ({
expect,
}) => {
setIsTTY(false);
mockGetMemberships([
{ id: "IG-88", account: { id: "1701", name: "enterprise" } },
]);

writeWranglerConfig({
d1_databases: [{ binding: "DATABASE", database_name: "db" }],
});

const seenDatabaseIds: string[] = [];

msw.use(
http.get("*/accounts/:accountId/d1/database", async () => {
return HttpResponse.json(
{
result: [
{
file_size: 7421952,
name: "db",
num_tables: 2,
uuid: "resolved-db-id",
version: "production",
},
],
success: true,
errors: [],
messages: [],
},
{ status: 200 }
);
}),
http.post(
"*/accounts/:accountId/d1/database/:databaseId/query",
async ({ params }) => {
seenDatabaseIds.push(params.databaseId as string);
return HttpResponse.json(
{
result: [
{
results: [],
success: true,
meta: {},
},
],
success: true,
errors: [],
messages: [],
},
{ status: 200 }
);
}
)
);

mockConfirm({
text: `No migrations folder found. Set \`migrations_dir\` in your wrangler.toml file to choose a different path.
Ok to create <cwd>/migrations?`,
result: true,
});
await runWrangler("d1 migrations create DATABASE test");

mockConfirm({
text: `About to apply 1 migration(s)
Your database may not be available to serve requests during the migration, continue?`,
result: true,
});

await runWrangler("d1 migrations apply DATABASE --remote");
expect(seenDatabaseIds).toContain("resolved-db-id");
});

it("should not attempt to login in local mode", async ({ expect }) => {
setIsTTY(false);
writeWranglerConfig({
Expand Down
72 changes: 72 additions & 0 deletions packages/wrangler/src/__tests__/d1/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,76 @@ describe("getDatabaseByNameOrBinding", () => {
getDatabaseByNameOrBinding(config, "123", "db")
).resolves.toStrictEqual(mockDb);
});

it("should resolve a config database without database_id via remote lookup", async ({
expect,
}) => {
mockGetMemberships([
{ id: "IG-88", account: { id: "1701", name: "enterprise" } },
]);
const mockDb = {
file_size: 7421952,
name: "db-from-config",
num_tables: 2,
uuid: "7b0c1d24-ec57-4179-8663-9b82dafe9277",
version: "alpha",
};

msw.use(
http.get("*/accounts/:accountId/d1/database", async () => {
return HttpResponse.json(
{
result: [mockDb],
success: true,
errors: [],
messages: [],
},
{ status: 200 }
);
})
);

const config = {
d1_databases: [
{
binding: "DB_BINDING",
database_name: "db-from-config",
},
],
} as unknown as Config;

await expect(
getDatabaseByNameOrBinding(config, "123", "DB_BINDING")
).resolves.toStrictEqual(mockDb);
});

it("should still error when config has no database_id and database_name cannot be resolved", async ({
expect,
}) => {
mockGetMemberships([
{ id: "IG-88", account: { id: "1701", name: "enterprise" } },
]);

msw.use(
http.get("*/accounts/:accountId/d1/database", async () => {
return HttpResponse.json(
{
result: [],
success: true,
errors: [],
messages: [],
},
{ status: 200 }
);
})
);

const config = {
d1_databases: [{ binding: "DB_BINDING" }],
} as unknown as Config;

await expect(
getDatabaseByNameOrBinding(config, "123", "DB_BINDING")
).rejects.toThrowError("Couldn't find DB with name 'DB_BINDING'");
});
});
5 changes: 3 additions & 2 deletions packages/wrangler/src/d1/migrations/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { createCommand } from "../../core/create-command";
import { confirm } from "../../dialogs";
import { isNonInteractiveOrCI } from "../../is-interactive";
import { logger } from "../../logger";
import { isLocal } from "../../utils/is-local";
import { DEFAULT_MIGRATION_PATH, DEFAULT_MIGRATION_TABLE } from "../constants";
import { executeSql } from "../execute";
import { getDatabaseInfoFromConfig } from "../utils";
Expand Down Expand Up @@ -75,8 +74,10 @@ export const d1MigrationsApplyCommand = createCommand({
);
}

// Migrations config lookup is only for local metadata (folder/table names).
// Remote execution resolves the real database UUID separately.
const databaseInfo = getDatabaseInfoFromConfig(config, database, {
requireDatabaseId: !isLocal({ local, remote }),
requireDatabaseId: false,
});

if (!databaseInfo && remote) {
Expand Down
5 changes: 3 additions & 2 deletions packages/wrangler/src/d1/migrations/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { configFileName, UserError } from "@cloudflare/workers-utils";
import { createCommand } from "../../core/create-command";
import { logger } from "../../logger";
import { requireAuth } from "../../user";
import { isLocal } from "../../utils/is-local";
import { DEFAULT_MIGRATION_PATH, DEFAULT_MIGRATION_TABLE } from "../constants";
import { getDatabaseInfoFromConfig } from "../utils";
import {
Expand Down Expand Up @@ -61,8 +60,10 @@ export const d1MigrationsListCommand = createCommand({
);
}

// Migrations config lookup is only for local metadata (folder/table names).
// Remote execution resolves the real database UUID separately.
const databaseInfo = getDatabaseInfoFromConfig(config, database, {
requireDatabaseId: !isLocal({ local, remote }), // Only require database_id for remote operations
requireDatabaseId: false,
});
if (!databaseInfo && remote) {
throw new UserError(
Expand Down
21 changes: 17 additions & 4 deletions packages/wrangler/src/d1/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,28 @@ export const getDatabaseByNameOrBinding = async (
accountId: string,
name: string
): Promise<Database> => {
const dbFromConfig = getDatabaseInfoFromConfig(config, name);
if (dbFromConfig) {
/**
* Resolve a D1 database from a config binding/name for remote operations.
*
* If the binding in config already has a `database_id`, return that directly.
* If it doesn't (for example, resource provisioning happened at deploy time),
* fall back to account-level lookup by `database_name` so remote commands still work.
*/
const dbFromConfig = getDatabaseInfoFromConfig(config, name, {
requireDatabaseId: false,
});
if (
dbFromConfig?.uuid !== undefined &&
dbFromConfig.uuid !== dbFromConfig.binding
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused by this. Why are we testing for .uuid != .binding?

Oh: getDatabaseInfoFromConfig() does some nonsense where it returns the db binding.

This is going to be fragile when we implement #11870. Can we parse the uuid to validate that it is a uuid instead?

) {
return dbFromConfig;
}

const allDBs = await listDatabases(config, accountId);
const matchingDB = allDBs.find((db) => db.name === name);
const remoteLookupName = dbFromConfig?.name ?? name;
const matchingDB = allDBs.find((db) => db.name === remoteLookupName);
if (!matchingDB) {
throw new UserError(`Couldn't find DB with name '${name}'`);
throw new UserError(`Couldn't find DB with name '${remoteLookupName}'`);
}
return matchingDB;
};
Expand Down
Loading