cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.42k stars 593 forks source link

🚀 Feature Request: allow `--name` alongside `--env` #4123

Open aroman opened 9 months ago

aroman commented 9 months ago

Describe the solution

We have this shameful in line our CI/CD:

# wrangler doesn't let us pass --name alongside --env which is infuriating
- name: Set gameserver worker name in wrangler.toml
  run: sed -i 's/name = "gg-preview-gameserver"/name = "'"$GAMESERVER_WORKER_NAME"'"/' server/wrangler.toml

I tried removing it, and sure enough, on the latest wrangler:

In legacy environment mode you cannot use --name and --env together. If you want to specify a Worker name for a specific environment you can add the following to your wrangler.toml config:

why does this limitation exist? this seems like a very straightforward need — for us, we create one worker per PR preview branch (like gg-preview-pr-3455-gameserver), so we do need to set the worker name dynamically, rather than checking it into git.

rozenmd commented 9 months ago

Looking at https://github.com/cloudflare/workers-sdk/pull/680 (the PR that added this limitation) and its related issue - https://github.com/cloudflare/workers-sdk/issues/672, makes it sound like this shouldn't happen for newer Workers?

~How long ago did you create the Worker @aroman ?~

Looking at the code in https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/src/config/validation.ts#L118C2-L121, looks like everyone's being hardcoded to a legacy environment.

Might be worth revisiting @penalosa

penalosa commented 9 months ago

This sounds like a behaviour we should change, since it's possible to overwrite names per environment in wrangler.toml anyway (worth noting that legacyEnv is a bit of a misnomer—most people should be using legacyEnv since service environments were deprecated). @petebacondarwin do you remember why this change was made?

aroman commented 1 month ago

@petebacondarwin any update here?

petebacondarwin commented 1 month ago

OK I think could probably relax this error now for name and env passed as command line args, since I believe we support specifying the name inside an environment.

The question previously was what should the user expect from these commands in the same Worker project, which has a wrangler.toml that includes:

name = "worker-in-toml"
env.staging.name = "worker-in-toml-env"
Command Current Proposed Comment
wrangler deploy --name=worker-in-args Worker deployed with name "worker-in-args" Worker deployed with name "worker-in-args" Just the CLI arg used
wrangler deploy --name=worker-in-args --env=prod Error Worker deployed with name "worker-in-args" Just CLI name used
wrangler deploy --name=worker-in-args --env=staging Error Worker deployed with name "worker-in-args" Just the CLI name used
wrangler deploy --env=prod Worker deployed with name "worker-in-toml-prod" Worker deployed with name "worker-in-toml-prod" CLI env and top-level toml name combined
wrangler deploy --env=staging Worker deployed with name "worker-in-toml-env" Worker deployed with name "worker-in-toml-env" Just name used from env in toml
wrangler deploy Worker deployed with name "worker-in-toml" Worker deployed with name "worker-in-toml" Just top-level toml name used
aroman commented 1 month ago

what you propose above matches my expectation perfectly: the give precedence to the CLI name arg, then the environment, then the toplevel.

petebacondarwin commented 1 month ago

My biggest concern here is the case where someone naively adds either --name or --env to a wrangler deploy command that already contains the other option and accidentally deploys a non-production Worker environment config to the production worker.

Imagine the case where you have an npm script like:

{
  "scripts": {
    "deploy": "wrangler deploy --name=MyWorker"
  }
}

Running pnpm run deploy will deploy to the production worker with the toplevel environment.

But running pnpm run deploy --env staging will deploy to the production worker with the staging environment, most likely breaking the users production system; whereas the user might have expected the deployment to go to the MyWorker-staging Worker.