object: Skip APE check for certain request roles #1039

aarifullin · 2024-03-12T12:12:28Z

aarifullin commented

2024-03-12 12:12:28 +00:00

Skip APE check if a role is Container.
Skip APE check if a role is IR and methods are get-like.

* Skip APE check if a role is Container. * Skip APE check if a role is IR and methods are get-like.

aarifullin added the

bug

label 2024-03-12 12:12:28 +00:00

aarifullin requested review from storage-core-committers 2024-03-12 12:12:37 +00:00

aarifullin requested review from storage-core-developers 2024-03-12 12:12:45 +00:00

fyrchik reviewed 2024-03-12 12:17:40 +00:00

pkg/services/object/ape/checker.go Outdated

					
				@ -63,0 +67,4 @@

						return nil

					case nativeschema.PropertyValueContainerRoleIR:

						switch prm.Method {

						case nativeschema.MethodPutObject, nativeschema.MethodDeleteObject:

fyrchik commented

2024-03-12 12:17:40 +00:00

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

Sure. Fixed

fyrchik marked this conversation as resolved

fyrchik reviewed 2024-03-12 12:17:41 +00:00

pkg/services/object/ape/checker.go Outdated

					
				@ -60,6 +61,20 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error {

						return fmt.Errorf("failed to create ape request: %w", err)

					}

					// APE check is ignored for some inter-node requests.

fyrchik commented

2024-03-12 12:17:41 +00:00

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

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`

fyrchik marked this conversation as resolved

aarifullin force-pushed fix/check_ape_role from 9803717515 to e593b7ecaa

2024-03-12 12:54:36 +00:00

Compare

aarifullin force-pushed fix/check_ape_role from e593b7ecaa to 6e3b36be38

2024-03-12 12:57:48 +00:00

Compare

aarifullin force-pushed fix/check_ape_role from 6e3b36be38 to 5c252c9193

2024-03-12 13:15:34 +00:00

Compare

fyrchik approved these changes 2024-03-12 13:39:45 +00:00

fyrchik commented

2024-03-12 13:40:06 +00:00

Please, check it on dev-env, should be ok to PUT some objects and see no errors in logs.

aarifullin commented

2024-03-12 13:59:56 +00:00

Please, check it on dev-env, should be ok to PUT some objects and see no errors in logs.

I checked. Also, I created a container with 4 nodes and was checking how is object policer dealing the replication - it's OK, no errors

> Please, check it on dev-env, should be ok to PUT some objects and see no errors in logs. I checked. Also, I created a container with 4 nodes and was checking how is object policer dealing the replication - it's OK, no errors

aarifullin requested review from fyrchik 2024-03-12 14:36:56 +00:00

fyrchik merged commit 5c252c9193 into master

2024-03-12 18:05:48 +00:00

fyrchik referenced this pull request from a commit

2024-03-12 18:05:51 +00:00

[#1039] object: Skip APE check for certain request roles

Sign in to join this conversation.

No reviewers

TrueCloudLab/storage-core-developers