Skip to content

Commit ad2c979

Browse files
authored
[confmap] Add featuregate to use stable expansion rules (#10391)
#### Description This PR adds a feature gate that will handle transitioning users away from expandconverter, specifically expanding `$VAR` syntax. The wholistic strategy is: 1. create a new feature gate, `confmap.unifyEnvVarExpansion`, that will be the single feature gate to manage unifying collector configuraiton resolution. 2. Update expandconverter to return an error if the feature gate is enabled and it is about to expand `$VAR` syntax. 3. Update `otelcol.NewCommand` to set a `DefaultScheme="env"` when the feature gate is enabled and no `DefaultScheme` is set, this handles `${VAR}` syntax. 4. Separately, deprecate `expandconverter`. 5. Follow a normal feature gate cycle. 6. Removed the `confmap.unifyEnvVarExpansion` feature gates and `expandconverter` at the same time Supersedes #10259 #### Link to tracking issue Related to #10161 Related to #8215 Related to #7111 #### Testing Unit tests
1 parent 65d59d1 commit ad2c979

File tree

19 files changed

+240
-32
lines changed

19 files changed

+240
-32
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: otelcol/expandconverter
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Add `confmap.unifyEnvVarExpansion` feature gate to allow enabling Collector/Configuration SIG environment variable expansion rules.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10391]
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+
When enabled, this feature gate will:
20+
- Disable expansion of BASH-style env vars (`$FOO`)
21+
- `${FOO}` will be expanded as if it was `${env:FOO}
22+
See https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/env-vars.md for more details.
23+
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: []
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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: enhancement
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: Add `confmap.unifyEnvVarExpansion` feature gate to allow enabling Collector/Configuration SIG environment variable expansion rules.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10259]
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+
When enabled, this feature gate will:
20+
- Disable expansion of BASH-style env vars (`$FOO`)
21+
- `${FOO}` will be expanded as if it was `${env:FOO}
22+
See https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/env-vars.md for more details.
23+
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: []

confmap/converter/expandconverter/expand.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package expandconverter // import "go.opentelemetry.io/collector/confmap/convert
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910
"os"
1011
"regexp"
@@ -13,6 +14,7 @@ import (
1314

1415
"go.opentelemetry.io/collector/confmap"
1516
"go.opentelemetry.io/collector/confmap/internal/envvar"
17+
"go.opentelemetry.io/collector/internal/featuregates"
1618
)
1719

1820
type converter struct {
@@ -92,6 +94,10 @@ func (c converter) expandEnv(s string) (string, error) {
9294
// in order to make sure we don't log a warning for ${VAR}
9395
var regex = regexp.MustCompile(fmt.Sprintf(`\$%s`, regexp.QuoteMeta(str)))
9496
if _, exists := c.loggedDeprecations[str]; !exists && regex.MatchString(s) {
97+
if featuregates.UseUnifiedEnvVarExpansionRules.IsEnabled() {
98+
err = errors.New("$VAR expansion is not supported when feature gate confmap.unifyEnvVarExpansion is enabled")
99+
return ""
100+
}
95101
msg := fmt.Sprintf("Variable substitution using $VAR will be deprecated in favor of ${VAR} and ${env:VAR}, please update $%s", str)
96102
c.logger.Warn(msg, zap.String("variable", str))
97103
c.loggedDeprecations[str] = struct{}{}

confmap/converter/expandconverter/expand_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"go.opentelemetry.io/collector/confmap"
1919
"go.opentelemetry.io/collector/confmap/confmaptest"
2020
"go.opentelemetry.io/collector/confmap/internal/envvar"
21+
"go.opentelemetry.io/collector/featuregate"
22+
"go.opentelemetry.io/collector/internal/featuregates"
2123
)
2224

2325
func TestNewExpandConverter(t *testing.T) {
@@ -56,6 +58,31 @@ func TestNewExpandConverter(t *testing.T) {
5658
}
5759
}
5860

61+
func TestNewExpandConverter_UseUnifiedEnvVarExpansionRules(t *testing.T) {
62+
require.NoError(t, featuregate.GlobalRegistry().Set(featuregates.UseUnifiedEnvVarExpansionRules.ID(), true))
63+
t.Cleanup(func() {
64+
require.NoError(t, featuregate.GlobalRegistry().Set(featuregates.UseUnifiedEnvVarExpansionRules.ID(), false))
65+
})
66+
67+
const valueExtra = "some string"
68+
const valueExtraMapValue = "some map value"
69+
const valueExtraListMapValue = "some list map value"
70+
const valueExtraListElement = "some list value"
71+
t.Setenv("EXTRA", valueExtra)
72+
t.Setenv("EXTRA_MAP_VALUE_1", valueExtraMapValue+"_1")
73+
t.Setenv("EXTRA_MAP_VALUE_2", valueExtraMapValue+"_2")
74+
t.Setenv("EXTRA_LIST_MAP_VALUE_1", valueExtraListMapValue+"_1")
75+
t.Setenv("EXTRA_LIST_MAP_VALUE_2", valueExtraListMapValue+"_2")
76+
t.Setenv("EXTRA_LIST_VALUE_1", valueExtraListElement+"_1")
77+
t.Setenv("EXTRA_LIST_VALUE_2", valueExtraListElement+"_2")
78+
79+
conf, err := confmaptest.LoadConf(filepath.Join("testdata", "expand-with-all-env.yaml"))
80+
require.NoError(t, err, "Unable to get config")
81+
82+
// Test that expanded configs are the same with the simple config with no env vars.
83+
require.ErrorContains(t, createConverter().Convert(context.Background(), conf), "$VAR expansion is not supported when feature gate confmap.unifyEnvVarExpansion is enabled")
84+
}
85+
5986
func TestNewExpandConverter_EscapedMaps(t *testing.T) {
6087
const receiverExtraMapValue = "some map value"
6188
t.Setenv("MAP_VALUE", receiverExtraMapValue)

confmap/converter/expandconverter/go.mod

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@ go 1.21.0
44

55
require (
66
github.com/stretchr/testify v1.9.0
7+
go.opentelemetry.io/collector v0.102.1
78
go.opentelemetry.io/collector/confmap v0.102.1
9+
go.opentelemetry.io/collector/featuregate v1.9.0
810
go.uber.org/goleak v1.3.0
911
go.uber.org/zap v1.27.0
1012
)
1113

1214
require (
1315
github.com/davecgh/go-spew v1.1.1 // indirect
1416
github.com/go-viper/mapstructure/v2 v2.0.0-alpha.1 // indirect
17+
github.com/hashicorp/go-version v1.7.0 // indirect
1518
github.com/knadh/koanf/maps v0.1.1 // indirect
1619
github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
1720
github.com/knadh/koanf/v2 v2.1.1 // indirect
@@ -22,4 +25,18 @@ require (
2225
gopkg.in/yaml.v3 v3.0.1 // indirect
2326
)
2427

25-
replace go.opentelemetry.io/collector/confmap => ../../
28+
replace go.opentelemetry.io/collector/component => ../../../component
29+
30+
replace go.opentelemetry.io/collector/confmap => ../..
31+
32+
replace go.opentelemetry.io/collector => ../../..
33+
34+
replace go.opentelemetry.io/collector/config/configtelemetry => ../../../config/configtelemetry
35+
36+
replace go.opentelemetry.io/collector/pdata/testdata => ../../../pdata/testdata
37+
38+
replace go.opentelemetry.io/collector/pdata => ../../../pdata
39+
40+
replace go.opentelemetry.io/collector/featuregate => ../../../featuregate
41+
42+
replace go.opentelemetry.io/collector/consumer => ../../../consumer

confmap/converter/expandconverter/go.sum

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

confmap/go.mod

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@ require (
1616

1717
require (
1818
github.com/davecgh/go-spew v1.1.1 // indirect
19+
github.com/kr/pretty v0.3.1 // indirect
1920
github.com/mitchellh/copystructure v1.2.0 // indirect
2021
github.com/mitchellh/reflectwalk v1.0.2 // indirect
2122
github.com/pmezard/go-difflib v1.0.0 // indirect
23+
github.com/rogpeppe/go-internal v1.10.0 // indirect
24+
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
2225
)
2326

2427
retract (

confmap/go.sum

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

confmap/internal/e2e/go.sum

Lines changed: 8 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

confmap/provider/envprovider/go.sum

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

0 commit comments

Comments
 (0)