diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a2b3e213..803e3977 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -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, diff --git a/backend/posix/versioning_test.go b/backend/posix/versioning_test.go index 9aa08cb3..6ab30578 100644 --- a/backend/posix/versioning_test.go +++ b/backend/posix/versioning_test.go @@ -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)) +}