-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: add --secrets-file parameter to wrangler versions upload #10896
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
base: main
Are you sure you want to change the base?
feat: add --secrets-file parameter to wrangler versions upload #10896
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🦋 Changeset detectedLatest commit: 3f78336 The changes in this PR will be included in the next version bump. 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 |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
} | ||
: undefined, | ||
logpush: undefined, // both logpush and observability are not supported in versions upload | ||
logpush: undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no reason to remove this comment
Fixed! I've restored the comment in commit 710a08c. The comment is now back on line 753: logpush: undefined, // both logpush and observability are not supported in versions upload All local tests pass. There are 2 macOS test failures in CI, but they appear to be unrelated flaky tests since my change only restored a comment. |
keepSecrets: props.secretsFile ? false : true, | ||
keepBindings: props.secretsFile | ||
? ["secret_key", "secret_text"] | ||
: undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this change is quite right. They cancel each other out.
When keepSecrets
is true (i.e. here secretsFile
is falsy) we automatically add secret_key
and secret_text
to the keep_bindings field in createWorkerUploadForm()
.
And here if secretsFile
is truthy, then we are explicitly adding secret_key
and secret_text
.
If we are saying that secretsFile
is the full source of truth for secrets then we should be setting keepSecrets
to false and not setting keepBindings
at all.
If we are saying that secretsFiles
is additive to the current secrets in the dashboard then keepSecrets
should always be true and keepBindings
should not be defined.
From looking at how wrangler versions secret bulk
works, we should not be treating the secetsFile as a source of truth but only additive. I think we should follow that here too.
So I think this should be:
keepSecrets: props.secretsFile ? false : true, | |
keepBindings: props.secretsFile | |
? ["secret_key", "secret_text"] | |
: undefined, | |
// we never delete secret bindings when uploading, even if we are setting secrets from a file | |
// so inherit all unchanged secrets from the previous Worker Version | |
keepSecrets: true, |
- Set keepSecrets: true unconditionally (regardless of secretsFile) - Remove keepBindings assignment when secretsFile is provided - createWorkerUploadForm() automatically adds keep_bindings when keepSecrets is true - Secrets from file are now additive to existing secrets (matching versions secret bulk behavior) - Update tests to expect correct keep_bindings order: ['secret_text', 'secret_key'] Co-Authored-By: [email protected] <[email protected]>
Fixed! I've updated the logic to always set Changes in commit 93e74d6:
This ensures that secrets not included in the file will be inherited from the previous version, making the secrets file truly additive. |
- Add --secrets-file argument to deploy command options - Parse secrets file and add to rawBindings in deploy implementation - Set keepSecrets: true to inherit existing secrets (additive behavior) - Add comprehensive unit tests for deploy with secrets file - Update changeset to document both deploy and versions upload commands Co-Authored-By: [email protected] <[email protected]>
- Set keepSecrets based on keepVars OR secretsFile (not unconditional) - Remove incorrect test that expected keepSecrets=true by default - Preserves backward compatibility while enabling --secrets-file feature Co-Authored-By: [email protected] <[email protected]>
Fixes #10068.
This PR adds a
--secrets-file
parameter to thewrangler versions upload
command, enabling users to upload secrets alongside their Worker code in a single atomic operation.Key Changes
Command Interface:
--secrets-file
parameter that accepts a path to a JSON or .env filewrangler versions secret bulk
commandImplementation:
parseBulkInputToObject
function for file parsingrawBindings
array withsecret_text
type bindings from parsed secretskeepSecrets: false
andkeepBindings: ["secret_key", "secret_text"]
when secrets file is provided to inherit non-provided secretsUsage Examples:
Critical Review Areas
keepSecrets: false
+keepBindings: ["secret_key", "secret_text"]
is critical - please verify this correctly inherits existing secrets not in the file while uploading new ones.rawBindings
withsecret_text
type is the correct way to upload secrets via the versions API.Testing
Link to Devin run: https://app.devin.ai/sessions/589ac34811cb43258f0107ed00840ed7
Requested by: @petebacondarwin