From 9524644969edf1e0889688f6211ddc63becfef06 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 28 Jun 2024 01:25:31 -0700 Subject: [PATCH] [confmap] Move confmap.unifyEnvVarExpansion to beta (#10435) #### Description Moves confmap.unifyEnvVarExpansion to beta. This means the collector will, by default, use the env var provider to expand `${FOO}` synatx and will error if the expandconverter is used to expand `$FOO` syntax. #### Link to tracking issue Related to https://github.com/open-telemetry/opentelemetry-collector/issues/10161 Related to https://github.com/open-telemetry/opentelemetry-collector/issues/8215 Related to https://github.com/open-telemetry/opentelemetry-collector/issues/7111 --------- Co-authored-by: Pablo Baeyens --- ...fmap-unifyEnvVarExpansion-gate-beta-2.yaml | 25 +++++++++++++++++++ ...onfmap-unifyEnvVarExpansion-gate-beta.yaml | 25 +++++++++++++++++++ confmap/converter/expandconverter/expand.go | 3 +-- .../converter/expandconverter/expand_test.go | 7 +++++- internal/featuregates/featuregates.go | 2 +- otelcol/command_test.go | 1 + 6 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 .chloggen/confmap-unifyEnvVarExpansion-gate-beta-2.yaml create mode 100644 .chloggen/confmap-unifyEnvVarExpansion-gate-beta.yaml diff --git a/.chloggen/confmap-unifyEnvVarExpansion-gate-beta-2.yaml b/.chloggen/confmap-unifyEnvVarExpansion-gate-beta-2.yaml new file mode 100644 index 00000000000..82eaabe4c5d --- /dev/null +++ b/.chloggen/confmap-unifyEnvVarExpansion-gate-beta-2.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: otelcol + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: By default, `otelcol.NewCommand` and `otelcol.NewCommandMustSetProvider` will set the `DefaultScheme` to `env`. + +# One or more tracking issues or pull requests related to the change +issues: [10435] + +# (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/.chloggen/confmap-unifyEnvVarExpansion-gate-beta.yaml b/.chloggen/confmap-unifyEnvVarExpansion-gate-beta.yaml new file mode 100644 index 00000000000..b5320642cc5 --- /dev/null +++ b/.chloggen/confmap-unifyEnvVarExpansion-gate-beta.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: expandconverter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: By default expandconverter will now error if it is about to expand `$FOO` syntax. Update configuration to use `${env:FOO}` instead or disable the feature gate. + +# One or more tracking issues or pull requests related to the change +issues: [10435] + +# (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.go b/confmap/converter/expandconverter/expand.go index 38b84140674..aa18d1a3817 100644 --- a/confmap/converter/expandconverter/expand.go +++ b/confmap/converter/expandconverter/expand.go @@ -5,7 +5,6 @@ package expandconverter // import "go.opentelemetry.io/collector/confmap/convert import ( "context" - "errors" "fmt" "os" "regexp" @@ -95,7 +94,7 @@ func (c converter) expandEnv(s string) (string, error) { var regex = regexp.MustCompile(fmt.Sprintf(`\$%s`, regexp.QuoteMeta(str))) if _, exists := c.loggedDeprecations[str]; !exists && regex.MatchString(s) { if featuregates.UseUnifiedEnvVarExpansionRules.IsEnabled() { - err = errors.New("$VAR expansion is not supported when feature gate confmap.unifyEnvVarExpansion is enabled") + err = fmt.Errorf("variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR} - please update $%s or temporarily disable the confmap.unifyEnvVarExpansion feature gate", str) return "" } msg := fmt.Sprintf("Variable substitution using $VAR will be deprecated in favor of ${VAR} and ${env:VAR}, please update $%s", str) diff --git a/confmap/converter/expandconverter/expand_test.go b/confmap/converter/expandconverter/expand_test.go index 6cd7fb881ee..ab2fa49c549 100644 --- a/confmap/converter/expandconverter/expand_test.go +++ b/confmap/converter/expandconverter/expand_test.go @@ -48,6 +48,11 @@ func TestNewExpandConverter(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { + require.NoError(t, featuregate.GlobalRegistry().Set(featuregates.UseUnifiedEnvVarExpansionRules.ID(), false)) + t.Cleanup(func() { + require.NoError(t, featuregate.GlobalRegistry().Set(featuregates.UseUnifiedEnvVarExpansionRules.ID(), true)) + }) + conf, err := confmaptest.LoadConf(filepath.Join("testdata", test.name)) require.NoError(t, err, "Unable to get config") @@ -80,7 +85,7 @@ func TestNewExpandConverter_UseUnifiedEnvVarExpansionRules(t *testing.T) { 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), "$VAR expansion is not supported when feature gate confmap.unifyEnvVarExpansion is enabled") + 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) { diff --git a/internal/featuregates/featuregates.go b/internal/featuregates/featuregates.go index da2017f5831..e74c75fe481 100644 --- a/internal/featuregates/featuregates.go +++ b/internal/featuregates/featuregates.go @@ -6,6 +6,6 @@ package featuregates // import "go.opentelemetry.io/collector/internal/featurega import "go.opentelemetry.io/collector/featuregate" var UseUnifiedEnvVarExpansionRules = featuregate.GlobalRegistry().MustRegister("confmap.unifyEnvVarExpansion", - featuregate.StageAlpha, + featuregate.StageBeta, featuregate.WithRegisterFromVersion("v0.103.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.")) diff --git a/otelcol/command_test.go b/otelcol/command_test.go index 388df2591d2..49fc1113970 100644 --- a/otelcol/command_test.go +++ b/otelcol/command_test.go @@ -36,6 +36,7 @@ func TestNewCommandProgrammaticallyPassedConfig(t *testing.T) { cmd := NewCommandMustSetProvider(CollectorSettings{Factories: nopFactories, ConfigProviderSettings: ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ ProviderFactories: []confmap.ProviderFactory{confmap.NewProviderFactory(newFailureProvider)}, + DefaultScheme: "file", }, }}) otelRunE := cmd.RunE