diff --git a/.chloggen/confmap-default-provider.yaml b/.chloggen/confmap-default-provider.yaml new file mode 100644 index 00000000000..2bc9fb28e67 --- /dev/null +++ b/.chloggen/confmap-default-provider.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: confmap + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Allow setting a default Provider on a Resolver to use when `${}` syntax is used without a scheme + +# One or more tracking issues or pull requests related to the change +issues: [10182] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/confmap/expand.go b/confmap/expand.go index 05ec5a31162..768395f76fd 100644 --- a/confmap/expand.go +++ b/confmap/expand.go @@ -76,21 +76,24 @@ func (mr *Resolver) expandValue(ctx context.Context, value any) (any, bool, erro return value, false, nil } -// findURI attempts to find the first expandable URI in input. It returns an expandable +// findURI attempts to find the first potentially expandable URI in input. It returns a potentially expandable // URI, or an empty string if none are found. // Note: findURI is only called when input contains a closing bracket. -func findURI(input string) string { +func (mr *Resolver) findURI(input string) string { closeIndex := strings.Index(input, "}") remaining := input[closeIndex+1:] openIndex := strings.LastIndex(input[:closeIndex+1], "${") - // if there is a missing "${" or the uri does not contain ":", check the next URI. - if openIndex < 0 || !strings.Contains(input[openIndex:closeIndex+1], ":") { + // if there is any of: + // - a missing "${" + // - there is no default scheme AND no scheme is detected because no `:` is found. + // then check the next URI. + if openIndex < 0 || (mr.defaultScheme == "" && !strings.Contains(input[openIndex:closeIndex+1], ":")) { // if remaining does not contain "}", there are no URIs left: stop recursion. if !strings.Contains(remaining, "}") { return "" } - return findURI(remaining) + return mr.findURI(remaining) } return input[openIndex : closeIndex+1] @@ -98,8 +101,9 @@ func findURI(input string) string { // findAndExpandURI attempts to find and expand the first occurrence of an expandable URI in input. If an expandable URI is found it // returns the input with the URI expanded, true and nil. Otherwise, it returns the unchanged input, false and the expanding error. +// This method expects input to start with ${ and end with } func (mr *Resolver) findAndExpandURI(ctx context.Context, input string) (any, bool, error) { - uri := findURI(input) + uri := mr.findURI(input) if uri == "" { // No URI found, return. return input, false, nil @@ -138,11 +142,19 @@ func toString(input any) (string, error) { } } -func (mr *Resolver) expandURI(ctx context.Context, uri string) (any, bool, error) { - lURI, err := newLocation(uri[2 : len(uri)-1]) +func (mr *Resolver) expandURI(ctx context.Context, input string) (any, bool, error) { + // strip ${ and } + uri := input[2 : len(input)-1] + + if !strings.Contains(uri, ":") { + uri = fmt.Sprintf("%s:%s", mr.defaultScheme, uri) + } + + lURI, err := newLocation(uri) if err != nil { return nil, false, err } + if strings.Contains(lURI.opaqueValue, "$") { return nil, false, fmt.Errorf("the uri %q contains unsupported characters ('$')", lURI.asString()) } diff --git a/confmap/expand_test.go b/confmap/expand_test.go index 02db3ea818d..b7bfc59483d 100644 --- a/confmap/expand_test.go +++ b/confmap/expand_test.go @@ -99,9 +99,10 @@ func TestResolverExpandMapAndSliceValues(t *testing.T) { func TestResolverExpandStringValues(t *testing.T) { tests := []struct { - name string - input string - output any + name string + input string + output any + defaultProvider bool }{ // Embedded. { @@ -109,11 +110,23 @@ func TestResolverExpandStringValues(t *testing.T) { input: "${HOST}:${PORT}", output: "${HOST}:${PORT}", }, + { + name: "NoMatchOldStyleDefaultProvider", + input: "${HOST}:${PORT}", + output: "localhost:3044", + defaultProvider: true, + }, { name: "NoMatchOldStyleNoBrackets", input: "${HOST}:$PORT", output: "${HOST}:$PORT", }, + { + name: "NoMatchOldStyleNoBracketsDefaultProvider", + input: "${HOST}:$PORT", + output: "localhost:$PORT", + defaultProvider: true, + }, { name: "ComplexValue", input: "${env:COMPLEX_VALUE}", @@ -139,6 +152,12 @@ func TestResolverExpandStringValues(t *testing.T) { input: "${env:HOST}:${PORT}", output: "localhost:${PORT}", }, + { + name: "EmbeddedNewAndOldStyleDefaultProvider", + input: "${env:HOST}:${PORT}", + output: "localhost:3044", + defaultProvider: true, + }, { name: "Int", input: "test_${env:INT}", @@ -181,11 +200,23 @@ func TestResolverExpandStringValues(t *testing.T) { input: "${test:localhost:${env:PORT}}", output: "localhost:3044", }, + { + name: "NestedDefaultProvider", + input: "${test:localhost:${PORT}}", + output: "localhost:3044", + defaultProvider: true, + }, { name: "EmbeddedInNested", input: "${test:${env:HOST}:${env:PORT}}", output: "localhost:3044", }, + { + name: "EmbeddedInNestedDefaultProvider", + input: "${test:${HOST}:${PORT}}", + output: "localhost:3044", + defaultProvider: true, + }, { name: "EmbeddedAndNested", input: "${test:localhost:${env:PORT}}?os=${env:OS}", @@ -202,31 +233,67 @@ func TestResolverExpandStringValues(t *testing.T) { input: "env:HOST}", output: "env:HOST}", }, + { + name: "NoMatchMissingOpeningBracketDefaultProvider", + input: "env:HOST}", + output: "env:HOST}", + defaultProvider: true, + }, { name: "NoMatchMissingClosingBracket", input: "${HOST", output: "${HOST", }, + { + name: "NoMatchMissingClosingBracketDefaultProvider", + input: "${HOST", + output: "${HOST", + defaultProvider: true, + }, { name: "NoMatchBracketsWithout$", input: "HO{ST}", output: "HO{ST}", }, + { + name: "NoMatchBracketsWithout$DefaultProvider", + input: "HO{ST}", + output: "HO{ST}", + defaultProvider: true, + }, { name: "NoMatchOnlyMissingClosingBracket", input: "${env:HOST${env:PORT?os=${env:OS", output: "${env:HOST${env:PORT?os=${env:OS", }, + { + name: "NoMatchOnlyMissingClosingBracketDefaultProvider", + input: "${env:HOST${env:PORT?os=${env:OS", + output: "${env:HOST${env:PORT?os=${env:OS", + defaultProvider: true, + }, { name: "NoMatchOnlyMissingOpeningBracket", input: "env:HOST}env:PORT}?os=env:OS}", output: "env:HOST}env:PORT}?os=env:OS}", }, + { + name: "NoMatchOnlyMissingOpeningBracketDefaultProvider", + input: "env:HOST}env:PORT}?os=env:OS}", + output: "env:HOST}env:PORT}?os=env:OS}", + defaultProvider: true, + }, { name: "NoMatchCloseBeforeOpen", input: "env:HOST}${env:PORT", output: "env:HOST}${env:PORT", }, + { + name: "NoMatchCloseBeforeOpenDefaultProvider", + input: "env:HOST}${env:PORT", + output: "env:HOST}${env:PORT", + defaultProvider: true, + }, { name: "NoMatchOldStyleNested", input: "${test:localhost:${PORT}}", @@ -238,6 +305,12 @@ func TestResolverExpandStringValues(t *testing.T) { input: "env:HOST}${env:PORT}", output: "env:HOST}3044", }, + { + name: "PartialMatchMissingOpeningBracketFirstDefaultProvider", + input: "env:HOST}${PORT}", + output: "env:HOST}3044", + defaultProvider: true, + }, { name: "PartialMatchMissingOpeningBracketLast", input: "${env:HOST}env:PORT}", @@ -288,11 +361,23 @@ func TestResolverExpandStringValues(t *testing.T) { input: "${HOST}${env:PORT}", output: "${HOST}3044", }, + { + name: "SchemeAfterNoSchemeIsExpandedDefaultProvider", + input: "${HOST}${env:PORT}", + output: "localhost3044", + defaultProvider: true, + }, { name: "SchemeBeforeNoSchemeIsExpanded", input: "${env:HOST}${PORT}", output: "localhost${PORT}", }, + { + name: "SchemeBeforeNoSchemeIsExpandedDefaultProvider", + input: "${env:HOST}${PORT}", + output: "localhost3044", + defaultProvider: true, + }, } for _, tt := range tests { @@ -305,7 +390,12 @@ func TestResolverExpandStringValues(t *testing.T) { return NewRetrieved(uri[5:]) }) - resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, newEnvProvider(), testProvider}, ConverterFactories: nil}) + envProvider := newEnvProvider() + set := ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, envProvider, testProvider}, ConverterFactories: nil} + if tt.defaultProvider { + set.DefaultScheme = "env" + } + resolver, err := NewResolver(set) require.NoError(t, err) cfgMap, err := resolver.Resolve(context.Background()) @@ -317,6 +407,11 @@ func TestResolverExpandStringValues(t *testing.T) { func newEnvProvider() ProviderFactory { return newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) { + // When using `env` as the default scheme for tests, the uri will not include `env:`. + // Instead of duplicating the switch cases, the scheme is added instead. + if uri[0:4] != "env:" { + uri = "env:" + uri + } switch uri { case "env:COMPLEX_VALUE": return NewRetrieved([]any{"localhost:3042"}) @@ -469,3 +564,16 @@ func TestResolverExpandStringValueInvalidReturnValue(t *testing.T) { _, err = resolver.Resolve(context.Background()) assert.EqualError(t, err, `expanding ${test:PORT}: expected convertable to string value type, got ['ӛ']([]interface {})`) } + +func TestResolverDefaultProviderExpand(t *testing.T) { + provider := newFakeProvider("input", func(context.Context, string, WatcherFunc) (*Retrieved, error) { + return NewRetrieved(map[string]any{"foo": "${HOST}"}) + }) + + resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, newEnvProvider()}, DefaultScheme: "env", ConverterFactories: nil}) + require.NoError(t, err) + + cfgMap, err := resolver.Resolve(context.Background()) + require.NoError(t, err) + assert.Equal(t, map[string]any{"foo": "localhost"}, cfgMap.ToStringMap()) +} diff --git a/confmap/resolver.go b/confmap/resolver.go index 304dc3f71c3..7c9ed303c73 100644 --- a/confmap/resolver.go +++ b/confmap/resolver.go @@ -20,9 +20,10 @@ var driverLetterRegexp = regexp.MustCompile("^[A-z]:") // Resolver resolves a configuration as a Conf. type Resolver struct { - uris []location - providers map[string]Provider - converters []Converter + uris []location + providers map[string]Provider + defaultScheme string + converters []Converter closers []CloseFunc watcher chan error @@ -34,11 +35,16 @@ type ResolverSettings struct { // It is required to have at least one location. URIs []string - // ProviderFactories is a list of Provider creation functions. - // It is required to have at least one ProviderFactory - // if a Provider is not given. + // ProviderFactories is a slice of Provider factories. + // It is required to have at least one factory. ProviderFactories []ProviderFactory + // DefaultScheme is the scheme that is used if ${} syntax is used but no schema is provided. + // If no DefaultScheme is set, ${} with no schema will not be expanded. + // It is strongly recommended to set "env" as the default scheme to align with the + // OpenTelemetry Configuration Specification + DefaultScheme string + // ProviderSettings contains settings that will be passed to Provider // factories when instantiating Providers. ProviderSettings ProviderSettings @@ -93,6 +99,13 @@ func NewResolver(set ResolverSettings) (*Resolver, error) { providers[provider.Scheme()] = provider } + if set.DefaultScheme != "" { + _, ok := providers[set.DefaultScheme] + if !ok { + return nil, errors.New("invalid 'confmap.ResolverSettings' configuration: DefaultScheme not found in providers list") + } + } + converters := make([]Converter, len(set.ConverterFactories)) for i, factory := range set.ConverterFactories { converters[i] = factory.Create(set.ConverterSettings) @@ -119,10 +132,11 @@ func NewResolver(set ResolverSettings) (*Resolver, error) { } return &Resolver{ - uris: uris, - providers: providers, - converters: converters, - watcher: make(chan error, 1), + uris: uris, + providers: providers, + defaultScheme: set.DefaultScheme, + converters: converters, + watcher: make(chan error, 1), }, nil } diff --git a/confmap/resolver_test.go b/confmap/resolver_test.go index 3a162c85247..fecb973d089 100644 --- a/confmap/resolver_test.go +++ b/confmap/resolver_test.go @@ -124,6 +124,7 @@ func TestResolverErrors(t *testing.T) { locations []string providers []Provider converters []Converter + defaultScheme string expectBuildErr bool expectResolveErr bool expectWatchErr bool @@ -136,6 +137,16 @@ func TestResolverErrors(t *testing.T) { providers: []Provider{&mockProvider{}}, expectBuildErr: true, }, + { + name: "default scheme not found", + locations: []string{"mock:", "err:"}, + providers: []Provider{ + &mockProvider{}, + &mockProvider{scheme: "err", errR: errors.New("retrieve_err")}, + }, + defaultScheme: "missing", + expectBuildErr: true, + }, { name: "retrieve location config error", locations: []string{"mock:", "err:"}, @@ -201,7 +212,7 @@ func TestResolverErrors(t *testing.T) { c := converter converterFuncs[i] = NewConverterFactory(func(_ ConverterSettings) Converter { return c }) } - resolver, err := NewResolver(ResolverSettings{URIs: tt.locations, ProviderFactories: mockProviderFuncs, ConverterFactories: converterFuncs}) + resolver, err := NewResolver(ResolverSettings{URIs: tt.locations, ProviderFactories: mockProviderFuncs, DefaultScheme: tt.defaultScheme, ConverterFactories: converterFuncs}) if tt.expectBuildErr { assert.Error(t, err) return @@ -395,3 +406,18 @@ func TestProvidesDefaultLogger(t *testing.T) { require.NoError(t, err) require.NotNil(t, provider.logger) } + +func TestResolverDefaultProviderSet(t *testing.T) { + envProvider := newEnvProvider() + fileProvider := newFileProvider(t) + + r, err := NewResolver(ResolverSettings{ + URIs: []string{"env:"}, + ProviderFactories: []ProviderFactory{fileProvider, envProvider}, + DefaultScheme: "env", + }) + require.NoError(t, err) + assert.NotNil(t, r.defaultScheme) + _, ok := r.providers["env"] + assert.True(t, ok) +}