fix: keep the last modified time of promoted object versions in posix

Deleting the current version of an object by `versionId` makes the newest remaining version current again. Posix copies that version from the versioning directory into a new temporary file and links it in place of the object, but never restored the file's modification time, so `HeadObject`, `ListObjectVersions` and `ListObjectsV2` reported the delete time as the promoted version's `LastModified`. This affected an older version, a version under a deleted delete marker and the null version alike, while AWS keeps the original `LastModified` in all three cases. Directory objects were not affected, as `deleteDirObjectLatestVersion` already restores the directory's mtime.

The temporary file now carries an optional modification time, set with `setModTime` and applied by `link()` right before the file is published: through `/proc/self/fd` before `linkat` for an `O_TMPFILE` file, and after the file is closed and before the rename on the `CreateTemp` fallback and on non-Linux platforms. Both the promotion in `DeleteObject` and `createObjVersion` use it. `createObjVersion` used to call `os.Chtimes` on the temporary file while it was still open for writing, and Windows resets the last write time when a handle the file was written through is closed.

backend/posix/otmpfile_common.go

@@ -96,6 +96,23 @@ func (tmp *tmpfile) File() *os.File {
 	return tmp.f
 }
 
+// setModTime makes link() publish the file with the modification time t
+// instead of the time it was last written
+func (tmp *tmpfile) setModTime(t time.Time) {
+	tmp.modTime = t
+}
+
+// applyModTime sets the modification time requested with setModTime on the
+// file at path. Windows updates the last write time when a handle the file
+// was written through is closed, so it has to be set after that handle is
+// closed.
+func (tmp *tmpfile) applyModTime(path string) error {
+	if tmp.modTime.IsZero() {
+		return nil
+	}
+	return os.Chtimes(path, time.Now(), tmp.modTime)
+}
+
 func sleepWithJitter(backoffMs int) {
 	if backoffMs <= 1 {
 		time.Sleep(1 * time.Millisecond)

backend/posix/posix.go

@@ -1496,11 +1496,8 @@ func (p *Posix) createObjVersion(bucket, key string, size int64, acc auth.Accoun
 		}
 	}
 
-	// Restore original mtime after copy
-	err = os.Chtimes(f.File().Name(), time.Now(), originalMTime)
-	if err != nil {
-		return versionPath, err
-	}
+	// the version keeps the last modified time of the object
+	f.setModTime(originalMTime)
 
 	versionPath = filepath.Join(versionBucketPath, versioningKey)
 
@@ -5328,6 +5325,9 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
 					return nil, fmt.Errorf("close obj version: %w", err)
 				}
 
+				// the restored version keeps its last modified time
+				f.setModTime(srcObjVersion.ModTime())
+
 				if err := f.link(); err != nil {
 					return nil, fmt.Errorf("link tmp file: %w", err)
 				}

backend/posix/with_otmpfile.go

@@ -50,6 +50,9 @@ type tmpfile struct {
 	gid         int
 	newDirPerm  fs.FileMode
 	newFilePerm fs.FileMode
+	// modTime is the modification time the file is published with,
+	// the time it was last written if zero
+	modTime time.Time
 }
 
 // openTmpFile opens a temporary file in dir (a filesystem path) that link()
@@ -286,6 +289,12 @@ func (tmp *tmpfile) link() error {
 		return tmp.fallbackLink()
 	}
 
+	// the unnamed file has no path until it's linked
+	err = tmp.applyModTime(filepath.Join(procfddir, tmp.procFDName))
+	if err != nil {
+		return fmt.Errorf("set tmpfile modification time: %w", err)
+	}
+
 	procdir, err := os.Open(procfddir)
 	if err != nil {
 		return fmt.Errorf("open proc dir: %w", err)
@@ -351,6 +360,11 @@ func (tmp *tmpfile) fallbackLink() error {
 		return fmt.Errorf("close tmpfile: %w", err)
 	}
 
+	err = tmp.applyModTime(tempname)
+	if err != nil {
+		return fmt.Errorf("set tmpfile modification time: %w", err)
+	}
+
 	objPath := filepath.Join(tmp.bucket, tmp.objname)
 	dir := filepath.Dir(objPath)
 	err = os.Rename(tempname, objPath)

backend/posix/without_otmpfile.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path/filepath"
 	"syscall"
+	"time"
 
 	"github.com/versity/versitygw/auth"
 	"github.com/versity/versitygw/backend"
@@ -49,6 +50,9 @@ type tmpfile struct {
 	uid         int
 	gid         int
 	doChown     bool
+	// modTime is the modification time the file is published with,
+	// the time it was last written if zero
+	modTime time.Time
 }
 
 // openTmpFile opens a temporary file in dir (a filesystem path) that link()
@@ -121,6 +125,11 @@ func (tmp *tmpfile) link() error {
 		return fmt.Errorf("close tmpfile: %w", err)
 	}
 
+	err = tmp.applyModTime(tempname)
+	if err != nil {
+		return fmt.Errorf("set tmpfile modification time: %w", err)
+	}
+
 	backoffMs := initialBackoffMs
 	for {
 		err = backend.MoveFile(tempname, objPath, tmp.newFilePerm)

tests/integration/group-tests.go

@@ -2057,6 +2057,7 @@ func TestVersioning(ts *TestState) {
 	ts.Run(Versioning_DeleteObject_latest_version_with_null_version)
 	ts.Run(Versioning_DeleteObject_latest_version_null_version_order)
 	ts.Run(Versioning_DeleteObject_promoted_version_attributes)
+	ts.Run(Versioning_DeleteObject_promoted_version_last_modified)
 	ts.Run(Versioning_DeleteObject_non_existing_object)
 	ts.Run(Versioning_DeleteObject_implicit_dir)
 	ts.Run(Versioning_DeleteObject_trailing_slash_counterpart)
@@ -3659,6 +3660,7 @@ func GetIntTests() IntTests {
 		"Versioning_DeleteObject_latest_version_with_null_version":                         Versioning_DeleteObject_latest_version_with_null_version,
 		"Versioning_DeleteObject_latest_version_null_version_order":                        Versioning_DeleteObject_latest_version_null_version_order,
 		"Versioning_DeleteObject_promoted_version_attributes":                              Versioning_DeleteObject_promoted_version_attributes,
+		"Versioning_DeleteObject_promoted_version_last_modified":                           Versioning_DeleteObject_promoted_version_last_modified,
 		"Versioning_DeleteObject_dir_object_latest_version":                                Versioning_DeleteObject_dir_object_latest_version,
 		"Versioning_DeleteObject_non_existing_object":                                      Versioning_DeleteObject_non_existing_object,
 		"Versioning_DeleteObject_implicit_dir":                                             Versioning_DeleteObject_implicit_dir,

tests/integration/versioning.go

@@ -2818,6 +2818,149 @@ func Versioning_DeleteObject_promoted_version_attributes(s *S3Conf) error {
 	}, withVersioning(types.BucketVersioningStatusEnabled))
 }
 
+// Versioning_DeleteObject_promoted_version_last_modified deletes the current
+// version of objects, which makes an older version the current one: the
+// version put before it, the version under a delete marker or the null
+// version. The promoted version keeps its last modified time.
+func Versioning_DeleteObject_promoted_version_last_modified(s *S3Conf) error {
+	testName := "Versioning_DeleteObject_promoted_version_last_modified"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		cases := []struct {
+			obj string
+			// the version to promote is put while versioning is suspended
+			null bool
+			// the version to promote is hidden by a delete marker
+			delMarker bool
+			// the last modified time of the version to promote
+			lastModified *time.Time
+		}{
+			{obj: "my-obj"},
+			{obj: "my-dir/"},
+			{obj: "my-dm-obj", delMarker: true},
+			{obj: "my-dm-dir/", delMarker: true},
+			{obj: "my-null-obj", null: true},
+			{obj: "my-null-dir/", null: true},
+		}
+
+		put := func(obj string) (*string, error) {
+			r, err := putObjectWithData(objDataLen(obj, 100), &s3.PutObjectInput{
+				Bucket: &bucket,
+				Key:    &obj,
+			}, s3client)
+			if err != nil {
+				return nil, err
+			}
+			return r.res.VersionId, nil
+		}
+
+		// the versions to promote are put first, the null versions while
+		// versioning is suspended
+		for _, status := range []types.BucketVersioningStatus{
+			types.BucketVersioningStatusSuspended,
+			types.BucketVersioningStatusEnabled,
+		} {
+			err := putBucketVersioningStatus(s3client, bucket, status)
+			if err != nil {
+				return err
+			}
+			for i, c := range cases {
+				if c.null != (status == types.BucketVersioningStatusSuspended) {
+					continue
+				}
+				if _, err := put(c.obj); err != nil {
+					return err
+				}
+
+				ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+				res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{
+					Bucket: &bucket,
+					Key:    &c.obj,
+				})
+				cancel()
+				if err != nil {
+					return err
+				}
+				if res.LastModified == nil {
+					return fmt.Errorf("%v: expected non nil LastModified", c.obj)
+				}
+				cases[i].lastModified = res.LastModified
+			}
+		}
+
+		// the versions are promoted in a later second than they were put
+		time.Sleep(time.Second)
+
+		for _, c := range cases {
+			var latest *string
+			if c.delMarker {
+				ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+				out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
+					Bucket: &bucket,
+					Key:    &c.obj,
+				})
+				cancel()
+				if err != nil {
+					return err
+				}
+				latest = out.VersionId
+			} else {
+				versionId, err := put(c.obj)
+				if err != nil {
+					return err
+				}
+				latest = versionId
+			}
+
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
+				Bucket:    &bucket,
+				Key:       &c.obj,
+				VersionId: latest,
+			})
+			cancel()
+			if err != nil {
+				return err
+			}
+
+			ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+			res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{
+				Bucket: &bucket,
+				Key:    &c.obj,
+			})
+			cancel()
+			if err != nil {
+				return err
+			}
+			if res.LastModified == nil || !res.LastModified.Equal(*c.lastModified) {
+				return fmt.Errorf("%v: expected the promoted version LastModified to be %v, instead got %v",
+					c.obj, *c.lastModified, res.LastModified)
+			}
+
+			ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+			versions, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
+				Bucket: &bucket,
+				Prefix: &c.obj,
+			})
+			cancel()
+			if err != nil {
+				return err
+			}
+
+			if len(versions.Versions) != 1 {
+				return fmt.Errorf("%v: expected 1 object version, instead got %v",
+					c.obj, len(versions.Versions))
+			}
+			v := versions.Versions[0]
+			if v.LastModified == nil || !v.LastModified.Equal(*c.lastModified) {
+				return fmt.Errorf("%v: expected the listed version LastModified to be %v, instead got %v",
+					c.obj, *c.lastModified, v.LastModified)
+			}
+		}
+
+		return nil
+	}, withVersioning(types.BucketVersioningStatusEnabled))
+}
+
 func Versioning_DeleteObject_non_existing_object(s *S3Conf) error {
 	testName := "Versioning_DeleteObject_non_existing_object"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {