From 20b3334ae9c108151169fe22bd6f82f3e4ce5b67 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Wed, 30 Sep 2026 23:24:39 +0200 Subject: [PATCH] fix: report the delete time as the last modified time of delete markers in posix Creating a delete marker in posix keeps the file of the hidden version at the object path and only marks it with the delete marker attribute, so the delete marker kept the modification time of the version it hides. `ListObjectVersions` listed that time for current and noncurrent delete markers, and `HeadObject` and `GetObject` sent it as the `Last-Modified` header of the `MethodNotAllowed` response for a delete marker `versionId`. AWS reports the time the delete marker is created, including for the null delete marker created while versioning is suspended. `DeleteObject` now sets the modification time of the delete marker to the time it's created. Setting the time needs the entry to be owned by the gateway, so it is left as is otherwise, like it is for directory objects. `ListObjectVersions` also listed the delete markers with the SDK `types.DeleteMarkerEntry`, which has no XML marshaling of its own, so `LastModified` was sent in the local time zone of the gateway and with a sub-second part, while the versions are sent in UTC to the second. The delete markers are now listed with the new `s3response.DeleteMarkerEntry`, which formats `LastModified` the same way as `s3response.ObjectVersion`, and the s3proxy backend converts the upstream delete markers to it. --- backend/posix/posix.go | 16 ++- backend/s3proxy/s3.go | 17 ++- backend/walk.go | 6 +- s3response/s3response.go | 26 +++- tests/integration/group-tests.go | 2 + tests/integration/versioning.go | 203 +++++++++++++++++++++++++++++++ 6 files changed, 260 insertions(+), 10 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 6cc994f0..4c36ca48 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1690,7 +1690,7 @@ func (p *Posix) checkCopySourceDeleteMarker(bucket, object, versionId string) er func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { return func(path, versionIdMarker string, pastVersionIdMarker *bool, availableObjCount int, d fs.DirEntry) (*backend.ObjVersionFuncResult, error) { var objects []s3response.ObjectVersion - var delMarkers []types.DeleteMarkerEntry + var delMarkers []s3response.DeleteMarkerEntry // if the number of available objects is 0, return truncated response if availableObjCount <= 0 { return &backend.ObjVersionFuncResult{ @@ -1752,7 +1752,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { } if isDel { - delMarkers = append(delMarkers, types.DeleteMarkerEntry{ + delMarkers = append(delMarkers, s3response.DeleteMarkerEntry{ IsLatest: getBoolPtr(true), VersionId: &versionId, LastModified: backend.GetTimePtr(fi.ModTime()), @@ -1819,7 +1819,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { // First find the null versionId object(if exists) // before starting the object versions listing var nullVersionIdObj *s3response.ObjectVersion - var nullObjDelMarker *types.DeleteMarkerEntry + var nullObjDelMarker *s3response.DeleteMarkerEntry var nullPos nullVersionPos nf, err := os.Stat(filepath.Join(versionPath, nullVersionId)) if err != nil && !errors.Is(err, fs.ErrNotExist) { @@ -1838,7 +1838,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { // Check to see if the null versionId object is delete marker or not if isDel { - nullObjDelMarker = &types.DeleteMarkerEntry{ + nullObjDelMarker = &s3response.DeleteMarkerEntry{ VersionId: backend.GetPtrFromString("null"), LastModified: backend.GetTimePtr(nf.ModTime()), Key: &key, @@ -1961,7 +1961,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { } if isDel { - delMarkers = append(delMarkers, types.DeleteMarkerEntry{ + delMarkers = append(delMarkers, s3response.DeleteMarkerEntry{ VersionId: &versionId, LastModified: backend.GetTimePtr(f.ModTime()), Key: &key, @@ -5200,6 +5200,12 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( } } + // The delete marker is last modified when it's created, not + // when the version it hides was. Setting the time needs the + // entry to be owned by the gateway, otherwise it's left as is. + now := time.Now() + _ = os.Chtimes(objpath, now, now) + return &s3.DeleteObjectOutput{ DeleteMarker: getBoolPtr(true), VersionId: &versionId, diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index f2e60012..821d7b29 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -345,7 +345,7 @@ func (s *S3Proxy) ListObjectVersions(ctx context.Context, input *s3.ListObjectVe return s3response.ListVersionsResult{ CommonPrefixes: out.CommonPrefixes, - DeleteMarkers: out.DeleteMarkers, + DeleteMarkers: convertDeleteMarkers(out.DeleteMarkers), Delimiter: out.Delimiter, EncodingType: out.EncodingType, IsTruncated: out.IsTruncated, @@ -1931,3 +1931,18 @@ func convertObjectVersions(versions []types.ObjectVersion) []s3response.ObjectVe return result } + +func convertDeleteMarkers(markers []types.DeleteMarkerEntry) []s3response.DeleteMarkerEntry { + result := make([]s3response.DeleteMarkerEntry, 0, len(markers)) + for _, m := range markers { + result = append(result, s3response.DeleteMarkerEntry{ + IsLatest: m.IsLatest, + Key: m.Key, + LastModified: m.LastModified, + Owner: m.Owner, + VersionId: m.VersionId, + }) + } + + return result +} diff --git a/backend/walk.go b/backend/walk.go index d0b9bfda..6fc4af0f 100644 --- a/backend/walk.go +++ b/backend/walk.go @@ -130,7 +130,7 @@ func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker strin type WalkVersioningResults struct { CommonPrefixes []types.CommonPrefix ObjectVersions []s3response.ObjectVersion - DelMarkers []types.DeleteMarkerEntry + DelMarkers []s3response.DeleteMarkerEntry Truncated bool NextMarker string NextVersionIdMarker string @@ -138,7 +138,7 @@ type WalkVersioningResults struct { type ObjVersionFuncResult struct { ObjectVersions []s3response.ObjectVersion - DelMarkers []types.DeleteMarkerEntry + DelMarkers []s3response.DeleteMarkerEntry NextVersionIdMarker string Truncated bool } @@ -150,7 +150,7 @@ type GetVersionsFunc func(path, versionIdMarker string, pastVersionIdMarker *boo func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyMarker, versionIdMarker string, max int, getObj GetVersionsFunc, skipdirs []string) (WalkVersioningResults, error) { cpmap := cpMap{} var objects []s3response.ObjectVersion - var delMarkers []types.DeleteMarkerEntry + var delMarkers []s3response.DeleteMarkerEntry var pastMarker bool if keyMarker == "" { diff --git a/s3response/s3response.go b/s3response/s3response.go index d9a24a17..6b2c6edc 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -511,7 +511,7 @@ type InitiateMultipartUploadResult struct { type ListVersionsResult struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListVersionsResult" json:"-"` CommonPrefixes []types.CommonPrefix - DeleteMarkers []types.DeleteMarkerEntry `xml:"DeleteMarker"` + DeleteMarkers []DeleteMarkerEntry `xml:"DeleteMarker"` Delimiter *string EncodingType types.EncodingType IsTruncated *bool @@ -555,6 +555,30 @@ func (o ObjectVersion) MarshalXML(e *xml.Encoder, start xml.StartElement) error return e.EncodeElement(aux, start) } +type DeleteMarkerEntry struct { + IsLatest *bool + Key *string + LastModified *time.Time + Owner *types.Owner + VersionId *string +} + +func (d DeleteMarkerEntry) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + type Alias DeleteMarkerEntry + aux := &struct { + LastModified string `xml:"LastModified"` + *Alias + }{ + Alias: (*Alias)(&d), + } + + if d.LastModified != nil { + aux.LastModified = d.LastModified.UTC().Format(time.RFC3339) + } + + return e.EncodeElement(aux, start) +} + type GetBucketVersioningOutput struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ VersioningConfiguration" json:"-"` MFADelete *types.MFADeleteStatus diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 8ed9c9b2..d9835c87 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -2058,6 +2058,7 @@ func TestVersioning(ts *TestState) { 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_delete_marker_last_modified) ts.Run(Versioning_DeleteObject_non_existing_object) ts.Run(Versioning_DeleteObject_implicit_dir) ts.Run(Versioning_DeleteObject_trailing_slash_counterpart) @@ -3661,6 +3662,7 @@ func GetIntTests() IntTests { "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_delete_marker_last_modified": Versioning_DeleteObject_delete_marker_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, diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index 9d7d8bd5..5e4553a1 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -17,6 +17,7 @@ package integration import ( "context" "crypto/sha256" + "errors" "fmt" "io" "net/http" @@ -25,6 +26,7 @@ import ( "sync" "time" + awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/s3err" @@ -2961,6 +2963,207 @@ func Versioning_DeleteObject_promoted_version_last_modified(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +// Versioning_DeleteObject_delete_marker_last_modified creates delete markers +// over an object version, over another delete marker and, while versioning +// is suspended, over the null version. A delete marker is last modified when +// it's created, not when the version it hides was. +func Versioning_DeleteObject_delete_marker_last_modified(s *S3Conf) error { + testName := "Versioning_DeleteObject_delete_marker_last_modified" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + cases := []struct { + obj string + // the object is put and deleted while versioning is suspended + null bool + // the last modified time of the object version + lastModified *time.Time + // the delete markers, from the oldest to the newest + delMarkers []string + }{ + {obj: "my-obj"}, + {obj: "my-dir/"}, + {obj: "my-null-obj", null: true}, + {obj: "my-null-dir/", null: true}, + } + + // delMarkerLastModified returns the last modified time of the delete + // marker of obj, which the listing and the Last-Modified header of a + // HeadObject request for it report alike + delMarkerLastModified := func(obj, versionId string) (time.Time, error) { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{ + Bucket: &bucket, + Prefix: &obj, + }) + cancel() + if err != nil { + return time.Time{}, err + } + + var lastModified *time.Time + for _, dm := range res.DeleteMarkers { + if getString(dm.Key) == obj && getString(dm.VersionId) == versionId { + lastModified = dm.LastModified + } + } + if lastModified == nil { + return time.Time{}, fmt.Errorf("%v: expected the delete marker %v to be listed with LastModified, instead got %v", + obj, versionId, res.DeleteMarkers) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + VersionId: &versionId, + }) + cancel() + if err := checkSdkApiErr(err, "MethodNotAllowed"); err != nil { + return time.Time{}, err + } + var respErr *awshttp.ResponseError + if !errors.As(err, &respErr) { + return time.Time{}, fmt.Errorf("expected an HTTP response error, instead got %w", err) + } + header, err := http.ParseTime(respErr.Response.Header.Get("Last-Modified")) + if err != nil { + return time.Time{}, fmt.Errorf("%v: parse the delete marker %v Last-Modified header: %w", + obj, versionId, err) + } + if !header.Equal(*lastModified) { + return time.Time{}, fmt.Errorf("%v: expected the delete marker %v Last-Modified header to be %v, instead got %v", + obj, versionId, *lastModified, header) + } + + return *lastModified, nil + } + + // deleteObjects creates a delete marker for each of the cases the + // versioning status applies to + deleteObjects := func(null bool) error { + for i, c := range cases { + if c.null != null { + continue + } + 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 + } + cases[i].delMarkers = append(cases[i].delMarkers, getString(out.VersionId)) + } + return nil + } + + // the null versions are put and deleted 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 + } + _, err := putObjectWithData(objDataLen(c.obj, 100), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &c.obj, + }, s3client) + 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 { + return fmt.Errorf("%v: expected non nil LastModified", c.obj) + } + cases[i].lastModified = res.LastModified + } + } + + // every delete marker is created in a later second than the + // version or the delete marker it hides + time.Sleep(time.Second) + if err := deleteObjects(false); err != nil { + return err + } + err := putBucketVersioningStatus(s3client, bucket, types.BucketVersioningStatusSuspended) + if err != nil { + return err + } + if err := deleteObjects(true); err != nil { + return err + } + err = putBucketVersioningStatus(s3client, bucket, types.BucketVersioningStatusEnabled) + if err != nil { + return err + } + time.Sleep(time.Second) + if err := deleteObjects(false); err != nil { + return err + } + + for _, c := range cases { + prev := *c.lastModified + var lastModified []time.Time + for _, versionId := range c.delMarkers { + dmLastModified, err := delMarkerLastModified(c.obj, versionId) + if err != nil { + return err + } + if !dmLastModified.After(prev) { + return fmt.Errorf("%v: expected the delete marker %v LastModified %v to be after %v", + c.obj, versionId, dmLastModified, prev) + } + prev = dmLastModified + lastModified = append(lastModified, dmLastModified) + } + + if c.null { + continue + } + + // deleting the newest delete marker makes the one it hides + // the latest, with its own last modified time + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &c.obj, + VersionId: &c.delMarkers[1], + }) + cancel() + if err != nil { + return err + } + + dmLastModified, err := delMarkerLastModified(c.obj, c.delMarkers[0]) + if err != nil { + return err + } + if !dmLastModified.Equal(lastModified[0]) { + return fmt.Errorf("%v: expected the promoted delete marker LastModified to be %v, instead got %v", + c.obj, lastModified[0], dmLastModified) + } + } + + 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 {