Version objects on azureblob - #1139
gavin-thompson-postman wants to merge 1 commit into
Conversation
|
It would be better to start with a PR for Azurite which will automate testing, particularly via s3-tests. I might consider using an fork to test since I also want the nextMarker functionality. This is not a hard blocker but I recommend pursuing this approach. |
|
@gaul Thanks for the input. I have added some unit tests that give partial coverage to the new code in the absence of Azurite support. I agree with you that ideally the Azurite PR would happen first, but I'm a bit worried about the timelines from our side. Would you be open to this being merged with the additional unit tests for now? |
|
@gaul I raised Azure/Azurite#2734 to try to get the Azurite support in first. |
|
@gaul One of the MSFT team actually raised their own PR for this Azure/Azurite#2735. If that one gets merged I will update the tests to run against Azurite. I notice Azurite doesn't get released that often, so to get this working in the CI for s3proxy I guess a fork (as you mentioned) or specific sha of Azurite would need to be pulled in temporarily until Azurite releases. |
7d4e306 to
c8c6464
Compare
|
@gaul - Would you be comfortable with this being merged in its current state with the additional unit tests that I put in place? Azure/Azurite#2735 is still pending review unfortunately so I can't plug in Azurite for these flows in the CI. I'd be happy to revisit this make sure the CI does make full use of Azurite if that PR does get merged. Thanks! |
Map S3 object versioning onto Azure's native blob versioning: version ids on writes, copies and multipart completes; reads, heads, deletes and 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. Deleting the current version by id, which Azure refuses on the root blob, deletes the base blob first and then the demoted version. Azure keeps no delete markers and cannot suspend versioning per bucket, so those are left unimplemented rather than faked. Unit-test the config gating, 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.
c8c6464 to
87c7fe6
Compare
I skipped this PR for 4.1.0. Azurite has shown some movement on the original implementation and your proposed PR so let's wait and see. I am willing to pin against a remote hash once they review and merge one of them so we don't have to wait for a versioned release. I don't believe that AI-generated unit tests are convincing. AIs regularly hallucinate and it is essential to ground them in reality, in this case using s3-tests against the live cloud service or using emulator. It is theoretically possible to set up a test against live Azure but every other S3Proxy feature has CI coverage through emulators so I don't want to make an exception unless I have to. |
2 participants
Maps S3 object versioning onto Azure's native blob versioning for the
azureblobbackend. Partially addresses #1137.This is phase 1 of 3, and is written to be reviewable and mergeable on its own — it stands up the full version-use surface, and each later phase adds an independent capability without reworking this one:
ListObjectVersions.ListObjectVersions).Covered
PutObject,CopyObjectandCompleteMultipartUploadGET/HEAD/DELETE/ copy by version idListObjectVersions(versions,IsLatest, prefix, pagination)Opt-in
Azure enables versioning per storage account on the management plane (ARM); the data-plane SDK can neither set nor read the setting. So it is opt-in through
s3proxy.azureblob.versioning=true, by which the operator asserts the account already has versioning on. Left unset, every versioning request answersNotImplementedexactly as before — no change for existing configs.Azure quirks handled
OperationNotAllowedOnRootBlob); handled by deleting the base blob first, then the demoted version.?versionid=, so the SAS joins with&(a bare?yieldsCannotVerifyCopySource).400rather than500.Deferred to later phases (not faked)
Azure has no analog, so these stay unimplemented rather than emulated for now:
setContainerVersioning(SUSPENDED)returns501. (phase 3)Testing
Azurite does not emulate blob versioning today, so there is no Azurite CI lane for this (unlike the SSE work). This PR adds a unit test for the opt-in gating that runs without a backend. The version-use paths were validated end-to-end against a real Azure storage account (with account versioning enabled) using an external S3 versioning conformance suite; the deferred items above are the only expected gaps.
mvn verify -Pjdepsis green.I am tracking Azure/Azurite#665, which has had some recent activity — if/when Azurite gains blob-versioning support I will wire an Azurite-backed CI lane for this the way the SSE work has one. If that issue stalls, I will attempt to contribute the versioning support to Azurite myself so this can be covered in CI.