Skip to content

Commit 4cc21d9

Browse files
committed
Add strictly typed input feature gate
1 parent 6ca551e commit 4cc21d9

File tree

9 files changed

+152
-13
lines changed

9 files changed

+152
-13
lines changed

confmap/confmap.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/knadh/koanf/providers/confmap"
1717
"github.com/knadh/koanf/v2"
1818

19+
"go.opentelemetry.io/collector/confmap/internal"
1920
encoder "go.opentelemetry.io/collector/confmap/internal/mapstructure"
2021
)
2122

@@ -156,7 +157,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
156157
ErrorUnused: errorUnused,
157158
Result: result,
158159
TagName: "mapstructure",
159-
WeaklyTypedInput: true,
160+
WeaklyTypedInput: !internal.StrictlyTypedInputGate.IsEnabled(),
160161
MatchName: caseSensitiveMatchName,
161162
DecodeHook: mapstructure.ComposeDecodeHookFunc(
162163
expandNilStructPointersHookFunc(),

confmap/expand.go

+48-6
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"regexp"
1212
"strconv"
1313
"strings"
14+
15+
"go.opentelemetry.io/collector/confmap/internal"
1416
)
1517

1618
// schemePattern defines the regexp pattern for scheme names.
@@ -111,22 +113,43 @@ func (mr *Resolver) findAndExpandURI(ctx context.Context, input string) (any, bo
111113
if uri == input {
112114
// If the value is a single URI, then the return value can be anything.
113115
// This is the case `foo: ${file:some_extra_config.yml}`.
114-
return mr.expandURI(ctx, input)
116+
ret, ok, err := mr.expandURI(ctx, input)
117+
if err != nil {
118+
return input, false, err
119+
}
120+
121+
raw, err := ret.AsRaw()
122+
if err != nil {
123+
return input, false, err
124+
}
125+
126+
return raw, ok, nil
115127
}
116128
expanded, changed, err := mr.expandURI(ctx, uri)
117129
if err != nil {
118130
return input, false, err
119131
}
120-
repl, err := toString(expanded)
132+
133+
var repl string
134+
if internal.StrictlyTypedInputGate.IsEnabled() {
135+
repl, err = toStringStrictType(*expanded)
136+
} else {
137+
repl, err = toString(expanded)
138+
}
121139
if err != nil {
122140
return input, false, fmt.Errorf("expanding %v: %w", uri, err)
123141
}
124142
return strings.ReplaceAll(input, uri, repl), changed, err
125143
}
126144

127145
// toString attempts to convert input to a string.
128-
func toString(input any) (string, error) {
146+
func toString(ret *Retrieved) (string, error) {
129147
// This list must be kept in sync with checkRawConfType.
148+
input, err := ret.AsRaw()
149+
if err != nil {
150+
return "", err
151+
}
152+
130153
val := reflect.ValueOf(input)
131154
switch val.Kind() {
132155
case reflect.String:
@@ -142,7 +165,27 @@ func toString(input any) (string, error) {
142165
}
143166
}
144167

145-
func (mr *Resolver) expandURI(ctx context.Context, input string) (any, bool, error) {
168+
func toStringStrictType(ret Retrieved) (string, error) {
169+
input, err := ret.AsRaw()
170+
if err != nil {
171+
return "", err
172+
}
173+
174+
str, ok := ret.getStringRepr()
175+
if !ok {
176+
return "", fmt.Errorf("expected convertable to string value type, got %v(%T)", input, input)
177+
}
178+
179+
val := reflect.ValueOf(input)
180+
switch val.Kind() {
181+
case reflect.String, reflect.Int, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64, reflect.Bool:
182+
return str, nil
183+
default:
184+
return "", fmt.Errorf("expected convertable to string value type, got %q(%T)", input, input)
185+
}
186+
}
187+
188+
func (mr *Resolver) expandURI(ctx context.Context, input string) (*Retrieved, bool, error) {
146189
// strip ${ and }
147190
uri := input[2 : len(input)-1]
148191

@@ -163,8 +206,7 @@ func (mr *Resolver) expandURI(ctx context.Context, input string) (any, bool, err
163206
return nil, false, err
164207
}
165208
mr.closers = append(mr.closers, ret.Close)
166-
val, err := ret.AsRaw()
167-
return val, true, err
209+
return ret, true, nil
168210
}
169211

170212
type location struct {

confmap/go.mod

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/knadh/koanf/providers/confmap v0.1.0
99
github.com/knadh/koanf/v2 v2.1.1
1010
github.com/stretchr/testify v1.9.0
11+
go.opentelemetry.io/collector/featuregate v1.9.0
1112
go.uber.org/goleak v1.3.0
1213
go.uber.org/multierr v1.11.0
1314
go.uber.org/zap v1.27.0
@@ -16,6 +17,7 @@ require (
1617

1718
require (
1819
github.com/davecgh/go-spew v1.1.1 // indirect
20+
github.com/hashicorp/go-version v1.7.0 // indirect
1921
github.com/mitchellh/copystructure v1.2.0 // indirect
2022
github.com/mitchellh/reflectwalk v1.0.2 // indirect
2123
github.com/pmezard/go-difflib v1.0.0 // indirect
@@ -25,3 +27,5 @@ retract (
2527
v0.76.0 // Depends on retracted pdata v1.0.0-rc10 module, use v0.76.1
2628
v0.69.0 // Release failed, use v0.69.1
2729
)
30+
31+
replace go.opentelemetry.io/collector/featuregate => ../featuregate

confmap/go.sum

+8-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

confmap/internal/e2e/go.mod

+2
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ require (
77
go.opentelemetry.io/collector/confmap v0.102.1
88
go.opentelemetry.io/collector/confmap/provider/envprovider v0.102.1
99
go.opentelemetry.io/collector/confmap/provider/fileprovider v0.102.1
10+
go.opentelemetry.io/collector/featuregate v1.9.0
1011
)
1112

1213
require (
1314
github.com/davecgh/go-spew v1.1.1 // indirect
1415
github.com/go-viper/mapstructure/v2 v2.0.0 // indirect
16+
github.com/hashicorp/go-version v1.7.0 // indirect
1517
github.com/knadh/koanf/maps v0.1.1 // indirect
1618
github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
1719
github.com/knadh/koanf/v2 v2.1.1 // indirect

confmap/internal/e2e/go.sum

+43
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

confmap/internal/featuregate.go

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package internal // import "go.opentelemetry.io/collector/confmap/internal"
5+
6+
import "go.opentelemetry.io/collector/featuregate"
7+
8+
const StrictlyTypedInputID = "confmap.strictlyTypedInput"
9+
10+
var StrictlyTypedInputGate = featuregate.GlobalRegistry().MustRegister(StrictlyTypedInputID,
11+
featuregate.StageAlpha,
12+
featuregate.WithRegisterFromVersion("v0.103.0"),
13+
featuregate.WithRegisterDescription("Makes type casting rules during configuration unmarshaling stricter. See https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/env-vars.md for more details."),
14+
)

confmap/provider.go

+23-2
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,15 @@ type ChangeEvent struct {
9999
type Retrieved struct {
100100
rawConf any
101101
closeFunc CloseFunc
102+
103+
stringRepresentation string
104+
isSetString bool
102105
}
103106

104107
type retrievedSettings struct {
105-
closeFunc CloseFunc
108+
stringRepresentation string
109+
isSetString bool
110+
closeFunc CloseFunc
106111
}
107112

108113
// RetrievedOption options to customize Retrieved values.
@@ -116,6 +121,13 @@ func WithRetrievedClose(closeFunc CloseFunc) RetrievedOption {
116121
}
117122
}
118123

124+
func WithStringRepresentation(stringRepresentation string) RetrievedOption {
125+
return func(settings *retrievedSettings) {
126+
settings.stringRepresentation = stringRepresentation
127+
settings.isSetString = true
128+
}
129+
}
130+
119131
// NewRetrieved returns a new Retrieved instance that contains the data from the raw deserialized config.
120132
// The rawConf can be one of the following types:
121133
// - Primitives: int, int32, int64, float32, float64, bool, string;
@@ -129,7 +141,12 @@ func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) {
129141
for _, opt := range opts {
130142
opt(&set)
131143
}
132-
return &Retrieved{rawConf: rawConf, closeFunc: set.closeFunc}, nil
144+
return &Retrieved{
145+
rawConf: rawConf,
146+
closeFunc: set.closeFunc,
147+
stringRepresentation: set.stringRepresentation,
148+
isSetString: set.isSetString,
149+
}, nil
133150
}
134151

135152
// AsConf returns the retrieved configuration parsed as a Conf.
@@ -152,6 +169,10 @@ func (r *Retrieved) AsRaw() (any, error) {
152169
return r.rawConf, nil
153170
}
154171

172+
func (r *Retrieved) getStringRepr() (string, bool) {
173+
return r.stringRepresentation, r.isSetString
174+
}
175+
155176
// Close and release any watchers that Provider.Retrieve may have created.
156177
//
157178
// Should block until all resources are closed, and guarantee that `onChange` is not

confmap/provider/internal/provider.go

+8
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,13 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...confmap.RetrievedOption) (*c
1717
if err := yaml.Unmarshal(yamlBytes, &rawConf); err != nil {
1818
return nil, err
1919
}
20+
21+
switch v := rawConf.(type) {
22+
case string:
23+
opts = append(opts, confmap.WithStringRepresentation(v))
24+
default:
25+
opts = append(opts, confmap.WithStringRepresentation(string(yamlBytes)))
26+
}
27+
2028
return confmap.NewRetrieved(rawConf, opts...)
2129
}

0 commit comments

Comments
 (0)