TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 101 Pull requests 5 Projects Releases 1 Activity Actions 8

cli: Improve chain rule managing commands #909

Merged
fyrchik merged 1 commit from aarifullin/frostfs-node:feat/cli-ape into master 2024-02-07 06:54:42 +00:00
Conversation 1 Commits 1 Files changed 4 +67 -59
4 changed files with 67 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
cmd/frostfs-cli/modules/control/add_rule.go
View file

@ -1,16 +1,13 @@
package control
import (
"crypto/sha256"
"encoding/hex"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/commonflags"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/key"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/modules/util"
commonCmd "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"github.com/spf13/cobra"
)
@ -33,6 +30,8 @@ var addRuleCmd = &cobra.Command{
func addRule(cmd *cobra.Command, _ []string) {
pk := key.Get(cmd)
target := parseTarget(cmd)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
@ -44,13 +43,6 @@ func addRule(cmd *cobra.Command, _ []string) {
commonCmd.ExitOnErr(cmd, "can't decode chain ID as hex: %w", err)
}
var cnr cid.ID
cidStr, _ := cmd.Flags().GetString(commonflags.CIDFlag)
commonCmd.ExitOnErr(cmd, "can't decode container ID: %w", cnr.DecodeString(cidStr))
rawCID := make([]byte, sha256.Size)
cnr.Encode(rawCID)
rule, _ := cmd.Flags().GetStringArray(ruleFlag)
chain := new(apechain.Chain)
@ -63,10 +55,7 @@ func addRule(cmd *cobra.Command, _ []string) {
req := &control.AddChainLocalOverrideRequest{
Body: &control.AddChainLocalOverrideRequest_Body{
Target: &control.ChainTarget{
Type: control.ChainTarget_CONTAINER,
Name: cidStr,
},
Target: target,
Chain: serializedChain,
},
}
@ -91,8 +80,10 @@ func initControlAddRuleCmd() {
initControlFlags(addRuleCmd)
ff := addRuleCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.StringArray(ruleFlag, []string{}, "Rule statement")
ff.String(chainIDFlag, "", "Assign ID to the parsed chain")
ff.String(targetNameFlag, "", targetNameDesc)
ff.String(targetTypeFlag, "", targetTypeDesc)
_ = addRuleCmd.MarkFlagRequired(targetTypeFlag)
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}

19
cmd/frostfs-cli/modules/control/get_rule.go
View file

@ -1,16 +1,13 @@
package control
import (
"crypto/sha256"
"encoding/hex"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/commonflags"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/key"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/modules/util"
commonCmd "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"github.com/spf13/cobra"
)
@ -25,12 +22,7 @@ var getRuleCmd = &cobra.Command{
func getRule(cmd *cobra.Command, _ []string) {
pk := key.Get(cmd)
var cnr cid.ID
cidStr, _ := cmd.Flags().GetString(commonflags.CIDFlag)
commonCmd.ExitOnErr(cmd, "can't decode container ID: %w", cnr.DecodeString(cidStr))
rawCID := make([]byte, sha256.Size)
cnr.Encode(rawCID)
target := parseTarget(cmd)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
@ -43,10 +35,7 @@ func getRule(cmd *cobra.Command, _ []string) {
req := &control.GetChainLocalOverrideRequest{
Body: &control.GetChainLocalOverrideRequest_Body{
Target: &control.ChainTarget{
Name: cidStr,
Type: control.ChainTarget_CONTAINER,
},
Target: target,
ChainId: []byte(chainID),
},
}
@ -74,7 +63,9 @@ func initControGetRuleCmd() {
initControlFlags(getRuleCmd)
ff := getRuleCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(targetNameFlag, "", targetNameDesc)
ff.String(targetTypeFlag, "", targetTypeDesc)
_ = getRuleCmd.MarkFlagRequired(targetTypeFlag)
ff.String(chainIDFlag, "", "Chain id")
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}

65
cmd/frostfs-cli/modules/control/list_rules.go
View file

@ -1,16 +1,17 @@
package control
import (
"crypto/sha256"
"fmt"
"strings"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/commonflags"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/key"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/modules/util"
commonCmd "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"github.com/nspcc-dev/neo-go/cli/input"
"github.com/spf13/cobra"
)
@ -21,22 +22,56 @@ var listRulesCmd = &cobra.Command{
Run: listRules,
}
const (
defaultNamespace = "root"
namespaceTarget = "namespace"
containerTarget = "container"
)
const (
targetNameFlag = "target-name"
targetNameDesc = "Resource name in APE resource name format"
targetTypeFlag = "target-type"
targetTypeDesc = "Resource type(container/namespace)"
)
func parseTarget(cmd *cobra.Command) *control.ChainTarget {
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2024-02-05 08:03:25 +00:00
Outdated
Review
Copy link

I suggest requiring you to enter "root' explicitly. Because if I forgot to enter correct namespace, then root namespace will be affected.

I suggest requiring you to enter "root' explicitly. Because if I forgot to enter correct namespace, then root namespace will be affected.
dstepanov-yadro commented 2024-02-05 08:03:45 +00:00
Outdated
Review
Copy link

@aarifullin @fyrchik what do you think?

@aarifullin @fyrchik what do you think?
fyrchik commented 2024-02-05 08:34:43 +00:00
Outdated
Review
Copy link

Root is somewhat default namespace, but I like the idea.
Though, if I don't know anything about namespaces, it is somewhat irrelevant.
We could also print it in verbose mode.

Root is somewhat default namespace, but I like the idea. Though, if I don't know anything about namespaces, it is somewhat irrelevant. We could also print it in verbose mode.
fyrchik commented 2024-02-05 08:35:12 +00:00
Outdated
Review
Copy link

Actually, I would argue, that rules on some specific namespace should not be a common scenario for a node administrator.

Actually, I would argue, that rules on some specific namespace should not be a common scenario for a node administrator.
dstepanov-yadro commented 2024-02-05 12:31:30 +00:00
Outdated
Review
Copy link

explanation:

  • frostfs-cli add-rule ... without namespace leads to using root namespace
  • frostfs-cli add-rule --namespace leads to error
  • frostfs-cli add-rule --namespace root leads to using root namespace
explanation: * `frostfs-cli add-rule ...` without namespace leads to using root namespace * `frostfs-cli add-rule --namespace ` leads to error * `frostfs-cli add-rule --namespace root` leads to using root namespace
aarifullin commented 2024-02-05 13:17:46 +00:00
Outdated
Review
Copy link

@dstepanov-yadro

I have decided to make the usage consistent with frostfs-adm and introduced flags --target-type --target-name. That is correct because flags make the command usage more intuitive (and, again, consistent with admin util).
Also, I liked the idea suggested by @fyrchik to make an interaction confirm if you wanna use root namespace if name flag is not set.
If no one likes how it looks then I will just make --target-name flag as required and "" won't be acceptable for input

@dstepanov-yadro I have decided to make the usage consistent with `frostfs-adm` and introduced flags `--target-type --target-name`. That is correct because flags make the command usage more intuitive (and, again, consistent with admin util). Also, I liked the idea suggested by @fyrchik to make an interaction `confirm if you wanna use root namespace` if name flag is not set. If no one likes how it looks then I will just make `--target-name` flag as required and `""` won't be acceptable for input
dstepanov-yadro commented 2024-02-05 15:35:50 +00:00
Outdated
Review
Copy link

Fine!

Fine!
typ, _ := cmd.Flags().GetString(targetTypeFlag)
name, _ := cmd.Flags().GetString(targetNameFlag)
switch typ {
case namespaceTarget:
if name == "" {
ln, err := input.ReadLine(fmt.Sprintf("Target name is not set. Confirm to use %s namespace (n|Y)> ", defaultNamespace))
commonCmd.ExitOnErr(cmd, "read line error: %w", err)
ln = strings.ToLower(ln)
if len(ln) > 0 && (ln[0] == 'n') {
commonCmd.ExitOnErr(cmd, "read namespace error: %w", fmt.Errorf("setting default value was declined"))
}
name = defaultNamespace
}
return &control.ChainTarget{
Name: name,
Type: control.ChainTarget_NAMESPACE,
}
case containerTarget:
var cnr cid.ID
commonCmd.ExitOnErr(cmd, "can't decode container ID: %w", cnr.DecodeString(name))
return &control.ChainTarget{
Name: name,
Type: control.ChainTarget_CONTAINER,
}
default:
commonCmd.ExitOnErr(cmd, "read target type error: %w", fmt.Errorf("unknown target type"))
}
return nil
}
func listRules(cmd *cobra.Command, _ []string) {
pk := key.Get(cmd)
var cnr cid.ID
cidStr, _ := cmd.Flags().GetString(commonflags.CIDFlag)
commonCmd.ExitOnErr(cmd, "can't decode container ID: %w", cnr.DecodeString(cidStr))
rawCID := make([]byte, sha256.Size)
cnr.Encode(rawCID)
req := &control.ListChainLocalOverridesRequest{
Body: &control.ListChainLocalOverridesRequest_Body{
Target: &control.ChainTarget{
Name: cidStr,
Type: control.ChainTarget_CONTAINER,
},
Target: parseTarget(cmd),
},
}
@ -71,5 +106,7 @@ func initControlListRulesCmd() {
initControlFlags(listRulesCmd)
ff := listRulesCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(targetNameFlag, "", targetNameDesc)
ff.String(targetTypeFlag, "", targetTypeDesc)
_ = listRulesCmd.MarkFlagRequired(targetTypeFlag)
}

19
cmd/frostfs-cli/modules/control/remove_rule.go
View file

@ -1,15 +1,12 @@
package control
import (
"crypto/sha256"
"encoding/hex"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/commonflags"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-cli/internal/key"
commonCmd "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
"github.com/spf13/cobra"
)
@ -28,13 +25,6 @@ var removeRuleCmd = &cobra.Command{
func removeRule(cmd *cobra.Command, _ []string) {
pk := key.Get(cmd)
var cnr cid.ID
cidStr, _ := cmd.Flags().GetString(commonflags.CIDFlag)
commonCmd.ExitOnErr(cmd, "can't decode container ID: %w", cnr.DecodeString(cidStr))
rawCID := make([]byte, sha256.Size)
cnr.Encode(rawCID)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
@ -48,10 +38,7 @@ func removeRule(cmd *cobra.Command, _ []string) {
req := &control.RemoveChainLocalOverrideRequest{
Body: &control.RemoveChainLocalOverrideRequest_Body{
Target: &control.ChainTarget{
Name: cidStr,
Type: control.ChainTarget_CONTAINER,
},
Target: parseTarget(cmd),
ChainId: chainIDRaw,
},
}
@ -81,7 +68,9 @@ func initControlRemoveRuleCmd() {
initControlFlags(removeRuleCmd)
ff := removeRuleCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(targetNameFlag, "", targetNameDesc)
ff.String(targetTypeFlag, "", targetTypeDesc)
_ = removeRuleCmd.MarkFlagRequired(targetTypeFlag)
ff.String(chainIDFlag, "", "Chain id")
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}