TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 93 Pull requests 4 Projects Releases 1 Activity Actions

eacl: Fix bug with casting to ObjectAccessDenied error #553

Merged
fyrchik merged 1 commit from aarifullin/frostfs-node:fix/eacl_errors into master 2023-08-02 07:22:49 +00:00
Conversation 0 Commits 1 Files changed 2 +32 -2
2 changed files with 32 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
pkg/services/object/acl/v2/errors.go
View file

@ -25,14 +25,14 @@ const accessDeniedACLReasonFmt = "access to operation %s is denied by basic ACL
const accessDeniedEACLReasonFmt = "access to operation %s is denied by extended ACL check: %v"
func basicACLErr(info RequestInfo) error {
var errAccessDenied apistatus.ObjectAccessDenied
errAccessDenied := &apistatus.ObjectAccessDenied{}
errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedACLReasonFmt, info.operation))
return errAccessDenied
}
func eACLErr(info RequestInfo, err error) error {
var errAccessDenied apistatus.ObjectAccessDenied
errAccessDenied := &apistatus.ObjectAccessDenied{}
errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedEACLReasonFmt, info.operation, err))
return errAccessDenied

30
pkg/services/object/acl/v2/errors_test.go Normal file
View file

@ -0,0 +1,30 @@
package v2
import (
"errors"
"testing"
apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
"github.com/stretchr/testify/require"
)
func TestBasicACLErr(t *testing.T) {
var reqInfo RequestInfo
err := basicACLErr(reqInfo)
var errAccessDenied *apistatus.ObjectAccessDenied
ale64bit commented 2023-08-01 13:29:05 +00:00
Outdated
Review
Copy link

require.True(...).
Same below.

`require.True(...)`. Same below.
fyrchik commented 2023-08-01 13:34:53 +00:00
Outdated
Review
Copy link

require.ErrorsAs is also possible

`require.ErrorsAs` is also possible
aarifullin commented 2023-08-01 13:37:55 +00:00
Outdated
Review
Copy link

Changed to require.ErrorsAs.
To be honest I don't really like message format that require package uses for errors. That's why I initally used t.Fatalf, but I am ok with require.ErrorsAs

Changed to `require.ErrorsAs`. To be honest I don't really like message format that `require` package uses for errors. That's why I initally used `t.Fatalf`, but I am ok with `require.ErrorsAs`
require.ErrorAs(t, err, &errAccessDenied,
"basicACLErr must be able to be casted to apistatus.ObjectAccessDenied")
}
func TestEACLErr(t *testing.T) {
var reqInfo RequestInfo
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2023-08-01 13:40:35 +00:00
Outdated
Review
Copy link

This is not "any error", the most generic would be sth with errors.New I believe

This is not "any error", the most generic would be sth with `errors.New` I believe
aarifullin commented 2023-08-01 13:44:42 +00:00
Outdated
Review
Copy link

Alright. I think you are right. I needed any error to create the instance :)

UPD: fixed to errors.New

Alright. I think you are right. I needed any error to create the instance :) UPD: fixed to `errors.New`
testErr := errors.New("test-eacl")
err := eACLErr(reqInfo, testErr)
var errAccessDenied *apistatus.ObjectAccessDenied
require.ErrorAs(t, err, &errAccessDenied,
"eACLErr must be able to be casted to apistatus.ObjectAccessDenied")
}