From 21d21ddd4d1de3c1a96ad1c502a98b61b7711d08 Mon Sep 17 00:00:00 2001 From: Darren Shepherd Date: Sat, 29 Aug 2020 20:30:07 -0700 Subject: [PATCH] Add config file support independent of CLI framework Signed-off-by: Darren Shepherd --- cmd/agent/main.go | 3 +- cmd/server/main.go | 3 +- main.go | 3 +- pkg/cli/cmds/agent.go | 4 +- pkg/cli/cmds/config.go | 17 ++ pkg/cli/cmds/root.go | 27 ++-- pkg/cli/cmds/server.go | 2 +- pkg/configfilearg/defaultparser.go | 20 +++ pkg/configfilearg/parser.go | 139 ++++++++++++++++ pkg/configfilearg/parser_test.go | 233 +++++++++++++++++++++++++++ pkg/configfilearg/testdata/data.yaml | 9 ++ 11 files changed, 445 insertions(+), 15 deletions(-) create mode 100644 pkg/cli/cmds/config.go create mode 100644 pkg/configfilearg/defaultparser.go create mode 100644 pkg/configfilearg/parser.go create mode 100644 pkg/configfilearg/parser_test.go create mode 100644 pkg/configfilearg/testdata/data.yaml diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 9d9049bcb1..dfb75dfe30 100644 --- a/cmd/agent/main.go +++ b/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) } diff --git a/cmd/server/main.go b/cmd/server/main.go index 7fcd6e047b..fffafb7248 100644 --- a/cmd/server/main.go +++ b/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) } diff --git a/main.go b/main.go index 62908bb7bb..10624f69bf 100644 --- a/main.go +++ b/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) } } diff --git a/pkg/cli/cmds/agent.go b/pkg/cli/cmds/agent.go index 01d9ce75fa..c9470ce8ee 100644 --- a/pkg/cli/cmds/agent.go +++ b/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, diff --git a/pkg/cli/cmds/config.go b/pkg/cli/cmds/config.go new file mode 100644 index 0000000000..1b05078efe --- /dev/null +++ b/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", + } +) diff --git a/pkg/cli/cmds/root.go b/pkg/cli/cmds/root.go index 212ec13075..8233fcb90e 100644 --- a/pkg/cli/cmds/root.go +++ b/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", - }, + DebugFlag, } + app.Before = SetupDebug(nil) + + return app +} - app.Before = func(ctx *cli.Context) error { +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 } - - return app } diff --git a/pkg/cli/cmds/server.go b/pkg/cli/cmds/server.go index b01ded9ce2..5b4e61ae76 100644 --- a/pkg/cli/cmds/server.go +++ b/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, diff --git a/pkg/configfilearg/defaultparser.go b/pkg/configfilearg/defaultparser.go new file mode 100644 index 0000000000..7a7f10c0fc --- /dev/null +++ b/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 +} diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go new file mode 100644 index 0000000000..600e56f413 --- /dev/null +++ b/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) + } +} diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go new file mode 100644 index 0000000000..47145e1daa --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/configfilearg/testdata/data.yaml b/pkg/configfilearg/testdata/data.yaml new file mode 100644 index 0000000000..61910e4d1a --- /dev/null +++ b/pkg/configfilearg/testdata/data.yaml @@ -0,0 +1,9 @@ +foo-bar: baz +a-slice: +- 1 +- "2" +- "" +- three +isempty: +c: b +islast: true \ No newline at end of file