diff --git a/registry/storage/driver/azure/azure.go b/registry/storage/driver/azure/azure.go index ba73dd9ea..7c5689316 100644 --- a/registry/storage/driver/azure/azure.go +++ b/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) } diff --git a/registry/storage/driver/azure/azure_test.go b/registry/storage/driver/azure/azure_test.go index 6598c9e26..2a19af41e 100644 --- a/registry/storage/driver/azure/azure_test.go +++ b/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{ {},