#30 add object name resolving #43
Labels
No Label
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-http-gw#43
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "pogpp/frostfs-http-gw:feature/tree_object_name_resolving"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
Signed-off-by: Pavel Pogodaev p.pogodaev@yadro.com
@ -162,6 +169,7 @@ func newApp(ctx context.Context, opt ...Option) App {
if err != nil {
a.log.Fatal("failed to dial pool", zap.Error(err))
}
a.key = getWalletKey(a.log, a.cfg)
Actually, you have already got this key on line 128. Probably you have to change some methods signature a little, but still
@ -0,0 +1,61 @@
package wallet
Can be dropped
@ -44,6 +50,7 @@ type (
services []*metrics.Service
settings *appSettings
servers []Server
treeSvc *tree.Tree
Currently this is unused
@ -0,0 +1,28 @@
package accessbox
Can be dropped along with
creds/accessbox/accessbox.pb.go
@ -0,0 +292,4 @@
}
func getBearer(ctx context.Context, bktInfo *data.BucketInfo) []byte {
if bd, ok := ctx.Value(api.BoxData).(*accessbox.Box); ok && bd != nil && bd.Gate != nil {
You should set bearer token to context there using something like that and use something like this here to get token back
@ -0,0 +1,174 @@
package layer
For now can be droped
@ -0,0 +8,4 @@
)
// TreeService provide interface to interact with tree service using s3 data models.
type TreeService interface {
We can simplify to:
@ -0,0 +1,7 @@
package api
Can be dropped
@ -0,0 +1,39 @@
package data
Can be dropped
@ -0,0 +1,50 @@
package data
Can be dropped
@ -0,0 +1,127 @@
package data
Can be dropped
@ -211,6 +213,7 @@ type Downloader struct {
pool *pool.Pool
containerResolver *resolver.ContainerResolver
settings *Settings
tree *tree.Tree
We should use interface
TreeService
hereOr we can postpone this, if we don't want to add tree mock now
713da1ac48
tofcbcf8ec8a
@ -0,0 +21,4 @@
// ServiceClient is a client to interact with tree service.
// Each method must return ErrNodeNotFound or ErrNodeAccessDenied if relevant.
ServiceClient interface {
Can be simplified to:
@ -0,0 +40,4 @@
}
GetNodesParams struct {
BktInfo *data.BucketInfo
Use
CnrID cid.ID
@ -0,0 +256,4 @@
return nil
}
func (c *ServiceClientGRPC) RemoveNode(ctx context.Context, bktInfo *data.BucketInfo, treeID string, nodeID uint64) error {
Drop this method and other unused
@ -0,0 +65,4 @@
}
// MultipartInfo is multipart upload information.
type MultipartInfo struct {
Drop this and other unused
fcbcf8ec8a
to187b3b3044
@ -119,3 +124,3 @@
a.webServer.StreamRequestBody = a.cfg.GetBool(cfgWebStreamRequestBody)
// -- -- -- -- -- -- -- -- -- -- -- -- -- --
key, err = getFrostFSKey(a)
pvtKey, err := getFrostFSKey(a)
We can simply write:
@ -120,2 +125,3 @@
// -- -- -- -- -- -- -- -- -- -- -- -- -- --
key, err = getFrostFSKey(a)
pvtKey, err := getFrostFSKey(a)
key = &pvtKey.PrivateKey
This cause panic if
err != nil
. Let's drop this line and below (line 136-137) write:@ -469,3 +477,3 @@
r.GET("/zip/{cid}/{prefix:*}", a.logger(downloadRoutes.DownloadZipped))
a.log.Info("added path /zip/{cid}/{prefix}")
r.GET("/s3get/{bucketname}/{key}", a.logger(downloadRoutes.DownloadByBucketName))
Probably we should use existing route
/get/{cid}/{oid}
@ -283,0 +314,4 @@
addr.SetContainer(*cnrID)
// TODO where i've to take UID?
foundOid, err := d.tree.GetLatestVersion(d.appCtx, cnrID, user.ID{}, key)
You don't have to use
user.ID
. You can just dropif
statement hereAlso you save bearer token to
c
but futher try to get it fromd.appCtx
so this won't work.@ -283,0 +315,4 @@
// TODO where i've to take UID?
foundOid, err := d.tree.GetLatestVersion(d.appCtx, cnrID, user.ID{}, key)
if err != nil {
You should also check if
foundOid.IsDeleteMarker()
is false otherwise returns404
@ -283,0 +292,4 @@
// prepares request and object address to it.
func (d *Downloader) byBucketname(c *fasthttp.RequestCtx, f func(request, *pool.Pool, oid.Address)) {
var (
bucketname, _ = c.UserValue("bucketname").(string)
Actually we can omit
, _
and write:@pogpp @dkirillov What you think about removing gRPC generated code from the repo and having the same script as in S3 Gateway?
69d8779daf/Makefile (L46)
@ -0,0 +30,4 @@
type BaseNodeVersion struct {
ID uint64
ParenID uint64
OID oid.ID
As far as I understand, none of these attributes in the tree are not used except
OID
in the code.While S3 Gateway requires data about etag, size, timestamp, etc., HTTP gateway requires only OID to make GET request to the storage node.
@ -0,0 +180,4 @@
}
func (c *Tree) GetLatestVersion(ctx context.Context, cnrID *cid.ID, UID user.ID, objectName string) (*api.NodeVersion, error) {
meta := []string{oidKV, isUnversionedKV, isDeleteMarkerKV, etagKV, sizeKV}
Can we simplify it by removing
etagKV
andsizeKV
? I don't think this is required to fetch OID from the tree.187b3b3044
toae98ef7daf
ae98ef7daf
to43e2b44045
@ -0,0 +10,4 @@
// NodeVersion represent node from tree service.
type NodeVersion struct {
BaseNodeVersion
DeleteMarker *DeleteMarkerInfo
Actually, we can simplify this to
bool
field.@ -0,0 +11,4 @@
type NodeVersion struct {
BaseNodeVersion
DeleteMarker *DeleteMarkerInfo
IsUnversioned bool
It seems we can drop this field
@ -468,4 +472,3 @@
a.log.Info("added path /get_by_attribute/{cid}/{attr_key}/{attr_val:*}")
r.GET("/zip/{cid}/{prefix:*}", a.logger(downloadRoutes.DownloadZipped))
a.log.Info("added path /zip/{cid}/{prefix}")
Unnecessary change
@ -0,0 +31,4 @@
ObjID oid.ID
TimeStamp uint64
Size int64
Meta map[string]string
It seems we can keep only
ObjID
andMeta
fields@ -0,0 +121,4 @@
return value, ok
}
func newNodeVersion(filePath string, node NodeResponse) (*api.NodeVersion, error) {
The first arg
filePath string
isn't used`@ -113,1 +112,3 @@
d.byAddress(c, request.headObject)
// HeadByAddressOrBucketName handles head requests using simple cid/oid or bucketname/key format.
func (d *Downloader) HeadByAddressOrBucketName(c *fasthttp.RequestCtx) {
test, _ := c.UserValue("cid").(string)
Actually we should check for
oid
here because bothd.byBucketname
andd.byAddress
can handle anycid
just fine43e2b44045
toc23c1ea730
c23c1ea730
tof196297d51
f196297d51
to5d991ce249
@ -0,0 +10,4 @@
DeleteMarker bool
}
func (v NodeVersion) IsDeleteMarker() bool {
Can be dropped
@ -127,2 +134,4 @@
}
}
This will be fixed by
go fmt
, so let's drop this change@ -0,0 +129,4 @@
},
}
if isDeleteMarker {
We can just write:
@ -0,0 +57,4 @@
// keys for delete marker nodes.
isDeleteMarkerKV = "IsDeleteMarker"
ownerKV = "Owner"
createdKV = "Created"
ownerKV
andcreatedKV
can be dropped now@ -0,0 +136,4 @@
}
func (c *Tree) GetLatestVersion(ctx context.Context, cnrID *cid.ID, objectName string) (*api.NodeVersion, error) {
meta := []string{oidKV, isUnversionedKV, isDeleteMarkerKV}
Actually, we don't use
isUnversionedKV
anymore@ -0,0 +1,520 @@
//*
We shouldn't commit these files. Let's add directory
internal/frostfs/services/tree
to.gitignore
@ -293,0 +329,4 @@
foundOid, err := d.tree.GetLatestVersion(d.appCtx, cnrID, key)
if err != nil || foundOid.IsDeleteMarker() {
log.Error("object wasn't found", zap.Error(err))
I wouldn't log
nil
error (error isnil
in case of foundID is delete marker). But I don't insist@ -293,0 +322,4 @@
response.Error(c, "could not fetch and store bearer token: "+err.Error(), fasthttp.StatusBadRequest)
return
}
d.appCtx = ctx
Why we do this? Let's just pass
ctx
futher5d991ce249
to6c4fc75bbc
@ -293,0 +333,4 @@
return
}
if foundOid.DeleteMarker {
response.Error(c, "object deleted", fasthttp.StatusNotFound)
Why don't we log such situation?
@ -0,0 +75,4 @@
GetMeta() []Meta
GetNodeID() uint64
GetParentID() uint64
GetTimestamp() uint64
Actually, we use only
GetMeta
from this interface. So other methods can be dropped@ -293,0 +324,4 @@
}
var addr oid.Address
addr.SetContainer(*cnrID)
Is there any reason to define
addr
variable here and not after successful getting last version (right before6c4fc75bbc/downloader/download.go (L340)
)6c4fc75bbc
to2e31a0794d
@ -40,2 +39,4 @@
@mkdir -p $@
# Synchronize tree service
sync-tree:
Let's also clean this newly created directory on
make clean
. See s3b366e75366/Makefile (L137)
@ -478,2 +482,2 @@
r.GET("/get/{cid}/{oid}", a.logger(downloadRoutes.DownloadByAddress))
r.HEAD("/get/{cid}/{oid}", a.logger(downloadRoutes.HeadByAddress))
r.GET("/get/{cid}/{oid}", a.logger(downloadRoutes.DownloadByAddressOrBucketName))
r.HEAD("/get/{cid}/{oid}", a.logger(downloadRoutes.HeadByAddressOrBucketName))
Actually, we should uses the following path (for
GET
andHEAD
):@ -293,0 +337,4 @@
var addr oid.Address
addr.SetContainer(*cnrID)
addr.SetObject(foundOid.OID)
It's a matter of taste but can we write:
instead of:
2e31a0794d
to30653bdca5
30653bdca5
to8c3c3782f5