mirror of
https://github.com/distribution/distribution.git
synced 2026-07-16 16:59:52 +00:00
Fix blob type migration in the azure driver
Currently, the Azure driver always creates blobs as an AppendBlob, but in previous versions, it used to create them as BlockBlobs. There is migration logic to handle this, but it's currently inversed, so if the blob exists as a BlockBlob, we don't delete it, and get an error back from Azure: ``` RESPONSE 409: 409 The blob type is invalid for this operation. ERROR CODE: InvalidBlobType ``` Since the check is inversed, this also means that any operation against AppendBlobs does an extra delete -> create for no reason. Signed-off-by: David Marby <david@dmarby.se>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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{
|
||||
{},
|
||||
|
||||
Reference in New Issue
Block a user