Skip to content

Commit

Permalink
Add strictly typed input feature gate
Browse files Browse the repository at this point in the history
  • Loading branch information
mx-psi committed Jun 13, 2024
1 parent 6ca551e commit 6781a9f
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 13 deletions.
3 changes: 2 additions & 1 deletion confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/knadh/koanf/providers/confmap"
"github.com/knadh/koanf/v2"

"go.opentelemetry.io/collector/confmap/internal"
encoder "go.opentelemetry.io/collector/confmap/internal/mapstructure"
)

Expand Down Expand Up @@ -156,7 +157,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
ErrorUnused: errorUnused,
Result: result,
TagName: "mapstructure",
WeaklyTypedInput: true,
WeaklyTypedInput: !internal.StrictlyTypedInputGate.IsEnabled(),
MatchName: caseSensitiveMatchName,
DecodeHook: mapstructure.ComposeDecodeHookFunc(
expandNilStructPointersHookFunc(),
Expand Down
54 changes: 48 additions & 6 deletions confmap/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"regexp"
"strconv"
"strings"

"go.opentelemetry.io/collector/confmap/internal"
)

// schemePattern defines the regexp pattern for scheme names.
Expand Down Expand Up @@ -111,22 +113,43 @@ func (mr *Resolver) findAndExpandURI(ctx context.Context, input string) (any, bo
if uri == input {
// If the value is a single URI, then the return value can be anything.
// This is the case `foo: ${file:some_extra_config.yml}`.
return mr.expandURI(ctx, input)
ret, ok, err := mr.expandURI(ctx, input)
if err != nil {
return input, false, err
}

raw, err := ret.AsRaw()
if err != nil {
return input, false, err
}

return raw, ok, nil
}
expanded, changed, err := mr.expandURI(ctx, uri)
if err != nil {
return input, false, err
}
repl, err := toString(expanded)

var repl string
if internal.StrictlyTypedInputGate.IsEnabled() {
repl, err = toStringStrictType(*expanded)
} else {
repl, err = toString(expanded)
}
if err != nil {
return input, false, fmt.Errorf("expanding %v: %w", uri, err)
}
return strings.ReplaceAll(input, uri, repl), changed, err
}

// toString attempts to convert input to a string.
func toString(input any) (string, error) {
func toString(ret *Retrieved) (string, error) {
// This list must be kept in sync with checkRawConfType.
input, err := ret.AsRaw()
if err != nil {
return "", err
}

val := reflect.ValueOf(input)
switch val.Kind() {
case reflect.String:
Expand All @@ -142,7 +165,27 @@ func toString(input any) (string, error) {
}
}

func (mr *Resolver) expandURI(ctx context.Context, input string) (any, bool, error) {
func toStringStrictType(ret Retrieved) (string, error) {
input, err := ret.AsRaw()
if err != nil {
return "", err
}

str, ok := ret.getStringRepr()
if !ok {
return "", fmt.Errorf("expected convertable to string value type, got %v(%T)", input, input)
}

val := reflect.ValueOf(input)
switch val.Kind() {
case reflect.String, reflect.Int, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64, reflect.Bool:
return str, nil
default:
return "", fmt.Errorf("expected convertable to string value type, got %q(%T)", input, input)
}
}

func (mr *Resolver) expandURI(ctx context.Context, input string) (*Retrieved, bool, error) {
// strip ${ and }
uri := input[2 : len(input)-1]

Expand All @@ -163,8 +206,7 @@ func (mr *Resolver) expandURI(ctx context.Context, input string) (any, bool, err
return nil, false, err
}
mr.closers = append(mr.closers, ret.Close)
val, err := ret.AsRaw()
return val, true, err
return ret, true, nil
}

type location struct {
Expand Down
4 changes: 4 additions & 0 deletions confmap/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/knadh/koanf/providers/confmap v0.1.0
github.com/knadh/koanf/v2 v2.1.1
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/collector/featuregate v1.9.0
go.uber.org/goleak v1.3.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.27.0
Expand All @@ -16,6 +17,7 @@ require (

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/hashicorp/go-version v1.7.0 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand All @@ -25,3 +27,5 @@ retract (
v0.76.0 // Depends on retracted pdata v1.0.0-rc10 module, use v0.76.1
v0.69.0 // Release failed, use v0.69.1
)

replace go.opentelemetry.io/collector/featuregate => ../featuregate
14 changes: 10 additions & 4 deletions confmap/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions confmap/internal/e2e/go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<<<<<<< HEAD
module go.opentelemetry.io/collector/confmap/internal/e2e

go 1.21.0
Expand Down Expand Up @@ -28,3 +29,38 @@ replace go.opentelemetry.io/collector/confmap => ../../
replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../provider/fileprovider

replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
||||||| parent of 53be34094 ([chore] Add end to end tests for type casting)
=======
module go.opentelemetry.io/collector/confmap/internal/e2e

go 1.21.0

require (
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/collector/confmap v0.102.1
go.opentelemetry.io/collector/confmap/provider/envprovider v0.102.1
go.opentelemetry.io/collector/confmap/provider/fileprovider v0.102.1
go.opentelemetry.io/collector/featuregate v1.9.0
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-viper/mapstructure/v2 v2.0.0 // indirect
github.com/hashicorp/go-version v1.7.0 // indirect
github.com/knadh/koanf/maps v0.1.1 // indirect
github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
github.com/knadh/koanf/v2 v2.1.1 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace go.opentelemetry.io/collector/confmap => ../../

replace go.opentelemetry.io/collector/confmap/provider/fileprovider => ../../provider/fileprovider

replace go.opentelemetry.io/collector/confmap/provider/envprovider => ../../provider/envprovider
>>>>>>> 53be34094 ([chore] Add end to end tests for type casting)
43 changes: 43 additions & 0 deletions confmap/internal/e2e/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions confmap/internal/featuregate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package internal // import "go.opentelemetry.io/collector/confmap/internal"

import "go.opentelemetry.io/collector/featuregate"

const StrictlyTypedInputID = "confmap.strictlyTypedInput"

var StrictlyTypedInputGate = featuregate.GlobalRegistry().MustRegister(StrictlyTypedInputID,
featuregate.StageAlpha,
featuregate.WithRegisterFromVersion("v0.103.0"),
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."),
)
25 changes: 23 additions & 2 deletions confmap/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,15 @@ type ChangeEvent struct {
type Retrieved struct {
rawConf any
closeFunc CloseFunc

stringRepresentation string
isSetString bool
}

type retrievedSettings struct {
closeFunc CloseFunc
stringRepresentation string
isSetString bool
closeFunc CloseFunc
}

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

func WithStringRepresentation(stringRepresentation string) RetrievedOption {
return func(settings *retrievedSettings) {
settings.stringRepresentation = stringRepresentation
settings.isSetString = true
}
}

// NewRetrieved returns a new Retrieved instance that contains the data from the raw deserialized config.
// The rawConf can be one of the following types:
// - Primitives: int, int32, int64, float32, float64, bool, string;
Expand All @@ -129,7 +141,12 @@ func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) {
for _, opt := range opts {
opt(&set)
}
return &Retrieved{rawConf: rawConf, closeFunc: set.closeFunc}, nil
return &Retrieved{
rawConf: rawConf,
closeFunc: set.closeFunc,
stringRepresentation: set.stringRepresentation,
isSetString: set.isSetString,
}, nil
}

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

func (r *Retrieved) getStringRepr() (string, bool) {
return r.stringRepresentation, r.isSetString
}

// Close and release any watchers that Provider.Retrieve may have created.
//
// Should block until all resources are closed, and guarantee that `onChange` is not
Expand Down
8 changes: 8 additions & 0 deletions confmap/provider/internal/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,13 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...confmap.RetrievedOption) (*c
if err := yaml.Unmarshal(yamlBytes, &rawConf); err != nil {
return nil, err
}

switch v := rawConf.(type) {
case string:
opts = append(opts, confmap.WithStringRepresentation(v))
default:
opts = append(opts, confmap.WithStringRepresentation(string(yamlBytes)))
}

return confmap.NewRetrieved(rawConf, opts...)
}

0 comments on commit 6781a9f

Please sign in to comment.