confmap.Resolver: Remove support for the old way of expanding env vars (#5863)

The new mechanism that allows embedding and expanding configs from all the Providers, does not doing it without explicitly specify the URI scheme.
If support for backwards compatibility is needed, continue to use the `expandconverter`.

Signed-off-by: Bogdan <bogdandrutu@gmail.com>

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
 		}

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{}{

confmap/testdata/expand-with-all-env-with-source.yaml

@@ -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}"
-    

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}"
     

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}"