[#166] Fix getting s3 object with the FrostFS OID name

Prioritize getting s3 object with the key, which equals to valid FrostFS OID, rather than getting non-existent object with OID via native protocol for GET and HEAD requests

Signed-off-by: Nikita Zinkevich <n.zinkevich@yadro.com>

internal/data/tree.go

@@ -1,4 +1,4 @@
-package api
+package data
 
 import (
 	oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
@@ -7,12 +7,21 @@ import (
 // NodeVersion represent node from tree service.
 type NodeVersion struct {
 	BaseNodeVersion
-	DeleteMarker bool
-	IsPrefixNode bool
 }
 
 // BaseNodeVersion is minimal node info from tree service.
 // Basically used for "system" object.
 type BaseNodeVersion struct {
-	OID oid.ID
+	ID             uint64
+	OID            oid.ID
+	IsDeleteMarker bool
+}
+
+type NodeInfo struct {
+	Meta []NodeMeta
+}
+
+type NodeMeta interface {
+	GetKey() string
+	GetValue() []byte
 }

internal/handler/browse.go

@@ -170,7 +170,7 @@ func (h *Handler) getDirObjectsS3(ctx context.Context, bucketInfo *data.BucketIn
 		objects: make([]ResponseObject, 0, len(nodes)),
 	}
 	for _, node := range nodes {
-		meta := node.GetMeta()
+		meta := node.Meta
 		if meta == nil {
 			continue
 		}

internal/handler/download.go

@@ -4,12 +4,14 @@ import (
 	"archive/zip"
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"time"
 
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/layer"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/response"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
@@ -23,21 +25,46 @@ import (
 
 // DownloadByAddressOrBucketName handles download requests using simple cid/oid or bucketname/key format.
 func (h *Handler) DownloadByAddressOrBucketName(c *fasthttp.RequestCtx) {
-	oidURLParam := c.UserValue("oid").(string)
-	downloadQueryParam := c.QueryArgs().GetBool("download")
+	cidParam := c.UserValue("cid").(string)
+	oidParam := c.UserValue("oid").(string)
+	downloadParam := c.QueryArgs().GetBool("download")
 
-	switch {
-	case isObjectID(oidURLParam):
-		h.byNativeAddress(c, h.receiveFile)
-	case !isContainerRoot(oidURLParam) && (downloadQueryParam || !isDir(oidURLParam)):
-		h.byS3Path(c, h.receiveFile)
-	default:
-		h.browseIndex(c)
+	ctx := utils.GetContextFromRequest(c)
+	log := utils.GetReqLogOrDefault(ctx, h.log).With(
+		zap.String("cid", cidParam),
+		zap.String("oid", oidParam),
+	)
+
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
+	if err != nil {
+		logAndSendBucketError(c, log, err)
+		return
+	}
+
+	checkS3Err := h.tree.CheckSettingsNodeExists(ctx, bktInfo)
+	if checkS3Err != nil && !errors.Is(checkS3Err, layer.ErrNodeNotFound) {
+		logAndSendBucketError(c, log, checkS3Err)
+		return
+	}
+
+	req := h.newRequest(c, log)
+
+	var objID oid.ID
+	if checkS3Err == nil && shouldDownload(oidParam, downloadParam) {
+		h.byS3Path(ctx, req, bktInfo.CID, oidParam, h.receiveFile)
+	} else if err = objID.DecodeString(oidParam); err == nil {
+		h.byNativeAddress(ctx, req, bktInfo.CID, objID, h.receiveFile)
+	} else {
+		h.browseIndex(c, checkS3Err != nil)
 	}
 }
 
-func (h *Handler) newRequest(ctx *fasthttp.RequestCtx, log *zap.Logger) *request {
-	return &request{
+func shouldDownload(oidParam string, downloadParam bool) bool {
+	return !isDir(oidParam) || downloadParam
+}
+
+func (h *Handler) newRequest(ctx *fasthttp.RequestCtx, log *zap.Logger) request {
+	return request{
 		RequestCtx: ctx,
 		log:        log,
 	}

internal/handler/handler.go

@@ -11,9 +11,9 @@ import (
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/cache"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/middleware"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/layer"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/response"
-	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
 	apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
@@ -165,7 +165,7 @@ type Handler struct {
 	ownerID           *user.ID
 	config            Config
 	containerResolver ContainerResolver
-	tree              *tree.Tree
+	tree              layer.TreeService
 	cache             *cache.BucketCache
 	workerPool        *ants.Pool
 }
@@ -178,7 +178,7 @@ type AppParams struct {
 	Cache    *cache.BucketCache
 }
 
-func New(params *AppParams, config Config, tree *tree.Tree, workerPool *ants.Pool) *Handler {
+func New(params *AppParams, config Config, tree layer.TreeService, workerPool *ants.Pool) *Handler {
 	return &Handler{
 		log:               params.Logger,
 		frostfs:           params.FrostFS,
@@ -193,77 +193,34 @@ func New(params *AppParams, config Config, tree *tree.Tree, workerPool *ants.Poo
 
 // byNativeAddress is a wrapper for function (e.g. request.headObject, request.receiveFile) that
 // prepares request and object address to it.
-func (h *Handler) byNativeAddress(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	idCnr, _ := c.UserValue("cid").(string)
-	idObj, _ := url.PathUnescape(c.UserValue("oid").(string))
-
-	ctx := utils.GetContextFromRequest(c)
-	reqLog := utils.GetReqLogOrDefault(ctx, h.log)
-	log := reqLog.With(zap.String("cid", idCnr), zap.String("oid", idObj))
-
-	bktInfo, err := h.getBucketInfo(ctx, idCnr, log)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	objID := new(oid.ID)
-	if err = objID.DecodeString(idObj); err != nil {
-		log.Error(logs.WrongObjectID, zap.Error(err))
-		response.Error(c, "wrong object id", fasthttp.StatusBadRequest)
-		return
-	}
-
-	addr := newAddress(bktInfo.CID, *objID)
-
-	f(ctx, *h.newRequest(c, log), addr)
+func (h *Handler) byNativeAddress(ctx context.Context, req request, cnrID cid.ID, objID oid.ID, handler func(context.Context, request, oid.Address)) {
+	addr := newAddress(cnrID, objID)
+	handler(ctx, req, addr)
 }
 
 // byS3Path is a wrapper for function (e.g. request.headObject, request.receiveFile) that
 // resolves object address from S3-like path <bucket name>/<object key>.
-func (h *Handler) byS3Path(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	bucketname := c.UserValue("cid").(string)
-	key := c.UserValue("oid").(string)
+func (h *Handler) byS3Path(ctx context.Context, req request, cnrID cid.ID, path string, handler func(context.Context, request, oid.Address)) {
+	c, log := req.RequestCtx, req.log
 
-	ctx := utils.GetContextFromRequest(c)
-	reqLog := utils.GetReqLogOrDefault(ctx, h.log)
-	log := reqLog.With(zap.String("bucketname", bucketname), zap.String("key", key))
-
-	unescapedKey, err := url.QueryUnescape(key)
+	foundOID, err := h.tree.GetLatestVersion(ctx, &cnrID, path)
 	if err != nil {
 		logAndSendBucketError(c, log, err)
 		return
 	}
-
-	bktInfo, err := h.getBucketInfo(ctx, bucketname, log)
-	if err != nil {
-		logAndSendBucketError(c, log, err)
-		return
-	}
-
-	foundOid, err := h.tree.GetLatestVersion(ctx, &bktInfo.CID, unescapedKey)
-	if err != nil {
-		if errors.Is(err, tree.ErrNodeAccessDenied) {
-			response.Error(c, "Access Denied", fasthttp.StatusForbidden)
-		} else {
-			response.Error(c, "object wasn't found", fasthttp.StatusNotFound)
-			log.Error(logs.GetLatestObjectVersion, zap.Error(err))
-		}
-		return
-	}
-	if foundOid.DeleteMarker {
+	if foundOID.IsDeleteMarker {
 		log.Error(logs.ObjectWasDeleted)
 		response.Error(c, "object deleted", fasthttp.StatusNotFound)
 		return
 	}
-	addr := newAddress(bktInfo.CID, foundOid.OID)
 
-	f(ctx, *h.newRequest(c, log), addr)
+	addr := newAddress(cnrID, foundOID.OID)
+	handler(ctx, h.newRequest(c, log), addr)
 }
 
 // byAttribute is a wrapper similar to byNativeAddress.
-func (h *Handler) byAttribute(c *fasthttp.RequestCtx, f func(context.Context, request, oid.Address)) {
-	scid, _ := c.UserValue("cid").(string)
+func (h *Handler) byAttribute(c *fasthttp.RequestCtx, handler func(context.Context, request, oid.Address)) {
+	cidParam, _ := c.UserValue("cid").(string)
 	key, _ := c.UserValue("attr_key").(string)
 	val, _ := c.UserValue("attr_val").(string)
 
@@ -272,21 +229,21 @@ func (h *Handler) byAttribute(c *fasthttp.RequestCtx, f func(context.Context, re
 
 	key, err := url.QueryUnescape(key)
 	if err != nil {
-		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_key", key), zap.Error(err))
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", cidParam), zap.String("attr_key", key), zap.Error(err))
 		response.Error(c, "could not unescape attr_key: "+err.Error(), fasthttp.StatusBadRequest)
 		return
 	}
 
 	val, err = url.QueryUnescape(val)
 	if err != nil {
-		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", scid), zap.String("attr_val", val), zap.Error(err))
+		log.Error(logs.FailedToUnescapeQuery, zap.String("cid", cidParam), zap.String("attr_val", val), zap.Error(err))
 		response.Error(c, "could not unescape attr_val: "+err.Error(), fasthttp.StatusBadRequest)
 		return
 	}
 
-	log = log.With(zap.String("cid", scid), zap.String("attr_key", key), zap.String("attr_val", val))
+	log = log.With(zap.String("cid", cidParam), zap.String("attr_key", key), zap.String("attr_val", val))
 
-	bktInfo, err := h.getBucketInfo(ctx, scid, log)
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
 	if err != nil {
 		logAndSendBucketError(c, log, err)
 		return
@@ -303,11 +260,11 @@ func (h *Handler) byAttribute(c *fasthttp.RequestCtx, f func(context.Context, re
 		return
 	}
 
-	var addrObj oid.Address
-	addrObj.SetContainer(bktInfo.CID)
-	addrObj.SetObject(objID)
+	var addr oid.Address
+	addr.SetContainer(bktInfo.CID)
+	addr.SetObject(objID)
 
-	f(ctx, *h.newRequest(c, log), addrObj)
+	handler(ctx, h.newRequest(c, log), addr)
 }
 
 func (h *Handler) findObjectByAttribute(ctx context.Context, log *zap.Logger, cnrID cid.ID, attrKey, attrVal string) (oid.ID, error) {
@@ -412,7 +369,7 @@ func (h *Handler) readContainer(ctx context.Context, cnrID cid.ID) (*data.Bucket
 	return bktInfo, err
 }
 
-func (h *Handler) browseIndex(c *fasthttp.RequestCtx) {
+func (h *Handler) browseIndex(c *fasthttp.RequestCtx, isNativeList bool) {
 	if !h.config.IndexPageEnabled() {
 		c.SetStatusCode(fasthttp.StatusNotFound)
 		return
@@ -438,18 +395,9 @@ func (h *Handler) browseIndex(c *fasthttp.RequestCtx) {
 	}
 
 	listFunc := h.getDirObjectsS3
-	isNativeList := false
-
-	err = h.tree.CheckSettingsNodeExist(ctx, bktInfo)
-	if err != nil {
-		if errors.Is(err, tree.ErrNodeNotFound) {
-			// tree probe failed, try to use native
-			listFunc = h.getDirObjectsNative
-			isNativeList = true
-		} else {
-			logAndSendBucketError(c, log, err)
-			return
-		}
+	if isNativeList {
+		// tree probe failed, trying to use native
+		listFunc = h.getDirObjectsNative
 	}
 
 	h.browseObjects(c, browseParams{

internal/handler/handler_test.go

@@ -14,8 +14,8 @@ import (
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/cache"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/middleware"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/layer"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/resolver"
-	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl"
@@ -32,14 +32,29 @@ import (
 	"go.uber.org/zap"
 )
 
-type treeClientMock struct {
+type treeServiceMock struct {
+	system map[string]map[string]*data.BaseNodeVersion
 }
 
-func (t *treeClientMock) GetNodes(context.Context, *tree.GetNodesParams) ([]tree.NodeResponse, error) {
-	return nil, nil
+func newTreeService() *treeServiceMock {
+	return &treeServiceMock{
+		system: make(map[string]map[string]*data.BaseNodeVersion),
+	}
 }
 
-func (t *treeClientMock) GetSubTree(context.Context, *data.BucketInfo, string, []uint64, uint32, bool) ([]tree.NodeResponse, error) {
+func (t *treeServiceMock) CheckSettingsNodeExists(context.Context, *data.BucketInfo) error {
+	_, ok := t.system["bucket-settings"]
+	if !ok {
+		return layer.ErrNodeNotFound
+	}
+	return nil
+}
+
+func (t *treeServiceMock) GetSubTreeByPrefix(context.Context, *data.BucketInfo, string, bool) ([]data.NodeInfo, string, error) {
+	return nil, "", nil
+}
+
+func (t *treeServiceMock) GetLatestVersion(context.Context, *cid.ID, string) (*data.NodeVersion, error) {
 	return nil, nil
 }
 
@@ -89,7 +104,7 @@ type handlerContext struct {
 
 	h       *Handler
 	frostfs *TestFrostFS
-	tree    *treeClientMock
+	tree    *treeServiceMock
 	cfg     *configMock
 }
 
@@ -130,14 +145,14 @@ func prepareHandlerContext() (*handlerContext, error) {
 		}),
 	}
 
-	treeMock := &treeClientMock{}
+	treeMock := newTreeService()
 	cfgMock := &configMock{}
 
-	workerPool, err := ants.NewPool(1000)
+	workerPool, err := ants.NewPool(1)
 	if err != nil {
 		return nil, err
 	}
-	handler := New(params, cfgMock, tree.NewTree(treeMock), workerPool)
+	handler := New(params, cfgMock, treeMock, workerPool)
 
 	return &handlerContext{
 		key:     key,

internal/handler/head.go

@@ -2,11 +2,13 @@ package handler
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"strconv"
 	"time"
 
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/layer"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs"
 	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
@@ -102,14 +104,36 @@ func idsToResponse(resp *fasthttp.Response, obj *object.Object) {
 
 // HeadByAddressOrBucketName handles head requests using simple cid/oid or bucketname/key format.
 func (h *Handler) HeadByAddressOrBucketName(c *fasthttp.RequestCtx) {
-	test, _ := c.UserValue("oid").(string)
-	var id oid.ID
+	cidParam, _ := c.UserValue("cid").(string)
+	oidParam, _ := c.UserValue("oid").(string)
 
-	err := id.DecodeString(test)
+	ctx := utils.GetContextFromRequest(c)
+	log := utils.GetReqLogOrDefault(ctx, h.log).With(
+		zap.String("cid", cidParam),
+		zap.String("oid", oidParam),
+	)
+
+	bktInfo, err := h.getBucketInfo(ctx, cidParam, log)
 	if err != nil {
-		h.byS3Path(c, h.headObject)
+		logAndSendBucketError(c, log, err)
+		return
+	}
+	checkS3Err := h.tree.CheckSettingsNodeExists(ctx, bktInfo)
+	if checkS3Err != nil && !errors.Is(checkS3Err, layer.ErrNodeNotFound) {
+		logAndSendBucketError(c, log, checkS3Err)
+		return
+	}
+
+	req := h.newRequest(c, log)
+
+	var objID oid.ID
+	if checkS3Err == nil {
+		h.byS3Path(ctx, req, bktInfo.CID, oidParam, h.headObject)
+	} else if err = objID.DecodeString(oidParam); err == nil {
+		h.byNativeAddress(ctx, req, bktInfo.CID, objID, h.headObject)
 	} else {
-		h.byNativeAddress(c, h.headObject)
+		logAndSendBucketError(c, log, checkS3Err)
+		return
 	}
 }
 

internal/handler/utils.go

@@ -42,16 +42,7 @@ func bearerToken(ctx context.Context) *bearer.Token {
 }
 
 func isDir(name string) bool {
-	return strings.HasSuffix(name, "/")
-}
-
-func isObjectID(s string) bool {
-	var objID oid.ID
-	return objID.DecodeString(s) == nil
-}
-
-func isContainerRoot(key string) bool {
-	return key == ""
+	return name == "" || strings.HasSuffix(name, "/")
 }
 
 func loadAttributes(attrs []object.Attribute) map[string]string {

internal/layer/tree_service.go

@@ -4,13 +4,15 @@ import (
 	"context"
 	"errors"
 
-	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api"
+	"git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data"
 	cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
 )
 
 // TreeService provide interface to interact with tree service using s3 data models.
 type TreeService interface {
-	GetLatestVersion(ctx context.Context, cnrID *cid.ID, objectName string) (*api.NodeVersion, error)
+	GetLatestVersion(ctx context.Context, cnrID *cid.ID, objectName string) (*data.NodeVersion, error)
+	GetSubTreeByPrefix(ctx context.Context, bktInfo *data.BucketInfo, prefix string, latestOnly bool) ([]data.NodeInfo, string, error)
+	CheckSettingsNodeExists(ctx context.Context, bktInfo *data.BucketInfo) error
 }
 
 var (

internal/logs/logs.go

@@ -79,6 +79,11 @@ const (
 	InvalidLifetimeUsingDefaultValue                                      = "invalid lifetime, using default value (in seconds)"                                     // Error in ../../cmd/http-gw/settings.go
 	InvalidCacheSizeUsingDefaultValue                                     = "invalid cache size, using default value"                                                // Error in ../../cmd/http-gw/settings.go
 	FailedToUnescapeQuery                                                 = "failed to unescape query"
+	FailedToParseAddressInTreeNode                                        = "failed to parse object addr in tree node"
+	SettingsNodeInvalidOwnerKey                                           = "settings node: invalid owner key"
+	SystemNodeHasMultipleIDs                                              = "system node has multiple ids"
+	FailedToRemoveOldSystemNode                                           = "failed to remove old system node"
+	BucketSettingsNodeHasMultipleIDs                                      = "bucket settings node has multiple ids"
 	ServerReconnecting                                                    = "reconnecting server..."
 	ServerReconnectedSuccessfully                                         = "server reconnected successfully"
 	ServerReconnectFailed                                                 = "failed to reconnect server"