Skip to content

Commit 17b39d1

Browse files
mx-psiTylerHelmuth
andauthored
[confmap] Use stringy representation on stringy types (#10938)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description <!-- Issue number if applicable --> Use string value for stringy types #### Link to tracking issue Fixes #10937 <!--Describe what testing was performed and which tests were added.--> #### Testing Adds tests for stringy type recognition and adds end to end tests --------- Co-authored-by: Tyler Helmuth <[email protected]>
1 parent b10029c commit 17b39d1

File tree

8 files changed

+186
-5
lines changed

8 files changed

+186
-5
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. 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: Use string representation for field types where all primitive types are strings.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10937]
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/confmap.go

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,36 @@ func (l *Conf) unsanitizedGet(key string) any {
111111
return l.k.Get(key)
112112
}
113113

114+
// sanitize recursively removes expandedValue references from the given data.
115+
// It uses the expandedValue.Value field to replace the expandedValue references.
114116
func sanitize(a any) any {
117+
return sanitizeExpanded(a, false)
118+
}
119+
120+
// sanitizeToStringMap recursively removes expandedValue references from the given data.
121+
// It uses the expandedValue.Original field to replace the expandedValue references.
122+
func sanitizeToStr(a any) any {
123+
return sanitizeExpanded(a, true)
124+
}
125+
126+
func sanitizeExpanded(a any, useOriginal bool) any {
115127
switch m := a.(type) {
116128
case map[string]any:
117129
c := maps.Copy(m)
118130
for k, v := range m {
119-
c[k] = sanitize(v)
131+
c[k] = sanitizeExpanded(v, useOriginal)
120132
}
121133
return c
122134
case []any:
123135
var newSlice []any
124136
for _, e := range m {
125-
newSlice = append(newSlice, sanitize(e))
137+
newSlice = append(newSlice, sanitizeExpanded(e, useOriginal))
126138
}
127139
return newSlice
128140
case expandedValue:
141+
if useOriginal {
142+
return m.Original
143+
}
129144
return m.Value
130145
}
131146
return a
@@ -134,7 +149,7 @@ func sanitize(a any) any {
134149
// Get can retrieve any value given the key to use.
135150
func (l *Conf) Get(key string) any {
136151
val := l.unsanitizedGet(key)
137-
return sanitize(val)
152+
return sanitizeExpanded(val, false)
138153
}
139154

140155
// IsSet checks to see if the key has been set in any of the data locations.
@@ -244,6 +259,22 @@ func castTo(exp expandedValue, useOriginal bool) (any, error) {
244259
return exp.Value, nil
245260
}
246261

262+
// Check if a reflect.Type is of the form T, where:
263+
// X is any type or interface
264+
// T = string | map[X]T | []T | [n]T
265+
func isStringyStructure(t reflect.Type) bool {
266+
if t.Kind() == reflect.String {
267+
return true
268+
}
269+
if t.Kind() == reflect.Map {
270+
return isStringyStructure(t.Elem())
271+
}
272+
if t.Kind() == reflect.Slice || t.Kind() == reflect.Array {
273+
return isStringyStructure(t.Elem())
274+
}
275+
return false
276+
}
277+
247278
// When a value has been loaded from an external source via a provider, we keep both the
248279
// parsed value and the original string value. This allows us to expand the value to its
249280
// original string representation when decoding into a string field, and use the original otherwise.
@@ -256,10 +287,13 @@ func useExpandValue() mapstructure.DecodeHookFuncType {
256287
return castTo(exp, to.Kind() == reflect.String)
257288
}
258289

259-
// If the target field is a map or slice, sanitize input to remove expandedValue references.
260290
switch to.Kind() {
261291
case reflect.Array, reflect.Slice, reflect.Map:
262-
// This does not handle map[string]string and []string explicitly.
292+
if isStringyStructure(to) {
293+
// If the target field is a stringy structure, sanitize to use the original string value everywhere.
294+
return sanitizeToStr(data), nil
295+
}
296+
// Otherwise, sanitize to use the parsed value everywhere.
263297
return sanitize(data), nil
264298
}
265299
return data, nil

confmap/confmap_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"os"
1010
"path/filepath"
11+
"reflect"
1112
"strings"
1213
"testing"
1314

@@ -876,3 +877,57 @@ func TestExpandedValue(t *testing.T) {
876877
cfgBool := ConfigBool{}
877878
assert.Error(t, cm.Unmarshal(&cfgBool))
878879
}
880+
881+
func TestStringyTypes(t *testing.T) {
882+
tests := []struct {
883+
valueOfType any
884+
isStringy bool
885+
}{
886+
{
887+
valueOfType: "string",
888+
isStringy: true,
889+
},
890+
{
891+
valueOfType: 1,
892+
isStringy: false,
893+
},
894+
{
895+
valueOfType: map[string]any{},
896+
isStringy: false,
897+
},
898+
{
899+
valueOfType: []any{},
900+
isStringy: false,
901+
},
902+
{
903+
valueOfType: map[string]string{},
904+
isStringy: true,
905+
},
906+
{
907+
valueOfType: []string{},
908+
isStringy: true,
909+
},
910+
{
911+
valueOfType: map[string][]string{},
912+
isStringy: true,
913+
},
914+
{
915+
valueOfType: map[string]map[string]string{},
916+
isStringy: true,
917+
},
918+
{
919+
valueOfType: []map[string]any{},
920+
isStringy: false,
921+
},
922+
{
923+
valueOfType: []map[string]string{},
924+
isStringy: true,
925+
},
926+
}
927+
928+
for _, tt := range tests {
929+
// Create a reflect.Type from the value
930+
to := reflect.TypeOf(tt.valueOfType)
931+
assert.Equal(t, tt.isStringy, isStringyStructure(to))
932+
}
933+
}

confmap/internal/e2e/go.mod

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.22.0
44

55
require (
66
github.com/stretchr/testify v1.9.0
7+
go.opentelemetry.io/collector/config/configopaque v1.13.0
78
go.opentelemetry.io/collector/confmap v0.107.0
89
go.opentelemetry.io/collector/confmap/provider/envprovider v0.107.0
910
go.opentelemetry.io/collector/confmap/provider/fileprovider v0.107.0
@@ -28,3 +29,5 @@ replace go.opentelemetry.io/collector/confmap => ../../
2829
replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../provider/fileprovider
2930

3031
replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
32+
33+
replace go.opentelemetry.io/collector/config/configopaque => ../../../config/configopaque
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
field: [key: ["${env:VALUE}"]]
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
field:
2+
key: ${env:VALUE}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
field: ["${env:VALUE}"]

confmap/internal/e2e/types_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/stretchr/testify/assert"
1212
"github.com/stretchr/testify/require"
1313

14+
"go.opentelemetry.io/collector/config/configopaque"
1415
"go.opentelemetry.io/collector/confmap"
1516
"go.opentelemetry.io/collector/confmap/provider/envprovider"
1617
"go.opentelemetry.io/collector/confmap/provider/fileprovider"
@@ -585,3 +586,62 @@ func TestIndirectSliceEnvVar(t *testing.T) {
585586
assert.Equal(t, collectorConf.Service.Pipelines.Logs.Receivers, []string{"nop", "otlp"})
586587
assert.Equal(t, collectorConf.Service.Pipelines.Logs.Exporters, []string{"otlp", "nop"})
587588
}
589+
590+
func TestIssue10937_MapType(t *testing.T) {
591+
t.Setenv("VALUE", "1234")
592+
593+
resolver := NewResolver(t, "types_map.yaml")
594+
conf, err := resolver.Resolve(context.Background())
595+
require.NoError(t, err)
596+
597+
var cfg TargetConfig[map[string]configopaque.String]
598+
err = conf.Unmarshal(&cfg)
599+
require.NoError(t, err)
600+
require.Equal(t, map[string]configopaque.String{"key": "1234"}, cfg.Field)
601+
}
602+
603+
func TestIssue10937_ArrayType(t *testing.T) {
604+
t.Setenv("VALUE", "1234")
605+
606+
resolver := NewResolver(t, "types_slice.yaml")
607+
conf, err := resolver.Resolve(context.Background())
608+
require.NoError(t, err)
609+
610+
var cfgStrSlice TargetConfig[[]string]
611+
err = conf.Unmarshal(&cfgStrSlice)
612+
require.NoError(t, err)
613+
require.Equal(t, []string{"1234"}, cfgStrSlice.Field)
614+
615+
var cfgStrArray TargetConfig[[1]string]
616+
err = conf.Unmarshal(&cfgStrArray)
617+
require.NoError(t, err)
618+
require.Equal(t, [1]string{"1234"}, cfgStrArray.Field)
619+
620+
var cfgAnySlice TargetConfig[[]any]
621+
err = conf.Unmarshal(&cfgAnySlice)
622+
require.NoError(t, err)
623+
require.Equal(t, []any{1234}, cfgAnySlice.Field)
624+
625+
var cfgAnyArray TargetConfig[[1]any]
626+
err = conf.Unmarshal(&cfgAnyArray)
627+
require.NoError(t, err)
628+
require.Equal(t, [1]any{1234}, cfgAnyArray.Field)
629+
}
630+
631+
func TestIssue10937_ComplexType(t *testing.T) {
632+
t.Setenv("VALUE", "1234")
633+
634+
resolver := NewResolver(t, "types_complex.yaml")
635+
conf, err := resolver.Resolve(context.Background())
636+
require.NoError(t, err)
637+
638+
var cfgStringy TargetConfig[[]map[string][]string]
639+
err = conf.Unmarshal(&cfgStringy)
640+
require.NoError(t, err)
641+
require.Equal(t, []map[string][]string{{"key": {"1234"}}}, cfgStringy.Field)
642+
643+
var cfgNotStringy TargetConfig[[]map[string][]any]
644+
err = conf.Unmarshal(&cfgNotStringy)
645+
require.NoError(t, err)
646+
require.Equal(t, []map[string][]any{{"key": {1234}}}, cfgNotStringy.Field)
647+
}

0 commit comments

Comments
 (0)