fix(storage): Fix blob type migration in the azure driver (#4871)

registry/storage/driver/azure/azure.go

@@ -126,24 +126,23 @@ func (d *driver) PutContent(ctx context.Context, path string, contents []byte) e
 		return fmt.Errorf("content size exceeds max allowed limit (%d): %d", blockblob.MaxUploadBlobBytes, len(contents))
 	}
 
-	// Historically, blobs uploaded via PutContent used to be of type AppendBlob
-	// (https://github.com/distribution/distribution/pull/1438). We can't replace
-	// these blobs atomically via a single "Put Blob" operation without
-	// deleting them first. Once we detect they are BlockBlob type, we can
-	// overwrite them with an atomically "Put Blob" operation.
+	// PutContent creates blobs as AppendBlob. Blobs written by older versions
+	// of the driver may be BlockBlob (or other types), and Azure returns
+	// 409 InvalidBlobType if we try to create an AppendBlob over an existing
+	// blob of a different type. Delete any such legacy blob first so the
+	// subsequent Create succeeds.
 	//
 	// While we delete the blob and create a new one, there will be a small
-	// window of inconsistency and if the Put Blob fails, we may end up with
-	// losing the existing data while migrating it to BlockBlob type. However,
-	// expectation is the clients pushing will be retrying when they get an error
-	// response.
+	// window of inconsistency and if the Create fails, we may end up losing
+	// the existing data. However, the expectation is that clients pushing
+	// will retry when they get an error response.
 	blobName := d.blobName(path)
 	blobRef := d.client.NewBlobClient(blobName)
 	props, err := blobRef.GetProperties(ctx, nil)
 	if err != nil && !is404(err) {
 		return fmt.Errorf("failed to get blob properties: %v", err)
 	}
-	if err == nil && props.BlobType != nil && *props.BlobType != blob.BlobTypeBlockBlob {
+	if err == nil && props.BlobType != nil && *props.BlobType != blob.BlobTypeAppendBlob {
 		if _, err := blobRef.Delete(ctx, nil); err != nil {
 			return fmt.Errorf("failed to delete legacy blob (%v): %v", *props.BlobType, err)
 		}

registry/storage/driver/azure/azure_test.go

@@ -174,6 +174,75 @@ func TestCommitAfterMove(t *testing.T) {
 	}
 }
 
+// TestPutContentOverExistingBlob regression-tests the migration check above
+// the AppendBlob Create call in PutContent. PutContent creates AppendBlobs,
+// and Azure returns 409 InvalidBlobType if Create is called over an existing
+// blob of a different type. The check must delete only blobs whose type does
+// not match what we're about to create.
+//
+// Two cases:
+//   - BlockBlob: written by an older version of the driver. Without the fix
+//     this failed with 409 InvalidBlobType.
+//   - AppendBlob: written by the current driver. Must still overwrite cleanly
+//     (one Put Blob, no delete) — the previous code reached this by deleting
+//     and recreating.
+func TestPutContentOverExistingBlob(t *testing.T) {
+	skipCheck(t)
+
+	sd, err := azureDriverConstructor()
+	if err != nil {
+		t.Fatalf("unexpected error creating azure driver: %v", err)
+	}
+	d := sd.(*Driver).Base.StorageDriver.(*driver)
+	ctx := context.Background()
+
+	tests := []struct {
+		name string
+		seed func(t *testing.T, blobName string)
+	}{
+		{
+			name: "BlockBlob",
+			seed: func(t *testing.T, blobName string) {
+				if _, err := d.client.NewBlockBlobClient(blobName).UploadBuffer(ctx, []byte("legacy"), nil); err != nil {
+					t.Fatalf("seeding BlockBlob: %v", err)
+				}
+			},
+		},
+		{
+			name: "AppendBlob",
+			seed: func(t *testing.T, blobName string) {
+				if err := sd.PutContent(ctx, "/regression/"+t.Name(), []byte("legacy")); err != nil {
+					t.Fatalf("seeding AppendBlob via PutContent: %v", err)
+				}
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			path := "/regression/" + t.Name()
+			blobName := d.blobName(path)
+			// nolint:errcheck
+			defer sd.Delete(ctx, path)
+
+			tc.seed(t, blobName)
+
+			newContents := []byte("new content")
+			if err := sd.PutContent(ctx, path, newContents); err != nil {
+				t.Fatalf("PutContent over existing %s: %v", tc.name, err)
+			}
+
+			got, err := sd.GetContent(ctx, path)
+			if err != nil {
+				t.Fatalf("GetContent after PutContent: %v", err)
+			}
+			if string(got) != string(newContents) {
+				t.Fatalf("GetContent: got %q, want %q", got, newContents)
+			}
+		})
+	}
+}
+
 func TestParamParsing(t *testing.T) {
 	expectErrors := []map[string]any{
 		{},