Skip to content

Conversation

@vincenttran-msft
Copy link
Member

@vincenttran-msft vincenttran-msft commented Nov 12, 2025

.tsp: Azure/azure-rest-api-specs#38750

  • Adds set/delete_immutability_policy to BlobClient
  • Adds a way to specify the Storage account type (since this test needs to be against a versioned account)

@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Nov 12, 2025
@github-actions
Copy link

github-actions bot commented Nov 12, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure_storage_blob

@vincenttran-msft vincenttran-msft changed the title [Storage] get/set_immutability_policy for BlobClient [Storage] delete/set_immutability_policy for BlobClient Nov 14, 2025
…to fix up legal hold to point to versioned as well + get bicep updated to work
}

#[recorded::test]
async fn test_immutability_policy(ctx: TestContext) -> Result<(), Box<dyn Error>> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing against a regular Storage account yields:

HTTP/1.1 400 ImmutableStorageWithVersioning: feature is not enabled.

Testing against versioned without immutable storage on:

ImmutableStorageWithVersioningIsNotEnabledImmutableStorageWithVersioning: feature is not enabled.

So, as a result, had to use a versioned account + immutable storage with versioning enabled.

As a result, having the same issues with deleting the container as set_legal_hold:

ContainerImmutableStorageWithVersioningEnabledThe requested operation is not allowed as the container has a immutable storage with versioning enabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark as playback only for now, I guess. Open an issue to track getting proper clean up on this and the legal hold tests so we can enable them to run Live.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3358
Done, will mark playback only once we get set_legal_hold merged since I unfortunately did not base it off of that PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't you update your test-resources.bicep file? That's the whole reason it's there and you should be using it, just like in all other languages (I wrote those original scripts for every Azure SDK language). I'd rather we don't have a confusing concept of "playback-only" tests. That implies you have to record them at some point which means modifying source which means people have to manually interject at least a couple times. Just use a #[tokio::test] with a MockHttpClient from our azure_core_test crate instead. I do that in lots of places. Pretty easy to use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heaths, we need to mark these tests as playback only because containers with immutability policies (and legal holds) are very difficult to clean up. They require the management plane to do so which we don't have yet in Rust. We will try and figure out a way to delete them and change them to regular recorded tests later. There is nothing that can be changed in the bicep to accomplish this.

We have the concept of playback only in every other language and we use it sparingly for situations like this. It would be nice to have that concept available in Rust. Mocking would require extra effort and means these tests will work and look different than the rest of our tests.

}

#[recorded::test]
async fn test_immutability_policy(ctx: TestContext) -> Result<(), Box<dyn Error>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark as playback only for now, I guess. Open an issue to track getting proper clean up on this and the legal hold tests so we can enable them to run Live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants