TrueCloudLab
/
frostfs-node
19
1
Fork 21
Code Issues 88 Pull Requests 8 Actions Releases Activity

adm: Add commands to invoke methods of policy contract #868

Merged
acid-ant merged 2 commits from acid-ant/frostfs-node:bugfix/cmd-invoke-policy into master 2023-12-21 07:29:16 +00:00
Conversation 0 Commits 2 Files Changed 8
acid-ant commented 2023-12-13 11:14:34 +00:00
Collaborator

Close #834

Added new three commands:

  • frostfs-adm morph ape add-rule-chain
$ frostfs-adm morph ape add-rule-chain -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --chain-id PutObjPolicy11 --target-name "qwe3" --rule-json policy-new.json 
Waiting for transaction to persist...
Rule chain added successfully
$
$ frostfs-adm morph ape add-rule-chain ... --rule "deny:QuotaLimitReached Object.Get Object.Resource:Department=HR Object.Request:Actor!=ownerA *"
Waiting for transaction to persist...
Rule chain added successfully
$
  • frostfs-adm morph ape list-rule-chains
$ frostfs-adm morph ape list-rule-chains -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --target-name "qwe5"
ChainID: PutObjPolicy11
Rules:
	Status: Quota limit reached
	Any: false
	Conditions:
		Resource Department StringEquals HR
		Request Actor StringNotEquals ownerA
	Actions:	Inverted:false
		GetObject
	Resources:	Inverted:false
		native:object//*
$
$ frostfs-adm morph ape list-rule-chains ... --json
[
 {
  "ID": "PutObjPolicy11",
  "Rules": [
   {
    "Status": 3,
    "Actions": {
     "Inverted": false,
...
  • frostfs-adm morph ape rm-rule-chain
$ frostfs-adm morph ape rm-rule-chain -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --chain-id PutObjPolicy11 --target-name "qwe3"
Waiting for transactions to persist...
Rule chain removed successfully
$

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Close #834 Added new three commands: - `frostfs-adm morph ape add-rule-chain` ``` $ frostfs-adm morph ape add-rule-chain -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --chain-id PutObjPolicy11 --target-name "qwe3" --rule-json policy-new.json Waiting for transaction to persist... Rule chain added successfully $ $ frostfs-adm morph ape add-rule-chain ... --rule "deny:QuotaLimitReached Object.Get Object.Resource:Department=HR Object.Request:Actor!=ownerA *" Waiting for transaction to persist... Rule chain added successfully $ ``` - `frostfs-adm morph ape list-rule-chains` ``` $ frostfs-adm morph ape list-rule-chains -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --target-name "qwe5" ChainID: PutObjPolicy11 Rules: Status: Quota limit reached Any: false Conditions: Resource Department StringEquals HR Request Actor StringNotEquals ownerA Actions: Inverted:false GetObject Resources: Inverted:false native:object//* $ $ frostfs-adm morph ape list-rule-chains ... --json [ { "ID": "PutObjPolicy11", "Rules": [ { "Status": 3, "Actions": { "Inverted": false, ... ``` - `frostfs-adm morph ape rm-rule-chain` ``` $ frostfs-adm morph ape rm-rule-chain -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --target-type container --chain-id PutObjPolicy11 --target-name "qwe3" Waiting for transactions to persist... Rule chain removed successfully $ ``` Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant requested review from storage-core-committers 2023-12-13 11:14:54 +00:00
acid-ant requested review from storage-core-developers 2023-12-13 11:15:09 +00:00
dstepanov-yadro reviewed 2023-12-13 12:18:38 +00:00
cmd/frostfs-adm/internal/modules/morph/root.go Outdated
@ -39,2 +41,4 @@
protoConfigPath = "protocol"
walletAddressFlag = "wallet-address"
jsonFlag = "json"
jsonFlagDesc = "Output rule chains in JSON format"
dstepanov-yadro commented 2023-12-13 12:16:35 +00:00
Collaborator

Is naming agreed?

What about put-policy, delete-policy, policy-id, policy, policy-json and so on? It sounds clearer this way I think.

Is naming agreed? What about `put-policy`, `delete-policy`, `policy-id`, `policy`, `policy-json` and so on? It sounds clearer this way I think.
acid-ant commented 2023-12-13 13:29:47 +00:00
Poster
Collaborator

No, names for commands can be discussed. @fyrchik, @aarifullin your thoughts?

No, names for commands can be discussed. @fyrchik, @aarifullin your thoughts?
aarifullin commented 2023-12-15 07:58:37 +00:00
Collaborator

@dstepanov-yadro , if you look at frostfs-adm modules, then you will see already used policy module and a user can misinterpret flags with policy prefix

@dstepanov-yadro , if you look at `frostfs-adm` modules, then you will see already used `policy` module and a user can misinterpret flags with `policy` prefix
fyrchik commented 2023-12-20 08:12:35 +00:00
Owner

I would add frostfs-adm morph ape command with add-chain etc. subcommands.

I would add `frostfs-adm morph ape` command with `add-chain` etc. subcommands.
acid-ant commented 2023-12-20 08:17:07 +00:00
Poster
Collaborator

Absolutely agree, now morph looks too big.

Absolutely agree, now `morph` looks too big.
aarifullin reviewed 2023-12-15 07:50:49 +00:00
cmd/frostfs-adm/internal/modules/morph/ape.go Outdated
@ -0,0 +84,4 @@
}
err := chain.DecodeBytes(rule)
commonCmd.ExitOnErr(cmd, "chain decode error: %w", err)
}
aarifullin commented 2023-12-15 07:50:49 +00:00
Collaborator

I suppose we need

else {
  commonCmd.ExinOnErr(cmd, "parse chain error: %w", fmt.Errorf("rule is not passed"))
}

meaning a rule has been given neither by json nor by statement

I suppose we need ```go else { commonCmd.ExinOnErr(cmd, "parse chain error: %w", fmt.Errorf("rule is not passed")) } ``` meaning a rule has been given neither by json nor by statement
acid-ant commented 2023-12-15 12:13:44 +00:00
Poster
Collaborator

Agree, fixed.

Agree, fixed.
aarifullin marked this conversation as resolved
aarifullin reviewed 2023-12-15 08:40:23 +00:00
cmd/frostfs-adm/internal/modules/morph/ape.go Outdated
@ -0,0 +94,4 @@
chain := parseChain(cmd)
target := parseTarget(cmd)
pci := newPolicyContractInterface(cmd)
_, _, err := pci.AddMorphRuleChain(apechain.Ingress, target, chain)
aarifullin commented 2023-12-15 08:39:18 +00:00
Collaborator

After MorphRuleChainStorage's methods have been changed, since AddMorphRuleChain and RemoveMorphRuleChain return txHash util.Uint256, vub uint32 .

@fyrchik, could you tell, please. Do we need to awaitTx here?
The current way to await a transaction is not applicable for the contract interface because it is a part of initializeContext where contracts are invoked in another manner

After `MorphRuleChainStorage`'s methods have been [changed](https://git.frostfs.info/TrueCloudLab/policy-engine/pulls/24/files#diff-73bcddb63d5304b12f2db25f6b19d47e9a7252aa), since `AddMorphRuleChain` and `RemoveMorphRuleChain` return `txHash util.Uint256, vub uint32` . @fyrchik, could you tell, please. Do we need to `awaitTx` here? The current [way](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/b892feeaf65b89af3b5aa712dca5aefd11d5d660/cmd/frostfs-adm/internal/modules/morph/initialize.go#L377) to await a transaction is not applicable for the contract interface because it is a part of `initializeContext` where contracts are invoked in another manner
fyrchik commented 2023-12-19 16:40:01 +00:00
Owner

Yes, we receive hash and VUB, we need to poll until VUB block and then check that tx with hash exists.

Yes, we receive hash and VUB, we need to poll until VUB block and then check that tx with `hash` exists.
👍 1
acid-ant commented 2023-12-20 07:29:23 +00:00
Poster
Collaborator

Added wait for result.

Added wait for result.
dstepanov-yadro approved these changes 2023-12-15 11:45:40 +00:00
acid-ant force-pushed bugfix/cmd-invoke-policy from 4d8ae74d9d to e8a362ab33 2023-12-15 12:13:37 +00:00 Compare
acid-ant force-pushed bugfix/cmd-invoke-policy from e8a362ab33 to efb3f55c9c 2023-12-18 11:22:17 +00:00 Compare
acid-ant force-pushed bugfix/cmd-invoke-policy from efb3f55c9c to 79fe04650c 2023-12-20 05:49:57 +00:00 Compare
acid-ant force-pushed bugfix/cmd-invoke-policy from 79fe04650c to d5771fc967 2023-12-20 07:28:45 +00:00 Compare
fyrchik approved these changes 2023-12-20 08:11:44 +00:00
cmd/frostfs-adm/internal/modules/morph/root.go Outdated
@ -239,2 +253,4 @@
Run: listNetmapCandidatesNodes,
}
addRuleChainCmd = &cobra.Command{
fyrchik commented 2023-12-20 08:11:16 +00:00
Owner

Can we make it a separate subcommand to group this?

Can we make it a separate subcommand to group this?
acid-ant commented 2023-12-20 12:48:06 +00:00
Poster
Collaborator

Moved all new commands under frostfs-adm morph ape.

Moved all new commands under `frostfs-adm morph ape`.
acid-ant force-pushed bugfix/cmd-invoke-policy from d5771fc967 to 32f4e72e6a 2023-12-20 12:44:57 +00:00 Compare
elebedeva approved these changes 2023-12-20 13:13:26 +00:00
aarifullin approved these changes 2023-12-21 07:23:42 +00:00
acid-ant merged commit 32f4e72e6a into master 2023-12-21 07:29:16 +00:00
acid-ant deleted branch bugfix/cmd-invoke-policy 2023-12-21 07:29:17 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
dstepanov-yadro
fyrchik
elebedeva
aarifullin
Labels
Clear labels   
P0

Highest   
P1

High   
P2

Medium   
P3

Low   
badger

Badger DB related issues   
frostfs-adm
   
frostfs-cli
   
frostfs-ir
   
frostfs-lens
   
frostfs-node
   
good first issue

Good for newcomers   
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff   
blocked

The issue or PR is blocked by something   
bug

Something is not working   
config

Configuration format update or breaking change   
discussion

Talking about something in order to reach a decision or to exchange ideas   
documentation

Documentation related   
duplicate

This issue or pull request already exists   
enhancement

New feature   
go

Language features adoption   
help wanted

Extra attention is needed   
internal
   
invalid

Something is wrong   
kludge

This is a kludge and we know it   
observability

Related to metrics, tracing and logs   
perfomance

Perfomance related   
question

More information is needed   
refactoring

Refactoring   
wontfix

This won't be fixed
No Label
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#868
There is no content yet.
Delete Branch "acid-ant/frostfs-node:bugfix/cmd-invoke-policy"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?