From a65a16122d625392048155920f3184ae3f727749 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 10 Mar 2023 13:41:32 -0600 Subject: [PATCH] chore: early merge clibase foundations (#6542) --- cli/clibase/clibase.go | 3 + cli/clibase/cmd.go | 371 ++++++++++++++++++++++++++++++++++++- cli/clibase/cmd_test.go | 368 ++++++++++++++++++++++++++++++++++++ cli/clibase/env.go | 33 +++- cli/clibase/env_test.go | 6 +- cli/clibase/option.go | 25 ++- cli/clibase/option_test.go | 6 +- cli/clibase/values.go | 24 +++ cli/server.go | 4 +- 9 files changed, 827 insertions(+), 13 deletions(-) create mode 100644 cli/clibase/cmd_test.go diff --git a/cli/clibase/clibase.go b/cli/clibase/clibase.go index 692ea5734c..cbca6b8143 100644 --- a/cli/clibase/clibase.go +++ b/cli/clibase/clibase.go @@ -5,6 +5,9 @@ // We will extend its usage to the rest of our application, completely replacing // cobra/viper. It's also a candidate to be broken out into its own open-source // library, so we avoid deep coupling with Coder concepts. +// +// The Command interface is loosely based on the chi middleware pattern and +// http.Handler/HandlerFunc. package clibase import ( diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go index d268e3b0bf..1b91f8e976 100644 --- a/cli/clibase/cmd.go +++ b/cli/clibase/cmd.go @@ -1,6 +1,15 @@ package clibase -import "strings" +import ( + "context" + "errors" + "io" + "os" + "strings" + + "github.com/spf13/pflag" + "golang.org/x/xerrors" +) // Cmd describes an executable command. type Cmd struct { @@ -10,13 +19,40 @@ type Cmd struct { Children []*Cmd // Use is provided in form "command [flags] [args...]". Use string + + // Aliases is a list of alternative names for the command. + Aliases []string + // Short is a one-line description of the command. Short string + + // Hidden determines whether the command should be hidden from help. + Hidden bool + + // RawArgs determines whether the command should receive unparsed arguments. + // No flags are parsed when set, and the command is responsible for parsing + // its own flags. + RawArgs bool + // Long is a detailed description of the command, // presented on its help page. It may contain examples. Long string Options OptionSet Annotations Annotations + + // Middleware is called before the Handler. + // Use Chain() to combine multiple middlewares. + Middleware MiddlewareFunc + Handler HandlerFunc + HelpHandler HandlerFunc +} + +// Walk calls fn for the command and all its children. +func (c *Cmd) Walk(fn func(*Cmd)) { + fn(c) + for _, child := range c.Children { + child.Walk(fn) + } } // Name returns the first word in the Use string. @@ -46,3 +82,336 @@ func (c *Cmd) FullUsage() string { uses = append(uses, c.Use) return strings.Join(uses, " ") } + +// Invoke creates a new invocation of the command, with +// stdio discarded. +// +// The returned invocation is not live until Run() is called. +func (c *Cmd) Invoke(args ...string) *Invocation { + return &Invocation{ + Command: c, + Args: args, + Stdout: io.Discard, + Stderr: io.Discard, + Stdin: strings.NewReader(""), + } +} + +// Invocation represents an instance of a command being executed. +type Invocation struct { + ctx context.Context + Command *Cmd + parsedFlags *pflag.FlagSet + Args []string + // Environ is a list of environment variables. Use EnvsWithPrefix to parse + // os.Environ. + Environ Environ + Stdout io.Writer + Stderr io.Writer + Stdin io.Reader +} + +// WithOS returns the invocation as a main package, filling in the invocation's unset +// fields with OS defaults. +func (i *Invocation) WithOS() *Invocation { + return i.with(func(i *Invocation) { + if i.Stdout == nil { + i.Stdout = os.Stdout + } + if i.Stderr == nil { + i.Stderr = os.Stderr + } + if i.Stdin == nil { + i.Stdin = os.Stdin + } + if i.Args == nil { + i.Args = os.Args[1:] + } + if i.Environ == nil { + i.Environ = ParseEnviron(os.Environ(), "") + } + }) +} + +func (i *Invocation) Context() context.Context { + if i.ctx == nil { + // Consider returning context.Background() instead? + panic("context not set, has WithContext() or Run() been called?") + } + return i.ctx +} + +func (i *Invocation) ParsedFlags() *pflag.FlagSet { + if i.parsedFlags == nil { + panic("flags not parsed, has Run() been called?") + } + return i.parsedFlags +} + +type runState struct { + allArgs []string + commandDepth int + + flagParseErr error +} + +// run recursively executes the command and its children. +// allArgs is wired through the stack so that global flags can be accepted +// anywhere in the command invocation. +func (i *Invocation) run(state *runState) error { + err := i.Command.Options.SetDefaults() + if err != nil { + return xerrors.Errorf("setting defaults: %w", err) + } + + err = i.Command.Options.ParseEnv(i.Environ) + if err != nil { + return xerrors.Errorf("parsing env: %w", err) + } + + // Now the fun part, argument parsing! + + children := make(map[string]*Cmd) + for _, child := range i.Command.Children { + for _, name := range append(child.Aliases, child.Name()) { + if _, ok := children[name]; ok { + return xerrors.Errorf("duplicate command name: %s", name) + } + children[name] = child + } + } + + if i.parsedFlags == nil { + i.parsedFlags = pflag.NewFlagSet(i.Command.Name(), pflag.ContinueOnError) + // We handle Usage ourselves. + i.parsedFlags.Usage = func() {} + } + + i.parsedFlags.AddFlagSet(i.Command.Options.FlagSet()) + + var parsedArgs []string + + if !i.Command.RawArgs { + // Flag parsing will fail on intermediate commands in the command tree, + // so we check the error after looking for a child command. + state.flagParseErr = i.parsedFlags.Parse(state.allArgs) + parsedArgs = i.parsedFlags.Args() + } + + // Run child command if found (next child only) + // We must do subcommand detection after flag parsing so we don't mistake flag + // values for subcommand names. + if len(parsedArgs) > 0 { + nextArg := parsedArgs[0] + if child, ok := children[nextArg]; ok { + child.Parent = i.Command + i.Command = child + state.commandDepth++ + err = i.run(state) + if err != nil { + return xerrors.Errorf( + "subcommand %s: %w", child.Name(), err, + ) + } + return nil + } + } + + // Flag parse errors are irrelevant for raw args commands. + if !i.Command.RawArgs && state.flagParseErr != nil && !errors.Is(state.flagParseErr, pflag.ErrHelp) { + return xerrors.Errorf( + "parsing flags (%v) for %q: %w", + state.allArgs, + i.Command.FullName(), state.flagParseErr, + ) + } + + if i.Command.RawArgs { + // If we're at the root command, then the name is omitted + // from the arguments, so we can just use the entire slice. + if state.commandDepth == 0 { + i.Args = state.allArgs + } else { + argPos, err := findArg(i.Command.Name(), state.allArgs, i.parsedFlags) + if err != nil { + panic(err) + } + i.Args = state.allArgs[argPos+1:] + } + } else { + // In non-raw-arg mode, we want to skip over flags. + i.Args = parsedArgs[state.commandDepth:] + } + + mw := i.Command.Middleware + if mw == nil { + mw = Chain() + } + + ctx := i.ctx + if ctx == nil { + ctx = context.Background() + } + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + i = i.WithContext(ctx) + + if i.Command.Handler == nil || errors.Is(state.flagParseErr, pflag.ErrHelp) { + if i.Command.HelpHandler == nil { + return xerrors.Errorf("no handler or help for command %s", i.Command.FullName()) + } + return i.Command.HelpHandler(i) + } + + err = mw(i.Command.Handler)(i) + if err != nil { + return xerrors.Errorf("running command %s: %w", i.Command.FullName(), err) + } + return nil +} + +// findArg returns the index of the first occurrence of arg in args, skipping +// over all flags. +func findArg(want string, args []string, fs *pflag.FlagSet) (int, error) { + for i := 0; i < len(args); i++ { + arg := args[i] + if !strings.HasPrefix(arg, "-") { + if arg == want { + return i, nil + } + continue + } + + // This is a flag! + if strings.Contains(arg, "=") { + // The flag contains the value in the same arg, just skip. + continue + } + + // We need to check if NoOptValue is set, then we should not wait + // for the next arg to be the value. + f := fs.Lookup(strings.TrimLeft(arg, "-")) + if f == nil { + return -1, xerrors.Errorf("unknown flag: %s", arg) + } + if f.NoOptDefVal != "" { + continue + } + + if i == len(args)-1 { + return -1, xerrors.Errorf("flag %s requires a value", arg) + } + + // Skip the value. + i++ + } + + return -1, xerrors.Errorf("arg %s not found", want) +} + +// Run executes the command. +// If two command share a flag name, the first command wins. +// +//nolint:revive +func (i *Invocation) Run() error { + return i.run(&runState{ + allArgs: i.Args, + }) +} + +// WithContext returns a copy of the Invocation with the given context. +func (i *Invocation) WithContext(ctx context.Context) *Invocation { + return i.with(func(i *Invocation) { + i.ctx = ctx + }) +} + +// with returns a copy of the Invocation with the given function applied. +func (i *Invocation) with(fn func(*Invocation)) *Invocation { + i2 := *i + fn(&i2) + return &i2 +} + +// MiddlewareFunc returns the next handler in the chain, +// or nil if there are no more. +type MiddlewareFunc func(next HandlerFunc) HandlerFunc + +func chain(ms ...MiddlewareFunc) MiddlewareFunc { + return MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + if len(ms) > 0 { + return chain(ms[1:]...)(ms[0](next)) + } + return next + }) +} + +// Chain returns a Handler that first calls middleware in order. +// +//nolint:revive +func Chain(ms ...MiddlewareFunc) MiddlewareFunc { + // We need to reverse the array to provide top-to-bottom execution + // order when defining a command. + reversed := make([]MiddlewareFunc, len(ms)) + for i := range ms { + reversed[len(ms)-1-i] = ms[i] + } + return chain(reversed...) +} + +func RequireNArgs(want int) MiddlewareFunc { + return RequireRangeArgs(want, want) +} + +// RequireRangeArgs returns a Middleware that requires the number of arguments +// to be between start and end (inclusive). If end is -1, then the number of +// arguments must be at least start. +func RequireRangeArgs(start, end int) MiddlewareFunc { + if start < 0 { + panic("start must be >= 0") + } + return func(next HandlerFunc) HandlerFunc { + return func(i *Invocation) error { + got := len(i.Args) + switch { + case start == end && got != start: + switch start { + case 0: + return xerrors.Errorf("wanted no args but got %v %v", got, i.Args) + default: + return xerrors.Errorf( + "wanted %v args but got %v %v", + start, + got, + i.Args, + ) + } + case start > 0 && end == -1: + switch { + case got < start: + return xerrors.Errorf( + "wanted at least %v args but got %v", + start, + got, + ) + default: + return next(i) + } + case start > end: + panic("start must be <= end") + case got < start || got > end: + return xerrors.Errorf( + "wanted between %v and %v args but got %v", + start, end, + got, + ) + default: + return next(i) + } + } + } +} + +// HandlerFunc handles an Invocation of a command. +type HandlerFunc func(i *Invocation) error diff --git a/cli/clibase/cmd_test.go b/cli/clibase/cmd_test.go new file mode 100644 index 0000000000..ac08b26837 --- /dev/null +++ b/cli/clibase/cmd_test.go @@ -0,0 +1,368 @@ +package clibase_test + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/cli/clibase" +) + +// ioBufs is the standard input, output, and error for a command. +type ioBufs struct { + Stdin bytes.Buffer + Stdout bytes.Buffer + Stderr bytes.Buffer +} + +// fakeIO sets Stdin, Stdout, and Stderr to buffers. +func fakeIO(i *clibase.Invocation) *ioBufs { + var b ioBufs + i.Stdout = &b.Stdout + i.Stderr = &b.Stderr + i.Stdin = &b.Stdin + return &b +} + +func TestCommand(t *testing.T) { + t.Parallel() + + cmd := func() *clibase.Cmd { + var ( + verbose bool + lower bool + prefix string + ) + return &clibase.Cmd{ + Use: "root [subcommand]", + Options: clibase.OptionSet{ + clibase.Option{ + Name: "verbose", + Flag: "verbose", + Value: clibase.BoolOf(&verbose), + }, + clibase.Option{ + Name: "prefix", + Flag: "prefix", + Value: clibase.StringOf(&prefix), + }, + }, + Children: []*clibase.Cmd{ + { + Use: "toupper [word]", + Short: "Converts a word to upper case", + Middleware: clibase.Chain( + clibase.RequireNArgs(1), + ), + Aliases: []string{"up"}, + Options: clibase.OptionSet{ + clibase.Option{ + Name: "lower", + Flag: "lower", + Value: clibase.BoolOf(&lower), + }, + }, + Handler: (func(i *clibase.Invocation) error { + i.Stdout.Write([]byte(prefix)) + w := i.Args[0] + if lower { + w = strings.ToLower(w) + } else { + w = strings.ToUpper(w) + } + _, _ = i.Stdout.Write( + []byte( + w, + ), + ) + if verbose { + i.Stdout.Write([]byte("!!!")) + } + return nil + }), + }, + }, + } + } + + t.Run("SimpleOK", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke("toupper", "hello") + io := fakeIO(i) + i.Run() + require.Equal(t, "HELLO", io.Stdout.String()) + }) + + t.Run("Alias", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "up", "hello", + ) + io := fakeIO(i) + i.Run() + require.Equal(t, "HELLO", io.Stdout.String()) + }) + + t.Run("NoSubcommand", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "na", + ) + io := fakeIO(i) + err := i.Run() + require.Empty(t, io.Stdout.String()) + require.Error(t, err) + }) + + t.Run("BadArgs", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "toupper", + ) + io := fakeIO(i) + err := i.Run() + require.Empty(t, io.Stdout.String()) + require.Error(t, err) + }) + + t.Run("UnknownFlags", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "toupper", "--unknown", + ) + io := fakeIO(i) + err := i.Run() + require.Empty(t, io.Stdout.String()) + require.Error(t, err) + }) + + t.Run("Verbose", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "--verbose", "toupper", "hello", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "HELLO!!!", io.Stdout.String()) + }) + + t.Run("Verbose=", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "--verbose=true", "toupper", "hello", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "HELLO!!!", io.Stdout.String()) + }) + + t.Run("PrefixSpace", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "--prefix", "conv: ", "toupper", "hello", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "conv: HELLO", io.Stdout.String()) + }) + + t.Run("GlobalFlagsAnywhere", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "toupper", "--prefix", "conv: ", "hello", "--verbose", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "conv: HELLO!!!", io.Stdout.String()) + }) + + t.Run("LowerVerbose", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "toupper", "--verbose", "hello", "--lower", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "hello!!!", io.Stdout.String()) + }) + + t.Run("ParsedFlags", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "toupper", "--verbose", "hello", "--lower", + ) + _ = fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, + "true", + i.ParsedFlags().Lookup("verbose").Value.String(), + ) + }) + + t.Run("NoDeepChild", func(t *testing.T) { + t.Parallel() + i := cmd().Invoke( + "root", "level", "level", "toupper", "--verbose", "hello", "--lower", + ) + fio := fakeIO(i) + require.Error(t, i.Run(), fio.Stdout.String()) + }) +} + +func TestCommand_MiddlewareOrder(t *testing.T) { + t.Parallel() + + mw := func(letter string) clibase.MiddlewareFunc { + return func(next clibase.HandlerFunc) clibase.HandlerFunc { + return (func(i *clibase.Invocation) error { + _, _ = i.Stdout.Write([]byte(letter)) + return next(i) + }) + } + } + + cmd := &clibase.Cmd{ + Use: "toupper [word]", + Short: "Converts a word to upper case", + Middleware: clibase.Chain( + mw("A"), + mw("B"), + mw("C"), + ), + Handler: (func(i *clibase.Invocation) error { + return nil + }), + } + + i := cmd.Invoke( + "hello", "world", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "ABC", io.Stdout.String()) +} + +func TestCommand_RawArgs(t *testing.T) { + t.Parallel() + + cmd := func() *clibase.Cmd { + return &clibase.Cmd{ + Use: "root", + Options: []clibase.Option{ + { + Name: "password", + Flag: "password", + Value: clibase.StringOf(new(string)), + }, + }, + Children: []*clibase.Cmd{ + { + Use: "sushi ", + Short: "Throws back raw output", + RawArgs: true, + Handler: (func(i *clibase.Invocation) error { + if v := i.ParsedFlags().Lookup("password").Value.String(); v != "codershack" { + return xerrors.Errorf("password %q is wrong!", v) + } + i.Stdout.Write([]byte(strings.Join(i.Args, " "))) + return nil + }), + }, + }, + } + } + + t.Run("OK", func(t *testing.T) { + // Flag parsed before the raw arg command should still work. + t.Parallel() + + i := cmd().Invoke( + "--password", "codershack", "sushi", "hello", "--verbose", "world", + ) + io := fakeIO(i) + require.NoError(t, i.Run()) + require.Equal(t, "hello --verbose world", io.Stdout.String()) + }) + + t.Run("BadFlag", func(t *testing.T) { + // Verbose before the raw arg command should fail. + t.Parallel() + + i := cmd().Invoke( + "--password", "codershack", "--verbose", "sushi", "hello", "world", + ) + io := fakeIO(i) + require.Error(t, i.Run()) + require.Empty(t, io.Stdout.String()) + }) + + t.Run("NoPassword", func(t *testing.T) { + // Flag parsed before the raw arg command should still work. + t.Parallel() + i := cmd().Invoke( + "sushi", "hello", "--verbose", "world", + ) + _ = fakeIO(i) + require.Error(t, i.Run()) + }) +} + +func TestCommand_RootRaw(t *testing.T) { + t.Parallel() + cmd := &clibase.Cmd{ + RawArgs: true, + Handler: func(i *clibase.Invocation) error { + i.Stdout.Write([]byte(strings.Join(i.Args, " "))) + return nil + }, + } + + inv := cmd.Invoke("hello", "--verbose", "--friendly") + stdio := fakeIO(inv) + err := inv.Run() + require.NoError(t, err) + + require.Equal(t, "hello --verbose --friendly", stdio.Stdout.String()) +} + +func TestCommand_HyphenHyphen(t *testing.T) { + t.Parallel() + cmd := &clibase.Cmd{ + Handler: (func(i *clibase.Invocation) error { + i.Stdout.Write([]byte(strings.Join(i.Args, " "))) + return nil + }), + } + + inv := cmd.Invoke("--", "--verbose", "--friendly") + stdio := fakeIO(inv) + err := inv.Run() + require.NoError(t, err) + + require.Equal(t, "--verbose --friendly", stdio.Stdout.String()) +} + +func TestCommand_ContextCancels(t *testing.T) { + t.Parallel() + + var gotCtx context.Context + + cmd := &clibase.Cmd{ + Handler: (func(i *clibase.Invocation) error { + gotCtx = i.Context() + if err := gotCtx.Err(); err != nil { + return xerrors.Errorf("unexpected context error: %w", i.Context().Err()) + } + return nil + }), + } + + err := cmd.Invoke().Run() + require.NoError(t, err) + + require.Error(t, gotCtx.Err()) +} diff --git a/cli/clibase/env.go b/cli/clibase/env.go index 0a5e19e0ff..1a73f66a34 100644 --- a/cli/clibase/env.go +++ b/cli/clibase/env.go @@ -25,9 +25,38 @@ type EnvVar struct { Value string } -// EnvsWithPrefix returns all environment variables starting with +type Environ []EnvVar + +func (e Environ) ToOS() []string { + var env []string + for _, v := range e { + env = append(env, v.Name+"="+v.Value) + } + return env +} + +func (e Environ) Lookup(name string) (string, bool) { + for _, v := range e { + if v.Name == name { + return v.Value, true + } + } + return "", false +} + +func (e *Environ) Set(name, value string) { + for i, v := range *e { + if v.Name == name { + (*e)[i].Value = value + return + } + } + *e = append(*e, EnvVar{Name: name, Value: value}) +} + +// ParseEnviron returns all environment variables starting with // prefix without said prefix. -func EnvsWithPrefix(environ []string, prefix string) []EnvVar { +func ParseEnviron(environ []string, prefix string) Environ { var filtered []EnvVar for _, line := range environ { name := envName(line) diff --git a/cli/clibase/env_test.go b/cli/clibase/env_test.go index 55fc0c6efd..d8830e64f5 100644 --- a/cli/clibase/env_test.go +++ b/cli/clibase/env_test.go @@ -16,7 +16,7 @@ func TestFilterNamePrefix(t *testing.T) { tests := []struct { name string args args - want []clibase.EnvVar + want clibase.Environ }{ {"empty", args{[]string{}, "SHIRE"}, nil}, { @@ -36,8 +36,8 @@ func TestFilterNamePrefix(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := clibase.EnvsWithPrefix(tt.args.environ, tt.args.prefix); !reflect.DeepEqual(got, tt.want) { - t.Errorf("EnvsWithPrefix() = %v, want %v", got, tt.want) + if got := clibase.ParseEnviron(tt.args.environ, tt.args.prefix); !reflect.DeepEqual(got, tt.want) { + t.Errorf("FilterNamePrefix() = %v, want %v", got, tt.want) } }) } diff --git a/cli/clibase/option.go b/cli/clibase/option.go index 3fd910441a..76fd4f5111 100644 --- a/cli/clibase/option.go +++ b/cli/clibase/option.go @@ -58,6 +58,10 @@ func (s *OptionSet) Add(opts ...Option) { // FlagSet returns a pflag.FlagSet for the OptionSet. func (s *OptionSet) FlagSet() *pflag.FlagSet { + if s == nil { + return &pflag.FlagSet{} + } + fs := pflag.NewFlagSet("", pflag.ContinueOnError) for _, opt := range *s { if opt.Flag == "" { @@ -71,11 +75,16 @@ func (s *OptionSet) FlagSet() *pflag.FlagSet { } } + val := opt.Value + if val == nil { + val = &DiscardValue{} + } + fs.AddFlag(&pflag.Flag{ Name: opt.Flag, Shorthand: opt.FlagShorthand, Usage: opt.Description, - Value: opt.Value, + Value: val, DefValue: "", Changed: false, Deprecated: "", @@ -90,14 +99,19 @@ func (s *OptionSet) FlagSet() *pflag.FlagSet { } // ParseEnv parses the given environment variables into the OptionSet. -func (s *OptionSet) ParseEnv(globalPrefix string, environ []string) error { +// Use EnvsWithPrefix to filter out prefixes. +func (s *OptionSet) ParseEnv(vs []EnvVar) error { + if s == nil { + return nil + } + var merr *multierror.Error // We parse environment variables first instead of using a nested loop to // avoid N*M complexity when there are a lot of options and environment // variables. envs := make(map[string]string) - for _, v := range EnvsWithPrefix(environ, globalPrefix) { + for _, v := range vs { envs[v.Name] = v.Value } @@ -129,7 +143,12 @@ func (s *OptionSet) ParseEnv(globalPrefix string, environ []string) error { // SetDefaults sets the default values for each Option. // It should be called before all parsing (e.g. ParseFlags, ParseEnv). func (s *OptionSet) SetDefaults() error { + if s == nil { + return nil + } + var merr *multierror.Error + for _, opt := range *s { if opt.Default == "" { continue diff --git a/cli/clibase/option_test.go b/cli/clibase/option_test.go index 77ede24270..7b3702d837 100644 --- a/cli/clibase/option_test.go +++ b/cli/clibase/option_test.go @@ -87,7 +87,9 @@ func TestOptionSet_ParseEnv(t *testing.T) { }, } - err := os.ParseEnv("CODER_", []string{"CODER_WORKSPACE_NAME=foo"}) + err := os.ParseEnv([]clibase.EnvVar{ + {Name: "WORKSPACE_NAME", Value: "foo"}, + }) require.NoError(t, err) require.EqualValues(t, "foo", workspaceName) }) @@ -109,7 +111,7 @@ func TestOptionSet_ParseEnv(t *testing.T) { err := os.SetDefaults() require.NoError(t, err) - err = os.ParseEnv("CODER_", []string{"CODER_WORKSPACE_NAME="}) + err = os.ParseEnv(clibase.ParseEnviron([]string{"CODER_WORKSPACE_NAME="}, "CODER_")) require.NoError(t, err) require.EqualValues(t, "defname", workspaceName) }) diff --git a/cli/clibase/values.go b/cli/clibase/values.go index 80113712c8..7fdfd6b730 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -28,6 +28,10 @@ type NoOptDefValuer interface { type Int64 int64 +func Int64Of(i *int64) *Int64 { + return (*Int64)(i) +} + func (i *Int64) Set(s string) error { ii, err := strconv.ParseInt(s, 10, 64) *i = Int64(ii) @@ -48,6 +52,10 @@ func (Int64) Type() string { type Bool bool +func BoolOf(b *bool) *Bool { + return (*Bool)(b) +} + func (b *Bool) Set(s string) error { if s == "" { *b = Bool(false) @@ -76,6 +84,10 @@ func (Bool) Type() string { type String string +func StringOf(s *string) *String { + return (*String)(s) +} + func (*String) NoOptDefValue() string { return "" } @@ -102,6 +114,10 @@ var _ pflag.SliceValue = &Strings{} // Strings is a slice of strings that implements pflag.Value and pflag.SliceValue. type Strings []string +func StringsOf(ss *[]string) *Strings { + return (*Strings)(ss) +} + func (s *Strings) Append(v string) error { *s = append(*s, v) return nil @@ -152,6 +168,10 @@ func (Strings) Type() string { type Duration time.Duration +func DurationOf(d *time.Duration) *Duration { + return (*Duration)(d) +} + func (d *Duration) Set(v string) error { dd, err := time.ParseDuration(v) *d = Duration(dd) @@ -185,6 +205,10 @@ func (Duration) Type() string { type URL url.URL +func URLOf(u *url.URL) *URL { + return (*URL)(u) +} + func (u *URL) Set(v string) error { uu, err := url.Parse(v) if err != nil { diff --git a/cli/server.go b/cli/server.go index d13e1dda91..76997b2ea6 100644 --- a/cli/server.go +++ b/cli/server.go @@ -95,7 +95,7 @@ func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.GitAuthConfig, er sort.Strings(environ) var providers []codersdk.GitAuthConfig - for _, v := range clibase.EnvsWithPrefix(environ, envPrefix+"GITAUTH_") { + for _, v := range clibase.ParseEnviron(environ, envPrefix+"GITAUTH_") { tokens := strings.SplitN(v.Name, "_", 2) if len(tokens) != 2 { return nil, xerrors.Errorf("invalid env var: %s", v.Name) @@ -185,7 +185,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close return xerrors.Errorf("set defaults: %w", err) } - err = cliOpts.ParseEnv(envPrefix, os.Environ()) + err = cliOpts.ParseEnv(clibase.ParseEnviron(os.Environ(), envPrefix)) if err != nil { return xerrors.Errorf("parse env: %w", err) }