Skip to content

Commit 1de05e2

Browse files
dmitryaxdmitryax
committed
[confmap] Fix double-expansion of escaped environment variables
This change fixes the issue where environment variables escaped with $$ were double-expanded. The collector now converts `$${ENV_VAR}` to `${ENV_VAR}` and `$$ENV_VAR` to `$ENV_VAR` without further expansion.
1 parent 49ea32b commit 1de05e2

File tree

7 files changed

+143
-24
lines changed

7 files changed

+143
-24
lines changed

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

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confmap
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix double-expansion of environment variables escaped with `$$`, e.g. `$${ENV_VAR}` and `$$ENV_VAR`.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10713]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
This change fixes the issue where environment variables escaped with $$ were double-expanded.
20+
The collector now converts `$${ENV_VAR}` to `${ENV_VAR}` and `$$ENV_VAR` to `$ENV_VAR` without further expansion.
21+
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [api]

‎confmap/converter/expandconverter/expand.go

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ func (c converter) expandEnv(s string) (string, error) {
8585
// - $FOO will be substituted with env var FOO
8686
// - $$FOO will be replaced with $FOO
8787
// - $$$FOO will be replaced with $ + substituted env var FOO
88+
// TODO: Move the escaping of $$ out from the expand converter to the resolver.
8889
if str == "$" {
8990
return "$"
9091
}

‎confmap/expand_test.go

+15-15
Original file line numberDiff line numberDiff line change
@@ -583,21 +583,21 @@ func Test_EscapedEnvVars(t *testing.T) {
583583

584584
expectedMap := map[string]any{
585585
"test_map": map[string]any{
586-
"recv.1": "$MAP_VALUE_1",
587-
"recv.2": "$$MAP_VALUE_2",
588-
"recv.3": "$$MAP_VALUE_3",
589-
"recv.4": "$" + mapValue2,
590-
"recv.5": "some${MAP_VALUE_4}text",
591-
"recv.6": "${ONE}${TWO}",
592-
"recv.7": "text$",
593-
"recv.8": "$",
594-
"recv.9": "${1}${env:2}",
595-
"recv.10": "some${env:MAP_VALUE_4}text",
596-
"recv.11": "${env:" + mapValue2 + "}",
597-
"recv.12": "${env:${MAP_VALUE_2}}",
598-
"recv.13": "env:MAP_VALUE_2}${MAP_VALUE_2}{",
599-
"recv.14": "${env:MAP_VALUE_2${MAP_VALUE_2}",
600-
"recv.15": "$" + mapValue2,
586+
"recv.1": "$$MAP_VALUE_1",
587+
"recv.2": "$$$MAP_VALUE_2",
588+
"recv.3": "$$$$MAP_VALUE_3",
589+
"recv.4": "$$" + mapValue2,
590+
"recv.5": "some$${MAP_VALUE_4}text",
591+
"recv.6": "$${ONE}$${TWO}",
592+
"recv.7": "text$$",
593+
"recv.8": "$$",
594+
"recv.9": "$${1}$${env:2}",
595+
"recv.10": "some$${env:MAP_VALUE_4}text",
596+
"recv.11": "$${env:" + mapValue2 + "}",
597+
"recv.12": "$${env:$${MAP_VALUE_2}}",
598+
"recv.13": "env:MAP_VALUE_2}$${MAP_VALUE_2}{",
599+
"recv.14": "${env:MAP_VALUE_2$${MAP_VALUE_2}",
600+
"recv.15": "$$" + mapValue2,
601601
}}
602602

603603
fileProvider := newFakeProvider("file", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {

‎confmap/internal/e2e/expand_test.go

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package e2etest
2+
3+
import (
4+
"context"
5+
"go.opentelemetry.io/collector/confmap/provider/fileprovider"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"go.opentelemetry.io/collector/confmap"
13+
"go.opentelemetry.io/collector/confmap/converter/expandconverter"
14+
"go.opentelemetry.io/collector/confmap/provider/envprovider"
15+
)
16+
17+
// Test_EscapedEnvVars tests that the resolver supports escaped env vars working together with expand converter.
18+
func Test_EscapedEnvVars(t *testing.T) {
19+
tests := []struct {
20+
name string
21+
scheme string
22+
}{
23+
{
24+
name: "no_default_scheme",
25+
scheme: "",
26+
},
27+
{
28+
name: "env",
29+
scheme: "env",
30+
},
31+
}
32+
const expandedValue = "some expanded value"
33+
t.Setenv("ENV_VALUE", expandedValue)
34+
for _, tt := range tests {
35+
t.Run(tt.name, func(t *testing.T) {
36+
expectedMap := map[string]any{
37+
"test_map": map[string]any{
38+
"key1": "$ENV_VALUE",
39+
"key2": "$$ENV_VALUE",
40+
"key3": "$" + expandedValue,
41+
"key4": "some${ENV_VALUE}text",
42+
"key5": "${ONE}${TWO}",
43+
"key6": "text$",
44+
"key7": "$",
45+
"key8": "${1}${env:2}",
46+
"key9": "some${env:ENV_VALUE}text",
47+
"key10": "${env:" + expandedValue + "}",
48+
"key11": "${env:${ENV_VALUE}}",
49+
"key12": "env:MAP_VALUE_2}${ENV_VALUE}{",
50+
"key13": "$" + expandedValue,
51+
},
52+
}
53+
54+
resolver, err := confmap.NewResolver(confmap.ResolverSettings{
55+
URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")},
56+
ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory(), envprovider.NewFactory()},
57+
ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()},
58+
DefaultScheme: tt.scheme,
59+
})
60+
require.NoError(t, err)
61+
62+
// Test that expanded configs are the same with the simple config with no env vars.
63+
cfgMap, err := resolver.Resolve(context.Background())
64+
require.NoError(t, err)
65+
m := cfgMap.ToStringMap()
66+
assert.Equal(t, expectedMap, m)
67+
})
68+
}
69+
}

‎confmap/internal/e2e/go.mod

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.21.0
55
require (
66
github.com/stretchr/testify v1.9.0
77
go.opentelemetry.io/collector/confmap v0.105.0
8+
go.opentelemetry.io/collector/confmap/converter/expandconverter v0.105.0
89
go.opentelemetry.io/collector/confmap/provider/envprovider v0.105.0
910
go.opentelemetry.io/collector/confmap/provider/fileprovider v0.105.0
1011
go.opentelemetry.io/collector/featuregate v1.12.0
@@ -35,3 +36,5 @@ replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../prov
3536
replace go.opentelemetry.io/collector/featuregate => ../../../featuregate
3637

3738
replace go.opentelemetry.io/collector/internal/globalgates => ../../../internal/globalgates
39+
40+
replace go.opentelemetry.io/collector/confmap/converter/expandconverter => ../../converter/expandconverter

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

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
test_map:
2+
# $$ -> escaped $
3+
key1: "$$ENV_VALUE"
4+
# $$$$ -> two escaped $
5+
key2: "$$$$ENV_VALUE"
6+
# $$ -> escaped $ + ${ENV_VALUE} expanded
7+
key3: "$$${ENV_VALUE}"
8+
# escaped $ in the middle
9+
key4: "some$${ENV_VALUE}text"
10+
# two escaped $
11+
key5: "$${ONE}$${TWO}"
12+
# trailing escaped $
13+
key6: "text$$"
14+
# escaped $ alone
15+
key7: "$$"
16+
# escaped number and uri
17+
key8: "$${1}$${env:2}"
18+
# escape provider
19+
key9: "some$${env:ENV_VALUE}text"
20+
# can escape outer when nested
21+
key10: "$${env:${ENV_VALUE}}"
22+
# can escape inner and outer when nested
23+
key11: "$${env:$${ENV_VALUE}}"
24+
# can escape partial
25+
key12: "env:MAP_VALUE_2}$${ENV_VALUE}{"
26+
# $$$ -> escaped $ + expanded env var
27+
key13: "$$${env:ENV_VALUE}"

‎confmap/resolver.go

+1-9
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import (
1212

1313
"go.uber.org/multierr"
1414
"go.uber.org/zap"
15-
16-
"go.opentelemetry.io/collector/internal/globalgates"
1715
)
1816

1917
// follows drive-letter specification:
@@ -173,13 +171,7 @@ func (mr *Resolver) Resolve(ctx context.Context) (*Conf, error) {
173171
if err != nil {
174172
return nil, err
175173
}
176-
177-
if v, ok := val.(string); ok && globalgates.UseUnifiedEnvVarExpansionRules.IsEnabled() {
178-
cfgMap[k] = strings.ReplaceAll(v, "$$", "$")
179-
} else {
180-
cfgMap[k] = val
181-
}
182-
174+
cfgMap[k] = val
183175
}
184176
retMap = NewFromStringMap(cfgMap)
185177

0 commit comments

Comments
 (0)