Skip to content

Commit

Permalink
[confmap] Add ability to set default provider (open-telemetry#10182)
Browse files Browse the repository at this point in the history 
#### Description
This PR adds support for expanding `${}` syntax when no schema is
provided by allowing Resolver to use a default provider.

In a followup PR I'll update otelcol with a feature gate that allow
switching between a default envProvider and the expandconverter.

#### Link to tracking issue
Related to
open-telemetry#10161
Related to
open-telemetry#7111

#### Testing
Added unit tests

#### Documentation
Added godoc strings

---------

Co-authored-by: Evan Bradley <[email protected]>
  • Loading branch information
@TylerHelmuth @evan-bradley @steves-canva
2 people authored and steves-canva committed Jun 13, 2024
1 parent 66f9a58 commit ab632d5
Show file tree
Hide file tree
Showing 5 changed files with 208 additions and 23 deletions.
25 changes: 25 additions & 0 deletions .chloggen/confmap-default-provider.yaml
View file
Original file line number Diff line number Diff line change
@@ -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]
28 changes: 20 additions & 8 deletions confmap/expand.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,30 +76,34 @@ 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]
}

// 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
Expand Down Expand Up @@ -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())
}
Expand Down
116 changes: 112 additions & 4 deletions confmap/expand_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,34 @@ 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.
{
name: "NoMatchOldStyle",
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}",
Expand All @@ -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}",
Expand Down Expand Up @@ -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}",
Expand All @@ -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}}",
Expand All @@ -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}",
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
Expand All @@ -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"})
Expand Down Expand Up @@ -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())
}
34 changes: 24 additions & 10 deletions confmap/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down
Loading

0 comments on commit ab632d5

Please sign in to comment.