WIP: Make IsAllow receive client defined overrides #82

aarifullin · 2024-06-28T11:42:31Z

aarifullin commented

2024-06-28 11:42:31 +00:00

Change ChainRouter interface: IsAllow should also receive client defined overrides that are checked after local overrides but before morph chains. Client defined overrides are checked per request.
Fix code that uses ChainRouter.
Fix unit-tests.

* Change `ChainRouter` interface: `IsAllow` should also receive client defined overrides that are checked after local overrides but before morph chains. Client defined overrides are checked per request. * Fix code that uses `ChainRouter`. * Fix unit-tests.

aarifullin added 1 commit 2024-06-28 11:42:45 +00:00

Tests and linters / Tests (1.21) (pull_request) Successful in 1m26s Details

DCO action / DCO (pull_request) Failing after 1m47s Details

Tests and linters / Tests (1.20) (pull_request) Successful in 2m13s Details

Tests and linters / Tests with -race (pull_request) Successful in 2m36s Details

Tests and linters / Lint (pull_request) Failing after 2m53s Details

Tests and linters / Staticcheck (pull_request) Successful in 2m33s Details

cf0e7369b5 [#XX] router: Make IsAllow receive client defined overrides

* Change `ChainRouter` interface: `IsAllow` should also receive
  client defined overrides that are checked after local overrides
  but before morph chains. Client defined overrides are checked for
  per request.
* Fix code that uses `ChainRouter`.
* Fix unit-tests.

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>

aarifullin force-pushed feat/clientdef_overrides from cf0e7369b5 to 0b24fc0f48

2024-06-28 11:43:34 +00:00

Compare

aarifullin requested review from storage-services-committers 2024-06-28 11:43:44 +00:00

aarifullin requested review from storage-services-developers 2024-06-28 11:43:49 +00:00

aarifullin requested review from storage-core-committers 2024-06-28 11:45:10 +00:00

aarifullin requested review from storage-core-developers 2024-06-28 11:45:25 +00:00

acid-ant commented

2024-06-28 11:51:02 +00:00

Please check issue from linter:

pkg/engine/interface.go:15:46  revive  var-declaration: should drop = nil from declaration of var NoClientDefined; it is the zero value
make: *** [Makefile:54: lint] Error 1

Please check issue from `linter`: ``` pkg/engine/interface.go:15:46 revive var-declaration: should drop = nil from declaration of var NoClientDefined; it is the zero value make: *** [Makefile:54: lint] Error 1 ```

👍 1

aarifullin force-pushed feat/clientdef_overrides from 0b24fc0f48 to b91348ed80

2024-06-28 12:06:48 +00:00

Compare

aarifullin force-pushed feat/clientdef_overrides from b91348ed80 to a847f28b01

2024-06-28 12:48:15 +00:00

Compare

dstepanov-yadro approved these changes 2024-06-28 14:47:26 +00:00

acid-ant approved these changes 2024-06-28 18:36:07 +00:00

achuprov approved these changes 2024-07-01 07:22:42 +00:00

fyrchik reviewed 2024-07-01 07:31:09 +00:00

pkg/engine/inmemory/inmemory_test.go

													
				@ -152,3 +204,3 @@

							"Actor":    actor2,

						})

						status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor)

						status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor, engine.NoClientDefined)

fyrchik commented

2024-07-01 07:31:05 +00:00

It seems to me that many times we need to make changes in the policy-engine, we extend some interface.
Here overrides seem to be more related to storage and not to method.
How about having some "combined overrides storage" instead? So you create interface { IsAllowed(...) } with some function taking local override storage + e.g. bearer token and use it.

It seems to me that many times we need to make changes in the policy-engine, we extend some interface. Here overrides seem to be more related to storage and not to method. How about having some "combined overrides storage" instead? So you create `interface { IsAllowed(...) }` with some function taking local override storage + e.g. bearer token and use it.

👍 1