From fda02fe98d17e9b61b2d67e578388b78d9415dd7 Mon Sep 17 00:00:00 2001 From: Garrett Date: Sun, 27 Mar 2022 23:44:17 +0000 Subject: [PATCH 1/5] chore: Add helper for uniform flags and env vars --- cli/cliflags/cliflags.go | 81 ++++++++++++++ cli/cliflags/cliflags_test.go | 203 ++++++++++++++++++++++++++++++++++ cli/start.go | 65 ++++------- cli/workspaceagent.go | 18 ++- 4 files changed, 313 insertions(+), 54 deletions(-) create mode 100644 cli/cliflags/cliflags.go create mode 100644 cli/cliflags/cliflags_test.go diff --git a/cli/cliflags/cliflags.go b/cli/cliflags/cliflags.go new file mode 100644 index 0000000000000..d68cf4981c50d --- /dev/null +++ b/cli/cliflags/cliflags.go @@ -0,0 +1,81 @@ +// Package cliflags provides helpers for uniform flags, env vars, and usage docs. +// Helpers will set flags to their default value if the environment variable and flag are unset. +// Helpers inject environment variable into flag usage docs if provided. +// +// Usage: +// +// cliflags.String(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") +// +// Will produce the following usage docs: +// +// -a, --address string The address to serve the API and dashboard (uses $CODER_ADDRESS). (default "127.0.0.1:3000") +// +package cliflags + +import ( + "fmt" + "os" + "strconv" + + "github.com/spf13/pflag" +) + +// String sets a string flag on the given flag set. +func String(flagset *pflag.FlagSet, p *string, name string, shorthand string, env string, def string, usage string) { + flagset.StringVarP(p, name, shorthand, envOrDefaultString(env, def), fmtUsage(usage, env)) +} + +// Int sets a int flag on the given flag set. +func Int(flagset *pflag.FlagSet, p *int, name string, shorthand string, env string, def int, usage string) { + flagset.IntVarP(p, name, shorthand, envOrDefaultInt(env, def), fmtUsage(usage, env)) +} + +// Bool sets a bool flag on the given flag set. +func Bool(flagset *pflag.FlagSet, p *bool, name string, shorthand string, env string, def bool, usage string) { + flagset.BoolVarP(p, name, shorthand, envOrDefaultBool(env, def), fmtUsage(usage, env)) +} + +func envOrDefaultString(env string, def string) string { + v, ok := os.LookupEnv(env) + if !ok { + return def + } + + return v +} + +func envOrDefaultInt(env string, def int) int { + v, ok := os.LookupEnv(env) + if !ok { + return def + } + + i, err := strconv.Atoi(v) + if err != nil { + return def + } + + return i +} + +func envOrDefaultBool(env string, def bool) bool { + v, ok := os.LookupEnv(env) + if !ok { + return def + } + + i, err := strconv.ParseBool(v) + if err != nil { + return def + } + + return i +} + +func fmtUsage(u string, env string) string { + if env == "" { + return fmt.Sprintf("%s.", u) + } + + return fmt.Sprintf("%s (uses $%s).", u, env) +} diff --git a/cli/cliflags/cliflags_test.go b/cli/cliflags/cliflags_test.go new file mode 100644 index 0000000000000..0792700418be0 --- /dev/null +++ b/cli/cliflags/cliflags_test.go @@ -0,0 +1,203 @@ +package cliflags_test + +import ( + "fmt" + "os" + "strconv" + "testing" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/cli/cliflags" + "github.com/coder/coder/cryptorand" +) + +func TestCliFlags(t *testing.T) { + t.Parallel() + + t.Run("StringDefault", func(t *testing.T) { + t.Parallel() + + var p string + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + def, _ := cryptorand.String(10) + usage, _ := cryptorand.String(10) + + cliflags.String(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("StringEnvVar", func(t *testing.T) { + t.Parallel() + + var p string + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + envValue, _ := cryptorand.String(10) + os.Setenv(env, envValue) + defer os.Unsetenv(env) + def, _ := cryptorand.String(10) + usage, _ := cryptorand.String(10) + + cliflags.String(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, envValue, got) + }) + + t.Run("EmptyEnvVar", func(t *testing.T) { + t.Parallel() + + var p string + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env := "" + def, _ := cryptorand.String(10) + usage, _ := cryptorand.String(10) + + cliflags.String(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.NotContains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("IntDefault", func(t *testing.T) { + t.Parallel() + + var p int + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + def, _ := cryptorand.Int() + usage, _ := cryptorand.String(10) + + cliflags.Int(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetInt(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("IntEnvVar", func(t *testing.T) { + t.Parallel() + + var p int + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + envValue, _ := cryptorand.Int() + os.Setenv(env, strconv.Itoa(envValue)) + defer os.Unsetenv(env) + def, _ := cryptorand.Int() + usage, _ := cryptorand.String(10) + + cliflags.Int(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetInt(name) + require.NoError(t, err) + require.Equal(t, envValue, got) + }) + + t.Run("IntFailParse", func(t *testing.T) { + t.Parallel() + + var p int + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + envValue, _ := cryptorand.String(10) + os.Setenv(env, envValue) + defer os.Unsetenv(env) + def, _ := cryptorand.Int() + usage, _ := cryptorand.String(10) + + cliflags.Int(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetInt(name) + require.NoError(t, err) + require.Equal(t, def, got) + }) + + t.Run("BoolDefault", func(t *testing.T) { + t.Parallel() + + var p bool + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + def, _ := cryptorand.Bool() + usage, _ := cryptorand.String(10) + + cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("BoolEnvVar", func(t *testing.T) { + t.Parallel() + + var p bool + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + envValue, _ := cryptorand.Bool() + os.Setenv(env, strconv.FormatBool(envValue)) + defer os.Unsetenv(env) + def, _ := cryptorand.Bool() + usage, _ := cryptorand.String(10) + + cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, envValue, got) + }) + + t.Run("BoolFailParse", func(t *testing.T) { + t.Parallel() + + var p bool + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + envValue, _ := cryptorand.String(10) + os.Setenv(env, envValue) + defer os.Unsetenv(env) + def, _ := cryptorand.Bool() + usage, _ := cryptorand.String(10) + + cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, def, got) + }) +} diff --git a/cli/start.go b/cli/start.go index 99260ed78565d..8fc19afad895c 100644 --- a/cli/start.go +++ b/cli/start.go @@ -13,7 +13,6 @@ import ( "net/url" "os" "os/signal" - "strconv" "time" "github.com/briandowns/spinner" @@ -25,6 +24,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" + "github.com/coder/coder/cli/cliflags" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/cli/config" "github.com/coder/coder/coderd" @@ -44,7 +44,7 @@ func start() *cobra.Command { address string dev bool postgresURL string - provisionerDaemonCount uint8 + provisionerDaemonCount int tlsCertFile string tlsClientCAFile string tlsClientAuth string @@ -57,10 +57,6 @@ func start() *cobra.Command { Use: "start", RunE: func(cmd *cobra.Command, args []string) error { printLogo(cmd) - if postgresURL == "" { - // Default to the environment variable! - postgresURL = os.Getenv("CODER_PG_CONNECTION_URL") - } listener, err := net.Listen("tcp", address) if err != nil { @@ -163,7 +159,7 @@ func start() *cobra.Command { } provisionerDaemons := make([]*provisionerd.Server, 0) - for i := uint8(0); i < provisionerDaemonCount; i++ { + for i := 0; i < provisionerDaemonCount; i++ { daemonClose, err := newProvisionerDaemon(cmd.Context(), client, logger) if err != nil { return xerrors.Errorf("create provisioner daemon: %w", err) @@ -305,46 +301,27 @@ func start() *cobra.Command { return nil }, } - defaultAddress := os.Getenv("CODER_ADDRESS") - if defaultAddress == "" { - defaultAddress = "127.0.0.1:3000" - } - root.Flags().StringVarP(&accessURL, "access-url", "", os.Getenv("CODER_ACCESS_URL"), "Specifies the external URL to access Coder (uses $CODER_ACCESS_URL).") - root.Flags().StringVarP(&address, "address", "a", defaultAddress, "The address to serve the API and dashboard (uses $CODER_ADDRESS).") - defaultDev, _ := strconv.ParseBool(os.Getenv("CODER_DEV_MODE")) - root.Flags().BoolVarP(&dev, "dev", "", defaultDev, "Serve Coder in dev mode for tinkering (uses $CODER_DEV_MODE).") - root.Flags().StringVarP(&postgresURL, "postgres-url", "", "", - "URL of a PostgreSQL database to connect to (defaults to $CODER_PG_CONNECTION_URL).") - root.Flags().Uint8VarP(&provisionerDaemonCount, "provisioner-daemons", "", 1, "The amount of provisioner daemons to create on start.") - defaultTLSEnable, _ := strconv.ParseBool(os.Getenv("CODER_TLS_ENABLE")) - root.Flags().BoolVarP(&tlsEnable, "tls-enable", "", defaultTLSEnable, "Specifies if TLS will be enabled (uses $CODER_TLS_ENABLE).") - root.Flags().StringVarP(&tlsCertFile, "tls-cert-file", "", os.Getenv("CODER_TLS_CERT_FILE"), + + cliflags.String(root.Flags(), &accessURL, "access-url", "", "CODER_ACCESS_URL", "", "Specifies the external URL to access Coder") + cliflags.String(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") + cliflags.Bool(root.Flags(), &dev, "dev", "", "CODER_DEV_MODE", false, "Serve Coder in dev mode for tinkering") + cliflags.String(root.Flags(), &postgresURL, "postgres-url", "", "CODER_PG_CONNECTION_URL", "", "URL of a PostgreSQL database to connect to") + cliflags.Int(root.Flags(), &provisionerDaemonCount, "provisioner-daemons", "", "CODER_PROVISIONER_DAEMONS", 1, "The amount of provisioner daemons to create on start.") + cliflags.Bool(root.Flags(), &tlsEnable, "tls-enable", "", "CODER_TLS_ENABLE", false, "Specifies if TLS will be enabled") + cliflags.String(root.Flags(), &tlsCertFile, "tls-cert-file", "", "CODER_TLS_CERT_FILE", "", "Specifies the path to the certificate for TLS. It requires a PEM-encoded file. "+ "To configure the listener to use a CA certificate, concatenate the primary certificate "+ - "and the CA certificate together. The primary certificate should appear first in the combined file (uses $CODER_TLS_CERT_FILE).") - root.Flags().StringVarP(&tlsClientCAFile, "tls-client-ca-file", "", os.Getenv("CODER_TLS_CLIENT_CA_FILE"), - "PEM-encoded Certificate Authority file used for checking the authenticity of client (uses $CODER_TLS_CLIENT_CA_FILE).") - defaultTLSClientAuth := os.Getenv("CODER_TLS_CLIENT_AUTH") - if defaultTLSClientAuth == "" { - defaultTLSClientAuth = "request" - } - root.Flags().StringVarP(&tlsClientAuth, "tls-client-auth", "", defaultTLSClientAuth, + "and the CA certificate together. The primary certificate should appear first in the combined file") + cliflags.String(root.Flags(), &tlsClientCAFile, "tls-client-ca-file", "", "CODER_TLS_CLIENT_CA_FILE", "", + "PEM-encoded Certificate Authority file used for checking the authenticity of client") + cliflags.String(root.Flags(), &tlsClientAuth, "tls-client-auth", "", "CODER_TLS_CLIENT_AUTH", "request", `Specifies the policy the server will follow for TLS Client Authentication. `+ - `Accepted values are "none", "request", "require-any", "verify-if-given", or "require-and-verify" (uses $CODER_TLS_CLIENT_AUTH).`) - root.Flags().StringVarP(&tlsKeyFile, "tls-key-file", "", os.Getenv("CODER_TLS_KEY_FILE"), - "Specifies the path to the private key for the certificate. It requires a PEM-encoded file (uses $CODER_TLS_KEY_FILE).") - defaultTLSMinVersion := os.Getenv("CODER_TLS_MIN_VERSION") - if defaultTLSMinVersion == "" { - defaultTLSMinVersion = "tls12" - } - root.Flags().StringVarP(&tlsMinVersion, "tls-min-version", "", defaultTLSMinVersion, - `Specifies the minimum supported version of TLS. Accepted values are "tls10", "tls11", "tls12" or "tls13" (uses $CODER_TLS_MIN_VERSION).`) - defaultTunnelRaw := os.Getenv("CODER_DEV_TUNNEL") - if defaultTunnelRaw == "" { - defaultTunnelRaw = "true" - } - defaultTunnel, _ := strconv.ParseBool(defaultTunnelRaw) - root.Flags().BoolVarP(&useTunnel, "tunnel", "", defaultTunnel, "Serve dev mode through a Cloudflare Tunnel for easy setup (uses $CODER_DEV_TUNNEL).") + `Accepted values are "none", "request", "require-any", "verify-if-given", or "require-and-verify"`) + cliflags.String(root.Flags(), &tlsKeyFile, "tls-key-file", "", "CODER_TLS_KEY_FILE", "", + "Specifies the path to the private key for the certificate. It requires a PEM-encoded file") + cliflags.String(root.Flags(), &tlsMinVersion, "tls-min-version", "", "CODER_TLS_MIN_VERSION", "tls12", + `Specifies the minimum supported version of TLS. Accepted values are "tls10", "tls11", "tls12" or "tls13"`) + cliflags.Bool(root.Flags(), &useTunnel, "tunnel", "", "CODER_DEV_TUNNEL", false, "Serve dev mode through a Cloudflare Tunnel for easy setup") _ = root.Flags().MarkHidden("tunnel") return root diff --git a/cli/workspaceagent.go b/cli/workspaceagent.go index 369fe8010445b..2608ffff59e47 100644 --- a/cli/workspaceagent.go +++ b/cli/workspaceagent.go @@ -3,7 +3,6 @@ package cli import ( "context" "net/url" - "os" "time" "cloud.google.com/go/compute/metadata" @@ -14,6 +13,7 @@ import ( "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/agent" + "github.com/coder/coder/cli/cliflags" "github.com/coder/coder/codersdk" "github.com/coder/coder/peer" "github.com/coder/retry" @@ -23,6 +23,7 @@ func workspaceAgent() *cobra.Command { var ( rawURL string auth string + token string ) cmd := &cobra.Command{ Use: "agent", @@ -40,11 +41,10 @@ func workspaceAgent() *cobra.Command { client := codersdk.New(coderURL) switch auth { case "token": - sessionToken, exists := os.LookupEnv("CODER_TOKEN") - if !exists { + if token == "" { return xerrors.Errorf("CODER_TOKEN must be set for token auth") } - client.SessionToken = sessionToken + client.SessionToken = token case "google-instance-identity": // This is *only* done for testing to mock client authentication. // This will never be set in a production scenario. @@ -83,12 +83,10 @@ func workspaceAgent() *cobra.Command { return closer.Close() }, } - defaultAuth := os.Getenv("CODER_AUTH") - if defaultAuth == "" { - defaultAuth = "token" - } - cmd.Flags().StringVarP(&auth, "auth", "", defaultAuth, "Specify the authentication type to use for the agent.") - cmd.Flags().StringVarP(&rawURL, "url", "", os.Getenv("CODER_URL"), "Specify the URL to access Coder.") + + cliflags.String(cmd.Flags(), &auth, "auth", "", "CODER_AUTH", "token", "Specify the authentication type to use for the agent") + cliflags.String(cmd.Flags(), &rawURL, "url", "", "CODER_URL", "", "Specify the URL to access Coder") + cliflags.String(cmd.Flags(), &auth, "token", "", "CODER_TOKEN", "", "Specifies the authentication token to access Coder") return cmd } From 15c9aaa50e1637e6f397f932dd8994c5652a2add Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 28 Mar 2022 18:54:54 +0000 Subject: [PATCH 2/5] pr comments --- cli/cliflags/cliflags.go | 81 -------------- cli/cliflags/cliflags_test.go | 203 ---------------------------------- cli/start.go | 39 +++---- cli/workspaceagent.go | 8 +- 4 files changed, 24 insertions(+), 307 deletions(-) delete mode 100644 cli/cliflags/cliflags.go delete mode 100644 cli/cliflags/cliflags_test.go diff --git a/cli/cliflags/cliflags.go b/cli/cliflags/cliflags.go deleted file mode 100644 index d68cf4981c50d..0000000000000 --- a/cli/cliflags/cliflags.go +++ /dev/null @@ -1,81 +0,0 @@ -// Package cliflags provides helpers for uniform flags, env vars, and usage docs. -// Helpers will set flags to their default value if the environment variable and flag are unset. -// Helpers inject environment variable into flag usage docs if provided. -// -// Usage: -// -// cliflags.String(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") -// -// Will produce the following usage docs: -// -// -a, --address string The address to serve the API and dashboard (uses $CODER_ADDRESS). (default "127.0.0.1:3000") -// -package cliflags - -import ( - "fmt" - "os" - "strconv" - - "github.com/spf13/pflag" -) - -// String sets a string flag on the given flag set. -func String(flagset *pflag.FlagSet, p *string, name string, shorthand string, env string, def string, usage string) { - flagset.StringVarP(p, name, shorthand, envOrDefaultString(env, def), fmtUsage(usage, env)) -} - -// Int sets a int flag on the given flag set. -func Int(flagset *pflag.FlagSet, p *int, name string, shorthand string, env string, def int, usage string) { - flagset.IntVarP(p, name, shorthand, envOrDefaultInt(env, def), fmtUsage(usage, env)) -} - -// Bool sets a bool flag on the given flag set. -func Bool(flagset *pflag.FlagSet, p *bool, name string, shorthand string, env string, def bool, usage string) { - flagset.BoolVarP(p, name, shorthand, envOrDefaultBool(env, def), fmtUsage(usage, env)) -} - -func envOrDefaultString(env string, def string) string { - v, ok := os.LookupEnv(env) - if !ok { - return def - } - - return v -} - -func envOrDefaultInt(env string, def int) int { - v, ok := os.LookupEnv(env) - if !ok { - return def - } - - i, err := strconv.Atoi(v) - if err != nil { - return def - } - - return i -} - -func envOrDefaultBool(env string, def bool) bool { - v, ok := os.LookupEnv(env) - if !ok { - return def - } - - i, err := strconv.ParseBool(v) - if err != nil { - return def - } - - return i -} - -func fmtUsage(u string, env string) string { - if env == "" { - return fmt.Sprintf("%s.", u) - } - - return fmt.Sprintf("%s (uses $%s).", u, env) -} diff --git a/cli/cliflags/cliflags_test.go b/cli/cliflags/cliflags_test.go deleted file mode 100644 index 0792700418be0..0000000000000 --- a/cli/cliflags/cliflags_test.go +++ /dev/null @@ -1,203 +0,0 @@ -package cliflags_test - -import ( - "fmt" - "os" - "strconv" - "testing" - - "github.com/spf13/pflag" - "github.com/stretchr/testify/require" - - "github.com/coder/coder/cli/cliflags" - "github.com/coder/coder/cryptorand" -) - -func TestCliFlags(t *testing.T) { - t.Parallel() - - t.Run("StringDefault", func(t *testing.T) { - t.Parallel() - - var p string - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - def, _ := cryptorand.String(10) - usage, _ := cryptorand.String(10) - - cliflags.String(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetString(name) - require.NoError(t, err) - require.Equal(t, def, got) - require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) - }) - - t.Run("StringEnvVar", func(t *testing.T) { - t.Parallel() - - var p string - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - envValue, _ := cryptorand.String(10) - os.Setenv(env, envValue) - defer os.Unsetenv(env) - def, _ := cryptorand.String(10) - usage, _ := cryptorand.String(10) - - cliflags.String(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetString(name) - require.NoError(t, err) - require.Equal(t, envValue, got) - }) - - t.Run("EmptyEnvVar", func(t *testing.T) { - t.Parallel() - - var p string - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env := "" - def, _ := cryptorand.String(10) - usage, _ := cryptorand.String(10) - - cliflags.String(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetString(name) - require.NoError(t, err) - require.Equal(t, def, got) - require.Contains(t, flagset.FlagUsages(), usage) - require.NotContains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) - }) - - t.Run("IntDefault", func(t *testing.T) { - t.Parallel() - - var p int - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - def, _ := cryptorand.Int() - usage, _ := cryptorand.String(10) - - cliflags.Int(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetInt(name) - require.NoError(t, err) - require.Equal(t, def, got) - require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) - }) - - t.Run("IntEnvVar", func(t *testing.T) { - t.Parallel() - - var p int - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - envValue, _ := cryptorand.Int() - os.Setenv(env, strconv.Itoa(envValue)) - defer os.Unsetenv(env) - def, _ := cryptorand.Int() - usage, _ := cryptorand.String(10) - - cliflags.Int(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetInt(name) - require.NoError(t, err) - require.Equal(t, envValue, got) - }) - - t.Run("IntFailParse", func(t *testing.T) { - t.Parallel() - - var p int - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - envValue, _ := cryptorand.String(10) - os.Setenv(env, envValue) - defer os.Unsetenv(env) - def, _ := cryptorand.Int() - usage, _ := cryptorand.String(10) - - cliflags.Int(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetInt(name) - require.NoError(t, err) - require.Equal(t, def, got) - }) - - t.Run("BoolDefault", func(t *testing.T) { - t.Parallel() - - var p bool - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - def, _ := cryptorand.Bool() - usage, _ := cryptorand.String(10) - - cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetBool(name) - require.NoError(t, err) - require.Equal(t, def, got) - require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) - }) - - t.Run("BoolEnvVar", func(t *testing.T) { - t.Parallel() - - var p bool - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - envValue, _ := cryptorand.Bool() - os.Setenv(env, strconv.FormatBool(envValue)) - defer os.Unsetenv(env) - def, _ := cryptorand.Bool() - usage, _ := cryptorand.String(10) - - cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetBool(name) - require.NoError(t, err) - require.Equal(t, envValue, got) - }) - - t.Run("BoolFailParse", func(t *testing.T) { - t.Parallel() - - var p bool - fsname, _ := cryptorand.String(10) - flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) - name, _ := cryptorand.String(10) - shorthand, _ := cryptorand.String(1) - env, _ := cryptorand.String(10) - envValue, _ := cryptorand.String(10) - os.Setenv(env, envValue) - defer os.Unsetenv(env) - def, _ := cryptorand.Bool() - usage, _ := cryptorand.String(10) - - cliflags.Bool(flagset, &p, name, shorthand, env, def, usage) - got, err := flagset.GetBool(name) - require.NoError(t, err) - require.Equal(t, def, got) - }) -} diff --git a/cli/start.go b/cli/start.go index 8fc19afad895c..7f7aa6d4c11dc 100644 --- a/cli/start.go +++ b/cli/start.go @@ -24,7 +24,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" - "github.com/coder/coder/cli/cliflags" + "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/cli/config" "github.com/coder/coder/coderd" @@ -40,11 +40,12 @@ import ( func start() *cobra.Command { var ( - accessURL string - address string - dev bool - postgresURL string - provisionerDaemonCount int + accessURL string + address string + dev bool + postgresURL string + // provisionerDaemonCount is a uint8 to ensure a number > 0. + provisionerDaemonCount uint8 tlsCertFile string tlsClientCAFile string tlsClientAuth string @@ -159,7 +160,7 @@ func start() *cobra.Command { } provisionerDaemons := make([]*provisionerd.Server, 0) - for i := 0; i < provisionerDaemonCount; i++ { + for i := 0; uint8(i) < provisionerDaemonCount; i++ { daemonClose, err := newProvisionerDaemon(cmd.Context(), client, logger) if err != nil { return xerrors.Errorf("create provisioner daemon: %w", err) @@ -302,26 +303,26 @@ func start() *cobra.Command { }, } - cliflags.String(root.Flags(), &accessURL, "access-url", "", "CODER_ACCESS_URL", "", "Specifies the external URL to access Coder") - cliflags.String(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") - cliflags.Bool(root.Flags(), &dev, "dev", "", "CODER_DEV_MODE", false, "Serve Coder in dev mode for tinkering") - cliflags.String(root.Flags(), &postgresURL, "postgres-url", "", "CODER_PG_CONNECTION_URL", "", "URL of a PostgreSQL database to connect to") - cliflags.Int(root.Flags(), &provisionerDaemonCount, "provisioner-daemons", "", "CODER_PROVISIONER_DAEMONS", 1, "The amount of provisioner daemons to create on start.") - cliflags.Bool(root.Flags(), &tlsEnable, "tls-enable", "", "CODER_TLS_ENABLE", false, "Specifies if TLS will be enabled") - cliflags.String(root.Flags(), &tlsCertFile, "tls-cert-file", "", "CODER_TLS_CERT_FILE", "", + cliflag.StringVarP(root.Flags(), &accessURL, "access-url", "", "CODER_ACCESS_URL", "", "Specifies the external URL to access Coder") + cliflag.StringVarP(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") + cliflag.BoolVarP(root.Flags(), &dev, "dev", "", "CODER_DEV_MODE", false, "Serve Coder in dev mode for tinkering") + cliflag.StringVarP(root.Flags(), &postgresURL, "postgres-url", "", "CODER_PG_CONNECTION_URL", "", "URL of a PostgreSQL database to connect to") + cliflag.Uint8VarP(root.Flags(), &provisionerDaemonCount, "provisioner-daemons", "", "CODER_PROVISIONER_DAEMONS", 1, "The amount of provisioner daemons to create on start.") + cliflag.BoolVarP(root.Flags(), &tlsEnable, "tls-enable", "", "CODER_TLS_ENABLE", false, "Specifies if TLS will be enabled") + cliflag.StringVarP(root.Flags(), &tlsCertFile, "tls-cert-file", "", "CODER_TLS_CERT_FILE", "", "Specifies the path to the certificate for TLS. It requires a PEM-encoded file. "+ "To configure the listener to use a CA certificate, concatenate the primary certificate "+ "and the CA certificate together. The primary certificate should appear first in the combined file") - cliflags.String(root.Flags(), &tlsClientCAFile, "tls-client-ca-file", "", "CODER_TLS_CLIENT_CA_FILE", "", + cliflag.StringVarP(root.Flags(), &tlsClientCAFile, "tls-client-ca-file", "", "CODER_TLS_CLIENT_CA_FILE", "", "PEM-encoded Certificate Authority file used for checking the authenticity of client") - cliflags.String(root.Flags(), &tlsClientAuth, "tls-client-auth", "", "CODER_TLS_CLIENT_AUTH", "request", + cliflag.StringVarP(root.Flags(), &tlsClientAuth, "tls-client-auth", "", "CODER_TLS_CLIENT_AUTH", "request", `Specifies the policy the server will follow for TLS Client Authentication. `+ `Accepted values are "none", "request", "require-any", "verify-if-given", or "require-and-verify"`) - cliflags.String(root.Flags(), &tlsKeyFile, "tls-key-file", "", "CODER_TLS_KEY_FILE", "", + cliflag.StringVarP(root.Flags(), &tlsKeyFile, "tls-key-file", "", "CODER_TLS_KEY_FILE", "", "Specifies the path to the private key for the certificate. It requires a PEM-encoded file") - cliflags.String(root.Flags(), &tlsMinVersion, "tls-min-version", "", "CODER_TLS_MIN_VERSION", "tls12", + cliflag.StringVarP(root.Flags(), &tlsMinVersion, "tls-min-version", "", "CODER_TLS_MIN_VERSION", "tls12", `Specifies the minimum supported version of TLS. Accepted values are "tls10", "tls11", "tls12" or "tls13"`) - cliflags.Bool(root.Flags(), &useTunnel, "tunnel", "", "CODER_DEV_TUNNEL", false, "Serve dev mode through a Cloudflare Tunnel for easy setup") + cliflag.BoolVarP(root.Flags(), &useTunnel, "tunnel", "", "CODER_DEV_TUNNEL", false, "Serve dev mode through a Cloudflare Tunnel for easy setup") _ = root.Flags().MarkHidden("tunnel") return root diff --git a/cli/workspaceagent.go b/cli/workspaceagent.go index 2608ffff59e47..ec1929ddf4659 100644 --- a/cli/workspaceagent.go +++ b/cli/workspaceagent.go @@ -13,7 +13,7 @@ import ( "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/agent" - "github.com/coder/coder/cli/cliflags" + "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/codersdk" "github.com/coder/coder/peer" "github.com/coder/retry" @@ -84,9 +84,9 @@ func workspaceAgent() *cobra.Command { }, } - cliflags.String(cmd.Flags(), &auth, "auth", "", "CODER_AUTH", "token", "Specify the authentication type to use for the agent") - cliflags.String(cmd.Flags(), &rawURL, "url", "", "CODER_URL", "", "Specify the URL to access Coder") - cliflags.String(cmd.Flags(), &auth, "token", "", "CODER_TOKEN", "", "Specifies the authentication token to access Coder") + cliflag.StringVarP(cmd.Flags(), &auth, "auth", "", "CODER_AUTH", "token", "Specify the authentication type to use for the agent") + cliflag.StringVarP(cmd.Flags(), &rawURL, "url", "", "CODER_URL", "", "Specify the URL to access Coder") + cliflag.StringVarP(cmd.Flags(), &auth, "token", "", "CODER_TOKEN", "", "Specifies the authentication token to access Coder") return cmd } From e22539bad455615ad34fac343af4f8fcde0614e4 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 28 Mar 2022 18:58:15 +0000 Subject: [PATCH 3/5] pr comments --- cli/cliflag/cliflag.go | 72 ++++++++++++++++++ cli/cliflag/cliflag_test.go | 147 ++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+) create mode 100644 cli/cliflag/cliflag.go create mode 100644 cli/cliflag/cliflag_test.go diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go new file mode 100644 index 0000000000000..76270c1eff5f9 --- /dev/null +++ b/cli/cliflag/cliflag.go @@ -0,0 +1,72 @@ +// Package cliflags extends flagset with environment variable defaults. +// Helpers will set flags to their default value if the environment variable and flag are unset. +// Helpers inject environment variable into flag usage docs if provided. +// +// Usage: +// +// cliflag.String(root.Flags(), &address, "address", "a", "CODER_ADDRESS", "127.0.0.1:3000", "The address to serve the API and dashboard") +// +// Will produce the following usage docs: +// +// -a, --address string The address to serve the API and dashboard (uses $CODER_ADDRESS). (default "127.0.0.1:3000") +// +package cliflag + +import ( + "fmt" + "os" + "strconv" + + "github.com/spf13/pflag" +) + +// StringVarP sets a string flag on the given flag set. +func StringVarP(flagset *pflag.FlagSet, p *string, name string, shorthand string, env string, def string, usage string) { + v, ok := os.LookupEnv(env) + if !ok || v == "" { + v = def + } + flagset.StringVarP(p, name, shorthand, v, fmtUsage(usage, env)) +} + +// Uint8VarP sets a uint8 flag on the given flag set. +func Uint8VarP(flagset *pflag.FlagSet, ptr *uint8, name string, shorthand string, env string, def uint8, usage string) { + val, ok := os.LookupEnv(env) + if !ok || val == "" { + flagset.Uint8VarP(ptr, name, shorthand, def, fmtUsage(usage, env)) + return + } + + vi64, err := strconv.ParseUint(val, 10, 8) + if err != nil { + flagset.Uint8VarP(ptr, name, shorthand, def, fmtUsage(usage, env)) + return + } + + flagset.Uint8VarP(ptr, name, shorthand, uint8(vi64), fmtUsage(usage, env)) +} + +// BoolVarP sets a bool flag on the given flag set. +func BoolVarP(flagset *pflag.FlagSet, ptr *bool, name string, shorthand string, env string, def bool, usage string) { + val, ok := os.LookupEnv(env) + if !ok || val == "" { + flagset.BoolVarP(ptr, name, shorthand, def, fmtUsage(usage, env)) + return + } + + valb, err := strconv.ParseBool(val) + if err != nil { + flagset.BoolVarP(ptr, name, shorthand, def, fmtUsage(usage, env)) + return + } + + flagset.BoolVarP(ptr, name, shorthand, valb, fmtUsage(usage, env)) +} + +func fmtUsage(u string, env string) string { + if env == "" { + return fmt.Sprintf("%s.", u) + } + + return fmt.Sprintf("%s (uses $%s).", u, env) +} diff --git a/cli/cliflag/cliflag_test.go b/cli/cliflag/cliflag_test.go new file mode 100644 index 0000000000000..36e9531e4449c --- /dev/null +++ b/cli/cliflag/cliflag_test.go @@ -0,0 +1,147 @@ +package cliflag_test + +import ( + "fmt" + "strconv" + "testing" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/cli/cliflag" + "github.com/coder/coder/cryptorand" +) + +// Testcliflag cannot run in parallel because it uses t.Setenv. +//nolint:paralleltest +func TestCliflag(t *testing.T) { + + t.Run("StringDefault", func(t *testing.T) { + var p string + flagset, name, shorthand, env, usage := randomFlag() + def, _ := cryptorand.String(10) + + cliflag.StringVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("StringEnvVar", func(t *testing.T) { + var p string + flagset, name, shorthand, env, usage := randomFlag() + envValue, _ := cryptorand.String(10) + t.Setenv(env, envValue) + def, _ := cryptorand.String(10) + + cliflag.StringVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, envValue, got) + }) + + t.Run("EmptyEnvVar", func(t *testing.T) { + var p string + flagset, name, shorthand, env, usage := randomFlag() + env = "" + def, _ := cryptorand.String(10) + + cliflag.StringVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetString(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.NotContains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("IntDefault", func(t *testing.T) { + var p uint8 + flagset, name, shorthand, env, usage := randomFlag() + def, _ := cryptorand.Int63n(10) + + cliflag.Uint8VarP(flagset, &p, name, shorthand, env, uint8(def), usage) + got, err := flagset.GetUint8(name) + require.NoError(t, err) + require.Equal(t, uint8(def), got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("IntEnvVar", func(t *testing.T) { + var p uint8 + flagset, name, shorthand, env, usage := randomFlag() + envValue, _ := cryptorand.Int63n(10) + t.Setenv(env, strconv.FormatUint(uint64(envValue), 10)) + def, _ := cryptorand.Int() + + cliflag.Uint8VarP(flagset, &p, name, shorthand, env, uint8(def), usage) + got, err := flagset.GetUint8(name) + require.NoError(t, err) + require.Equal(t, uint8(envValue), got) + }) + + t.Run("IntFailParse", func(t *testing.T) { + var p uint8 + flagset, name, shorthand, env, usage := randomFlag() + envValue, _ := cryptorand.String(10) + t.Setenv(env, envValue) + def, _ := cryptorand.Int63n(10) + + cliflag.Uint8VarP(flagset, &p, name, shorthand, env, uint8(def), usage) + got, err := flagset.GetUint8(name) + require.NoError(t, err) + require.Equal(t, uint8(def), got) + }) + + t.Run("BoolDefault", func(t *testing.T) { + var p bool + flagset, name, shorthand, env, usage := randomFlag() + def, _ := cryptorand.Bool() + + cliflag.BoolVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, def, got) + require.Contains(t, flagset.FlagUsages(), usage) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + }) + + t.Run("BoolEnvVar", func(t *testing.T) { + var p bool + flagset, name, shorthand, env, usage := randomFlag() + envValue, _ := cryptorand.Bool() + t.Setenv(env, strconv.FormatBool(envValue)) + def, _ := cryptorand.Bool() + + cliflag.BoolVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, envValue, got) + }) + + t.Run("BoolFailParse", func(t *testing.T) { + var p bool + flagset, name, shorthand, env, usage := randomFlag() + envValue, _ := cryptorand.String(10) + t.Setenv(env, envValue) + def, _ := cryptorand.Bool() + + cliflag.BoolVarP(flagset, &p, name, shorthand, env, def, usage) + got, err := flagset.GetBool(name) + require.NoError(t, err) + require.Equal(t, def, got) + }) +} + +func randomFlag() (*pflag.FlagSet, string, string, string, string) { + fsname, _ := cryptorand.String(10) + flagset := pflag.NewFlagSet(fsname, pflag.PanicOnError) + name, _ := cryptorand.String(10) + shorthand, _ := cryptorand.String(1) + env, _ := cryptorand.String(10) + usage, _ := cryptorand.String(10) + + return flagset, name, shorthand, env, usage +} From e95c20d20ef5521f9fa4089e872ec69dc8e179fa Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 28 Mar 2022 19:12:12 +0000 Subject: [PATCH 4/5] pr comments --- cli/cliflag/cliflag.go | 4 +--- cli/cliflag/cliflag_test.go | 14 ++++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go index 76270c1eff5f9..c011bc0681cfe 100644 --- a/cli/cliflag/cliflag.go +++ b/cli/cliflag/cliflag.go @@ -1,6 +1,4 @@ // Package cliflags extends flagset with environment variable defaults. -// Helpers will set flags to their default value if the environment variable and flag are unset. -// Helpers inject environment variable into flag usage docs if provided. // // Usage: // @@ -68,5 +66,5 @@ func fmtUsage(u string, env string) string { return fmt.Sprintf("%s.", u) } - return fmt.Sprintf("%s (uses $%s).", u, env) + return fmt.Sprintf("%s - consumes $%s.", u, env) } diff --git a/cli/cliflag/cliflag_test.go b/cli/cliflag/cliflag_test.go index 36e9531e4449c..542bb04abfd9d 100644 --- a/cli/cliflag/cliflag_test.go +++ b/cli/cliflag/cliflag_test.go @@ -15,7 +15,6 @@ import ( // Testcliflag cannot run in parallel because it uses t.Setenv. //nolint:paralleltest func TestCliflag(t *testing.T) { - t.Run("StringDefault", func(t *testing.T) { var p string flagset, name, shorthand, env, usage := randomFlag() @@ -26,7 +25,7 @@ func TestCliflag(t *testing.T) { require.NoError(t, err) require.Equal(t, def, got) require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf(" - consumes $%s", env)) }) t.Run("StringEnvVar", func(t *testing.T) { @@ -44,16 +43,15 @@ func TestCliflag(t *testing.T) { t.Run("EmptyEnvVar", func(t *testing.T) { var p string - flagset, name, shorthand, env, usage := randomFlag() - env = "" + flagset, name, shorthand, _, usage := randomFlag() def, _ := cryptorand.String(10) - cliflag.StringVarP(flagset, &p, name, shorthand, env, def, usage) + cliflag.StringVarP(flagset, &p, name, shorthand, "", def, usage) got, err := flagset.GetString(name) require.NoError(t, err) require.Equal(t, def, got) require.Contains(t, flagset.FlagUsages(), usage) - require.NotContains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + require.NotContains(t, flagset.FlagUsages(), " - consumes") }) t.Run("IntDefault", func(t *testing.T) { @@ -66,7 +64,7 @@ func TestCliflag(t *testing.T) { require.NoError(t, err) require.Equal(t, uint8(def), got) require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf(" - consumes $%s", env)) }) t.Run("IntEnvVar", func(t *testing.T) { @@ -105,7 +103,7 @@ func TestCliflag(t *testing.T) { require.NoError(t, err) require.Equal(t, def, got) require.Contains(t, flagset.FlagUsages(), usage) - require.Contains(t, flagset.FlagUsages(), fmt.Sprintf("(uses $%s).", env)) + require.Contains(t, flagset.FlagUsages(), fmt.Sprintf(" - consumes $%s", env)) }) t.Run("BoolEnvVar", func(t *testing.T) { From 13c71df231bb387cc4d1f5f48fe8d97ffd10ec67 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 28 Mar 2022 19:13:53 +0000 Subject: [PATCH 5/5] pr comments --- cli/cliflag/cliflag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go index c011bc0681cfe..e846d5fc391ae 100644 --- a/cli/cliflag/cliflag.go +++ b/cli/cliflag/cliflag.go @@ -1,4 +1,4 @@ -// Package cliflags extends flagset with environment variable defaults. +// Package cliflag extends flagset with environment variable defaults. // // Usage: //