Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[confmap] Add converter and provider settings to confmap.ResolverSettings #9516

Merged
merged 7 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/pass-confmap-settings.yaml
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: Add `ProviderSettings`, `ConverterSettings`, `ProviderFactories`, and `ConverterFactories` fields to `confmap.ResolverSettings`

# One or more tracking issues or pull requests related to the change
issues: [9516]

# (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: This allows configuring providers and converters, which are instantiated by `NewResolver` using the given factories.

# 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: []
20 changes: 20 additions & 0 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,23 @@ func zeroSliceHookFunc() mapstructure.DecodeHookFuncValue {
return from.Interface(), nil
}
}

type moduleFactory[T any, S any] interface {
Create(s S) T
}

type createConfmapFunc[T any, S any] func(s S) T

type confmapModuleFactory[T any, S any] struct {
f createConfmapFunc[T, S]
}

func (c confmapModuleFactory[T, S]) Create(s S) T {
return c.f(s)
}

func newConfmapModuleFactory[T any, S any](f createConfmapFunc[T, S]) moduleFactory[T, S] {
return confmapModuleFactory[T, S]{
f: f,
}
}
13 changes: 12 additions & 1 deletion confmap/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,20 @@ import (
)

// ConverterSettings are the settings to initialize a Converter.
// Any Converter should take this as a parameter in its constructor.
type ConverterSettings struct{}

// ConverterFactory defines a factory that can be used to instantiate
// new instances of a Converter.
type ConverterFactory = moduleFactory[Converter, ConverterSettings]

// CreateConverterFunc is a function that creates a Converter instance.
type CreateConverterFunc = createConfmapFunc[Converter, ConverterSettings]

// NewConverterFactory can be used to create a ConverterFactory.
func NewConverterFactory(f CreateConverterFunc) ConverterFactory {
return newConfmapModuleFactory(f)
}

// Converter is a converter interface for the confmap.Conf that allows distributions
// (in the future components as well) to build backwards compatible config converters.
type Converter interface {
Expand Down
8 changes: 8 additions & 0 deletions confmap/converter/expandconverter/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@ type converter struct {
// New returns a confmap.Converter, that expands all environment variables for a given confmap.Conf.
//
// Notice: This API is experimental.
//
// Deprecated: [v0.99.0] Use NewFactory instead.
func New(_ confmap.ConverterSettings) confmap.Converter {
return converter{
loggedDeprecations: make(map[string]struct{}),
logger: zap.NewNop(), // TODO: pass logger in ConverterSettings
}
}

// NewFactory returns a factory for a confmap.Converter,
// which expands all environment variables for a given confmap.Conf.
func NewFactory() confmap.ConverterFactory {
return confmap.NewConverterFactory(New)
}

func (c converter) Convert(_ context.Context, conf *confmap.Conf) error {
out := make(map[string]any)
for _, k := range conf.AllKeys() {
Expand Down
12 changes: 8 additions & 4 deletions confmap/converter/expandconverter/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestNewExpandConverter(t *testing.T) {
require.NoError(t, err, "Unable to get config")

// Test that expanded configs are the same with the simple config with no env vars.
require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
require.NoError(t, createConverter().Convert(context.Background(), conf))
assert.Equal(t, expectedCfgMap.ToStringMap(), conf.ToStringMap())
})
}
Expand All @@ -68,7 +68,7 @@ func TestNewExpandConverter_EscapedMaps(t *testing.T) {
"recv": "$MAP_VALUE",
}},
)
require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
require.NoError(t, createConverter().Convert(context.Background(), conf))

expectedMap := map[string]any{
"test_string_map": map[string]any{
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestNewExpandConverter_EscapedEnvVars(t *testing.T) {
// escaped $ alone
"recv.7": "$",
}}
require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
require.NoError(t, createConverter().Convert(context.Background(), conf))
assert.Equal(t, expectedMap, conf.ToStringMap())
}

Expand Down Expand Up @@ -158,7 +158,7 @@ func TestNewExpandConverterHostPort(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
conf := confmap.NewFromStringMap(tt.input)
require.NoError(t, New(confmap.ConverterSettings{}).Convert(context.Background(), conf))
require.NoError(t, createConverter().Convert(context.Background(), conf))
assert.Equal(t, tt.expected, conf.ToStringMap())
})
}
Expand Down Expand Up @@ -252,3 +252,7 @@ func TestDeprecatedWarning(t *testing.T) {
})
}
}

func createConverter() confmap.Converter {
return NewFactory().Create(confmap.ConverterSettings{})
}
22 changes: 11 additions & 11 deletions confmap/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestResolverExpandEnvVars(t *testing.T) {

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
resolver, err := NewResolver(ResolverSettings{URIs: []string{filepath.Join("testdata", test.name)}, Providers: makeMapProvidersMap(fileProvider, envProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{filepath.Join("testdata", test.name)}, ProviderFactories: []ProviderFactory{fileProvider, envProvider}, ConverterFactories: nil})
require.NoError(t, err)

// Test that expanded configs are the same with the simple config with no env vars.
Expand All @@ -65,7 +65,7 @@ func TestResolverDoneNotExpandOldEnvVars(t *testing.T) {
return NewRetrieved("some string")
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"test:"}, Providers: makeMapProvidersMap(fileProvider, envProvider, emptySchemeProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"test:"}, ProviderFactories: []ProviderFactory{fileProvider, envProvider, emptySchemeProvider}, ConverterFactories: nil})
require.NoError(t, err)

// Test that expanded configs are the same with the simple config with no env vars.
Expand All @@ -86,7 +86,7 @@ func TestResolverExpandMapAndSliceValues(t *testing.T) {
return NewRetrieved(receiverExtraMapValue)
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

cfgMap, err := resolver.Resolve(context.Background())
Expand Down Expand Up @@ -295,7 +295,7 @@ func TestResolverExpandStringValues(t *testing.T) {
return NewRetrieved(uri[5:])
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, newEnvProvider(), testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, newEnvProvider(), testProvider}, ConverterFactories: nil})
require.NoError(t, err)

cfgMap, err := resolver.Resolve(context.Background())
Expand All @@ -305,7 +305,7 @@ func TestResolverExpandStringValues(t *testing.T) {
}
}

func newEnvProvider() Provider {
func newEnvProvider() ProviderFactory {
return newFakeProvider("env", func(_ context.Context, uri string, _ WatcherFunc) (*Retrieved, error) {
switch uri {
case "env:COMPLEX_VALUE":
Expand Down Expand Up @@ -369,7 +369,7 @@ func TestResolverExpandReturnError(t *testing.T) {
return nil, myErr
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand All @@ -388,7 +388,7 @@ func TestResolverInfiniteExpand(t *testing.T) {
return NewRetrieved(receiverValue)
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand All @@ -404,7 +404,7 @@ func TestResolverExpandInvalidScheme(t *testing.T) {
panic("must not be called")
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand All @@ -421,7 +421,7 @@ func TestResolverExpandInvalidOpaqueValue(t *testing.T) {
panic("must not be called")
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand All @@ -437,7 +437,7 @@ func TestResolverExpandUnsupportedScheme(t *testing.T) {
panic("must not be called")
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand All @@ -453,7 +453,7 @@ func TestResolverExpandStringValueInvalidReturnValue(t *testing.T) {
return NewRetrieved([]any{1243})
})

resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, Providers: makeMapProvidersMap(provider, testProvider), Converters: nil})
resolver, err := NewResolver(ResolverSettings{URIs: []string{"input:"}, ProviderFactories: []ProviderFactory{provider, testProvider}, ConverterFactories: nil})
require.NoError(t, err)

_, err = resolver.Resolve(context.Background())
Expand Down
17 changes: 16 additions & 1 deletion confmap/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,26 @@ import (
)

// ProviderSettings are the settings to initialize a Provider.
// Any Provider should take this as a parameter in its constructor.
type ProviderSettings struct {
// Logger is a zap.Logger that will be passed to Providers.
// Providers should be able to rely on the Logger being non-nil;
// when instantiating a Provider with a ProviderFactory,
// nil Logger references should be replaced with a no-op Logger.
Logger *zap.Logger
}

// ProviderFactory defines a factory that can be used to instantiate
// new instances of a Provider.
type ProviderFactory = moduleFactory[Provider, ProviderSettings]

// CreateProviderFunc is a function that creates a Provider instance.
type CreateProviderFunc = createConfmapFunc[Provider, ProviderSettings]

// NewProviderFactory can be used to create a ProviderFactory.
func NewProviderFactory(f CreateProviderFunc) ProviderFactory {
return newConfmapModuleFactory(f)
}

// Provider is an interface that helps to retrieve a config map and watch for any
// changes to the config map. Implementations may load the config from a file,
// a database or any other source.
Expand Down
10 changes: 10 additions & 0 deletions confmap/provider/envprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,22 @@ type provider struct {
//
// This Provider supports "env" scheme, and can be called with a selector:
// `env:NAME_OF_ENVIRONMENT_VARIABLE`
//
// Deprecated: [v0.99.0] Use NewFactory instead.
func NewWithSettings(ps confmap.ProviderSettings) confmap.Provider {
return &provider{
logger: ps.Logger,
}
}

// NewFactory returns a factory for a confmap.Provider that reads the configuration from the given environment variable.
//
// This Provider supports "env" scheme, and can be called with a selector:
// `env:NAME_OF_ENVIRONMENT_VARIABLE`
func NewFactory() confmap.ProviderFactory {
return confmap.NewProviderFactory(NewWithSettings)
}

func (emp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
if !strings.HasPrefix(uri, schemeName+":") {
return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName)
Expand Down
14 changes: 9 additions & 5 deletions confmap/provider/envprovider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ exporters:
`

func TestValidateProviderScheme(t *testing.T) {
assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmaptest.NewNopProviderSettings())))
assert.NoError(t, confmaptest.ValidateProviderScheme(createProvider()))
}

func TestEmptyName(t *testing.T) {
env := NewWithSettings(confmaptest.NewNopProviderSettings())
env := createProvider()
_, err := env.Retrieve(context.Background(), "", nil)
require.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
}

func TestUnsupportedScheme(t *testing.T) {
env := NewWithSettings(confmaptest.NewNopProviderSettings())
env := createProvider()
_, err := env.Retrieve(context.Background(), "https://", nil)
assert.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
Expand All @@ -47,7 +47,7 @@ func TestUnsupportedScheme(t *testing.T) {
func TestInvalidYAML(t *testing.T) {
const envName = "invalid-yaml"
t.Setenv(envName, "[invalid,")
env := NewWithSettings(confmaptest.NewNopProviderSettings())
env := createProvider()
_, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
assert.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
Expand All @@ -57,7 +57,7 @@ func TestEnv(t *testing.T) {
const envName = "default-config"
t.Setenv(envName, validYAML)

env := NewWithSettings(confmaptest.NewNopProviderSettings())
env := createProvider()
ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
require.NoError(t, err)
retMap, err := ret.AsConf()
Expand Down Expand Up @@ -137,3 +137,7 @@ func TestEmptyEnvWithLoggerWarn(t *testing.T) {
assert.Equal(t, zap.InfoLevel, logLine.Level)
assert.Equal(t, envName, logLine.Context[0].String)
}

func createProvider() confmap.Provider {
return NewFactory().Create(confmaptest.NewNopProviderSettings())
}
21 changes: 21 additions & 0 deletions confmap/provider/fileprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,31 @@ type provider struct{}
// `file:/path/to/file` - absolute path (unix, windows)
// `file:c:/path/to/file` - absolute path including drive-letter (windows)
// `file:c:\path\to\file` - absolute path including drive-letter (windows)
//
// Deprecated: [v0.99.0] Use NewFactory instead.
func NewWithSettings(confmap.ProviderSettings) confmap.Provider {
return &provider{}
}

// NewFactory returns a factory for a confmap.Provider that reads the configuration from a file.
//
// This Provider supports "file" scheme, and can be called with a "uri" that follows:
//
// file-uri = "file:" local-path
// local-path = [ drive-letter ] file-path
// drive-letter = ALPHA ":"
//
// The "file-path" can be relative or absolute, and it can be any OS supported format.
//
// Examples:
// `file:path/to/file` - relative path (unix, windows)
// `file:/path/to/file` - absolute path (unix, windows)
// `file:c:/path/to/file` - absolute path including drive-letter (windows)
// `file:c:\path\to\file` - absolute path including drive-letter (windows)
func NewFactory() confmap.ProviderFactory {
return confmap.NewProviderFactory(NewWithSettings)
}

func (fmp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
if !strings.HasPrefix(uri, schemeName+":") {
return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName)
Expand Down
Loading
Loading