Fix support for concatenating envvars with colon

Signed-off-by: Bogdan Drutu <[email protected]>

.chloggen/fixconfmap.yaml

@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix support for concatenating envvars with colon
+
+# One or more tracking issues or pull requests related to the change
+issues: [6580]

confmap/converter/expandconverter/expand_test.go

@@ -115,3 +115,58 @@ func TestNewExpandConverter_EscapedEnvVars(t *testing.T) {
 	require.NoError(t, New().Convert(context.Background(), conf))
 	assert.Equal(t, expectedMap, conf.ToStringMap())
 }
+
+func TestNewExpandConverterHostPort(t *testing.T) {
+	t.Setenv("HOST", "127.0.0.1")
+	t.Setenv("PORT", "4317")
+
+	var testCases = []struct {
+		name     string
+		input    map[string]any
+		expected map[string]any
+	}{
+		{
+			name: "brackets",
+			input: map[string]any{
+				"test": "${HOST}:${PORT}",
+			},
+			expected: map[string]any{
+				"test": "127.0.0.1:4317",
+			},
+		},
+		{
+			name: "no brackets",
+			input: map[string]any{
+				"test": "$HOST:$PORT",
+			},
+			expected: map[string]any{
+				"test": "127.0.0.1:4317",
+			},
+		},
+		{
+			name: "mix",
+			input: map[string]any{
+				"test": "${HOST}:$PORT",
+			},
+			expected: map[string]any{
+				"test": "127.0.0.1:4317",
+			},
+		},
+		{
+			name: "reverse mix",
+			input: map[string]any{
+				"test": "$HOST:${PORT}",
+			},
+			expected: map[string]any{
+				"test": "127.0.0.1:4317",
+			},
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			conf := confmap.NewFromStringMap(tt.input)
+			require.NoError(t, New().Convert(context.Background(), conf))
+			assert.Equal(t, tt.expected, conf.ToStringMap())
+		})
+	}
+}

confmap/resolver.go

@@ -36,6 +36,8 @@ var (
 	// Need to match new line as well in the OpaqueValue, so setting the "s" flag. See https://pkg.go.dev/regexp/syntax.
 	locationRegexp = regexp.MustCompile(`(?s:^(?P<Scheme>[A-Za-z][A-Za-z0-9+.-]+):(?P<OpaqueValue>.*)$)`)
 
+	embeddedLocationRegexp = regexp.MustCompile("^\\${[A-Za-z][A-Za-z0-9+.-]+:.*}$")
+
 	errTooManyRecursiveExpansions = errors.New("too many recursive expansions")
 )
 
@@ -239,13 +241,15 @@ func (mr *Resolver) expandValueRecursively(ctx context.Context, value interface{
 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 it doesn't have the format "${scheme:opaque}" no need to expand, or contains "
 		if !strings.HasPrefix(v, "${") || !strings.HasSuffix(v, "}") || !strings.Contains(v, ":") {
 			return value, false, nil
 		}
 		lURI, err := newLocation(v[2 : len(v)-1])
 		if err != nil {
-			return nil, false, err
+			// Cannot return error, since a case like "${HOST}:${PORT}" is invalid location,
+			// but is supported in the legacy implementation.
+			return value, false, nil
 		}
 		if strings.Contains(lURI.opaqueValue, "$") {
 			return nil, false, fmt.Errorf("the uri %q contains unsupported characters ('$')", lURI.asString())

confmap/resolver_test.go

@@ -380,7 +380,7 @@ func TestResolverExpandEnvVars(t *testing.T) {
 }
 
 func TestResolverDoneNotExpandOldEnvVars(t *testing.T) {
-	expectedCfgMap := map[string]interface{}{"test.1": "${EXTRA}", "test.2": "$EXTRA"}
+	expectedCfgMap := map[string]interface{}{"test.1": "${EXTRA}", "test.2": "$EXTRA", "test.3": "${EXTRA}:${EXTRA}"}
 	fileProvider := newFakeProvider("test", func(context.Context, string, WatcherFunc) (*Retrieved, error) {
 		return NewRetrieved(expectedCfgMap)
 	})
@@ -486,7 +486,9 @@ func TestResolverExpandInvalidScheme(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = resolver.Resolve(context.Background())
-	assert.EqualError(t, err, `invalid uri: "g_c_s:VALUE"`)
+	// TODO: Investigate how to return an error like `invalid uri: "g_c_s:VALUE"` and keep the legacy behavior
+	//  for cases like "${HOST}:${PORT}"
+	assert.NoError(t, err)
 }
 
 func TestResolverExpandInvalidOpaqueValue(t *testing.T) {