TrueCloudLab/frostfs-node
21
1
Fork 27
Code Issues 108 Pull requests 3 Projects Releases 2 Activity Actions

object: Skip APE check for certain request roles #1039

Merged
fyrchik merged 1 commit from aarifullin/frostfs-node:fix/check_ape_role into master 2024-03-12 18:05:48 +00:00
Conversation 2 Commits 1 Files changed 1 +16
1 changed files with 16 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
pkg/services/object/ape/checker.go
View file

@ -10,6 +10,7 @@ import (
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
)
type checkerImpl struct {
@ -56,6 +57,21 @@ var errMissingOID = errors.New("object ID is not set")
// CheckAPE checks if a request or a response is permitted creating an ape request and passing
// it to chain router.
func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error {
// APE check is ignored for some inter-node requests.
if prm.Role == nativeschema.PropertyValueContainerRoleContainer {
return nil
} else if prm.Role == nativeschema.PropertyValueContainerRoleIR {
switch prm.Method {
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-03-12 12:17:41 +00:00
Outdated
Review
Copy link

Can we do this check before newAPERequest call? It seems we do not use the result.

Can we do this check before `newAPERequest` call? It seems we do not use the result.
aarifullin commented 2024-03-12 12:59:15 +00:00
Outdated
Review
Copy link

Moved this checks at the top. Also, I replaced checking prm.Role by if-else statements instead switch

Moved this checks at the top. Also, I replaced checking `prm.Role` by if-else statements instead `switch`
case nativeschema.MethodGetObject,
nativeschema.MethodHeadObject,
nativeschema.MethodSearchObject,
nativeschema.MethodRangeObject,
nativeschema.MethodHashObject:
return nil
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-03-12 12:17:40 +00:00
Outdated
Review
Copy link

I would rather make return nil on explicit enumeration of read methods: this way we won't accidentally allow writing something when new method is added.

I would rather make `return nil` on explicit enumeration of read methods: this way we won't accidentally allow writing something when new method is added.
aarifullin commented 2024-03-12 12:57:52 +00:00
Outdated
Review
Copy link

Sure. Fixed

Sure. Fixed
default:
}
}
r, err := c.newAPERequest(ctx, prm)
if err != nil {
return fmt.Errorf("failed to create ape request: %w", err)