Add YAML to core clibase parsing

coder · ammario · Apr 7, 2023 · Mar 30, 2023 · Mar 30, 2023 · Mar 30, 2023
commit 7edea99f8a679761fabbca4abe280370f8ea10f7
diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go
@@ -13,6 +13,7 @@ import (
 	"github.com/spf13/pflag"
 	"golang.org/x/exp/slices"
 	"golang.org/x/xerrors"
+	"gopkg.in/yaml.v3"
 )
 
 // Cmd describes an executable command.
@@ -262,17 +263,42 @@ func (inv *Invocation) run(state *runState) error {
 		parsedArgs = inv.parsedFlags.Args()
 	}
 
-	// Set defaults for flags that weren't set by the user.
-	skipDefaults := make(map[int]struct{}, len(inv.Command.Options))
+	// Set value sources for flags.
 	for i, opt := range inv.Command.Options {
 		if fl := inv.parsedFlags.Lookup(opt.Flag); fl != nil && fl.Changed {
-			skipDefaults[i] = struct{}{}
+			inv.Command.Options[i].ValueSource = ValueSourceFlag
 		}
-		if opt.envChanged {
-			skipDefaults[i] = struct{}{}
+	}
+
+	// Read configs, if any.
+	for _, opt := range inv.Command.Options {
+		path, ok := opt.Value.(*YAMLConfigPath)
+		if !ok || path.String() == "" {
+			continue
+		}
+
+		fi, err := os.OpenFile(path.String(), os.O_RDONLY, 0)
+		if err != nil {
+			return xerrors.Errorf("opening config file: %w", err)
+		}
+		//nolint:revive
+		defer fi.Close()
+
+		dec := yaml.NewDecoder(fi)
+
+		var n yaml.Node
+		err = dec.Decode(&n)
+		if err != nil {
+			return xerrors.Errorf("decoding config: %w", err)
+		}
+
+		err = inv.Command.Options.FromYAML(&n)
+		if err != nil {
+			return xerrors.Errorf("applying config: %w", err)
 		}
 	}
-	err = inv.Command.Options.SetDefaults(skipDefaults)
+
+	err = inv.Command.Options.SetDefaults()
 	if err != nil {
 		return xerrors.Errorf("setting defaults: %w", err)
 	}

diff --git a/cli/clibase/cmd_test.go b/cli/clibase/cmd_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -555,42 +556,80 @@ func TestCommand_EmptySlice(t *testing.T) {
 func TestCommand_DefaultsOverride(t *testing.T) {
 	t.Parallel()
 
-	var got string
-	cmd := &clibase.Cmd{
-		Options: clibase.OptionSet{
-			{
-				Name:    "url",
-				Flag:    "url",
-				Default: "def.com",
-				Env:     "URL",
-				Value:   clibase.StringOf(&got),
-			},
-		},
-		Handler: (func(i *clibase.Invocation) error {
-			_, _ = fmt.Fprintf(i.Stdout, "%s", got)
-			return nil
-		}),
+	test := func(name string, want string, fn func(t *testing.T, inv *clibase.Invocation)) {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var (
+				got    string
+				config clibase.YAMLConfigPath
+			)
+			cmd := &clibase.Cmd{
+				Options: clibase.OptionSet{
+					{
+						Name:    "url",
+						Flag:    "url",
+						Default: "def.com",
+						Env:     "URL",
+						Value:   clibase.StringOf(&got),
+						YAML:    "url",
+					},
+					{
+						Name:    "config",
+						Flag:    "config",
+						Default: "",
+						Value:   &config,
+					},
+				},
+				Handler: (func(i *clibase.Invocation) error {
+					_, _ = fmt.Fprintf(i.Stdout, "%s", got)
+					return nil
+				}),
+			}
+
+			inv := cmd.Invoke()
+			stdio := fakeIO(inv)
+			fn(t, inv)
+			err := inv.Run()
+			require.NoError(t, err)
+			require.Equal(t, want, stdio.Stdout.String())
+		})
 	}
 
-	// Base case
-	inv := cmd.Invoke()
-	stdio := fakeIO(inv)
-	err := inv.Run()
-	require.NoError(t, err)
-	require.Equal(t, "def.com", stdio.Stdout.String())
+	test("DefaultOverNothing", "def.com", func(t *testing.T, inv *clibase.Invocation) {})
 
-	// Flag overrides
-	inv = cmd.Invoke("--url", "good.com")
-	stdio = fakeIO(inv)
-	err = inv.Run()
-	require.NoError(t, err)
-	require.Equal(t, "good.com", stdio.Stdout.String())
+	test("FlagOverDefault", "good.com", func(t *testing.T, inv *clibase.Invocation) {
+		inv.Args = []string{"--url", "good.com"}
+	})
 
-	// Env overrides
-	inv = cmd.Invoke()
-	inv.Environ.Set("URL", "good.com")
-	stdio = fakeIO(inv)
-	err = inv.Run()
-	require.NoError(t, err)
-	require.Equal(t, "good.com", stdio.Stdout.String())
+	test("EnvOverDefault", "good.com", func(t *testing.T, inv *clibase.Invocation) {
+		inv.Environ.Set("URL", "good.com")
+	})
+
+	test("FlagOverEnv", "good.com", func(t *testing.T, inv *clibase.Invocation) {
+		inv.Environ.Set("URL", "bad.com")
+		inv.Args = []string{"--url", "good.com"}
+	})
+
+	test("FlagOverYAML", "good.com", func(t *testing.T, inv *clibase.Invocation) {
+		fi, err := os.CreateTemp(t.TempDir(), "config.yaml")
+		require.NoError(t, err)
+		defer fi.Close()
+
+		_, err = fi.WriteString("url: bad.com")
+		require.NoError(t, err)
+
+		inv.Args = []string{"--config", fi.Name(), "--url", "good.com"}
+	})
+
+	test("YAMLOverDefault", "good.com", func(t *testing.T, inv *clibase.Invocation) {
+		fi, err := os.CreateTemp(t.TempDir(), "config.yaml")
+		require.NoError(t, err)
+		defer fi.Close()
+
+		_, err = fi.WriteString("url: good.com")
+		require.NoError(t, err)
+
+		inv.Args = []string{"--config", fi.Name()}
+	})
 }
diff --git a/cli/clibase/option.go b/cli/clibase/option.go
@@ -8,6 +8,16 @@ import (
 	"golang.org/x/xerrors"
 )
 
+type ValueSource string
+
+const (
+	ValueSourceNone    ValueSource = ""
+	ValueSourceFlag    ValueSource = "flag"
+	ValueSourceEnv     ValueSource = "env"
+	ValueSourceYAML    ValueSource = "yaml"
+	ValueSourceDefault ValueSource = "default"
+)
+
 // Option is a configuration option for a CLI application.
 type Option struct {
 	Name        string `json:"name,omitempty"`
@@ -47,7 +57,7 @@ type Option struct {
 
 	Hidden bool `json:"hidden,omitempty"`
 
-	envChanged bool
+	ValueSource ValueSource
 }
 
 // OptionSet is a group of options that can be applied to a command.
@@ -135,8 +145,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 			continue
 		}
 
-		opt.envChanged = true
-		(*s)[i] = opt
+		(*s)[i].ValueSource = ValueSourceEnv
 		if err := opt.Value.Set(envVal); err != nil {
 			merr = multierror.Append(
 				merr, xerrors.Errorf("parse %q: %w", opt.Name, err),
@@ -148,8 +157,8 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 }
 
 // SetDefaults sets the default values for each Option, skipping values
-// that have already been set as indicated by the skip map.
-func (s *OptionSet) SetDefaults(skip map[int]struct{}) error {
+// that already have a value source.
+func (s *OptionSet) SetDefaults() error {
 	if s == nil {
 		return nil
 	}
@@ -158,10 +167,8 @@ func (s *OptionSet) SetDefaults(skip map[int]struct{}) error {
 
 	for i, opt := range *s {
 		// Skip values that may have already been set by the user.
-		if len(skip) > 0 {
-			if _, ok := skip[i]; ok {
-				continue
-			}
+		if opt.ValueSource != ValueSourceNone {
+			continue
 		}
 
 		if opt.Default == "" {
@@ -178,6 +185,7 @@ func (s *OptionSet) SetDefaults(skip map[int]struct{}) error {
 			)
 			continue
 		}
+		(*s)[i].ValueSource = ValueSourceDefault
 		if err := opt.Value.Set(opt.Default); err != nil {
 			merr = multierror.Append(
 				merr, xerrors.Errorf("parse %q: %w", opt.Name, err),

diff --git a/cli/clibase/option_test.go b/cli/clibase/option_test.go
@@ -49,7 +49,7 @@ func TestOptionSet_ParseFlags(t *testing.T) {
 			},
 		}
 
-		err := os.SetDefaults(nil)
+		err := os.SetDefaults()
 		require.NoError(t, err)
 
 		err = os.FlagSet().Parse([]string{"--name", "foo", "--name", "bar"})
@@ -111,7 +111,7 @@ func TestOptionSet_ParseEnv(t *testing.T) {
 			},
 		}
 
-		err := os.SetDefaults(nil)
+		err := os.SetDefaults()
 		require.NoError(t, err)
 
 		err = os.ParseEnv(clibase.ParseEnviron([]string{"CODER_WORKSPACE_NAME="}, "CODER_"))

diff --git a/cli/clibase/yaml.go b/cli/clibase/yaml.go
@@ -181,7 +181,7 @@ func fromYAML(os OptionSet, ofGroup *Group, n *yaml.Node) error {
 			continue
 		}
 
-		if opt.Group.YAML == "" {
+		if opt.Group != nil && opt.Group.YAML == "" {
 			return xerrors.Errorf("group yaml name is empty for %q", opt.Name)
 		}
 
@@ -213,10 +213,18 @@ func fromYAML(os OptionSet, ofGroup *Group, n *yaml.Node) error {
 			continue
 		}
 
+		opt, foundOpt := optionsByName[name]
+		if foundOpt {
+			if opt.ValueSource != ValueSourceNone {
+				continue
+			}
+			opt.ValueSource = ValueSourceYAML
+		}
+
 		switch item.Kind {
 		case yaml.MappingNode:
 			// Item is either a group or an option with a complex object.
-			if opt, ok := optionsByName[name]; ok {
+			if foundOpt {
 				unmarshaler, ok := opt.Value.(yaml.Unmarshaler)
 				if !ok {
 					return xerrors.Errorf("complex option %q must support unmarshaling", opt.Name)
@@ -237,8 +245,7 @@ func fromYAML(os OptionSet, ofGroup *Group, n *yaml.Node) error {
 			}
 			merr = errors.Join(merr, xerrors.Errorf("unknown option or subgroup %q", name))
 		case yaml.ScalarNode, yaml.SequenceNode:
-			opt, ok := optionsByName[name]
-			if !ok {
+			if !foundOpt {
 				merr = errors.Join(merr, xerrors.Errorf("unknown option %q", name))
 				continue
 			}

diff --git a/cli/clibase/yaml_test.go b/cli/clibase/yaml_test.go
@@ -41,12 +41,12 @@ func TestOptionSet_YAML(t *testing.T) {
 				Value:       &workspaceName,
 				Default:     "billie",
 				Description: "The workspace's name.",
-				Group:       &clibase.Group{Name: "Names"},
+				Group:       &clibase.Group{YAML: "names"},
 				YAML:        "workspaceName",
 			},
 		}
 
-		err := os.SetDefaults(nil)
+		err := os.SetDefaults()
 		require.NoError(t, err)
 
 		n, err := os.ToYAML()
@@ -139,6 +139,7 @@ func TestOptionSet_YAMLIsomorphism(t *testing.T) {
 			os2 := slices.Clone(tc.os)
 			for i := range os2 {
 				os2[i].Value = tc.zeroValue()
+				os2[i].ValueSource = clibase.ValueSourceNone
 			}
 
 			// os2 values should be zeroed whereas tc.os should be
@@ -148,6 +149,11 @@ func TestOptionSet_YAMLIsomorphism(t *testing.T) {
 			err = os2.FromYAML(&y2)
 			require.NoError(t, err)
 
+			want := tc.os
+			for i := range want {
+				want[i].ValueSource = clibase.ValueSourceYAML
+			}
+
 			require.Equal(t, tc.os, os2)
 		})
 	}

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -1090,7 +1090,7 @@ QastnN77KfUwdj3SJt44U/uh1jAIv4oSLBr8HYUkbnI8
 func DeploymentValues(t *testing.T) *codersdk.DeploymentValues {
 	var cfg codersdk.DeploymentValues
 	opts := cfg.Options()
-	err := opts.SetDefaults(nil)
+	err := opts.SetDefaults()
 	require.NoError(t, err)
 	return &cfg
 }
diff --git a/go.mod b/go.mod
@@ -262,7 +262,6 @@ require (
 	github.com/hashicorp/terraform-plugin-log v0.7.0 // indirect
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.20.0 // indirect
 	github.com/hdevalence/ed25519consensus v0.0.0-20220222234857-c00d1f31bab3 // indirect
-	github.com/iancoleman/strcase v0.2.0
 	github.com/illarion/gonotify v1.0.1 // indirect
 	github.com/imdario/mergo v0.3.13 // indirect
 	github.com/insomniacslk/dhcp v0.0.0-20221215072855-de60144f33f8 // indirect

diff --git a/go.sum b/go.sum
@@ -2,8 +2,6 @@
 bazil.org/fuse v0.0.0-20160811212531-371fbbdaa898/go.mod h1:Xbm+BRKSBEpa4q4hTSxohYNQpsxXPbPry4JJWOB3LB8=
 bazil.org/fuse v0.0.0-20200407214033-5883e5a4b512/go.mod h1:FbcW6z/2VytnFDhZfumh8Ss8zxHE6qpMP5sHTRe0EaM=
 bitbucket.org/creachadair/shell v0.0.6/go.mod h1:8Qqi/cYk7vPnsOePHroKXDJYmb5x7ENhtiFtfZq8K+M=
-cdr.dev/slog v1.4.2-0.20230228204227-60d22dceaf04 h1:d5MQ+iI2zk7t0HrHwBP9p7k2XfRsXnRclSe8Kpp3xOo=
-cdr.dev/slog v1.4.2-0.20230228204227-60d22dceaf04/go.mod h1:YPVZsUbRMaLaPgme0RzlPWlC7fI7YmDj/j/kZLuvICs=
 cdr.dev/slog v1.4.2 h1:fIfiqASYQFJBZiASwL825atyzeA96NsqSxx2aL61P8I=
 cdr.dev/slog v1.4.2/go.mod h1:0EkH+GkFNxizNR+GAXUEdUHanxUH5t9zqPILmPM/Vn8=
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
@@ -1093,7 +1091,6 @@ github.com/huandu/xstrings v1.3.1/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq
 github.com/huandu/xstrings v1.3.2/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
 github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714/go.mod h1:2Goc3h8EklBH5mspfHFxBnEoURQCGzQQH1ga9Myjvis=
 github.com/iancoleman/orderedmap v0.2.0 h1:sq1N/TFpYH++aViPcaKjys3bDClUEU7s5B+z6jq8pNA=
-github.com/iancoleman/strcase v0.2.0 h1:05I4QRnGpI0m37iZQRuskXh+w77mr6Z41lwQzuHLwW0=
 github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
 github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=