From 70b75c75a2d99f9c968fc1f2fec01ce5f0a6fa84 Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick <10867373+rmfitzpatrick@users.noreply.github.com> Date: Mon, 3 Apr 2023 12:10:52 -0400 Subject: [PATCH] Refactor config sources into provider wrapper and support all service providers (#2893) * refactor config source and its provider * forward wrapped providers to config source resolver * support raw confmap.Provider retrieved values * remove unnecessary previous value merging * update configsource.Provider for clarity * exercise both env provider types in vault test --- cmd/otelcol/main.go | 28 +- internal/configconverter/config_server.go | 4 +- internal/configconverter/dry_run.go | 4 +- internal/configprovider/builder.go | 49 --- internal/configprovider/builder_test.go | 138 ------- internal/configprovider/component.go | 51 --- internal/configprovider/config.go | 62 --- .../configprovider/config_source_provider.go | 132 ------ internal/configprovider/factory.go | 56 --- internal/configprovider/parser.go | 106 ----- .../configprovider/testconfigsource_test.go | 85 ---- .../configsource/envvarconfigsource/config.go | 8 +- .../envvarconfigsource/config_test.go | 19 +- .../envvarconfigsource/factory.go | 13 +- .../envvarconfigsource/factory_test.go | 7 +- .../configsource/envvarconfigsource/source.go | 9 +- .../envvarconfigsource/source_test.go | 2 - .../configsource/etcd2configsource/config.go | 10 +- .../etcd2configsource/config_test.go | 20 +- .../configsource/etcd2configsource/factory.go | 15 +- .../etcd2configsource/factory_test.go | 7 +- .../configsource/etcd2configsource/source.go | 10 +- .../etcd2configsource/source_test.go | 2 - .../includeconfigsource/config.go | 10 +- .../includeconfigsource/config_test.go | 21 +- .../includeconfigsource/factory.go | 13 +- .../includeconfigsource/factory_test.go | 7 +- .../includeconfigsource/source.go | 8 +- .../includeconfigsource/source_test.go | 19 +- internal/configsource/settings.go | 136 ++++++ .../settings_test.go} | 85 ++-- .../manager.go => configsource/source.go} | 194 ++++++--- .../source_test.go} | 389 ++++++++++++++++-- .../testdata/arrays_and_maps.yaml | 0 .../testdata/arrays_and_maps_expected.yaml | 0 .../testdata/bad_name.yaml | 0 .../testdata/basic_config.yaml | 0 .../testdata/cfgsrc_load_use_cfgsrc.yaml | 0 .../testdata/duplicated_name.yaml | 0 .../testdata/env_var_on_load.yaml | 0 .../testdata/envvar_cfgsrc_mix.yaml | 0 .../testdata/envvar_cfgsrc_mix_expected.yaml | 0 .../testdata/manager_resolve_error.yaml | 0 .../testdata/params_handling.yaml | 0 .../testdata/params_handling_expected.yaml | 0 .../testdata/unknown_field.yaml | 0 .../testdata/yaml_injection.yaml | 0 .../testdata/yaml_injection_expected.yaml | 0 .../configsource/vaultconfigsource/config.go | 8 +- .../vaultconfigsource/config_test.go | 19 +- .../configsource/vaultconfigsource/factory.go | 15 +- .../vaultconfigsource/factory_test.go | 7 +- .../configsource/vaultconfigsource/source.go | 11 +- .../vaultconfigsource/source_test.go | 30 +- .../zookeeperconfigsource/config.go | 8 +- .../zookeeperconfigsource/config_test.go | 19 +- .../zookeeperconfigsource/factory.go | 13 +- .../zookeeperconfigsource/factory_test.go | 7 +- .../zookeeperconfigsource/source.go | 15 +- .../zookeeperconfigsource/source_test.go | 10 +- internal/configsources/configsources.go | 37 -- internal/configsources/configsources_test.go | 61 --- .../confmapprovider/configsource/provider.go | 174 ++++++++ .../configsource/provider_test.go} | 179 ++++++-- .../testdata/arrays_and_maps_expected.yaml | 12 + .../configsource/testdata/basic_config.yaml | 5 + .../testdata/manager_resolve_error.yaml | 4 + .../testdata/yaml_injection_expected.yaml | 8 + tests/general/configproviders/file_test.go | 43 ++ .../configproviders/testdata/file_config.yaml | 13 + .../testdata/file_with_config_content | 3 + .../testdata/resource_metrics/memory.yaml | 8 + .../configsources/testdata/vault_config.yaml | 9 +- tests/general/envvar_expansion_test.go | 1 - .../incompat_env_config_source_labels.yaml | 10 +- 75 files changed, 1203 insertions(+), 1245 deletions(-) delete mode 100644 internal/configprovider/builder.go delete mode 100644 internal/configprovider/builder_test.go delete mode 100644 internal/configprovider/component.go delete mode 100644 internal/configprovider/config.go delete mode 100644 internal/configprovider/config_source_provider.go delete mode 100644 internal/configprovider/factory.go delete mode 100644 internal/configprovider/parser.go delete mode 100644 internal/configprovider/testconfigsource_test.go create mode 100644 internal/configsource/settings.go rename internal/{configprovider/parser_test.go => configsource/settings_test.go} (65%) rename internal/{configprovider/manager.go => configsource/source.go} (74%) rename internal/{configprovider/manager_test.go => configsource/source_test.go} (55%) rename internal/{configprovider => configsource}/testdata/arrays_and_maps.yaml (100%) rename internal/{configprovider => configsource}/testdata/arrays_and_maps_expected.yaml (100%) rename internal/{configprovider => configsource}/testdata/bad_name.yaml (100%) rename internal/{configprovider => configsource}/testdata/basic_config.yaml (100%) rename internal/{configprovider => configsource}/testdata/cfgsrc_load_use_cfgsrc.yaml (100%) rename internal/{configprovider => configsource}/testdata/duplicated_name.yaml (100%) rename internal/{configprovider => configsource}/testdata/env_var_on_load.yaml (100%) rename internal/{configprovider => configsource}/testdata/envvar_cfgsrc_mix.yaml (100%) rename internal/{configprovider => configsource}/testdata/envvar_cfgsrc_mix_expected.yaml (100%) rename internal/{configprovider => configsource}/testdata/manager_resolve_error.yaml (100%) rename internal/{configprovider => configsource}/testdata/params_handling.yaml (100%) rename internal/{configprovider => configsource}/testdata/params_handling_expected.yaml (100%) rename internal/{configprovider => configsource}/testdata/unknown_field.yaml (100%) rename internal/{configprovider => configsource}/testdata/yaml_injection.yaml (100%) rename internal/{configprovider => configsource}/testdata/yaml_injection_expected.yaml (100%) delete mode 100644 internal/configsources/configsources.go delete mode 100644 internal/configsources/configsources_test.go create mode 100644 internal/confmapprovider/configsource/provider.go rename internal/{configprovider/config_source_provider_test.go => confmapprovider/configsource/provider_test.go} (51%) create mode 100644 internal/confmapprovider/configsource/testdata/arrays_and_maps_expected.yaml create mode 100644 internal/confmapprovider/configsource/testdata/basic_config.yaml create mode 100644 internal/confmapprovider/configsource/testdata/manager_resolve_error.yaml create mode 100644 internal/confmapprovider/configsource/testdata/yaml_injection_expected.yaml create mode 100644 tests/general/configproviders/file_test.go create mode 100644 tests/general/configproviders/testdata/file_config.yaml create mode 100644 tests/general/configproviders/testdata/file_with_config_content create mode 100644 tests/general/configproviders/testdata/resource_metrics/memory.yaml diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go index 0b872822eb..abd43e079a 100644 --- a/cmd/otelcol/main.go +++ b/cmd/otelcol/main.go @@ -32,8 +32,7 @@ import ( "github.com/signalfx/splunk-otel-collector/internal/components" "github.com/signalfx/splunk-otel-collector/internal/configconverter" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" - "github.com/signalfx/splunk-otel-collector/internal/configsources" + "github.com/signalfx/splunk-otel-collector/internal/confmapprovider/configsource" "github.com/signalfx/splunk-otel-collector/internal/confmapprovider/discovery" "github.com/signalfx/splunk-otel-collector/internal/settings" "github.com/signalfx/splunk-otel-collector/internal/version" @@ -72,31 +71,20 @@ func main() { log.Fatalf("failed to create discovery provider: %v", err) } - hooks := []configprovider.Hook{configServer, dryRun} + hooks := []configsource.Hook{configServer, dryRun} envProvider := envprovider.New() fileProvider := fileprovider.New() + configSourceProvider := configsource.New(zap.NewNop(), hooks) serviceConfigProvider, err := otelcol.NewConfigProvider( otelcol.ConfigProviderSettings{ ResolverSettings: confmap.ResolverSettings{ URIs: collectorSettings.ResolverURIs(), Providers: map[string]confmap.Provider{ - discovery.PropertyScheme(): configprovider.NewConfigSourceConfigMapProvider( - discovery.PropertyProvider(), zap.NewNop(), info, hooks, configsources.Get()..., - ), - discovery.ConfigDScheme(): configprovider.NewConfigSourceConfigMapProvider( - discovery.ConfigDProvider(), - zap.NewNop(), // The service logger is not available yet, setting it to Nop. - info, hooks, configsources.Get()..., - ), - discovery.DiscoveryModeScheme(): configprovider.NewConfigSourceConfigMapProvider( - discovery.DiscoveryModeProvider(), zap.NewNop(), info, hooks, configsources.Get()..., - ), - envProvider.Scheme(): configprovider.NewConfigSourceConfigMapProvider( - envProvider, zap.NewNop(), info, hooks, configsources.Get()..., - ), - fileProvider.Scheme(): configprovider.NewConfigSourceConfigMapProvider( - fileProvider, zap.NewNop(), info, hooks, configsources.Get()..., - ), + discovery.PropertyScheme(): configSourceProvider.Wrap(discovery.PropertyProvider()), + discovery.ConfigDScheme(): configSourceProvider.Wrap(discovery.ConfigDProvider()), + discovery.DiscoveryModeScheme(): configSourceProvider.Wrap(discovery.DiscoveryModeProvider()), + envProvider.Scheme(): configSourceProvider.Wrap(envProvider), + fileProvider.Scheme(): configSourceProvider.Wrap(fileProvider), }, Converters: confMapConverters, }, }) diff --git a/internal/configconverter/config_server.go b/internal/configconverter/config_server.go index c2dd1a1fd4..270488bce8 100644 --- a/internal/configconverter/config_server.go +++ b/internal/configconverter/config_server.go @@ -32,7 +32,7 @@ import ( "go.opentelemetry.io/collector/confmap" "gopkg.in/yaml.v2" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/confmapprovider/configsource" ) const ( @@ -51,7 +51,7 @@ const ( ) var _ confmap.Converter = (*ConfigServer)(nil) -var _ configprovider.Hook = (*ConfigServer)(nil) +var _ configsource.Hook = (*ConfigServer)(nil) type ConfigServer struct { // Use get/set methods instead of direct usage diff --git a/internal/configconverter/dry_run.go b/internal/configconverter/dry_run.go index bcf128c66f..e4e9513399 100644 --- a/internal/configconverter/dry_run.go +++ b/internal/configconverter/dry_run.go @@ -23,11 +23,11 @@ import ( "go.opentelemetry.io/collector/confmap" "gopkg.in/yaml.v2" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/confmapprovider/configsource" ) var _ confmap.Converter = (*DryRun)(nil) -var _ configprovider.Hook = (*DryRun)(nil) +var _ configsource.Hook = (*DryRun)(nil) type DryRun struct { *sync.Mutex diff --git a/internal/configprovider/builder.go b/internal/configprovider/builder.go deleted file mode 100644 index aa28636d14..0000000000 --- a/internal/configprovider/builder.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - "fmt" - - "go.uber.org/zap" -) - -// Build builds the ConfigSource objects according to the given ConfigSettings. -func Build(ctx context.Context, configSourcesSettings map[string]Source, params CreateParams, factories Factories) (map[string]ConfigSource, error) { - cfgSources := make(map[string]ConfigSource, len(configSourcesSettings)) - for fullName, cfgSrcSettings := range configSourcesSettings { - // If we have the setting we also have the factory. - factory, ok := factories[cfgSrcSettings.ID().Type()] - if !ok { - return nil, fmt.Errorf("unknown %s config source type for %s", cfgSrcSettings.ID().Type(), fullName) - } - - params.Logger = params.Logger.With(zap.String("config_source", fullName)) - cfgSrc, err := factory.CreateConfigSource(ctx, params, cfgSrcSettings) - if err != nil { - return nil, fmt.Errorf("failed to create config source %s: %w", fullName, err) - } - - if cfgSrc == nil { - return nil, fmt.Errorf("factory for %q produced a nil extension", fullName) - } - - cfgSources[fullName] = cfgSrc - } - - return cfgSources, nil -} diff --git a/internal/configprovider/builder_test.go b/internal/configprovider/builder_test.go deleted file mode 100644 index 6618fb5a7c..0000000000 --- a/internal/configprovider/builder_test.go +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - "errors" - "testing" - - "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/component" - "go.uber.org/zap" -) - -func TestConfigSourceBuild(t *testing.T) { - ctx := context.Background() - params := CreateParams{ - Logger: zap.NewNop(), - BuildInfo: component.NewDefaultBuildInfo(), - } - - testFactories := Factories{ - "tstcfgsrc": &mockCfgSrcFactory{}, - } - - tests := []struct { - configSettings map[string]Source - factories Factories - expectedCfgSources map[string]ConfigSource - wantErr string - name string - }{ - { - name: "unknown_config_source", - configSettings: map[string]Source{ - "tstcfgsrc": &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewIDWithName("unknown_config_source", "tstcfgsrc")), - }, - }, - factories: testFactories, - wantErr: "unknown unknown_config_source config source type for tstcfgsrc", - }, - { - name: "creation_error", - configSettings: map[string]Source{ - "tstcfgsrc": &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), - }, - }, - factories: Factories{ - "tstcfgsrc": &mockCfgSrcFactory{ - ErrOnCreateConfigSource: errors.New("forced test error"), - }, - }, - wantErr: "failed to create config source tstcfgsrc: forced test error", - }, - { - name: "factory_return_nil", - configSettings: map[string]Source{ - "tstcfgsrc": &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), - }, - }, - factories: Factories{ - "tstcfgsrc": &mockNilCfgSrcFactory{}, - }, - wantErr: "factory for \"tstcfgsrc\" produced a nil extension", - }, - { - name: "base_case", - configSettings: map[string]Source{ - "tstcfgsrc/named": &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewIDWithName("tstcfgsrc", "named")), - Endpoint: "some_endpoint", - Token: "some_token", - }, - }, - factories: testFactories, - expectedCfgSources: map[string]ConfigSource{ - "tstcfgsrc/named": &testConfigSource{ - ValueMap: map[string]valueEntry{ - "tstcfgsrc/named": { - Value: &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewIDWithName("tstcfgsrc", "named")), - Endpoint: "some_endpoint", - Token: "some_token", - }, - }, - }, - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - builtCfgSources, err := Build(ctx, tt.configSettings, params, tt.factories) - if tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - require.Equal(t, tt.expectedCfgSources, builtCfgSources) - }) - } -} - -type mockNilCfgSrcFactory struct{} - -func (m *mockNilCfgSrcFactory) Type() component.Type { - return "tstcfgsrc" -} - -var _ (Factory) = (*mockNilCfgSrcFactory)(nil) - -func (m *mockNilCfgSrcFactory) CreateDefaultConfig() Source { - return &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), - Endpoint: "default_endpoint", - } -} - -func (m *mockNilCfgSrcFactory) CreateConfigSource(context.Context, CreateParams, Source) (ConfigSource, error) { - return nil, nil -} diff --git a/internal/configprovider/component.go b/internal/configprovider/component.go deleted file mode 100644 index 94b4837db3..0000000000 --- a/internal/configprovider/component.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - - "go.opentelemetry.io/collector/confmap" -) - -// ConfigSource is the interface to be implemented by objects used by the collector -// to retrieve external configuration information. -// -// ConfigSource object will be used to retrieve full configuration or data to be -// injected into a configuration. -// -// The ConfigSource object should use its creation according to the source needs: -// lock resources, open connections, etc. An implementation, for instance, -// can use the creation time to prevent torn configurations, by acquiring a lock -// (or some other mechanism) that prevents concurrent changes to the configuration -// during time that data is being retrieved from the source. -// -// The code managing the ConfigSource instance must guarantee that the object is not used concurrently. -type ConfigSource interface { - // Retrieve goes to the configuration source and retrieves the selected data which - // contains the value to be injected in the configuration and the corresponding watcher that - // will be used to monitor for updates of the retrieved value. The retrieved value is selected - // according to the selector and the params passed in the call to Retrieve. - // - // The selector is a string that is required on all invocations, the params are optional. Each - // implementation handles the generic params according to their requirements. - Retrieve(ctx context.Context, selector string, paramsConfigMap *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) - - // Shutdown signals that the configuration for which it was used to retrieve values is no longer in use - // and the object should close and release any watchers that it may have created. - // This method must be called when the configuration session ends, either in case of success or error. - Shutdown(ctx context.Context) error -} diff --git a/internal/configprovider/config.go b/internal/configprovider/config.go deleted file mode 100644 index 7c91050735..0000000000 --- a/internal/configprovider/config.go +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "go.opentelemetry.io/collector/component" -) - -// SourceSettings defines common settings of a Source configuration. -// Specific config sources can embed this struct and extend it with more fields if needed. -// When embedded it must be with `mapstructure:",squash"` tag. -type SourceSettings struct { - id component.ID `mapstructure:"-"` -} - -// ID returns the ID of the component that this configuration belongs to. -func (s *SourceSettings) ID() component.ID { - return s.id -} - -// SetIDName updates the name part of the ID for the component that this configuration belongs to. -func (s *SourceSettings) SetIDName(idName string) { - s.id = component.NewIDWithName(s.id.Type(), idName) -} - -// NewSourceSettings return a new config.SourceSettings struct with the given ComponentID. -func NewSourceSettings(id component.ID) SourceSettings { - return SourceSettings{id} -} - -// Source is the configuration of a config source. Specific config sources must implement this -// interface and will typically embed SourceSettings struct or a struct that extends it. -type Source interface { - // TODO: While config sources are experimental and not in the config package they can't - // reference the private interfaces config.identifiable and config.validatable. - // Defining the required methods of the interfaces temporarily here. - - // From config.identifiable: - - // ID returns the ID of the component that this configuration belongs to. - ID() component.ID - // SetIDName updates the name part of the ID for the component that this configuration belongs to. - SetIDName(idName string) - - // From config.validatable: - - // Validate validates the configuration and returns an error if invalid. - Validate() error -} diff --git a/internal/configprovider/config_source_provider.go b/internal/configprovider/config_source_provider.go deleted file mode 100644 index 15bdba796a..0000000000 --- a/internal/configprovider/config_source_provider.go +++ /dev/null @@ -1,132 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - "fmt" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/confmap" - "go.uber.org/zap" -) - -var ( - _ confmap.Provider = (*configSourceConfigMapProvider)(nil) -) - -type Hook interface { - OnNew() - OnRetrieve(scheme string, retrieved map[string]any) - OnShutdown() -} - -type configSourceConfigMapProvider struct { - logger *zap.Logger - hooks []Hook - wrappedProvider confmap.Provider - wrappedRetrieved *confmap.Retrieved - buildInfo component.BuildInfo - factories []Factory -} - -// NewConfigSourceConfigMapProvider creates a ParserProvider that uses config sources. -func NewConfigSourceConfigMapProvider(wrappedProvider confmap.Provider, logger *zap.Logger, - buildInfo component.BuildInfo, hooks []Hook, factories ...Factory) confmap.Provider { - for _, h := range hooks { - h.OnNew() - } - return &configSourceConfigMapProvider{ - hooks: hooks, - wrappedProvider: wrappedProvider, - logger: logger, - factories: factories, - buildInfo: buildInfo, - wrappedRetrieved: &confmap.Retrieved{}, - } -} - -func (c *configSourceConfigMapProvider) Retrieve(ctx context.Context, uri string, onChange confmap.WatcherFunc) (*confmap.Retrieved, error) { - var tmpWR *confmap.Retrieved - var err error - newWrappedRetrieved := &confmap.Retrieved{} - if tmpWR, err = c.wrappedProvider.Retrieve(ctx, uri, onChange); err != nil { - return nil, err - } else if tmpWR != nil { - newWrappedRetrieved = tmpWR - } - - existingMap, err := c.wrappedRetrieved.AsConf() - if err != nil { - return nil, err - } - - // Need to merge config maps that we've encountered so far - if existingMap != nil { - wrMap, _ := newWrappedRetrieved.AsConf() - wrMap.Merge(existingMap) - if tmpWR, err = confmap.NewRetrieved(wrMap.ToStringMap()); err != nil { - return nil, err - } else if tmpWR != nil { - newWrappedRetrieved = tmpWR - } - } - c.wrappedRetrieved = newWrappedRetrieved - - factories, err := makeFactoryMap(c.factories) - if err != nil { - return nil, err - } - - wrappedMap, err := c.wrappedRetrieved.AsConf() - if err != nil { - return nil, err - } - - scheme, stringMap := c.Scheme(), wrappedMap.ToStringMap() - for _, h := range c.hooks { - h.OnRetrieve(scheme, stringMap) - } - - retrieved, closeFunc, err := Resolve(ctx, wrappedMap, c.logger, c.buildInfo, factories, onChange) - if err != nil { - return nil, err - } - - return confmap.NewRetrieved(retrieved, confmap.WithRetrievedClose(mergeCloseFuncs([]confmap.CloseFunc{closeFunc, c.wrappedRetrieved.Close}))) -} - -func (c *configSourceConfigMapProvider) Scheme() string { - return c.wrappedProvider.Scheme() -} - -func (c *configSourceConfigMapProvider) Shutdown(ctx context.Context) error { - for _, h := range c.hooks { - h.OnShutdown() - } - return c.wrappedProvider.Shutdown(ctx) -} - -func makeFactoryMap(factories []Factory) (Factories, error) { - fMap := make(Factories) - for _, f := range factories { - if _, ok := fMap[f.Type()]; ok { - return nil, fmt.Errorf("duplicate config source factory %q", f.Type()) - } - fMap[f.Type()] = f - } - return fMap, nil -} diff --git a/internal/configprovider/factory.go b/internal/configprovider/factory.go deleted file mode 100644 index c71fab4493..0000000000 --- a/internal/configprovider/factory.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - - "go.opentelemetry.io/collector/component" - "go.uber.org/zap" -) - -// CreateParams is passed to Factory.Create* functions. -type CreateParams struct { - // Logger that the factory can use during creation and can pass to the created - // ConfigSource to be used later as well. - Logger *zap.Logger - - // BuildInfo can be used to retrieve data according to version, etc. - BuildInfo component.BuildInfo -} - -// Factory is a factory interface for configuration sources. Given it's not an accepted component and -// because of the direct Factory usage restriction from https://github.com/open-telemetry/opentelemetry-collector/commit/9631ceabb7dc4ca5cc187bab26d8319783bcc562 -// it's not a proper Collector config.Factory. -type Factory interface { - // CreateDefaultConfig creates the default configuration settings for the ConfigSource. - // This method can be called multiple times depending on the pipeline - // configuration and should not cause side-effects that prevent the creation - // of multiple instances of the ConfigSource. - // The object returned by this method needs to pass the checks implemented by - // 'configcheck.ValidateConfig'. It is recommended to have such check in the - // tests of any implementation of the Factory interface. - CreateDefaultConfig() Source - - // CreateConfigSource creates a configuration source based on the given config. - CreateConfigSource(ctx context.Context, params CreateParams, cfg Source) (ConfigSource, error) - - // Type gets the type of the component created by this factory. - Type() component.Type -} - -// Factories maps the type of a ConfigSource to the respective factory object. -type Factories map[component.Type]Factory diff --git a/internal/configprovider/parser.go b/internal/configprovider/parser.go deleted file mode 100644 index 9883d760a1..0000000000 --- a/internal/configprovider/parser.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - "fmt" - "strings" - - "github.com/spf13/cast" - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/confmap" -) - -const ( - configSourcesKey = "config_sources" -) - -// Load reads the configuration for ConfigSource objects from the given parser and returns a map -// from the full name of config sources to the respective ConfigSettings. -func Load(ctx context.Context, v *confmap.Conf, factories Factories) (map[string]Source, error) { - processedParser, err := processParser(ctx, v) - if err != nil { - return nil, err - } - - cfgSrcSettings, err := loadSettings(cast.ToStringMap(processedParser.Get(configSourcesKey)), factories) - if err != nil { - return nil, err - } - - return cfgSrcSettings, nil -} - -// processParser prepares a confmap.Conf to be used to load config source settings. -func processParser(ctx context.Context, v *confmap.Conf) (*confmap.Conf, error) { - processedParser := map[string]any{} - for _, key := range v.AllKeys() { - if !strings.HasPrefix(key, configSourcesKey) { - // In Load we only care about config sources, ignore everything else. - continue - } - - value, _, err := parseConfigValue(ctx, make(map[string]ConfigSource), v.Get(key), nil) - if err != nil { - return nil, err - } - processedParser[key] = value - } - - return confmap.NewFromStringMap(processedParser), nil -} - -func loadSettings(css map[string]any, factories Factories) (map[string]Source, error) { - // Prepare resulting map. - cfgSrcToSettings := make(map[string]Source) - - // Iterate over extensions and create a config for each. - for key, value := range css { - settingsParser := confmap.NewFromStringMap(cast.ToStringMap(value)) - - // Decode the key into type and fullName components. - componentID := component.ID{} - if err := componentID.UnmarshalText([]byte(key)); err != nil { - return nil, fmt.Errorf("invalid %s type and name key %q: %w", configSourcesKey, key, err) - } - - // Find the factory based on "type" that we read from config source. - factory := factories[componentID.Type()] - if factory == nil { - return nil, fmt.Errorf("unknown %s type %q for %q", configSourcesKey, componentID.Type(), componentID) - } - - // Create the default config. - cfgSrcSettings := factory.CreateDefaultConfig() - cfgSrcSettings.SetIDName(componentID.Name()) - - // Now that the default settings struct is created we can Unmarshal into it - // and it will apply user-defined config on top of the default. - if err := settingsParser.Unmarshal(&cfgSrcSettings, confmap.WithErrorUnused()); err != nil { - return nil, fmt.Errorf("error reading %s configuration for %q: %w", configSourcesKey, componentID, err) - } - - fullName := componentID.String() - if cfgSrcToSettings[fullName] != nil { - return nil, fmt.Errorf("duplicate %s name %s", configSourcesKey, fullName) - } - - cfgSrcToSettings[fullName] = cfgSrcSettings - } - - return cfgSrcToSettings, nil -} diff --git a/internal/configprovider/testconfigsource_test.go b/internal/configprovider/testconfigsource_test.go deleted file mode 100644 index 0e85b2274f..0000000000 --- a/internal/configprovider/testconfigsource_test.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configprovider - -import ( - "context" - "fmt" - - "go.opentelemetry.io/collector/confmap" -) - -// testConfigSource a ConfigSource to be used in tests. -type testConfigSource struct { - ValueMap map[string]valueEntry - - ErrOnRetrieve error - ErrOnRetrieveEnd error - ErrOnClose error - - OnRetrieve func(ctx context.Context, selector string, paramsConfigMap *confmap.Conf) error -} - -type valueEntry struct { - Value any - WatchForUpdateCh chan error -} - -var _ ConfigSource = (*testConfigSource)(nil) - -func (t *testConfigSource) Retrieve(ctx context.Context, selector string, paramsConfigMap *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) { - if t.OnRetrieve != nil { - if err := t.OnRetrieve(ctx, selector, paramsConfigMap); err != nil { - return nil, err - } - } - - if t.ErrOnRetrieve != nil { - return nil, t.ErrOnRetrieve - } - - entry, ok := t.ValueMap[selector] - if !ok { - return nil, fmt.Errorf("no value for selector %q", selector) - } - - if entry.WatchForUpdateCh != nil { - doneCh := make(chan struct{}) - startWatch(entry.WatchForUpdateCh, doneCh, watcher) - return confmap.NewRetrieved(entry.Value, confmap.WithRetrievedClose(func(ctx context.Context) error { - close(doneCh) - return nil - })) - } - - return confmap.NewRetrieved(entry.Value) -} - -func (t *testConfigSource) Shutdown(context.Context) error { - return t.ErrOnClose -} - -func startWatch(watchForUpdateCh chan error, doneCh chan struct{}, watcher confmap.WatcherFunc) { - go func() { - select { - case err := <-watchForUpdateCh: - watcher(&confmap.ChangeEvent{Error: err}) - return - case <-doneCh: - return - } - }() -} diff --git a/internal/configsource/envvarconfigsource/config.go b/internal/configsource/envvarconfigsource/config.go index 8d3fa907b5..a086c4c73d 100644 --- a/internal/configsource/envvarconfigsource/config.go +++ b/internal/configsource/envvarconfigsource/config.go @@ -15,16 +15,12 @@ package envvarconfigsource -import "github.com/signalfx/splunk-otel-collector/internal/configprovider" +import "github.com/signalfx/splunk-otel-collector/internal/configsource" // Config holds the configuration for the creation of environment variable config source objects. type Config struct { // Defaults specify a map to fallback if a given environment variable is not defined. Defaults map[string]any `mapstructure:"defaults"` - configprovider.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct -} - -func (*Config) Validate() error { - return nil + configsource.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct } diff --git a/internal/configsource/envvarconfigsource/config_test.go b/internal/configsource/envvarconfigsource/config_test.go index 64cbcc3e82..c4e25cc20c 100644 --- a/internal/configsource/envvarconfigsource/config_test.go +++ b/internal/configsource/envvarconfigsource/config_test.go @@ -25,7 +25,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestEnvVarConfigSourceLoadConfig(t *testing.T) { @@ -33,19 +33,20 @@ func TestEnvVarConfigSourceLoadConfig(t *testing.T) { v, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - factories := map[component.Type]configprovider.Factory{ + factories := map[component.Type]configsource.Factory{ typeStr: NewFactory(), } - actualSettings, err := configprovider.Load(context.Background(), v, factories) + actualSettings, splitConf, err := configsource.SettingsFromConf(context.Background(), v, factories, nil) require.NoError(t, err) + require.NotNil(t, splitConf) - expectedSettings := map[string]configprovider.Source{ + expectedSettings := map[string]configsource.Settings{ "env": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), }, "env/with_fallback": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "with_fallback")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "with_fallback")), Defaults: map[string]any{ "k0": 42, "m0": map[string]any{ @@ -57,10 +58,8 @@ func TestEnvVarConfigSourceLoadConfig(t *testing.T) { } require.Equal(t, expectedSettings, actualSettings) + require.Empty(t, splitConf.ToStringMap()) - params := configprovider.CreateParams{ - Logger: zap.NewNop(), - } - _, err = configprovider.Build(context.Background(), actualSettings, params, factories) + _, err = configsource.BuildConfigSources(context.Background(), actualSettings, zap.NewNop(), factories) require.NoError(t, err) } diff --git a/internal/configsource/envvarconfigsource/factory.go b/internal/configsource/envvarconfigsource/factory.go index 3e702165da..ea1ba6be6b 100644 --- a/internal/configsource/envvarconfigsource/factory.go +++ b/internal/configsource/envvarconfigsource/factory.go @@ -19,8 +19,9 @@ import ( "context" "go.opentelemetry.io/collector/component" + "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const ( @@ -34,17 +35,17 @@ func (e *envVarFactory) Type() component.Type { return typeStr } -func (e *envVarFactory) CreateDefaultConfig() configprovider.Source { +func (e *envVarFactory) CreateDefaultConfig() configsource.Settings { return &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), } } -func (e *envVarFactory) CreateConfigSource(_ context.Context, params configprovider.CreateParams, cfg configprovider.Source) (configprovider.ConfigSource, error) { - return newConfigSource(params, cfg.(*Config)), nil +func (e *envVarFactory) CreateConfigSource(_ context.Context, settings configsource.Settings, _ *zap.Logger) (configsource.ConfigSource, error) { + return newConfigSource(settings.(*Config)), nil } // NewFactory creates a factory for Vault ConfigSource objects. -func NewFactory() configprovider.Factory { +func NewFactory() configsource.Factory { return &envVarFactory{} } diff --git a/internal/configsource/envvarconfigsource/factory_test.go b/internal/configsource/envvarconfigsource/factory_test.go index 0ceda213d6..5e6e8f7633 100644 --- a/internal/configsource/envvarconfigsource/factory_test.go +++ b/internal/configsource/envvarconfigsource/factory_test.go @@ -22,16 +22,11 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestEnvVarConfigSourceFactory_CreateConfigSource(t *testing.T) { factory := NewFactory() assert.Equal(t, component.Type("env"), factory.Type()) - createParams := configprovider.CreateParams{ - Logger: zap.NewNop(), - } tests := []struct { config *Config name string @@ -51,7 +46,7 @@ func TestEnvVarConfigSourceFactory_CreateConfigSource(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := factory.CreateConfigSource(context.Background(), createParams, tt.config) + actual, err := factory.CreateConfigSource(context.Background(), tt.config, zap.NewNop()) assert.NoError(t, err) assert.NotNil(t, actual) }) diff --git a/internal/configsource/envvarconfigsource/source.go b/internal/configsource/envvarconfigsource/source.go index f3a2aae2d5..a8ae079f34 100644 --- a/internal/configsource/envvarconfigsource/source.go +++ b/internal/configsource/envvarconfigsource/source.go @@ -22,7 +22,7 @@ import ( "go.opentelemetry.io/collector/confmap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) // Private error types to help with testability. @@ -39,12 +39,11 @@ type retrieveParams struct { Optional bool `mapstructure:"optional"` } -// envVarConfigSource implements the configprovider.Session interface. type envVarConfigSource struct { defaults map[string]any } -func newConfigSource(_ configprovider.CreateParams, cfg *Config) configprovider.ConfigSource { +func newConfigSource(cfg *Config) configsource.ConfigSource { defaults := make(map[string]any) if cfg.Defaults != nil { defaults = cfg.Defaults @@ -79,7 +78,3 @@ func (e *envVarConfigSource) Retrieve(_ context.Context, selector string, params return confmap.NewRetrieved(defaultValue) } - -func (e *envVarConfigSource) Shutdown(context.Context) error { - return nil -} diff --git a/internal/configsource/envvarconfigsource/source_test.go b/internal/configsource/envvarconfigsource/source_test.go index f97d6b7ca3..6ccc7d0805 100644 --- a/internal/configsource/envvarconfigsource/source_test.go +++ b/internal/configsource/envvarconfigsource/source_test.go @@ -93,7 +93,6 @@ func TestEnvVarConfigSource_Session(t *testing.T) { if tt.wantErr != nil { assert.Nil(t, r) require.IsType(t, tt.wantErr, err) - assert.NoError(t, source.Shutdown(ctx)) return } require.NoError(t, err) @@ -104,7 +103,6 @@ func TestEnvVarConfigSource_Session(t *testing.T) { assert.Equal(t, tt.expected, val) assert.NoError(t, r.Close(ctx)) - assert.NoError(t, source.Shutdown(ctx)) }) } } diff --git a/internal/configsource/etcd2configsource/config.go b/internal/configsource/etcd2configsource/config.go index 7a8509a5e1..38ee488d2d 100644 --- a/internal/configsource/etcd2configsource/config.go +++ b/internal/configsource/etcd2configsource/config.go @@ -15,11 +15,13 @@ package etcd2configsource -import "github.com/signalfx/splunk-otel-collector/internal/configprovider" +import ( + "github.com/signalfx/splunk-otel-collector/internal/configsource" +) // Config defines etcd2configsource configuration type Config struct { - configprovider.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct + configsource.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct // Authentication defines the authentication method to be used. Authentication *Authentication `mapstructure:"auth"` @@ -37,7 +39,3 @@ type Authentication struct { // Password can be optionally used to authenticate with etcd2 cluster. Password string `mapstructure:"password"` } - -func (*Config) Validate() error { - return nil -} diff --git a/internal/configsource/etcd2configsource/config_test.go b/internal/configsource/etcd2configsource/config_test.go index 73d72004f7..aa50ad0571 100644 --- a/internal/configsource/etcd2configsource/config_test.go +++ b/internal/configsource/etcd2configsource/config_test.go @@ -25,7 +25,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestEtcd2LoadConfig(t *testing.T) { @@ -33,20 +33,21 @@ func TestEtcd2LoadConfig(t *testing.T) { v, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - factories := map[component.Type]configprovider.Factory{ + factories := map[component.Type]configsource.Factory{ typeStr: NewFactory(), } - actualSettings, err := configprovider.Load(context.Background(), v, factories) + actualSettings, splitConf, err := configsource.SettingsFromConf(context.Background(), v, factories, nil) require.NoError(t, err) + require.NotNil(t, splitConf) - expectedSettings := map[string]configprovider.Source{ + expectedSettings := map[string]configsource.Settings{ "etcd2": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), Endpoints: []string{"http://localhost:1234"}, }, "etcd2/auth": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "auth")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "auth")), Endpoints: []string{"https://localhost:3456"}, Authentication: &Authentication{ Username: "user", @@ -54,12 +55,9 @@ func TestEtcd2LoadConfig(t *testing.T) { }, }, } - require.Equal(t, expectedSettings, actualSettings) + require.Empty(t, splitConf.ToStringMap()) - params := configprovider.CreateParams{ - Logger: zap.NewNop(), - } - _, err = configprovider.Build(context.Background(), actualSettings, params, factories) + _, err = configsource.BuildConfigSources(context.Background(), actualSettings, zap.NewNop(), factories) require.NoError(t, err) } diff --git a/internal/configsource/etcd2configsource/factory.go b/internal/configsource/etcd2configsource/factory.go index c5b1b6bbf7..65cea54204 100644 --- a/internal/configsource/etcd2configsource/factory.go +++ b/internal/configsource/etcd2configsource/factory.go @@ -21,8 +21,9 @@ import ( "net/url" "go.opentelemetry.io/collector/component" + "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const ( @@ -44,15 +45,15 @@ func (v *etcd2Factory) Type() component.Type { return typeStr } -func (v *etcd2Factory) CreateDefaultConfig() configprovider.Source { +func (v *etcd2Factory) CreateDefaultConfig() configsource.Settings { return &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), Endpoints: []string{defaultEndpoints}, } } -func (v *etcd2Factory) CreateConfigSource(_ context.Context, params configprovider.CreateParams, cfg configprovider.Source) (configprovider.ConfigSource, error) { - etcd2Cfg := cfg.(*Config) +func (v *etcd2Factory) CreateConfigSource(_ context.Context, settings configsource.Settings, logger *zap.Logger) (configsource.ConfigSource, error) { + etcd2Cfg := settings.(*Config) if len(etcd2Cfg.Endpoints) == 0 { return nil, &errMissingEndpoint{errors.New("cannot connect to etcd2 without any endpoints")} @@ -64,10 +65,10 @@ func (v *etcd2Factory) CreateConfigSource(_ context.Context, params configprovid } } - return newConfigSource(params, etcd2Cfg) + return newConfigSource(etcd2Cfg, logger) } // NewFactory creates a new etcd2Factory instance -func NewFactory() configprovider.Factory { +func NewFactory() configsource.Factory { return &etcd2Factory{} } diff --git a/internal/configsource/etcd2configsource/factory_test.go b/internal/configsource/etcd2configsource/factory_test.go index ec5ccd6761..bad1be469a 100644 --- a/internal/configsource/etcd2configsource/factory_test.go +++ b/internal/configsource/etcd2configsource/factory_test.go @@ -23,16 +23,11 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestEtcd2Factory_CreateConfigSource(t *testing.T) { factory := NewFactory() assert.Equal(t, component.Type("etcd2"), factory.Type()) - createParams := configprovider.CreateParams{ - Logger: zap.NewNop(), - } tests := []struct { wantErr error config *Config @@ -70,7 +65,7 @@ func TestEtcd2Factory_CreateConfigSource(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := factory.CreateConfigSource(context.Background(), createParams, tt.config) + actual, err := factory.CreateConfigSource(context.Background(), tt.config, zap.NewNop()) require.IsType(t, tt.wantErr, err) if tt.wantErr == nil { assert.NotNil(t, actual) diff --git a/internal/configsource/etcd2configsource/source.go b/internal/configsource/etcd2configsource/source.go index 76398e775d..e1e0f70169 100644 --- a/internal/configsource/etcd2configsource/source.go +++ b/internal/configsource/etcd2configsource/source.go @@ -25,7 +25,7 @@ import ( "go.opentelemetry.io/collector/confmap" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const maxBackoffTime = time.Second * 60 @@ -36,7 +36,7 @@ type etcd2ConfigSource struct { kapi client.KeysAPI } -func newConfigSource(params configprovider.CreateParams, cfg *Config) (configprovider.ConfigSource, error) { +func newConfigSource(cfg *Config, logger *zap.Logger) (configsource.ConfigSource, error) { var username, password string if cfg.Authentication != nil { username = cfg.Authentication.Username @@ -54,7 +54,7 @@ func newConfigSource(params configprovider.CreateParams, cfg *Config) (configpro kapi := client.NewKeysAPI(etcdClient) return &etcd2ConfigSource{ - logger: params.Logger, + logger: logger, kapi: kapi, }, nil } @@ -70,10 +70,6 @@ func (s *etcd2ConfigSource) Retrieve(ctx context.Context, selector string, _ *co return confmap.NewRetrieved(resp.Node.Value, confmap.WithRetrievedClose(s.newWatcher(selector, resp.Node.ModifiedIndex, watcher))) } -func (s *etcd2ConfigSource) Shutdown(context.Context) error { - return nil -} - func (s *etcd2ConfigSource) newWatcher(selector string, index uint64, watcherFunc confmap.WatcherFunc) confmap.CloseFunc { watchCtx, cancel := context.WithCancel(context.Background()) watcher := s.kapi.Watcher(selector, &client.WatcherOptions{AfterIndex: index}) diff --git a/internal/configsource/etcd2configsource/source_test.go b/internal/configsource/etcd2configsource/source_test.go index 481ec85a92..a10affe0bf 100644 --- a/internal/configsource/etcd2configsource/source_test.go +++ b/internal/configsource/etcd2configsource/source_test.go @@ -64,7 +64,6 @@ func TestSessionRetrieve(t *testing.T) { assert.Nil(t, retrieved) }) } - assert.NoError(t, source.Shutdown(context.Background())) } func TestWatcher(t *testing.T) { @@ -116,7 +115,6 @@ func TestWatcher(t *testing.T) { assert.NoError(t, ce.Error) assert.NoError(t, retrieved.Close(context.Background())) } - assert.NoError(t, source.Shutdown(context.Background())) }) } } diff --git a/internal/configsource/includeconfigsource/config.go b/internal/configsource/includeconfigsource/config.go index c57416e59f..a7a648ac21 100644 --- a/internal/configsource/includeconfigsource/config.go +++ b/internal/configsource/includeconfigsource/config.go @@ -15,11 +15,13 @@ package includeconfigsource -import "github.com/signalfx/splunk-otel-collector/internal/configprovider" +import ( + "github.com/signalfx/splunk-otel-collector/internal/configsource" +) // Config holds the configuration for the creation of include config source objects. type Config struct { - configprovider.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct + configsource.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct // DeleteFiles is used to instruct the config source to delete the // files after its content is read. The default value is 'false'. // Set it to 'true' to force the deletion of the file as soon @@ -30,7 +32,3 @@ type Config struct { // Set it to 'true' to watch the referenced files for changes. WatchFiles bool `mapstructure:"watch_files"` } - -func (*Config) Validate() error { - return nil -} diff --git a/internal/configsource/includeconfigsource/config_test.go b/internal/configsource/includeconfigsource/config_test.go index f65e49183b..2821c96d75 100644 --- a/internal/configsource/includeconfigsource/config_test.go +++ b/internal/configsource/includeconfigsource/config_test.go @@ -26,7 +26,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestIncludeConfigSourceLoadConfig(t *testing.T) { @@ -34,33 +34,32 @@ func TestIncludeConfigSourceLoadConfig(t *testing.T) { v, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - factories := map[component.Type]configprovider.Factory{ + factories := map[component.Type]configsource.Factory{ typeStr: NewFactory(), } - actualSettings, err := configprovider.Load(context.Background(), v, factories) + actualSettings, splitConf, err := configsource.SettingsFromConf(context.Background(), v, factories, nil) require.NoError(t, err) + require.NotNil(t, splitConf) - expectedSettings := map[string]configprovider.Source{ + expectedSettings := map[string]configsource.Settings{ "include": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), }, "include/delete_files": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "delete_files")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "delete_files")), DeleteFiles: true, }, "include/watch_files": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "watch_files")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "watch_files")), WatchFiles: true, }, } require.Equal(t, expectedSettings, actualSettings) + require.Empty(t, splitConf.ToStringMap()) - params := configprovider.CreateParams{ - Logger: zap.NewNop(), - } - cfgSrcs, err := configprovider.Build(context.Background(), actualSettings, params, factories) + cfgSrcs, err := configsource.BuildConfigSources(context.Background(), actualSettings, zap.NewNop(), factories) require.NoError(t, err) for k := range expectedSettings { assert.Contains(t, cfgSrcs, k) diff --git a/internal/configsource/includeconfigsource/factory.go b/internal/configsource/includeconfigsource/factory.go index 1e06bb1881..a18d7d9fe3 100644 --- a/internal/configsource/includeconfigsource/factory.go +++ b/internal/configsource/includeconfigsource/factory.go @@ -19,8 +19,9 @@ import ( "context" "go.opentelemetry.io/collector/component" + "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const ( @@ -34,17 +35,17 @@ func (f *includeFactory) Type() component.Type { return typeStr } -func (f *includeFactory) CreateDefaultConfig() configprovider.Source { +func (f *includeFactory) CreateDefaultConfig() configsource.Settings { return &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), } } -func (f *includeFactory) CreateConfigSource(_ context.Context, params configprovider.CreateParams, cfg configprovider.Source) (configprovider.ConfigSource, error) { - return newConfigSource(params, cfg.(*Config)) +func (f *includeFactory) CreateConfigSource(_ context.Context, settings configsource.Settings, _ *zap.Logger) (configsource.ConfigSource, error) { + return newConfigSource(settings.(*Config)) } // NewFactory creates a factory for include ConfigSource objects. -func NewFactory() configprovider.Factory { +func NewFactory() configsource.Factory { return &includeFactory{} } diff --git a/internal/configsource/includeconfigsource/factory_test.go b/internal/configsource/includeconfigsource/factory_test.go index 87be4fba38..ad32efba02 100644 --- a/internal/configsource/includeconfigsource/factory_test.go +++ b/internal/configsource/includeconfigsource/factory_test.go @@ -22,16 +22,11 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestIncludeConfigSourceFactory_CreateConfigSource(t *testing.T) { factory := NewFactory() assert.Equal(t, component.Type("include"), factory.Type()) - createParams := configprovider.CreateParams{ - Logger: zap.NewNop(), - } tests := []struct { expected *includeConfigSource @@ -74,7 +69,7 @@ func TestIncludeConfigSourceFactory_CreateConfigSource(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := factory.CreateConfigSource(context.Background(), createParams, &tt.config) + actual, err := factory.CreateConfigSource(context.Background(), &tt.config, zap.NewNop()) if tt.wantErr { assert.Error(t, err) assert.Nil(t, actual) diff --git a/internal/configsource/includeconfigsource/source.go b/internal/configsource/includeconfigsource/source.go index bbe3bea08d..378025b7db 100644 --- a/internal/configsource/includeconfigsource/source.go +++ b/internal/configsource/includeconfigsource/source.go @@ -26,7 +26,7 @@ import ( "github.com/fsnotify/fsnotify" "go.opentelemetry.io/collector/confmap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) // Private error types to help with testability. @@ -41,7 +41,7 @@ type includeConfigSource struct { watchedFiles map[string]struct{} } -func newConfigSource(_ configprovider.CreateParams, config *Config) (configprovider.ConfigSource, error) { +func newConfigSource(config *Config) (configsource.ConfigSource, error) { if config.DeleteFiles && config.WatchFiles { return nil, errors.New(`cannot be configured with "delete_files" and "watch_files" at the same time`) } @@ -86,10 +86,6 @@ func (is *includeConfigSource) Retrieve(_ context.Context, selector string, para return confmap.NewRetrieved(buf.String(), confmap.WithRetrievedClose(closeFunc)) } -func (is *includeConfigSource) Shutdown(context.Context) error { - return nil -} - func (is *includeConfigSource) watchFile(file string, watcherFunc confmap.WatcherFunc) (confmap.CloseFunc, error) { if _, watched := is.watchedFiles[file]; watched { // This file is already watched another watch function is not needed. diff --git a/internal/configsource/includeconfigsource/source_test.go b/internal/configsource/includeconfigsource/source_test.go index 478431fcc8..1d3abcd470 100644 --- a/internal/configsource/includeconfigsource/source_test.go +++ b/internal/configsource/includeconfigsource/source_test.go @@ -25,8 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/confmap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestIncludeConfigSource_Session(t *testing.T) { @@ -65,7 +63,7 @@ func TestIncludeConfigSource_Session(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, err := newConfigSource(configprovider.CreateParams{}, &Config{}) + s, err := newConfigSource(&Config{}) require.NoError(t, err) ctx := context.Background() @@ -74,7 +72,6 @@ func TestIncludeConfigSource_Session(t *testing.T) { if tt.wantErr != nil { assert.Nil(t, r) require.IsType(t, tt.wantErr, err) - assert.NoError(t, s.Shutdown(ctx)) return } require.NoError(t, err) @@ -84,13 +81,12 @@ func TestIncludeConfigSource_Session(t *testing.T) { require.NoError(t, err) assert.Equal(t, tt.expected, val) require.NoError(t, r.Close(context.Background())) - require.NoError(t, s.Shutdown(ctx)) }) } } func TestIncludeConfigSourceWatchFileClose(t *testing.T) { - s, err := newConfigSource(configprovider.CreateParams{}, &Config{WatchFiles: true}) + s, err := newConfigSource(&Config{WatchFiles: true}) require.NoError(t, err) require.NotNil(t, s) @@ -117,11 +113,10 @@ func TestIncludeConfigSourceWatchFileClose(t *testing.T) { assert.Equal(t, "val1", val) require.NoError(t, r.Close(context.Background())) - require.NoError(t, s.Shutdown(ctx)) } func TestIncludeConfigSource_WatchFileUpdate(t *testing.T) { - s, err := newConfigSource(configprovider.CreateParams{}, &Config{WatchFiles: true}) + s, err := newConfigSource(&Config{WatchFiles: true}) require.NoError(t, err) require.NotNil(t, s) @@ -158,11 +153,10 @@ func TestIncludeConfigSource_WatchFileUpdate(t *testing.T) { require.NoError(t, err) assert.Equal(t, "val2", val) require.NoError(t, r.Close(context.Background())) - require.NoError(t, s.Shutdown(ctx)) } func TestIncludeConfigSourceDeleteFile(t *testing.T) { - s, err := newConfigSource(configprovider.CreateParams{}, &Config{DeleteFiles: true}) + s, err := newConfigSource(&Config{DeleteFiles: true}) require.NoError(t, err) require.NotNil(t, s) @@ -184,7 +178,6 @@ func TestIncludeConfigSourceDeleteFile(t *testing.T) { assert.Equal(t, "42", val) require.NoError(t, r.Close(context.Background())) - require.NoError(t, s.Shutdown(ctx)) } func TestIncludeConfigSource_DeleteFileError(t *testing.T) { @@ -194,7 +187,7 @@ func TestIncludeConfigSource_DeleteFileError(t *testing.T) { t.Skip("Windows only test") } - s, err := newConfigSource(configprovider.CreateParams{}, &Config{DeleteFiles: true}) + s, err := newConfigSource(&Config{DeleteFiles: true}) require.NoError(t, err) // Copy test file @@ -213,6 +206,4 @@ func TestIncludeConfigSource_DeleteFileError(t *testing.T) { r, err := s.Retrieve(ctx, dst, nil, nil) assert.IsType(t, &errFailedToDeleteFile{}, err) assert.Nil(t, r) - - require.NoError(t, s.Shutdown(ctx)) } diff --git a/internal/configsource/settings.go b/internal/configsource/settings.go new file mode 100644 index 0000000000..97b81506f9 --- /dev/null +++ b/internal/configsource/settings.go @@ -0,0 +1,136 @@ +// Copyright Splunk, Inc. +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configsource + +import ( + "context" + "fmt" + "strings" + + "github.com/spf13/cast" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/confmap" +) + +const ( + configSourcesKey = "config_sources" +) + +// Settings is the configuration of a config source. Specific config sources must implement this +// interface and will typically embed SourceSettings struct or a struct that extends it. +type Settings interface { + // ID returns the ID of the component that this configuration belongs to. + ID() component.ID + // SetIDName updates the name part of the ID for the component that this configuration belongs to. + SetIDName(idName string) +} + +// SourceSettings defines common settings of a Settings configuration. +// Specific config sources can embed this struct and extend it with more fields if needed. +// When embedded it must be with `mapstructure:",squash"` tag. +type SourceSettings struct { + id component.ID `mapstructure:"-"` +} + +// ID returns the ID of the component that this configuration belongs to. +func (s *SourceSettings) ID() component.ID { + return s.id +} + +// SetIDName updates the name part of the ID for the component that this configuration belongs to. +func (s *SourceSettings) SetIDName(idName string) { + s.id = component.NewIDWithName(s.id.Type(), idName) +} + +// NewSourceSettings return a new config.SourceSettings struct with the given ComponentID. +func NewSourceSettings(id component.ID) SourceSettings { + return SourceSettings{id} +} + +// SettingsFromConf reads the configuration for ConfigSource objects from the given confmap.Conf +// and returns a map of Settings and the remaining Conf without the `config_sources` mapping +func SettingsFromConf(ctx context.Context, conf *confmap.Conf, factories Factories, confmapProviders map[string]confmap.Provider) (map[string]Settings, *confmap.Conf, error) { + settings, err := sourceSettings(ctx, conf, factories, confmapProviders) + if err != nil { + return nil, nil, fmt.Errorf("failed resolving config sources settings: %w", err) + } + + splitMap := map[string]any{} + for _, k := range conf.AllKeys() { + if !strings.HasPrefix(k, configSourcesKey) { + splitMap[k] = conf.Get(k) + } + } + + return settings, confmap.NewFromStringMap(splitMap), nil +} + +func sourceSettings(ctx context.Context, v *confmap.Conf, factories Factories, confmapProviders map[string]confmap.Provider) (map[string]Settings, error) { + splitMap := map[string]any{} + for _, key := range v.AllKeys() { + if strings.HasPrefix(key, configSourcesKey) { + value, _, err := resolveConfigValue(ctx, make(map[string]ConfigSource), confmapProviders, v.Get(key), nil) + if err != nil { + return nil, err + } + splitMap[key] = value + } + } + settingsMap, err := confmap.NewFromStringMap(splitMap).Sub(configSourcesKey) + if err != nil { + return nil, fmt.Errorf("invalid settings sub map content: %w", err) + } + + return loadSettings(settingsMap.ToStringMap(), factories) +} + +func loadSettings(settingsMap map[string]any, factories Factories) (map[string]Settings, error) { + settings := make(map[string]Settings) + + for key, value := range settingsMap { + settingsValue := confmap.NewFromStringMap(cast.ToStringMap(value)) + + // Decode the key into type and fullName components. + componentID := component.ID{} + if err := componentID.UnmarshalText([]byte(key)); err != nil { + return nil, fmt.Errorf("invalid %s type and name key %q: %w", configSourcesKey, key, err) + } + + // Find the factory based on "type" that we read from config source. + factory, ok := factories[componentID.Type()] + if !ok || factory == nil { + return nil, fmt.Errorf("unknown %s type %q for %q", configSourcesKey, componentID.Type(), componentID) + } + + cfgSrcSettings := factory.CreateDefaultConfig() + cfgSrcSettings.SetIDName(componentID.Name()) + + // Now that the default settings struct is created we can Unmarshal into it + // and it will apply user-defined config on top of the default. + if err := settingsValue.Unmarshal(&cfgSrcSettings, confmap.WithErrorUnused()); err != nil { + return nil, fmt.Errorf("error reading %s configuration for %q: %w", configSourcesKey, componentID, err) + } + + fullName := componentID.String() + if _, ok = settings[fullName]; ok { + return nil, fmt.Errorf("duplicate %s name %s", configSourcesKey, fullName) + } + + settings[fullName] = cfgSrcSettings + } + + return settings, nil +} diff --git a/internal/configprovider/parser_test.go b/internal/configsource/settings_test.go similarity index 65% rename from internal/configprovider/parser_test.go rename to internal/configsource/settings_test.go index 968ea71f41..8bedc386ca 100644 --- a/internal/configprovider/parser_test.go +++ b/internal/configsource/settings_test.go @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package configprovider +package configsource import ( "context" @@ -31,27 +31,28 @@ func TestConfigSourceParser(t *testing.T) { ctx := context.Background() testFactories := Factories{ - "tstcfgsrc": &mockCfgSrcFactory{}, + "tstcfgsrc": &MockCfgSrcFactory{}, } tests := []struct { - factories Factories - expected map[string]Source - envvars map[string]string - wantErr string - name string - file string + factories Factories + expectedSettings map[string]Settings + expectedConf map[string]any + envvars map[string]string + wantErr string + name string + file string }{ { name: "basic_config", file: "basic_config", factories: testFactories, - expected: map[string]Source{ - "tstcfgsrc": &mockCfgSrcSettings{ + expectedSettings: map[string]Settings{ + "tstcfgsrc": &MockCfgSrcSettings{ SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), Endpoint: "some_endpoint", Token: "some_token", }, - "tstcfgsrc/named": &mockCfgSrcSettings{ + "tstcfgsrc/named": &MockCfgSrcSettings{ SourceSettings: NewSourceSettings(component.NewIDWithName("tstcfgsrc", "named")), Endpoint: "default_endpoint", }, @@ -65,19 +66,20 @@ func TestConfigSourceParser(t *testing.T) { "ENV_VAR_ENDPOINT": "env_var_endpoint", "ENV_VAR_TOKEN": "env_var_token", }, - expected: map[string]Source{ - "tstcfgsrc": &mockCfgSrcSettings{ + expectedSettings: map[string]Settings{ + "tstcfgsrc": &MockCfgSrcSettings{ SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), Endpoint: "https://env_var_endpoint:8200", Token: "env_var_token", }, }, + expectedConf: map[string]any{"ignored_by_parser": map[string]any{"some_field": "$ENV_VAR_TOKEN"}}, }, { name: "cfgsrc_load_cannot_use_cfgsrc", file: "cfgsrc_load_use_cfgsrc", factories: testFactories, - wantErr: "config source \"tstcfgsrc\" not found if this was intended to be an environment variable use \"${tstcfgsrc}\" instead\"", + wantErr: "config source \"tstcfgsrc\" not found", }, { name: "bad_name", @@ -89,7 +91,7 @@ func TestConfigSourceParser(t *testing.T) { name: "missing_factory", file: "basic_config", factories: Factories{ - "not_in_basic_config": &mockCfgSrcFactory{}, + "not_in_basic_config": &MockCfgSrcFactory{}, }, wantErr: "unknown config_sources type \"tstcfgsrc\"", }, @@ -120,55 +122,20 @@ func TestConfigSourceParser(t *testing.T) { }() } - cfgSrcSettings, err := Load(ctx, v, tt.factories) + cfgSrcSettings, splitConf, err := SettingsFromConf(ctx, v, tt.factories, nil) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, splitConf) } else { require.NoError(t, err) + expectedConf := tt.expectedConf + if expectedConf == nil { + expectedConf = map[string]any{} + + } + require.Equal(t, expectedConf, splitConf.ToStringMap()) } - assert.Equal(t, tt.expected, cfgSrcSettings) + assert.Equal(t, tt.expectedSettings, cfgSrcSettings) }) } } - -type mockCfgSrcSettings struct { - SourceSettings - Endpoint string `mapstructure:"endpoint"` - Token string `mapstructure:"token"` -} - -func (m mockCfgSrcSettings) Validate() error { - return nil -} - -var _ Source = (*mockCfgSrcSettings)(nil) - -type mockCfgSrcFactory struct { - ErrOnCreateConfigSource error -} - -var _ Factory = (*mockCfgSrcFactory)(nil) - -func (m *mockCfgSrcFactory) Type() component.Type { - return "tstcfgsrc" -} - -func (m *mockCfgSrcFactory) CreateDefaultConfig() Source { - return &mockCfgSrcSettings{ - SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), - Endpoint: "default_endpoint", - } -} - -func (m *mockCfgSrcFactory) CreateConfigSource(_ context.Context, _ CreateParams, cfg Source) (ConfigSource, error) { - if m.ErrOnCreateConfigSource != nil { - return nil, m.ErrOnCreateConfigSource - } - return &testConfigSource{ - ValueMap: map[string]valueEntry{ - cfg.ID().String(): { - Value: cfg, - }, - }, - }, nil -} diff --git a/internal/configprovider/manager.go b/internal/configsource/source.go similarity index 74% rename from internal/configprovider/manager.go rename to internal/configsource/source.go index 69ee31e441..e49f337899 100644 --- a/internal/configprovider/manager.go +++ b/internal/configsource/source.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package configprovider +package configsource import ( "context" @@ -58,11 +58,83 @@ var ddBackwardCompatible = func() bool { return true }() -// Resolve inspects the given confmap.Conf and resolves all config sources referenced -// in the configuration, returning a confmap.Conf in which all env vars and config sources on +type ConfigSource interface { + // Retrieve goes to the configuration source and retrieves the selected data which + // contains the value to be injected in the configuration and the corresponding watcher that + // will be used to monitor for updates of the retrieved value. The retrieved value is selected + // according to the selector and the params arguments. + // + // The selector is a string that is required on all invocations, the params are optional. Each + // implementation handles the generic params according to their requirements. + Retrieve(ctx context.Context, selector string, params *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) +} + +// Factory is a factory interface for configuration sources. Given it's not an accepted component and +// because of the direct Factory usage restriction from https://github.com/open-telemetry/opentelemetry-collector/commit/9631ceabb7dc4ca5cc187bab26d8319783bcc562 +// it's not a proper Collector config.Factory. +type Factory interface { + // CreateDefaultConfig creates the default configuration settings for the ConfigSource. + // This method can be called multiple times depending on the pipeline + // configuration and should not cause side-effects that prevent the creation + // of multiple instances of the ConfigSource. + // The object returned by this method needs to pass the checks implemented by + // 'configcheck.ValidateConfig'. It is recommended to have such check in the + // tests of any implementation of the Factory interface. + CreateDefaultConfig() Settings + + // CreateConfigSource creates a configuration source based on the given config. + CreateConfigSource(context.Context, Settings, *zap.Logger) (ConfigSource, error) + + // Type gets the type of the component created by this factory. + Type() component.Type +} + +// Factories maps the type of a ConfigSource to the respective factory object. +type Factories map[component.Type]Factory + +// BuildConfigSourcesFromConf inspects the given confmap.Conf and builds all config sources referenced +// in the configuration intended to be used with ResolveWithConfigSources(). +func BuildConfigSourcesFromConf(ctx context.Context, confToFurtherResolve *confmap.Conf, logger *zap.Logger, factories Factories, confmapProviders map[string]confmap.Provider) (map[string]ConfigSource, *confmap.Conf, error) { + configSourceSettings, confWithoutSettings, err := SettingsFromConf(ctx, confToFurtherResolve, factories, confmapProviders) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse settings from conf: %w", err) + } + + configSources, err := BuildConfigSources(context.Background(), configSourceSettings, logger, factories) + if err != nil { + return nil, nil, fmt.Errorf("failed to build config sources: %w", err) + } + return configSources, confWithoutSettings, nil +} + +func BuildConfigSources(ctx context.Context, configSourcesSettings map[string]Settings, logger *zap.Logger, factories Factories) (map[string]ConfigSource, error) { + cfgSources := make(map[string]ConfigSource, len(configSourcesSettings)) + for fullName, cfgSrcSettings := range configSourcesSettings { + // If we have the setting we also have the factory. + factory, ok := factories[cfgSrcSettings.ID().Type()] + if !ok { + return nil, fmt.Errorf("unknown %s config source type for %s", cfgSrcSettings.ID().Type(), fullName) + } + + cfgSrc, err := factory.CreateConfigSource(ctx, cfgSrcSettings, logger.With(zap.String("config_source", fullName))) + if err != nil { + return nil, fmt.Errorf("failed to create config source %s: %w", fullName, err) + } + + if cfgSrc == nil { + return nil, fmt.Errorf("factory for %q produced a nil extension", fullName) + } + + cfgSources[fullName] = cfgSrc + } + + return cfgSources, nil +} + +// ResolveWithConfigSources returns a confmap.Conf in which all env vars and config sources on // the given input config map are resolved to actual literal values of the env vars or config sources. // -// 1. Resolve to inject the data from config sources into a configuration; +// 1. ResolveWithConfigSources to inject the data from config sources into a configuration; // 2. Wait for an update on "watcher" func. // 3. Close the confmap.Retrieved instance; // @@ -168,61 +240,36 @@ var ddBackwardCompatible = func() bool { // results. // // For an overview about the internals of the Manager refer to the package README.md. -func Resolve(ctx context.Context, configMap *confmap.Conf, logger *zap.Logger, buildInfo component.BuildInfo, factories Factories, watcher confmap.WatcherFunc) (map[string]any, confmap.CloseFunc, error) { - configSourcesSettings, err := Load(context.Background(), configMap, factories) - if err != nil { - return nil, nil, err - } - - params := CreateParams{ - Logger: logger, - BuildInfo: buildInfo, - } - cfgSources, err := Build(context.Background(), configSourcesSettings, params, factories) - if err != nil { - return nil, nil, err - } - - return resolve(ctx, cfgSources, configMap, watcher) -} - -func resolve(ctx context.Context, configSources map[string]ConfigSource, configMap *confmap.Conf, watcher confmap.WatcherFunc) (map[string]any, confmap.CloseFunc, error) { - res := map[string]any{} - allKeys := configMap.AllKeys() +func ResolveWithConfigSources(ctx context.Context, configSources map[string]ConfigSource, confmapProviders map[string]confmap.Provider, conf *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Conf, confmap.CloseFunc, error) { + resolved := map[string]any{} var closeFuncs []confmap.CloseFunc - for _, k := range allKeys { - if strings.HasPrefix(k, configSourcesKey) { - // Remove everything under the config_sources section. The `config_sources` section - // is read when loading the config sources used in the configuration, but it is not - // part of the resulting configuration returned via *confmap.Conf. - continue - } - - value, closeFunc, err := parseConfigValue(ctx, configSources, configMap.Get(k), watcher) + for _, k := range conf.AllKeys() { + v := conf.Get(k) + value, closeFunc, err := resolveConfigValue(ctx, configSources, confmapProviders, v, watcher) if err != nil { return nil, nil, err } - res[k] = value + resolved[k] = value if closeFunc != nil { closeFuncs = append(closeFuncs, closeFunc) } } - maps.IntfaceKeysToStrings(res) - return res, mergeCloseFuncs(closeFuncs), nil + maps.IntfaceKeysToStrings(resolved) + return confmap.NewFromStringMap(resolved), MergeCloseFuncs(closeFuncs), nil } -// parseConfigValue takes the value of a "config node" and process it recursively. The processing consists +// resolveConfigValue takes the value of a "config node" and process it recursively. The processing consists // in transforming invocations of config sources and/or environment variables into literal data that can be // used directly from a `confmap.Conf` object. -func parseConfigValue(ctx context.Context, configSources map[string]ConfigSource, value any, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { - switch v := value.(type) { +func resolveConfigValue(ctx context.Context, configSources map[string]ConfigSource, confmapProviders map[string]confmap.Provider, valueToResolve any, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { + switch v := valueToResolve.(type) { case string: - // Only if the value of the node is a string it can contain an env var or config source + // Only if the valueToResolve of the node is a string it can contain an env var or config source // invocation that requires transformation. - return parseStringValue(ctx, configSources, v, watcher) + return resolveStringValue(ctx, configSources, confmapProviders, v, watcher) case []any: - // The value is of type []any when an array is used in the configuration, YAML example: + // The valueToResolve is of type []any when an array is used in the configuration, YAML example: // // array0: // - elem0 @@ -237,7 +284,7 @@ func parseConfigValue(ctx context.Context, configSources map[string]ConfigSource nslice := make([]any, 0, len(v)) var closeFuncs []confmap.CloseFunc for _, vint := range v { - value, closeFunc, err := parseConfigValue(ctx, configSources, vint, watcher) + value, closeFunc, err := resolveConfigValue(ctx, configSources, confmapProviders, vint, watcher) if err != nil { return nil, nil, err } @@ -246,15 +293,15 @@ func parseConfigValue(ctx context.Context, configSources map[string]ConfigSource } nslice = append(nslice, value) } - return nslice, mergeCloseFuncs(closeFuncs), nil + return nslice, MergeCloseFuncs(closeFuncs), nil case map[string]any: - // The value is of type map[string]any when an array in the configuration is populated with map - // elements. From the case above (for type []any) each element of "array1" is going to hit the + // The valueToResolve is of type map[string]any when an array in the configuration is populated with map + // elements. From the case above (for type []any) each element of "array1" is going to hit // the current case block. nmap := make(map[any]any, len(v)) var closeFuncs []confmap.CloseFunc for k, vint := range v { - value, closeFunc, err := parseConfigValue(ctx, configSources, vint, watcher) + value, closeFunc, err := resolveConfigValue(ctx, configSources, confmapProviders, vint, watcher) if err != nil { return nil, nil, err } @@ -263,16 +310,16 @@ func parseConfigValue(ctx context.Context, configSources map[string]ConfigSource } nmap[k] = value } - return nmap, mergeCloseFuncs(closeFuncs), nil + return nmap, MergeCloseFuncs(closeFuncs), nil default: // All other literals (int, boolean, etc) can't be further expanded so just return them as they are. return v, nil, nil } } -// parseStringValue transforms environment variables and config sources, if any are present, on +// resolveStringValue transforms environment variables and config sources, if any are present, on // the given string in the configuration into an object to be inserted into the resulting configuration. -func parseStringValue(ctx context.Context, configSources map[string]ConfigSource, s string, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { +func resolveStringValue(ctx context.Context, configSources map[string]ConfigSource, confmapProviders map[string]confmap.Provider, s string, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { var closeFuncs []confmap.CloseFunc // Code based on os.Expand function. All delimiters that are checked against are @@ -353,7 +400,7 @@ func parseStringValue(ctx context.Context, configSources map[string]ConfigSource default: // A config source, retrieve and apply results. - retrieved, closeFunc, err := retrieveConfigSourceData(ctx, configSources, cfgSrcName, expandableContent, watcher) + retrieved, closeFunc, err := retrieveConfigSourceData(ctx, configSources, confmapProviders, cfgSrcName, expandableContent, watcher) if err != nil { return nil, nil, err } @@ -385,7 +432,7 @@ func parseStringValue(ctx context.Context, configSources map[string]ConfigSource retrieved = cast.ToStringMap(mapIFace) } - return retrieved, mergeCloseFuncs(closeFuncs), nil + return retrieved, MergeCloseFuncs(closeFuncs), nil } // Either there was a prefix already or there are still characters to be processed. @@ -405,11 +452,11 @@ func parseStringValue(ctx context.Context, configSources map[string]ConfigSource if buf == nil { // No changes to original string, just return it. - return s, mergeCloseFuncs(closeFuncs), nil + return s, MergeCloseFuncs(closeFuncs), nil } // Return whatever was accumulated on the buffer plus the remaining of the original string. - return string(buf) + s[i:], mergeCloseFuncs(closeFuncs), nil + return string(buf) + s[i:], MergeCloseFuncs(closeFuncs), nil } func getBracketedExpandableContent(s string, i int) (expandableContent string, consumed int, cfgSrcName string) { @@ -445,11 +492,15 @@ func getBareExpandableContent(s string, i int) (expandableContent string, consum // retrieveConfigSourceData retrieves data from the specified config source and injects them into // the configuration. The Manager tracks sessions and watcher objects as needed. -func retrieveConfigSourceData(ctx context.Context, configSources map[string]ConfigSource, cfgSrcName, cfgSrcInvocation string, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { +func retrieveConfigSourceData(ctx context.Context, configSources map[string]ConfigSource, confmapProviders map[string]confmap.Provider, cfgSrcName, cfgSrcInvocation string, watcher confmap.WatcherFunc) (any, confmap.CloseFunc, error) { var closeFuncs []confmap.CloseFunc cfgSrc, ok := configSources[cfgSrcName] + var provider confmap.Provider + var providerFound bool if !ok { - return nil, nil, newErrUnknownConfigSource(cfgSrcName) + if provider, providerFound = confmapProviders[cfgSrcName]; !providerFound { + return nil, nil, newErrUnknownConfigSource(cfgSrcName) + } } cfgSrcName, selector, paramsConfigMap, err := parseCfgSrcInvocation(cfgSrcInvocation) @@ -457,8 +508,23 @@ func retrieveConfigSourceData(ctx context.Context, configSources map[string]Conf return nil, nil, err } + if providerFound { + if provider == nil { + return nil, nil, fmt.Errorf("resolving confmap.Provider %q with config sources failed", cfgSrcName) + } + retrieved, e := provider.Retrieve(ctx, fmt.Sprintf("%s:%s", cfgSrcName, selector), watcher) + if e != nil { + return nil, nil, fmt.Errorf("retrieve error from confmap provider %q: %w", cfgSrcName, e) + } + raw, e := retrieved.AsRaw() + if e != nil { + return nil, nil, e + } + return raw, retrieved.Close, nil + } + // Recursively expand the selector. - expandedSelector, closeFunc, err := parseStringValue(ctx, configSources, selector, watcher) + expandedSelector, closeFunc, err := resolveStringValue(ctx, configSources, confmapProviders, selector, watcher) if err != nil { return nil, nil, fmt.Errorf("failed to process selector for config source %q selector %q: %w", cfgSrcName, selector, err) } @@ -469,16 +535,16 @@ func retrieveConfigSourceData(ctx context.Context, configSources map[string]Conf closeFuncs = append(closeFuncs, closeFunc) } - // Recursively resolve/parse any config source on the parameters. + // Recursively ResolveWithConfigSources/parse any config source on the parameters. if paramsConfigMap != nil { - paramsConfigMapRet, closeFunc, errResolve := resolve(ctx, configSources, paramsConfigMap, watcher) + paramsConfigMapRet, closeFunc, errResolve := ResolveWithConfigSources(ctx, configSources, confmapProviders, paramsConfigMap, watcher) if errResolve != nil { return nil, nil, fmt.Errorf("failed to process parameters for config source %q invocation %q: %w", cfgSrcName, cfgSrcInvocation, errResolve) } if closeFunc != nil { closeFuncs = append(closeFuncs, closeFunc) } - paramsConfigMap = confmap.NewFromStringMap(paramsConfigMapRet) + paramsConfigMap = confmap.NewFromStringMap(paramsConfigMapRet.ToStringMap()) } retrieved, err := cfgSrc.Retrieve(ctx, selector, paramsConfigMap, watcher) @@ -488,12 +554,12 @@ func retrieveConfigSourceData(ctx context.Context, configSources map[string]Conf closeFuncs = append(closeFuncs, retrieved.Close) val, err := retrieved.AsRaw() - return val, mergeCloseFuncs(closeFuncs), err + return val, MergeCloseFuncs(closeFuncs), err } func newErrUnknownConfigSource(cfgSrcName string) error { return &errUnknownConfigSource{ - fmt.Errorf(`config source %q not found if this was intended to be an environment variable use "${%s}" instead"`, cfgSrcName, cfgSrcName), + fmt.Errorf("config source %q not found", cfgSrcName), } } @@ -671,7 +737,7 @@ func isAlphaNum(c uint8) bool { return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' } -func mergeCloseFuncs(closeFuncs []confmap.CloseFunc) confmap.CloseFunc { +func MergeCloseFuncs(closeFuncs []confmap.CloseFunc) confmap.CloseFunc { if len(closeFuncs) == 0 { return nil } diff --git a/internal/configprovider/manager_test.go b/internal/configsource/source_test.go similarity index 55% rename from internal/configprovider/manager_test.go rename to internal/configsource/source_test.go index 6dde76f901..0bcdc96f1f 100644 --- a/internal/configprovider/manager_test.go +++ b/internal/configsource/source_test.go @@ -12,16 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -package configprovider +package configsource import ( "context" "errors" + "fmt" "os" "path" "testing" - "github.com/knadh/koanf/maps" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" @@ -32,6 +32,15 @@ import ( var errValueUpdated = errors.New("configuration must retrieve the updated value") +func BuildConfigSourcesAndResolve(ctx context.Context, confToFurtherResolve *confmap.Conf, logger *zap.Logger, factories Factories, watcher confmap.WatcherFunc) (*confmap.Conf, confmap.CloseFunc, error) { + cfgSources, conf, err := BuildConfigSourcesFromConf(ctx, confToFurtherResolve, logger, factories, nil) + if err != nil { + return nil, nil, err + } + + return ResolveWithConfigSources(ctx, cfgSources, nil, conf, watcher) +} + func TestConfigSourceManagerNewManager(t *testing.T) { tests := []struct { factories Factories @@ -43,7 +52,7 @@ func TestConfigSourceManagerNewManager(t *testing.T) { name: "basic_config", file: "basic_config", factories: Factories{ - "tstcfgsrc": &mockCfgSrcFactory{}, + "tstcfgsrc": &MockCfgSrcFactory{}, }, }, { @@ -56,7 +65,7 @@ func TestConfigSourceManagerNewManager(t *testing.T) { name: "build_error", file: "basic_config", factories: Factories{ - "tstcfgsrc": &mockCfgSrcFactory{ + "tstcfgsrc": &MockCfgSrcFactory{ ErrOnCreateConfigSource: errors.New("forced test error"), }, }, @@ -70,7 +79,7 @@ func TestConfigSourceManagerNewManager(t *testing.T) { parser, err := confmaptest.LoadConf(filename) require.NoError(t, err) - _, _, err = Resolve(context.Background(), parser, zap.NewNop(), component.NewDefaultBuildInfo(), tt.factories, nil) + _, _, err = BuildConfigSourcesAndResolve(context.Background(), parser, zap.NewNop(), tt.factories, nil) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) } else { @@ -80,9 +89,9 @@ func TestConfigSourceManagerNewManager(t *testing.T) { } } -func TestConfigSourceManagerSimple(t *testing.T) { +func TestConfigSourceResolved(t *testing.T) { cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "test_selector": {Value: "test_value"}, }, @@ -104,36 +113,33 @@ func TestConfigSourceManagerSimple(t *testing.T) { cp := confmap.NewFromStringMap(originalCfg) - res, closeFunc, err := resolve(context.Background(), cfgSources, cp, func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, cp, func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) - assert.Equal(t, expectedCfg, maps.Unflatten(res, confmap.KeyDelimiter)) + assert.Equal(t, expectedCfg, res.ToStringMap()) assert.NoError(t, closeFunc(context.Background())) } func TestConfigSourceManagerResolveRemoveConfigSourceSection(t *testing.T) { cfg := map[string]any{ - "config_sources": map[string]any{ - "testcfgsrc": nil, - }, "another_section": map[string]any{ "int": 42, }, } cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{}, + "tstcfgsrc": &TestConfigSource{}, } - res, closeFunc, err := resolve(context.Background(), cfgSources, confmap.NewFromStringMap(cfg), func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, confmap.NewFromStringMap(cfg), func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) require.NotNil(t, res) delete(cfg, "config_sources") - assert.Equal(t, cfg, maps.Unflatten(res, confmap.KeyDelimiter)) + assert.Equal(t, cfg, res.ToStringMap()) assert.NoError(t, callClose(closeFunc)) } @@ -151,7 +157,7 @@ func TestConfigSourceManagerResolveErrors(t *testing.T) { "cfgsrc": "$tstcfgsrc:selector?{invalid}", }, configSourceMap: map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{}, + "tstcfgsrc": &TestConfigSource{}, }, }, { @@ -160,13 +166,13 @@ func TestConfigSourceManagerResolveErrors(t *testing.T) { "cfgsrc": "$tstcfgsrc:selector", }, configSourceMap: map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ErrOnRetrieve: testErr}, + "tstcfgsrc": &TestConfigSource{ErrOnRetrieve: testErr}, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - res, closeFunc, err := resolve(context.Background(), tt.configSourceMap, confmap.NewFromStringMap(tt.config), func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), tt.configSourceMap, nil, confmap.NewFromStringMap(tt.config), func(event *confmap.ChangeEvent) { panic("must not be called") }) require.Error(t, err) @@ -178,7 +184,7 @@ func TestConfigSourceManagerResolveErrors(t *testing.T) { func TestConfigSourceManagerYAMLInjection(t *testing.T) { cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "valid_yaml_str": {Value: ` bool: true @@ -202,17 +208,17 @@ map: require.NoError(t, err) expectedCfg := expectedParser.ToStringMap() - res, closeFunc, err := resolve(context.Background(), cfgSources, cp, func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, cp, func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) - assert.Equal(t, expectedCfg, maps.Unflatten(res, confmap.KeyDelimiter)) + assert.Equal(t, expectedCfg, res.ToStringMap()) assert.NoError(t, callClose(closeFunc)) } func TestConfigSourceManagerArraysAndMaps(t *testing.T) { cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "elem0": {Value: "elem0_value"}, "elem1": {Value: "elem1_value"}, @@ -230,16 +236,16 @@ func TestConfigSourceManagerArraysAndMaps(t *testing.T) { expectedParser, err := confmaptest.LoadConf(expectedFile) require.NoError(t, err) - res, closeFunc, err := resolve(context.Background(), cfgSources, cp, func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, cp, func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) - assert.Equal(t, expectedParser.ToStringMap(), maps.Unflatten(res, confmap.KeyDelimiter)) + assert.Equal(t, expectedParser.ToStringMap(), res.ToStringMap()) assert.NoError(t, callClose(closeFunc)) } func TestConfigSourceManagerParamsHandling(t *testing.T) { - tstCfgSrc := testConfigSource{ + tstCfgSrc := TestConfigSource{ ValueMap: map[string]valueEntry{ "elem0": {Value: nil}, "elem1": { @@ -263,7 +269,7 @@ func TestConfigSourceManagerParamsHandling(t *testing.T) { }, } - // Set OnRetrieve to check if the parameters were parsed as expected. + // Set OnRetrieve to check if the parameters were parsed as expectedSettings. tstCfgSrc.OnRetrieve = func(ctx context.Context, selector string, paramsConfigMap *confmap.Conf) error { var val any if paramsConfigMap != nil { @@ -281,11 +287,11 @@ func TestConfigSourceManagerParamsHandling(t *testing.T) { expectedParser, err := confmaptest.LoadConf(expectedFile) require.NoError(t, err) - res, closeFunc, err := resolve(context.Background(), map[string]ConfigSource{"tstcfgsrc": &tstCfgSrc}, cp, func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), map[string]ConfigSource{"tstcfgsrc": &tstCfgSrc}, nil, cp, func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) - assert.Equal(t, expectedParser.ToStringMap(), maps.Unflatten(res, confmap.KeyDelimiter)) + assert.Equal(t, expectedParser.ToStringMap(), res.ToStringMap()) assert.NoError(t, callClose(closeFunc)) } @@ -293,7 +299,7 @@ func TestConfigSourceManagerWatchForUpdate(t *testing.T) { watchForUpdateCh := make(chan error, 1) cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "test_selector": { Value: "test_value", @@ -311,7 +317,7 @@ func TestConfigSourceManagerWatchForUpdate(t *testing.T) { cp := confmap.NewFromStringMap(originalCfg) watchCh := make(chan *confmap.ChangeEvent) - _, closeFunc, err := resolve(context.Background(), cfgSources, cp, func(event *confmap.ChangeEvent) { + _, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, cp, func(event *confmap.ChangeEvent) { watchCh <- event }) require.NoError(t, err) @@ -326,7 +332,7 @@ func TestConfigSourceManagerWatchForUpdate(t *testing.T) { func TestConfigSourceManagerMultipleWatchForUpdate(t *testing.T) { watchForUpdateCh := make(chan error, 2) cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "test_selector": { Value: "test_value", @@ -347,7 +353,7 @@ func TestConfigSourceManagerMultipleWatchForUpdate(t *testing.T) { cp := confmap.NewFromStringMap(originalCfg) watchCh := make(chan *confmap.ChangeEvent) - _, closeFunc, err := resolve(context.Background(), cfgSources, cp, func(event *confmap.ChangeEvent) { + _, closeFunc, err := ResolveWithConfigSources(context.Background(), cfgSources, nil, cp, func(event *confmap.ChangeEvent) { watchCh <- event }) require.NoError(t, err) @@ -367,7 +373,7 @@ func TestConfigSourceManagerEnvVarHandling(t *testing.T) { assert.NoError(t, os.Unsetenv("envvar")) }() - tstCfgSrc := testConfigSource{ + tstCfgSrc := TestConfigSource{ ValueMap: map[string]valueEntry{ "int_key": {Value: 42}, }, @@ -393,25 +399,25 @@ func TestConfigSourceManagerEnvVarHandling(t *testing.T) { expectedParser, err := confmaptest.LoadConf(expectedFile) require.NoError(t, err) - res, closeFunc, err := resolve(context.Background(), map[string]ConfigSource{"tstcfgsrc": &tstCfgSrc}, cp, func(event *confmap.ChangeEvent) { + res, closeFunc, err := ResolveWithConfigSources(context.Background(), map[string]ConfigSource{"tstcfgsrc": &tstCfgSrc}, nil, cp, func(event *confmap.ChangeEvent) { panic("must not be called") }) require.NoError(t, err) - assert.Equal(t, expectedParser.ToStringMap(), res) + assert.Equal(t, expectedParser.ToStringMap(), res.ToStringMap()) assert.NoError(t, callClose(closeFunc)) } func TestManagerExpandString(t *testing.T) { ctx := context.Background() cfgSources := map[string]ConfigSource{ - "tstcfgsrc": &testConfigSource{ + "tstcfgsrc": &TestConfigSource{ ValueMap: map[string]valueEntry{ "str_key": {Value: "test_value"}, "int_key": {Value: 1}, "nil_key": {Value: nil}, }, }, - "tstcfgsrc/named": &testConfigSource{ + "tstcfgsrc/named": &TestConfigSource{ ValueMap: map[string]valueEntry{ "int_key": {Value: 42}, }, @@ -535,7 +541,7 @@ func TestManagerExpandString(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, closeFunc, err := parseStringValue(ctx, cfgSources, tt.input, func(event *confmap.ChangeEvent) { + got, closeFunc, err := resolveStringValue(ctx, cfgSources, nil, tt.input, func(event *confmap.ChangeEvent) { panic("must not be called") }) if tt.wantErr != nil { @@ -647,3 +653,310 @@ func callClose(closeFunc confmap.CloseFunc) error { } return closeFunc(context.Background()) } + +func TestConfigSourceBuild(t *testing.T) { + ctx := context.Background() + testFactories := Factories{ + "tstcfgsrc": &MockCfgSrcFactory{}, + } + + tests := []struct { + configSettings map[string]Settings + factories Factories + expectedCfgSources map[string]ConfigSource + wantErr string + name string + }{ + { + name: "unknown_config_source", + configSettings: map[string]Settings{ + "tstcfgsrc": &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewIDWithName("unknown_config_source", "tstcfgsrc")), + }, + }, + factories: testFactories, + wantErr: "unknown unknown_config_source config source type for tstcfgsrc", + }, + { + name: "creation_error", + configSettings: map[string]Settings{ + "tstcfgsrc": &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), + }, + }, + factories: Factories{ + "tstcfgsrc": &MockCfgSrcFactory{ + ErrOnCreateConfigSource: errors.New("forced test error"), + }, + }, + wantErr: "failed to create config source tstcfgsrc: forced test error", + }, + { + name: "factory_return_nil", + configSettings: map[string]Settings{ + "tstcfgsrc": &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), + }, + }, + factories: Factories{ + "tstcfgsrc": &mockNilCfgSrcFactory{}, + }, + wantErr: "factory for \"tstcfgsrc\" produced a nil extension", + }, + { + name: "base_case", + configSettings: map[string]Settings{ + "tstcfgsrc/named": &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewIDWithName("tstcfgsrc", "named")), + Endpoint: "some_endpoint", + Token: "some_token", + }, + }, + factories: testFactories, + expectedCfgSources: map[string]ConfigSource{ + "tstcfgsrc/named": &TestConfigSource{ + ValueMap: map[string]valueEntry{ + "tstcfgsrc/named": { + Value: &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewIDWithName("tstcfgsrc", "named")), + Endpoint: "some_endpoint", + Token: "some_token", + }, + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + builtCfgSources, err := BuildConfigSources(ctx, tt.configSettings, zap.NewNop(), tt.factories) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.expectedCfgSources, builtCfgSources) + }) + } +} + +type mockNilCfgSrcFactory struct{} + +func (m *mockNilCfgSrcFactory) Type() component.Type { + return "tstcfgsrc" +} + +var _ (Factory) = (*mockNilCfgSrcFactory)(nil) + +func (m *mockNilCfgSrcFactory) CreateDefaultConfig() Settings { + return &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), + Endpoint: "default_endpoint", + } +} + +func (m *mockNilCfgSrcFactory) CreateConfigSource(context.Context, Settings, *zap.Logger) (ConfigSource, error) { + return nil, nil +} + +type MockCfgSrcFactory struct { + ErrOnCreateConfigSource error +} + +type MockCfgSrcSettings struct { + SourceSettings + Endpoint string `mapstructure:"endpoint"` + Token string `mapstructure:"token"` +} + +var _ Settings = (*MockCfgSrcSettings)(nil) + +var _ Factory = (*MockCfgSrcFactory)(nil) + +func (m *MockCfgSrcFactory) Type() component.Type { + return "tstcfgsrc" +} + +func (m *MockCfgSrcFactory) CreateDefaultConfig() Settings { + return &MockCfgSrcSettings{ + SourceSettings: NewSourceSettings(component.NewID("tstcfgsrc")), + Endpoint: "default_endpoint", + } +} + +func (m *MockCfgSrcFactory) CreateConfigSource(_ context.Context, cfg Settings, _ *zap.Logger) (ConfigSource, error) { + if m.ErrOnCreateConfigSource != nil { + return nil, m.ErrOnCreateConfigSource + } + return &TestConfigSource{ + ValueMap: map[string]valueEntry{ + cfg.ID().String(): { + Value: cfg, + }, + }, + }, nil +} + +// TestConfigSource a ConfigSource to be used in tests. +type TestConfigSource struct { + ValueMap map[string]valueEntry + + ErrOnRetrieve error + ErrOnRetrieveEnd error + ErrOnClose error + + OnRetrieve func(ctx context.Context, selector string, paramsConfigMap *confmap.Conf) error +} + +type valueEntry struct { + Value any + WatchForUpdateCh chan error +} + +var _ ConfigSource = (*TestConfigSource)(nil) + +func (t *TestConfigSource) Retrieve(ctx context.Context, selector string, paramsConfigMap *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) { + if t.OnRetrieve != nil { + if err := t.OnRetrieve(ctx, selector, paramsConfigMap); err != nil { + return nil, err + } + } + + if t.ErrOnRetrieve != nil { + return nil, t.ErrOnRetrieve + } + + entry, ok := t.ValueMap[selector] + if !ok { + return nil, fmt.Errorf("no value for selector %q", selector) + } + + if entry.WatchForUpdateCh != nil { + doneCh := make(chan struct{}) + startWatch(entry.WatchForUpdateCh, doneCh, watcher) + return confmap.NewRetrieved(entry.Value, confmap.WithRetrievedClose(func(ctx context.Context) error { + close(doneCh) + return nil + })) + } + + return confmap.NewRetrieved(entry.Value) +} + +func (t *TestConfigSource) Shutdown(context.Context) error { + return t.ErrOnClose +} + +func startWatch(watchForUpdateCh chan error, doneCh chan struct{}, watcher confmap.WatcherFunc) { + go func() { + select { + case err := <-watchForUpdateCh: + watcher(&confmap.ChangeEvent{Error: err}) + return + case <-doneCh: + return + } + }() +} + +var _ confmap.Provider = (*confmapProvider)(nil) + +type confmapProvider struct { + scheme string + shouldError bool +} + +func (c confmapProvider) Scheme() string { + return c.scheme +} + +func (c confmapProvider) Retrieve(context.Context, string, confmap.WatcherFunc) (*confmap.Retrieved, error) { + if c.shouldError { + return nil, fmt.Errorf("confmap provider error") + } + return confmap.NewRetrieved("value from confmap provider") +} + +func (confmapProvider) Shutdown(ctx context.Context) error { + return nil +} + +func TestConfigSourceConfmapProviderFallbackResolved(t *testing.T) { + configSources := map[string]ConfigSource{ + "config_source": &TestConfigSource{ + ValueMap: map[string]valueEntry{ + "test_selector": {Value: "value from config source"}, + }, + }, + "conflicting_name": &TestConfigSource{ + ValueMap: map[string]valueEntry{ + "used_selector": {Value: "value from conflicting config source"}, + }, + }, + } + + confmapProviders := map[string]confmap.Provider{ + "confmap_provider": confmapProvider{scheme: "confmap_provider"}, + "conflicting_name": confmapProvider{scheme: "conflicting_name"}, + } + + originalCfg := map[string]any{ + "top0": map[string]any{ + "fromConfigSource": "${config_source:test_selector}", + "fromConflictingConfigSource": "${conflicting_name:used_selector}", + "fromConfmapProvider": "${confmap_provider:confmap_provider_selector}", + }, + } + expectedCfg := map[string]any{ + "top0": map[string]any{ + "fromConfigSource": "value from config source", + "fromConflictingConfigSource": "value from conflicting config source", + "fromConfmapProvider": "value from confmap provider", + }, + } + + cp := confmap.NewFromStringMap(originalCfg) + + res, closeFunc, err := ResolveWithConfigSources( + context.Background(), configSources, confmapProviders, cp, + func(event *confmap.ChangeEvent) { + t.Fatal("shouldn't be called") + }, + ) + require.NoError(t, err) + assert.Equal(t, expectedCfg, res.ToStringMap()) + assert.NoError(t, closeFunc(context.Background())) +} + +func TestConfigSourceConfmapProviderFallbackWithError(t *testing.T) { + configSources := map[string]ConfigSource{ + "config_source": &TestConfigSource{ + ValueMap: map[string]valueEntry{ + "test_selector": {Value: "value from config source"}, + }, + }, + } + + confmapProviders := map[string]confmap.Provider{ + "confmap_provider": confmapProvider{scheme: "confmap_provider", shouldError: true}, + } + + cfg := confmap.NewFromStringMap(map[string]any{ + "top0": map[string]any{ + "fromConfigSource": "${config_source:test_selector}", + "fromConfmapProvider": "${confmap_provider:confmap_provider_selector}", + }, + }) + + res, closeFunc, err := ResolveWithConfigSources( + context.Background(), configSources, confmapProviders, cfg, + func(event *confmap.ChangeEvent) { + t.Fatal("shouldn't be called") + }, + ) + require.EqualError(t, err, `retrieve error from confmap provider "confmap_provider": confmap provider error`) + assert.Nil(t, res) + assert.Nil(t, closeFunc) +} diff --git a/internal/configprovider/testdata/arrays_and_maps.yaml b/internal/configsource/testdata/arrays_and_maps.yaml similarity index 100% rename from internal/configprovider/testdata/arrays_and_maps.yaml rename to internal/configsource/testdata/arrays_and_maps.yaml diff --git a/internal/configprovider/testdata/arrays_and_maps_expected.yaml b/internal/configsource/testdata/arrays_and_maps_expected.yaml similarity index 100% rename from internal/configprovider/testdata/arrays_and_maps_expected.yaml rename to internal/configsource/testdata/arrays_and_maps_expected.yaml diff --git a/internal/configprovider/testdata/bad_name.yaml b/internal/configsource/testdata/bad_name.yaml similarity index 100% rename from internal/configprovider/testdata/bad_name.yaml rename to internal/configsource/testdata/bad_name.yaml diff --git a/internal/configprovider/testdata/basic_config.yaml b/internal/configsource/testdata/basic_config.yaml similarity index 100% rename from internal/configprovider/testdata/basic_config.yaml rename to internal/configsource/testdata/basic_config.yaml diff --git a/internal/configprovider/testdata/cfgsrc_load_use_cfgsrc.yaml b/internal/configsource/testdata/cfgsrc_load_use_cfgsrc.yaml similarity index 100% rename from internal/configprovider/testdata/cfgsrc_load_use_cfgsrc.yaml rename to internal/configsource/testdata/cfgsrc_load_use_cfgsrc.yaml diff --git a/internal/configprovider/testdata/duplicated_name.yaml b/internal/configsource/testdata/duplicated_name.yaml similarity index 100% rename from internal/configprovider/testdata/duplicated_name.yaml rename to internal/configsource/testdata/duplicated_name.yaml diff --git a/internal/configprovider/testdata/env_var_on_load.yaml b/internal/configsource/testdata/env_var_on_load.yaml similarity index 100% rename from internal/configprovider/testdata/env_var_on_load.yaml rename to internal/configsource/testdata/env_var_on_load.yaml diff --git a/internal/configprovider/testdata/envvar_cfgsrc_mix.yaml b/internal/configsource/testdata/envvar_cfgsrc_mix.yaml similarity index 100% rename from internal/configprovider/testdata/envvar_cfgsrc_mix.yaml rename to internal/configsource/testdata/envvar_cfgsrc_mix.yaml diff --git a/internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml b/internal/configsource/testdata/envvar_cfgsrc_mix_expected.yaml similarity index 100% rename from internal/configprovider/testdata/envvar_cfgsrc_mix_expected.yaml rename to internal/configsource/testdata/envvar_cfgsrc_mix_expected.yaml diff --git a/internal/configprovider/testdata/manager_resolve_error.yaml b/internal/configsource/testdata/manager_resolve_error.yaml similarity index 100% rename from internal/configprovider/testdata/manager_resolve_error.yaml rename to internal/configsource/testdata/manager_resolve_error.yaml diff --git a/internal/configprovider/testdata/params_handling.yaml b/internal/configsource/testdata/params_handling.yaml similarity index 100% rename from internal/configprovider/testdata/params_handling.yaml rename to internal/configsource/testdata/params_handling.yaml diff --git a/internal/configprovider/testdata/params_handling_expected.yaml b/internal/configsource/testdata/params_handling_expected.yaml similarity index 100% rename from internal/configprovider/testdata/params_handling_expected.yaml rename to internal/configsource/testdata/params_handling_expected.yaml diff --git a/internal/configprovider/testdata/unknown_field.yaml b/internal/configsource/testdata/unknown_field.yaml similarity index 100% rename from internal/configprovider/testdata/unknown_field.yaml rename to internal/configsource/testdata/unknown_field.yaml diff --git a/internal/configprovider/testdata/yaml_injection.yaml b/internal/configsource/testdata/yaml_injection.yaml similarity index 100% rename from internal/configprovider/testdata/yaml_injection.yaml rename to internal/configsource/testdata/yaml_injection.yaml diff --git a/internal/configprovider/testdata/yaml_injection_expected.yaml b/internal/configsource/testdata/yaml_injection_expected.yaml similarity index 100% rename from internal/configprovider/testdata/yaml_injection_expected.yaml rename to internal/configsource/testdata/yaml_injection_expected.yaml diff --git a/internal/configsource/vaultconfigsource/config.go b/internal/configsource/vaultconfigsource/config.go index 7984489381..3034afa3e2 100644 --- a/internal/configsource/vaultconfigsource/config.go +++ b/internal/configsource/vaultconfigsource/config.go @@ -18,12 +18,12 @@ package vaultconfigsource import ( "time" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) // Config holds the configuration for the creation of Vault config source objects. type Config struct { - configprovider.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct + configsource.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct // Authentication defines the authentication method to be used. Authentication *Authentication `mapstructure:"auth"` // Endpoint is the address of the Vault server, typically it is set via the @@ -49,7 +49,3 @@ type Authentication struct { // are the same as the vault CLI tool, see https://github.com/hashicorp/vault-plugin-auth-gcp/blob/e1f6784b379d277038ca0661606aa8d23791e392/plugin/cli.go#L120. GCPAuthentication *GCPAuthentication `mapstructure:"gcp"` } - -func (*Config) Validate() error { - return nil -} diff --git a/internal/configsource/vaultconfigsource/config_test.go b/internal/configsource/vaultconfigsource/config_test.go index 59b7993a25..3585de32ff 100644 --- a/internal/configsource/vaultconfigsource/config_test.go +++ b/internal/configsource/vaultconfigsource/config_test.go @@ -26,7 +26,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestVaultLoadConfig(t *testing.T) { @@ -34,18 +34,19 @@ func TestVaultLoadConfig(t *testing.T) { v, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - factories := map[component.Type]configprovider.Factory{ + factories := map[component.Type]configsource.Factory{ typeStr: NewFactory(), } - actualSettings, err := configprovider.Load(context.Background(), v, factories) + actualSettings, splitConf, err := configsource.SettingsFromConf(context.Background(), v, factories, nil) require.NoError(t, err) + require.NotNil(t, splitConf) devToken := "dev_token" otherToken := "other_token" - expectedSettings := map[string]configprovider.Source{ + expectedSettings := map[string]configsource.Settings{ "vault": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), Endpoint: "http://localhost:8200", Path: "secret/kv", PollInterval: 1 * time.Minute, @@ -54,7 +55,7 @@ func TestVaultLoadConfig(t *testing.T) { }, }, "vault/poll_interval": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "poll_interval")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "poll_interval")), Endpoint: "https://localhost:8200", Path: "other/path/kv", PollInterval: 10 * time.Second, @@ -65,10 +66,8 @@ func TestVaultLoadConfig(t *testing.T) { } require.Equal(t, expectedSettings, actualSettings) + require.Empty(t, splitConf.ToStringMap()) - params := configprovider.CreateParams{ - Logger: zap.NewNop(), - } - _, err = configprovider.Build(context.Background(), actualSettings, params, factories) + _, err = configsource.BuildConfigSources(context.Background(), actualSettings, zap.NewNop(), factories) require.NoError(t, err) } diff --git a/internal/configsource/vaultconfigsource/factory.go b/internal/configsource/vaultconfigsource/factory.go index 204e1ec4b2..55c17656ff 100644 --- a/internal/configsource/vaultconfigsource/factory.go +++ b/internal/configsource/vaultconfigsource/factory.go @@ -22,8 +22,9 @@ import ( "time" "go.opentelemetry.io/collector/component" + "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const ( @@ -51,15 +52,15 @@ func (v *vaultFactory) Type() component.Type { return typeStr } -func (v *vaultFactory) CreateDefaultConfig() configprovider.Source { +func (v *vaultFactory) CreateDefaultConfig() configsource.Settings { return &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), PollInterval: defaultPollInterval, } } -func (v *vaultFactory) CreateConfigSource(_ context.Context, params configprovider.CreateParams, cfg configprovider.Source) (configprovider.ConfigSource, error) { - vaultCfg := cfg.(*Config) +func (v *vaultFactory) CreateConfigSource(_ context.Context, settings configsource.Settings, logger *zap.Logger) (configsource.ConfigSource, error) { + vaultCfg := settings.(*Config) if vaultCfg.Endpoint == "" { return nil, &errMissingEndpoint{errors.New("cannot connect to vault with an empty endpoint")} @@ -81,11 +82,11 @@ func (v *vaultFactory) CreateConfigSource(_ context.Context, params configprovid return nil, &errNonPositivePollInterval{errors.New("poll_interval must to be positive")} } - return newConfigSource(params, vaultCfg) + return newConfigSource(vaultCfg, logger) } // NewFactory creates a factory for Vault ConfigSource objects. -func NewFactory() configprovider.Factory { +func NewFactory() configsource.Factory { return &vaultFactory{} } diff --git a/internal/configsource/vaultconfigsource/factory_test.go b/internal/configsource/vaultconfigsource/factory_test.go index 02294fc59b..507546896e 100644 --- a/internal/configsource/vaultconfigsource/factory_test.go +++ b/internal/configsource/vaultconfigsource/factory_test.go @@ -24,17 +24,12 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestVaultFactory_CreateConfigSource(t *testing.T) { emptyStr := "" factory := NewFactory() assert.Equal(t, component.Type("vault"), factory.Type()) - createParams := configprovider.CreateParams{ - Logger: zap.NewNop(), - } tests := []struct { config *Config wantErr error @@ -128,7 +123,7 @@ func TestVaultFactory_CreateConfigSource(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := factory.CreateConfigSource(context.Background(), createParams, tt.config) + actual, err := factory.CreateConfigSource(context.Background(), tt.config, zap.NewNop()) require.IsType(t, tt.wantErr, err) if tt.wantErr == nil { assert.NotNil(t, actual) diff --git a/internal/configsource/vaultconfigsource/source.go b/internal/configsource/vaultconfigsource/source.go index a68a378dc0..edb399c8b8 100644 --- a/internal/configsource/vaultconfigsource/source.go +++ b/internal/configsource/vaultconfigsource/source.go @@ -27,7 +27,7 @@ import ( "go.opentelemetry.io/collector/confmap" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) var errInvalidPollInterval = errors.New("poll interval must be greater than zero") @@ -51,7 +51,7 @@ type vaultConfigSource struct { pollInterval time.Duration } -func newConfigSource(params configprovider.CreateParams, cfg *Config) (configprovider.ConfigSource, error) { +func newConfigSource(cfg *Config, logger *zap.Logger) (configsource.ConfigSource, error) { // Client doesn't connect on creation and can't be closed. Keeping the same instance // for all sessions is ok. client, err := api.NewClient(&api.Config{ @@ -73,7 +73,7 @@ func newConfigSource(params configprovider.CreateParams, cfg *Config) (configpro } return &vaultConfigSource{ - logger: params.Logger, + logger: logger, client: client, path: cfg.Path, pollInterval: cfg.PollInterval, @@ -123,11 +123,6 @@ func (v *vaultConfigSource) Retrieve(_ context.Context, selector string, _ *conf return confmap.NewRetrieved(value, confmap.WithRetrievedClose(closeFunc)) } -func (v *vaultConfigSource) Shutdown(context.Context) error { - // Vault doesn't have a close for its client, close is completed. - return nil -} - // readSecret reads the secret from the vaultConfigSource path and if successful // it stores the secret on the vaultConfigSource secret field. func (v *vaultConfigSource) readSecret() error { diff --git a/internal/configsource/vaultconfigsource/source_test.go b/internal/configsource/vaultconfigsource/source_test.go index da6342adfd..cf8faa4e64 100644 --- a/internal/configsource/vaultconfigsource/source_test.go +++ b/internal/configsource/vaultconfigsource/source_test.go @@ -30,8 +30,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/confmap" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) const ( @@ -75,7 +73,7 @@ func TestVaultSessionForKV(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -97,8 +95,6 @@ func TestVaultSessionForKV(t *testing.T) { require.NoError(t, retrieved.Close(context.Background())) require.NoError(t, retrievedMetadata.Close(context.Background())) - - require.NoError(t, source.Shutdown(context.Background())) } func TestVaultPollingKVUpdate(t *testing.T) { @@ -115,7 +111,7 @@ func TestVaultPollingKVUpdate(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -147,10 +143,9 @@ func TestVaultPollingKVUpdate(t *testing.T) { // Close current source. require.NoError(t, retrievedK0.Close(context.Background())) require.NoError(t, retrievedK1.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) // Create a new source and repeat the process. - source, err = newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err = newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -164,7 +159,6 @@ func TestVaultPollingKVUpdate(t *testing.T) { require.Equal(t, "v1.1", valUpdatedK1) require.NoError(t, retrievedUpdatedK1.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) } func TestVaultRenewableSecret(t *testing.T) { @@ -186,7 +180,7 @@ func TestVaultRenewableSecret(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -213,10 +207,9 @@ func TestVaultRenewableSecret(t *testing.T) { // Close current source. require.NoError(t, retrievedUser.Close(context.Background())) require.NoError(t, retrievedPwd.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) // Create a new source and repeat the process. - source, err = newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err = newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -240,7 +233,6 @@ func TestVaultRenewableSecret(t *testing.T) { require.NoError(t, retrievedUpdatedUser.Close(context.Background())) require.NoError(t, retrievedUpdatedPwd.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) } func TestVaultV1SecretWithTTL(t *testing.T) { @@ -259,7 +251,7 @@ func TestVaultV1SecretWithTTL(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -280,10 +272,9 @@ func TestVaultV1SecretWithTTL(t *testing.T) { // Close current source. require.NoError(t, retrievedValue.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) // Create a new source and repeat the process. - source, err = newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err = newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -297,7 +288,6 @@ func TestVaultV1SecretWithTTL(t *testing.T) { require.Equal(t, "s3cr3t", retrievedVal) require.NoError(t, retrievedValue.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) } func TestVaultV1NonWatchableSecret(t *testing.T) { @@ -316,7 +306,7 @@ func TestVaultV1NonWatchableSecret(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -331,7 +321,6 @@ func TestVaultV1NonWatchableSecret(t *testing.T) { // Close current source. require.NoError(t, retrievedValue.Close(context.Background())) - require.NoError(t, source.Shutdown(context.Background())) } func TestVaultRetrieveErrors(t *testing.T) { @@ -388,7 +377,7 @@ func TestVaultRetrieveErrors(t *testing.T) { PollInterval: 2 * time.Second, } - source, err := newConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, &config) + source, err := newConfigSource(&config, zap.NewNop()) require.NoError(t, err) require.NotNil(t, source) @@ -396,7 +385,6 @@ func TestVaultRetrieveErrors(t *testing.T) { require.Error(t, err) assert.IsType(t, tt.err, err) assert.Nil(t, r) - assert.NoError(t, source.Shutdown(ctx)) }) } } diff --git a/internal/configsource/zookeeperconfigsource/config.go b/internal/configsource/zookeeperconfigsource/config.go index a0dd5b3823..8362db70be 100644 --- a/internal/configsource/zookeeperconfigsource/config.go +++ b/internal/configsource/zookeeperconfigsource/config.go @@ -18,12 +18,12 @@ package zookeeperconfigsource import ( "time" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) // Config defines zookeeperconfigsource configuration type Config struct { - configprovider.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct + configsource.SourceSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct // Endpoints is an array of Zookeeper server addresses. Thr ConfigSource will try to connect // to these endpoints to access Zookeeper clusters. Endpoints []string `mapstructure:"endpoints"` @@ -32,7 +32,3 @@ type Config struct { // to a different server and keep the same session. Timeout time.Duration `mapstructure:"timeout"` } - -func (*Config) Validate() error { - return nil -} diff --git a/internal/configsource/zookeeperconfigsource/config_test.go b/internal/configsource/zookeeperconfigsource/config_test.go index 6068302dc5..4d00103765 100644 --- a/internal/configsource/zookeeperconfigsource/config_test.go +++ b/internal/configsource/zookeeperconfigsource/config_test.go @@ -26,7 +26,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestZookeeperLoadConfig(t *testing.T) { @@ -34,31 +34,30 @@ func TestZookeeperLoadConfig(t *testing.T) { v, err := confmaptest.LoadConf(fileName) require.NoError(t, err) - factories := map[component.Type]configprovider.Factory{ + factories := map[component.Type]configsource.Factory{ typeStr: NewFactory(), } - actualSettings, err := configprovider.Load(context.Background(), v, factories) + actualSettings, splitConf, err := configsource.SettingsFromConf(context.Background(), v, factories, nil) require.NoError(t, err) + require.NotNil(t, splitConf) - expectedSettings := map[string]configprovider.Source{ + expectedSettings := map[string]configsource.Settings{ "zookeeper": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), Endpoints: []string{"http://localhost:1234"}, Timeout: time.Second * 10, }, "zookeeper/timeout": &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewIDWithName(typeStr, "timeout")), + SourceSettings: configsource.NewSourceSettings(component.NewIDWithName(typeStr, "timeout")), Endpoints: []string{"https://localhost:3010"}, Timeout: time.Second * 8, }, } require.Equal(t, expectedSettings, actualSettings) + require.Empty(t, splitConf.ToStringMap()) - params := configprovider.CreateParams{ - Logger: zap.NewNop(), - } - _, err = configprovider.Build(context.Background(), actualSettings, params, factories) + _, err = configsource.BuildConfigSources(context.Background(), actualSettings, zap.NewNop(), factories) require.NoError(t, err) } diff --git a/internal/configsource/zookeeperconfigsource/factory.go b/internal/configsource/zookeeperconfigsource/factory.go index 448fee76a2..93602cd7d6 100644 --- a/internal/configsource/zookeeperconfigsource/factory.go +++ b/internal/configsource/zookeeperconfigsource/factory.go @@ -19,8 +19,9 @@ import ( "time" "go.opentelemetry.io/collector/component" + "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) const ( @@ -40,19 +41,19 @@ func (v *zkFactory) Type() component.Type { return typeStr } -func (v *zkFactory) CreateDefaultConfig() configprovider.Source { +func (v *zkFactory) CreateDefaultConfig() configsource.Settings { return &Config{ - SourceSettings: configprovider.NewSourceSettings(component.NewID(typeStr)), + SourceSettings: configsource.NewSourceSettings(component.NewID(typeStr)), Endpoints: []string{defaultEndpoint}, Timeout: defaultTimeout, } } -func (v *zkFactory) CreateConfigSource(_ context.Context, params configprovider.CreateParams, cfg configprovider.Source) (configprovider.ConfigSource, error) { - return newConfigSource(params, cfg.(*Config)) +func (v *zkFactory) CreateConfigSource(_ context.Context, settings configsource.Settings, _ *zap.Logger) (configsource.ConfigSource, error) { + return newConfigSource(settings.(*Config)) } // NewFactory returns a new zookeekeper config source factory -func NewFactory() configprovider.Factory { +func NewFactory() configsource.Factory { return &zkFactory{} } diff --git a/internal/configsource/zookeeperconfigsource/factory_test.go b/internal/configsource/zookeeperconfigsource/factory_test.go index 2da84c2512..c6551b42d8 100644 --- a/internal/configsource/zookeeperconfigsource/factory_test.go +++ b/internal/configsource/zookeeperconfigsource/factory_test.go @@ -23,16 +23,11 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func TestZookeeperFactory_CreateConfigSource(t *testing.T) { factory := NewFactory() assert.Equal(t, component.Type("zookeeper"), factory.Type()) - createParams := configprovider.CreateParams{ - Logger: zap.NewNop(), - } tests := []struct { config *Config wantErr error @@ -52,7 +47,7 @@ func TestZookeeperFactory_CreateConfigSource(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := factory.CreateConfigSource(context.Background(), createParams, tt.config) + actual, err := factory.CreateConfigSource(context.Background(), tt.config, zap.NewNop()) require.IsType(t, tt.wantErr, err) if tt.wantErr == nil { assert.NotNil(t, actual) diff --git a/internal/configsource/zookeeperconfigsource/source.go b/internal/configsource/zookeeperconfigsource/source.go index 9ce59c5f6e..50af7c2da2 100644 --- a/internal/configsource/zookeeperconfigsource/source.go +++ b/internal/configsource/zookeeperconfigsource/source.go @@ -23,28 +23,25 @@ import ( "github.com/go-zookeeper/zk" "go.opentelemetry.io/collector/confmap" - "go.uber.org/zap" - "github.com/signalfx/splunk-otel-collector/internal/configprovider" + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) // zkConfigSource implements the configprovider.Session interface. type zkConfigSource struct { - logger *zap.Logger connect connectFunc } -func newConfigSource(params configprovider.CreateParams, cfg *Config) (configprovider.ConfigSource, error) { +func newConfigSource(cfg *Config) (configsource.ConfigSource, error) { if len(cfg.Endpoints) == 0 { return nil, &errMissingEndpoint{errors.New("cannot connect to zk without any endpoints")} } - return newZkConfigSource(params, newConnectFunc(cfg.Endpoints, cfg.Timeout)), nil + return newZkConfigSource(newConnectFunc(cfg.Endpoints, cfg.Timeout)), nil } -func newZkConfigSource(params configprovider.CreateParams, connect connectFunc) *zkConfigSource { +func newZkConfigSource(connect connectFunc) *zkConfigSource { return &zkConfigSource{ - logger: params.Logger, connect: connect, } } @@ -68,10 +65,6 @@ func (s *zkConfigSource) Retrieve(ctx context.Context, selector string, _ *confm })) } -func (s *zkConfigSource) Shutdown(context.Context) error { - return nil -} - func startWatcher(watchCh <-chan zk.Event, closeCh <-chan struct{}, watcher confmap.WatcherFunc) { go func() { select { diff --git a/internal/configsource/zookeeperconfigsource/source_test.go b/internal/configsource/zookeeperconfigsource/source_test.go index fd9072beee..6b716dcfd0 100644 --- a/internal/configsource/zookeeperconfigsource/source_test.go +++ b/internal/configsource/zookeeperconfigsource/source_test.go @@ -24,9 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/confmap" - "go.uber.org/zap" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" ) func sPtr(s string) *string { @@ -38,7 +35,7 @@ func TestSessionRetrieve(t *testing.T) { "k1": "v1", "d1/d2/k1": "v5", }) - source := newZkConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, newMockConnectFunc(conn)) + source := newZkConfigSource(newMockConnectFunc(conn)) testsCases := []struct { expect *string @@ -57,14 +54,12 @@ func TestSessionRetrieve(t *testing.T) { if c.expect != nil { assert.NoError(t, err) assert.NoError(t, retrieved.Close(context.Background())) - assert.NoError(t, source.Shutdown(context.Background())) return } assert.Error(t, err) assert.Nil(t, retrieved) }) } - assert.NoError(t, source.Shutdown(context.Background())) } func TestWatcher(t *testing.T) { @@ -83,7 +78,7 @@ func TestWatcher(t *testing.T) { t.Run(c.name, func(t *testing.T) { conn := newMockConnection(map[string]string{"k1": "v1"}) connect := newMockConnectFunc(conn) - source := newZkConfigSource(configprovider.CreateParams{Logger: zap.NewNop()}, connect) + source := newZkConfigSource(connect) assert.Nil(t, conn.watcherCh) watchChannel := make(chan *confmap.ChangeEvent, 1) @@ -118,7 +113,6 @@ func TestWatcher(t *testing.T) { assert.NoError(t, retrieved.Close(context.Background())) assert.Nil(t, conn.watcherCh) } - assert.NoError(t, source.Shutdown(context.Background())) }) } } diff --git a/internal/configsources/configsources.go b/internal/configsources/configsources.go deleted file mode 100644 index d0bc798454..0000000000 --- a/internal/configsources/configsources.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package configsources lists all config sources that can be used in the configuration. -package configsources - -import ( - "github.com/signalfx/splunk-otel-collector/internal/configprovider" - "github.com/signalfx/splunk-otel-collector/internal/configsource/envvarconfigsource" - "github.com/signalfx/splunk-otel-collector/internal/configsource/etcd2configsource" - "github.com/signalfx/splunk-otel-collector/internal/configsource/includeconfigsource" - "github.com/signalfx/splunk-otel-collector/internal/configsource/vaultconfigsource" - "github.com/signalfx/splunk-otel-collector/internal/configsource/zookeeperconfigsource" -) - -// Get returns the factories to all config sources available to the user. -func Get() []configprovider.Factory { - return []configprovider.Factory{ - envvarconfigsource.NewFactory(), - etcd2configsource.NewFactory(), - includeconfigsource.NewFactory(), - vaultconfigsource.NewFactory(), - zookeeperconfigsource.NewFactory(), - } -} diff --git a/internal/configsources/configsources_test.go b/internal/configsources/configsources_test.go deleted file mode 100644 index 4cb15fa356..0000000000 --- a/internal/configsources/configsources_test.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright Splunk, Inc. -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package configsources - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/component" - - "github.com/signalfx/splunk-otel-collector/internal/configprovider" -) - -func TestConfigSourcesGet(t *testing.T) { - tests := []struct { - configSourceType component.Type - }{ - {"env"}, - {"etcd2"}, - {"include"}, - {"vault"}, - {"zookeeper"}, - } - - defaultCfgSrcFactories := Get() - require.Equal(t, len(tests), len(defaultCfgSrcFactories)) - - cfgSrcFactoryMap := make(map[component.Type]struct{}) - for _, tt := range tests { - t.Run(string(tt.configSourceType), func(t *testing.T) { - var factory configprovider.Factory - for _, f := range defaultCfgSrcFactories { - if f.Type() == tt.configSourceType { - // Ensure no duplicated factories. - if _, ok := cfgSrcFactoryMap[tt.configSourceType]; ok { - assert.Fail(t, "duplicated config source factory") - } - cfgSrcFactoryMap[f.Type()] = struct{}{} - factory = f - break - } - } - - require.NotNil(t, factory, "missing or nil config source factory") - }) - } -} diff --git a/internal/confmapprovider/configsource/provider.go b/internal/confmapprovider/configsource/provider.go new file mode 100644 index 0000000000..da6de3dc7a --- /dev/null +++ b/internal/confmapprovider/configsource/provider.go @@ -0,0 +1,174 @@ +// Copyright Splunk, Inc. +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configsource + +import ( + "context" + "fmt" + "sync" + + "go.opentelemetry.io/collector/confmap" + "go.uber.org/zap" + + "github.com/signalfx/splunk-otel-collector/internal/configsource" + "github.com/signalfx/splunk-otel-collector/internal/configsource/envvarconfigsource" + "github.com/signalfx/splunk-otel-collector/internal/configsource/etcd2configsource" + "github.com/signalfx/splunk-otel-collector/internal/configsource/includeconfigsource" + "github.com/signalfx/splunk-otel-collector/internal/configsource/vaultconfigsource" + "github.com/signalfx/splunk-otel-collector/internal/configsource/zookeeperconfigsource" +) + +var configSourceFactories = func() configsource.Factories { + factories := make(configsource.Factories) + for _, f := range []configsource.Factory{ + envvarconfigsource.NewFactory(), + includeconfigsource.NewFactory(), + vaultconfigsource.NewFactory(), + zookeeperconfigsource.NewFactory(), + etcd2configsource.NewFactory(), + } { + if _, ok := factories[f.Type()]; ok { + panic(fmt.Sprintf("duplicate config source factory %q", f.Type())) + } + factories[f.Type()] = f + } + return factories +}() + +// Hook is a means of providing introspection to a confmap.Provider's lifecycle, +// useful for evaluating Retrieve()'ed content. +type Hook interface { + OnNew() + OnRetrieve(scheme string, retrieved map[string]any) + OnShutdown() +} + +// Provider is the entrypoint for existing confmap.Providers to be provided with +// configsource.ConfigSource retrieval functionality. Once Wrap()'ed, their +// Retrieve() method as invoked by the service's confmap.Resolver will be +// further resolved by any applicable ConfigSource directives (including +// any initial `config_sources:` settings declarations). +type Provider interface { + Wrap(provider confmap.Provider) confmap.Provider +} + +func New(logger *zap.Logger, hooks []Hook) Provider { + return &providerWrapper{ + hooks: hooks, + providers: map[string]confmap.Provider{}, + providersLock: &sync.Mutex{}, + logger: logger, + factories: configSourceFactories, + } +} + +type providerWrapper struct { + providers map[string]confmap.Provider + providersLock *sync.Mutex + logger *zap.Logger + factories configsource.Factories + hooks []Hook +} + +var _ confmap.Provider = (*wrappedProvider)(nil) + +type wrappedProvider struct { + wrapper *providerWrapper + provider confmap.Provider +} + +func (w *wrappedProvider) Retrieve(ctx context.Context, uri string, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) { + return w.wrapper.ResolveForWrapped(ctx, uri, watcher, w) +} + +func (w *wrappedProvider) Scheme() string { + return w.provider.Scheme() +} + +func (w *wrappedProvider) Shutdown(ctx context.Context) error { + for _, h := range w.wrapper.hooks { + h.OnShutdown() + } + w.wrapper.providersLock.Lock() + delete(w.wrapper.providers, w.Scheme()) + w.wrapper.providersLock.Unlock() + return w.provider.Shutdown(ctx) +} + +// Wrap registers the provided confmap.Provider in a provider store to proxy +// its methods and expose ConfigSource content resolution capabilities. +func (pw *providerWrapper) Wrap(provider confmap.Provider) confmap.Provider { + for _, h := range pw.hooks { + h.OnNew() + } + pw.providersLock.Lock() + defer pw.providersLock.Unlock() + pw.providers[provider.Scheme()] = provider + return &wrappedProvider{provider: provider, wrapper: pw} +} + +// ResolveForWrapped will retrieve from the wrappedProvider and, if possible, resolve all config source directives with their resolved values. +// If the wrappedProvider's retrieved value is only valid AsRaw() (scalar/array) then that will be returned without further evaluation. +func (pw *providerWrapper) ResolveForWrapped(ctx context.Context, uri string, onChange confmap.WatcherFunc, w *wrappedProvider) (*confmap.Retrieved, error) { + retrieved, err := w.provider.Retrieve(ctx, uri, onChange) + if err != nil { + return nil, fmt.Errorf("configsource provider failed retrieving: %w", err) + } + + conf, err := retrieved.AsConf() + if err != nil { + // raw fallback attempt, or return AsConf() error + if raw, e := retrieved.AsRaw(); e == nil { + if rawRetrieved, ee := confmap.NewRetrieved(raw); ee == nil { + // raw confmap.Retrieved values shouldn't be further processed so return here + return rawRetrieved, nil + } + } + return nil, fmt.Errorf("failed retrieving from wrappedProvider: %w", err) + } + + if conf == nil { + return nil, fmt.Errorf("retrieved confmap.Conf is unexpectedly nil") + } + + scheme, stringMap := w.provider.Scheme(), conf.ToStringMap() + for _, h := range pw.hooks { + h.OnRetrieve(scheme, stringMap) + } + + // copy providers map for downstream resolution + pw.providersLock.Lock() + providers := map[string]confmap.Provider{} + for s, p := range pw.providers { + providers[s] = p + } + pw.providersLock.Unlock() + configSources, confToResolve, err := configsource.BuildConfigSourcesFromConf(ctx, conf, pw.logger, pw.factories, providers) + if err != nil { + return nil, fmt.Errorf("failed resolving latestConf: %w", err) + } + + resolved, closeFunc, err := configsource.ResolveWithConfigSources(ctx, configSources, providers, confToResolve, onChange) + if err != nil { + return nil, fmt.Errorf("failed resolving with config sources: %w", err) + } + + return confmap.NewRetrieved( + resolved.ToStringMap(), confmap.WithRetrievedClose( + configsource.MergeCloseFuncs([]confmap.CloseFunc{closeFunc, retrieved.Close}), + ), + ) +} diff --git a/internal/configprovider/config_source_provider_test.go b/internal/confmapprovider/configsource/provider_test.go similarity index 51% rename from internal/configprovider/config_source_provider_test.go rename to internal/confmapprovider/configsource/provider_test.go index 1de7e0e1a8..872b1ce443 100644 --- a/internal/configprovider/config_source_provider_test.go +++ b/internal/confmapprovider/configsource/provider_test.go @@ -13,11 +13,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -package configprovider +package configsource import ( "context" "errors" + "fmt" "path" "testing" @@ -28,15 +29,17 @@ import ( "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/provider/fileprovider" "go.uber.org/zap" + + "github.com/signalfx/splunk-otel-collector/internal/configsource" ) func TestConfigSourceConfigMapProvider(t *testing.T) { tests := []struct { parserProvider confmap.Provider - configLocation []string + factories configsource.Factories wantErr string name string - factories []Factory + uris []string }{ { name: "success", @@ -48,45 +51,36 @@ func TestConfigSourceConfigMapProvider(t *testing.T) { }, wantErr: "mockParserProvider.Get() forced test error", }, - { - name: "duplicated_factory_type", - factories: []Factory{ - &mockCfgSrcFactory{}, - &mockCfgSrcFactory{}, - }, - wantErr: "duplicate config source factory \"tstcfgsrc\"", - }, { name: "new_manager_builder_error", - factories: []Factory{ - &mockCfgSrcFactory{ + factories: configsource.Factories{ + "tstcfgsrc": &MockCfgSrcFactory{ ErrOnCreateConfigSource: errors.New("new_manager_builder_error forced error"), }, }, parserProvider: fileprovider.New(), - configLocation: []string{"file:" + path.Join("testdata", "basic_config.yaml")}, + uris: []string{"file:" + path.Join("testdata", "basic_config.yaml")}, wantErr: "failed to create config source tstcfgsrc", }, { name: "manager_resolve_error", parserProvider: fileprovider.New(), - configLocation: []string{"file:" + path.Join("testdata", "manager_resolve_error.yaml")}, + uris: []string{"file:" + path.Join("testdata", "manager_resolve_error.yaml")}, wantErr: "config source \"tstcfgsrc\" failed to retrieve value: no value for selector \"selector\"", }, { name: "multiple_config_success", parserProvider: fileprovider.New(), - configLocation: []string{"file:" + path.Join("testdata", "arrays_and_maps_expected.yaml"), + uris: []string{"file:" + path.Join("testdata", "arrays_and_maps_expected.yaml"), "file:" + path.Join("testdata", "yaml_injection_expected.yaml")}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - factories := tt.factories - if factories == nil { - factories = []Factory{ - &mockCfgSrcFactory{}, + if tt.factories == nil { + tt.factories = configsource.Factories{ + "tstcfgsrc": &MockCfgSrcFactory{}, } } @@ -99,14 +93,17 @@ func TestConfigSourceConfigMapProvider(t *testing.T) { h.On("OnShutdown") } - pp := NewConfigSourceConfigMapProvider( - &mockParserProvider{}, - zap.NewNop(), - component.NewDefaultBuildInfo(), - []Hook{hookOne, hookTwo}, - factories..., - ) - require.NotNil(t, pp) + var provider confmap.Provider + if tt.parserProvider == nil { + provider = &mockParserProvider{} + } else { + provider = tt.parserProvider + } + p := New(zap.NewNop(), []Hook{hookOne, hookTwo}) + require.NotNil(t, p) + + p.(*providerWrapper).factories = tt.factories + pp := p.Wrap(provider) for _, h := range hooks { h.AssertCalled(t, "OnNew") @@ -114,24 +111,18 @@ func TestConfigSourceConfigMapProvider(t *testing.T) { h.AssertNotCalled(t, "OnShutdown") } - var expectedScheme string - // Do not use the config.Default() to simplify the test setup. - cspp := pp.(*configSourceConfigMapProvider) - if tt.parserProvider != nil { - cspp.wrappedProvider = tt.parserProvider - expectedScheme = tt.parserProvider.Scheme() - } + expectedScheme := provider.Scheme() - // Need to run Retrieve method no matter what, so we can't just iterate passed in config locations i := 0 for ok := true; ok; { - var configLocation string - if tt.configLocation != nil { - configLocation = tt.configLocation[i] + var uri string + if tt.uris != nil { + uri = tt.uris[i] } else { - configLocation = "" + uri = fmt.Sprintf("%s:", provider.Scheme()) } - r, err := pp.Retrieve(context.Background(), configLocation, nil) + + r, err := pp.Retrieve(context.Background(), uri, nil) if tt.wantErr == "" { require.NoError(t, err) @@ -146,7 +137,7 @@ func TestConfigSourceConfigMapProvider(t *testing.T) { break } i++ - ok = i < len(tt.configLocation) + ok = i < len(tt.uris) } for _, h := range hooks { @@ -158,7 +149,7 @@ func TestConfigSourceConfigMapProvider(t *testing.T) { h.AssertNotCalled(t, "OnShutdown") } - assert.NoError(t, cspp.Shutdown(context.Background())) + assert.NoError(t, pp.Shutdown(context.Background())) for _, h := range hooks { h.AssertCalled(t, "OnShutdown") @@ -185,7 +176,7 @@ func (mpp *mockParserProvider) Shutdown(context.Context) error { } func (mpp *mockParserProvider) Scheme() string { - return "" + return "mock" } type mockHook struct { @@ -205,3 +196,103 @@ func (m *mockHook) OnRetrieve(scheme string, _ map[string]any) { func (m *mockHook) OnShutdown() { m.Called() } + +type MockCfgSrcFactory struct { + ErrOnCreateConfigSource error +} + +type MockCfgSrcSettings struct { + configsource.SourceSettings + Endpoint string `mapstructure:"endpoint"` + Token string `mapstructure:"token"` +} + +var _ configsource.Settings = (*MockCfgSrcSettings)(nil) + +var _ configsource.Factory = (*MockCfgSrcFactory)(nil) + +func (m *MockCfgSrcFactory) Type() component.Type { + return "tstcfgsrc" +} + +func (m *MockCfgSrcFactory) CreateDefaultConfig() configsource.Settings { + return &MockCfgSrcSettings{ + SourceSettings: configsource.NewSourceSettings(component.NewID("tstcfgsrc")), + Endpoint: "default_endpoint", + } +} + +func (m *MockCfgSrcFactory) CreateConfigSource(_ context.Context, cfg configsource.Settings, _ *zap.Logger) (configsource.ConfigSource, error) { + if m.ErrOnCreateConfigSource != nil { + return nil, m.ErrOnCreateConfigSource + } + return &TestConfigSource{ + ValueMap: map[string]valueEntry{ + cfg.ID().String(): { + Value: cfg, + }, + }, + }, nil +} + +// TestConfigSource a ConfigSource to be used in tests. +type TestConfigSource struct { + ValueMap map[string]valueEntry + + ErrOnRetrieve error + ErrOnRetrieveEnd error + ErrOnClose error + + OnRetrieve func(ctx context.Context, selector string, paramsConfigMap *confmap.Conf) error +} + +type valueEntry struct { + Value any + WatchForUpdateCh chan error +} + +var _ configsource.ConfigSource = (*TestConfigSource)(nil) + +func (t *TestConfigSource) Retrieve(ctx context.Context, selector string, paramsConfigMap *confmap.Conf, watcher confmap.WatcherFunc) (*confmap.Retrieved, error) { + if t.OnRetrieve != nil { + if err := t.OnRetrieve(ctx, selector, paramsConfigMap); err != nil { + return nil, err + } + } + + if t.ErrOnRetrieve != nil { + return nil, t.ErrOnRetrieve + } + + entry, ok := t.ValueMap[selector] + if !ok { + return nil, fmt.Errorf("no value for selector %q", selector) + } + + if entry.WatchForUpdateCh != nil { + doneCh := make(chan struct{}) + startWatch(entry.WatchForUpdateCh, doneCh, watcher) + return confmap.NewRetrieved(entry.Value, confmap.WithRetrievedClose(func(ctx context.Context) error { + close(doneCh) + return nil + })) + } + + return confmap.NewRetrieved(entry.Value) +} + +func (t *TestConfigSource) Shutdown(context.Context) error { + return t.ErrOnClose +} + +func startWatch(watchForUpdateCh chan error, doneCh chan struct{}, watcher confmap.WatcherFunc) { + go func() { + select { + case err := <-watchForUpdateCh: + watcher(&confmap.ChangeEvent{Error: err}) + return + case <-doneCh: + return + } + }() +} diff --git a/internal/confmapprovider/configsource/testdata/arrays_and_maps_expected.yaml b/internal/confmapprovider/configsource/testdata/arrays_and_maps_expected.yaml new file mode 100644 index 0000000000..9abf036675 --- /dev/null +++ b/internal/confmapprovider/configsource/testdata/arrays_and_maps_expected.yaml @@ -0,0 +1,12 @@ +top0: + array0: + - elem0_value + - elem1_value + array1: + - entry: + str: elem0_value + - entry: + str: elem1_value + map0: + k0: k0_value + k1: k1_value diff --git a/internal/confmapprovider/configsource/testdata/basic_config.yaml b/internal/confmapprovider/configsource/testdata/basic_config.yaml new file mode 100644 index 0000000000..6b21c8a483 --- /dev/null +++ b/internal/confmapprovider/configsource/testdata/basic_config.yaml @@ -0,0 +1,5 @@ +config_sources: + tstcfgsrc: + endpoint: some_endpoint + token: some_token + tstcfgsrc/named: diff --git a/internal/confmapprovider/configsource/testdata/manager_resolve_error.yaml b/internal/confmapprovider/configsource/testdata/manager_resolve_error.yaml new file mode 100644 index 0000000000..9657989950 --- /dev/null +++ b/internal/confmapprovider/configsource/testdata/manager_resolve_error.yaml @@ -0,0 +1,4 @@ +config_sources: + tstcfgsrc: + +cfgsrc: $tstcfgsrc:selector?{invalid} diff --git a/internal/confmapprovider/configsource/testdata/yaml_injection_expected.yaml b/internal/confmapprovider/configsource/testdata/yaml_injection_expected.yaml new file mode 100644 index 0000000000..ea9741e40c --- /dev/null +++ b/internal/confmapprovider/configsource/testdata/yaml_injection_expected.yaml @@ -0,0 +1,8 @@ +yaml_00: + bool: true + int: 42 + source: string + map: + k0: v0 + k1: v1 +yaml_01: ":" diff --git a/tests/general/configproviders/file_test.go b/tests/general/configproviders/file_test.go new file mode 100644 index 0000000000..caa08652a5 --- /dev/null +++ b/tests/general/configproviders/file_test.go @@ -0,0 +1,43 @@ +// Copyright Splunk, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build integration + +package tests + +import ( + "path" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/signalfx/splunk-otel-collector/tests/testutils" +) + +func TestFileProvider(t *testing.T) { + testdataPath, err := filepath.Abs(path.Join(".", "testdata")) + require.NoError(t, err) + testutils.AssertAllMetricsReceived( + t, "memory.yaml", "file_config.yaml", nil, + []testutils.CollectorBuilder{ + func(collector testutils.Collector) testutils.Collector { + if cc, ok := collector.(*testutils.CollectorContainer); ok { + return cc.WithMount(testdataPath, "/testdata") + } + return collector + }, + }, + ) +} diff --git a/tests/general/configproviders/testdata/file_config.yaml b/tests/general/configproviders/testdata/file_config.yaml new file mode 100644 index 0000000000..fd50debc09 --- /dev/null +++ b/tests/general/configproviders/testdata/file_config.yaml @@ -0,0 +1,13 @@ +receivers: + hostmetrics: ${file:./testdata/file_with_config_content} +exporters: + otlp: + endpoint: ${env:OTLP_ENDPOINT} + tls: + insecure: true + +service: + pipelines: + metrics: + receivers: [hostmetrics] + exporters: [otlp] \ No newline at end of file diff --git a/tests/general/configproviders/testdata/file_with_config_content b/tests/general/configproviders/testdata/file_with_config_content new file mode 100644 index 0000000000..f6417f97cc --- /dev/null +++ b/tests/general/configproviders/testdata/file_with_config_content @@ -0,0 +1,3 @@ +collection_interval: 1s +scrapers: + memory: diff --git a/tests/general/configproviders/testdata/resource_metrics/memory.yaml b/tests/general/configproviders/testdata/resource_metrics/memory.yaml new file mode 100644 index 0000000000..d2ec28bc95 --- /dev/null +++ b/tests/general/configproviders/testdata/resource_metrics/memory.yaml @@ -0,0 +1,8 @@ +resource_metrics: + - scope_metrics: + - instrumentation_scope: + name: otelcol/hostmetricsreceiver/memory + version: + metrics: + - name: system.memory.usage + type: IntNonmonotonicCumulativeSum diff --git a/tests/general/configsources/testdata/vault_config.yaml b/tests/general/configsources/testdata/vault_config.yaml index 2d31315a98..8bfe6dc84d 100644 --- a/tests/general/configsources/testdata/vault_config.yaml +++ b/tests/general/configsources/testdata/vault_config.yaml @@ -1,6 +1,9 @@ config_sources: + env/with-name: + defaults: + INSERT_ACTION: to.be.overridden vault: - endpoint: http://${VAULT_HOSTNAME}:8200 + endpoint: http://${env:VAULT_HOSTNAME}:8200 path: secret/data/kv auth: token: token @@ -19,10 +22,10 @@ processors: attributes: - key: expands-vault-path-value value: ${vault:data.k0} - action: insert + action: ${env:INSERT_ACTION} - key: also-expands-vault-path-value value: ${vault:data.k1} - action: insert + action: ${env/with-name:INSERT_ACTION} service: pipelines: diff --git a/tests/general/envvar_expansion_test.go b/tests/general/envvar_expansion_test.go index f546e81c30..7015fa5eb6 100644 --- a/tests/general/envvar_expansion_test.go +++ b/tests/general/envvar_expansion_test.go @@ -48,7 +48,6 @@ func TestExpandedDollarSignsViaEnvConfigSource(t *testing.T) { } func TestIncompatibleExpandedDollarSignsViaEnvConfigSource(t *testing.T) { - t.Skip("Skip until issue https://github.com/signalfx/splunk-otel-collector/issues/2628 is resolved.") testutils.AssertAllMetricsReceived( t, "incompat_env_config_source_labels.yaml", "env_config_source_labels.yaml", nil, []testutils.CollectorBuilder{ diff --git a/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml b/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml index 18e8f10adf..b0abdb2883 100644 --- a/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml +++ b/tests/general/testdata/resource_metrics/incompat_env_config_source_labels.yaml @@ -10,21 +10,21 @@ resource_metrics: state: used single-dollar: an-envvar-value-suffix single-dollar-no-curly-braces: prefix-an-envvar-value - double-dollar: ${env:AN_ENVVAR}-suffix + double-dollar: an-envvar-value-suffix double-dollar-no-curly-braces: prefix-$env:AN_ENVVAR triple-dollar: $an-envvar-value-suffix triple-dollar-no-curly-braces: prefix-$an-envvar-value - quadruple-dollar: $${env:AN_ENVVAR}-suffix + quadruple-dollar: $an-envvar-value-suffix quadruple-dollar-no-curly-braces: prefix-$$env:AN_ENVVAR quintuple-dollar: $$an-envvar-value-suffix quintuple-dollar-no-curly-braces: prefix-$$an-envvar-value - sextuple-dollar: $$${env:AN_ENVVAR}-suffix + sextuple-dollar: $$an-envvar-value-suffix sextuple-dollar-no-curly-braces: prefix-$$$env:AN_ENVVAR septuple-dollar: $$$an-envvar-value-suffix septuple-dollar-no-curly-braces: prefix-$$$an-envvar-value - octuple-dollar: $$$${env:AN_ENVVAR}-suffix + octuple-dollar: $$$an-envvar-value-suffix octuple-dollar-no-curly-braces: prefix-$$$$env:AN_ENVVAR nonuple-dollar: $$$$an-envvar-value-suffix nonuple-dollar-no-curly-braces: prefix-$$$$an-envvar-value - decuple-dollar: $$$$${env:AN_ENVVAR}-suffix + decuple-dollar: $$$$an-envvar-value-suffix decuple-dollar-no-curly-braces: prefix-$$$$$env:AN_ENVVAR