[#17] iam: Add converter to native policy #17

Merged
fyrchik merged 1 commit from dkirillov/policy-engine:feature/support-iam_to_native_converter into master 2023-11-21 09:05:27 +00:00
Member

Signed-off-by: Denis Kirillov d.kirillov@yadro.com

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
dkirillov self-assigned this 2023-11-10 14:57:29 +00:00
dkirillov added 1 commit 2023-11-10 14:57:31 +00:00
[#XX] iam: Add converter to native policy
All checks were successful
DCO action / DCO (pull_request) Successful in 1m0s
Tests and linters / Tests (1.21) (pull_request) Successful in 1m11s
Tests and linters / Tests (1.20) (pull_request) Successful in 1m35s
Tests and linters / Tests with -race (pull_request) Successful in 1m29s
Tests and linters / Staticcheck (pull_request) Successful in 1m33s
Tests and linters / Lint (pull_request) Successful in 2m16s
0566a2b058
Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
dkirillov changed title from WIP: [#XX] iam: Add converter to native policy to WIP: [#17] iam: Add converter to native policy 2023-11-10 15:03:29 +00:00
dkirillov added 1 commit 2023-11-13 13:55:57 +00:00
[#XX] iam: Support native converter
Some checks failed
DCO action / DCO (pull_request) Successful in 1m57s
Tests and linters / Tests (1.20) (pull_request) Successful in 2m41s
Tests and linters / Tests (1.21) (pull_request) Successful in 2m37s
Tests and linters / Lint (pull_request) Failing after 2m51s
Tests and linters / Staticcheck (pull_request) Successful in 2m31s
Tests and linters / Tests with -race (pull_request) Successful in 2m41s
dfba3eb75b
Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

Is this PR duplicate of TrueCloudLab/frostfs-node#800 ?

Is this PR duplicate of https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/800 ?
Author
Member

Is this PR duplicate of TrueCloudLab/frostfs-node#800 ?

Not really, here we want to have converter from IAM (not EACL) policy to native policy

> Is this PR duplicate of https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/800 ? Not really, here we want to have converter from IAM (not EACL) policy to native policy

Is this PR duplicate of TrueCloudLab/frostfs-node#800 ?

Not really, here we want to have converter from IAM (not EACL) policy to native policy

Hm, but looks like this will be frostfs-storage - specific, so i think it is better to do in frostfs-node repository.

> > Is this PR duplicate of https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/800 ? > > Not really, here we want to have converter from IAM (not EACL) policy to native policy Hm, but looks like this will be `frostfs-storage` - specific, so i think it is better to do in `frostfs-node` repository.
dkirillov force-pushed feature/support-iam_to_native_converter from dfba3eb75b to 3191bab0fc 2023-11-14 12:10:54 +00:00 Compare
dkirillov force-pushed feature/support-iam_to_native_converter from 3191bab0fc to cec3744f9f 2023-11-14 12:20:51 +00:00 Compare
dkirillov force-pushed feature/support-iam_to_native_converter from cec3744f9f to e2e53d8f1e 2023-11-14 12:27:22 +00:00 Compare
dkirillov force-pushed feature/support-iam_to_native_converter from e2e53d8f1e to 6f140f0df8 2023-11-15 12:07:16 +00:00 Compare
dkirillov force-pushed feature/support-iam_to_native_converter from 6f140f0df8 to 9226dc32b9 2023-11-15 12:35:25 +00:00 Compare
dkirillov reviewed 2023-11-15 12:44:19 +00:00
@ -0,0 +78,4 @@
},
Condition: append(ruleConditions, conditions...),
}
engineChain.Rules = append(engineChain.Rules, r)
Author
Member

Note: we have to form several rules because of currently conditions can be combined only in one level OR/AND but in iam we have more complex scheme.
Also we need separate conditions for user in different rules to achieve OR behavior. The same for resources.

Explanation:
Consider applying initial policy for resources:

  • bucket1/object1
  • bucket2/*

In case of buket1/object1 resource we form native resource native:object//<bucket1-cid>/* and additional condition: FilePath must equal object1. So if we place both native resources (native:object//<bucket1-cid>/* and native:object//<bucket2-cid>/*) into one rule we will match only requests that operates on object with FilePath: object1 even for bucket2 but we want to apply rule to any object in bucket2

cc @fyrchik

Note: we have to form several rules because of currently conditions can be combined only in one level OR/AND but in iam we have more [complex scheme](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-logic-multiple-context-keys-or-values.html). Also we need separate conditions for user in different rules to achieve OR behavior. The same for resources. Explanation: Consider applying initial policy for resources: * `bucket1/object1` * `bucket2/*` In case of `buket1/object1` resource we form native resource `native:object//<bucket1-cid>/*` and additional condition: `FilePath` must equal `object1`. So if we place both native resources (`native:object//<bucket1-cid>/*` and `native:object//<bucket2-cid>/*`) into one rule we will match only requests that operates on object with `FilePath: object1` even for `bucket2` but we want to apply rule to any object in `bucket2` cc @fyrchik
dkirillov changed title from WIP: [#17] iam: Add converter to native policy to [#17] iam: Add converter to native policy 2023-11-15 12:44:31 +00:00
dkirillov requested review from storage-core-committers 2023-11-15 12:44:40 +00:00
dkirillov requested review from storage-core-developers 2023-11-15 12:44:41 +00:00
dkirillov requested review from storage-services-committers 2023-11-15 12:44:41 +00:00
dkirillov requested review from storage-services-developers 2023-11-15 12:44:41 +00:00
alexvanin approved these changes 2023-11-15 13:43:20 +00:00
iam/policy.go Outdated
@ -223,2 +223,4 @@
func (p Policy) validate() error {
if len(p.Statement) == 0 {
return errors.New("'Statement' missing")
Owner

typo: 'Statement' is missing

typo: 'Statement' *is* missing
dkirillov force-pushed feature/support-iam_to_native_converter from 9226dc32b9 to 3d8b63ac2a 2023-11-15 13:51:14 +00:00 Compare
dstepanov-yadro reviewed 2023-11-15 15:48:56 +00:00
@ -0,0 +30,4 @@
supportedS3ActionListBucket = "ListBucket"
)
//nolint:funlen

:-(

:-(
Author
Member

Whoops. It's not necessary anymore

Whoops. It's not necessary anymore
dstepanov-yadro approved these changes 2023-11-15 15:50:00 +00:00
dkirillov force-pushed feature/support-iam_to_native_converter from 3d8b63ac2a to c9d4d15db6 2023-11-16 06:36:53 +00:00 Compare
fyrchik reviewed 2023-11-21 08:15:28 +00:00
go.mod Outdated
@ -4,2 +4,3 @@
require github.com/stretchr/testify v1.8.1
require (
git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20231114081800-3787477133f3
Owner

sdk-go could depend on this repo, not the other way around.

`sdk-go` could depend on this repo, not the other way around.
Owner

Should we avoid neo-go dependency as well? I think it is possible by making interfaces more abstract and return strings, instead of keys and container ids.

Should we avoid neo-go dependency as well? I think it is possible by making interfaces more abstract and return strings, instead of keys and container ids.
Owner

I would avoid if possible, though neo-go couldn't introduce circles, SDK easily can.

I would avoid if possible, though neo-go couldn't introduce circles, SDK easily can.
Author
Member

Ok, I'll fix this

Ok, I'll fix this
fyrchik approved these changes 2023-11-21 08:16:28 +00:00
dkirillov force-pushed feature/support-iam_to_native_converter from c9d4d15db6 to 5fa9d91903 2023-11-21 08:45:58 +00:00 Compare
fyrchik merged commit 5fa9d91903 into master 2023-11-21 09:05:27 +00:00
dkirillov deleted branch feature/support-iam_to_native_converter 2023-11-21 09:06:58 +00:00
Sign in to join this conversation.
No description provided.