WC-5290 Use PATCH APIs for 'wrangler versions secret' commands#14448
WC-5290 Use PATCH APIs for 'wrangler versions secret' commands#14448GregBrimble wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 0317402 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
Pre-existing, not introduced by this PR. The PR is a clean refactor: it replaces the fragile version-reconstruction logic (download full version + reconstruct + re-upload) with a single PATCH to
I found no logic bugs, security issues, or backward-compatibility problems. The deletions of the old reconstruction code are appropriate. LGTM |
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
| annotations: { | ||
| "workers/message": versionMessage, | ||
| "workers/tag": versionTag, | ||
| }, |
There was a problem hiding this comment.
🚩 Merge-patch semantics cause annotations to be inherited from previous version
The old code created a brand-new version via POST, so annotations like workers/tag were only present if explicitly set. The new code uses application/merge-patch+json (packages/wrangler/src/versions/secrets/index.ts:108-111), and when versionTag is undefined, JSON.stringify omits workers/tag from the body. Per RFC 7396, omitting a field in a nested merge-patch object means 'preserve existing value'. So if the previous version had a workers/tag (e.g. from a prior --tag v1 invocation), the new version will inherit it — unlike the old behavior where the new version would have no tag. The same applies to any other annotation fields on the previous version (e.g. workers/triggered_by). This is arguably correct for a patch operation (only change what's specified), but it is a semantic difference from the old behavior. Whether this matters depends on how the PATCH API server handles annotations in practice.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes WC-5290.
Use PATCH APIs for
wrangler versions secretcommands. These APIs do server-side persistence of everything else so remove the need for Wrangler to try and reconstruct a version from a previous one, and so should reduce the number of bugs like #13843.Reviewable, but ask Greg before merging. We're landing one or two more internal things first and then we'll be ready to go.
A picture of a cute animal (not mandatory, but encouraged)