Add config file support independent of CLI framework

Signed-off-by: Darren Shepherd <darren@rancher.com>

cmd/agent/main.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/rancher/k3s/pkg/cli/agent"
 	"github.com/rancher/k3s/pkg/cli/cmds"
+	"github.com/rancher/k3s/pkg/configfilearg"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -15,7 +16,7 @@ func main() {
 		cmds.NewAgentCommand(agent.Run),
 	}
 
-	err := app.Run(os.Args)
+	err := app.Run(configfilearg.MustParse(os.Args))
 	if err != nil {
 		logrus.Fatal(err)
 	}

cmd/server/main.go

@@ -12,6 +12,7 @@ import (
 	"github.com/rancher/k3s/pkg/cli/ctr"
 	"github.com/rancher/k3s/pkg/cli/kubectl"
 	"github.com/rancher/k3s/pkg/cli/server"
+	"github.com/rancher/k3s/pkg/configfilearg"
 	"github.com/rancher/k3s/pkg/containerd"
 	ctr2 "github.com/rancher/k3s/pkg/ctr"
 	kubectl2 "github.com/rancher/k3s/pkg/kubectl"
@@ -43,7 +44,7 @@ func main() {
 		cmds.NewCtrCommand(ctr.Run),
 	}
 
-	err := app.Run(os.Args)
+	err := app.Run(configfilearg.MustParse(os.Args))
 	if err != nil {
 		logrus.Fatal(err)
 	}

main.go

@@ -14,6 +14,7 @@ import (
 	"github.com/rancher/k3s/pkg/cli/crictl"
 	"github.com/rancher/k3s/pkg/cli/kubectl"
 	"github.com/rancher/k3s/pkg/cli/server"
+	"github.com/rancher/k3s/pkg/configfilearg"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -27,7 +28,7 @@ func main() {
 		cmds.NewCRICTL(crictl.Run),
 	}
 
-	if err := app.Run(os.Args); err != nil {
+	if err := app.Run(configfilearg.MustParse(os.Args)); err != nil {
 		logrus.Fatal(err)
 	}
 }

pkg/cli/cmds/agent.go

@@ -172,9 +172,11 @@ func NewAgentCommand(action func(ctx *cli.Context) error) cli.Command {
 		Name:      "agent",
 		Usage:     "Run node agent",
 		UsageText: appName + " agent [OPTIONS]",
-		Before:    CheckSELinuxFlags,
+		Before:    SetupDebug(CheckSELinuxFlags),
 		Action:    action,
 		Flags: []cli.Flag{
+			ConfigFlag,
+			DebugFlag,
 			VLevel,
 			VModule,
 			LogFile,

pkg/cli/cmds/config.go

@@ -0,0 +1,17 @@
+package cmds
+
+import (
+	"github.com/rancher/k3s/pkg/version"
+	"github.com/urfave/cli"
+)
+
+var (
+	// ConfigFlag is here to show to the user, but the actually processing is done by configfileargs before
+	// call urfave
+	ConfigFlag = cli.StringFlag{
+		Name:   "config,c",
+		Usage:  "(config) Load configuration from `FILE`",
+		EnvVar: "K3S_CONFIG_FILE",
+		Value:  "/etc/rancher/" + version.Program + "/config.yaml",
+	}
+)

pkg/cli/cmds/root.go

@@ -10,7 +10,13 @@ import (
 )
 
 var (
-	Debug bool
+	Debug     bool
+	DebugFlag = cli.BoolFlag{
+		Name:        "debug",
+		Usage:       "Turn on debug logs",
+		Destination: &Debug,
+		EnvVar:      version.ProgramUpper + "_DEBUG",
+	}
 )
 
 func init() {
@@ -29,20 +35,21 @@ func NewApp() *cli.App {
 		fmt.Printf("%s version %s\n", app.Name, app.Version)
 	}
 	app.Flags = []cli.Flag{
-		cli.BoolFlag{
-			Name:        "debug",
-			Usage:       "Turn on debug logs",
-			Destination: &Debug,
-			EnvVar:      version.ProgramUpper + "_DEBUG",
-		},
-	}
-
-	app.Before = func(ctx *cli.Context) error {
-		if Debug {
-			logrus.SetLevel(logrus.DebugLevel)
-		}
-		return nil
+		DebugFlag,
 	}
+	app.Before = SetupDebug(nil)
 
 	return app
 }
+
+func SetupDebug(next func(ctx *cli.Context) error) func(ctx *cli.Context) error {
+	return func(ctx *cli.Context) error {
+		if Debug {
+			logrus.SetLevel(logrus.DebugLevel)
+		}
+		if next != nil {
+			return next(ctx)
+		}
+		return nil
+	}
+}

pkg/cli/cmds/server.go

@@ -74,7 +74,7 @@ func NewServerCommand(action func(*cli.Context) error) cli.Command {
 		Name:      "server",
 		Usage:     "Run management server",
 		UsageText: appName + " server [OPTIONS]",
-		Before:    CheckSELinuxFlags,
+		Before:    SetupDebug(CheckSELinuxFlags),
 		Action:    action,
 		Flags: []cli.Flag{
 			VLevel,

pkg/configfilearg/defaultparser.go

@@ -0,0 +1,20 @@
+package configfilearg
+
+import (
+	"github.com/rancher/k3s/pkg/version"
+	"github.com/sirupsen/logrus"
+)
+
+func MustParse(args []string) []string {
+	parser := &Parser{
+		After:         []string{"server", "agent"},
+		FlagNames:     []string{"--config", "-c"},
+		EnvName:       version.ProgramUpper + "_CONFIG_FILE",
+		DefaultConfig: "/etc/rancher/" + version.Program + "/config.yaml",
+	}
+	result, err := parser.Parse(args)
+	if err != nil {
+		logrus.Fatal(err)
+	}
+	return result
+}

pkg/configfilearg/parser.go

@@ -0,0 +1,139 @@
+package configfilearg
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"os"
+	"strings"
+
+	"github.com/rancher/wrangler/pkg/data/convert"
+	"gopkg.in/yaml.v2"
+)
+
+type Parser struct {
+	After         []string
+	FlagNames     []string
+	EnvName       string
+	DefaultConfig string
+}
+
+// Parser will parse an os.Args style slice looking for Parser.FlagNames after Parse.After.
+// It will read the parameter value of Parse.FlagNames and read the file, appending all flags directly after
+// the Parser.After value. This means a the non-config file flags will override, or if a slice append to, the config
+// file values.
+// If Parser.DefaultConfig is set, the existence of the config file is optional if not set in the os.Args. This means
+// if Parser.DefaultConfig is set we will always try to read the config file but only fail if it's not found if the
+// args contains Parser.FlagNames
+func (p *Parser) Parse(args []string) ([]string, error) {
+	prefix, suffix, found := p.findStart(args)
+	if !found {
+		return args, nil
+	}
+
+	configFile, isSet := p.findConfigFileFlag(args)
+	if configFile != "" {
+		values, err := readConfigFile(configFile)
+		if !isSet && os.IsNotExist(err) {
+			return args, nil
+		} else if err != nil {
+			return nil, err
+		}
+		return append(prefix, append(values, suffix...)...), nil
+	}
+
+	return args, nil
+}
+
+func (p *Parser) findConfigFileFlag(args []string) (string, bool) {
+	if envVal := os.Getenv(p.EnvName); p.EnvName != "" && envVal != "" {
+		return envVal, true
+	}
+
+	for i, arg := range args {
+		for _, flagName := range p.FlagNames {
+			if flagName == arg {
+				if len(args) > i+1 {
+					return args[i+1], true
+				}
+				// This is actually invalid, so we rely on the CLI parser after the fact flagging it as bad
+				return "", false
+			} else if strings.HasPrefix(arg, flagName+"=") {
+				return arg[len(flagName)+1:], true
+			}
+		}
+	}
+
+	return p.DefaultConfig, false
+}
+
+func (p *Parser) findStart(args []string) ([]string, []string, bool) {
+	if len(p.After) == 0 {
+		return []string{}, args, true
+	}
+
+	for i, val := range args {
+		for _, test := range p.After {
+			if val == test {
+				return args[0 : i+1], args[i+1:], true
+			}
+		}
+	}
+
+	return args, nil, false
+}
+
+func readConfigFile(file string) (result []string, _ error) {
+	bytes, err := readConfigFileData(file)
+	if err != nil {
+		return nil, err
+	}
+
+	data := yaml.MapSlice{}
+	if err := yaml.Unmarshal(bytes, &data); err != nil {
+		return nil, err
+	}
+
+	for _, i := range data {
+		k, v := convert.ToString(i.Key), i.Value
+		prefix := "--"
+		if len(k) == 1 {
+			prefix = "-"
+		}
+
+		if slice, ok := v.([]interface{}); ok {
+			for _, v := range slice {
+				result = append(result, prefix+k, convert.ToString(v))
+				result = append(result)
+			}
+		} else {
+			str := convert.ToString(v)
+			result = append(result, prefix+k)
+			if str != "" {
+				result = append(result, str)
+			}
+		}
+	}
+
+	return
+}
+
+func readConfigFileData(file string) ([]byte, error) {
+	u, err := url.Parse(file)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse config location %s: %w", file, err)
+	}
+
+	switch u.Scheme {
+	case "http", "https":
+		resp, err := http.Get(file)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read http config %s: %w", file, err)
+		}
+		defer resp.Body.Close()
+		return ioutil.ReadAll(resp.Body)
+	default:
+		return ioutil.ReadFile(file)
+	}
+}

pkg/configfilearg/parser_test.go

@@ -0,0 +1,233 @@
+package configfilearg
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFindStart(t *testing.T) {
+	testCases := []struct {
+		input  []string
+		prefix []string
+		suffix []string
+		found  bool
+		what   string
+	}{
+		{
+			input:  nil,
+			prefix: nil,
+			suffix: nil,
+			found:  false,
+			what:   "default case",
+		},
+		{
+			input:  []string{"server"},
+			prefix: []string{"server"},
+			suffix: []string{},
+			found:  true,
+			what:   "simple case",
+		},
+		{
+			input:  []string{"server", "foo"},
+			prefix: []string{"server"},
+			suffix: []string{"foo"},
+			found:  true,
+			what:   "also simple case",
+		},
+		{
+			input:  []string{"server", "foo", "bar"},
+			prefix: []string{"server"},
+			suffix: []string{"foo", "bar"},
+			found:  true,
+			what:   "longer simple case",
+		},
+		{
+			input:  []string{"not-server", "foo", "bar"},
+			prefix: []string{"not-server", "foo", "bar"},
+			found:  false,
+			what:   "not found",
+		},
+	}
+
+	p := Parser{
+		After: []string{"server", "agent"},
+	}
+
+	for _, testCase := range testCases {
+		prefix, suffix, found := p.findStart(testCase.input)
+		assert.Equal(t, testCase.prefix, prefix)
+		assert.Equal(t, testCase.suffix, suffix)
+		assert.Equal(t, testCase.found, found)
+	}
+}
+
+func TestConfigFile(t *testing.T) {
+	testCases := []struct {
+		input      []string
+		env        string
+		def        string
+		configFile string
+		found      bool
+		what       string
+	}{
+		{
+			input: nil,
+			found: false,
+			what:  "default case",
+		},
+		{
+			input:      []string{"asdf", "-c", "value"},
+			configFile: "value",
+			found:      true,
+			what:       "simple case",
+		},
+		{
+			input: []string{"-c"},
+			found: false,
+			what:  "invalid args string",
+		},
+		{
+			input: []string{"-c="},
+			found: true,
+			what:  "empty arg value",
+		},
+		{
+			def:   "def",
+			input: []string{"-c="},
+			found: true,
+			what:  "empty arg value override default",
+		},
+		{
+			def:   "def",
+			input: []string{"-c"},
+			found: false,
+			what:  "invalid args always return no value",
+		},
+		{
+			def:        "def",
+			input:      []string{"-c", "value"},
+			configFile: "value",
+			found:      true,
+			what:       "value override default",
+		},
+		{
+			def:        "def",
+			configFile: "def",
+			found:      false,
+			what:       "default gets used when nothing is passed",
+		},
+		{
+			def:        "def",
+			input:      []string{"-c", "value"},
+			env:        "env",
+			configFile: "env",
+			found:      true,
+			what:       "env override args",
+		},
+		{
+			def:        "def",
+			input:      []string{"before", "-c", "value", "after"},
+			configFile: "value",
+			found:      true,
+			what:       "garbage in start and end",
+		},
+	}
+
+	for _, testCase := range testCases {
+		p := Parser{
+			FlagNames:     []string{"--config", "-c"},
+			EnvName:       "_TEST_FLAG_ENV",
+			DefaultConfig: testCase.def,
+		}
+		os.Setenv(p.EnvName, testCase.env)
+		configFile, found := p.findConfigFileFlag(testCase.input)
+		assert.Equal(t, testCase.configFile, configFile, testCase.what)
+		assert.Equal(t, testCase.found, found, testCase.what)
+	}
+}
+
+func TestParse(t *testing.T) {
+	testDataOutput := []string{
+		"--foo-bar", "baz",
+		"--a-slice", "1",
+		"--a-slice", "2",
+		"--a-slice", "",
+		"--a-slice", "three",
+		"--isempty",
+		"-c", "b",
+		"--islast", "true",
+	}
+
+	defParser := Parser{
+		After:         []string{"server", "agent"},
+		FlagNames:     []string{"-c", "--config"},
+		EnvName:       "_TEST_ENV",
+		DefaultConfig: "./testdata/data.yaml",
+	}
+
+	testCases := []struct {
+		parser Parser
+		env    string
+		input  []string
+		output []string
+		err    string
+		what   string
+	}{
+		{
+			parser: defParser,
+			what:   "default case",
+		},
+		{
+			parser: defParser,
+			input:  []string{"server"},
+			output: append([]string{"server"}, testDataOutput...),
+			what:   "read config file when not specified",
+		},
+		{
+			parser: Parser{
+				After:         []string{"server", "agent"},
+				FlagNames:     []string{"-c", "--config"},
+				DefaultConfig: "missing",
+			},
+			input:  []string{"server"},
+			output: []string{"server"},
+			what:   "ignore missing config when not set",
+		},
+		{
+			parser: Parser{
+				After:         []string{"server", "agent"},
+				FlagNames:     []string{"-c", "--config"},
+				DefaultConfig: "missing",
+			},
+			input:  []string{"server", "-c=missing"},
+			output: []string{"server", "-c=missing"},
+			what:   "fail when missing config",
+			err:    "open missing: no such file or directory",
+		},
+		{
+			parser: Parser{
+				After:         []string{"server", "agent"},
+				FlagNames:     []string{"-c", "--config"},
+				DefaultConfig: "missing",
+			},
+			input:  []string{"before", "server", "before", "-c", "./testdata/data.yaml", "after"},
+			output: append(append([]string{"before", "server"}, testDataOutput...), "before", "-c", "./testdata/data.yaml", "after"),
+			what:   "read config file",
+		},
+	}
+
+	for _, testCase := range testCases {
+		os.Setenv(testCase.parser.EnvName, testCase.env)
+		output, err := testCase.parser.Parse(testCase.input)
+		if err == nil {
+			assert.Equal(t, testCase.err, "", testCase.what)
+		} else {
+			assert.Equal(t, testCase.err, err.Error(), testCase.what)
+		}
+		if testCase.err == "" {
+			assert.Equal(t, testCase.output, output, testCase.what)
+		}
+	}
+}

pkg/configfilearg/testdata/data.yaml

@@ -0,0 +1,9 @@
+foo-bar: baz
+a-slice:
+- 1
+- "2"
+- ""
+- three
+isempty:
+c: b
+islast: true