diff --git a/test/s3/versioning/s3_suspended_versioning_test.go b/test/s3/versioning/s3_suspended_versioning_test.go index c1e8c7277..2a039c330 100644 --- a/test/s3/versioning/s3_suspended_versioning_test.go +++ b/test/s3/versioning/s3_suspended_versioning_test.go @@ -8,6 +8,8 @@ import ( "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/stretchr/testify/require" ) // TestSuspendedVersioningNullOverwrite tests the scenario where: @@ -255,3 +257,41 @@ func TestEnabledVersioningReturnsVersionId(t *testing.T) { t.Logf(" Version %d: VersionId=%s, Size=%d, IsLatest=%v", i, *v.VersionId, v.Size, v.IsLatest) } } + +// The suspended PUT retires the null delete marker a preceding DELETE left, the same +// as the copy and multipart paths. While the regular-path object owns the null slot a +// leftover marker is shadowed, so it only shows once that null version goes away. +func TestSuspendedPutRetiresDeleteMarker(t *testing.T) { + client := getS3Client(t) + bucketName := getNewBucketName() + + createBucket(t, client, bucketName) + defer deleteBucket(t, client, bucketName) + + objectKey := "suspended-put-after-delete.txt" + + enableVersioning(t, client, bucketName) + putObject(t, client, bucketName, objectKey, "pre-suspension-content") + suspendVersioning(t, client, bucketName) + + putObject(t, client, bucketName, objectKey, "null-version-content") + deleteKey(t, client, bucketName, objectKey) + putObject(t, client, bucketName, objectKey, "replacement-content") + + headObject(t, client, bucketName, objectKey) + + // Drop the null version just written; a retired marker leaves nothing behind. + _, err := client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + VersionId: aws.String("null"), + }) + require.NoError(t, err) + + listResp, err := client.ListObjectVersions(context.TODO(), &s3.ListObjectVersionsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String(objectKey), + }) + require.NoError(t, err) + assert.Empty(t, listResp.DeleteMarkers, "the suspended PUT should have retired the null delete marker") +} diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 5055f61b6..3e30b6774 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -1293,10 +1293,6 @@ func (s3a *S3ApiServer) putSuspendedVersioningObject(r *http.Request, bucket, ob glog.V(3).Infof("putSuspendedVersioningObject: START bucket=%s, object=%s, normalized=%s", bucket, object, normalizedObject) - // The null version now lives at the regular path, so any null version recorded in - // .versions is stale (S3 has the suspended write overwrite it, not accumulate). - s3a.removeNullVersionFile(bucket, normalizedObject) - filePath := s3a.toFilerPath(bucket, normalizedObject) body := dataReader @@ -1360,15 +1356,18 @@ func (s3a *S3ApiServer) putSuspendedVersioningObject(r *http.Request, bucket, ob } // Versioned/suspended bucket → resolver returns 0; pass it directly. - // afterCreate clears the prior latest pointer and stamps the displaced version - // with NoncurrentSinceNs — off-ring under the write lock, routed off-lock after - // the PUT. Best-effort either way: a stale flag self-heals on the next list. + // afterCreate retires the null delete marker a preceding DELETE left, clears the + // prior latest pointer and stamps the displaced version with NoncurrentSinceNs — + // off-ring under the write lock, routed off-lock after the PUT. Only once the write + // has committed: retiring the marker for a write that then fails leaves the pointer + // naming a marker that is gone, and the read path heals that by promoting an older + // version, republishing the deleted key. Best-effort, as a stale flag self-heals on + // the next list. etag, errCode, sseMetadata = s3a.putToFiler(r, filePath, body, bucket, normalizedObject, 1, 0, &putFinalize{ - afterCreate: func(_ *filer_pb.Entry) s3err.ErrorCode { - if err := s3a.updateIsLatestFlagsForSuspendedVersioning(bucket, normalizedObject); err != nil { - // Best-effort: a stale IsLatest flag is recoverable on the - // next list-versions resync, so don't fail the PUT. - glog.Warningf("putSuspendedVersioningObject: failed to update IsLatest flags: %v", err) + afterCreate: func(entry *filer_pb.Entry) s3err.ErrorCode { + writtenETag := string(entry.Extended[s3_constants.ExtETagKey]) + if err := s3a.finalizeSuspendedNullWrite("", bucket, normalizedObject, s3_constants.ExtETagKey, writtenETag); err != nil { + glog.Warningf("putSuspendedVersioningObject: failed to retire the null delete marker: %v", err) } return s3err.ErrNone },