Skip to content

Commit 06db8d7

Browse files
dmitryaxdmitryax
committed
[confmap] Fix expansion of escaped environment variables
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.
1 parent 49ea32b commit 06db8d7

File tree

9 files changed

+160
-83
lines changed

9 files changed

+160
-83
lines changed

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

Lines changed: 27 additions & 0 deletions
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -577,46 +577,3 @@ func TestResolverDefaultProviderExpand(t *testing.T) {
577577
require.NoError(t, err)
578578
assert.Equal(t, map[string]any{"foo": "localhost"}, cfgMap.ToStringMap())
579579
}
580-
581-
func Test_EscapedEnvVars(t *testing.T) {
582-
const mapValue2 = "some map value"
583-
584-
expectedMap := map[string]any{
585-
"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,
601-
}}
602-
603-
fileProvider := newFakeProvider("file", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
604-
return NewRetrieved(newConfFromFile(t, uri[5:]))
605-
})
606-
envProvider := newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
607-
if uri == "env:MAP_VALUE_2" {
608-
return NewRetrieved(mapValue2)
609-
}
610-
return nil, errors.New("should not be expanding any other env vars")
611-
})
612-
613-
resolver, err := NewResolver(ResolverSettings{URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")}, ProviderFactories: []ProviderFactory{fileProvider, envProvider}, ConverterFactories: nil, DefaultScheme: "env"})
614-
require.NoError(t, err)
615-
616-
// Test that expanded configs are the same with the simple config with no env vars.
617-
cfgMap, err := resolver.Resolve(context.Background())
618-
require.NoError(t, err)
619-
m := cfgMap.ToStringMap()
620-
assert.Equal(t, expectedMap, m)
621-
622-
}

‎confmap/internal/e2e/expand_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package e2etest
5+
6+
import (
7+
"context"
8+
"fmt"
9+
"path/filepath"
10+
"testing"
11+
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
15+
"go.opentelemetry.io/collector/confmap"
16+
"go.opentelemetry.io/collector/confmap/converter/expandconverter"
17+
"go.opentelemetry.io/collector/confmap/provider/envprovider"
18+
"go.opentelemetry.io/collector/confmap/provider/fileprovider"
19+
"go.opentelemetry.io/collector/confmap/provider/yamlprovider"
20+
)
21+
22+
// Test_EscapedEnvVars tests that the resolver supports escaped env vars working together with expand converter.
23+
func Test_EscapedEnvVars(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
scheme string
27+
}{
28+
{
29+
name: "no_default_scheme",
30+
scheme: "",
31+
},
32+
{
33+
name: "env",
34+
scheme: "env",
35+
},
36+
}
37+
38+
const expandedValue = "some expanded value"
39+
t.Setenv("ENV_VALUE", expandedValue)
40+
41+
expectedFailures := map[string]string{
42+
"$ENV_VALUE": "variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR}",
43+
"$$$ENV_VALUE": "variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR}",
44+
}
45+
46+
for _, tt := range tests {
47+
t.Run(tt.name, func(t *testing.T) {
48+
expectedMap := map[string]any{
49+
"test_map": map[string]any{
50+
"key1": "$ENV_VALUE",
51+
"key2": "$$ENV_VALUE",
52+
"key3": "$" + expandedValue,
53+
"key4": "some" + expandedValue + "text",
54+
"key5": "some${ENV_VALUE}text",
55+
"key6": "${ONE}${TWO}",
56+
"key7": "text$",
57+
"key8": "$",
58+
"key9": "${1}${env:2}",
59+
"key10": "some${env:ENV_VALUE}text",
60+
"key11": "${env:" + expandedValue + "}",
61+
"key12": "${env:${ENV_VALUE}}",
62+
"key13": "env:MAP_VALUE_2}${ENV_VALUE}{",
63+
"key14": "$" + expandedValue,
64+
},
65+
}
66+
67+
resolver, err := confmap.NewResolver(confmap.ResolverSettings{
68+
URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")},
69+
ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory(), envprovider.NewFactory()},
70+
ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()},
71+
DefaultScheme: tt.scheme,
72+
})
73+
require.NoError(t, err)
74+
75+
// Test that expanded configs are the same with the simple config with no env vars.
76+
cfgMap, err := resolver.Resolve(context.Background())
77+
require.NoError(t, err)
78+
m := cfgMap.ToStringMap()
79+
assert.Equal(t, expectedMap, m)
80+
81+
for val, expectedErr := range expectedFailures {
82+
resolver, err = confmap.NewResolver(confmap.ResolverSettings{
83+
URIs: []string{fmt.Sprintf("yaml: test: %s", val)},
84+
ProviderFactories: []confmap.ProviderFactory{yamlprovider.NewFactory(), envprovider.NewFactory()},
85+
ConverterFactories: []confmap.ConverterFactory{expandconverter.NewFactory()},
86+
DefaultScheme: tt.scheme,
87+
})
88+
require.NoError(t, err)
89+
_, err := resolver.Resolve(context.Background())
90+
require.ErrorContains(t, err, expectedErr)
91+
}
92+
})
93+
}
94+
}

‎confmap/internal/e2e/go.mod

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ 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
11+
go.opentelemetry.io/collector/confmap/provider/yamlprovider v0.105.0
1012
go.opentelemetry.io/collector/featuregate v1.12.0
1113
go.opentelemetry.io/collector/internal/globalgates v0.105.0
1214
)
@@ -32,6 +34,10 @@ replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../pro
3234

3335
replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
3436

37+
replace go.opentelemetry.io/collector/confmap/provider/yamlprovider => ../../provider/yamlprovider
38+
3539
replace go.opentelemetry.io/collector/featuregate => ../../../featuregate
3640

3741
replace go.opentelemetry.io/collector/internal/globalgates => ../../../internal/globalgates
42+
43+
replace go.opentelemetry.io/collector/confmap/converter/expandconverter => ../../converter/expandconverter

‎confmap/internal/e2e/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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+
# expanded in the middle
9+
key4: "some${ENV_VALUE}text"
10+
# escaped $ in the middle
11+
key5: "some$${ENV_VALUE}text"
12+
# two escaped $
13+
key6: "$${ONE}$${TWO}"
14+
# trailing escaped $
15+
key7: "text$$"
16+
# escaped $ alone
17+
key8: "$$"
18+
# escaped number and uri
19+
key9: "$${1}$${env:2}"
20+
# escape provider
21+
key10: "some$${env:ENV_VALUE}text"
22+
# can escape outer when nested
23+
key11: "$${env:${ENV_VALUE}}"
24+
# can escape inner and outer when nested
25+
key12: "$${env:$${ENV_VALUE}}"
26+
# can escape partial
27+
key13: "env:MAP_VALUE_2}$${ENV_VALUE}{"
28+
# $$$ -> escaped $ + expanded env var
29+
key14: "$$${env:ENV_VALUE}"

‎confmap/resolver.go

Lines changed: 1 addition & 9 deletions
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

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

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)