s3: retire the suspended null marker only once the PUT has committed (#10589)

The suspended PUT dropped the null delete marker before writing, so a failed
write left the .versions pointer naming a marker that was gone. The read path
heals a dangling pointer by promoting the newest survivor, so a key the caller
had deleted came back serving an older version, and the heal persisted that
pointer. Move the retire into afterCreate via the shared finalize, which also
brings the ownership check the copy and multipart paths already have.

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")
+}

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
 		},