Skip to content

Commit 68015e6

Browse files
committed
suggestions:
- renamed featuregate - added test with a stringy collection - added changelog entry Co-authored-by: Jade Guiton <[email protected]>
1 parent a74434c commit 68015e6

File tree

5 files changed

+65
-10
lines changed

5 files changed

+65
-10
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
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. receiver/otlp)
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 ExpandedValue sanitization when parsed as non-string but assigned to string fields'
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: []
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+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: []

confmap/internal/decoder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func useExpandValue() mapstructure.DecodeHookFuncType {
9393
return v, nil
9494
}
9595

96-
if !DeferExpandedValueSanitizationOnStructCollection.IsEnabled() {
96+
if !NewExpandedValueSanitizer.IsEnabled() {
9797
switch to.Kind() {
9898
case reflect.Array, reflect.Slice, reflect.Map:
9999
if isStringyStructure(to) {

confmap/internal/e2e/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
go.opentelemetry.io/collector/confmap v1.49.0
99
go.opentelemetry.io/collector/confmap/provider/envprovider v1.49.0
1010
go.opentelemetry.io/collector/confmap/provider/fileprovider v1.49.0
11+
go.opentelemetry.io/collector/featuregate v1.49.0
1112
)
1213

1314
require (
@@ -22,7 +23,6 @@ require (
2223
github.com/mitchellh/reflectwalk v1.0.2 // indirect
2324
github.com/pmezard/go-difflib v1.0.0 // indirect
2425
go.opentelemetry.io/collector/confmap/xconfmap v0.143.0 // indirect
25-
go.opentelemetry.io/collector/featuregate v1.49.0 // indirect
2626
go.uber.org/multierr v1.11.0 // indirect
2727
go.uber.org/zap v1.27.1 // indirect
2828
go.yaml.in/yaml/v3 v3.0.4 // indirect

confmap/internal/e2e/maplist_expanded_test.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ func TestMapListWithExpandedValueIntValue(t *testing.T) {
6060
},
6161
}
6262

63-
originalState := internal.DeferExpandedValueSanitizationOnStructCollection.IsEnabled()
63+
originalState := internal.NewExpandedValueSanitizer.IsEnabled()
6464
defer func() {
65-
require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), originalState))
65+
require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), originalState))
6666
}()
6767

68-
require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), true))
68+
require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), true))
6969

7070
conf := confmap.NewFromStringMap(data)
7171
var tc testHeadersConfig
@@ -76,7 +76,7 @@ func TestMapListWithExpandedValueIntValue(t *testing.T) {
7676
require.True(t, ok)
7777
require.Equal(t, configopaque.String("8080"), val)
7878

79-
require.NoError(t, featuregate.GlobalRegistry().Set(internal.DeferExpandedValueSanitizationOnStructCollection.ID(), false))
79+
require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), false))
8080

8181
// This will fail because when reverting to old behavior, ExpandedValues get decoded at collection time and doesn't
8282
// take struct collections into account.
@@ -105,3 +105,33 @@ func TestDirectConfigopaqueStringWithExpandedValueIntValue(t *testing.T) {
105105
require.NoError(t, err)
106106
require.Equal(t, configopaque.String("8080"), tc.Value)
107107
}
108+
109+
// TestStringyStructureWithExpandedValue tests the isStringyStructure path in useExpandValue
110+
func TestStringyStructureWithExpandedValue(t *testing.T) {
111+
type testConfig struct {
112+
Tags []string `mapstructure:"tags"`
113+
}
114+
115+
data := map[string]any{
116+
"tags": []any{
117+
internal.ExpandedValue{
118+
Value: 8080,
119+
Original: "8080",
120+
},
121+
},
122+
}
123+
124+
originalState := internal.NewExpandedValueSanitizer.IsEnabled()
125+
defer func() {
126+
require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), originalState))
127+
}()
128+
129+
// With feature gate disabled, useExpandValue should detect []string as stringy
130+
require.NoError(t, featuregate.GlobalRegistry().Set(internal.NewExpandedValueSanitizer.ID(), false))
131+
132+
conf := confmap.NewFromStringMap(data)
133+
var tc testConfig
134+
err := conf.Unmarshal(&tc)
135+
require.NoError(t, err)
136+
require.Equal(t, []string{"8080"}, tc.Tags)
137+
}

confmap/internal/featuregates.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ var EnableMergeAppendOption = featuregate.GlobalRegistry().MustRegister(
1313
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/issues/8754"),
1414
)
1515

16-
var DeferExpandedValueSanitizationOnStructCollection = featuregate.GlobalRegistry().MustRegister(
17-
"confmap.deferExpandedValueSanitizationOnStructCollection",
16+
var NewExpandedValueSanitizer = featuregate.GlobalRegistry().MustRegister(
17+
"confmap.newExpandedValueSanitizer",
1818
featuregate.StageBeta,
1919
featuregate.WithRegisterFromVersion("v0.144.0"),
20-
featuregate.WithRegisterDescription("Disables early sanitization of ExpandedValue during config unmarshalling, allowing mapstructure to handle type conversion at the field level. Fixes decoding errors when environment variable values are parsed as non-string types (e.g., numbers, booleans) but need to be assigned to string fields."),
21-
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/pull/14413#issuecomment-3754949484"),
20+
featuregate.WithRegisterDescription("Fixes some types of decoding errors where environment variables are parsed as non-string types but assigned to string fields."),
21+
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/pull/14413"),
2222
)

0 commit comments

Comments
 (0)