Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.57k stars 4.82k forks source link

[BUG] BlobState.Deleted flag fails to return soft deletes (happens when versioning and soft delete both are enabled) #16268

Closed frigon closed 3 years ago

frigon commented 4 years ago

Describe the bug Using 12.6 of the Azure.Storage.Blob package I can not retrieve soft deletes via the API using the BlobStates.Deleted flag.

            BlobContainerClient container = BlobService.GetBlobContainerClient(containerId);
            var results = container.GetBlobsAsync(BlobTraits.All, BlobStates.Deleted);

If the flag is BlobStates.All the soft deletes are returned but in that case none of the items are marked as deleted in the BlobItem property nor does the DeletedOn property of the BlobItemProperties show any values for soft deleted items.

Expected behavior

  1. Soft deletes can be retrieved with the BlobStates.Deleted flag
  2. The deleted flags and deletedon properties should be properly populated for soft deletes

Actual behavior (include Exception or Stack Trace) Described above.

To Reproduce described above

Environment: -Azure.Storage.Blobs 12.6

frigon commented 4 years ago

Note if a blob version itself is deleted then the Deleted flags on that version ARE correctly populated. But since the native behavior is to retain all versions upon soft delete... there is no way via the SDK or API to find out which blobs have been soft deleted. Currently a REALLY expensive work around is the call the the API twice... once with BlobStates.None and once with BlobStates.All and compare the differences. If versioning is on this is really expensive.

ghost commented 4 years ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

jsquire commented 4 years ago

Thank you for your feedback. Tagging and routing to the team best able to assist.

seanmcc-msft commented 4 years ago

Hi @frigon, unfortunately I am unable to re-produce. Are you sure Blob Soft Delete has been enabled on your storage account?

We have a unit test for this scenario - https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Blobs/tests/ContainerClientTests.cs#L1876

When I run this test, I get a single BlobItem back, with Deleted = true, and Properties.DeletedOn is populated.

frigon commented 4 years ago

Thanks Sean...

Below is the structure of the container.
image

The credentials below are not sensitive... so you can try it yourself.

` var creds = "DefaultEndpointsProtocol=https;AccountName=springdata;AccountKey=CNbC3vn2xGJ/Qx0/PbaqCkDO0rVdgbZY06/zt6iYPRicHpM8ZnEnZKoApmAoK+tqSv4vZtKhy0f8Z/u1CXh0jg==;EndpointSuffix=core.windows.net";

var server = new BlobServiceClient(creds);

var container = server.GetBlobContainerClient("temp");

var blobs = container.GetBlobs(BlobTraits.All, BlobStates.Deleted).ToList(); `

When i rerun this "blob" only has 1 element... and it excludes the "one" soft deleted file. I'm using version 12.6.0 of the Nuget Package.

frigon commented 4 years ago

to be clear... yes soft delete is enabled. as is versioning.... see below...

image

frigon commented 3 years ago

@seanmcc-msft bump on this... i've resolved the problem is only occurring when both Versioning and Soft Deletes are both enabled. Are they compatible?

To duplicate the issue you can run the following to duplicate using the 12.7 SDK.

    static void Main(string[] args)
    {
        var creds = "DefaultEndpointsProtocol=https;AccountName=softdeletebroken;AccountKey=gMxDKHG7xM43g4VMboHWqI/WbokooOYStArX2JQSTQ+ZTjXJtc1y5eXn39MqtIm5w+hzeYWZYQ5paPdrst+x6A==;EndpointSuffix=core.windows.net";
        var server = new BlobServiceClient(creds);
        var container = server.GetBlobContainerClient("test");
        var blobs = container.GetBlobs( BlobTraits.All, BlobStates.Deleted).ToList();
        Console.WriteLine($"There are only {blobs.Count} blobs returned which makes me sad.");
        Main(args);
    }

Here is the contents of the container....

image

Here is the Data Protection settings....

image

adiazcan commented 3 years ago

Hello! I found the same behavior on sdk 12.8.0.

any plan to solve it on next version?

tigertechdev commented 3 years ago

Hi @frigon, unfortunately I am unable to re-produce. Are you sure Blob Soft Delete has been enabled on your storage account?

We have a unit test for this scenario - https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Blobs/tests/ContainerClientTests.cs#L1876

When I run this test, I get a single BlobItem back, with Deleted = true, and Properties.DeletedOn is populated.

@seanmcc-msft Hi, I'm having the same issue with 12.8.0. As far as I can tell the blob service does not consider blobs as soft-deleted when versioning is enabled.

Here's a self-contained repro:

namespace UnitTests
{
    using Azure.Storage.Blobs;
    using Azure.Storage.Blobs.Models;
    using Microsoft.VisualStudio.TestTools.UnitTesting;
    using System.IO;
    using System.Text;
    using System.Linq;

    [TestClass]
    public class UnitTest2
    {
        const string ContainerName = "tests-container";
        const string TestData = "Testing 1-2-3";

        [DataTestMethod]
        [DataRow("<redacted-connection-string-soft-delete>")]
        [DataRow("<redacted-connection-string-soft-delete-and-versioning>")]
        public void ListSoftDeletedBlobs(string ConnectionString)
        {
            string folder = $"versioning-and-softdelete-test-{System.Guid.NewGuid()}";

            var serviceClient = new BlobServiceClient(ConnectionString);
            if (!serviceClient.GetProperties().Value.DeleteRetentionPolicy.Enabled)
            {
                Assert.Inconclusive("Soft delete is not enabled on the blob service");
            }

            // don't know how to query versioning but it's enabled for one of the storage accounts and disabled for the other

            var client = new BlobContainerClient(ConnectionString, ContainerName);
            client.CreateIfNotExists();

            var blob = client.GetBlobClient($"{folder}/referenceblob");
            blob.Upload(new MemoryStream(Encoding.ASCII.GetBytes(TestData)));

            blob = client.GetBlobClient($"{folder}/deletedblob");
            blob.Upload(new MemoryStream(Encoding.ASCII.GetBytes(TestData)));
            blob.Delete();

            blob = client.GetBlobClient($"{folder}/deletedversionedblob");
            blob.Upload(new MemoryStream(Encoding.ASCII.GetBytes(TestData)));
            blob.Upload(new MemoryStream(Encoding.ASCII.GetBytes(TestData + TestData)), true);
            blob.Delete();

            var listResponse = client.GetBlobsByHierarchy(
                BlobTraits.None,
                BlobStates.Deleted,
                null,
                $"{folder}");

            int deletedCount = listResponse.Count((x) => x.IsBlob && x.Blob.Deleted);

            // this will fail for the storage account with both versioning and soft delete enabled
            Assert.AreNotEqual(0, deletedCount, "There should be some deleted blobs!");
        }
    }
}

Edit: Here's a screenshot from the portal showing the blob as deleted and it's versions as active: image

seanmcc-msft commented 3 years ago

Hi everyone,

I can repo the issue on a storage account with Blob Soft Delete and Blob Versioning both enabled, and I have reached out to our backend teams to further explore this issue. I'll post an update here when I hear back.

-Sean

seanmcc-msft commented 3 years ago

When soft delete and versioning are enabled together, soft delete protects only deleted versions. So if you want to list soft deleted entities, you must include versions as well (include=deleted,versions)

-Sean

KevinDitschke commented 3 years ago

Hello Sean,

I've tried your workaround, but sadly it is still not working. I'm using Azure.Storag.Blobs Version 12.9.1. As you can see soft delete and versioning are enabled in the storage account on Azure:

image

To test this scenario, I'm creating a blob by uploading binary data and deleting it:

image

When I want to check the deletion timestamp for restoration purposes the properties "DeletedOn"is null and Deleted" is false:

image

The complete metadata is also null or false even with BlobTraits specified for the GetBlobsAsync method.

Is there something I've overlooked or is it currently not possible to access this information?

Thank you in advance

Kevin

seanmcc-msft commented 3 years ago

For getting the metadata on BlobContainerClient.GetBlobs(), you need to also specify BlobTraits.Metadata, and have previously set metadata on the blob in question. Here is an example of setting metadata on a blob, and then listing blobs with metadata: https://github.com/Azure/azure-sdk-for-net/blob/cfac87d2cbf13252a46db4b975c922d5f783498e/sdk/storage/Azure.Storage.Blobs/tests/ContainerClientTests.cs#L2212

As I said previously, if Blob Versioning and Blob Soft Delete are both enabled together, soft delete only protected deleted versions, not the base blob. In your example above, you are deleting a base blob, which is not protected by blob soft delete. The blob you are seeing in the response is a version of the base blob you deleted.

For Blob Soft Delete to function as you expect, you need to disable Blob Versioning.

Ralf1108 commented 3 years ago

To clarify the expectation and the implementation in Azure.

The expectation was with versioning + soft-delete enabled:

B -> Blob V -> Version

inital creation: B1 - V0 overwriting/changing: B1 - V1 deleting: B1 - V2 (with deleted flag/info)

But it seems that the Azure implementation behaves like this:

inital creation: B1 - V0 overwriting/changing: B1 - V1 deleting: [not existing but older versions are still accessible via BlobStates.Version]

So the workaround to get the deleted version "B1 - V2" info would be to reconstruct the delete information via a second "GetBlobs(BlobStates.None)" call (like proposed in comment above