mirror of
https://github.com/versity/versitygw.git
synced 2026-09-22 16:04:15 +00:00
fix: clear stale delete-marker metadata on reupload
When versioning uses sidecar metadata, deleting the current object can leave the delete-marker attribute behind after the data file is removed. A later PUT or multipart completion using the same key then inherits that stale marker and the object remains hidden from the normal object view. Clear the marker when publishing a replacement, including orphaned sidecar and suspended-versioning cases, and remove all object metadata when deleting the final version without an older version to restore. Co-authored-by: Ben McClelland <ben.mcclelland@versity.com>
This commit is contained in:
committed by
Ben McClelland
co-authored by
Ben McClelland
parent
bce4ad4870
commit
00fa54d391
+23
-1
@@ -2668,6 +2668,13 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C
|
||||
_ = p.meta.DeleteAttribute(bucket, object, objectRetentionKey)
|
||||
}
|
||||
|
||||
// Clear the live marker after any snapshot, including when the data file
|
||||
// is missing or versioning does not require archiving the previous object.
|
||||
err = p.meta.DeleteAttribute(bucket, object, deleteMarkerKey)
|
||||
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) && !errors.Is(err, fs.ErrNotExist) {
|
||||
return res, "", fmt.Errorf("delete object delete-marker: %w", err)
|
||||
}
|
||||
|
||||
// if the versioning is enabled, generate a new versionID for the object
|
||||
var versionID string
|
||||
if p.versioningEnabled() && vEnabled {
|
||||
@@ -4495,6 +4502,13 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje
|
||||
return s3response.PutObjectOutput{}, verr
|
||||
}
|
||||
|
||||
// Marker cleanup belongs to publishing: snapshotting can be skipped for
|
||||
// orphaned sidecars, suspended versioning, or a missing version ID.
|
||||
err = p.meta.DeleteAttribute(*po.Bucket, *po.Key, deleteMarkerKey)
|
||||
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) && !errors.Is(err, fs.ErrNotExist) {
|
||||
return s3response.PutObjectOutput{}, fmt.Errorf("delete object delete-marker: %w", err)
|
||||
}
|
||||
|
||||
// Before finalizing the object creation remove
|
||||
// null versionId object from versioning directory
|
||||
// if it exists and the versioning status is Suspended
|
||||
@@ -4816,12 +4830,16 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
|
||||
return nil, err
|
||||
}
|
||||
err = os.Remove(objpath)
|
||||
if err != nil {
|
||||
if err != nil && !errors.Is(err, fs.ErrNotExist) && !isErrNotDir(err) {
|
||||
return nil, fmt.Errorf("remove obj version: %w", err)
|
||||
}
|
||||
|
||||
ents, err := os.ReadDir(versionPath)
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
if err := p.meta.DeleteAttributes(bucket, object); err != nil &&
|
||||
!errors.Is(err, meta.ErrNoSuchKey) && !errors.Is(err, fs.ErrNotExist) {
|
||||
return nil, fmt.Errorf("delete object attributes: %w", err)
|
||||
}
|
||||
p.removeParents(bucket, object)
|
||||
return &s3.DeleteObjectOutput{
|
||||
DeleteMarker: &isDelMarker,
|
||||
@@ -4833,6 +4851,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
|
||||
}
|
||||
|
||||
if len(ents) == 0 {
|
||||
if err := p.meta.DeleteAttributes(bucket, object); err != nil &&
|
||||
!errors.Is(err, meta.ErrNoSuchKey) && !errors.Is(err, fs.ErrNotExist) {
|
||||
return nil, fmt.Errorf("delete object attributes: %w", err)
|
||||
}
|
||||
p.removeParents(bucket, object)
|
||||
return &s3.DeleteObjectOutput{
|
||||
DeleteMarker: &isDelMarker,
|
||||
|
||||
@@ -17,13 +17,19 @@ package posix
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/aws"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/versity/versitygw/backend/meta"
|
||||
"github.com/versity/versitygw/s3err"
|
||||
"github.com/versity/versitygw/s3response"
|
||||
)
|
||||
|
||||
// newUnversionedGateway creates a Posix backend over a temporary root
|
||||
@@ -100,3 +106,69 @@ func TestVersioningUnconfigured(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestVersioningDeleteMarkerStaleSidecarClearedOnSameKeyReupload(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
vdir := filepath.Join(t.TempDir(), "versions")
|
||||
sidecarDir := filepath.Join(t.TempDir(), "sidecar")
|
||||
if err := os.MkdirAll(vdir, 0o755); err != nil {
|
||||
t.Fatalf("mkdir versioning dir: %v", err)
|
||||
}
|
||||
if err := os.MkdirAll(sidecarDir, 0o755); err != nil {
|
||||
t.Fatalf("mkdir sidecar: %v", err)
|
||||
}
|
||||
|
||||
sc, err := meta.NewSideCar(sidecarDir)
|
||||
if err != nil {
|
||||
t.Fatalf("new sidecar: %v", err)
|
||||
}
|
||||
p, err := New(root, sc, PosixOpts{
|
||||
ValidateBucketNames: true,
|
||||
VersioningDir: vdir,
|
||||
SideCarDir: sidecarDir,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new posix: %v", err)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
bucket, key := "bucket", "object"
|
||||
createTestBucket(t, p, bucket)
|
||||
if err := p.PutBucketVersioning(ctx, bucket, types.BucketVersioningStatusEnabled); err != nil {
|
||||
t.Fatalf("put bucket versioning: %v", err)
|
||||
}
|
||||
|
||||
put := func(body string) {
|
||||
t.Helper()
|
||||
_, err := p.PutObject(ctx, s3response.PutObjectInput{
|
||||
Bucket: &bucket,
|
||||
Key: &key,
|
||||
Body: strings.NewReader(body),
|
||||
ContentLength: aws.Int64(int64(len(body))),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("put object %q: %v", body, err)
|
||||
}
|
||||
}
|
||||
|
||||
put("one")
|
||||
|
||||
_, err = p.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: &bucket, Key: &key})
|
||||
assert.NoError(t, err)
|
||||
|
||||
put("two")
|
||||
|
||||
_, err = p.meta.RetrieveAttribute(nil, bucket, key, deleteMarkerKey)
|
||||
assert.ErrorIs(t, err, meta.ErrNoSuchKey)
|
||||
|
||||
out, err := p.GetObject(ctx, &s3.GetObjectInput{Bucket: &bucket, Key: &key})
|
||||
if err != nil {
|
||||
t.Fatalf("get object after reupload: %v", err)
|
||||
}
|
||||
defer out.Body.Close()
|
||||
body, err := io.ReadAll(out.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read object body: %v", err)
|
||||
}
|
||||
assert.Equal(t, "two", string(body))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user