Repository navigation
Emulate suspended versioning on a versioned azureblob store - #1234
Open
gavin-thompson-postman wants to merge 13 commits into
Open
gavin-thompson-postman wants to merge 13 commits into
gavin-thompson-postman wants to merge 13 commits into
Conversation
Map S3 object versioning onto Azure's native blob versioning: version ids on writes, copies and multipart completes; reads, heads, deletes, copies and part copies by version id; and ListObjectVersions. Azure enables versioning per storage account on the management plane (ARM), which the data-plane SDK can neither set nor read, so the support is opt-in through s3proxy.azureblob.versioning: the operator asserts the account has versioning on, and only then does the store report supportsVersioning and honor the requests. Left unset it answers NotImplemented as before. Reading or enabling a bucket's versioning still answers NoSuchBucket for a container that does not exist, since the frontend leaves that check to a store that versions. Deleting the current version by id, which Azure refuses on the root blob, deletes the base blob first and then the demoted version. Azure judges a delete's If-Match against the current blob rather than the version named, so a conditional delete of a version is refused rather than carried out unconditionally. Azure keeps no delete markers and cannot suspend versioning per bucket, so those are left unimplemented rather than faked. A version client's URL already carries a versionid query, so the signed copy-source URL joins its SAS with '&' when the URL has one. Unit-test the config gating, the conditional version delete, the ListObjectVersions page mapping, the copy-source SAS join, and the versionId and headerless-error translations -- the paths Azurite cannot exercise -- with hand-built doubles, leaving Azurite for the rest. See gaul#1137. Co-Authored-By: Claude <noreply@anthropic.com>
Removing the current version by id deletes the base blob to demote it, then removes the demoted version. A write landing between Azure's refusal and that delete had its own fresh version demoted instead, leaving the key with no current version. Condition the base delete on the named version's ETag, which the base blob carries while that version is current; when the condition fails or the base is already gone, something else has demoted the version and it is simply removed. Co-Authored-By: Claude <noreply@anthropic.com>
The doubles assumed Azure behavior that a real, versioned storage account does not show: - A listing returns a key's versions oldest first and leaves isCurrentVersion unset, not false, on all but the current one. - A version delete's If-Match is judged against the version named: a non-current version deletes under its own ETag and is refused 412 ConditionNotMet under the current blob's. - A malformed versionId is answered 400 with no x-ms-error-code, on GET, HEAD and DELETE alike; InvalidQueryParameterValue never comes back. A well-formed but unknown one is answered 404 BlobNotFound, where S3 answers NoSuchVersion. A HEAD's 404 still carries its code in the header. With the doubles corrected three tests fail against the store as it stands, each marking code built on the old assumption: testVersionsPageMapsVersions (versions come out oldest first), testConditionalDeleteOfVersionReachesAzure (the condition is refused rather than sent) and testUnknownVersionIdIsNoSuchVersion (NoSuchKey). The fixes follow separately. Co-Authored-By: Claude <noreply@anthropic.com>
A blob written while the account's versioning was off comes back, once versioning is on again, after the versions written before it with no version id -- as the double had it -- but with isCurrentVersion unset like every other version, not true, though it is the one a plain read serves. With the double corrected testVersionsPageNamesUnversionedBlobNull also asserts that this "null" version is the latest, as S3 reports it, and fails: no version of such a key comes out flagged latest. Co-Authored-By: Claude <noreply@anthropic.com>
Modernizer flags Optional.get. Co-Authored-By: Claude <noreply@anthropic.com>
Azure lists a key's versions oldest first; S3 lists them newest first. Gather each key's versions whole and report them reversed, at most MaxKeys entries. Page on S3's own (key, version-id) markers rather than Azure's continuation token: the listing restarts from the key marker, which Azure's startFrom includes, and drops what the client has seen, so a page that fills inside a key -- even one whose versions straddle Azure's pages -- resumes inside it. The latest version is the one Azure flags current or, failing that, the one with no version id: a blob written while the account was unversioned, which Azure lists last and leaves unflagged though a plain read serves it. Azure answers a version the blob does not have 404 BlobNotFound, as for a blob that is not there; when the request named a version, report NoSuchVersion as S3 does. Reads, heads, copies and part copies by version translate so. The refusal of If-Match on a delete naming a version stays, but for the reason S3 gives: S3 itself answers such a delete 501 NotImplemented, whether or not the ETag matches, and honors If-Match only on a delete of the current object. bfc488d had read the refusal as a bug and flipped its test to expect the request to reach Azure; restore it. Co-Authored-By: Claude <noreply@anthropic.com>
On a versioned store "null" names S3's null version: the base blob while it carries no version id, a blob written while the account was unversioned. It was treated as no version at all, so DELETE ?versionId=null removed the current version of a key that had no null version, which S3 leaves alone; and GET, HEAD and the copy sources handed Azure the literal "null", which it rejects as malformed. Reads and copy sources now resolve "null" to the base blob when it has no version id and answer NoSuchVersion otherwise. A delete of "null" removes such a base blob under its ETag, and also the non-current version Azure keeps of it (deleting a base blob on a versioned account keeps its content under a fresh id, where S3 removes the null version for good); with no null version it succeeds and changes nothing. An unversioned store still takes "null" as the object itself. A malformed versionId, which Azure answers 400 with no error code, is InvalidArgument "Invalid version id specified", as S3 answers it, rather than the BadDigest a bare 400 becomes. Found by a GitHub Copilot review run outside this PR. Co-Authored-By: Claude <noreply@anthropic.com>
Azure keeps no delete markers. As the google-cloud-storage store does, a delete naming no version on a versioned store now writes an empty blob carrying s3proxy_delete_marker=true, which Azure versions over the current blob like any write, and reports it as the marker S3 would lay down. A key whose current version is such a blob holds no object: - GET and HEAD answer NoSuchKey with x-amz-delete-marker and the marker's version id; naming the marker's version answers 405. - An unversioned listing leaves the key out; ListObjectVersions reports the marker as a DeleteMarker entry. - A copy from it answers NoSuchKey, and one naming the marker's version is refused as InvalidRequest. - If-None-Match: * on a put or a multipart complete replaces the marker, under its ETag, rather than failing on it. - DeleteObjects routes each key through removeBlob, since a batch cannot write blobs, and reports each marker written. Deleting the current version by id still leaves the key with no current version, since Azure cannot make an existing version current again. S3 promotes the newest remaining one; reads, copies and listings now take that one as current, as the google-cloud-storage store does when its live generation is deleted. This only holds together with markers: a plain delete no longer demotes the current blob, so the newest version is never the content a plain delete removed. Deleting a marker this way is how S3 undeletes a key. If-None-Match: * likewise fails on a key whose current version was deleted by id while a version remains. An unversioned store is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
…rsions on azureblob Addresses review findings on the delete-marker emulation: - drop s3proxy_delete_marker / s3proxy_target_blob_name from user metadata on PUT, copy and multipart, so a client can't make an object read as a delete marker - with versioning on, ListObjectsV2 lists versions and reports each key by its latest, so a key whose current version was deleted by id (including one undeleted by deleting its marker) is listed as GET serves it; pages resume by name - time versions by their version id, not the whole-second Last-Modified, and break ties latest-first, so a PUT and its delete marker in the same second list in order Co-Authored-By: Claude <noreply@anthropic.com>
A versioned azureblob store reads a key with no current version from its newest remaining one, as S3 promotes it once the current version is deleted by id, and finds that version by listing the key's versions. A credential allowed to read and write blobs can still be refused the listing -- an account SAS with resource type o alone, which Azure answers 403 AuthorizationResourceTypeMismatch -- and that refusal escaped untranslated, so a GET or HEAD of a missing key, a copy from a missing source, and an If-None-Match: * write of a missing key all answered 500 InternalError. They now answer 403 AccessDenied, naming what the credential lacks: containers/blobs/read, or for a SAS permission l and resource type c. S3 itself answers a missing key 403 rather than 404 to a caller that may not list the bucket, and NoSuchKey would deny a version that may be there. Any other listing failure is translated as usual. Co-Authored-By: Claude <noreply@anthropic.com>
Azure versions every write on a versioned account and cannot stop doing so for one container, so PutBucketVersioning Suspended used to answer 501. With s3proxy.azureblob.versioning=true it now records the state as container metadata (s3proxy_versioning=Suspended; absent means Enabled), merged into whatever else the container carries, and GetBucketVersioning reports it. Every write to a versioned store reads it, so all proxy instances agree; a credential refused the container's properties writes as Enabled. While a container is Suspended: - PUT, copy and CompleteMultipartUpload land tagged s3proxy_null_version=true and report no version id, as the transient store does. Once the write has landed, the null version it replaced is removed: older tagged versions by id, and the version Azure minted for a base blob written before the account was versioned, by its ETag. - A delete naming no version writes a delete marker that is itself the null version, replacing the one before, and reports version id null. The null version is the base blob while it has no id, else the newest tagged version. versionId=null reads, copies and deletes resolve to it; reads and listings report it as "null"; DELETE versionId=null removes every tagged version. An older tagged version, left when a removal fails or races, is hidden from listings and swept by the next suspended write. Version paging resolves a "null" marker to the version it names, since after re-enabling the null version is no longer the newest. The tag is reserved like the marker key: dropped from caller metadata and from what reads report. A versioned copy now sends its metadata in the Put Blob From URL request itself, as x-ms-meta-* headers, and copies the source's content headers explicitly; otherwise Azure copies the source's metadata, tag included, whenever the copy names none, and the setMetadata call REPLACE used to follow with minted a second version and left the response naming the wrong one. An unversioned store is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
A versioned azureblob store reads its container's properties on every write to learn whether versioning is Suspended. A credential refused them -- a container service SAS, or an account SAS without resource type c -- used to write as Enabled, on the theory that it could not have suspended the container either. But a broader credential can, and then every write through the narrow one landed as a version of its own where S3 would have replaced the null version, silently. Such a write is now refused 403 AccessDenied, naming what the credential lacks: containers/read, or for a SAS resource type c. That covers PUT, copy, CompleteMultipartUpload and a delete naming no version. A suspended write that lands but cannot remove the null version it replaced still succeeds, since failing it would only have the client retry and leave another version. When the removal was refused for want of a permission -- 403, or OperationNotAllowedOnAutomaticSnapshot from a credential without deleteBlobVersion, as Storage Blob Data Contributor is -- every suspended overwrite will be refused alike and hidden versions accumulate, so that is now logged at ERROR naming the permission, at most once per container every five minutes. Any other failure is still logged at WARN. The README spells out what a credential needs with versioning on. Co-Authored-By: Claude <noreply@anthropic.com>
gavin-thompson-postman
force-pushed
the
azureblob-versioning-suspend
branch
from
November 3, 2026 11:48
1aab9fe to
711d752
Compare
When Put Blob From URL answers 501 -- only Azurite does -- copyBlob falls back to the asynchronous Copy Blob API. That fallback cannot copy correctly onto a versioned store: - Copy Blob copies the source's metadata when the request names none, so a copy of the null version made while Enabled, whose kept metadata is empty once the store's own entries are dropped, would keep the null version's tag and be reported as the null version itself. - REPLACE's content headers can only be set after the copy, a second write that Azure versions, leaving a version the caller never made. Azurite keeps no versions, so a versioned store never needs the fallback. It now refuses the copy as not supported instead, and the versioned handling inside the fallback, unreachable, is removed; an unversioned store falls back as before. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
@gaul I believe the versioning work is complete (at least from a known gap standpoint) with this PR (This one is 3 of 3). I did see your note on #1139 so will watch for movement on the Azurite side. We are testing internally with a fork of the s3 proxy (pinned to this PRs hash) which hopefully results in this new code becoming a little more battle hardened. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1207, which is stacked on #1139. This branch contains both PRs' commits. Until they merge, review only the last three commits,
8db4b07d(the suspended state),711d7526(restricted-credential fixes) andbd5c8c59(no Copy Blob fallback when versioned): diff against #1207. I'll rebase once they're in.Rebased onto #1207's fix for refused version listings (
d4b1917b). That fix addsS3Exceptions.accessDenied, and711d7526now reuses it instead of adding its own. Because of it, an account SAS withsrt=onow gets403 AccessDeniedinstead of500on a GET or HEAD of a missing key and on a copy from a missing source.This is phase 3 of the plan in #1139: suspended-state semantics (the null version and suspended delete markers). Until now
PutBucketVersioning Suspendedanswered501. Everything here applies only whens3proxy.azureblob.versioning=true. An unversioned store is unchanged.Why it needs emulating. Azure turns versioning on for the whole storage account and can't stop versioning one container. S3 keeps the state per bucket. A suspended bucket keeps its existing versions, but each new write becomes the single
nullversion and replaces the one before it.What S3Proxy does now:
PutBucketVersioning/GetBucketVersioning: Suspended is stored as container metadata,s3proxy_versioning=Suspended. Enabled is stored as the entry being absent. Azure replaces container metadata as a whole, so the entry is merged into what the container already has. A container nobody has suspended still reportsEnabled, never "unversioned" (same gap as before).CompleteMultipartUploadwhile suspended: the blob is written withs3proxy_null_version=true, and the response has nox-amz-version-id. The transient store also leaves the header out. After the write lands, the null version it replaced is deleted for good:deleteNullVersionfinds itx-amz-delete-marker: true,x-amz-version-id: null. DeleteObjects reportsDeleteMarkerVersionIdnull.versionId=null: GET, HEAD, copy-source and DELETE resolve it to the null version. That's the base blob while it has no version id, or else the newest tagged version. GET, HEAD and ListObjectVersions report that version asnull, and the null delete marker as aDeleteMarkernamednull.DELETE versionId=nullremoves every tagged version. When the null version was current, the newest remaining version is promoted, as in Emulate delete markers on a versioned azureblob store #1207.nullamong them. It's no longer the newest version, so anullversion-id marker can't be compared as a timestamp. Paging now resolves it to the version it names.s3proxy_null_versionis reserved like the marker key. It's dropped from caller metadata, and from the metadata GET and HEAD report on a versioned store. A copy of the null version made while Enabled isn't tagged.One fix to #1139's copy along the way. On a versioned store, copy now sends metadata in the Put Blob From URL request itself, as
x-ms-meta-*headers through the SDK'sAddHeadersFromContextPolicy. The SDK's options have no metadata field. It also passesx-ms-copy-source-blob-properties: falseand sets the source's content headers explicitly, with REPLACE overriding each one. Without that, Azure copies the source's metadata, null tag included, whenever the request names none.The old REPLACE path ran
setMetadataafter the copy. On a real account that call creates a version of its own. So the copy left two versions, and the response named the wrong one. Measured through S3Proxy on the test account:VersionId≠ HEADVersionId, 2 versions listedContent-type and content-language behave as before: the source's are kept unless REPLACE names them.
No Copy Blob fallback on a versioned store (
bd5c8c59). When Put Blob From URL answers501(only Azurite does), copy falls back to the asynchronous Copy Blob API. That fallback can't copy correctly onto a versioned store: Copy Blob copies the source's metadata when the request names none, so an Enabled copy of the null version would keep its tag, and REPLACE's content headers are set by a second, versioned write. Azurite keeps no versions, so a versioned store never needs the fallback; it now refuses the copy as not supported (501). An unversioned store falls back as before.Costs (versioning on):
versionId=nullcosts a properties call. If the null version isn't current, it also costs a version listing.Races and failure ordering:
DELETE versionId=null. A failed delete doesn't fail a write that already landed, since a client retry would only add another version. How it's logged depends on the cause (next section).Restricted credentials:
c): with versioning on, every write is refused403 AccessDenied, and nothing is written. That covers PUT, copy,CompleteMultipartUploadand a plain delete. The error message names what's missing. Such a credential can't tell whether a broader one has suspended the container. Before this, it wrote as Enabled, so in a Suspended bucket every write silently landed as a real-id version.deleteBlobVersion, for example Storage Blob Data Contributor or a SAS withoutx): the suspended write still succeeds. Azure refuses the cleanup with403 OperationNotAllowedOnAutomaticSnapshot, and every suspended overwrite will be refused the same way, so hidden versions pile up. This is now logged at ERROR, naming the permission, at most once per container every 5 minutes. Before, it was a WARN on every write. Other cleanup failures are still logged at WARN.Permissions (versioning on). These are in the README too:
Microsoft.Storage/storageAccounts/blobServices/containers/read, or an account SAS with resource typec.Microsoft.Storage/storageAccounts/blobServices/containers/blobs/deleteBlobVersion/action(SAS permissionx). Storage Blob Data Contributor lacks it; Storage Blob Data Owner or a custom role has it.Not covered / open questions:
x-amz-version-id: nullon a suspended PUT, or no header, I couldn't confirm: my AWS session had expired, so I couldn't probe it. This follows the transient store, which leaves it out, andVersionedBlobStoreTest.testSuspendedWritesReplaceTheNullVersion.DELETE versionId=nullon a null delete marker doesn't reportx-amz-delete-marker: true. Deleting any marker by id has the same gap in Emulate delete markers on a versioned azureblob store #1207.deleteNullVersionlogic, and the unit tests cover it.Testing:
Unit tests (
AzureBlobStoreTest, row-level test doubles in the existing style), 15 new and 2 updated:nullmarkerx-ms-meta-*headersAccessDeniedwhen Get Container Properties answers 403, with nothing written; a suspended overwrite whose version delete answers403 OperationNotAllowedOnAutomaticSnapshotstill succeeds and takes the permission path; a 500 on cleanup still succeeds and takes the WARN pathAfter the rebase, the full
mvn testpasses (676 tests, 0 failures; 677 withbd5c8c59's test), as doesmvn verify -DskipTests -Pjdeps(checkstyle, Error Prone/NullAway) on JDK 25.Against the real account (blob versioning on), with the Azure SDK first:
x-ms-meta-*headers replaces the source's metadata and keeps its content headerscopy-source-blob-properties: falsecopies neitherEnd to end through a local S3Proxy (
azureblob-sdk,s3proxy.azureblob.versioning=true), all passing. Raw Azure state was checked withaz storage blob list --include vm.VersionId, one null version, Azure holds exactly 2 versions (the enabled one and the null one)versionId=nullreportnull; the tag doesn't leak into metadata; the enabled version is still readableversionId=nullanswers 405DELETE versionId=nullremoves the marker and promotes the enabled version; repeating it is a no-opVersionId, a single null version, metadata and content-type correctDeleteMarkerVersionIdnullMaxKeys=1yields E4, E3,nullversionId=nullwhile Enabled is untaggedDELETE versionId=nullwhile Enabled removes only the null version; a plain delete writes a real-id markerx-amz-meta-s3proxy_null_versionis droppedThe external S3 versioning conformance suite used for Version objects on azureblob #1139: 40 of 41 pass (Emulate delete markers on a versioned azureblob store #1207: 37 of 41). The 4 Suspended-state tests now pass. The one remaining failure is "fresh bucket versioning unset", which is by design (Azure versioning is account-wide).
Restricted credentials, end to end on the real account (one local S3Proxy per credential, raw state from
az storage blob list --include vm). Before is5e1d100a; after is1aab9fe2(now rebased as8db4b07dand711d7526):srt=o,racwdlx200with real version ids; 2 PUTs left 2 untagged Azure versions403 AccessDeniednamingcontainers/read/ resource typec; 0 Azure versions writtenracwdlx403 AccessDenied(Azure codeAuthorizationFailure); 0 versions writtensrt=co,racwdl(nox)200, one WARN per write, 4 PUTs left 4 Azure versions200with no version id, listed as onenullversion; 4 PUTs + PUT + DELETE left 6 Azure versions (documented); exactly one ERROR namingdeleteBlobVersionfor the container;DELETE versionId=null403srt=co,racdlx(now)403, overwrite403DELETE versionId=nullleaves 0; PUT + DELETE leaves only the null markerThe account-key regression of the main flow also passed: suspend, 2 PUTs leave one null version beside the enabled one, a plain DELETE writes the null marker,
DELETE versionId=nullpromotes the enabled version, and re-enabling gives real ids again. After the rebase it passed again on711d7526: suspend, 2 PUTs leave 1 null version, a plain DELETE writes the null marker,DELETE versionId=nullleaves none, and re-enabling gives a real id. With the account key, a missing key answers404 NoSuchKey, and promotion after a delete by id still serves the previous version to GET, HEAD and copy.With versioning unset: the same script against Emulate delete markers on a versioned azureblob store #1207's build and this branch's gave identical output. It covered PutBucketVersioning Enabled and Suspended (
501), GetBucketVersioning (no status), put, get, head, copy, REPLACE copy, multipart, delete,versionId=nulland ListObjectVersions (501), and it confirmed no container metadata is written. A rerun with1aab9fe2gave the same results.Generated with Claude Code