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

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 2024-09-04 19:51:05 +00:00
Conversation 0 Commits 2 Files changed 8 +422 -91
acid-ant commented 2023-12-13 11:14:34 +00:00
Member
Copy link

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
Member
Copy link

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
Author
Member
Copy link

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
Member
Copy link

@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
Copy link

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
Author
Member
Copy link

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
Member
Copy link

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
Author
Member
Copy link

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
Member
Copy link

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
Copy link

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
Author
Member
Copy link

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
Copy link

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
Author
Member
Copy link

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 labels
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
Projects
Clear projects
No items
No project
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
No description provided.
Delete branch "acid-ant/frostfs-node:bugfix/cmd-invoke-policy"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?