[confmap] Fix expansion of escaped environment variables (#10716)

This change fixes the issue where environment variables escaped with $$
are expanded. The collector now converts `$${ENV_VAR}` to `${ENV_VAR}`
and `$$ENV_VAR` to `$ENV_VAR` without further expansion (or the
expansion failure in case of `$ENV_VAR`).

Fixes
#10713

.chloggen/fix-env-var-double-escaping.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# 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 wrong expansion of environment variables escaped with `$$`, e.g. `$${ENV_VAR}` and `$$ENV_VAR`.
+
+# One or more tracking issues or pull requests related to the change
+issues: [10713]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+ This change fixes the issue where environment variables escaped with $$ were expanded. 
+ The collector now converts `$${ENV_VAR}` to `${ENV_VAR}` and `$$ENV_VAR` to `$ENV_VAR` without further expansion.
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

confmap/converter/expandconverter/expand.go

@@ -85,6 +85,7 @@ func (c converter) expandEnv(s string) (string, error) {
  // - $FOO will be substituted with env var FOO
  // - $$FOO will be replaced with $FOO
  // - $$$FOO will be replaced with $ + substituted env var FOO
+ // TODO: Move the escaping of $$ out from the expand converter to the resolver.
  if str == "$" {
  return "$"
  }

confmap/expand_test.go

@@ -577,46 +577,3 @@ func TestResolverDefaultProviderExpand(t *testing.T) {
  require.NoError(t, err)
  assert.Equal(t, map[string]any{"foo": "localhost"}, cfgMap.ToStringMap())
 }
-
-func Test_EscapedEnvVars(t *testing.T) {
- const mapValue2 = "some map value"
-
- expectedMap := map[string]any{
- "test_map": map[string]any{
- "recv.1": "$MAP_VALUE_1",
- "recv.2": "$$MAP_VALUE_2",
- "recv.3": "$$MAP_VALUE_3",
- "recv.4": "$" + mapValue2,
- "recv.5": "some${MAP_VALUE_4}text",
- "recv.6": "${ONE}${TWO}",
- "recv.7": "text$",
- "recv.8": "$",
- "recv.9": "${1}${env:2}",
- "recv.10": "some${env:MAP_VALUE_4}text",
- "recv.11": "${env:" + mapValue2 + "}",
- "recv.12": "${env:${MAP_VALUE_2}}",
- "recv.13": "env:MAP_VALUE_2}${MAP_VALUE_2}{",
- "recv.14": "${env:MAP_VALUE_2${MAP_VALUE_2}",
- "recv.15": "$" + mapValue2,
- }}
-
- fileProvider := newFakeProvider("file", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
- return NewRetrieved(newConfFromFile(t, uri[5:]))
- })
- envProvider := newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
- if uri == "env:MAP_VALUE_2" {
- return NewRetrieved(mapValue2)
- }
- return nil, errors.New("should not be expanding any other env vars")
- })
-
- resolver, err := NewResolver(ResolverSettings{URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")}, ProviderFactories: []ProviderFactory{fileProvider, envProvider}, ConverterFactories: nil, DefaultScheme: "env"})
- require.NoError(t, err)
-
- // 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)
- m := cfgMap.ToStringMap()
- assert.Equal(t, expectedMap, m)
-
-}

confmap/internal/e2e/expand_test.go

@@ -0,0 +1,94 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package e2etest
+
+import (
+ "context"
+ "fmt"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "go.opentelemetry.io/collector/confmap"
+ "go.opentelemetry.io/collector/confmap/converter/expandconverter"
+ "go.opentelemetry.io/collector/confmap/provider/envprovider"
+ "go.opentelemetry.io/collector/confmap/provider/fileprovider"
+ "go.opentelemetry.io/collector/confmap/provider/yamlprovider"
+)
+
+// Test_EscapedEnvVars tests that the resolver supports escaped env vars working together with expand converter.
+func Test_EscapedEnvVars(t *testing.T) {
+ tests := []struct {
+ name string
+ scheme string
+ }{
+ {
+ name: "no_default_scheme",
+ scheme: "",
+ },
+ {
+ name: "env",
+ scheme: "env",
+ },
+ }
+
+ const expandedValue = "some expanded value"
+ t.Setenv("ENV_VALUE", expandedValue)
+
+ expectedFailures := map[string]string{
+ "$ENV_VALUE": "variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR}",
+ "$$$ENV_VALUE": "variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR}",
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ expectedMap := map[string]any{
+ "test_map": map[string]any{
+ "key1": "$ENV_VALUE",
+ "key2": "$$ENV_VALUE",
+ "key3": "$" + expandedValue,
+ "key4": "some" + expandedValue + "text",
+ "key5": "some${ENV_VALUE}text",
+ "key6": "${ONE}${TWO}",
+ "key7": "text$",
+ "key8": "$",
+ "key9": "${1}${env:2}",
+ "key10": "some${env:ENV_VALUE}text",
+ "key11": "${env:" + expandedValue + "}",
+ "key12": "${env:${ENV_VALUE}}",
+ "key13": "env:MAP_VALUE_2}${ENV_VALUE}{",
+ "key14": "$" + expandedValue,
+ },
+ }
+
+ resolver, err := confmap.NewResolver(confmap.ResolverSettings{
+ URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")},
+ ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory(), envprovider.NewFactory()},
+ ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()},
+ DefaultScheme: tt.scheme,
+ })
+ require.NoError(t, err)
+
+ // 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)
+ m := cfgMap.ToStringMap()
+ assert.Equal(t, expectedMap, m)
+
+ for val, expectedErr := range expectedFailures {
+ resolver, err = confmap.NewResolver(confmap.ResolverSettings{
+ URIs: []string{fmt.Sprintf("yaml: test: %s", val)},
+ ProviderFactories: []confmap.ProviderFactory{yamlprovider.NewFactory(), envprovider.NewFactory()},
+ ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()},
+ DefaultScheme: tt.scheme,
+ })
+ require.NoError(t, err)
+ _, err := resolver.Resolve(context.Background())
+ require.ErrorContains(t, err, expectedErr)
+ }
+ })
+ }
+}

confmap/internal/e2e/go.mod

@@ -5,8 +5,10 @@ go 1.21.0
 require (
  github.com/stretchr/testify v1.9.0
  go.opentelemetry.io/collector/confmap v0.105.0
+ go.opentelemetry.io/collector/confmap/converter/expandconverter v0.105.0
  go.opentelemetry.io/collector/confmap/provider/envprovider v0.105.0
  go.opentelemetry.io/collector/confmap/provider/fileprovider v0.105.0
+ go.opentelemetry.io/collector/confmap/provider/yamlprovider v0.105.0
  go.opentelemetry.io/collector/featuregate v1.12.0
  go.opentelemetry.io/collector/internal/globalgates v0.105.0
 )
@@ -32,6 +34,10 @@ replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../pro
 
 replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
 
+replace go.opentelemetry.io/collector/confmap/provider/yamlprovider => ../../provider/yamlprovider
+
 replace go.opentelemetry.io/collector/featuregate => ../../../featuregate
 
 replace go.opentelemetry.io/collector/internal/globalgates => ../../../internal/globalgates
+
+replace go.opentelemetry.io/collector/confmap/converter/expandconverter => ../../converter/expandconverter

confmap/internal/e2e/testdata/expand-escaped-env.yaml

@@ -0,0 +1,29 @@
+test_map:
+ # $$ -> escaped $
+ key1: "$$ENV_VALUE"
+ # $$$$ -> two escaped $
+ key2: "$$$$ENV_VALUE"
+ # $$ -> escaped $ + ${ENV_VALUE} expanded
+ key3: "$$${ENV_VALUE}"
+ # expanded in the middle
+ key4: "some${ENV_VALUE}text"
+ # escaped $ in the middle
+ key5: "some$${ENV_VALUE}text"
+ # two escaped $
+ key6: "$${ONE}$${TWO}"
+ # trailing escaped $
+ key7: "text$$"
+ # escaped $ alone
+ key8: "$$"
+ # escaped number and uri
+ key9: "$${1}$${env:2}"
+ # escape provider
+ key10: "some$${env:ENV_VALUE}text"
+ # can escape outer when nested
+ key11: "$${env:${ENV_VALUE}}"
+ # can escape inner and outer when nested
+ key12: "$${env:$${ENV_VALUE}}"
+ # can escape partial
+ key13: "env:MAP_VALUE_2}$${ENV_VALUE}{"
+ # $$$ -> escaped $ + expanded env var
+ key14: "$$${env:ENV_VALUE}"

confmap/resolver.go

@@ -12,8 +12,6 @@ import (
 
  "go.uber.org/multierr"
  "go.uber.org/zap"
-
- "go.opentelemetry.io/collector/internal/globalgates"
 )
 
 // follows drive-letter specification:
@@ -173,13 +171,7 @@ func (mr *Resolver) Resolve(ctx context.Context) (*Conf, error) {
  if err != nil {
  return nil, err
  }
-
- if v, ok := val.(string); ok && globalgates.UseUnifiedEnvVarExpansionRules.IsEnabled() {
- cfgMap[k] = strings.ReplaceAll(v, "$$", "$")
- } else {
- cfgMap[k] = val
- }
-
+ cfgMap[k] = val
  }
  retMap = NewFromStringMap(cfgMap)