From be03c5178f36a6461e199327c7e57186abb5b79f Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 29 Jun 2023 15:46:42 +0300 Subject: [PATCH] [#143] Fix NoSuchKey error on get/head Signed-off-by: Denis Kirillov --- api/handler/delete_test.go | 25 ++++++++++++++++++++----- api/handler/handlers_test.go | 33 ++++++++++++++++++++++++++++++++- api/handler/head_test.go | 20 ++++++++++++++++++++ api/layer/object.go | 3 +++ 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go index 713aad4..7c5160c 100644 --- a/api/handler/delete_test.go +++ b/api/handler/delete_test.go @@ -8,6 +8,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" + apiErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "github.com/stretchr/testify/require" @@ -25,11 +26,7 @@ func TestDeleteBucketOnAlreadyRemovedError(t *testing.T) { putObject(t, hc, bktName, objName) - nodeVersion, err := hc.tree.GetUnversioned(hc.context, bktInfo, objName) - require.NoError(t, err) - var addr oid.Address - addr.SetContainer(bktInfo.CID) - addr.SetObject(nodeVersion.OID) + addr := getAddressOfLastVersion(hc, bktInfo, objName) hc.tp.SetObjectError(addr, apistatus.ObjectAlreadyRemoved{}) deleteObjects(t, hc, bktName, [][2]string{{objName, emptyVersion}}) @@ -37,6 +34,15 @@ func TestDeleteBucketOnAlreadyRemovedError(t *testing.T) { deleteBucket(t, hc, bktName, http.StatusNoContent) } +func getAddressOfLastVersion(hc *handlerContext, bktInfo *data.BucketInfo, objName string) oid.Address { + nodeVersion, err := hc.tree.GetLatestVersion(hc.context, bktInfo, objName) + require.NoError(hc.t, err) + var addr oid.Address + addr.SetContainer(bktInfo.CID) + addr.SetObject(nodeVersion.OID) + return addr +} + func TestDeleteBucket(t *testing.T) { tc := prepareHandlerContext(t) @@ -434,6 +440,15 @@ func checkNotFound(t *testing.T, tc *handlerContext, bktName, objName, version s assertStatus(t, w, http.StatusNotFound) } +func headObjectAssertS3Error(hc *handlerContext, bktName, objName, version string, code apiErrors.ErrorCode) { + query := make(url.Values) + query.Add(api.QueryVersionID, version) + + w, r := prepareTestFullRequest(hc, bktName, objName, query, nil) + hc.Handler().HeadObjectHandler(w, r) + assertS3Error(hc.t, w, apiErrors.GetAPIError(code)) +} + func checkFound(t *testing.T, tc *handlerContext, bktName, objName, version string) { query := make(url.Values) query.Add(api.QueryVersionID, version) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index b817600..df5677f 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -14,6 +14,7 @@ import ( "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/cache" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/resolver" @@ -83,6 +84,14 @@ func (p *xmlDecoderProviderMock) NewCompleteMultipartDecoder(r io.Reader) *xml.D } func prepareHandlerContext(t *testing.T) *handlerContext { + return prepareHandlerContextBase(t, false) +} + +func prepareHandlerContextWithMinCache(t *testing.T) *handlerContext { + return prepareHandlerContextBase(t, true) +} + +func prepareHandlerContextBase(t *testing.T, minCache bool) *handlerContext { key, err := keys.NewPrivateKey() require.NoError(t, err) @@ -99,8 +108,13 @@ func prepareHandlerContext(t *testing.T) *handlerContext { treeMock := NewTreeServiceMock(t) + cacheCfg := layer.DefaultCachesConfigs(l) + if minCache { + cacheCfg = getMinCacheConfig(l) + } + layerCfg := &layer.Config{ - Caches: layer.DefaultCachesConfigs(zap.NewExample()), + Caches: cacheCfg, AnonKey: layer.AnonymousKey{Key: key}, Resolver: testResolver, TreeService: treeMock, @@ -129,6 +143,23 @@ func prepareHandlerContext(t *testing.T) *handlerContext { } } +func getMinCacheConfig(logger *zap.Logger) *layer.CachesConfig { + minCacheCfg := &cache.Config{ + Size: 1, + Lifetime: 1, + Logger: logger, + } + return &layer.CachesConfig{ + Logger: logger, + Objects: minCacheCfg, + ObjectsList: minCacheCfg, + Names: minCacheCfg, + Buckets: minCacheCfg, + System: minCacheCfg, + AccessControl: minCacheCfg, + } +} + func NewTreeServiceMock(t *testing.T) *tree.Tree { memCli, err := tree.NewTreeServiceClientMemory() require.NoError(t, err) diff --git a/api/handler/head_test.go b/api/handler/head_test.go index 2c1ea1e..a0e4a8a 100644 --- a/api/handler/head_test.go +++ b/api/handler/head_test.go @@ -7,8 +7,10 @@ import ( "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" + s3errors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" + apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/stretchr/testify/require" @@ -86,6 +88,24 @@ func TestInvalidAccessThroughCache(t *testing.T) { assertStatus(t, w, http.StatusForbidden) } +func TestHeadObject(t *testing.T) { + hc := prepareHandlerContextWithMinCache(t) + bktName, objName := "bucket", "obj" + bktInfo, objInfo := createVersionedBucketAndObject(hc.t, hc, bktName, objName) + + putObject(hc.t, hc, bktName, objName) + + checkFound(hc.t, hc, bktName, objName, objInfo.VersionID()) + checkFound(hc.t, hc, bktName, objName, emptyVersion) + + addr := getAddressOfLastVersion(hc, bktInfo, objName) + hc.tp.SetObjectError(addr, apistatus.ObjectNotFound{}) + hc.tp.SetObjectError(objInfo.Address(), apistatus.ObjectNotFound{}) + + headObjectAssertS3Error(hc, bktName, objName, objInfo.VersionID(), s3errors.ErrNoSuchVersion) + headObjectAssertS3Error(hc, bktName, objName, emptyVersion, s3errors.ErrNoSuchKey) +} + func TestIsAvailableToResolve(t *testing.T) { list := []string{"container", "s3"} diff --git a/api/layer/object.go b/api/layer/object.go index 81c112a..95f7b9d 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -319,6 +319,9 @@ func (n *layer) headLastVersionIfNotDeleted(ctx context.Context, bkt *data.Bucke meta, err := n.objectHead(ctx, bkt, node.OID) if err != nil { + if client.IsErrObjectNotFound(err) { + return nil, fmt.Errorf("%w: %s", apiErrors.GetAPIError(apiErrors.ErrNoSuchKey), err.Error()) + } return nil, err } objInfo := objectInfoFromMeta(bkt, meta)