Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

services/object: fallback to GET in GET_RANGE #1884

Merged
merged 1 commit into from
Oct 12, 2022
Merged

services/object: fallback to GET in GET_RANGE #1884

merged 1 commit into from
Oct 12, 2022

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Oct 12, 2022

Current spec allows denying GET_RANGE requests from other storage nodes. However, GET should always be allowed and it is enough to perform GET_RANGE locally

Signed-off-by: Evgenii Stratonikov evgeniy@morphbits.ru

@fyrchik
Copy link
Contributor Author

fyrchik commented Oct 12, 2022

Depends on #1879 .

@codecov
Copy link

codecov bot commented Oct 12, 2022 •
edited
Loading

Codecov Report

Merging #1884 (8df3739) into master (c5fdb7b) will decrease coverage by 0.00%.
The diff coverage is 74.24%.

❗ Current head 8df3739 differs from pull request most recent head ab57fd2. Consider uploading reports for the commit ab57fd2 to get more accurate results

@@            Coverage Diff             @@
##           master    #1884      +/-   ##
==========================================
- Coverage   30.87%   30.86%   -0.01%     
==========================================
  Files         378      378              
  Lines       27029    27059      +30     
==========================================
+ Hits         8345     8353       +8     
- Misses      17992    18013      +21     
- Partials      692      693       +1
Impacted Files Coverage Δ
cmd/neofs-node/netmap.go 0.00% <0.00%> (ø)
pkg/services/control/service.pb.go 7.46% <0.00%> (ø)
pkg/services/object/get/util.go 13.74% <0.00%> (-1.52%) ⬇️
pkg/services/control/service_neofs.pb.go 10.19% <33.33%> (ø)
pkg/local_object_storage/engine/evacuate.go 83.33% <84.74%> (+2.77%) ⬆️
cmd/neofs-adm/internal/modules/config/root.go 100.00% <100.00%> (ø)
cmd/neofs-adm/internal/modules/morph/root.go 50.34% <100.00%> (ø)
cmd/neofs-cli/modules/acl/extended/create.go 52.51% <100.00%> (ø)
pkg/services/control/service.go 20.73% <100.00%> (ø)
pkg/services/object/get/exec.go 72.37% <100.00%> (-2.21%) ⬇️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fyrchik
[nspcc-dev#1884] services/object: Fallback to GET in GET_RANGE
ab57fd2 
Current spec allows denying GET_RANGE requests from other storage nodes.
However, GET should always be allowed and it is enough to perform
GET_RANGE locally

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
@fyrchik fyrchik merged commit 4baf00a into nspcc-dev:master Oct 12, 2022
@fyrchik fyrchik mentioned this pull request Oct 12, 2022
Closed
@alexchetaev alexchetaev added 2022Q3 and removed 2022Q4 labels Oct 12, 2022
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
@fyrchik @aprasolova
[nspcc-dev#1884] services/object: Fallback to GET in GET_RANGE
7da9991 
Current spec allows denying GET_RANGE requests from other storage nodes.
However, GET should always be allowed and it is enough to perform
GET_RANGE locally

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 7, 2023
@carpawell
node: Forward GETRANGEHASH requests
384b334 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 7, 2023
@carpawell
node: Forward GETRANGEHASH requests
9b529c3 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell mentioned this pull request Sep 7, 2023
Merged
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 7, 2023
@carpawell
node: Forward GETRANGEHASH requests
a0b8d77 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 7, 2023
@carpawell
node: Forward GETRANGEHASH requests
0b4f854 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 14, 2023
@carpawell
node: Forward GETRANGEHASH requests
94883f0 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 21, 2023
@carpawell
node: Forward GETRANGEHASH requests
0bb103a 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Sep 21, 2023
@carpawell
node: Forward GETRANGEHASH requests
5ab5ccd 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit to carpawell/neofs-node that referenced this pull request Oct 23, 2023
@carpawell
node: Forward GETRANGEHASH requests
2f4977c 
Do it as it is already done for GET, HEAD, GETRANGE. In the case of a container
node that does not have an object locally, the node spawns GETRANGE request in
order to hash it. That is not allowed operation in the NeoFS. Even with nspcc-dev#1884,
GET may fail because the node may not be a container part. Moreover, attached
bearer token is not allowed for the node's key usage so that is another way to
get unexpected results. Forwarding requests is the only sane fix for the nspcc-dev#2541.
The code smells but this is not this commit's responsibility: it is hard to fix
that bug nicely without a get service refactor.
Closes nspcc-dev#2541.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cthulhu-rider cthulhu-rider cthulhu-rider approved these changes

@carpawell carpawell Awaiting requested review from carpawell carpawell is a code owner

@acid-ant acid-ant Awaiting requested review from acid-ant

Assignees
No one assigned
Labels
bug Something isn't working U2 Seriously planned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fyrchik @cthulhu-rider @carpawell @alexchetaev