From de7aac1f25dcbd906f205e22ee830e9093fe3711 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Mon, 23 Sep 2024 20:50:45 +0200 Subject: [PATCH] Refactor command-line parsing, pass --network to most commands --- go/cmd/entrypoint/client.go | 2 +- go/cmd/entrypoint/daemon.go | 17 ++-- go/cmd/entrypoint/daemon_util.go | 9 ++ go/cmd/entrypoint/garage.go | 27 +++--- go/cmd/entrypoint/host.go | 29 +++--- go/cmd/entrypoint/main.go | 2 +- go/cmd/entrypoint/nebula.go | 21 ++--- go/cmd/entrypoint/network.go | 36 ++++---- go/cmd/entrypoint/sub_cmd.go | 147 ++++++++++++++++++------------- go/cmd/entrypoint/version.go | 5 +- 10 files changed, 167 insertions(+), 128 deletions(-) diff --git a/go/cmd/entrypoint/client.go b/go/cmd/entrypoint/client.go index 6787ac3..249226c 100644 --- a/go/cmd/entrypoint/client.go +++ b/go/cmd/entrypoint/client.go @@ -6,7 +6,7 @@ import ( ) func (ctx subCmdCtx) getHosts() ([]bootstrap.Host, error) { - res, err := ctx.daemonRPC.GetHosts(ctx) + res, err := newDaemonRPCClient().GetHosts(ctx) if err != nil { return nil, fmt.Errorf("calling GetHosts: %w", err) } diff --git a/go/cmd/entrypoint/daemon.go b/go/cmd/entrypoint/daemon.go index 6edeed5..a8407c2 100644 --- a/go/cmd/entrypoint/daemon.go +++ b/go/cmd/entrypoint/daemon.go @@ -16,28 +16,27 @@ import ( // restart the service into a configuration that will definitely fail. var subCmdDaemon = subCmd{ - name: "daemon", - descr: "Runs the isle daemon (Default if no sub-command given)", + name: "daemon", + descr: "Runs the isle daemon (Default if no sub-command given)", + noNetwork: true, do: func(ctx subCmdCtx) error { - - flags := ctx.flagSet(false) - - daemonConfigPath := flags.StringP( + daemonConfigPath := ctx.flags.StringP( "config-path", "c", "", "Optional path to a daemon.yml file to load configuration from.", ) - dumpConfig := flags.Bool( + dumpConfig := ctx.flags.Bool( "dump-config", false, "Write the default configuration file to stdout and exit.", ) - logLevelStr := flags.StringP( + logLevelStr := ctx.flags.StringP( "log-level", "l", "info", `Maximum log level which should be output. Values can be "debug", "info", "warn", "error", "fatal". Does not apply to sub-processes`, ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } diff --git a/go/cmd/entrypoint/daemon_util.go b/go/cmd/entrypoint/daemon_util.go index 3f415b5..d6ddaca 100644 --- a/go/cmd/entrypoint/daemon_util.go +++ b/go/cmd/entrypoint/daemon_util.go @@ -6,6 +6,7 @@ import ( "fmt" "io/fs" "isle/daemon" + "isle/daemon/jsonrpc2" "net" "net/http" "os" @@ -16,6 +17,14 @@ import ( const daemonHTTPRPCPath = "/rpc/v0.json" +func newDaemonRPCClient() daemon.RPC { + return daemon.RPCFromClient( + jsonrpc2.NewUnixHTTPClient( + daemon.HTTPSocketPath(), daemonHTTPRPCPath, + ), + ) +} + func newHTTPServer( ctx context.Context, logger *mlog.Logger, daemonInst *daemon.Daemon, ) ( diff --git a/go/cmd/entrypoint/garage.go b/go/cmd/entrypoint/garage.go index a6fac18..25e84b6 100644 --- a/go/cmd/entrypoint/garage.go +++ b/go/cmd/entrypoint/garage.go @@ -32,27 +32,26 @@ func initMCConfigDir(envVars daecommon.EnvVars) (string, error) { } var subCmdGarageMC = subCmd{ - name: "mc", - descr: "Runs the mc (minio-client) binary. The isle garage can be accessed under the `garage` alias", + name: "mc", + descr: "Runs the mc (minio-client) binary. The isle garage can be accessed under the `garage` alias", + passthroughArgs: true, do: func(ctx subCmdCtx) error { - - flags := ctx.flagSet(true) - - keyID := flags.StringP( + keyID := ctx.flags.StringP( "key-id", "i", "", "Optional key ID to use, defaults to that of the shared global key", ) - keySecret := flags.StringP( + keySecret := ctx.flags.StringP( "key-secret", "s", "", "Optional key secret to use, defaults to that of the shared global key", ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } - clientParams, err := ctx.daemonRPC.GetGarageClientParams(ctx) + clientParams, err := newDaemonRPCClient().GetGarageClientParams(ctx) if err != nil { return fmt.Errorf("calling GetGarageClientParams: %w", err) } @@ -67,9 +66,9 @@ var subCmdGarageMC = subCmd{ *keySecret = clientParams.GlobalBucketS3APICredentials.Secret } - args := flags.Args() + args := ctx.flags.Args() - if i := flags.ArgsLenAtDash(); i >= 0 { + if i := ctx.flags.ArgsLenAtDash(); i >= 0 { args = args[i:] } @@ -117,8 +116,12 @@ var subCmdGarageCLI = subCmd{ name: "cli", descr: "Runs the garage binary, automatically configured to point to the garage sub-process of a running isle daemon", do: func(ctx subCmdCtx) error { + ctx, err := ctx.withParsedFlags() + if err != nil { + return fmt.Errorf("parsing flags: %w", err) + } - clientParams, err := ctx.daemonRPC.GetGarageClientParams(ctx) + clientParams, err := newDaemonRPCClient().GetGarageClientParams(ctx) if err != nil { return fmt.Errorf("calling GetGarageClientParams: %w", err) } diff --git a/go/cmd/entrypoint/host.go b/go/cmd/entrypoint/host.go index 6ade1c2..e2fb3df 100644 --- a/go/cmd/entrypoint/host.go +++ b/go/cmd/entrypoint/host.go @@ -16,26 +16,26 @@ var subCmdHostCreate = subCmd{ descr: "Creates a new host in the network, writing its new bootstrap.json to stdout", do: func(ctx subCmdCtx) error { var ( - flags = ctx.flagSet(false) hostName hostNameFlag ip ipFlag ) - hostNameF := flags.VarPF( + hostNameF := ctx.flags.VarPF( &hostName, "hostname", "n", "Name of the host to generate bootstrap.json for", ) - flags.VarP(&ip, "ip", "i", "IP of the new host. An available IP will be chosen if none is given.") + ctx.flags.VarP(&ip, "ip", "i", "IP of the new host. An available IP will be chosen if none is given.") - canCreateHosts := flags.Bool( + canCreateHosts := ctx.flags.Bool( "can-create-hosts", false, "The new host should have the ability to create hosts too", ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -43,11 +43,7 @@ var subCmdHostCreate = subCmd{ return errors.New("--hostname is required") } - var ( - res network.JoiningBootstrap - err error - ) - res, err = ctx.daemonRPC.CreateHost( + res, err := newDaemonRPCClient().CreateHost( ctx, hostName.V, network.CreateHostOpts{ IP: ip.V, CanCreateHosts: *canCreateHosts, @@ -65,6 +61,11 @@ var subCmdHostList = subCmd{ name: "list", descr: "Lists all hosts in the network, and their IPs", do: func(ctx subCmdCtx) error { + ctx, err := ctx.withParsedFlags() + if err != nil { + return fmt.Errorf("parsing flags: %w", err) + } + hostsRes, err := ctx.getHosts() if err != nil { return fmt.Errorf("calling GetHosts: %w", err) @@ -102,17 +103,17 @@ var subCmdHostRemove = subCmd{ descr: "Removes a host from the network", do: func(ctx subCmdCtx) error { var ( - flags = ctx.flagSet(false) hostName hostNameFlag ) - hostNameF := flags.VarPF( + hostNameF := ctx.flags.VarPF( &hostName, "hostname", "n", "Name of the host to remove", ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -120,7 +121,7 @@ var subCmdHostRemove = subCmd{ return errors.New("--hostname is required") } - if err := ctx.daemonRPC.RemoveHost(ctx, hostName.V); err != nil { + if err := newDaemonRPCClient().RemoveHost(ctx, hostName.V); err != nil { return fmt.Errorf("calling RemoveHost: %w", err) } diff --git a/go/cmd/entrypoint/main.go b/go/cmd/entrypoint/main.go index 89b06ef..4512b0c 100644 --- a/go/cmd/entrypoint/main.go +++ b/go/cmd/entrypoint/main.go @@ -56,8 +56,8 @@ func main() { err := subCmdCtx{ Context: ctx, - args: os.Args[1:], logger: logger, + args: os.Args[1:], }.doSubCmd( subCmdDaemon, subCmdGarage, diff --git a/go/cmd/entrypoint/nebula.go b/go/cmd/entrypoint/nebula.go index 30df076..5138181 100644 --- a/go/cmd/entrypoint/nebula.go +++ b/go/cmd/entrypoint/nebula.go @@ -12,23 +12,21 @@ var subCmdNebulaCreateCert = subCmd{ name: "create-cert", descr: "Creates a signed nebula certificate file for an existing host and writes it to stdout", do: func(ctx subCmdCtx) error { - var ( - flags = ctx.flagSet(false) - hostName hostNameFlag - ) - hostNameF := flags.VarPF( + var hostName hostNameFlag + hostNameF := ctx.flags.VarPF( &hostName, "hostname", "n", "Name of the host to generate a certificate for", ) - pubKeyPath := flags.StringP( + pubKeyPath := ctx.flags.StringP( "public-key-path", "p", "", `Path to PEM file containing public key which will be embedded in the cert.`, ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -46,7 +44,7 @@ var subCmdNebulaCreateCert = subCmd{ return fmt.Errorf("unmarshaling public key as PEM: %w", err) } - res, err := ctx.daemonRPC.CreateNebulaCertificate( + res, err := newDaemonRPCClient().CreateNebulaCertificate( ctx, hostName.V, hostPub, ) if err != nil { @@ -70,9 +68,8 @@ var subCmdNebulaShow = subCmd{ name: "show", descr: "Writes nebula network information to stdout in JSON format", do: func(ctx subCmdCtx) error { - - flags := ctx.flagSet(false) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -81,7 +78,7 @@ var subCmdNebulaShow = subCmd{ return fmt.Errorf("getting hosts: %w", err) } - caPublicCreds, err := ctx.daemonRPC.GetNebulaCAPublicCredentials(ctx) + caPublicCreds, err := newDaemonRPCClient().GetNebulaCAPublicCredentials(ctx) if err != nil { return fmt.Errorf("calling GetNebulaCAPublicCredentials: %w", err) } diff --git a/go/cmd/entrypoint/network.go b/go/cmd/entrypoint/network.go index 37e072e..8d43673 100644 --- a/go/cmd/entrypoint/network.go +++ b/go/cmd/entrypoint/network.go @@ -8,39 +8,40 @@ import ( ) var subCmdNetworkCreate = subCmd{ - name: "create", - descr: "Create's a new network, with this host being the first host in that network.", + name: "create", + descr: "Create's a new network, with this host being the first host in that network.", + noNetwork: true, do: func(ctx subCmdCtx) error { var ( - flags = ctx.flagSet(false) ipNet ipNetFlag hostName hostNameFlag ) - name := flags.StringP( + name := ctx.flags.StringP( "name", "N", "", "Human-readable name to identify the network as.", ) - domain := flags.StringP( + domain := ctx.flags.StringP( "domain", "d", "", "Domain name that should be used as the root domain in the network.", ) - ipNetF := flags.VarPF( + ipNetF := ctx.flags.VarPF( &ipNet, "ip-net", "i", `An IP subnet, in CIDR form, which will be the overall range of`+ ` possible IPs in the network. The first IP in this network`+ ` range will become this first host's IP.`, ) - hostNameF := flags.VarPF( + hostNameF := ctx.flags.VarPF( &hostName, "hostname", "n", "Name of this host, which will be the first host in the network", ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -51,7 +52,7 @@ var subCmdNetworkCreate = subCmd{ return errors.New("--name, --domain, --ip-net, and --hostname are required") } - err := ctx.daemonRPC.CreateNetwork( + err = newDaemonRPCClient().CreateNetwork( ctx, *name, *domain, ipNet.V, hostName.V, ) if err != nil { @@ -63,17 +64,16 @@ var subCmdNetworkCreate = subCmd{ } var subCmdNetworkJoin = subCmd{ - name: "join", - descr: "Joins this host to an existing network", + name: "join", + descr: "Joins this host to an existing network", + noNetwork: true, do: func(ctx subCmdCtx) error { - var ( - flags = ctx.flagSet(false) - bootstrapPath = flags.StringP( - "bootstrap-path", "b", "", "Path to a bootstrap.json file.", - ) + bootstrapPath := ctx.flags.StringP( + "bootstrap-path", "b", "", "Path to a bootstrap.json file.", ) - if err := flags.Parse(ctx.args); err != nil { + ctx, err := ctx.withParsedFlags() + if err != nil { return fmt.Errorf("parsing flags: %w", err) } @@ -88,7 +88,7 @@ var subCmdNetworkJoin = subCmd{ ) } - return ctx.daemonRPC.JoinNetwork(ctx, newBootstrap) + return newDaemonRPCClient().JoinNetwork(ctx, newBootstrap) }, } diff --git a/go/cmd/entrypoint/sub_cmd.go b/go/cmd/entrypoint/sub_cmd.go index 415fe90..c8f50db 100644 --- a/go/cmd/entrypoint/sub_cmd.go +++ b/go/cmd/entrypoint/sub_cmd.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "isle/daemon" - "isle/daemon/jsonrpc2" "os" "strings" @@ -14,30 +13,8 @@ import ( type flagSet struct { *pflag.FlagSet -} -func (fs flagSet) Parse(args []string) error { - fs.VisitAll(func(f *pflag.Flag) { - if f.Shorthand == "h" { - panic(fmt.Sprintf("flag %+v has reserved shorthand `-h`", f)) - } - if f.Name == "help" { - panic(fmt.Sprintf("flag %+v has reserved name `--help`", f)) - } - }) - return fs.FlagSet.Parse(args) -} - -// subCmdCtx contains all information available to a subCmd's do method. -type subCmdCtx struct { - context.Context - - subCmd subCmd // the subCmd itself - args []string // command-line arguments, excluding the subCmd itself. - subCmdNames []string // names of subCmds so far, including this one - - logger *mlog.Logger - daemonRPC daemon.RPC + network string } type subCmd struct { @@ -47,11 +24,74 @@ type subCmd struct { // If set then the name will be allowed to be suffixed with this string. plural string + + // noNetwork, if true, means the call doesn't require a network to be + // specified on the command-line if there are more than one networks + // configured. + noNetwork bool + + // Extra arguments on the command-line will be passed through to some + // underlying command. + passthroughArgs bool } -func (ctx subCmdCtx) usagePrefix() string { +// subCmdCtx contains all information available to a subCmd's do method. +type subCmdCtx struct { + context.Context + logger *mlog.Logger - subCmdNamesStr := strings.Join(ctx.subCmdNames, " ") + subCmd subCmd // the subCmd itself + args []string // command-line arguments, excluding the subCmd itself. + subCmdNames []string // names of subCmds so far, including this one + + flags flagSet +} + +func newSubCmdCtx( + ctx context.Context, + logger *mlog.Logger, + subCmd subCmd, + args []string, + subCmdNames []string, +) subCmdCtx { + flags := pflag.NewFlagSet(subCmd.name, pflag.ExitOnError) + flags.Usage = func() { + var passthroughStr string + if subCmd.passthroughArgs { + passthroughStr = " [--] [args...]" + } + + fmt.Fprintf( + os.Stderr, "%s[-h|--help] [%s flags...]%s\n\n", + usagePrefix(subCmdNames), subCmd.name, passthroughStr, + ) + fmt.Fprintf(os.Stderr, "%s FLAGS:\n\n", strings.ToUpper(subCmd.name)) + fmt.Fprintln(os.Stderr, flags.FlagUsages()) + + os.Stderr.Sync() + os.Exit(2) + } + + fs := flagSet{FlagSet: flags} + + if !subCmd.noNetwork { + fs.FlagSet.StringVar( + &fs.network, "network", "", "Which network to perform the command against, if more than one is joined. Can be ID, name, or domain", + ) + } + + return subCmdCtx{ + Context: ctx, + logger: logger, + subCmd: subCmd, + args: args, + subCmdNames: subCmdNames, + flags: fs, + } +} + +func usagePrefix(subCmdNames []string) string { + subCmdNamesStr := strings.Join(subCmdNames, " ") if subCmdNamesStr != "" { subCmdNamesStr += " " } @@ -59,26 +99,22 @@ func (ctx subCmdCtx) usagePrefix() string { return fmt.Sprintf("\nUSAGE: %s %s", os.Args[0], subCmdNamesStr) } -func (ctx subCmdCtx) flagSet(withPassthrough bool) flagSet { - flags := pflag.NewFlagSet(ctx.subCmd.name, pflag.ExitOnError) - flags.Usage = func() { - - var passthroughStr string - if withPassthrough { - passthroughStr = " [--] [args...]" +func (ctx subCmdCtx) withParsedFlags() (subCmdCtx, error) { + ctx.flags.VisitAll(func(f *pflag.Flag) { + if f.Shorthand == "h" { + panic(fmt.Sprintf("flag %+v has reserved shorthand `-h`", f)) } + if f.Name == "help" { + panic(fmt.Sprintf("flag %+v has reserved name `--help`", f)) + } + }) - fmt.Fprintf( - os.Stderr, "%s[-h|--help] [%s flags...]%s\n\n", - ctx.usagePrefix(), ctx.subCmd.name, passthroughStr, - ) - fmt.Fprintf(os.Stderr, "%s FLAGS:\n\n", strings.ToUpper(ctx.subCmd.name)) - fmt.Fprintln(os.Stderr, flags.FlagUsages()) - - os.Stderr.Sync() - os.Exit(2) + if err := ctx.flags.Parse(ctx.args); err != nil { + return ctx, err } - return flagSet{flags} + + ctx.Context = daemon.WithNetwork(ctx.Context, ctx.flags.network) + return ctx, nil } func (ctx subCmdCtx) doSubCmd(subCmds ...subCmd) error { @@ -90,7 +126,7 @@ func (ctx subCmdCtx) doSubCmd(subCmds ...subCmd) error { fmt.Fprintf( os.Stderr, "%s [-h|--help] [sub-command flags...]\n", - ctx.usagePrefix(), + usagePrefix(ctx.subCmdNames), ) fmt.Fprintf(os.Stderr, "\nSUB-COMMANDS:\n\n") @@ -123,28 +159,21 @@ func (ctx subCmdCtx) doSubCmd(subCmds ...subCmd) error { } subCmdName, args := args[0], args[1:] - subCmd, ok := subCmdsMap[subCmdName] + subCmd, ok := subCmdsMap[subCmdName] if !ok { printUsageExit(subCmdName) } - daemonRPC := daemon.RPCFromClient( - jsonrpc2.NewUnixHTTPClient( - daemon.HTTPSocketPath(), daemonHTTPRPCPath, - ), + nextSubCmdCtx := newSubCmdCtx( + ctx.Context, + ctx.logger, + subCmd, + args, + append(ctx.subCmdNames, subCmdName), ) - err := subCmd.do(subCmdCtx{ - Context: ctx.Context, - subCmd: subCmd, - args: args, - subCmdNames: append(ctx.subCmdNames, subCmdName), - logger: ctx.logger, - daemonRPC: daemonRPC, - }) - - if err != nil { + if err := subCmd.do(nextSubCmdCtx); err != nil { return err } diff --git a/go/cmd/entrypoint/version.go b/go/cmd/entrypoint/version.go index 0bc4a09..a046da4 100644 --- a/go/cmd/entrypoint/version.go +++ b/go/cmd/entrypoint/version.go @@ -7,8 +7,9 @@ import ( ) var subCmdVersion = subCmd{ - name: "version", - descr: "Dumps version and build info to stdout", + name: "version", + descr: "Dumps version and build info to stdout", + noNetwork: true, do: func(ctx subCmdCtx) error { versionPath := filepath.Join(envAppDirPath, "share/version")