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

add dotenv support to wrangler secret bulk #7674

Merged
merged 7 commits into from
Jan 9, 2025
Merged

Conversation

Ankcorn
Copy link
Member

@Ankcorn Ankcorn commented Jan 6, 2025

Fixes #6388

Adds support for env files in all the secret bulk commands

Questions for the team:

currently the parsing is toggled between JSON and env by adding a flag --experimental-env-file.
Should this be made "no config" by trying to parse both and seeing which succeeds?

Will update the docs once the flag stuff is confirmed


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: changes don't require workers and can be effectively unit tested
  • Public documentation

Copy link

changeset-bot bot commented Jan 6, 2025

🦋 Changeset detected

Latest commit: 3bd3768

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jan 6, 2025

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-wrangler-7674

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7674/npm-package-wrangler-7674

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-wrangler-7674 dev path/to/script.js
Additional artifacts:
wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-workers-bindings-extension-7674 -O ./cloudflare-workers-bindings-extension.0.0.0-v7d1f2fb5d.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v7d1f2fb5d.vsix
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-create-cloudflare-7674 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-kv-asset-handler-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-miniflare-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-pages-shared-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-unenv-preset-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-vitest-pool-workers-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-workers-editor-shared-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-workers-shared-7674
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12695830110/npm-package-cloudflare-workflows-shared-7674

Note that these links will no longer work once the GitHub Actions artifact expires.


wrangler@3.100.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241230.0
workerd 1.20241230.0 1.20241230.0
workerd --version 1.20241230.0 2024-12-30

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@Ankcorn Ankcorn force-pushed the thomas/secret-put-dotenv branch 3 times, most recently from d66cd60 to 29ec739 Compare January 7, 2025 11:13
@Ankcorn Ankcorn marked this pull request as ready for review January 7, 2025 11:31
@Ankcorn Ankcorn requested review from a team as code owners January 7, 2025 11:31
packages/wrangler/src/pages/secret/index.ts Outdated Show resolved Hide resolved
packages/wrangler/src/pages/secret/index.ts Outdated Show resolved Hide resolved
packages/wrangler/src/secret/index.ts Outdated Show resolved Hide resolved
packages/wrangler/src/secret/index.ts Outdated Show resolved Hide resolved
@Ankcorn Ankcorn added e2e Run e2e tests on a PR and removed e2e Run e2e tests on a PR labels Jan 7, 2025
@Ankcorn Ankcorn force-pushed the thomas/secret-put-dotenv branch from 1ce2664 to 6b9707e Compare January 7, 2025 15:59
@Ankcorn Ankcorn requested a review from penalosa January 8, 2025 11:00
Copy link
Contributor

@irvinebroque irvinebroque left a comment

Choose a reason for hiding this comment

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

So this is basically

Before: you can pass wrangler secret bulk a path to a file, but has to be JSON

After: you can pass wrangler secret bulk a path to a file, and that file can be either JSON, or KEY=VALUE ala env vars

...making it easy to go "I have a .env file, I want to add all of it to secrets.
...but not coupling it specifically to the filename .env, or to dotenv behaviors, etc.

Correct?

@Ankcorn
Copy link
Member Author

Ankcorn commented Jan 8, 2025

So this is basically

Before: you can pass wrangler secret bulk a path to a file, but has to be JSON

After: you can pass wrangler secret bulk a path to a file, and that file can be either JSON, or KEY=VALUE ala env vars

...making it easy to go "I have a .env file, I want to add all of it to secrets. ...but not coupling it specifically to the filename .env, or to dotenv behaviors, etc.

Correct?

Exactly

@Ankcorn Ankcorn requested a review from irvinebroque January 8, 2025 16:53
@penalosa
Copy link
Contributor

penalosa commented Jan 8, 2025

So this is basically

Before: you can pass wrangler secret bulk a path to a file, but has to be JSON

After: you can pass wrangler secret bulk a path to a file, and that file can be either JSON, or KEY=VALUE ala env vars

...making it easy to go "I have a .env file, I want to add all of it to secrets. ...but not coupling it specifically to the filename .env, or to dotenv behaviors, etc.

Correct?

In Wrangler's case, I think it'd likely be a case of having a .dev.vars file which you want to upload

"wrangler": minor
---

Add support for env files to wrangler secret bulk
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you expand this with an example, and call out .dev.vars?

@@ -1065,7 +1087,7 @@ describe("wrangler secret", () => {
"wrangler secret:bulk [json]

POSITIONALS
json The JSON file of key-value pairs to upload, in form {\\"key\\": value, ...} [string]
json The file of key-value pairs to upload, as JSON in form {\\"key\\": value, ...} or .env file in the form KEY=VALUE [string]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
json The file of key-value pairs to upload, as JSON in form {\\"key\\": value, ...} or .env file in the form KEY=VALUE [string]
json The file of key-value pairs to upload, as JSON in form {\\"key\\": value, ...} or .dev.vars file in the form KEY=VALUE [string]

@irvinebroque
Copy link
Contributor

currently the parsing is toggled between JSON and env by adding a flag --experimental-env-file. Should this be made "no config" by trying to parse both and seeing which succeeds?

I'm having a hard time seeing a reason not to do that — you give us file, we detect format

@penalosa penalosa merged commit 45d1d1e into main Jan 9, 2025
29 checks passed
@penalosa penalosa deleted the thomas/secret-put-dotenv branch January 9, 2025 18:41
@workers-devprod workers-devprod mentioned this pull request Jan 9, 2025
penalosa pushed a commit that referenced this pull request Jan 10, 2025
* add dotenv support to wrangler secret bulk

* remove x-versions arg

* remove old --x-versions flag

* PR feedback

---------

Co-authored-by: Thomas Ankcorn <tankcorn@cloudflare.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: wrangler secret:bulk should support uploading .dev.vars file format
3 participants