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

Use bytes in control service proto definition for chainID #885

Merged
fyrchik merged 2 commits from dkirillov/frostfs-node:feature/control_api_make_chain_id_bytes into master 2024-01-11 07:24:27 +00:00
Conversation 0 Commits 2 Files changed 10 +45 -12
10 changed files with 45 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -3,6 +3,7 @@ package control
import (
"bytes"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
@ -42,6 +43,15 @@ func addRule(cmd *cobra.Command, _ []string) {
pk := key.Get(cmd)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
chainIDRaw := []byte(chainID)
if hexEncoded {
var err error
chainIDRaw, err = hex.DecodeString(chainID)
commonCmd.ExitOnErr(cmd, "can't decode chain ID as hex: %w", err)
}
var cnr cid.ID
cidStr, _ := cmd.Flags().GetString(commonflags.CIDFlag)
@ -54,7 +64,7 @@ func addRule(cmd *cobra.Command, _ []string) {
chain := new(apechain.Chain)
commonCmd.ExitOnErr(cmd, "parser error: %w", util.ParseAPEChain(chain, []string{rule}))
chain.ID = apechain.ID(chainID)
chain.ID = apechain.ID(chainIDRaw)
serializedChain := chain.Bytes()
cmd.Println("CID: " + cidStr)
@ -84,7 +94,8 @@ func addRule(cmd *cobra.Command, _ []string) {
verifyResponse(cmd, resp.GetSignature(), resp.GetBody())
cmd.Println("Rule has been added. Chain id: ", resp.GetBody().GetChainId())
chainIDRaw = resp.GetBody().GetChainId()
cmd.Printf("Rule has been added.\nChain id: '%s'\nChain id hex: '%x'\n", string(chainIDRaw), chainIDRaw)
}
func initControlAddRuleCmd() {
@ -94,4 +105,5 @@ func initControlAddRuleCmd() {
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(ruleFlag, "", "Rule statement")
ff.String(chainIDFlag, "", "Assign ID to the parsed chain")
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}

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

@ -2,6 +2,7 @@ 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"
@ -31,6 +32,13 @@ func getRule(cmd *cobra.Command, _ []string) {
cnr.Encode(rawCID)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
if hexEncoded {
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2023-12-21 15:46:44 +00:00
Outdated
Review
Copy link

Is chain ID user-defined string? If so, then hex looks redundant

Is chain ID user-defined string? If so, then hex looks redundant
dkirillov commented 2023-12-22 06:18:09 +00:00
Outdated
Review
Copy link

Yes, it's can be user-defined. But in case when we add local overrides from IAM service ids can be not-printable so we need some way to provide such id using frostfs-cli

Yes, it's can be user-defined. But in case when we add local overrides from IAM service ids can be not-printable so we need some way to provide such id using `frostfs-cli`
chainIDBytes, err := hex.DecodeString(chainID)
commonCmd.ExitOnErr(cmd, "can't decode chain ID as hex: %w", err)
chainID = string(chainIDBytes)
}
req := &control.GetChainLocalOverrideRequest{
Body: &control.GetChainLocalOverrideRequest_Body{
@ -38,7 +46,7 @@ func getRule(cmd *cobra.Command, _ []string) {
Name: cidStr,
Type: control.ChainTarget_CONTAINER,
},
ChainId: chainID,
ChainId: []byte(chainID),
},
}
@ -60,7 +68,7 @@ func getRule(cmd *cobra.Command, _ []string) {
commonCmd.ExitOnErr(cmd, "decode error: %w", chain.DecodeBytes(resp.GetBody().GetChain()))
// TODO (aarifullin): make pretty-formatted output for chains.
cmd.Println("Parsed chain:\n" + prettyJSONFormat(cmd, chain.Bytes()))
cmd.Printf("Parsed chain (chain id hex: '%x'):\n%s\n", chain.ID, prettyJSONFormat(cmd, chain.Bytes()))
}
func initControGetRuleCmd() {
@ -69,4 +77,5 @@ func initControGetRuleCmd() {
ff := getRuleCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(chainIDFlag, "", "Chain id")
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}

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

@ -63,7 +63,7 @@ func listRules(cmd *cobra.Command, _ []string) {
// TODO (aarifullin): make pretty-formatted output for chains.
var chain apechain.Chain
commonCmd.ExitOnErr(cmd, "decode error: %w", chain.DecodeBytes(c))
cmd.Println("Parsed chain:\n" + prettyJSONFormat(cmd, chain.Bytes()))
cmd.Printf("Parsed chain (chain id hex: '%x'):\n%s\n", chain.ID, prettyJSONFormat(cmd, chain.Bytes()))
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2023-12-22 10:49:22 +00:00
Outdated
Review
Copy link

Can we use utf8.IsValid and do not output string on false?

Can we use `utf8.IsValid` and do not output string on false?
dkirillov commented 2023-12-22 12:39:08 +00:00
Outdated
Review
Copy link

Do you mean not output json chain or which one string?

Do you mean not output json chain or which one string?
fyrchik commented 2023-12-22 12:49:15 +00:00
Outdated
Review
Copy link

Oh, sorry, thought it was a name.

Oh, sorry, thought it was a name.
}
}

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

@ -2,6 +2,7 @@ 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"
@ -14,6 +15,7 @@ import (
const (
chainIDFlag = "chain-id"
chainIDHexFlag = "chain-id-hex"
fyrchik commented 2023-12-22 10:49:58 +00:00
Outdated
Review
Copy link

Hex is not needed for cli, cli is for user.

Hex is not needed for cli, cli is for user.
dkirillov commented 2023-12-22 12:24:20 +00:00
Outdated
Review
Copy link

Yes, but we can set overrides via IAM #885 (comment)

Yes, but we can set overrides via IAM https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/885#issuecomment-29780
fyrchik commented 2023-12-22 12:49:59 +00:00
Outdated
Review
Copy link

We need this in control service, but hex-encoding is a purely CLI thing.

We need this in control service, but hex-encoding is a purely CLI thing.
dkirillov commented 2023-12-22 12:58:31 +00:00
Outdated
Review
Copy link

Let's consider the use-case:

  1. Someone (e.g. IAM) set non-utf8 id
  2. User using cli lists all policies and see policy from step 1
  3. User want to delete/get policy from step 1. He needs somehow pass non-utf8 id via CLI

If this isn't a case, I can keep only this commit

Let's consider the use-case: 1. Someone (e.g. IAM) set non-utf8 id 2. User using cli lists all policies and see policy from step 1 3. User want to delete/get policy from step 1. He needs somehow pass non-utf8 id via CLI If this isn't a case, I can keep only [this commit](https://git.frostfs.info/TrueCloudLab/frostfs-node/commit/2a1d79a37dc45c723c9edf49d2ee46084d10f5fb)
fyrchik commented 2023-12-22 19:22:43 +00:00
Outdated
Review
Copy link

Control service lists only local overrides, IAM uses contract.

Control service lists only local overrides, IAM uses contract.
dkirillov commented 2024-01-09 06:44:09 +00:00
Outdated
Review
Copy link

IAM uses not only contract. It sets local overrides in s3/node too

IAM uses not only contract. It sets local overrides in s3/node too
👍 1
)
var removeRuleCmd = &cobra.Command{
@ -34,6 +36,15 @@ func removeRule(cmd *cobra.Command, _ []string) {
cnr.Encode(rawCID)
chainID, _ := cmd.Flags().GetString(chainIDFlag)
hexEncoded, _ := cmd.Flags().GetBool(chainIDHexFlag)
chainIDRaw := []byte(chainID)
if hexEncoded {
var err error
chainIDRaw, err = hex.DecodeString(chainID)
commonCmd.ExitOnErr(cmd, "can't decode chain ID as hex: %w", err)
}
req := &control.RemoveChainLocalOverrideRequest{
Body: &control.RemoveChainLocalOverrideRequest_Body{
@ -41,7 +52,7 @@ func removeRule(cmd *cobra.Command, _ []string) {
Name: cidStr,
Type: control.ChainTarget_CONTAINER,
},
ChainId: chainID,
ChainId: chainIDRaw,
},
}
@ -72,4 +83,5 @@ func initControlRemoveRuleCmd() {
ff := removeRuleCmd.Flags()
ff.String(commonflags.CIDFlag, "", commonflags.CIDFlagUsage)
ff.String(chainIDFlag, "", "Chain id")
ff.Bool(chainIDHexFlag, false, "Flag to parse chain ID as hex")
}

BIN
pkg/services/control/ir/service_grpc.pb.go generated
View file

Binary file not shown.

4
pkg/services/control/server/policy_engine.go
View file

@ -7,7 +7,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
engine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
@ -51,7 +51,7 @@ func (s *Server) AddChainLocalOverride(_ context.Context, req *control.AddChainL
resp := &control.AddChainLocalOverrideResponse{
Body: &control.AddChainLocalOverrideResponse_Body{
ChainId: string(chain.ID),
ChainId: []byte(chain.ID),
},
}
err = SignMessage(s.key, resp)

BIN
pkg/services/control/service.pb.go generated
View file

Binary file not shown.

6
pkg/services/control/service.proto
View file

@ -446,7 +446,7 @@ message AddChainLocalOverrideResponse {
// Chain ID assigned for the added rule chain.
// If chain ID is left empty in the request, then
// it will be generated.
string chain_id = 1;
bytes chain_id = 1;
}
Body body = 1;
@ -461,7 +461,7 @@ message GetChainLocalOverrideRequest {
ChainTarget target = 1;
// Chain ID assigned for the added rule chain.
string chain_id = 2;
bytes chain_id = 2;
}
Body body = 1;
@ -511,7 +511,7 @@ message RemoveChainLocalOverrideRequest {
ChainTarget target = 1;
// Chain ID assigned for the added rule chain.
string chain_id = 2;
bytes chain_id = 2;
}
Body body = 1;

BIN
pkg/services/control/service_frostfs.pb.go generated
View file

Binary file not shown.

BIN
pkg/services/tree/service_grpc.pb.go generated
View file

Binary file not shown.