From e7bc3f88f19a604ce5d70aa24f73f86d18ef875d Mon Sep 17 00:00:00 2001 From: "May.Buzaglo" Date: Thu, 24 Sep 2026 19:14:58 +0300 Subject: [PATCH 1/2] json manipulation automation skeleton Signed-off-by: May.Buzaglo --- tools/fxadmin/core/cli/cli.go | 177 ++++++++++++++++++-- tools/fxadmin/core/cli/cli_test.go | 153 +++++++++++++++++ tools/fxadmin/core/cli/handlers.go | 45 ++++- tools/fxadmin/core/modify/modify.go | 85 ++++++++++ tools/fxadmin/core/modify/modify_test.go | 33 ++++ tools/fxadmin/core/update/compute_update.go | 9 + 6 files changed, 490 insertions(+), 12 deletions(-) create mode 100644 tools/fxadmin/core/modify/modify.go create mode 100644 tools/fxadmin/core/modify/modify_test.go diff --git a/tools/fxadmin/core/cli/cli.go b/tools/fxadmin/core/cli/cli.go index d6fa51b2a..817462829 100644 --- a/tools/fxadmin/core/cli/cli.go +++ b/tools/fxadmin/core/cli/cli.go @@ -22,6 +22,19 @@ const ( flagCurrentBlock = "current-block" flagOutput = "output" flagTimeout = "timeout" + + // Flags used by compute-update block mode and the modify commands. + flagPB = "pb" + flagBlock = "block" + flagOrg = "org" + flagParty = "party" + flagCert = "cert" + flagRole = "role" + flagShard = "shard" + flagHost = "host" + flagPort = "port" + flagTLSCert = "tls-cert" + flagSignCert = "sign-cert" ) // CLI is the fxadmin command-line application. It owns the kingpin command @@ -47,6 +60,7 @@ func New(handlers Handlers, version string) *CLI { c.addLedgerCommands() c.addDecodeCommand() + c.addModifyCommands() c.addComputeUpdateCommand() c.addTxCommands() c.addFollowCommand() @@ -131,24 +145,30 @@ func (c *CLI) addDecodeCommand() { } // addComputeUpdateCommand wires `fxadmin compute-update`, which computes the -// ConfigUpdate delta between the original and modified configuration JSON. The -// channel ID the update targets is read from the current config block supplied with --current-block. +// ConfigUpdate delta between an original and a modified configuration. In the +// default JSON mode the two positional arguments are decoded-config JSON files +// and the channel ID is read from --current-block. In block mode (--pb) they are +// two config block files (original, next). func (c *CLI) addComputeUpdateCommand() { - cmd := c.app.Command("compute-update", "Compute the ConfigUpdate delta between two config JSON files.") - current := cmd.Arg("current.json", "Original configuration JSON, as decoded from --current-block."). + cmd := c.app.Command("compute-update", + "Compute the ConfigUpdate delta between two configs (JSON files, or config blocks).") + current := cmd.Arg("current", "Original config: JSON (default) or config block (--pb)."). Required().ExistingFile() - modified := cmd.Arg("modified.json", "Modified configuration JSON (edited copy of current.json)."). + modified := cmd.Arg("modified", "Modified config: JSON (default) or the edited \"next\" config block (--pb)."). Required().ExistingFile() + pb := cmd.Flag(flagPB, "Block mode: treat the two arguments as config blocks (original, next) instead of JSON."). + Bool() currentBlock := cmd. - Flag( - flagCurrentBlock, - "Path to the current config block whose channel ID the update targets; "+ - "must be the same channel as current.json and modified.json.", - ). - Required(). + Flag(flagCurrentBlock, "JSON mode only: Path to the current config block whose channel ID the update targets."). ExistingFile() output := cmd.Flag(flagOutput, "Path to the output ConfigUpdate protobuf file.").Required().String() c.register(cmd, func() error { + if *pb { + return c.handlers.Update.RunFromBlocks(*current, *modified, *output) + } + if *currentBlock == "" { + return errors.Newf("--%s is required in JSON mode (or pass --%s for block mode)", flagCurrentBlock, flagPB) + } return c.handlers.Update.Run(*current, *modified, *currentBlock, *output) }) } @@ -254,3 +274,138 @@ func (c *CLI) addFollowCommand() { return c.handlers.Follow.Run(*config, *currentBlock, *output, *timeout) }) } + +// addModifyCommands wires `fxadmin modify` and its app/party subcommands, which +// apply a structured change directly to a config block file, +// automating the manual decode/hand-edit step. +func (c *CLI) addModifyCommands() { + modify := c.app.Command("modify", + "Apply a structured configuration change to a config block file.") + c.addModifyAppCommands(modify) + c.addModifyPartyCommands(modify) +} + +// blockFlag registers the --block flag common to every modify command: the +// config block file the command edits in place. +func blockFlag(cmd *kingpin.CmdClause) *string { + return cmd.Flag(flagBlock, "Config block file to edit in place (the \"next\" block)."). + Required().ExistingFile() +} + +// addModifyAppCommands wires `fxadmin modify app` (add / remove / known-certs). +func (c *CLI) addModifyAppCommands(modify *kingpin.CmdClause) { + app := modify.Command("app", "Add or remove application organizations and their known-certs.") + + add := app.Command("add", "Add an application organization.") + addOrg := add.Flag(flagOrg, "Path to the organization definition YAML.").Required().ExistingFile() + addBlockFlag := blockFlag(add) + c.register(add, func() error { + return c.handlers.Modify.AppAdd(*addOrg, *addBlockFlag) + }) + + remove := app.Command("remove", "Remove an application organization.") + removeOrg := remove.Arg("org", "Application organization name to remove.").Required().String() + removeBlockFlag := blockFlag(remove) + c.register(remove, func() error { + return c.handlers.Modify.AppRemove(*removeOrg, *removeBlockFlag) + }) + + c.addModifyKnownCertsCommands(app) +} + +// addModifyKnownCertsCommands wires `fxadmin modify app known-certs add|remove`. +func (c *CLI) addModifyKnownCertsCommands(app *kingpin.CmdClause) { + known := app.Command("known-certs", "Add or remove entries in an application org's MSP known-certs list.") + + add := known.Command("add", "Add known-certs to an application organization.") + addOrg := add.Flag(flagOrg, "Application organization whose known-certs change.").Required().String() + addCerts := add.Flag(flagCert, "PEM path to add (repeatable).").Required().ExistingFiles() + addBlockFlag := blockFlag(add) + c.register(add, func() error { + return c.handlers.Modify.AppKnownCertsAdd(*addOrg, *addCerts, *addBlockFlag) + }) + + remove := known.Command("remove", "Remove known-certs from an application organization.") + removeOrg := remove.Flag(flagOrg, "Application organization whose known-certs change.").Required().String() + removeCerts := remove.Flag(flagCert, "PEM path to remove (repeatable).").Required().ExistingFiles() + removeBlockFlag := blockFlag(remove) + c.register(remove, func() error { + return c.handlers.Modify.AppKnownCertsRemove(*removeOrg, *removeCerts, *removeBlockFlag) + }) +} + +// addModifyPartyCommands wires `fxadmin modify party` (add / remove / node / ca). +func (c *CLI) addModifyPartyCommands(modify *kingpin.CmdClause) { + party := modify.Command("party", "Add or remove ARMA parties, or change a party's nodes and CA lists.") + + add := party.Command("add", "Add a new ARMA party and its orderer organization, if it does not already exist.") + partyDef := add.Flag(flagParty, "Path to the party definition YAML.").Required().ExistingFile() + addBlockFlag := blockFlag(add) + c.register(add, func() error { + return c.handlers.Modify.PartyAdd(*partyDef, *addBlockFlag) + }) + + remove := party.Command("remove", + "Remove an ARMA party (and its orderer org, unless another party is still associated with it).") + partyID := remove.Arg("party-id", "Numeric PartyID to remove.").Required().Uint32() + removeBlockFlag := blockFlag(remove) + c.register(remove, func() error { + return c.handlers.Modify.PartyRemove(*partyID, *removeBlockFlag) + }) + + c.addModifyPartyNodeCommand(party) + c.addModifyPartyCACommands(party) +} + +// addModifyPartyNodeCommand wires `fxadmin modify party node`, which changes any +// subset of one node's endpoint and certificate fields. +func (c *CLI) addModifyPartyNodeCommand(party *kingpin.CmdClause) { + node := party.Command("node", "Change one party node's endpoint and/or certificates (any subset of fields).") + partyID := node.Flag(flagParty, "Party ID.").Required().Uint32() + role := node.Flag(flagRole, "Node role.").Required().Enum("router", "batcher", "consenter", "assembler") + shard := node.Flag(flagShard, "Batcher shard ID (required for --role batcher).").Uint32() + host := node.Flag(flagHost, "New host (unchanged if omitted).").String() + port := node.Flag(flagPort, "New port (unchanged if omitted).").Uint32() + tlsCert := node.Flag(flagTLSCert, "Path to new TLS certificate (unchanged if omitted).").ExistingFile() + signCert := node.Flag(flagSignCert, "Path to new signing certificate (unchanged if omitted).").ExistingFile() + block := blockFlag(node) + c.register(node, func() error { + return c.handlers.Modify.PartyNode(NodeChange{ + Party: *partyID, + Role: *role, + Shard: *shard, + Host: *host, + Port: *port, + TLSCert: *tlsCert, + SignCert: *signCert, + BlockPath: *block, + }) + }) +} + +// addModifyPartyCACommands wires `fxadmin modify party ca add|remove|set` over a +// party's CA (--sign-cert) and TLS-CA (--tls-cert) certificate lists. +func (c *CLI) addModifyPartyCACommands(party *kingpin.CmdClause) { + ca := party.Command("ca", "Change a party's CA and/or TLS-CA certificate lists.") + for _, op := range []string{"add", "remove", "set"} { + c.registerPartyCAOp(ca, op) + } +} + +// registerPartyCAOp wires one `modify party ca` sub-verb (add, remove, or set). +func (c *CLI) registerPartyCAOp(ca *kingpin.CmdClause, op string) { + cmd := ca.Command(op, "Apply the "+op+" operation to a party's CA / TLS-CA certificate lists.") + partyID := cmd.Flag(flagParty, "Party whose CA list(s) change.").Required().Uint32() + signCerts := cmd.Flag(flagSignCert, "PEM path(s) for the signing-CA list (repeatable).").ExistingFiles() + tlsCerts := cmd.Flag(flagTLSCert, "PEM path(s) for the TLS-CA list (repeatable).").ExistingFiles() + block := blockFlag(cmd) + c.register(cmd, func() error { + return c.handlers.Modify.PartyCA(CAChange{ + Op: op, + Party: *partyID, + SignCerts: *signCerts, + TLSCerts: *tlsCerts, + BlockPath: *block, + }) + }) +} diff --git a/tools/fxadmin/core/cli/cli_test.go b/tools/fxadmin/core/cli/cli_test.go index 811c0461f..508fea341 100644 --- a/tools/fxadmin/core/cli/cli_test.go +++ b/tools/fxadmin/core/cli/cli_test.go @@ -9,6 +9,7 @@ package cli_test import ( "os" "path/filepath" + "strconv" "testing" "time" @@ -21,13 +22,23 @@ import ( const ( cmdLedger = "ledger" cmdFollow = "follow" + cmdModify = "modify" subHeight = "height" + subRemove = "remove" + subAdd = "add" + subApp = "app" + subParty = "party" flagConfig = "--config" flagCurrentBlock = "--current-block" flagOutput = "--output" + flagBlock = "--block" + flagOrg = "--org" + flagParty = "--party" + flagCert = "--cert" refLatest = "latest" + orgPeer1 = "peer1" fileConfigUpdate = "config_update.pb" fileEndorsement1 = "endorsed_config_update1.pb" @@ -55,6 +66,7 @@ func newHandlers(invoked *call) cli.Handlers { Update: fakeUpdate{invoked}, Tx: fakeTx{invoked}, Follow: fakeFollow{invoked}, + Modify: fakeModify{invoked}, } } @@ -92,6 +104,14 @@ func (f fakeUpdate) Run(currentPath, modifiedPath, currentBlockPath, outputPath return nil } +func (f fakeUpdate) RunFromBlocks(currentBlockPath, nextBlockPath, outputPath string) error { + *f.invoked = call{ + handler: "ComputeUpdatePB", + args: []string{currentBlockPath, nextBlockPath, outputPath}, + } + return nil +} + type fakeTx struct{ invoked *call } func (f fakeTx) Endorse(inputPath, configPath, outputPath string) error { @@ -126,6 +146,65 @@ func (f fakeFollow) Run(configPath, currentBlockPath, outputPath string, timeout return nil } +type fakeModify struct{ invoked *call } + +func (f fakeModify) AppAdd(orgPath, blockPath string) error { + *f.invoked = call{handler: "ModifyAppAdd", args: []string{orgPath, blockPath}} + return nil +} + +func (f fakeModify) AppRemove(org, blockPath string) error { + *f.invoked = call{handler: "ModifyAppRemove", args: []string{org, blockPath}} + return nil +} + +func (f fakeModify) AppKnownCertsAdd(org string, certPaths []string, blockPath string) error { + *f.invoked = certCall("ModifyAppKnownCertsAdd", org, certPaths, blockPath) + return nil +} + +func (f fakeModify) AppKnownCertsRemove(org string, certPaths []string, blockPath string) error { + *f.invoked = certCall("ModifyAppKnownCertsRemove", org, certPaths, blockPath) + return nil +} + +// certCall records a known-certs invocation as {org, certPaths…, blockPath}. +func certCall(handler, org string, certPaths []string, blockPath string) call { + args := make([]string, 0, len(certPaths)+2) + args = append(args, org) + args = append(args, certPaths...) + args = append(args, blockPath) + return call{handler: handler, args: args} +} + +func (f fakeModify) PartyAdd(partyPath, blockPath string) error { + *f.invoked = call{handler: "ModifyPartyAdd", args: []string{partyPath, blockPath}} + return nil +} + +func (f fakeModify) PartyRemove(partyID uint32, blockPath string) error { + *f.invoked = call{handler: "ModifyPartyRemove", args: []string{strconv.FormatUint(uint64(partyID), 10), blockPath}} + return nil +} + +func (f fakeModify) PartyNode(ch cli.NodeChange) error { + *f.invoked = call{handler: "ModifyPartyNode", args: []string{ + strconv.FormatUint(uint64(ch.Party), 10), ch.Role, strconv.FormatUint(uint64(ch.Shard), 10), + ch.Host, strconv.FormatUint(uint64(ch.Port), 10), ch.TLSCert, ch.SignCert, ch.BlockPath, + }} + return nil +} + +func (f fakeModify) PartyCA(ch cli.CAChange) error { + args := make([]string, 0, len(ch.SignCerts)+len(ch.TLSCerts)+3) + args = append(args, ch.Op, strconv.FormatUint(uint64(ch.Party), 10)) + args = append(args, ch.SignCerts...) + args = append(args, ch.TLSCerts...) + args = append(args, ch.BlockPath) + *f.invoked = call{handler: "ModifyPartyCA", args: args} + return nil +} + // TestRunRoutesToHandler feeds argument vectors through the real command tree // and asserts each one selects the expected handler with the expected values. // The placeholder tokens are replaced with real temp-file paths so the ExistingFile flag validation passes. @@ -141,6 +220,11 @@ func TestRunRoutesToHandler(t *testing.T) { endorsement2 := writeTempFile(t, fileEndorsement2) endorsed := writeTempFile(t, fileEndorsed) configTx := writeTempFile(t, fileConfigTx) + nextBlock := writeTempFile(t, "next.pb") + orgYAML := writeTempFile(t, "org.yaml") + partyYAML := writeTempFile(t, "party.yaml") + cert1 := writeTempFile(t, "cert1.pem") + cert2 := writeTempFile(t, "cert2.pem") for _, tc := range []struct { name string @@ -207,6 +291,75 @@ func TestRunRoutesToHandler(t *testing.T) { wantHandler: "ComputeUpdate", wantArgs: []string{currentJSON, modifiedJSON, currBlock, fileConfigUpdate}, }, + { + name: "compute-update block mode", + args: []string{ + "compute-update", "--pb", currBlock, nextBlock, flagOutput, fileConfigUpdate, + }, + wantHandler: "ComputeUpdatePB", + wantArgs: []string{currBlock, nextBlock, fileConfigUpdate}, + }, + { + name: "modify app add", + args: []string{cmdModify, subApp, subAdd, flagOrg, orgYAML, flagBlock, nextBlock}, + wantHandler: "ModifyAppAdd", + wantArgs: []string{orgYAML, nextBlock}, + }, + { + name: "modify app remove", + args: []string{cmdModify, subApp, subRemove, "peer2", flagBlock, nextBlock}, + wantHandler: "ModifyAppRemove", + wantArgs: []string{"peer2", nextBlock}, + }, + { + name: "modify app known-certs add", + args: []string{ + cmdModify, subApp, "known-certs", subAdd, flagOrg, orgPeer1, + flagCert, cert1, flagCert, cert2, flagBlock, nextBlock, + }, + wantHandler: "ModifyAppKnownCertsAdd", + wantArgs: []string{orgPeer1, cert1, cert2, nextBlock}, + }, + { + name: "modify app known-certs remove", + args: []string{ + cmdModify, subApp, "known-certs", subRemove, flagOrg, orgPeer1, flagCert, cert1, flagBlock, nextBlock, + }, + wantHandler: "ModifyAppKnownCertsRemove", + wantArgs: []string{orgPeer1, cert1, nextBlock}, + }, + { + name: "modify party add", + args: []string{cmdModify, subParty, subAdd, flagParty, partyYAML, flagBlock, nextBlock}, + wantHandler: "ModifyPartyAdd", + wantArgs: []string{partyYAML, nextBlock}, + }, + { + name: "modify party remove", + args: []string{cmdModify, subParty, subRemove, "5", flagBlock, nextBlock}, + wantHandler: "ModifyPartyRemove", + wantArgs: []string{"5", nextBlock}, + }, + { + name: "modify party node", + args: []string{ + cmdModify, subParty, "node", flagParty, "1", "--role", "batcher", "--shard", "1", + "--port", "9014", "--tls-cert", cert1, flagBlock, nextBlock, + }, + wantHandler: "ModifyPartyNode", + // party, role, shard, host, port, tls-cert, sign-cert, block + wantArgs: []string{"1", "batcher", "1", "", "9014", cert1, "", nextBlock}, + }, + { + name: "modify party ca add", + args: []string{ + cmdModify, subParty, "ca", subAdd, flagParty, "1", + "--sign-cert", cert1, "--tls-cert", cert2, flagBlock, nextBlock, + }, + wantHandler: "ModifyPartyCA", + // op, party, sign-certs…, tls-certs…, block + wantArgs: []string{"add", "1", cert1, cert2, nextBlock}, + }, { name: "tx endorse", args: []string{"tx", "endorse", configUpdate, flagConfig, admin, flagOutput, fileEndorsement1}, diff --git a/tools/fxadmin/core/cli/handlers.go b/tools/fxadmin/core/cli/handlers.go index e8f22fb2a..481c249ef 100644 --- a/tools/fxadmin/core/cli/handlers.go +++ b/tools/fxadmin/core/cli/handlers.go @@ -31,9 +31,50 @@ type DecodeHandler interface { Run(blockPath, outputPath string) error } -// UpdateHandler executes `fxadmin compute-update`. +// UpdateHandler executes `fxadmin compute-update` in its two modes: JSON mode +// (Run) diffs two decoded-config JSON files, block mode (RunFromBlocks, the +// --pb flag) diffs two config block files. type UpdateHandler interface { Run(currentPath, modifiedPath, currentBlockPath, outputPath string) error + RunFromBlocks(currentBlockPath, nextBlockPath, outputPath string) error +} + +// NodeChange is a `modify party node` request. It selects one node of a party +// (Role, plus Shard for a batcher) and carries the fields to change; an empty +// field is left unchanged. +type NodeChange struct { + Party uint32 + Role string + Shard uint32 + Host string + Port uint32 + TLSCert string + SignCert string + BlockPath string +} + +// CAChange is a `modify party ca add|remove|set` request over a party's CA +// (SignCerts) and TLS-CA (TLSCerts) certificate lists. Op is the sub-verb. +// An empty field is left unchanged. +type CAChange struct { + Op string + Party uint32 + SignCerts []string + TLSCerts []string + BlockPath string +} + +// ModifyHandler executes the `fxadmin modify` subcommands, which apply a +// structured configuration change directly to a config block file. +type ModifyHandler interface { + AppAdd(orgPath, blockPath string) error + AppRemove(org, blockPath string) error + AppKnownCertsAdd(org string, certPaths []string, blockPath string) error + AppKnownCertsRemove(org string, certPaths []string, blockPath string) error + PartyAdd(partyPath, blockPath string) error + PartyRemove(partyID uint32, blockPath string) error + PartyNode(change NodeChange) error + PartyCA(change CAChange) error } // TxHandler executes the `fxadmin tx` subcommands. @@ -58,6 +99,7 @@ type Handlers struct { Update UpdateHandler Tx TxHandler Follow FollowHandler + Modify ModifyHandler } // validate reports any command handler that was left nil, so a misconfigured @@ -70,6 +112,7 @@ func (h Handlers) validate() error { "compute-update": isHandlerNil(h.Update), "tx": isHandlerNil(h.Tx), "follow": isHandlerNil(h.Follow), + "modify": isHandlerNil(h.Modify), } { if !set { missing = append(missing, name) diff --git a/tools/fxadmin/core/modify/modify.go b/tools/fxadmin/core/modify/modify.go new file mode 100644 index 000000000..7363739c7 --- /dev/null +++ b/tools/fxadmin/core/modify/modify.go @@ -0,0 +1,85 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +// Package modify implements the `fxadmin modify` commands, which apply a +// structured configuration change directly to a config block file (automating +// the manual decode/hand-edit step). Each command reads the block, applies the +// change to the embedded common.Config, and writes the block back so further +// modify commands can accumulate onto it; `compute-update --pb` then computes +// the ConfigUpdate from the original and edited blocks. +// +// The command surface is wired and validated by the CLI; the change logic is +// not implemented yet, so every handler currently returns ErrNotImplemented. +package modify + +import ( + "github.com/cockroachdb/errors" + "github.com/hyperledger/fabric-lib-go/common/flogging" + + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/cli" +) + +var logger = flogging.MustGetLogger("fxadmin.modify") + +// ErrNotImplemented is returned by every modify command until the change logic +// is implemented. +var ErrNotImplemented = errors.New("not implemented") + +// Handler executes the modify commands. +type Handler struct{} + +// New returns a modify command handler. +func New() *Handler { + return &Handler{} +} + +// AppAdd implements `fxadmin modify app add`. +func (*Handler) AppAdd(orgPath, blockPath string) error { + logger.Debugf("modify app add: org=%s block=%s (not implemented)", orgPath, blockPath) + return ErrNotImplemented +} + +// AppRemove implements `fxadmin modify app remove`. +func (*Handler) AppRemove(org, blockPath string) error { + logger.Debugf("modify app remove: org=%s block=%s (not implemented)", org, blockPath) + return ErrNotImplemented +} + +// AppKnownCertsAdd implements `fxadmin modify app known-certs add`. +func (*Handler) AppKnownCertsAdd(org string, certPaths []string, blockPath string) error { + logger.Debugf("modify app known-certs add: org=%s certs=%v block=%s (not implemented)", org, certPaths, blockPath) + return ErrNotImplemented +} + +// AppKnownCertsRemove implements `fxadmin modify app known-certs remove`. +func (*Handler) AppKnownCertsRemove(org string, certPaths []string, blockPath string) error { + logger.Debugf("modify app known-certs remove: org=%s certs=%v block=%s (not impl.)", org, certPaths, blockPath) + return ErrNotImplemented +} + +// PartyAdd implements `fxadmin modify party add`. +func (*Handler) PartyAdd(partyPath, blockPath string) error { + logger.Debugf("modify party add: party=%s block=%s (not implemented)", partyPath, blockPath) + return ErrNotImplemented +} + +// PartyRemove implements `fxadmin modify party remove`. +func (*Handler) PartyRemove(partyID uint32, blockPath string) error { + logger.Debugf("modify party remove: partyID=%d block=%s (not implemented)", partyID, blockPath) + return ErrNotImplemented +} + +// PartyNode implements `fxadmin modify party node`. +func (*Handler) PartyNode(change cli.NodeChange) error { + logger.Debugf("modify party node: %+v (not implemented)", change) + return ErrNotImplemented +} + +// PartyCA implements `fxadmin modify party ca add|remove|set`. +func (*Handler) PartyCA(change cli.CAChange) error { + logger.Debugf("modify party ca: %+v (not implemented)", change) + return ErrNotImplemented +} diff --git a/tools/fxadmin/core/modify/modify_test.go b/tools/fxadmin/core/modify/modify_test.go new file mode 100644 index 000000000..2944bb5de --- /dev/null +++ b/tools/fxadmin/core/modify/modify_test.go @@ -0,0 +1,33 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package modify_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/cli" + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify" +) + +// TestHandlerNotImplemented asserts every modify command currently returns +// ErrNotImplemented, since the change logic is not implemented yet. +func TestHandlerNotImplemented(t *testing.T) { + t.Parallel() + + h := modify.New() + + require.ErrorIs(t, h.AppAdd("org.yaml", "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.AppRemove("peer2", "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.AppKnownCertsAdd("peer1", []string{"c.pem"}, "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.AppKnownCertsRemove("peer1", []string{"c.pem"}, "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyAdd("party.yaml", "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyRemove(5, "next.pb"), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyNode(cli.NodeChange{Party: 1, Role: "batcher"}), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyCA(cli.CAChange{Op: "add", Party: 1}), modify.ErrNotImplemented) +} diff --git a/tools/fxadmin/core/update/compute_update.go b/tools/fxadmin/core/update/compute_update.go index e07938a61..5fc29c722 100644 --- a/tools/fxadmin/core/update/compute_update.go +++ b/tools/fxadmin/core/update/compute_update.go @@ -92,6 +92,15 @@ func (*Handler) Run(currentPath, modifiedPath, currentBlockPath, outputPath stri return nil } +// RunFromBlocks implements the block mode of `fxadmin compute-update` (--pb): it +// computes the ConfigUpdate delta between two config block files — the original +// block and the "next" block edited by the modify commands. Not implemented yet. +func (*Handler) RunFromBlocks(currentBlockPath, nextBlockPath, outputPath string) error { + logger.Debugf("compute-update --pb: current=%s next=%s output=%s (not implemented)", + currentBlockPath, nextBlockPath, outputPath) + return errors.New("compute-update block mode (--pb) is not implemented yet") +} + // channelIDFromBlock reads the config block at path and returns the channel ID // from its channel header, which is the channel the ConfigUpdate targets. An // empty channel ID is rejected. From 47b0ecf4048768e4ffb4ae50c3318762cc716329 Mon Sep 17 00:00:00 2001 From: "May.Buzaglo" Date: Sun, 4 Oct 2026 22:37:38 +0300 Subject: [PATCH 2/2] address review comments Signed-off-by: May.Buzaglo --- tools/fxadmin/core/cli/cli.go | 53 ++++++-- tools/fxadmin/core/cli/cli_test.go | 151 +++++++++++++++++---- tools/fxadmin/core/cli/handlers.go | 31 +---- tools/fxadmin/core/modify/change/change.go | 33 +++++ tools/fxadmin/core/modify/modify.go | 10 +- tools/fxadmin/core/modify/modify_test.go | 6 +- 6 files changed, 218 insertions(+), 66 deletions(-) create mode 100644 tools/fxadmin/core/modify/change/change.go diff --git a/tools/fxadmin/core/cli/cli.go b/tools/fxadmin/core/cli/cli.go index 817462829..227d9187f 100644 --- a/tools/fxadmin/core/cli/cli.go +++ b/tools/fxadmin/core/cli/cli.go @@ -12,8 +12,12 @@ SPDX-License-Identifier: Apache-2.0 package cli import ( + "math" + "github.com/alecthomas/kingpin/v2" "github.com/cockroachdb/errors" + + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify/change" ) // Flag names shared across commands. @@ -37,6 +41,9 @@ const ( flagSignCert = "sign-cert" ) +// roleBatcher is the --role value of a batcher, the only node role addressed by --shard. +const roleBatcher = "batcher" + // CLI is the fxadmin command-line application. It owns the kingpin command // tree and a dispatch table mapping each command to a closure // that invokes the corresponding Handlers method with the bound values. @@ -162,13 +169,19 @@ func (c *CLI) addComputeUpdateCommand() { Flag(flagCurrentBlock, "JSON mode only: Path to the current config block whose channel ID the update targets."). ExistingFile() output := cmd.Flag(flagOutput, "Path to the output ConfigUpdate protobuf file.").Required().String() + // --current-block is optional because block mode does not use it, so JSON + // mode requires it here. ExistingFile rejects an empty path, so an empty value + // means the flag was omitted. + cmd.Validate(func(*kingpin.CmdClause) error { + if !*pb && *currentBlock == "" { + return errors.Newf("--%s is required in JSON mode (or pass --%s for block mode)", flagCurrentBlock, flagPB) + } + return nil + }) c.register(cmd, func() error { if *pb { return c.handlers.Update.RunFromBlocks(*current, *modified, *output) } - if *currentBlock == "" { - return errors.Newf("--%s is required in JSON mode (or pass --%s for block mode)", flagCurrentBlock, flagPB) - } return c.handlers.Update.Run(*current, *modified, *currentBlock, *output) }) } @@ -362,15 +375,33 @@ func (c *CLI) addModifyPartyCommands(modify *kingpin.CmdClause) { func (c *CLI) addModifyPartyNodeCommand(party *kingpin.CmdClause) { node := party.Command("node", "Change one party node's endpoint and/or certificates (any subset of fields).") partyID := node.Flag(flagParty, "Party ID.").Required().Uint32() - role := node.Flag(flagRole, "Node role.").Required().Enum("router", "batcher", "consenter", "assembler") - shard := node.Flag(flagShard, "Batcher shard ID (required for --role batcher).").Uint32() + role := node.Flag(flagRole, "Node role.").Required().Enum("router", roleBatcher, "consenter", "assembler") + var shardSet bool + shard := node.Flag(flagShard, "Batcher shard ID (required for, and only valid with, --role batcher)."). + IsSetByUser(&shardSet).Uint32() host := node.Flag(flagHost, "New host (unchanged if omitted).").String() - port := node.Flag(flagPort, "New port (unchanged if omitted).").Uint32() + var portSet bool + port := node.Flag(flagPort, "New port, 1-65535 (unchanged if omitted).").IsSetByUser(&portSet).Uint32() tlsCert := node.Flag(flagTLSCert, "Path to new TLS certificate (unchanged if omitted).").ExistingFile() signCert := node.Flag(flagSignCert, "Path to new signing certificate (unchanged if omitted).").ExistingFile() block := blockFlag(node) + // A batcher is selected by its shard and no other role has one, so --shard is + // required exactly when --role is batcher. Whether the flag was given is checked, + // not its value, so an explicit --shard 0 is accepted. + node.Validate(func(*kingpin.CmdClause) error { + if *role == roleBatcher && !shardSet { + return errors.Newf("--%s is required for --%s %s", flagShard, flagRole, roleBatcher) + } + if *role != roleBatcher && shardSet { + return errors.Newf("--%s is only valid with --%s %s", flagShard, flagRole, roleBatcher) + } + if portSet && (*port == 0 || *port > math.MaxUint16) { + return errors.Newf("--%s must be between 1 and %d", flagPort, math.MaxUint16) + } + return nil + }) c.register(node, func() error { - return c.handlers.Modify.PartyNode(NodeChange{ + return c.handlers.Modify.PartyNode(change.Node{ Party: *partyID, Role: *role, Shard: *shard, @@ -399,8 +430,14 @@ func (c *CLI) registerPartyCAOp(ca *kingpin.CmdClause, op string) { signCerts := cmd.Flag(flagSignCert, "PEM path(s) for the signing-CA list (repeatable).").ExistingFiles() tlsCerts := cmd.Flag(flagTLSCert, "PEM path(s) for the TLS-CA list (repeatable).").ExistingFiles() block := blockFlag(cmd) + cmd.Validate(func(*kingpin.CmdClause) error { + if len(*signCerts) == 0 && len(*tlsCerts) == 0 { + return errors.Newf("at least one of --%s or --%s is required", flagSignCert, flagTLSCert) + } + return nil + }) c.register(cmd, func() error { - return c.handlers.Modify.PartyCA(CAChange{ + return c.handlers.Modify.PartyCA(change.CA{ Op: op, Party: *partyID, SignCerts: *signCerts, diff --git a/tools/fxadmin/core/cli/cli_test.go b/tools/fxadmin/core/cli/cli_test.go index 508fea341..ef71056af 100644 --- a/tools/fxadmin/core/cli/cli_test.go +++ b/tools/fxadmin/core/cli/cli_test.go @@ -16,18 +16,23 @@ import ( "github.com/stretchr/testify/require" "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/cli" + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify/change" ) // Repeated command names, flags, and argument tokens used across the tables. const ( - cmdLedger = "ledger" - cmdFollow = "follow" - cmdModify = "modify" - subHeight = "height" - subRemove = "remove" - subAdd = "add" - subApp = "app" - subParty = "party" + cmdLedger = "ledger" + cmdFollow = "follow" + cmdModify = "modify" + cmdComputeUpdate = "compute-update" + subHeight = "height" + subRemove = "remove" + subAdd = "add" + subApp = "app" + subParty = "party" + subNode = "node" + subCA = "ca" + subSet = "set" flagConfig = "--config" flagCurrentBlock = "--current-block" @@ -36,9 +41,16 @@ const ( flagOrg = "--org" flagParty = "--party" flagCert = "--cert" + flagRole = "--role" + flagShard = "--shard" + flagPort = "--port" + flagSignCert = "--sign-cert" + flagTLSCert = "--tls-cert" - refLatest = "latest" - orgPeer1 = "peer1" + refLatest = "latest" + orgPeer1 = "peer1" + roleBatcher = "batcher" + roleRouter = "router" fileConfigUpdate = "config_update.pb" fileEndorsement1 = "endorsed_config_update1.pb" @@ -46,6 +58,12 @@ const ( fileEndorsed = "endorsed_config_update.pb" fileConfigTx = "config_tx.pb" fileNextConfig = "next_config.pb" + + errNoCALists = "at least one of --sign-cert or --tls-cert is required" + errPortRange = "--port must be between 1 and 65535" + + handlerPartyNode = "ModifyPartyNode" + handlerPartyCA = "ModifyPartyCA" ) // call records the handler that was invoked and the values it received, so a @@ -187,21 +205,21 @@ func (f fakeModify) PartyRemove(partyID uint32, blockPath string) error { return nil } -func (f fakeModify) PartyNode(ch cli.NodeChange) error { - *f.invoked = call{handler: "ModifyPartyNode", args: []string{ +func (f fakeModify) PartyNode(ch change.Node) error { + *f.invoked = call{handler: handlerPartyNode, args: []string{ strconv.FormatUint(uint64(ch.Party), 10), ch.Role, strconv.FormatUint(uint64(ch.Shard), 10), ch.Host, strconv.FormatUint(uint64(ch.Port), 10), ch.TLSCert, ch.SignCert, ch.BlockPath, }} return nil } -func (f fakeModify) PartyCA(ch cli.CAChange) error { +func (f fakeModify) PartyCA(ch change.CA) error { args := make([]string, 0, len(ch.SignCerts)+len(ch.TLSCerts)+3) args = append(args, ch.Op, strconv.FormatUint(uint64(ch.Party), 10)) args = append(args, ch.SignCerts...) args = append(args, ch.TLSCerts...) args = append(args, ch.BlockPath) - *f.invoked = call{handler: "ModifyPartyCA", args: args} + *f.invoked = call{handler: handlerPartyCA, args: args} return nil } @@ -280,7 +298,7 @@ func TestRunRoutesToHandler(t *testing.T) { { name: "compute-update", args: []string{ - "compute-update", + cmdComputeUpdate, currentJSON, modifiedJSON, flagCurrentBlock, @@ -294,7 +312,7 @@ func TestRunRoutesToHandler(t *testing.T) { { name: "compute-update block mode", args: []string{ - "compute-update", "--pb", currBlock, nextBlock, flagOutput, fileConfigUpdate, + cmdComputeUpdate, "--pb", currBlock, nextBlock, flagOutput, fileConfigUpdate, }, wantHandler: "ComputeUpdatePB", wantArgs: []string{currBlock, nextBlock, fileConfigUpdate}, @@ -343,23 +361,58 @@ func TestRunRoutesToHandler(t *testing.T) { { name: "modify party node", args: []string{ - cmdModify, subParty, "node", flagParty, "1", "--role", "batcher", "--shard", "1", - "--port", "9014", "--tls-cert", cert1, flagBlock, nextBlock, + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleBatcher, flagShard, "1", + flagPort, "9014", flagTLSCert, cert1, flagBlock, nextBlock, }, - wantHandler: "ModifyPartyNode", + wantHandler: handlerPartyNode, // party, role, shard, host, port, tls-cert, sign-cert, block - wantArgs: []string{"1", "batcher", "1", "", "9014", cert1, "", nextBlock}, + wantArgs: []string{"1", roleBatcher, "1", "", "9014", cert1, "", nextBlock}, + }, + { + name: "modify party node batcher with explicit shard 0", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleBatcher, flagShard, "0", + flagSignCert, cert2, flagBlock, nextBlock, + }, + wantHandler: handlerPartyNode, + wantArgs: []string{"1", roleBatcher, "0", "", "0", "", cert2, nextBlock}, + }, + { + name: "modify party node non-batcher without shard", + args: []string{ + cmdModify, subParty, subNode, flagParty, "2", flagRole, roleRouter, + "--host", "router.example.com", flagBlock, nextBlock, + }, + wantHandler: handlerPartyNode, + wantArgs: []string{"2", roleRouter, "0", "router.example.com", "0", "", "", nextBlock}, + }, + { + name: "modify party node highest valid port", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleRouter, + flagPort, "65535", flagBlock, nextBlock, + }, + wantHandler: handlerPartyNode, + wantArgs: []string{"1", roleRouter, "0", "", "65535", "", "", nextBlock}, }, { name: "modify party ca add", args: []string{ - cmdModify, subParty, "ca", subAdd, flagParty, "1", - "--sign-cert", cert1, "--tls-cert", cert2, flagBlock, nextBlock, + cmdModify, subParty, subCA, subAdd, flagParty, "1", + flagSignCert, cert1, flagTLSCert, cert2, flagBlock, nextBlock, }, - wantHandler: "ModifyPartyCA", + wantHandler: handlerPartyCA, // op, party, sign-certs…, tls-certs…, block wantArgs: []string{"add", "1", cert1, cert2, nextBlock}, }, + { + name: "modify party ca set with only one list", + args: []string{ + cmdModify, subParty, subCA, subSet, flagParty, "1", flagTLSCert, cert2, flagBlock, nextBlock, + }, + wantHandler: handlerPartyCA, + wantArgs: []string{subSet, "1", cert2, nextBlock}, + }, { name: "tx endorse", args: []string{"tx", "endorse", configUpdate, flagConfig, admin, flagOutput, fileEndorsement1}, @@ -425,6 +478,8 @@ func TestParseErrors(t *testing.T) { admin := writeTempFile(t, "admin.yaml") currBlock := writeTempFile(t, "current_block.pb") + currentJSON := writeTempFile(t, "current.json") + modifiedJSON := writeTempFile(t, "modified.json") for _, tc := range []struct { name string @@ -459,6 +514,56 @@ func TestParseErrors(t *testing.T) { }, wantErr: "invalid duration", }, + { + name: "compute-update JSON mode without current-block", + args: []string{cmdComputeUpdate, currentJSON, modifiedJSON, flagOutput, fileConfigUpdate}, + wantErr: "--current-block is required in JSON mode", + }, + { + name: "modify party node batcher without shard", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleBatcher, flagBlock, currBlock, + }, + wantErr: "--shard is required for --role batcher", + }, + { + name: "modify party node shard with non-batcher role", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleRouter, + flagShard, "1", flagBlock, currBlock, + }, + wantErr: "--shard is only valid with --role batcher", + }, + { + name: "modify party node explicit port 0", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleRouter, flagPort, "0", flagBlock, currBlock, + }, + wantErr: errPortRange, + }, + { + name: "modify party node port above 65535", + args: []string{ + cmdModify, subParty, subNode, flagParty, "1", flagRole, roleRouter, + flagPort, "65536", flagBlock, currBlock, + }, + wantErr: errPortRange, + }, + { + name: "modify party ca add without cert lists", + args: []string{cmdModify, subParty, subCA, subAdd, flagParty, "1", flagBlock, currBlock}, + wantErr: errNoCALists, + }, + { + name: "modify party ca remove without cert lists", + args: []string{cmdModify, subParty, subCA, subRemove, flagParty, "1", flagBlock, currBlock}, + wantErr: errNoCALists, + }, + { + name: "modify party ca set without cert lists", + args: []string{cmdModify, subParty, subCA, subSet, flagParty, "1", flagBlock, currBlock}, + wantErr: errNoCALists, + }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() diff --git a/tools/fxadmin/core/cli/handlers.go b/tools/fxadmin/core/cli/handlers.go index 481c249ef..208209fba 100644 --- a/tools/fxadmin/core/cli/handlers.go +++ b/tools/fxadmin/core/cli/handlers.go @@ -13,6 +13,8 @@ import ( "time" "github.com/cockroachdb/errors" + + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify/change" ) // The CLI is responsible for parsing and validating command-line arguments, @@ -39,31 +41,6 @@ type UpdateHandler interface { RunFromBlocks(currentBlockPath, nextBlockPath, outputPath string) error } -// NodeChange is a `modify party node` request. It selects one node of a party -// (Role, plus Shard for a batcher) and carries the fields to change; an empty -// field is left unchanged. -type NodeChange struct { - Party uint32 - Role string - Shard uint32 - Host string - Port uint32 - TLSCert string - SignCert string - BlockPath string -} - -// CAChange is a `modify party ca add|remove|set` request over a party's CA -// (SignCerts) and TLS-CA (TLSCerts) certificate lists. Op is the sub-verb. -// An empty field is left unchanged. -type CAChange struct { - Op string - Party uint32 - SignCerts []string - TLSCerts []string - BlockPath string -} - // ModifyHandler executes the `fxadmin modify` subcommands, which apply a // structured configuration change directly to a config block file. type ModifyHandler interface { @@ -73,8 +50,8 @@ type ModifyHandler interface { AppKnownCertsRemove(org string, certPaths []string, blockPath string) error PartyAdd(partyPath, blockPath string) error PartyRemove(partyID uint32, blockPath string) error - PartyNode(change NodeChange) error - PartyCA(change CAChange) error + PartyNode(ch change.Node) error + PartyCA(ch change.CA) error } // TxHandler executes the `fxadmin tx` subcommands. diff --git a/tools/fxadmin/core/modify/change/change.go b/tools/fxadmin/core/modify/change/change.go new file mode 100644 index 000000000..d7ae7b1d9 --- /dev/null +++ b/tools/fxadmin/core/modify/change/change.go @@ -0,0 +1,33 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +// Package change defines the `fxadmin modify party` requests. +package change + +// Node is a `modify party node` request. It selects one node of a party +// (Role, plus Shard for a batcher) and carries the fields to change; an empty +// field is left unchanged. +type Node struct { + Party uint32 + Role string + Shard uint32 + Host string + Port uint32 + TLSCert string + SignCert string + BlockPath string +} + +// CA is a `modify party ca add|remove|set` request over a party's CA +// (SignCerts) and TLS-CA (TLSCerts) certificate lists. Op is the sub-verb. +// An empty field is left unchanged. +type CA struct { + Op string + Party uint32 + SignCerts []string + TLSCerts []string + BlockPath string +} diff --git a/tools/fxadmin/core/modify/modify.go b/tools/fxadmin/core/modify/modify.go index 7363739c7..353790d57 100644 --- a/tools/fxadmin/core/modify/modify.go +++ b/tools/fxadmin/core/modify/modify.go @@ -19,7 +19,7 @@ import ( "github.com/cockroachdb/errors" "github.com/hyperledger/fabric-lib-go/common/flogging" - "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/cli" + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify/change" ) var logger = flogging.MustGetLogger("fxadmin.modify") @@ -73,13 +73,13 @@ func (*Handler) PartyRemove(partyID uint32, blockPath string) error { } // PartyNode implements `fxadmin modify party node`. -func (*Handler) PartyNode(change cli.NodeChange) error { - logger.Debugf("modify party node: %+v (not implemented)", change) +func (*Handler) PartyNode(ch change.Node) error { + logger.Debugf("modify party node: %+v (not implemented)", ch) return ErrNotImplemented } // PartyCA implements `fxadmin modify party ca add|remove|set`. -func (*Handler) PartyCA(change cli.CAChange) error { - logger.Debugf("modify party ca: %+v (not implemented)", change) +func (*Handler) PartyCA(ch change.CA) error { + logger.Debugf("modify party ca: %+v (not implemented)", ch) return ErrNotImplemented } diff --git a/tools/fxadmin/core/modify/modify_test.go b/tools/fxadmin/core/modify/modify_test.go index 2944bb5de..e7e5daa2b 100644 --- a/tools/fxadmin/core/modify/modify_test.go +++ b/tools/fxadmin/core/modify/modify_test.go @@ -11,8 +11,8 @@ import ( "github.com/stretchr/testify/require" - "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/cli" "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify" + "github.com/hyperledger/fabric-x-common/tools/fxadmin/core/modify/change" ) // TestHandlerNotImplemented asserts every modify command currently returns @@ -28,6 +28,6 @@ func TestHandlerNotImplemented(t *testing.T) { require.ErrorIs(t, h.AppKnownCertsRemove("peer1", []string{"c.pem"}, "next.pb"), modify.ErrNotImplemented) require.ErrorIs(t, h.PartyAdd("party.yaml", "next.pb"), modify.ErrNotImplemented) require.ErrorIs(t, h.PartyRemove(5, "next.pb"), modify.ErrNotImplemented) - require.ErrorIs(t, h.PartyNode(cli.NodeChange{Party: 1, Role: "batcher"}), modify.ErrNotImplemented) - require.ErrorIs(t, h.PartyCA(cli.CAChange{Op: "add", Party: 1}), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyNode(change.Node{Party: 1, Role: "batcher"}), modify.ErrNotImplemented) + require.ErrorIs(t, h.PartyCA(change.CA{Op: "add", Party: 1}), modify.ErrNotImplemented) }