diff --git a/.chloggen/unify-env-var-gate-stable.yaml b/.chloggen/unify-env-var-gate-stable.yaml new file mode 100644 index 00000000000..e75a001f686 --- /dev/null +++ b/.chloggen/unify-env-var-gate-stable.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# 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: Set the `confmap.unifyEnvVarExpansion` feature gate to Stable. Expansion of `$FOO` env vars is no longer supported. Use `${FOO}` or `${env:FOO}` instead. + +# One or more tracking issues or pull requests related to the change +issues: [10508] + +# (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: + +# 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: [] diff --git a/confmap/converter/expandconverter/expand_test.go b/confmap/converter/expandconverter/expand_test.go deleted file mode 100644 index 4c7ff254e15..00000000000 --- a/confmap/converter/expandconverter/expand_test.go +++ /dev/null @@ -1,367 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package expandconverter - -import ( - "context" - "fmt" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" - "go.uber.org/zap/zaptest/observer" - - "go.opentelemetry.io/collector/confmap" - "go.opentelemetry.io/collector/confmap/confmaptest" - "go.opentelemetry.io/collector/confmap/internal/envvar" - "go.opentelemetry.io/collector/featuregate" - "go.opentelemetry.io/collector/internal/globalgates" -) - -func TestNewExpandConverter(t *testing.T) { - var testCases = []struct { - name string // test case name (also file name containing config yaml) - }{ - {name: "expand-with-no-env.yaml"}, - {name: "expand-with-partial-env.yaml"}, - {name: "expand-with-all-env.yaml"}, - } - - const valueExtra = "some string" - const valueExtraMapValue = "some map value" - const valueExtraListMapValue = "some list map value" - const valueExtraListElement = "some list value" - t.Setenv("EXTRA", valueExtra) - t.Setenv("EXTRA_MAP_VALUE_1", valueExtraMapValue+"_1") - t.Setenv("EXTRA_MAP_VALUE_2", valueExtraMapValue+"_2") - t.Setenv("EXTRA_LIST_MAP_VALUE_1", valueExtraListMapValue+"_1") - t.Setenv("EXTRA_LIST_MAP_VALUE_2", valueExtraListMapValue+"_2") - t.Setenv("EXTRA_LIST_VALUE_1", valueExtraListElement+"_1") - t.Setenv("EXTRA_LIST_VALUE_2", valueExtraListElement+"_2") - - expectedCfgMap, errExpected := confmaptest.LoadConf(filepath.Join("testdata", "expand-with-no-env.yaml")) - require.NoError(t, errExpected, "Unable to get expected config") - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), false)) - t.Cleanup(func() { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), true)) - }) - - conf, err := confmaptest.LoadConf(filepath.Join("testdata", test.name)) - require.NoError(t, err, "Unable to get config") - - // Test that expanded configs are the same with the simple config with no env vars. - require.NoError(t, createConverter().Convert(context.Background(), conf)) - assert.Equal(t, expectedCfgMap.ToStringMap(), conf.ToStringMap()) - }) - } -} - -func TestNewExpandConverter_UseUnifiedEnvVarExpansionRules(t *testing.T) { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), true)) - t.Cleanup(func() { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), false)) - }) - - const valueExtra = "some string" - const valueExtraMapValue = "some map value" - const valueExtraListMapValue = "some list map value" - const valueExtraListElement = "some list value" - t.Setenv("EXTRA", valueExtra) - t.Setenv("EXTRA_MAP_VALUE_1", valueExtraMapValue+"_1") - t.Setenv("EXTRA_MAP_VALUE_2", valueExtraMapValue+"_2") - t.Setenv("EXTRA_LIST_MAP_VALUE_1", valueExtraListMapValue+"_1") - t.Setenv("EXTRA_LIST_MAP_VALUE_2", valueExtraListMapValue+"_2") - t.Setenv("EXTRA_LIST_VALUE_1", valueExtraListElement+"_1") - t.Setenv("EXTRA_LIST_VALUE_2", valueExtraListElement+"_2") - - conf, err := confmaptest.LoadConf(filepath.Join("testdata", "expand-with-all-env.yaml")) - require.NoError(t, err, "Unable to get config") - - // Test that expanded configs are the same with the simple config with no env vars. - require.ErrorContains(t, createConverter().Convert(context.Background(), conf), "variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR}") -} - -func TestNewExpandConverter_EscapedMaps(t *testing.T) { - const receiverExtraMapValue = "some map value" - t.Setenv("MAP_VALUE", receiverExtraMapValue) - - conf := confmap.NewFromStringMap( - map[string]any{ - "test_string_map": map[string]any{ - "recv": "$MAP_VALUE", - }, - "test_interface_map": map[any]any{ - "recv": "$MAP_VALUE", - }}, - ) - require.NoError(t, createConverter().Convert(context.Background(), conf)) - - expectedMap := map[string]any{ - "test_string_map": map[string]any{ - "recv": receiverExtraMapValue, - }, - "test_interface_map": map[string]any{ - "recv": receiverExtraMapValue, - }} - assert.Equal(t, expectedMap, conf.ToStringMap()) -} - -func TestNewExpandConverter_EscapedEnvVars(t *testing.T) { - const receiverExtraMapValue = "some map value" - t.Setenv("MAP_VALUE_2", receiverExtraMapValue) - - // Retrieve the config - conf, err := confmaptest.LoadConf(filepath.Join("testdata", "expand-escaped-env.yaml")) - require.NoError(t, err, "Unable to get config") - - expectedMap := map[string]any{ - "test_map": map[string]any{ - // $$ -> escaped $ - "recv.1": "$MAP_VALUE_1", - // $$$ -> escaped $ + substituted env var - "recv.2": "$" + receiverExtraMapValue, - // $$$$ -> two escaped $ - "recv.3": "$$MAP_VALUE_3", - // escaped $ in the middle - "recv.4": "some${MAP_VALUE_4}text", - // $$$$ -> two escaped $ - "recv.5": "${ONE}${TWO}", - // trailing escaped $ - "recv.6": "text$", - // escaped $ alone - "recv.7": "$", - }} - require.NoError(t, createConverter().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, createConverter().Convert(context.Background(), conf)) - assert.Equal(t, tt.expected, conf.ToStringMap()) - }) - } -} - -func NewTestConverter() (confmap.Converter, *observer.ObservedLogs) { - core, logs := observer.New(zapcore.InfoLevel) - conv := converter{loggedDeprecations: make(map[string]struct{}), logger: zap.New(core)} - return conv, logs -} - -func TestDeprecatedWarning(t *testing.T) { - msgTemplate := `Variable substitution using $VAR will be deprecated in favor of ${VAR} and ${env:VAR}, please update $%s` - t.Setenv("HOST", "127.0.0.1") - t.Setenv("PORT", "4317") - - t.Setenv("HOST_NAME", "127.0.0.2") - t.Setenv("HOSTNAME", "127.0.0.3") - - t.Setenv("BAD!HOST", "127.0.0.2") - - var testCases = []struct { - name string - input map[string]any - expectedOutput map[string]any - expectedWarnings []string - expectedError error - }{ - { - name: "no warning", - input: map[string]any{ - "test": "${HOST}:${PORT}", - }, - expectedOutput: map[string]any{ - "test": "127.0.0.1:4317", - }, - expectedWarnings: []string{}, - expectedError: nil, - }, - { - name: "one deprecated var", - input: map[string]any{ - "test": "${HOST}:$PORT", - }, - expectedOutput: map[string]any{ - "test": "127.0.0.1:4317", - }, - expectedWarnings: []string{"PORT"}, - expectedError: nil, - }, - { - name: "two deprecated vars", - input: map[string]any{ - "test": "$HOST:$PORT", - }, - expectedOutput: map[string]any{ - "test": "127.0.0.1:4317", - }, - expectedWarnings: []string{"HOST", "PORT"}, - expectedError: nil, - }, - { - name: "one depracated serveral times", - input: map[string]any{ - "test": "$HOST,$HOST", - "test2": "$HOST", - }, - expectedOutput: map[string]any{ - "test": "127.0.0.1,127.0.0.1", - "test2": "127.0.0.1", - }, - expectedWarnings: []string{"HOST"}, - expectedError: nil, - }, - { - name: "one warning", - input: map[string]any{ - "test": "$HOST_NAME,${HOSTNAME}", - }, - expectedOutput: map[string]any{ - "test": "127.0.0.2,127.0.0.3", - }, - expectedWarnings: []string{"HOST_NAME"}, - expectedError: nil, - }, - { - name: "malformed environment variable", - input: map[string]any{ - "test": "${BAD!HOST}", - }, - expectedOutput: map[string]any{ - "test": "blah", - }, - expectedWarnings: []string{}, - expectedError: fmt.Errorf("environment variable \"BAD!HOST\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - { - name: "malformed environment variable number", - input: map[string]any{ - "test": "${2BADHOST}", - }, - expectedOutput: map[string]any{ - "test": "blah", - }, - expectedWarnings: []string{}, - expectedError: fmt.Errorf("environment variable \"2BADHOST\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - { - name: "malformed environment variable unicode", - input: map[string]any{ - "test": "${😊BADHOST}", - }, - expectedOutput: map[string]any{ - "test": "blah", - }, - expectedWarnings: []string{}, - expectedError: fmt.Errorf("environment variable \"😊BADHOST\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - conf := confmap.NewFromStringMap(tt.input) - conv, logs := NewTestConverter() - err := conv.Convert(context.Background(), conf) - assert.Equal(t, tt.expectedError, err) - if tt.expectedError == nil { - assert.Equal(t, tt.expectedOutput, conf.ToStringMap()) - } - assert.Equal(t, len(tt.expectedWarnings), len(logs.All())) - for i, variable := range tt.expectedWarnings { - errorMsg := fmt.Sprintf(msgTemplate, variable) - assert.Equal(t, errorMsg, logs.All()[i].Message) - } - }) - } -} - -func TestNewExpandConverterWithErrors(t *testing.T) { - var testCases = []struct { - name string // test case name (also file name containing config yaml) - expectedError error - }{ - { - name: "expand-list-error.yaml", - expectedError: fmt.Errorf("environment variable \"EXTRA_LIST_^VALUE_2\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - { - name: "expand-list-map-error.yaml", - expectedError: fmt.Errorf("environment variable \"EXTRA_LIST_MAP_V#ALUE_2\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - { - name: "expand-map-error.yaml", - expectedError: fmt.Errorf("environment variable \"EX#TRA\" has invalid name: must match regex %s", envvar.ValidationRegexp), - }, - } - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - conf, err := confmaptest.LoadConf(filepath.Join("testdata", "errors", test.name)) - require.NoError(t, err, "Unable to get config") - - // Test that expanded configs are the same with the simple config with no env vars. - err = createConverter().Convert(context.Background(), conf) - - assert.Equal(t, test.expectedError, err) - }) - } -} - -func createConverter() confmap.Converter { - return NewFactory().Create(confmap.ConverterSettings{Logger: zap.NewNop()}) -} diff --git a/confmap/converter/expandconverter/go.mod b/confmap/converter/expandconverter/go.mod index f6b64102757..a0b3216d4b4 100644 --- a/confmap/converter/expandconverter/go.mod +++ b/confmap/converter/expandconverter/go.mod @@ -3,16 +3,13 @@ module go.opentelemetry.io/collector/confmap/converter/expandconverter go 1.21.0 require ( - github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/confmap v0.106.1 - go.opentelemetry.io/collector/featuregate v1.12.0 go.opentelemetry.io/collector/internal/globalgates v0.106.1 go.uber.org/goleak v1.3.0 go.uber.org/zap v1.27.0 ) require ( - github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-viper/mapstructure/v2 v2.0.0 // indirect github.com/hashicorp/go-version v1.7.0 // indirect github.com/knadh/koanf/maps v0.1.1 // indirect @@ -20,7 +17,7 @@ require ( github.com/knadh/koanf/v2 v2.1.1 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect + go.opentelemetry.io/collector/featuregate v1.12.0 // indirect go.uber.org/multierr v1.11.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/confmap/internal/e2e/expand_test.go b/confmap/internal/e2e/expand_test.go index 018725d30d5..b6a41fafc26 100644 --- a/confmap/internal/e2e/expand_test.go +++ b/confmap/internal/e2e/expand_test.go @@ -5,7 +5,6 @@ package e2etest import ( "context" - "fmt" "path/filepath" "testing" @@ -13,82 +12,90 @@ import ( "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", - }, - } - +func Test_EscapedEnvVars_NoDefaultScheme(t *testing.T) { const expandedValue = "some expanded value" t.Setenv("ENV_VALUE", expandedValue) + t.Setenv("ENV_LIST", "['$$ESCAPE_ME','$${ESCAPE_ME}','$${env:ESCAPE_ME}']") + t.Setenv("ENV_MAP", "{'key1':'$$ESCAPE_ME','key2':'$${ESCAPE_ME}','key3':'$${env:ESCAPE_ME}'}") - 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}", + expectedMap := map[string]any{ + "test_map": map[string]any{ + "key1": "$ENV_VALUE", + "key2": "$$ENV_VALUE", + "key3": "$${ENV_VALUE}", + "key4": "some${ENV_VALUE}text", + "key5": "some${ENV_VALUE}text", + "key6": "${ONE}${TWO}", + "key7": "text$", + "key8": "$", + "key9": "${1}${env:2}", + "key10": "some${env:ENV_VALUE}text", + "key11": "${env:${ENV_VALUE}}", + "key12": "${env:${ENV_VALUE}}", + "key13": "env:MAP_VALUE_2}${ENV_VALUE}{", + "key14": "$" + expandedValue, + "key15": "$ENV_VALUE", + "key16": []any{"$ESCAPE_ME", "${ESCAPE_ME}", "${env:ESCAPE_ME}"}, + "key17": map[string]any{"key1": "$ESCAPE_ME", "key2": "${ESCAPE_ME}", "key3": "${env:ESCAPE_ME}"}, + }, } - 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()}, + DefaultScheme: "", + }) + require.NoError(t, err) - 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) +} - // 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) +func Test_EscapedEnvVars_DefaultScheme(t *testing.T) { + const expandedValue = "some expanded value" + t.Setenv("ENV_VALUE", expandedValue) + t.Setenv("ENV_LIST", "['$$ESCAPE_ME','$${ESCAPE_ME}','$${env:ESCAPE_ME}']") + t.Setenv("ENV_MAP", "{'key1':'$$ESCAPE_ME','key2':'$${ESCAPE_ME}','key3':'$${env:ESCAPE_ME}'}") - 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) - } - }) + 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, + "key15": "$ENV_VALUE", + "key16": []any{"$ESCAPE_ME", "${ESCAPE_ME}", "${env:ESCAPE_ME}"}, + "key17": map[string]any{"key1": "$ESCAPE_ME", "key2": "${ESCAPE_ME}", "key3": "${env:ESCAPE_ME}"}, + }, } + + resolver, err := confmap.NewResolver(confmap.ResolverSettings{ + URIs: []string{filepath.Join("testdata", "expand-escaped-env.yaml")}, + ProviderFactories: []confmap.ProviderFactory{fileprovider.NewFactory(), envprovider.NewFactory()}, + 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) } diff --git a/confmap/internal/e2e/go.mod b/confmap/internal/e2e/go.mod index 6b7913c671b..1792377658f 100644 --- a/confmap/internal/e2e/go.mod +++ b/confmap/internal/e2e/go.mod @@ -5,10 +5,8 @@ go 1.21.0 require ( github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/confmap v0.106.1 - go.opentelemetry.io/collector/confmap/converter/expandconverter v0.106.1 go.opentelemetry.io/collector/confmap/provider/envprovider v0.106.1 go.opentelemetry.io/collector/confmap/provider/fileprovider v0.106.1 - go.opentelemetry.io/collector/confmap/provider/yamlprovider v0.106.1 go.opentelemetry.io/collector/featuregate v1.12.0 go.opentelemetry.io/collector/internal/globalgates v0.106.1 ) @@ -34,10 +32,6 @@ 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 diff --git a/confmap/internal/e2e/testdata/expand-escaped-env.yaml b/confmap/internal/e2e/testdata/expand-escaped-env.yaml index ae8cb280ede..74a1d9fa2af 100644 --- a/confmap/internal/e2e/testdata/expand-escaped-env.yaml +++ b/confmap/internal/e2e/testdata/expand-escaped-env.yaml @@ -27,3 +27,9 @@ test_map: key13: "env:MAP_VALUE_2}$${ENV_VALUE}{" # $$$ -> escaped $ + expanded env var key14: "$$${env:ENV_VALUE}" + # $ -> $ + key15: "$ENV_VALUE" + # list is escaped + key16: "${env:ENV_LIST}" + # map is escaped + key17: "${env:ENV_MAP}" diff --git a/confmap/internal/e2e/types_test.go b/confmap/internal/e2e/types_test.go index 88dbe5a7162..2d72b624313 100644 --- a/confmap/internal/e2e/types_test.go +++ b/confmap/internal/e2e/types_test.go @@ -47,6 +47,7 @@ func NewResolver(t testing.TB, path string) *confmap.Resolver { fileprovider.NewFactory(), envprovider.NewFactory(), }, + DefaultScheme: "env", }) require.NoError(t, err) return resolver @@ -244,6 +245,8 @@ func TestTypeCasting(t *testing.T) { } func TestStrictTypeCasting(t *testing.T) { + t.Setenv("ENV_VALUE", "testreceiver") + values := []Test{ { value: "123", @@ -393,6 +396,66 @@ func TestStrictTypeCasting(t *testing.T) { targetField: TargetFieldInlineString, expected: "inline field with [filelog,windowseventlog/application] expansion", }, + { + value: "$$ENV", + targetField: TargetFieldString, + expected: "$ENV", + }, + { + value: "$$ENV", + targetField: TargetFieldInlineString, + expected: "inline field with $ENV expansion", + }, + { + value: "$${ENV}", + targetField: TargetFieldString, + expected: "${ENV}", + }, + { + value: "$${ENV}", + targetField: TargetFieldInlineString, + expected: "inline field with ${ENV} expansion", + }, + { + value: "$${env:ENV}", + targetField: TargetFieldString, + expected: "${env:ENV}", + }, + { + value: "$${env:ENV}", + targetField: TargetFieldInlineString, + expected: "inline field with ${env:ENV} expansion", + }, + { + value: `[filelog,${env:ENV_VALUE}]`, + targetField: TargetFieldString, + expected: "[filelog,testreceiver]", + }, + { + value: `[filelog,${ENV_VALUE}]`, + targetField: TargetFieldString, + expected: "[filelog,testreceiver]", + }, + { + value: `["filelog","$${env:ENV_VALUE}"]`, + targetField: TargetFieldString, + expected: `["filelog","${env:ENV_VALUE}"]`, + }, + { + value: `["filelog","$${ENV_VALUE}"]`, + targetField: TargetFieldString, + expected: `["filelog","${ENV_VALUE}"]`, + }, + { + value: `["filelog","$$ENV_VALUE"]`, + targetField: TargetFieldString, + expected: `["filelog","$ENV_VALUE"]`, + }, + { + value: `["filelog","$ENV_VALUE"]`, + targetField: TargetFieldString, + expected: `["filelog","$ENV_VALUE"]`, + }, } previousValue := globalgates.StrictlyTypedInputGate.IsEnabled() @@ -595,7 +658,58 @@ func TestStructMappingIssue10787(t *testing.T) { }() resolver := NewResolver(t, "types_expand.yaml") - t.Setenv("ENV", `# ${hello.world} + t.Setenv("ENV", `# this is a comment +logging: + verbosity: detailed`) + conf, err := resolver.Resolve(context.Background()) + require.NoError(t, err) + + type Logging struct { + Verbosity string `mapstructure:"verbosity"` + } + type Exporters struct { + Logging Logging `mapstructure:"logging"` + } + type Target struct { + Field Exporters `mapstructure:"field"` + } + + var cfg Target + err = conf.Unmarshal(&cfg) + require.NoError(t, err) + require.Equal(t, + Target{Field: Exporters{ + Logging: Logging{ + Verbosity: "detailed", + }, + }}, + cfg, + ) + + confStr, err := resolver.Resolve(context.Background()) + require.NoError(t, err) + var cfgStr TargetConfig[string] + err = confStr.Unmarshal(&cfgStr) + require.NoError(t, err) + require.Equal(t, `# this is a comment +logging: + verbosity: detailed`, + cfgStr.Field, + ) +} + +func TestStructMappingIssue10787_ExpandComment(t *testing.T) { + previousValue := globalgates.StrictlyTypedInputGate.IsEnabled() + err := featuregate.GlobalRegistry().Set(globalgates.StrictlyTypedInputID, true) + require.NoError(t, err) + defer func() { + seterr := featuregate.GlobalRegistry().Set(globalgates.StrictlyTypedInputID, previousValue) + require.NoError(t, seterr) + }() + + resolver := NewResolver(t, "types_expand.yaml") + t.Setenv("EXPAND_ME", "an expanded env var") + t.Setenv("ENV", `# this is a comment with ${EXPAND_ME} logging: verbosity: detailed`) conf, err := resolver.Resolve(context.Background()) @@ -628,7 +742,7 @@ logging: var cfgStr TargetConfig[string] err = confStr.Unmarshal(&cfgStr) require.NoError(t, err) - require.Equal(t, `# ${hello.world} + require.Equal(t, `# this is a comment with an expanded env var logging: verbosity: detailed`, cfgStr.Field, diff --git a/confmap/resolver.go b/confmap/resolver.go index f3a7c7092e3..e635ea99564 100644 --- a/confmap/resolver.go +++ b/confmap/resolver.go @@ -181,7 +181,7 @@ func (mr *Resolver) Resolve(ctx context.Context) (*Conf, error) { if err != nil { return nil, err } - cfgMap[k] = val + cfgMap[k] = escapeDollarSigns(val) } retMap = NewFromStringMap(cfgMap) @@ -195,6 +195,31 @@ func (mr *Resolver) Resolve(ctx context.Context) (*Conf, error) { return retMap, nil } +func escapeDollarSigns(val any) any { + switch v := val.(type) { + case string: + return strings.ReplaceAll(v, "$$", "$") + case expandedValue: + v.Original = strings.ReplaceAll(v.Original, "$$", "$") + v.Value = escapeDollarSigns(v.Value) + return v + case []any: + nslice := make([]any, len(v)) + for i, x := range v { + nslice[i] = escapeDollarSigns(x) + } + return nslice + case map[string]any: + nmap := make(map[string]any, len(v)) + for k, x := range v { + nmap[k] = escapeDollarSigns(x) + } + return nmap + default: + return val + } +} + // Watch blocks until any configuration change was detected or an unrecoverable error // happened during monitoring the configuration changes. // diff --git a/internal/globalgates/globalgates.go b/internal/globalgates/globalgates.go index 6cf27950b15..346bd2d64fd 100644 --- a/internal/globalgates/globalgates.go +++ b/internal/globalgates/globalgates.go @@ -6,8 +6,9 @@ package globalgates // import "go.opentelemetry.io/collector/internal/globalgate import "go.opentelemetry.io/collector/featuregate" var UseUnifiedEnvVarExpansionRules = featuregate.GlobalRegistry().MustRegister("confmap.unifyEnvVarExpansion", - featuregate.StageBeta, + featuregate.StageStable, featuregate.WithRegisterFromVersion("v0.103.0"), + featuregate.WithRegisterToVersion("v0.109.0"), featuregate.WithRegisterDescription("`${FOO}` will now be expanded as if it was `${env:FOO}` and no longer expands $ENV syntax. See https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/env-vars.md for more details. When this feature gate is stable, expandconverter will be removed.")) const StrictlyTypedInputID = "confmap.strictlyTypedInput" diff --git a/otelcol/command_test.go b/otelcol/command_test.go index 42b2212b06e..1a5680fe4ab 100644 --- a/otelcol/command_test.go +++ b/otelcol/command_test.go @@ -15,7 +15,6 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/featuregate" - "go.opentelemetry.io/collector/internal/globalgates" ) func TestNewCommandVersion(t *testing.T) { @@ -139,10 +138,6 @@ func Test_UseUnifiedEnvVarExpansionRules(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), true)) - t.Cleanup(func() { - require.NoError(t, featuregate.GlobalRegistry().Set(globalgates.UseUnifiedEnvVarExpansionRules.ID(), false)) - }) fileProvider := newFakeProvider("file", func(_ context.Context, _ string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { return &confmap.Retrieved{}, nil }) diff --git a/otelcol/command_validate_test.go b/otelcol/command_validate_test.go index 03f93f51d16..9c0240910bc 100644 --- a/otelcol/command_validate_test.go +++ b/otelcol/command_validate_test.go @@ -30,6 +30,7 @@ func TestValidateSubCommandInvalidComponents(t *testing.T) { ResolverSettings: confmap.ResolverSettings{ URIs: []string{filePath}, ProviderFactories: []confmap.ProviderFactory{fileProvider}, + DefaultScheme: "file", }, }}, flags(featuregate.GlobalRegistry())) err := cmd.Execute()