diff --git a/confmap/resolver.go b/confmap/resolver.go index fe31f75b77..8276c818f7 100644 --- a/confmap/resolver.go +++ b/confmap/resolver.go @@ -209,17 +209,20 @@ func (mr *Resolver) expandValueRecursively(ctx context.Context, value interface{ return nil, errors.New("too many recursive expansions") } +// Scheme name consist of a sequence of characters beginning with a letter and followed by any +// combination of letters, digits, plus ("+"), period ("."), or hyphen ("-"). +var expandRegexp = regexp.MustCompile(`^\$\{[A-Za-z][A-Za-z0-9+.-]+:.*}$`) + func (mr *Resolver) expandValue(ctx context.Context, value interface{}) (interface{}, bool, error) { switch v := value.(type) { case string: // If it doesn't have the format "${scheme:opaque}" no need to expand. - if !strings.HasPrefix(v, "${") || !strings.HasSuffix(v, "}") { + if !expandRegexp.MatchString(v) { return value, false, nil } uri := v[2 : len(v)-1] - // For backwards compatibility: - // - empty scheme means "env". - ret, err := mr.retrieveValue(ctx, location{uri: uri, defaultScheme: "env"}) + // At this point it is guaranteed to have a valid "scheme" based on the expandRegexp, so no default. + ret, err := mr.retrieveValue(ctx, location{uri: uri}) if err != nil { return nil, false, err } diff --git a/confmap/resolver_test.go b/confmap/resolver_test.go index a9a6df771a..6e4572d864 100644 --- a/confmap/resolver_test.go +++ b/confmap/resolver_test.go @@ -321,7 +321,6 @@ func TestResolverExpandEnvVars(t *testing.T) { {name: "expand-with-no-env.yaml"}, {name: "expand-with-partial-env.yaml"}, {name: "expand-with-all-env.yaml"}, - {name: "expand-with-all-env-with-source.yaml"}, } envs := map[string]string{ @@ -355,6 +354,27 @@ func TestResolverExpandEnvVars(t *testing.T) { } } +func TestResolverDoneNotExpandOldEnvVars(t *testing.T) { + expectedCfgMap := map[string]interface{}{"test.1": "${EXTRA}", "test.2": "$EXTRA"} + fileProvider := newFakeProvider("test", func(_ context.Context, uri string, _ WatcherFunc) (Retrieved, error) { + return NewRetrieved(expectedCfgMap) + }) + envProvider := newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (Retrieved, error) { + return NewRetrieved("some string") + }) + emptySchemeProvider := newFakeProvider("", func(_ context.Context, uri string, _ WatcherFunc) (Retrieved, error) { + return NewRetrieved("some string") + }) + + resolver, err := NewResolver(ResolverSettings{URIs: []string{"test:"}, Providers: makeMapProvidersMap(fileProvider, envProvider, emptySchemeProvider), Converters: nil}) + require.NoError(t, err) + resolver.enableExpand = true + // Test that expanded configs are the same with the simple config with no env vars. + cfgMap, err := resolver.Resolve(context.Background()) + require.NoError(t, err) + assert.Equal(t, expectedCfgMap, cfgMap.ToStringMap()) +} + func TestResolverExpandMapAndSliceValues(t *testing.T) { provider := newFakeProvider("input", func(context.Context, string, WatcherFunc) (*Retrieved, error) { return NewRetrieved(map[string]interface{}{ diff --git a/confmap/testdata/expand-with-all-env-with-source.yaml b/confmap/testdata/expand-with-all-env-with-source.yaml deleted file mode 100644 index 5386e05158..0000000000 --- a/confmap/testdata/expand-with-all-env-with-source.yaml +++ /dev/null @@ -1,11 +0,0 @@ -test_map: - extra: "${env:EXTRA}" - extra_map: - recv.1: "${env:EXTRA_MAP_VALUE_1}" - recv.2: "${env:EXTRA_MAP_VALUE_2}" - extra_list_map: - - { recv.1: "${env:EXTRA_LIST_MAP_VALUE_1}",recv.2: "${env:EXTRA_LIST_MAP_VALUE_2}" } - extra_list: - - "${env:EXTRA_LIST_VALUE_1}" - - "${env:EXTRA_LIST_VALUE_2}" - diff --git a/confmap/testdata/expand-with-all-env.yaml b/confmap/testdata/expand-with-all-env.yaml index ed623bf9a5..5386e05158 100644 --- a/confmap/testdata/expand-with-all-env.yaml +++ b/confmap/testdata/expand-with-all-env.yaml @@ -1,11 +1,11 @@ test_map: - extra: "${EXTRA}" + extra: "${env:EXTRA}" extra_map: - recv.1: "${EXTRA_MAP_VALUE_1}" - recv.2: "${EXTRA_MAP_VALUE_2}" + recv.1: "${env:EXTRA_MAP_VALUE_1}" + recv.2: "${env:EXTRA_MAP_VALUE_2}" extra_list_map: - - { recv.1: "${EXTRA_LIST_MAP_VALUE_1}",recv.2: "${EXTRA_LIST_MAP_VALUE_2}" } + - { recv.1: "${env:EXTRA_LIST_MAP_VALUE_1}",recv.2: "${env:EXTRA_LIST_MAP_VALUE_2}" } extra_list: - - "${EXTRA_LIST_VALUE_1}" - - "${EXTRA_LIST_VALUE_2}" + - "${env:EXTRA_LIST_VALUE_1}" + - "${env:EXTRA_LIST_VALUE_2}" diff --git a/confmap/testdata/expand-with-partial-env.yaml b/confmap/testdata/expand-with-partial-env.yaml index fb8ffe51b8..d995c36fa0 100644 --- a/confmap/testdata/expand-with-partial-env.yaml +++ b/confmap/testdata/expand-with-partial-env.yaml @@ -1,10 +1,10 @@ test_map: - extra: "${EXTRA}" + extra: "${env:EXTRA}" extra_map: - recv.1: "${EXTRA_MAP_VALUE_1}" + recv.1: "${env:EXTRA_MAP_VALUE_1}" recv.2: "some map value_2" extra_list_map: - - { recv.1: "some list map value_1",recv.2: "${EXTRA_LIST_MAP_VALUE_2}" } + - { recv.1: "some list map value_1",recv.2: "${env:EXTRA_LIST_MAP_VALUE_2}" } extra_list: - "some list value_1" - - "${EXTRA_LIST_VALUE_2}" + - "${env:EXTRA_LIST_VALUE_2}"