From 51ba901737bd3205cc5b2d3a769792def295d113 Mon Sep 17 00:00:00 2001 From: Michal Niewrzal Date: Mon, 20 Nov 2023 16:03:15 +0100 Subject: [PATCH] satellite/metainfo: (Get|Download)Object returns MethodNotAllowed on delete marker For S3 compatibility we should not allow returning pure delete marker object. If metabase returns delete marker metainfo will return MethodNotAllowed rpc status. https://github.com/storj/storj/issues/6522 Change-Id: I89804b2bd22da0e5beec8f106e74b74733e19a52 --- go.mod | 2 +- go.sum | 4 ++-- satellite/metainfo/endpoint_object.go | 11 +++++++---- satellite/metainfo/endpoint_object_test.go | 21 ++++++++++++++------- testsuite/storjscan/go.mod | 2 +- testsuite/storjscan/go.sum | 4 ++-- testsuite/ui/go.mod | 2 +- testsuite/ui/go.sum | 4 ++-- 8 files changed, 30 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index 59772d39612a..2da27a9c204e 100644 --- a/go.mod +++ b/go.mod @@ -61,7 +61,7 @@ require ( golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e gopkg.in/segmentio/analytics-go.v3 v3.1.0 gopkg.in/yaml.v3 v3.0.1 - storj.io/common v0.0.0-20231120181527-7551d20a723d + storj.io/common v0.0.0-20231121103209-30983e2956c6 storj.io/drpc v0.0.33 storj.io/monkit-jaeger v0.0.0-20230707083646-f15e6e8b7e8c storj.io/private v0.0.0-20231012141933-ae62725d6691 diff --git a/go.sum b/go.sum index e80407a19483..d7ebe91125cd 100644 --- a/go.sum +++ b/go.sum @@ -1013,8 +1013,8 @@ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8 sourcegraph.com/sourcegraph/go-diff v0.5.0/go.mod h1:kuch7UrkMzY0X+p9CRK03kfuPQ2zzQcaEFbx8wA8rck= sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0= storj.io/common v0.0.0-20220719163320-cd2ef8e1b9b0/go.mod h1:mCYV6Ud5+cdbuaxdPD5Zht/HYaIn0sffnnws9ErkrMQ= -storj.io/common v0.0.0-20231120181527-7551d20a723d h1:grxc+N773G2/r2MENiLNC78sgFQYCIaZLCURy8j2S6I= -storj.io/common v0.0.0-20231120181527-7551d20a723d/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= +storj.io/common v0.0.0-20231121103209-30983e2956c6 h1:EjYzS9qr58wV7/798FG9ESTZODdtLkPVzlli9aCOSfI= +storj.io/common v0.0.0-20231121103209-30983e2956c6/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= storj.io/drpc v0.0.32/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg= storj.io/drpc v0.0.33 h1:yCGZ26r66ZdMP0IcTYsj7WDAUIIjzXk6DJhbhvt9FHI= storj.io/drpc v0.0.33/go.mod h1:vR804UNzhBa49NOJ6HeLjd2H3MakC1j5Gv8bsOQT6N4= diff --git a/satellite/metainfo/endpoint_object.go b/satellite/metainfo/endpoint_object.go index e08f1a936f65..5ddfdaf5ab86 100644 --- a/satellite/metainfo/endpoint_object.go +++ b/satellite/metainfo/endpoint_object.go @@ -392,10 +392,9 @@ func (endpoint *Endpoint) GetObject(ctx context.Context, req *pb.ObjectGetReques return nil, endpoint.convertMetabaseErr(err) } - // TODO(ver): initially we returned 'object not found' error if delete marker is returned but - // S3 HeadObject request in such case requires 'Method Not Allowed' error so libuplink needs - // to know that delete marker was retured. We can remove this comment when we will know that - // there is no better aproach. + if mbObject.Status.IsDeleteMarker() { + return nil, rpcstatus.Error(rpcstatus.MethodNotAllowed, "method not allowed") + } { tags := []eventkit.Tag{ @@ -535,6 +534,10 @@ func (endpoint *Endpoint) DownloadObject(ctx context.Context, req *pb.ObjectDown return nil, endpoint.convertMetabaseErr(err) } + if object.Status.IsDeleteMarker() { + return nil, rpcstatus.Error(rpcstatus.MethodNotAllowed, "method not allowed") + } + // get the range segments streamRange, err := calculateStreamRange(object, req.Range) if err != nil { diff --git a/satellite/metainfo/endpoint_object_test.go b/satellite/metainfo/endpoint_object_test.go index f5f84fd767f9..cf500e4ddde5 100644 --- a/satellite/metainfo/endpoint_object_test.go +++ b/satellite/metainfo/endpoint_object_test.go @@ -2845,18 +2845,25 @@ func TestEndpoint_Object_No_StorageNodes_Versioning(t *testing.T) { require.Error(t, err) require.True(t, errs2.IsRPC(err, rpcstatus.NotFound)) - // with version set we should get object delete marker - getResponse, err := satelliteSys.API.Metainfo.Endpoint.GetObject(ctx, &pb.GetObjectRequest{ + // with version set we should get MethodNotAllowed error + _, err = satelliteSys.API.Metainfo.Endpoint.GetObject(ctx, &pb.GetObjectRequest{ Header: &pb.RequestHeader{ApiKey: apiKey}, Bucket: []byte(bucketName), EncryptedObjectKey: []byte(objects[0].ObjectKey), ObjectVersion: response.Object.ObjectVersion, }) - require.NoError(t, err) - require.Zero(t, getResponse.Object.PlainSize) - require.Zero(t, getResponse.Object.TotalSize) - require.Nil(t, getResponse.Object.RedundancyScheme) - require.Equal(t, pb.Object_DELETE_MARKER_VERSIONED, getResponse.Object.Status) + require.Error(t, err) + require.True(t, errs2.IsRPC(err, rpcstatus.MethodNotAllowed)) + + // with version set we should get MethodNotAllowed error + _, err = satelliteSys.API.Metainfo.Endpoint.DownloadObject(ctx, &pb.DownloadObjectRequest{ + Header: &pb.RequestHeader{ApiKey: apiKey}, + Bucket: []byte(bucketName), + EncryptedObjectKey: []byte(objects[0].ObjectKey), + ObjectVersion: response.Object.ObjectVersion, + }) + require.Error(t, err) + require.True(t, errs2.IsRPC(err, rpcstatus.MethodNotAllowed)) }) t.Run("listing objects, different versioning state", func(t *testing.T) { diff --git a/testsuite/storjscan/go.mod b/testsuite/storjscan/go.mod index d204a7a8e11f..2bdb20282613 100644 --- a/testsuite/storjscan/go.mod +++ b/testsuite/storjscan/go.mod @@ -9,7 +9,7 @@ require ( github.com/zeebo/errs v1.3.0 go.uber.org/zap v1.21.0 golang.org/x/sync v0.3.0 - storj.io/common v0.0.0-20231120181527-7551d20a723d + storj.io/common v0.0.0-20231121103209-30983e2956c6 storj.io/private v0.0.0-20231012141933-ae62725d6691 storj.io/storj v1.63.1 storj.io/storjscan v0.0.0-20220926140643-1623c3b391b0 diff --git a/testsuite/storjscan/go.sum b/testsuite/storjscan/go.sum index 47be457cd887..6190490f746f 100644 --- a/testsuite/storjscan/go.sum +++ b/testsuite/storjscan/go.sum @@ -1255,8 +1255,8 @@ rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= sourcegraph.com/sourcegraph/go-diff v0.5.0/go.mod h1:kuch7UrkMzY0X+p9CRK03kfuPQ2zzQcaEFbx8wA8rck= sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0= storj.io/common v0.0.0-20220719163320-cd2ef8e1b9b0/go.mod h1:mCYV6Ud5+cdbuaxdPD5Zht/HYaIn0sffnnws9ErkrMQ= -storj.io/common v0.0.0-20231120181527-7551d20a723d h1:grxc+N773G2/r2MENiLNC78sgFQYCIaZLCURy8j2S6I= -storj.io/common v0.0.0-20231120181527-7551d20a723d/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= +storj.io/common v0.0.0-20231121103209-30983e2956c6 h1:EjYzS9qr58wV7/798FG9ESTZODdtLkPVzlli9aCOSfI= +storj.io/common v0.0.0-20231121103209-30983e2956c6/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= storj.io/drpc v0.0.32/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg= storj.io/drpc v0.0.33 h1:yCGZ26r66ZdMP0IcTYsj7WDAUIIjzXk6DJhbhvt9FHI= storj.io/drpc v0.0.33/go.mod h1:vR804UNzhBa49NOJ6HeLjd2H3MakC1j5Gv8bsOQT6N4= diff --git a/testsuite/ui/go.mod b/testsuite/ui/go.mod index 5077bae883d2..2a04eabaf7a9 100644 --- a/testsuite/ui/go.mod +++ b/testsuite/ui/go.mod @@ -10,7 +10,7 @@ require ( github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.23.0 - storj.io/common v0.0.0-20231120181527-7551d20a723d + storj.io/common v0.0.0-20231121103209-30983e2956c6 storj.io/gateway-mt v1.51.1-0.20230417204402-7d9bb25bc297 storj.io/private v0.0.0-20231012141933-ae62725d6691 storj.io/storj v0.12.1-0.20221125175451-ef4b564b82f7 diff --git a/testsuite/ui/go.sum b/testsuite/ui/go.sum index 93ced691a218..1f0e29aded86 100644 --- a/testsuite/ui/go.sum +++ b/testsuite/ui/go.sum @@ -1970,8 +1970,8 @@ sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0/go.mod h1 sourcegraph.com/sourcegraph/go-diff v0.5.0/go.mod h1:kuch7UrkMzY0X+p9CRK03kfuPQ2zzQcaEFbx8wA8rck= sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0= storj.io/common v0.0.0-20220719163320-cd2ef8e1b9b0/go.mod h1:mCYV6Ud5+cdbuaxdPD5Zht/HYaIn0sffnnws9ErkrMQ= -storj.io/common v0.0.0-20231120181527-7551d20a723d h1:grxc+N773G2/r2MENiLNC78sgFQYCIaZLCURy8j2S6I= -storj.io/common v0.0.0-20231120181527-7551d20a723d/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= +storj.io/common v0.0.0-20231121103209-30983e2956c6 h1:EjYzS9qr58wV7/798FG9ESTZODdtLkPVzlli9aCOSfI= +storj.io/common v0.0.0-20231121103209-30983e2956c6/go.mod h1:qjHfzW5RlGg5z04CwIEjJd1eQ3HCGhUNtxZ6K/W7yqM= storj.io/dotworld v0.0.0-20210324183515-0d11aeccd840 h1:oqMwoF6vaOrCe92SKRyr8cc2WSjLYAd8fjpAHA7rNqY= storj.io/drpc v0.0.32/go.mod h1:6rcOyR/QQkSTX/9L5ZGtlZaE2PtXTTZl8d+ulSeeYEg= storj.io/drpc v0.0.33 h1:yCGZ26r66ZdMP0IcTYsj7WDAUIIjzXk6DJhbhvt9FHI=