refactor: replace global slices with functions (#94)

I started to see strange behavior where flags would merge across
subcommands. I'm not sure if the bug was in our 'append' usage or
in urfave/cli's handling of flag-slices, but this change seems to
fix the problem either way.
This commit is contained in:
Daniel Moran 2021-05-14 16:10:07 -04:00 committed by GitHub
parent 9fe4197625
commit 8e73906437
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 289 additions and 267 deletions

View File

@ -18,7 +18,6 @@ func newBucketCmd() *cli.Command {
newBucketUpdateCmd(),
},
}
}
func newBucketCreateCmd() *cli.Command {
@ -33,7 +32,7 @@ func newBucketCreateCmd() *cli.Command {
withApi(true),
),
Flags: append(
commonFlags,
commonFlags(),
&cli.StringFlag{
Name: "name",
Usage: "New bucket name",
@ -100,7 +99,7 @@ func newBucketDeleteCmd() *cli.Command {
Usage: "Delete bucket",
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(
commonFlags,
commonFlags(),
&cli.StringFlag{
Name: "id",
Usage: "The bucket ID, required if name isn't provided",
@ -147,7 +146,7 @@ func newBucketListCmd() *cli.Command {
Aliases: []string{"find", "ls"},
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(
commonFlags,
commonFlags(),
&cli.StringFlag{
Name: "id",
Usage: "The bucket ID, required if name isn't provided",
@ -194,7 +193,7 @@ func newBucketUpdateCmd() *cli.Command {
Aliases: []string{"find", "ls"},
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(
commonFlags,
commonFlags(),
&cli.StringFlag{
Name: "name",
Usage: "New name to set on the bucket",

View File

@ -56,7 +56,7 @@ func newBucketSchemaCreateCmd() *cli.Command {
Usage: "Create a measurement schema for a bucket",
Before: withBucketSchemaClient(),
Flags: append(
commonFlags,
commonFlags(),
append(
getOrgBucketFlags(&params.OrgBucketParams),
&cli.StringFlag{
@ -111,7 +111,7 @@ func newBucketSchemaUpdateCmd() *cli.Command {
Usage: "Update a measurement schema for a bucket",
Before: withBucketSchemaClient(),
Flags: append(
commonFlags,
commonFlags(),
append(
getOrgBucketFlags(&params.OrgBucketParams),
&cli.GenericFlag{
@ -165,7 +165,7 @@ func newBucketSchemaListCmd() *cli.Command {
Usage: "List schemas for a bucket",
Before: withBucketSchemaClient(),
Flags: append(
commonFlags,
commonFlags(),
append(
getOrgBucketFlags(&params.OrgBucketParams),
&cli.StringFlag{

View File

@ -10,7 +10,7 @@ import (
"github.com/urfave/cli/v2"
)
var configPathAndPrintFlags = append([]cli.Flag{&configPathFlag}, printFlags...)
var configPathAndPrintFlags = append([]cli.Flag{configPathFlag()}, printFlags()...)
func newConfigCmd() *cli.Command {
return &cli.Command{
@ -151,7 +151,7 @@ and
https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/rm/
`,
Before: withCli(),
Flags: append([]cli.Flag{&configPathFlag}, printFlags...),
Flags: append([]cli.Flag{configPathFlag()}, printFlags()...),
Action: func(ctx *cli.Context) error {
client := config.Client{CLI: getCLI(ctx)}
return client.Delete(ctx.Args().Slice())

275
cmd/influx/global.go Normal file
View File

@ -0,0 +1,275 @@
package main
import (
"crypto/tls"
"fmt"
"net/http"
"net/url"
"runtime"
"github.com/influxdata/influx-cli/v2/internal/api"
"github.com/influxdata/influx-cli/v2/internal/cmd"
"github.com/influxdata/influx-cli/v2/internal/config"
"github.com/influxdata/influx-cli/v2/internal/stdio"
"github.com/influxdata/influx-cli/v2/pkg/cli/middleware"
"github.com/urfave/cli/v2"
)
const (
tokenFlagName = "token"
hostFlagName = "host"
skipVerifyFlagName = "skip-verify"
traceIdFlagName = "trace-debug-id"
configPathFlagName = "configs-path"
configNameFlagName = "active-config"
httpDebugFlagName = "http-debug"
printJsonFlagName = "json"
hideHeadersFlagName = "hide-headers"
)
// newCli builds a CLI core that reads from stdin, writes to stdout/stderr, manages a local config store,
// and optionally tracks a trace ID specified over the CLI.
func newCli(ctx *cli.Context) (cmd.CLI, error) {
configPath := ctx.String(configPathFlagName)
var err error
if configPath == "" {
configPath, err = config.DefaultPath()
if err != nil {
return cmd.CLI{}, err
}
}
configSvc := config.NewLocalConfigService(configPath)
var activeConfig config.Config
if ctx.IsSet(configNameFlagName) {
if activeConfig, err = configSvc.SwitchActive(ctx.String(configNameFlagName)); err != nil {
return cmd.CLI{}, err
}
} else if activeConfig, err = configSvc.Active(); err != nil {
return cmd.CLI{}, err
}
return cmd.CLI{
StdIO: stdio.TerminalStdio,
PrintAsJSON: ctx.Bool(printJsonFlagName),
HideTableHeaders: ctx.Bool(hideHeadersFlagName),
ActiveConfig: activeConfig,
ConfigService: configSvc,
}, nil
}
// newApiClient returns an API client configured to communicate with a remote InfluxDB instance over HTTP.
// Client parameters are pulled from the CLI context.
func newApiClient(ctx *cli.Context, configSvc config.Service, injectToken bool) (*api.APIClient, error) {
cfg, err := configSvc.Active()
if err != nil {
return nil, err
}
if ctx.IsSet(tokenFlagName) {
cfg.Token = ctx.String(tokenFlagName)
}
if ctx.IsSet(hostFlagName) {
cfg.Host = ctx.String(hostFlagName)
}
parsedHost, err := url.Parse(cfg.Host)
if err != nil {
return nil, fmt.Errorf("host URL %q is invalid: %w", cfg.Host, err)
}
clientTransport := http.DefaultTransport.(*http.Transport)
clientTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: ctx.Bool(skipVerifyFlagName)}
apiConfig := api.NewConfiguration()
apiConfig.Host = parsedHost.Host
apiConfig.Scheme = parsedHost.Scheme
apiConfig.UserAgent = fmt.Sprintf("influx/%s (%s) Sha/%s Date/%s", version, runtime.GOOS, commit, date)
apiConfig.HTTPClient = &http.Client{Transport: clientTransport}
if injectToken {
apiConfig.DefaultHeader["Authorization"] = fmt.Sprintf("Token %s", cfg.Token)
}
if ctx.IsSet(traceIdFlagName) {
// NOTE: This is circumventing our codegen. If the header we use for tracing ever changes,
// we'll need to manually update the string here to match.
//
// The alternative is to pass the trace ID to the business logic for every CLI command, and
// use codegen'd logic to set the header on every HTTP request. Early versions of the CLI
// used that technique, and we found it to be error-prone and easy to forget during testing.
apiConfig.DefaultHeader["Zap-Trace-Span"] = ctx.String(traceIdFlagName)
}
apiConfig.Debug = ctx.Bool(httpDebugFlagName)
return api.NewAPIClient(apiConfig), nil
}
func withCli() cli.BeforeFunc {
return func(ctx *cli.Context) error {
c, err := newCli(ctx)
if err != nil {
return err
}
ctx.App.Metadata["cli"] = c
return nil
}
}
func getCLI(ctx *cli.Context) cmd.CLI {
i, ok := ctx.App.Metadata["cli"].(cmd.CLI)
if !ok {
panic("missing CLI")
}
return i
}
func withApi(injectToken bool) cli.BeforeFunc {
key := "api-no-token"
if injectToken {
key = "api"
}
makeFn := func(ctx *cli.Context) error {
c := getCLI(ctx)
apiClient, err := newApiClient(ctx, c.ConfigService, injectToken)
if err != nil {
return err
}
ctx.App.Metadata[key] = apiClient
return nil
}
return middleware.WithBeforeFns(makeFn)
}
func getAPI(ctx *cli.Context) *api.APIClient {
i, ok := ctx.App.Metadata["api"].(*api.APIClient)
if !ok {
panic("missing APIClient with token")
}
return i
}
func getAPINoToken(ctx *cli.Context) *api.APIClient {
i, ok := ctx.App.Metadata["api-no-token"].(*api.APIClient)
if !ok {
panic("missing APIClient without token")
}
return i
}
// NOTE: urfave/cli has dedicated support for global flags, but it only parses those flags
// if they're specified before any command names. This is incompatible with the old influx
// CLI, which repeatedly registered common flags on every "leaf" command, forcing the flags
// to be specified after _all_ command names were given.
//
// We replicate the pattern from the old CLI so existing scripts and docs stay valid.
// configPathFlag returns the flag used by commands that access the CLI's local config store.
func configPathFlag() cli.Flag {
return &cli.PathFlag{
Name: configPathFlagName,
Usage: "Path to the influx CLI configurations",
EnvVars: []string{"INFLUX_CLI_CONFIGS_PATH"},
}
}
// coreFlags returns flags used by all CLI commands that make HTTP requests.
func coreFlags() []cli.Flag {
return []cli.Flag{
&cli.StringFlag{
Name: hostFlagName,
Usage: "HTTP address of InfluxDB",
EnvVars: []string{"INFLUX_HOST"},
},
&cli.BoolFlag{
Name: skipVerifyFlagName,
Usage: "Skip TLS certificate chain and host name verification",
},
configPathFlag(),
&cli.StringFlag{
Name: configNameFlagName,
Usage: "Config name to use for command",
Aliases: []string{"c"},
EnvVars: []string{"INFLUX_ACTIVE_CONFIG"},
},
&cli.StringFlag{
Name: traceIdFlagName,
Hidden: true,
EnvVars: []string{"INFLUX_TRACE_DEBUG_ID"},
},
&cli.BoolFlag{
Name: httpDebugFlagName,
Hidden: true,
},
}
}
// printFlags returns flags used by commands that display API resources to the user.
func printFlags() []cli.Flag {
return []cli.Flag{
&cli.BoolFlag{
Name: printJsonFlagName,
Usage: "Output data as JSON",
EnvVars: []string{"INFLUX_OUTPUT_JSON"},
},
&cli.BoolFlag{
Name: hideHeadersFlagName,
Usage: "Hide the table headers in output data",
EnvVars: []string{"INFLUX_HIDE_HEADERS"},
},
}
}
// commonTokenFlag returns the flag used by commands that hit an authenticated API.
func commonTokenFlag() cli.Flag {
return &cli.StringFlag{
Name: tokenFlagName,
Usage: "Authentication token",
Aliases: []string{"t"},
EnvVars: []string{"INFLUX_TOKEN"},
}
}
// commonFlagsNoToken returns flags used by commands that display API resources to the user, but don't need auth.
func commonFlagsNoToken() []cli.Flag {
return append(coreFlags(), printFlags()...)
}
// commonFlagsNoPrint returns flags used by commands that need auth, but don't display API resources to the user.
func commonFlagsNoPrint() []cli.Flag {
return append(coreFlags(), commonTokenFlag())
}
// commonFlags returns flags used by commands that need auth and display API resources to the user.
func commonFlags() []cli.Flag {
return append(commonFlagsNoToken(), commonTokenFlag())
}
// getOrgBucketFlags returns flags used by commands that are scoped to a single org/bucket, binding
// the flags to the given params container.
func getOrgBucketFlags(c *cmd.OrgBucketParams) []cli.Flag {
return []cli.Flag{
&cli.GenericFlag{
Name: "bucket-id",
Usage: "The bucket ID, required if name isn't provided",
Aliases: []string{"i"},
Value: &c.BucketID,
},
&cli.StringFlag{
Name: "bucket",
Usage: "The bucket name, org or org-id will be required by choosing this",
Aliases: []string{"n"},
Destination: &c.BucketName,
},
&cli.GenericFlag{
Name: "org-id",
Usage: "The ID of the organization",
EnvVars: []string{"INFLUX_ORG_ID"},
Value: &c.OrgID,
},
&cli.StringFlag{
Name: "org",
Usage: "The name of the organization",
Aliases: []string{"o"},
EnvVars: []string{"INFLUX_ORG"},
Destination: &c.OrgName,
},
}
}

View File

@ -2,19 +2,10 @@ package main
import (
"context"
"crypto/tls"
"fmt"
"net/http"
"net/url"
"os"
"runtime"
"time"
"github.com/influxdata/influx-cli/v2/internal/api"
"github.com/influxdata/influx-cli/v2/internal/cmd"
"github.com/influxdata/influx-cli/v2/internal/config"
"github.com/influxdata/influx-cli/v2/internal/stdio"
"github.com/influxdata/influx-cli/v2/pkg/cli/middleware"
"github.com/influxdata/influx-cli/v2/pkg/signals"
"github.com/urfave/cli/v2"
)
@ -34,160 +25,6 @@ func init() {
cli.VersionFlag = nil
}
var (
tokenFlagName = "token"
hostFlagName = "host"
skipVerifyFlagName = "skip-verify"
traceIdFlagName = "trace-debug-id"
configPathFlagName = "configs-path"
configNameFlagName = "active-config"
httpDebugFlagName = "http-debug"
printJsonFlagName = "json"
hideHeadersFlagName = "hide-headers"
)
// NOTE: urfave/cli has dedicated support for global flags, but it only parses those flags
// if they're specified before any command names. This is incompatible with the old influx
// CLI, which repeatedly registered common flags on every "leaf" command, forcing the flags
// to be specified after _all_ command names were given.
//
// We replicate the pattern from the old CLI so existing scripts and docs stay valid.
var configPathFlag = cli.PathFlag{
Name: configPathFlagName,
Usage: "Path to the influx CLI configurations",
EnvVars: []string{"INFLUX_CLI_CONFIGS_PATH"},
}
// Flags used by all CLI commands that make HTTP requests.
var coreFlags = []cli.Flag{
&cli.StringFlag{
Name: hostFlagName,
Usage: "HTTP address of InfluxDB",
EnvVars: []string{"INFLUX_HOST"},
},
&cli.BoolFlag{
Name: skipVerifyFlagName,
Usage: "Skip TLS certificate chain and host name verification",
},
&configPathFlag,
&cli.StringFlag{
Name: configNameFlagName,
Usage: "Config name to use for command",
Aliases: []string{"c"},
EnvVars: []string{"INFLUX_ACTIVE_CONFIG"},
},
&cli.StringFlag{
Name: traceIdFlagName,
Hidden: true,
EnvVars: []string{"INFLUX_TRACE_DEBUG_ID"},
},
&cli.BoolFlag{
Name: httpDebugFlagName,
Hidden: true,
},
}
// Flags used by commands that display API resources to the user.
var printFlags = []cli.Flag{
&cli.BoolFlag{
Name: printJsonFlagName,
Usage: "Output data as JSON",
EnvVars: []string{"INFLUX_OUTPUT_JSON"},
},
&cli.BoolFlag{
Name: hideHeadersFlagName,
Usage: "Hide the table headers in output data",
EnvVars: []string{"INFLUX_HIDE_HEADERS"},
},
}
// Flag used by commands that hit an authenticated API.
var commonTokenFlag = cli.StringFlag{
Name: tokenFlagName,
Usage: "Authentication token",
Aliases: []string{"t"},
EnvVars: []string{"INFLUX_TOKEN"},
}
var commonFlagsNoToken = append(coreFlags, printFlags...)
var commonFlagsNoPrint = append(coreFlags, &commonTokenFlag)
var commonFlags = append(commonFlagsNoToken, &commonTokenFlag)
// newCli builds a CLI core that reads from stdin, writes to stdout/stderr, manages a local config store,
// and optionally tracks a trace ID specified over the CLI.
func newCli(ctx *cli.Context) (cmd.CLI, error) {
configPath := ctx.String(configPathFlagName)
var err error
if configPath == "" {
configPath, err = config.DefaultPath()
if err != nil {
return cmd.CLI{}, err
}
}
configSvc := config.NewLocalConfigService(configPath)
var activeConfig config.Config
if ctx.IsSet(configNameFlagName) {
if activeConfig, err = configSvc.SwitchActive(ctx.String(configNameFlagName)); err != nil {
return cmd.CLI{}, err
}
} else if activeConfig, err = configSvc.Active(); err != nil {
return cmd.CLI{}, err
}
return cmd.CLI{
StdIO: stdio.TerminalStdio,
PrintAsJSON: ctx.Bool(printJsonFlagName),
HideTableHeaders: ctx.Bool(hideHeadersFlagName),
ActiveConfig: activeConfig,
ConfigService: configSvc,
}, nil
}
// newApiClient returns an API client configured to communicate with a remote InfluxDB instance over HTTP.
// Client parameters are pulled from the CLI context.
func newApiClient(ctx *cli.Context, configSvc config.Service, injectToken bool) (*api.APIClient, error) {
cfg, err := configSvc.Active()
if err != nil {
return nil, err
}
if ctx.IsSet(tokenFlagName) {
cfg.Token = ctx.String(tokenFlagName)
}
if ctx.IsSet(hostFlagName) {
cfg.Host = ctx.String(hostFlagName)
}
parsedHost, err := url.Parse(cfg.Host)
if err != nil {
return nil, fmt.Errorf("host URL %q is invalid: %w", cfg.Host, err)
}
clientTransport := http.DefaultTransport.(*http.Transport)
clientTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: ctx.Bool(skipVerifyFlagName)}
apiConfig := api.NewConfiguration()
apiConfig.Host = parsedHost.Host
apiConfig.Scheme = parsedHost.Scheme
apiConfig.UserAgent = fmt.Sprintf("influx/%s (%s) Sha/%s Date/%s", version, runtime.GOOS, commit, date)
apiConfig.HTTPClient = &http.Client{Transport: clientTransport}
if injectToken {
apiConfig.DefaultHeader["Authorization"] = fmt.Sprintf("Token %s", cfg.Token)
}
if ctx.IsSet(traceIdFlagName) {
// NOTE: This is circumventing our codegen. If the header we use for tracing ever changes,
// we'll need to manually update the string here to match.
//
// The alternative is to pass the trace ID to the business logic for every CLI command, and
// use codegen'd logic to set the header on every HTTP request. Early versions of the CLI
// used that technique, and we found it to be error-prone and easy to forget during testing.
apiConfig.DefaultHeader["Zap-Trace-Span"] = ctx.String(traceIdFlagName)
}
apiConfig.Debug = ctx.Bool(httpDebugFlagName)
return api.NewAPIClient(apiConfig), nil
}
var app = cli.App{
Name: "influx",
Usage: "Influx Client",
@ -206,59 +43,6 @@ var app = cli.App{
},
}
func withCli() cli.BeforeFunc {
return func(ctx *cli.Context) error {
c, err := newCli(ctx)
if err != nil {
return err
}
ctx.App.Metadata["cli"] = c
return nil
}
}
func getCLI(ctx *cli.Context) cmd.CLI {
i, ok := ctx.App.Metadata["cli"].(cmd.CLI)
if !ok {
panic("missing CLI")
}
return i
}
func withApi(injectToken bool) cli.BeforeFunc {
key := "api-no-token"
if injectToken {
key = "api"
}
makeFn := func(ctx *cli.Context) error {
c := getCLI(ctx)
apiClient, err := newApiClient(ctx, c.ConfigService, injectToken)
if err != nil {
return err
}
ctx.App.Metadata[key] = apiClient
return nil
}
return middleware.WithBeforeFns(makeFn)
}
func getAPI(ctx *cli.Context) *api.APIClient {
i, ok := ctx.App.Metadata["api"].(*api.APIClient)
if !ok {
panic("missing APIClient with token")
}
return i
}
func getAPINoToken(ctx *cli.Context) *api.APIClient {
i, ok := ctx.App.Metadata["api-no-token"].(*api.APIClient)
if !ok {
panic("missing APIClient without token")
}
return i
}
func main() {
ctx := signals.WithStandardSignals(context.Background())
if err := app.RunContext(ctx, os.Args); err != nil {

View File

@ -1,36 +0,0 @@
package main
import (
"github.com/influxdata/influx-cli/v2/internal/cmd"
"github.com/urfave/cli/v2"
)
func getOrgBucketFlags(c *cmd.OrgBucketParams) []cli.Flag {
return []cli.Flag{
&cli.GenericFlag{
Name: "bucket-id",
Usage: "The bucket ID, required if name isn't provided",
Aliases: []string{"i"},
Value: &c.BucketID,
},
&cli.StringFlag{
Name: "bucket",
Usage: "The bucket name, org or org-id will be required by choosing this",
Aliases: []string{"n"},
Destination: &c.BucketName,
},
&cli.GenericFlag{
Name: "org-id",
Usage: "The ID of the organization",
EnvVars: []string{"INFLUX_ORG_ID"},
Value: &c.OrgID,
},
&cli.StringFlag{
Name: "org",
Usage: "The name of the organization",
Aliases: []string{"o"},
EnvVars: []string{"INFLUX_ORG"},
Destination: &c.OrgName,
},
}
}

View File

@ -11,7 +11,7 @@ func newPingCmd() *cli.Command {
Name: "ping",
Usage: "Check the InfluxDB /health endpoint",
Before: middleware.WithBeforeFns(withCli(), withApi(false)),
Flags: coreFlags,
Flags: coreFlags(),
Action: func(ctx *cli.Context) error {
client := ping.Client{
CLI: getCLI(ctx),

View File

@ -22,7 +22,7 @@ func newQueryCmd() *cli.Command {
ArgsUsage: "[query literal or '-' for stdin]",
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(
commonFlagsNoPrint,
commonFlagsNoPrint(),
&cli.GenericFlag{
Name: "org-id",
Usage: "The ID of the organization",

View File

@ -13,7 +13,7 @@ func newSetupCmd() *cli.Command {
Usage: "Setup instance with initial user, org, bucket",
Before: middleware.WithBeforeFns(withCli(), withApi(false)),
Flags: append(
commonFlagsNoToken,
commonFlagsNoToken(),
&cli.StringFlag{
Name: "username",
Usage: "Name of initial user to create",

View File

@ -189,7 +189,7 @@ func newWriteCmd() *cli.Command {
Usage: "Write points to InfluxDB",
Description: "Write data to InfluxDB via stdin, or add an entire file specified with the -f flag",
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(commonFlagsNoPrint, params.Flags()...),
Flags: append(commonFlagsNoPrint(), params.Flags()...),
Action: func(ctx *cli.Context) error {
errorFile, err := params.makeErrorFile()
if err != nil {
@ -229,7 +229,7 @@ func newWriteDryRun() *cli.Command {
Usage: "Write to stdout instead of InfluxDB",
Description: "Write protocol lines to stdout instead of InfluxDB. Troubleshoot conversion from CSV to line protocol",
Before: middleware.WithBeforeFns(withCli(), withApi(true)),
Flags: append(commonFlagsNoPrint, params.Flags()...),
Flags: append(commonFlagsNoPrint(), params.Flags()...),
Action: func(ctx *cli.Context) error {
errorFile, err := params.makeErrorFile()
if err != nil {