Add Blob Versioning Support to Azurite - #2735
Add Blob Versioning Support to Azurite#2735Rodolfo Orozco Vasquez (rorozcov) wants to merge 76 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
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 58 changed files in this pull request and generated no new comments.
Suppressed comments (6)
src/blob/persistence/SqlBlobMetadataStore.ts:2782
setTierin the SQL metadata store can never succeed:versionIdis typed asundefinedand the guardif (!versionId)always throws. This breaks Set Blob Tier for non-versioned blobs in SQL mode. The method should accept an optional versionId, reject non-empty version IDs (since SQL doesn’t support versioning), and proceed normally when no version is provided.
public setTier(
context: Context,
account: string,
container: string,
blob: string,
tests/common/LokiAccountModelStore.test.ts:15
substr()is deprecated; preferslice()/substring()to avoid deprecation warnings and to align with modern JS usage.
src/blob/utils/utils.ts:148- The doc comment says this parser only accepts
...fffffffZ(7 fractional digits), but the implementation intentionally accepts 3–7 fractional digits (/\.\d{3,7}Z$/). Please update the comment so callers don’t incorrectly assume 7 digits are required.
* This function will only attempt to parse strings in the specific ISO 8601 format: YYYY-MM-DDTHH:mm:ss.fffffffZ
src/common/account/LokiAccountModelStore.ts:81
- The error message in
clean()referencesLokiBlobMetadataStore, but this isLokiAccountModelStore. This makes failures harder to diagnose when the account model DB can’t be cleaned.
docs/designs/2025-12-blob-versioning.md:56 - The design doc states Azurite version IDs end in 3 fractional digits (milliseconds), but the implementation generates RFC3339 timestamps with 7 fractional digits (it pads to 7 digits and uses the last 4 digits as a per-blob counter within the same millisecond). This doc mismatch can mislead users writing versionId validation logic.
- Each version is assigned a unique version ID in ISO 8601 date-time format
- **Note:** Azurite's version IDs end in 3 digits + Z (e.g., `2024-12-06T10:30:45.123Z`) due to JavaScript's Date implementation, while Azure's version IDs end in 7 digits + Z (e.g., `2024-12-06T10:30:45.1234567Z`). If your application relies on this specific format, plan accordingly.
src/blob/handlers/BlobHandler.ts:199
- VersionId is validated for delete, but it isn’t forwarded to the metadata store. In SQL mode this means a request like
?versionid=...will delete the base blob instead of rejecting/handling a version delete (SqlBlobMetadataStore only checks itsversionIdparameter, notoptions.versionId). Passoptions.versionIdthrough when callingdeleteBlob.
this.validateVersionId(
options.snapshot,
options.versionId,
context.contextId!
);
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 58 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/common/VSCServerManagerBlob.ts:55
- VSCServerManagerBlob.createImpl no longer calls AzuriteTelemetryClient.init(), but startImpl/closeImpl still emit telemetry events. Without initialization, telemetry settings (including VSC mode/workspace config) won’t be applied and events may be dropped or mis-attributed. Also, eagerly calling accountModelStore.init() keeps the store open, which can cause BlobServer.clean() (used by the “clean” command) to fail because clean requires stores to be closed.
public async createImpl(): Promise<void> {
const env = new VSCEnvironment();
const location = await env.location();
// Create account model store
const accountModels = env.getAccountModels();
const accountModelStore = new LokiAccountModelStore(
join(location, DEFAULT_ACCOUNT_MODEL_LOKI_DB_PATH),
env.inMemoryPersistence(),
accountModels
);
await accountModelStore.init();
const blobServerFactory = new BlobServerFactory();
this.server = await blobServerFactory.createServer(env, accountModelStore);
const config = this.server.config;
src/common/IAccountModelEnvironment.ts:18
- The JSDoc
@memberoftag references IAccountDataEnvironment, but the interface name here is IAccountModelEnvironment. This is misleading in generated docs and IDE tooltips.
/**
* Gets the account models configuration from environment flags.
* Returns a map of account name to AccountModel, or undefined if no account configuration is provided.
*
* @returns {(Map<string, AccountModel> | undefined)}
* @memberof IAccountDataEnvironment
*/
getAccountModels(): Map<string, AccountModel> | undefined;
src/blob/BlobServer.ts:196
- BlobServer.clean() now unconditionally calls accountModelStore.clean(), but LokiAccountModelStore.clean() throws unless the store is closed. This makes
server.clean()fail in valid scenarios where the server is in Closed status but the account model store was initialized (e.g., VS Code manager calls createImpl then clean without starting/closing). Close the account model store (if open) before cleaning it, consistent with other stores’ expectations.
if (this.metadataStore !== undefined) {
await this.metadataStore.clean();
}
await this.accountModelStore.clean();
|
Rodolfo Orozco Vasquez (@rorozcov) Is there anything I can do to help get this merged? Of the 38 test I'd been using against my branch, 35/38 pass against yours and the remaining 3 are down to two issues.
Let me know if I can help here. |
Gavin Thompson (@gavin-thompson-postman) Will take a look in the morning to provide a better answer. On PDT time. I'll also ping the azurite team to take a look. I know they're quite busy but it is on their radar! |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Gavin Thompson (@gavin-thompson-postman) Thanks for looking into this and running your tests against the PR. I’ve added I also ran the 36 non-loose tests from the test set you described, and they all pass now. Regarding loose mode, my general philosophy when working with Azure emulators is that supported features should behave as closely as possible to the real Azure resource. Ideally, an application or SDK should not need emulator-specific branches for behavior that the emulator claims to support. That is a large part of why I started the versioning work in the first place. I can see the historical argument for ignoring First, silently accepting a malformed version ID could allow incorrect SDK or application usage to pass against Azurite, only to fail later against a real storage account. It could also cause the emulator to operate against the current blob rather than the version the caller intended, which is particularly concerning for destructive operations. Second, limiting that behavior to accounts with versioning disabled does not fully solve it. Disabling versioning does not delete existing versions, and those versions remain addressable by version ID. We would therefore need additional state-dependent logic to determine whether ignoring the parameter was safe. For those reasons, I would prefer to keep That said, I’m not an Azurite maintainer, so the broader loose-mode compatibility policy is not ultimately my decision. A lot of this implementation was developed with input from the Azurite team, and I’m happy to defer to their preference here. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 58 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/testutils.ts:27
- The exported helper uses an unusual line break between
exportand the function declaration, which makes the file harder to read and may trip formatting/linting rules. Use a normalexport async function ...declaration.
tests/blob/apis/versioning.azurite.parity.test.ts:185 - This test is in the Azurite parity suite but is tagged with "Indicee Production Account (@production)" in the title, which is misleading and can break tag-based test selection. Update the tag to "Marko Nikic (@azurite)" (or remove the tag if it’s not used).
| public async createServer( | ||
| blobEnvironment?: IBlobEnvironment | ||
| blobEnvironment?: IBlobEnvironment, | ||
| accountModelStore?: IAccountModelStore | ||
| ): Promise<BlobServer | SqlBlobServer> { | ||
| // TODO: Check it's in Visual Studio Code environment or not |
|
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
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