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.

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

Upgrade NeoFS SDK with eacl changes #2618

Closed
wants to merge 2 commits into from
Closed

Conversation

cthulhu-rider
Copy link
Contributor

Behavior change: response headers are no longer processed because eACL
may include rules over request headers only.

Signed-off-by: Leonard Lyubich <[email protected]>
Adopt eACL changes.

Signed-off-by: Leonard Lyubich <[email protected]>
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #2618 (5e52c81) into master (1006609) will decrease coverage by 0.22%.
The diff coverage is 3.08%.

❗ Current head 5e52c81 differs from pull request most recent head fb43a5f. Consider uploading reports for the commit fb43a5f to get more accurate results

@@            Coverage Diff             @@
##           master    #2618      +/-   ##
==========================================
- Coverage   29.74%   29.52%   -0.22%     
==========================================
  Files         408      400       -8     
  Lines       31215    30773     -442     
==========================================
- Hits         9284     9086     -198     
+ Misses      21119    20883     -236     
+ Partials      812      804       -8     
Files Coverage Δ
cmd/neofs-node/container.go 0.00% <0.00%> (ø)
pkg/services/tree/signature.go 67.21% <90.90%> (-0.79%) ⬇️
cmd/neofs-node/object.go 0.00% <0.00%> (ø)
cmd/neofs-cli/modules/acl/extended/create.go 13.20% <0.00%> (ø)
cmd/neofs-cli/modules/acl/extended/print.go 29.41% <0.00%> (-1.84%) ⬇️
pkg/services/container/morph/executor.go 36.80% <41.66%> (-0.24%) ⬇️
pkg/services/object/acl/errors.go 0.00% <0.00%> (ø)
pkg/services/object/acl/headers.go 0.00% <0.00%> (ø)
cmd/neofs-node/node.go 0.00% <0.00%> (ø)
pkg/services/object/acl/acl.go 0.00% <0.00%> (-28.69%) ⬇️

... and 2 files with indirect coverage changes

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

@@ -58,7 +59,7 @@ func boolToString(b bool) string {
}

// PrettyPrintTableEACL print extended ACL in table format.
func PrettyPrintTableEACL(cmd *cobra.Command, table *eacl.Table) {
func PrettyPrintTableEACL(cmd *cobra.Command, table eacl.Table) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

return nil, errors.New("invalid action (expected 'allow' or 'deny')")
case "allow":
action = eacl.ActionAllow
case "deny":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have these strings somewhere?

ops[i] = acl.OpObjectSearch
case "getrange":
ops[i] = acl.OpObjectRange
case "getrangehash":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And these ones also?

"github.com/nspcc-dev/neofs-sdk-go/user"
)

// node is wrapper over cfg providing interface needed by app components.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need another wrapper?

// static

node Node

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why

oh

why

is

it

always

like

this?

@cthulhu-rider
Copy link
Contributor Author

too outdated, wontfix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants