From 262f771df5728a886d5690a6de8c751d066908f2 Mon Sep 17 00:00:00 2001 From: Andrew Mains Date: Tue, 22 Oct 2019 14:29:42 -0400 Subject: [PATCH] Use go.uber.org/config --- CHANGELOG.md | 5 ++ config/README.md | 32 ++++++- src/x/config/config.go | 45 ++++++---- src/x/config/config_test.go | 167 +++++++++++++++++++++++++++++++++--- 4 files changed, 217 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2875e14285..1bc25afb69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +# 0.14.2 (upcoming) + +## Features +- **Config**: support env var expansion using go.uber.org/config. + # 0.14.1 ## Features diff --git a/config/README.md b/config/README.md index 4a7f25f985..836eeac33c 100644 --- a/config/README.md +++ b/config/README.md @@ -36,4 +36,34 @@ std.manifestYamlDoc(lib(cluster, coordinator)) 3. Run the following `jsonnet` command. This will generate a file called `generated.yaml` within the directory. ``` make config-gen -``` \ No newline at end of file +``` + +## Configuration Schemas + +At this time, each service's configuration is defined as a Go struct in a +corresponding `config` package. For instance, `m3aggregator`'s config is +in [src/cmd/services/m3aggregator/config/config.go](https://github.com/m3db/m3/blob/master/src/cmd/services/m3aggregator/config/config.go). + +Validation is done using https://godoc.org/gopkg.in/go-playground/validator.v2 ; +look for `validate` tags. + +## Advanced: Environment Variable Interpolation +M3 uses [go.uber.org/config](https://godoc.org/go.uber.org/config) to load +its configuration. This means that we support environment variable interpolation +of configs. For example: + +Given the following config: + +``` +foo: ${MY_ENV_VAR} +``` + +and an environment of `MY_ENV_VAR=bar`, the interpolated YAML will be: + +``` +foo: bar +``` +. + +This can be useful for further tweaking your configuration without generating +different files for every config permutation. \ No newline at end of file diff --git a/src/x/config/config.go b/src/x/config/config.go index bf7d91f4a3..1c5b671c4a 100644 --- a/src/x/config/config.go +++ b/src/x/config/config.go @@ -23,13 +23,12 @@ package config import ( "errors" - "io/ioutil" "reflect" "strings" + "go.uber.org/config" "go.uber.org/zap" validator "gopkg.in/validator.v2" - yaml "gopkg.in/yaml.v2" ) const ( @@ -42,37 +41,49 @@ var errNoFilesToLoad = errors.New("attempt to load config with no files") type Options struct { DisableUnmarshalStrict bool DisableValidate bool + Expand config.LookupFunc } // LoadFile loads a config from a file. -func LoadFile(config interface{}, file string, opts Options) error { - return LoadFiles(config, []string{file}, opts) +func LoadFile(dst interface{}, file string, opts Options) error { + return LoadFiles(dst, []string{file}, opts) } // LoadFiles loads a config from list of files. If value for a property is // present in multiple files, the value from the last file will be applied. // Validation is done after merging all values. -func LoadFiles(config interface{}, files []string, opts Options) error { +func LoadFiles(dst interface{}, files []string, opts Options) error { if len(files) == 0 { return errNoFilesToLoad } + + yamlOpts := make([]config.YAMLOption, 0, len(files)) for _, name := range files { - data, err := ioutil.ReadFile(name) - if err != nil { - return err - } - unmarshal := yaml.UnmarshalStrict - if opts.DisableUnmarshalStrict { - unmarshal = yaml.Unmarshal - } - if err := unmarshal(data, config); err != nil { - return err - } + yamlOpts = append(yamlOpts, config.File(name)) + } + + if opts.DisableUnmarshalStrict { + yamlOpts = append(yamlOpts, config.Permissive()) + } + + if opts.Expand != nil { + yamlOpts = append(yamlOpts, config.Expand(opts.Expand)) + } + + provider, err := config.NewYAML(yamlOpts...) + if err != nil { + return err } + + if err := provider.Get(config.Root).Populate(dst); err != nil { + return err + } + if opts.DisableValidate { return nil } - return validator.Validate(config) + + return validator.Validate(dst) } // deprecationCheck checks the config for deprecated fields and returns any in diff --git a/src/x/config/config_test.go b/src/x/config/config_test.go index 59752d1c4b..234898b25b 100644 --- a/src/x/config/config_test.go +++ b/src/x/config/config_test.go @@ -21,12 +21,15 @@ package config import ( + "errors" "fmt" "io/ioutil" "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/config" ) const ( @@ -100,7 +103,9 @@ func TestLoadFile(t *testing.T) { require.Error(t, err) fname := writeFile(t, goodConfig) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err = LoadFile(&cfg, fname, Options{}) require.NoError(t, err) @@ -126,7 +131,9 @@ func TestLoadWithInvalidFile(t *testing.T) { require.Error(t, err) fname := writeFile(t, goodConfig) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() // non-exist file in the file list err = LoadFiles(&cfg, []string{fname, "./no-config.yaml"}, Options{}) @@ -141,7 +148,9 @@ func TestLoadFileInvalidKey(t *testing.T) { var cfg configuration fname := writeFile(t, badConfigInvalidKey) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{}) require.Error(t, err) @@ -151,7 +160,9 @@ func TestLoadFileInvalidKeyDisableMarshalStrict(t *testing.T) { var cfg configuration fname := writeFile(t, badConfigInvalidKey) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{DisableUnmarshalStrict: true}) require.NoError(t, err) @@ -164,7 +175,9 @@ func TestLoadFileInvalidValue(t *testing.T) { var cfg configuration fname := writeFile(t, badConfigInvalidValue) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{}) require.Error(t, err) @@ -174,7 +187,9 @@ func TestLoadFileInvalidValueDisableValidate(t *testing.T) { var cfg configuration fname := writeFile(t, badConfigInvalidValue) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{DisableValidate: true}) require.NoError(t, err) @@ -185,7 +200,9 @@ func TestLoadFileInvalidValueDisableValidate(t *testing.T) { func TestLoadFilesExtends(t *testing.T) { fname := writeFile(t, goodConfig) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() partialConfig := ` buffer_space: 8080 @@ -194,7 +211,9 @@ servers: - server4:8080 ` partial := writeFile(t, partialConfig) - defer os.Remove(partial) + defer func() { + require.NoError(t, os.Remove(partial)) + }() var cfg configuration err := LoadFiles(&cfg, []string{fname, partial}, Options{}) @@ -205,6 +224,42 @@ servers: require.Equal(t, []string{"server3:8080", "server4:8080"}, cfg.Servers) } +func TestLoadFilesDeepExtends(t *testing.T) { + type innerConfig struct { + K1 string `yaml:"k1"` + K2 string `yaml:"k2"` + } + + type nestedConfig struct { + Foo innerConfig `yaml:"foo"` + } + + const ( + base = ` +foo: + k1: v1_base + k2: v2_base +` + override = ` +foo: + k1: v1_override +` + ) + + baseFile, overrideFile := writeFile(t, base), writeFile(t, override) + + var cfg nestedConfig + require.NoError(t, LoadFiles(&cfg, []string{baseFile, overrideFile}, Options{})) + + assert.Equal(t, nestedConfig{ + Foo: innerConfig{ + K1: "v1_override", + K2: "v2_base", + }, + }, cfg) + +} + func TestLoadFilesValidateOnce(t *testing.T) { const invalidConfig1 = ` listen_address: @@ -219,10 +274,14 @@ func TestLoadFilesValidateOnce(t *testing.T) { ` fname1 := writeFile(t, invalidConfig1) - defer os.Remove(fname1) + defer func() { + require.NoError(t, os.Remove(fname1)) + }() fname2 := writeFile(t, invalidConfig2) - defer os.Remove(invalidConfig2) + defer func() { + require.NoError(t, os.Remove(fname2)) + }() // Either config by itself will not pass validation. var cfg1 configuration @@ -243,12 +302,86 @@ func TestLoadFilesValidateOnce(t *testing.T) { require.Equal(t, []string{"server2:8010"}, mergedCfg.Servers) } +func TestLoadFilesEnvExpansion(t *testing.T) { + const withEnv = ` + listen_address: localhost:${PORT:8080} + buffer_space: ${BUFFER_SPACE} + servers: + - server2:8010 + ` + + mapLookup := func(m map[string]string) config.LookupFunc { + return func(s string) (string, bool) { + r, ok := m[s] + return r, ok + } + } + + cases := []struct { + Name string + Input map[string]string + Expected configuration + ExpectedErr error + }{{ + Name: "all_provided", + Input: map[string]string{ + "PORT": "9090", + "BUFFER_SPACE": "256", + }, + Expected: configuration{ + ListenAddress: "localhost:9090", + BufferSpace: 256, + Servers: []string{"server2:8010"}, + }, + }, { + Name: "missing_no_default", + Input: map[string]string{ + "PORT": "9090", + // missing BUFFER_SPACE, + }, + ExpectedErr: errors.New("couldn't expand environment: default is empty for \"BUFFER_SPACE\" (use \"\" for empty string)"), + }, { + Name: "missing_with_default", + Input: map[string]string{ + "BUFFER_SPACE": "256", + }, + Expected: configuration{ + ListenAddress: "localhost:8080", + BufferSpace: 256, + Servers: []string{"server2:8010"}, + }, + }} + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + fname1 := writeFile(t, withEnv) + defer func() { + require.NoError(t, os.Remove(fname1)) + }() + var cfg configuration + + err := LoadFile(&cfg, fname1, Options{ + Expand: mapLookup(tc.Input), + }) + if tc.ExpectedErr != nil { + require.EqualError(t, err, tc.ExpectedErr.Error()) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.Expected, cfg) + }) + } +} + func TestDeprecationCheck(t *testing.T) { t.Run("StandardConfig", func(t *testing.T) { // OK var cfg configuration fname := writeFile(t, goodConfig) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{}) require.NoError(t, err) @@ -269,7 +402,9 @@ bar: 42 ` var cfg2 configurationDeprecated fname2 := writeFile(t, badConfig) - defer os.Remove(fname2) + defer func() { + require.NoError(t, os.Remove(fname2)) + }() err = LoadFile(&cfg2, fname2, Options{}) require.NoError(t, err) @@ -295,7 +430,9 @@ commitlog: blockSize: 23 ` fname := writeFile(t, nc) - defer os.Remove(fname) + defer func() { + require.NoError(t, os.Remove(fname)) + }() err := LoadFile(&cfg, fname, Options{}) require.NoError(t, err) @@ -329,7 +466,9 @@ multiple: ` fname2 := writeFile(t, nc) - defer os.Remove(fname2) + defer func() { + require.NoError(t, os.Remove(fname2)) + }() err = LoadFile(&cfg2, fname2, Options{}) require.NoError(t, err)