diff --git a/.changeset/orange-cycles-rule.md b/.changeset/orange-cycles-rule.md new file mode 100644 index 000000000000..700bf4d857df --- /dev/null +++ b/.changeset/orange-cycles-rule.md @@ -0,0 +1,8 @@ +--- +"wrangler": patch +--- + +Do not crash when processing environment configuration. + +Previously there were corner cases where the configuration might just crash. +These are now handled more cleanly with more appropriate warnings. diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index a9b38d252b95..26e0c90b21ae 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -1,3 +1,5 @@ +import assert from "node:assert"; + /** * This is the static type definition for the configuration object. * It reflects the configuration that you can write in wrangler.toml, @@ -485,3 +487,160 @@ export async function validateConfig( return results; } + +/** + * Process the environments (`env`) specified in the `config`. + * + * The environments configuration is complicated since each environment is a customized version of the main config. + * Some of the configuration can be inherited from the main config, while other configuration must replace what is in the main config. + * + * This function ensures that each environment is set up correctly with inherited configuration, as necessary. + * It will log a warning if an environment is missing required configuration. + */ +export function normaliseAndValidateEnvironmentsConfig(config: Config) { + if (config.env == undefined) { + // There are no environments specified so there is nothing to do here. + return; + } + + const environments = config.env; + + for (const envKey of Object.keys(environments)) { + const environment = environments[envKey]; + + // Given how TOML works, there should never be an environment containing nothing. + // I.e. if there is a section in a TOML file, then the parser will create an object for it. + // But it may be possible in the future if we change how the configuration is stored. + assert( + environment, + `Environment ${envKey} is specified in the config but not defined.` + ); + + // Fall back on "inherited fields" from the config, if not specified in the environment. + const inheritedFields = [ + "name", + "account_id", + "workers_dev", + "compatibility_date", + "compatibility_flags", + "zone_id", + "routes", + "route", + "jsx_factory", + "jsx_fragment", + "site", + "triggers", + "usage_model", + ]; + for (const inheritedField of inheritedFields) { + if (config[inheritedField] !== undefined) { + if (environment[inheritedField] === undefined) { + environment[inheritedField] = config[inheritedField]; // TODO: - shallow or deep copy? + } + } + } + + // Warn if there is a "required" field in the top level config that has not been specified specified in the environment. + // These required fields are `vars`, `durable_objects`, `kv_namespaces` and `experimental_services`. + // Each of them has different characteristics that need to be checked. + + // `vars` is just an object + if (config.vars !== undefined) { + if (environment.vars === undefined) { + console.warn( + `In your configuration, "vars" exists at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "vars" is not inherited by environments.\n` + + `Please add "vars" to "env.${envKey}".` + ); + } else { + for (const varField of Object.keys(config.vars)) { + if (!(varField in environment.vars)) { + console.warn( + `In your configuration, "vars.${varField}" exists at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "vars" is not inherited by environments.\n` + + `Please add "vars.${varField}" to "env.${envKey}".` + ); + } + } + } + } + + // `durable_objects` is an object containing a `bindings` array + if (config.durable_objects !== undefined) { + if (environment.durable_objects === undefined) { + console.warn( + `In your configuration, "durable_objects.bindings" exists at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "durable_objects" is not inherited by environments.\n` + + `Please add "durable_objects.bindings" to "env.${envKey}".` + ); + } else { + const envBindingNames = new Set( + environment.durable_objects.bindings.map((b) => b.name) + ); + for (const bindingName of config.durable_objects.bindings.map( + (b) => b.name + )) { + if (!envBindingNames.has(bindingName)) { + console.warn( + `In your configuration, there is a durable_objects binding with name "${bindingName}" at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "durable_objects" is not inherited by environments.\n` + + `Please add a binding for "${bindingName}" to "env.${envKey}.durable_objects.bindings".` + ); + } + } + } + } + + // `kv_namespaces` contains an array of namespace bindings + if (config.kv_namespaces !== undefined) { + if (environment.kv_namespaces === undefined) { + console.warn( + `In your configuration, "kv_namespaces" exists at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "kv_namespaces" is not inherited by environments.\n` + + `Please add "kv_namespaces" to "env.${envKey}".` + ); + } else { + const envBindings = new Set( + environment.kv_namespaces.map((kvNamespace) => kvNamespace.binding) + ); + for (const bindingName of config.kv_namespaces.map( + (kvNamespace) => kvNamespace.binding + )) { + if (!envBindings.has(bindingName)) { + console.warn( + `In your configuration, there is a kv_namespaces with binding "${bindingName}" at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "kv_namespaces" is not inherited by environments.\n` + + `Please add a binding for "${bindingName}" to "env.${envKey}.kv_namespaces".` + ); + } + } + } + } + + // `experimental_services` contains an array of namespace bindings + if (config.experimental_services !== undefined) { + if (environment.experimental_services === undefined) { + console.warn( + `In your configuration, "experimental_services" exists at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "experimental_services" is not inherited by environments.\n` + + `Please add "experimental_services" to "env.${envKey}".` + ); + } else { + const envBindingNames = new Set( + environment.experimental_services.map((service) => service.name) + ); + for (const bindingName of config.experimental_services.map( + (service) => service.name + )) { + if (!envBindingNames.has(bindingName)) { + console.warn( + `In your configuration, there is a experimental_services with binding name "${bindingName}" at the top level, but not on "env.${envKey}".\n` + + `This is not what you probably want, since "experimental_services" is not inherited by environments.\n` + + `Please add a service for "${bindingName}" to "env.${envKey}.experimental_services".` + ); + } + } + } + } + } +} diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index 14d4907631b7..53b2dfc45aee 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -6,6 +6,7 @@ import makeCLI from "yargs"; import type Yargs from "yargs"; import { findUp } from "find-up"; import TOML from "@iarna/toml"; +import { normaliseAndValidateEnvironmentsConfig } from "./config"; import type { Config } from "./config"; import { confirm, prompt } from "./dialogs"; import { version as wranglerVersion } from "../package.json"; @@ -56,48 +57,7 @@ async function readConfig(configPath?: string): Promise { Object.assign(config, parsed); } - const inheritedFields = [ - "name", - "account_id", - "workers_dev", - "compatibility_date", - "compatibility_flags", - "zone_id", - "routes", - "route", - "jsx_factory", - "jsx_fragment", - "site", - "triggers", - "usage_model", - ]; - - Object.keys(config.env || {}).forEach((env) => { - inheritedFields.forEach((field) => { - if (config[field] !== undefined && config.env[env][field] === undefined) { - config.env[env][field] = config[field]; // TODO: - shallow copy? - } - }); - }); - - const mirroredFields = [ - "vars", - "kv_namespaces", - "durable_objects", - "experimental_services", - ]; - Object.keys(config.env || {}).forEach((env) => { - mirroredFields.forEach((field) => { - // if it exists on top level, it should exist on env definitions - Object.keys(config[field] || {}).forEach((fieldKey) => { - if (!(fieldKey in config.env[env][field])) { - console.warn( - `In your configuration, "${field}.${fieldKey}" exists at a top level, but not on "env.${env}". This is not what you probably want, since the field "${field}" is not inherited by environments. Please add "${field}.${fieldKey}" to "env.${env}".` - ); - } - }); - }); - }); + normaliseAndValidateEnvironmentsConfig(config); if ("experimental_services" in config) { console.warn( @@ -421,7 +381,11 @@ export async function main(argv: string[]): Promise { "👂 Start a local server for developing your worker", (yargs) => { return yargs - .positional("filename", { describe: "entry point", type: "string" }) + .positional("filename", { + describe: "entry point", + type: "string", + demandOption: true, + }) .option("name", { describe: "name of the script", type: "string", @@ -508,26 +472,26 @@ export async function main(argv: string[]): Promise { ); } - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - - const envRootObj = args.env ? config.env[args.env] || {} : config; + const environments = config.env ?? {}; + const envRootObj = args.env ? environments[args.env] || {} : config; // TODO: this error shouldn't actually happen, // but we haven't fixed it internally yet @@ -686,24 +650,24 @@ export async function main(argv: string[]): Promise { ); } - // -- snip, extract -- if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - await publish({ config: args.config as Config, name: args.name, @@ -768,6 +732,12 @@ export async function main(argv: string[]): Promise { // TODO: filter by client ip, which can be 'self' or an ip address }, async (args) => { + if (args.local) { + throw new NotImplementedError( + `local mode is not yet supported for this command` + ); + } + const config = args.config as Config; if (!(args.name || config.name)) { @@ -778,22 +748,19 @@ export async function main(argv: string[]): Promise { }`; // -- snip, extract -- + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + // didn't login, let's just quit + console.log("Did not login, quitting..."); + return; + } - if (!args.local) { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - // didn't login, let's just quit - console.log("Did not login, quitting..."); - return; - } + if (!config.account_id) { + config.account_id = await getAccountId(); if (!config.account_id) { - config.account_id = await getAccountId(); - if (!config.account_id) { - throw new Error("No account id found, quitting..."); - } + throw new Error("No account id found, quitting..."); } } - // -- snip, end -- const accountId = config.account_id; @@ -999,25 +966,24 @@ export async function main(argv: string[]): Promise { throw new Error("Missing script name"); } - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - const secretValue = await prompt( "Enter a secret value:", "password" @@ -1103,25 +1069,24 @@ export async function main(argv: string[]): Promise { throw new Error("Missing script name"); } - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - if (await confirm("Are you sure you want to delete this secret?")) { console.log( `Deleting the secret ${args.key} on script ${scriptName}.` @@ -1165,25 +1130,24 @@ export async function main(argv: string[]): Promise { throw new Error("Missing script name"); } - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - console.log( await fetchResult( `/accounts/${config.account_id}/workers/scripts/${scriptName}/secrets` @@ -1209,6 +1173,7 @@ export async function main(argv: string[]): Promise { .positional("namespace", { describe: "The name of the new namespace", type: "string", + demandOption: true, }) .option("env", { type: "string", @@ -1254,44 +1219,43 @@ export async function main(argv: string[]): Promise { }); await mf.getKVNamespace(title); // this should "create" the namespace console.log(`✨ Success! Created KV namespace ${title}`); - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- + // -- snip, end -- - // TODO: generate a binding name stripping non alphanumeric chars + // TODO: generate a binding name stripping non alphanumeric chars - console.log(`🌀 Creating namespace with title "${title}"`); - const namespaceId = await createNamespace(config.account_id, title); + console.log(`🌀 Creating namespace with title "${title}"`); + const namespaceId = await createNamespace( + config.account_id, + title + ); - console.log("✨ Success!"); - const envString = args.env ? ` under [env.${args.env}]` : ""; - const previewString = args.preview ? "preview_" : ""; - console.log( - `Add the following to your configuration file in your kv_namespaces array${envString}:` - ); - console.log( - `{ binding = "${args.namespace}", ${previewString}id = "${namespaceId}" }` - ); + console.log("✨ Success!"); + const envString = args.env ? ` under [env.${args.env}]` : ""; + const previewString = args.preview ? "preview_" : ""; + console.log( + `Add the following to your configuration file in your kv_namespaces array${envString}:` + ); + console.log( + `{ binding = "${args.namespace}", ${previewString}id = "${namespaceId}" }` + ); - // TODO: automatically write this block to the wrangler.toml config file?? + // TODO: automatically write this block to the wrangler.toml config file?? + } } ) .command( @@ -1299,42 +1263,39 @@ export async function main(argv: string[]): Promise { "Outputs a list of all KV namespaces associated with your account id.", {}, async (args) => { + const config = args.config as Config; + if (args.local) { throw new NotImplementedError( `local mode is not yet supported for this command` ); - } - - const config = args.config as Config; - - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit console.log("Did not login, quitting..."); return; } + if (!config.account_id) { config.account_id = await getAccountId(); if (!config.account_id) { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- + // -- snip, end -- - // TODO: we should show bindings if they exist for given ids + // TODO: we should show bindings if they exist for given ids - console.log( - JSON.stringify( - await listNamespaces(config.account_id), - null, - " " - ) - ); + console.log( + JSON.stringify( + await listNamespaces(config.account_id), + null, + " " + ) + ); + } } ) .command( @@ -1361,25 +1322,23 @@ export async function main(argv: string[]): Promise { }); }, async (args) => { + const config = args.config as Config; + if (args.local) { throw new NotImplementedError( `local mode is not yet supported for this command` ); - } - const config = args.config as Config; - - let id; - try { - id = getNamespaceId(args); - } catch (e) { - throw new CommandLineArgsError( - "Not able to delete namespace.\n" + e.message - ); - } - - // -- snip, extract -- + } else { + let id; + try { + id = getNamespaceId(args); + } catch (e) { + throw new CommandLineArgsError( + "Not able to delete namespace.\n" + e.message + ); + } - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1393,32 +1352,31 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- + // -- snip, end -- - await fetchResult<{ id: string }>( - `/accounts/${config.account_id}/storage/kv/namespaces/${id}`, - { method: "DELETE" } - ); + await fetchResult<{ id: string }>( + `/accounts/${config.account_id}/storage/kv/namespaces/${id}`, + { method: "DELETE" } + ); - // TODO: recommend they remove it from wrangler.toml + // TODO: recommend they remove it from wrangler.toml - // test-mf wrangler kv:namespace delete --namespace-id 2a7d3d8b23fc4159b5afa489d6cfd388 - // Are you sure you want to delete namespace 2a7d3d8b23fc4159b5afa489d6cfd388? [y/n] - // n - // 💁 Not deleting namespace 2a7d3d8b23fc4159b5afa489d6cfd388 - // ➜ test-mf wrangler kv:namespace delete --namespace-id 2a7d3d8b23fc4159b5afa489d6cfd388 - // Are you sure you want to delete namespace 2a7d3d8b23fc4159b5afa489d6cfd388? [y/n] - // y - // 🌀 Deleting namespace 2a7d3d8b23fc4159b5afa489d6cfd388 - // ✨ Success - // ⚠️ Make sure to remove this "kv-namespace" entry from your configuration file! - // ➜ test-mf + // test-mf wrangler kv:namespace delete --namespace-id 2a7d3d8b23fc4159b5afa489d6cfd388 + // Are you sure you want to delete namespace 2a7d3d8b23fc4159b5afa489d6cfd388? [y/n] + // n + // 💁 Not deleting namespace 2a7d3d8b23fc4159b5afa489d6cfd388 + // ➜ test-mf wrangler kv:namespace delete --namespace-id 2a7d3d8b23fc4159b5afa489d6cfd388 + // Are you sure you want to delete namespace 2a7d3d8b23fc4159b5afa489d6cfd388? [y/n] + // y + // 🌀 Deleting namespace 2a7d3d8b23fc4159b5afa489d6cfd388 + // ✨ Success + // ⚠️ Make sure to remove this "kv-namespace" entry from your configuration file! + // ➜ test-mf - // TODO: do it automatically + // TODO: do it automatically - // TODO: delete the preview namespace as well? + // TODO: delete the preview namespace as well? + } } ); } @@ -1438,6 +1396,7 @@ export async function main(argv: string[]): Promise { .positional("key", { type: "string", describe: "The key to write to.", + demandOption: true, }) .positional("value", { type: "string", @@ -1477,9 +1436,11 @@ export async function main(argv: string[]): Promise { }, async ({ key, ttl, expiration, ...args }) => { const namespaceId = getNamespaceId(args); + // One of `args.path` and `args.value` must be defined const value = args.path ? await readFile(args.path, "utf-8") - : args.value; + : // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + args.value!; const config = args.config as Config; if (args.path) { @@ -1501,11 +1462,8 @@ export async function main(argv: string[]): Promise { }); const ns = await mf.getKVNamespace(namespaceId); await ns.put(key, value, { expiration, expirationTtl: ttl }); - return; - } - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1519,14 +1477,13 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } + // -- snip, end -- - // -- snip, end -- - - await putKeyValue(config.account_id, namespaceId, key, value, { - expiration, - expiration_ttl: ttl, - }); + await putKeyValue(config.account_id, namespaceId, key, value, { + expiration, + expiration_ttl: ttl, + }); + } } ) .command( @@ -1574,12 +1531,8 @@ export async function main(argv: string[]): Promise { const ns = await mf.getKVNamespace(namespaceId); const listResponse = await ns.list({ prefix }); console.log(JSON.stringify(listResponse.keys, null, " ")); // TODO: paginate, collate - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1593,13 +1546,12 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- + // -- snip, end -- - console.log( - await listNamespaceKeys(config.account_id, namespaceId, prefix) - ); + console.log( + await listNamespaceKeys(config.account_id, namespaceId, prefix) + ); + } } ) .command( @@ -1610,6 +1562,7 @@ export async function main(argv: string[]): Promise { .positional("key", { describe: "The key value to get.", type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1651,9 +1604,8 @@ export async function main(argv: string[]): Promise { return; } - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1667,10 +1619,9 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - console.log( await fetchRaw( `/accounts/${config.account_id}/storage/kv/namespaces/${namespaceId}/values/${key}` @@ -1686,6 +1637,7 @@ export async function main(argv: string[]): Promise { .positional("key", { describe: "The key value to delete", type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1726,9 +1678,8 @@ export async function main(argv: string[]): Promise { const config = args.config as Config; - // -- snip, extract -- - if (!args.local) { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1742,10 +1693,9 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } + // -- snip, end -- } - // -- snip, end -- - await fetchResult( `/accounts/${config.account_id}/storage/kv/namespaces/${namespaceId}/values/${key}`, { method: "DELETE" } @@ -1769,6 +1719,7 @@ export async function main(argv: string[]): Promise { .positional("filename", { describe: `The JSON file of key-value pairs to upload, in form [{"key":..., "value":...}"...]`, type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1824,13 +1775,8 @@ export async function main(argv: string[]): Promise { expirationTtl: expiration_ttl, }); } - - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1844,13 +1790,12 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- + // -- snip, end -- - console.log( - await putBulkKeyValue(config.account_id, namespaceId, content) - ); + console.log( + await putBulkKeyValue(config.account_id, namespaceId, content) + ); + } } ) .command( @@ -1861,6 +1806,7 @@ export async function main(argv: string[]): Promise { .positional("filename", { describe: `The JSON file of keys to delete, in the form ["key1", "key2", ...]`, type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1904,13 +1850,8 @@ export async function main(argv: string[]): Promise { for (const key of parsedContent) { await ns.delete(key); } - - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { + // -- snip, extract -- const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1924,13 +1865,16 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } + // -- snip, end -- - // -- snip, end -- - - console.log( - await deleteBulkKeyValue(config.account_id, namespaceId, content) - ); + console.log( + await deleteBulkKeyValue( + config.account_id, + namespaceId, + content + ) + ); + } } ); }