Add Blob Versioning Support to Azurite - #2735
Add Blob Versioning Support to Azurite#2735Rodolfo Orozco Vasquez (rorozcov) wants to merge 83 commits into
Conversation
…eate blob. No testing yet. All theoretical
…tore. Now local testing needed. Saving progress
…bs and api changes
…tests with versioning enabled. Must add versioning related checks
|
Rodolfo Orozco Vasquez (@rorozcov) Following your latest commit all of my tests now pass. Your PR now covers everything mine did and more. Your comments about 'Loose mode' make sense to me. I just wanted to flag it to get your thoughts as it looks like it can be reasonably argued either way. Do you know which maintainers should be tagged for review? |
Akanksha Jain (@jainakanksha-msft) is aware of the change and tracking it. It's actively being looked at but I'll let her provide the ETA. |
|
Gavin Thompson (@gavin-thompson-postman) spoke to the Azurite team. They are focused on 3.37.0 right now but that their immediate priority will be this PR. No specific ETA but I presume it's coming in the next few weeks. |
Akanksha Jain (jainakanksha-msft)
left a comment
Akanksha Jain (jainakanksha-msft)
left a comment
There was a problem hiding this comment.
-
Impact analysis
- Package:
azuriteBlob service; no package version change. - Version change: Blob Versioning is added as an opt-in, per-account Loki feature.
- Usage: Configured through
--accountConfigFilePathor--accountConfigAsJson; authentication remains inAZURITE_ACCOUNTS. - Impact and risk: Core version lifecycle behavior is broadly aligned with Azure, but the two inline findings are merge blockers because they affect data preservation and API compatibility. SQL metadata and version-specific SAS remain unsupported and should stay explicit limitations.
- Package:
-
Build and conflict resolution
- Commands run:
npm ci --include=dev,npm run build,npm run lint, and merge-conflict inspection against currentmain. - Issues found: The PR is currently reported as dirty/conflicting with
main. - Result: Build and lint pass at
fb848e0; rebase and conflict resolution are still required.
- Commands run:
-
Tests
- Tests added or updated: Broad Blob Versioning coverage across block/page/append blobs, snapshots, listing, metadata, copy, deletes, account parsing, and persistence.
- Coverage: Missing regressions for more than 5,000 versions/snapshots sharing one blob name and cross-type replacement while previous versions exist.
- Result: Please add both cases with the fixes.
-
Test suite
- Commands run:
npm run build,npm run lint,npm run test:blob. - Passing: 668 passing.
- Pending or failing: 3 pending, 0 failing. Production Azure parity tests are skipped/manual, so they do not provide repeatable parity validation.
- Commands run:
-
Changelog
- Entry added: No.
- Location: Please add this user-visible feature under
ChangeLog.mdUpcoming Release.
-
PR comments
- Threads reviewed: All 7 existing threads.
- Actions taken: Verified the 6 resolved threads against the final head and retained two new blockers inline.
- Unresolved items: The existing
BlobServerFactory.createServer()thread remains valid:accountModelStoreis optional in the public signature but required at runtime byBlobServer.
-
Self-rating
- Score: 5/10 in the current state.
- Reasoning: Strong design direction and test breadth, offset by a potential GC data-loss path, an Azure-incompatible blob-type transition, the runtime factory break, conflicts, and missing changelog.
-
Iteration
- Improvements made after self-review: Traced GC through
BlobReferredExtentsAsyncIterator, checked type-transition behavior against Azure Blob Versioning semantics, and validated the exact PR head locally.
- Improvements made after self-review: Traced GC through
-
Summary
- Why the update matters: This closes an important Azurite parity gap.
- Risk if not applied: Version-aware applications cannot be tested locally; applied unchanged, historical version data can be at risk and Azurite can accept operations Azure rejects.
- Final status: Request changes until both inline blockers, the factory contract, conflicts, and changelog are addressed.
| const coll = this.db.getCollection(this.BLOBS_COLLECTION); | ||
|
|
||
| // By default, we include all versions. This method is mostly for | ||
| // the GC, so there is no point in adding blob versioning support. |
There was a problem hiding this comment.
Blocking: GC can skip live version extents. listAllBlobs() is consumed by BlobReferredExtentsAsyncIterator, but both ordering and continuation use only blob.name. If one blob has more than maxResults versions/snapshots, the first page returns records with that name and sets the same name as its marker; the next query (obj.name > marker) excludes every remaining record for that blob. Those skipped records' extents are then absent from GC's referred-extent set and can be reclaimed while still referenced. Please use a stable unique ordering/marker such as name plus version/snapshot/record identity, and add a GC regression with more than one page of records sharing the same blob name.
| } | ||
| coll.remove(blobDoc); | ||
|
|
||
| if (this.isBlobVersioningEnabled(blob.accountName) || blobDoc.isCurrentVersion) { |
There was a problem hiding this comment.
Blocking Azure parity issue: Azure requires all versions of a blob to have the same blob type. A block/page/append blob cannot be overwritten with another type while previous versions remain; the base blob and all versions must first be deleted. This branch archives the old current record and later inserts the incoming blob without comparing blobDoc.properties.blobType with blob.properties.blobType, so Put Blob (and the analogous Copy Blob destination path) can create mixed-type history that Azure rejects. Please validate the invariant before mutating the old record and add tests for each cross-type transition, including Copy Blob.
|
Rodolfo Orozco Vasquez (@rorozcov) could you please update changelog and readme file, and add use case in regression test also. |
Will do! Give me 1-2 days at most. |
…nto copilot/testblob-versioning-gc-regression-again # Conflicts: # src/blob/BlobConfiguration.ts # src/blob/BlobEnvironment.ts # src/blob/BlobServer.ts # src/blob/BlobServerFactory.ts # src/blob/errors/StorageErrorFactory.ts # src/blob/handlers/BlobHandler.ts # src/blob/persistence/IBlobMetadataStore.ts # src/blob/persistence/LokiBlobMetadataStore.ts # src/blob/persistence/PageWithDelimiter.ts # src/blob/persistence/SqlBlobMetadataStore.ts # src/common/Environment.ts # src/common/VSCEnvironment.ts # src/common/VSCServerManagerBlob.ts Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
…gression-again
There was a problem hiding this comment.
🔵 Needs a closer look
The version-aware continuation token encoding introduced in PageWithDelimiter can become ambiguous with user-controlled blob names, risking broken pagination for valid names and should be made unambiguous before approval.
Review details
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
src/blob/persistence/PageWithDelimiter.ts:16
- The continuation token format for version-aware listings uses a plain delimiter string (
__version_marker__) to join[name, timestamp]. Since blob names are user-controlled, a blob name containing this substring will make markers ambiguous and can cause listing to fail when the marker is later parsed/split. Consider encoding the marker as a structured value (e.g., JSON array likeListAllBlobsMarker) or escaping the name component before concatenation.
tests/blob/apis/versioning.azurite.parity.test.ts:185 - This Azurite parity test is tagged
@productionin its title, which appears to be a copy/paste mistake (the file/suite is for Azurite). If you use these tags to filter test runs, this mislabel can cause the test to be skipped or miscategorized.
src/blob/BlobServer.ts:82 - The error thrown when
accountModelStoreis missing is correct, but it can be confusing because BlobServer still has a code path that constructs a defaultBlobConfigurationwhen none is provided. Consider clarifying the message so callers know BlobServer must be created via BlobServerFactory (or with an explicit configuration that includesaccountModelStore).
tests/testutils.ts:30 exportis split across lines (export\n async function ...), which is unusual in this codebase and can be reformatted by Prettier/linting. Keepingexport async functionon one line improves readability and avoids style tool churn.
- Files reviewed: 58/60 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The change set is large and feature-defining (new persistence/config abstractions plus handler/store contract changes), so it warrants final human verification despite only minor specific issues noted.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/blob/apis/versioning.azurite.parity.test.ts:185
- The test case name ends with
@productioneven though this is the Azurite parity suite. This can cause the test to be skipped or mis-grouped when running with grep/tag filters.
- Files reviewed: 59/61 changed files
- Comments generated: 1
- Review effort level: Lite
| const accountModels = new Map<string, AccountModel>(); | ||
| accountModels.set(EMULATOR_ACCOUNT_NAME, { | ||
| key: EMULATOR_ACCOUNT_KEY, | ||
| isBlobVersioningEnabled: versioningEnabled | ||
| }); | ||
| return new LokiAccountModelStore(databaseFile, true, accountModels); |
Co-authored-by: rorozcov <44987991+rorozcov@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The new continuation-token encoding for version-aware pagination is ambiguous for certain valid blob names and can break paging behavior, so it should be made robust before merge.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/blob/persistence/PageWithDelimiter.ts:226
- Continuation tokens for name+timestamp mode are currently built by joining
[name, timestamp]with the hard-coded__version_marker__separator. If a blob name contains that substring, the token becomes ambiguous andmarker.split(VERSIONING_MARKER)(e.g., in LokiBlobMetadataStore.listBlobs) will fail and return 400, breaking pagination for valid blob names. Consider encoding the marker tuple as an unambiguous format (e.g., JSON string or base64 of JSON) and accepting the legacy joined format for backward compatibility.
src/blob/persistence/LokiBlobMetadataStore.ts:166 - PR description mentions version IDs using JavaScript millisecond precision rather than Azure's 7 fractional digits, but the implementation converts timestamps to 7 fractional digits and even uses a per-blob sub-millisecond counter (see generateVersionId). Please align the PR description (and any external docs) with the implemented 7-digit RFC3339 versionId format so expectations match runtime behavior.
- Files reviewed: 59/61 changed files
- Comments generated: 0 new
- Review effort level: Lite
Akanksha Jain (@jainakanksha-msft) Your comments have been addressed. I am merging main into my branch again. I am also working on the pagination continuation token and wanted your input. Initially, I made it very simple to keep it backwards compatible with how azurite used to do it. However, it is technically opaque from Azure's point of view so I am thinking of making it opaque as well rather than azurite's current use of the blob name as the continuation token and my PR's use of blobname__Version_marker__timestamp. Let me know what you think. If not once I merge main we should be good to go |
Title: Add Blob Versioning Support to Azurite
Summary
Adds Azure Blob Storage versioning support to Azurite, enabling applications to create, list, retrieve, and delete previous blob versions during local development.
Closes #665.
Account model
This work introduces the account model abstraction designed in collaboration with the Azurite team. The account model provides a central place for account-scoped feature configuration so additional Azure Storage capabilities can be added without coupling them directly to server startup or individual blob handlers.
Blob versioning is currently the first and only feature backed by this abstraction. The design supports Azurite's multi-account mode, allowing each configured account to enable or disable versioning independently. Accounts without an explicit versioning setting retain the existing non-versioned behavior for backward compatibility.
Motivation
Blob versioning automatically preserves previous blob states after modification or deletion. Without emulator support, applications that depend on version-aware workflows cannot be tested locally before deployment to Azure.
Implementation
AccountModelsetting for enabling blob versioning.versionId.includeVersionswith version-aware continuation markers.Configuration
Blob versioning can be configured using:
--accountConfigFilePathfor JSON configuration files.--accountConfigAsJsonfor inline JSON configuration.Configured accounts must also be present in
AZURITE_ACCOUNTSfor authentication.Behavior
Limitations
This change does not currently support:
Validation