Drop/resolve TODO's #847

Closed
dstepanov-yadro wants to merge 1 commit from dstepanov-yadro/frostfs-node:fix/todo into master
30 changed files with 38 additions and 65 deletions

View file

@ -25,7 +25,7 @@ linters-settings:
# report about shadowed variables
check-shadowing: false
staticcheck:
checks: ["all", "-SA1019"] # TODO Enable SA1019 after deprecated warning are fixed.
checks: ["all"]
funlen:
lines: 80 # default 60
statements: 60 # default 40

View file

@ -51,7 +51,6 @@ func verifyResponse(cmd *cobra.Command,
commonCmd.ExitOnErr(cmd, "", errors.New("missing response signature"))
}
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sigV2.SetScheme(refs.ECDSA_SHA512)
sigV2.SetKey(sigControl.GetKey())

View file

@ -108,7 +108,6 @@ func getObject(cmd *cobra.Command, _ []string) {
func processResult(cmd *cobra.Command, res *internalclient.GetObjectRes, binary bool, payloadBuffer *bytes.Buffer, out io.Writer, filename string) {
if binary {
objToStore := res.Header()
// TODO(@acid-ant): #1932 Use streams to marshal/unmarshal payload
Review

Why do I need a link to the task in the code? I think it is possible to delete this TODO, especially if there is a task.

Why do I need a link to the task in the code? I think it is possible to delete this TODO, especially if there is a task.
Review

Yeah, agree.

Yeah, agree.
objToStore.SetPayload(payloadBuffer.Bytes())
objBytes, err := objToStore.Marshal()
commonCmd.ExitOnErr(cmd, "", err)

View file

@ -162,8 +162,6 @@ func printHeader(cmd *cobra.Command, obj *objectSDK.Object) error {
if signature := obj.Signature(); signature != nil {
cmd.Print("ID signature:\n")
// TODO(@carpawell): #468 implement and use another approach to avoid conversion
var sigV2 refs.Signature
signature.WriteToV2(&sigV2)

View file

@ -156,7 +156,6 @@ func readFilePayload(filename string, cmd *cobra.Command) (io.Reader, cid.ID, us
buf, err := os.ReadFile(filename)
commonCmd.ExitOnErr(cmd, "unable to read given file: %w", err)
objTemp := objectSDK.New()
// TODO(@acid-ant): #1932 Use streams to marshal/unmarshal payload
commonCmd.ExitOnErr(cmd, "can't unmarshal object from given file: %w", objTemp.Unmarshal(buf))
payloadReader := bytes.NewReader(objTemp.Payload())
cnr, _ := objTemp.ContainerID()

View file

@ -67,10 +67,7 @@ func configureEACLAndContainerSources(c *cfg, client *cntClient.Client, cnrSrc c
subscribeToContainerCreation(c, func(e event.Event) {
ev := e.(containerEvent.PutSuccess)
// read owner of the created container in order to update the reading cache.
// TODO: use owner directly from the event after neofs-contract#256 will become resolved
// but don't forget about the profit of reading the new container and caching it:
// creation success are most commonly tracked by polling GET op.
// read owner of the created container in order to update the reading cache
cnr, err := cnrSrc.Get(ev.ID)
if err == nil {
cachedContainerStorage.containerCache.set(ev.ID, cnr, nil)

View file

@ -45,6 +45,8 @@ import (
"go.uber.org/zap"
)
var errInvalidSignature = errors.New("invalid signature of the eACL table")
type objectSvc struct {
put *putsvcV2.Service
@ -448,8 +450,7 @@ func (s *morphEACLFetcher) GetEACL(cnr cid.ID) (*containercore.EACL, error) {
}
if !eaclInfo.Signature.Verify(binTable) {
// TODO(@cthulhu-rider): #468 use "const" error
return nil, errors.New("invalid signature of the eACL table")
return nil, errInvalidSignature
}
return eaclInfo, nil

View file

@ -47,8 +47,6 @@ func (c SenderClassifier) Classify(
) (res *ClassifyResult, err error) {
ownerKeyInBytes := ownerKey.Bytes()
// TODO: #767 get owner from frostfs.id if present
// if request owner is the same as container owner, return RoleUser
if ownerID.Equals(cnr.Owner()) {
return &ClassifyResult{

View file

@ -275,9 +275,7 @@ func (e *StorageEngine) evacuateShard(ctx context.Context, shardID string, prm E
var c *meta.Cursor
for {
listPrm.WithCursor(c)
// TODO (@fyrchik): #1731 this approach doesn't work in degraded modes
Review

Ok, reverted comment.

Ok, reverted comment.
Review

It is different now and misses the link to the task.

It is different now and misses the link to the task.
Review

The meaning of the comment is the same. Why store references to tasks in the code?

The meaning of the comment is the same. Why store references to tasks in the code?
Review

To link to the task and discussion (and not to create it again).

To link to the task and discussion (and not to create it again).
Review

Source code is not the best place for discussions:)

Then we need to add links to all tasks from gitea to the source code.

Source code is not the best place for discussions:) Then we need to add links to all tasks from gitea to the source code.
Review

Source code is not the best place for discussions:)

Exactly, and link links to a proper place for discussion.

Then we need to add links to all tasks from gitea to the source code.

No, we don't. In some cases we feel the need to leave a comment, nothing wrong with it.

>Source code is not the best place for discussions:) Exactly, and link links to a proper place for discussion. >Then we need to add links to all tasks from gitea to the source code. No, we don't. In _some_ cases we feel the need to leave a comment, nothing wrong with it.
// because ListWithCursor works only with the metabase.
// this approach doesn't work in degraded modes because ListWithCursor works only with the metabase.
listRes, err := sh.ListWithCursor(ctx, listPrm)
if err != nil {
if errors.Is(err, meta.ErrEndOfListing) || errors.Is(err, shard.ErrDegradedMode) {

View file

@ -239,7 +239,7 @@ func (db *DB) updateShardObjectCounter(tx *bbolt.Tx, typ objectType, delta uint6
return b.Put(counterKey, newCounter)
}
func (db *DB) updateContainerCounter(tx *bbolt.Tx, delta map[cid.ID]ObjectCounters, inc bool) error { // TODO #838
func (db *DB) updateContainerCounter(tx *bbolt.Tx, delta map[cid.ID]ObjectCounters, inc bool) error {
b := tx.Bucket(containerCounterBucketName)
if b == nil {
return nil

View file

@ -18,7 +18,6 @@ type memoryForest struct {
var _ Forest = (*memoryForest)(nil)
// NewMemoryForest creates new empty forest.
// TODO: this function will eventually be removed and is here for debugging.
func NewMemoryForest() ForestStorage {
return &memoryForest{
treeMap: make(map[string]*memoryTree),

View file

@ -7,15 +7,12 @@ import (
)
// Timestamp is an alias for integer timestamp type.
// TODO: remove after the debugging.
type Timestamp = uint64
// Node is used to represent nodes.
// TODO: remove after the debugging.
type Node = uint64
// Meta represents arbitrary meta information.
// TODO: remove after the debugging or create a proper interface.
type Meta struct {
Time Timestamp
Items []KeyValue

View file

@ -84,7 +84,6 @@ func (c *Client) GetEACL(cnr cid.ID) (*container.EACL, error) {
}
}
// TODO(@cthulhu-rider): #468 implement and use another approach to avoid conversion
var sigV2 refs.Signature
sigV2.SetKey(pub)
sigV2.SetSign(sig)

View file

@ -31,7 +31,6 @@ func PutEACL(c *Client, eaclInfo containercore.EACL) error {
prm.SetToken(eaclInfo.Session.Marshal())
}
// TODO(@cthulhu-rider): #468 implement and use another approach to avoid conversion
var sigV2 refs.Signature
eaclInfo.Signature.WriteToV2(&sigV2)

View file

@ -103,7 +103,6 @@ func (c *Client) Get(cid []byte) (*containercore.Container, error) {
}
}
// TODO(@cthulhu-rider): #468 implement and use another approach to avoid conversion
var sigV2 refs.Signature
sigV2.SetKey(pub)
sigV2.SetSign(sigBytes)

View file

@ -28,7 +28,6 @@ func Put(c *Client, cnr containercore.Container) (*cid.ID, error) {
prm.SetToken(cnr.Session.Marshal())
}
// TODO(@cthulhu-rider): #468 implement and use another approach to avoid conversion
var sigV2 refs.Signature
cnr.Signature.WriteToV2(&sigV2)

View file

@ -42,6 +42,13 @@ type Writer interface {
PutEACL(containercore.EACL) error
}
var (
errMissingSignature = errors.New("missing signature")
errMissingContainerField = errors.New("missing container field")
errMissingContainerID = errors.New("missing container ID")
errMissingUserID = errors.New("missing user ID")
)
func NewExecutor(rdr Reader, wrt Writer) containerSvc.ServiceExecutor {
return &morphExecutor{
rdr: rdr,
@ -52,13 +59,12 @@ func NewExecutor(rdr Reader, wrt Writer) containerSvc.ServiceExecutor {
func (s *morphExecutor) Put(_ context.Context, tokV2 *sessionV2.Token, body *container.PutRequestBody) (*container.PutResponseBody, error) {
sigV2 := body.GetSignature()
if sigV2 == nil {
// TODO(@cthulhu-rider): #468 use "const" error
return nil, errors.New("missing signature")
return nil, errMissingSignature
}
cnrV2 := body.GetContainer()
if cnrV2 == nil {
return nil, errors.New("missing container field")
return nil, errMissingContainerField
}
var cnr containercore.Container
@ -99,7 +105,7 @@ func (s *morphExecutor) Put(_ context.Context, tokV2 *sessionV2.Token, body *con
func (s *morphExecutor) Delete(_ context.Context, tokV2 *sessionV2.Token, body *container.DeleteRequestBody) (*container.DeleteResponseBody, error) {
idV2 := body.GetContainerID()
if idV2 == nil {
return nil, errors.New("missing container ID")
return nil, errMissingContainerID
}
var id cid.ID
@ -137,7 +143,7 @@ func (s *morphExecutor) Delete(_ context.Context, tokV2 *sessionV2.Token, body *
func (s *morphExecutor) Get(_ context.Context, body *container.GetRequestBody) (*container.GetResponseBody, error) {
idV2 := body.GetContainerID()
if idV2 == nil {
return nil, errors.New("missing container ID")
return nil, errMissingContainerID
}
var id cid.ID
@ -177,7 +183,7 @@ func (s *morphExecutor) Get(_ context.Context, body *container.GetRequestBody) (
func (s *morphExecutor) List(_ context.Context, body *container.ListRequestBody) (*container.ListResponseBody, error) {
idV2 := body.GetOwnerID()
if idV2 == nil {
return nil, fmt.Errorf("missing user ID")
return nil, errMissingUserID
}
var id user.ID
@ -206,8 +212,7 @@ func (s *morphExecutor) List(_ context.Context, body *container.ListRequestBody)
func (s *morphExecutor) SetExtendedACL(_ context.Context, tokV2 *sessionV2.Token, body *container.SetExtendedACLRequestBody) (*container.SetExtendedACLResponseBody, error) {
sigV2 := body.GetSignature()
if sigV2 == nil {
// TODO(@cthulhu-rider): #468 use "const" error
return nil, errors.New("missing signature")
return nil, errMissingSignature
}
eaclInfo := containercore.EACL{
@ -239,7 +244,7 @@ func (s *morphExecutor) SetExtendedACL(_ context.Context, tokV2 *sessionV2.Token
func (s *morphExecutor) GetExtendedACL(_ context.Context, body *container.GetExtendedACLRequestBody) (*container.GetExtendedACLResponseBody, error) {
idV2 := body.GetContainerID()
if idV2 == nil {
return nil, errors.New("missing container ID")
return nil, errMissingContainerID
}
var id cid.ID

View file

@ -82,7 +82,7 @@ func TestInvalidToken(t *testing.T) {
reqBody.SetContainer(&cnrV2)
sign(&reqBody)
_, err = e.Put(context.TODO(), tokV2, &reqBody)
_, err = e.Put(context.Background(), tokV2, &reqBody)
return
},
},
@ -92,7 +92,7 @@ func TestInvalidToken(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cnrV2)
_, err = e.Delete(context.TODO(), tokV2, &reqBody)
_, err = e.Delete(context.Background(), tokV2, &reqBody)
return
},
},
@ -103,7 +103,7 @@ func TestInvalidToken(t *testing.T) {
reqBody.SetSignature(new(refs.Signature))
sign(&reqBody)
_, err = e.SetExtendedACL(context.TODO(), tokV2, &reqBody)
_, err = e.SetExtendedACL(context.Background(), tokV2, &reqBody)
return
},
},

View file

@ -19,13 +19,16 @@ type SignedMessage interface {
SetSignature(*control.Signature)
}
var errDisallowedKey = errors.New("key is not in the allowed list")
var (
errDisallowedKey = errors.New("key is not in the allowed list")
errInvalidSignature = errors.New("invalid signature")
errMissingSignature = errors.New("missing signature")
)
func (s *Server) isValidRequest(req SignedMessage) error {
sign := req.GetSignature()
if sign == nil {
// TODO(@cthulhu-rider): #468 use "const" error
return errors.New("missing signature")
return errMissingSignature
}
var (
@ -50,7 +53,6 @@ func (s *Server) isValidRequest(req SignedMessage) error {
return fmt.Errorf("marshal request body: %w", err)
}
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sigV2.SetKey(sign.GetKey())
sigV2.SetSign(sign.GetSign())
@ -62,8 +64,7 @@ func (s *Server) isValidRequest(req SignedMessage) error {
}
if !sig.Verify(binBody) {
// TODO(@cthulhu-rider): #468 use "const" error
return errors.New("invalid signature")
return errInvalidSignature
}
return nil
@ -83,7 +84,6 @@ func SignMessage(key *ecdsa.PrivateKey, msg SignedMessage) error {
return fmt.Errorf("calculate signature: %w", err)
}
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sig.WriteToV2(&sigV2)

View file

@ -19,13 +19,16 @@ type SignedMessage interface {
SetSignature(*control.Signature)
}
var errDisallowedKey = errors.New("key is not in the allowed list")
var (
errDisallowedKey = errors.New("key is not in the allowed list")
errMissingSignature = errors.New("missing signature")
errInvalidSignature = errors.New("invalid signature")
)
func (s *Server) isValidRequest(req SignedMessage) error {
sign := req.GetSignature()
if sign == nil {
// TODO(@cthulhu-rider): #468 use "const" error
return errors.New("missing signature")
return errMissingSignature
}
var (
@ -50,7 +53,6 @@ func (s *Server) isValidRequest(req SignedMessage) error {
return fmt.Errorf("marshal request body: %w", err)
}
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sigV2.SetKey(sign.GetKey())
sigV2.SetSign(sign.GetSign())
@ -62,8 +64,7 @@ func (s *Server) isValidRequest(req SignedMessage) error {
}
if !sig.Verify(binBody) {
// TODO(@cthulhu-rider): #468 use "const" error
return errors.New("invalid signature")
return errInvalidSignature
}
return nil
@ -83,7 +84,6 @@ func SignMessage(key *ecdsa.PrivateKey, msg SignedMessage) error {
return fmt.Errorf("calculate signature: %w", err)
}
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sig.WriteToV2(&sigV2)

View file

@ -219,7 +219,6 @@ func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) error {
// 4. Then check if container owner signed this token.
if !bearerSDK.ResolveIssuer(*token).Equals(ownerCnr) {
// TODO: #767 in this case we can issue all owner keys from frostfs.id and check once again
return errBearerNotSignedByOwner
}
@ -235,7 +234,6 @@ func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) error {
user.IDFromKey(&usrSender, ecdsa.PublicKey(keySender))
if !token.AssertUser(usrSender) {
// TODO: #767 in this case we can issue all owner keys from frostfs.id and check once again
return errBearerInvalidOwner
}

View file

@ -124,7 +124,6 @@ func ownerFromToken(token *sessionSDK.Object) (*user.ID, *keys.PublicKey, error)
}
// 2. Then check if session token owner issued the session token
// TODO(@cthulhu-rider): #468 implement and use another approach to avoid conversion
var tokV2 sessionV2.Token
token.WriteToV2(&tokV2)
@ -136,7 +135,6 @@ func ownerFromToken(token *sessionSDK.Object) (*user.ID, *keys.PublicKey, error)
tokenIssuer := token.Issuer()
if !isOwnerFromKey(tokenIssuer, tokenIssuerKey) {
// TODO: #767 in this case we can issue all owner keys from frostfs.id and check once again
return nil, nil, errInvalidSessionOwner
}

View file

@ -77,9 +77,6 @@ func (r *request) processCurrentEpoch(ctx context.Context) bool {
default:
}
// TODO: #1142 consider parallel execution
// TODO: #1142 consider optimization: if status == SPLIT we can continue until
// we reach the best result - split info with linking object ID.
var info client.NodeInfo
client.NodeInfoFromNetmapElement(&info, addrs[i])

View file

@ -44,7 +44,6 @@ func (f *getRequestForwarder) forwardRequestToNode(ctx context.Context, addr net
// compose meta header of the local server
metaHdr := new(session.RequestMetaHeader)
metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1)
// TODO: #1165 think how to set the other fields
metaHdr.SetOrigin(f.Request.GetMetaHeader())
writeCurrentVersion(metaHdr)
f.Request.SetMetaHeader(metaHdr)

View file

@ -43,7 +43,6 @@ func (f *getRangeRequestForwarder) forwardRequestToNode(ctx context.Context, add
// compose meta header of the local server
metaHdr := new(session.RequestMetaHeader)
metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1)
// TODO: #1165 think how to set the other fields
metaHdr.SetOrigin(f.Request.GetMetaHeader())
writeCurrentVersion(metaHdr)

View file

@ -43,7 +43,6 @@ func (f *headRequestForwarder) forwardRequestToNode(ctx context.Context, addr ne
// compose meta header of the local server
metaHdr := new(session.RequestMetaHeader)
metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1)
// TODO: #1165 think how to set the other fields
metaHdr.SetOrigin(f.Request.GetMetaHeader())
writeCurrentVersion(metaHdr)

View file

@ -33,7 +33,6 @@ func (f *requestForwarder) forwardRequest(ctx context.Context, addr network.Addr
// compose meta header of the local server
metaHdr := new(session.RequestMetaHeader)
metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1)
// TODO: #1165 think how to set the other fields
metaHdr.SetOrigin(f.Request.GetMetaHeader())
f.Request.SetMetaHeader(metaHdr)

Binary file not shown.

View file

@ -154,7 +154,6 @@ message RemoveResponse {
message MoveRequest {
message Body {
// TODO import neo.fs.v2.refs.ContainerID directly.
// Container ID in V2 format.
bytes container_id = 1;
// The name of the tree.

View file

@ -175,7 +175,6 @@ func verifyMessage(m message) error {
sig := m.GetSignature()
// TODO(@cthulhu-rider): #468 use Signature message from FrostFS API to avoid conversion
var sigV2 refs.Signature
sigV2.SetKey(sig.GetKey())
sigV2.SetSign(sig.GetSign())