diff --git a/pkg/configfilearg/parser.go b/pkg/configfilearg/parser.go index 077b89922b98..c17a8f184ae7 100644 --- a/pkg/configfilearg/parser.go +++ b/pkg/configfilearg/parser.go @@ -42,12 +42,12 @@ func (p *Parser) Parse(args []string) ([]string, error) { return args, nil } - configFile, isSet := p.findConfigFileFlag(args) - if configFile != "" { + if configFile := p.findConfigFileFlag(args); configFile != "" { values, err := readConfigFile(configFile) - if !isSet && os.IsNotExist(err) { - return args, nil - } else if err != nil { + if err != nil { + if os.IsNotExist(err) { + return args, nil + } return nil, err } if len(args) > 1 { @@ -99,49 +99,50 @@ func (p *Parser) stripInvalidFlags(command string, args []string) ([]string, err return result, nil } +// FindString returns the string value of a flag, checking the CLI args, +// config file, and config file dropins. If the value is not found, +// an empty string is returned. It is not an error if no args, +// configfile, or dropins are present. func (p *Parser) FindString(args []string, target string) (string, error) { - // Check for --help or --version flags, which override any other flags if val, found := p.findOverrideFlag(args); found { return val, nil } - configFile, isSet := p.findConfigFileFlag(args) + var files []string var lastVal string - if configFile != "" { - _, err := os.Stat(configFile) - if !isSet && os.IsNotExist(err) { - return "", nil - } else if err != nil { + if configFile := p.findConfigFileFlag(args); configFile != "" { + if _, err := os.Stat(configFile); err == nil { + files = append(files, configFile) + } + + dropinFiles, err := dotDFiles(configFile) + if err != nil { return "", err } + files = append(files, dropinFiles...) + } - files, err := dotDFiles(configFile) + for _, file := range files { + bytes, err := readConfigFileData(file) if err != nil { return "", err } - files = append([]string{configFile}, files...) - for _, file := range files { - bytes, err := readConfigFileData(file) - if err != nil { - return "", err - } - data := yaml.MapSlice{} - if err := yaml.Unmarshal(bytes, &data); err != nil { - return "", err - } - for _, i := range data { - k, v := convert.ToString(i.Key), convert.ToString(i.Value) - isAppend := strings.HasSuffix(k, "+") - k = strings.TrimSuffix(k, "+") - if k == target { - if isAppend { - lastVal = lastVal + "," + v - } else { - lastVal = v - } + data := yaml.MapSlice{} + if err := yaml.Unmarshal(bytes, &data); err != nil { + return "", err + } + for _, i := range data { + k, v := convert.ToString(i.Key), convert.ToString(i.Value) + isAppend := strings.HasSuffix(k, "+") + k = strings.TrimSuffix(k, "+") + if k == target { + if isAppend { + lastVal = lastVal + "," + v + } else { + lastVal = v } } } @@ -161,26 +162,28 @@ func (p *Parser) findOverrideFlag(args []string) (string, bool) { return "", false } -func (p *Parser) findConfigFileFlag(args []string) (string, bool) { +// findConfigFileFlag returns the value of the config file env var or CLI flag. +// If neither are set, it returns the default value. +func (p *Parser) findConfigFileFlag(args []string) string { if envVal := os.Getenv(p.EnvName); p.EnvName != "" && envVal != "" { - return envVal, true + return envVal } for i, arg := range args { for _, flagName := range p.ConfigFlags { if flagName == arg { if len(args) > i+1 { - return args[i+1], true + return args[i+1] } // This is actually invalid, so we rely on the CLI parser after the fact flagging it as bad - return "", false + return "" } else if strings.HasPrefix(arg, flagName+"=") { - return arg[len(flagName)+1:], true + return arg[len(flagName)+1:] } } } - return p.DefaultConfig, false + return p.DefaultConfig } func (p *Parser) findStart(args []string) ([]string, []string, bool) { @@ -237,17 +240,23 @@ func dotDFiles(basefile string) (result []string, _ error) { return } +// readConfigFile returns a flattened arg list generated from the specified config +// file, and any config file dropins in the dropin directory that corresponds to that +// config file. The config file or at least one dropin must exist. func readConfigFile(file string) (result []string, _ error) { files, err := dotDFiles(file) if err != nil { return nil, err } - _, err = os.Stat(file) - if os.IsNotExist(err) && len(files) > 0 { - } else if err != nil { - return nil, err + if _, err = os.Stat(file); err != nil { + // If the config file doesn't exist and we have dropins that's fine. + // Other errors are bubbled up regardless of how many dropins we have. + if !(os.IsNotExist(err) && len(files) > 0) { + return nil, err + } } else { + // The config file exists, load it first. files = append([]string{file}, files...) } @@ -321,6 +330,7 @@ func toSlice(v interface{}) []interface{} { } } +// readConfigFileData returns the contents of a local or remote file func readConfigFileData(file string) ([]byte, error) { u, err := url.Parse(file) if err != nil { diff --git a/pkg/configfilearg/parser_test.go b/pkg/configfilearg/parser_test.go index 1dc4640ab9d8..84f7bac59c45 100644 --- a/pkg/configfilearg/parser_test.go +++ b/pkg/configfilearg/parser_test.go @@ -120,61 +120,52 @@ func Test_UnitParser_findConfigFileFlag(t *testing.T) { fields fields arg []string want string - found bool }{ { - name: "default case", - arg: nil, - found: false, + name: "default case", + arg: nil, }, { - name: "simple case", - arg: []string{"asdf", "-c", "value"}, - want: "value", - found: true, + name: "simple case", + arg: []string{"asdf", "-c", "value"}, + want: "value", }, { - name: "invalid args string", - arg: []string{"-c"}, - found: false, + name: "invalid args string", + arg: []string{"-c"}, }, { - name: "empty arg value", - arg: []string{"-c="}, - found: true, + name: "empty arg value", + arg: []string{"-c="}, }, { name: "empty arg value override default", fields: fields{ DefaultConfig: "def", }, - arg: []string{"-c="}, - found: true, + arg: []string{"-c="}, }, { fields: fields{ DefaultConfig: "def", }, - arg: []string{"-c"}, - found: false, - name: "invalid args always return no value", + arg: []string{"-c"}, + name: "invalid args always return no value", }, { fields: fields{ DefaultConfig: "def", }, - arg: []string{"-c", "value"}, - want: "value", - found: true, - name: "value override default", + arg: []string{"-c", "value"}, + want: "value", + name: "value override default", }, { fields: fields{ DefaultConfig: "def", }, - want: "def", - found: false, - name: "default gets used when nothing is passed", + want: "def", + name: "default gets used when nothing is passed", }, { name: "env override args", @@ -182,18 +173,16 @@ func Test_UnitParser_findConfigFileFlag(t *testing.T) { DefaultConfig: "def", env: "env", }, - arg: []string{"-c", "value"}, - want: "env", - found: true, + arg: []string{"-c", "value"}, + want: "env", }, { name: "garbage in start and end", fields: fields{ DefaultConfig: "def", }, - arg: []string{"before", "-c", "value", "after"}, - want: "value", - found: true, + arg: []string{"before", "-c", "value", "after"}, + want: "value", }, } for _, tt := range tests { @@ -207,13 +196,10 @@ func Test_UnitParser_findConfigFileFlag(t *testing.T) { defer os.Unsetenv(tt.fields.env) os.Setenv(p.EnvName, tt.fields.env) - got, found := p.findConfigFileFlag(tt.arg) + got := p.findConfigFileFlag(tt.arg) if got != tt.want { t.Errorf("Parser.findConfigFileFlag() got = %+v\nWant = %+v", got, tt.want) } - if found != tt.found { - t.Errorf("Parser.findConfigFileFlag() found = %+v\nWant = %+v", found, tt.found) - } }) } } @@ -286,13 +272,33 @@ func Test_UnitParser_Parse(t *testing.T) { want: []string{"server"}, }, { - name: "fail when missing config", + name: "ignore missing config when set", fields: fields{ After: []string{"server", "agent"}, FlagNames: []string{"-c", "--config"}, DefaultConfig: "missing", }, - arg: []string{"server", "-c=missing"}, + arg: []string{"server", "-c=missing"}, + want: []string{"server", "-c=missing"}, + }, + { + name: "fail when config cannot be parsed", + fields: fields{ + After: []string{"server", "agent"}, + FlagNames: []string{"-c", "--config"}, + DefaultConfig: "./testdata/invalid.yaml", + }, + arg: []string{"server"}, + wantErr: true, + }, + { + name: "fail when dropin cannot be parsed", + fields: fields{ + After: []string{"server", "agent"}, + FlagNames: []string{"-c", "--config"}, + DefaultConfig: "./testdata/invalid-dropin.yaml", + }, + arg: []string{"server"}, wantErr: true, }, { @@ -404,7 +410,59 @@ func Test_UnitParser_FindString(t *testing.T) { want: "", }, { - name: "Multiple custom configs exist, target exists in a secondary config", + name: "Default config file does not exist but dropin exists, target does not exist", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/dropin-only.yaml", + }, + args: args{ + osArgs: []string{}, + target: "tls", + }, + want: "", + }, + { + name: "Default config file does not exist but dropin exists, target exists", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/dropin-only.yaml", + }, + args: args{ + osArgs: []string{}, + target: "foo-bar", + }, + want: "bar-foo", + }, + { + name: "Custom config file does not exist but dropin exists, target does not exist", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/defaultdata.yaml", + }, + args: args{ + osArgs: []string{"-c", "./testdata/dropin-only.yaml"}, + target: "tls", + }, + want: "", + }, + { + name: "Custom config file does not exist but dropin exists, target exists", + fields: fields{ + FlagNames: []string{"-c", "--config"}, + EnvName: "_TEST_ENV", + DefaultConfig: "./testdata/defaultdata.yaml", + }, + args: args{ + osArgs: []string{"-c", "./testdata/dropin-only.yaml"}, + target: "foo-bar", + }, + want: "bar-foo", + }, + { + name: "Multiple custom configs exist, target exists in a dropin config", fields: fields{ FlagNames: []string{"-c", "--config"}, EnvName: "_TEST_ENV", @@ -417,7 +475,7 @@ func Test_UnitParser_FindString(t *testing.T) { want: "beta", }, { - name: "Multiple custom configs exist, multiple targets exist in multiple secondary config, replacement", + name: "Multiple custom configs exist, multiple targets exist in multiple dropin config, replacement", fields: fields{ FlagNames: []string{"-c", "--config"}, EnvName: "_TEST_ENV", @@ -430,7 +488,7 @@ func Test_UnitParser_FindString(t *testing.T) { want: "bar-foo", }, { - name: "Multiple custom configs exist, multiple targets exist in multiple secondary config, appending", + name: "Multiple custom configs exist, multiple targets exist in multiple dropin config, appending", fields: fields{ FlagNames: []string{"-c", "--config"}, EnvName: "_TEST_ENV", diff --git a/pkg/configfilearg/testdata/dropin-only.yaml.d/01-data.yml b/pkg/configfilearg/testdata/dropin-only.yaml.d/01-data.yml new file mode 100644 index 000000000000..9a0a098f7c73 --- /dev/null +++ b/pkg/configfilearg/testdata/dropin-only.yaml.d/01-data.yml @@ -0,0 +1,15 @@ +foo-bar: get-overriden +a-slice: +- 1 +- "1.5" +- "2" +- "" +- three +b-string: one +c-slice: +- one +- two +d-slice: +- one +- two +f-string: beta \ No newline at end of file diff --git a/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data-ignore-this.txt b/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data-ignore-this.txt new file mode 100644 index 000000000000..f8048ad0559b --- /dev/null +++ b/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data-ignore-this.txt @@ -0,0 +1 @@ +foo-bar: ignored diff --git a/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data.yaml b/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data.yaml new file mode 100644 index 000000000000..0947f7ec4bd7 --- /dev/null +++ b/pkg/configfilearg/testdata/dropin-only.yaml.d/02-data.yaml @@ -0,0 +1,10 @@ +foo-bar: bar-foo +b-string+: two +c-slice+: +- three +d-slice: +- three +- four +e-slice+: +- one +- two \ No newline at end of file diff --git a/pkg/configfilearg/testdata/invalid-dropin.yaml.d/01-data.yml b/pkg/configfilearg/testdata/invalid-dropin.yaml.d/01-data.yml new file mode 100644 index 000000000000..4bd4de28404f --- /dev/null +++ b/pkg/configfilearg/testdata/invalid-dropin.yaml.d/01-data.yml @@ -0,0 +1 @@ +!invalid diff --git a/pkg/configfilearg/testdata/invalid.yaml b/pkg/configfilearg/testdata/invalid.yaml new file mode 100644 index 000000000000..4bd4de28404f --- /dev/null +++ b/pkg/configfilearg/testdata/invalid.yaml @@ -0,0 +1 @@ +!invalid