From 2f42fa13e018bac693d8b1bb1a23073f8b9d89c7 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 25 Oct 2022 13:20:28 -0700 Subject: [PATCH] Avoid using deprecated config.Config in servicetest (#6393) Signed-off-by: Bogdan Signed-off-by: Bogdan --- service/collector.go | 4 +++ service/collector_test.go | 17 ++++++++++ service/config_provider.go | 4 --- service/config_provider_test.go | 15 --------- service/internal/pipelines/pipelines_test.go | 26 +++++++++++---- service/service_test.go | 6 ++-- service/servicetest/configprovider.go | 34 ++++++++++++++++---- 7 files changed, 70 insertions(+), 36 deletions(-) diff --git a/service/collector.go b/service/collector.go index 5b06bf65aef..1976e70a4b6 100644 --- a/service/collector.go +++ b/service/collector.go @@ -132,6 +132,10 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error { return fmt.Errorf("failed to get config: %w", err) } + if err = cfg.Validate(); err != nil { + return fmt.Errorf("invalid configuration: %w", err) + } + col.service, err = newService(&settings{ BuildInfo: col.set.BuildInfo, Factories: col.set.Factories, diff --git a/service/collector_test.go b/service/collector_test.go index 6a0bafa528d..da661b9682a 100644 --- a/service/collector_test.go +++ b/service/collector_test.go @@ -236,6 +236,23 @@ func TestCollectorFailedShutdown(t *testing.T) { assert.Equal(t, Closed, col.GetState()) } +func TestCollectorStartInvalidConfig(t *testing.T) { + factories, err := componenttest.NopFactories() + require.NoError(t, err) + + cfgProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")})) + require.NoError(t, err) + + col, err := New(CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: factories, + ConfigProvider: cfgProvider, + telemetry: newColTelemetry(featuregate.NewRegistry()), + }) + require.NoError(t, err) + assert.Error(t, col.Run(context.Background())) +} + // mapConverter applies extraMap of config settings. Useful for overriding the config // for testing purposes. Keys must use "::" delimiter between levels. type mapConverter struct { diff --git a/service/config_provider.go b/service/config_provider.go index 1b2115caadb..3516f0cfdb8 100644 --- a/service/config_provider.go +++ b/service/config_provider.go @@ -110,10 +110,6 @@ func (cm *configProvider) Get(ctx context.Context, factories component.Factories return nil, fmt.Errorf("cannot unmarshal the configuration: %w", err) } - if err = cfg.Validate(); err != nil { - return nil, fmt.Errorf("invalid configuration: %w", err) - } - return cfg, nil } diff --git a/service/config_provider_test.go b/service/config_provider_test.go index a6b7189975b..1adce1cc358 100644 --- a/service/config_provider_test.go +++ b/service/config_provider_test.go @@ -33,21 +33,6 @@ import ( "go.opentelemetry.io/collector/service/telemetry" ) -func TestConfigProviderValidationError(t *testing.T) { - factories, errF := componenttest.NopFactories() - require.NoError(t, errF) - - set := newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")}) - - cfgW, err := NewConfigProvider(set) - assert.NoError(t, err) - - _, err = cfgW.Get(context.Background(), factories) - assert.Error(t, err) - - assert.NoError(t, cfgW.Shutdown(context.Background())) -} - var configNop = &Config{ Receivers: map[config.ComponentID]config.Receiver{config.NewComponentID("nop"): componenttest.NewNopReceiverFactory().CreateDefaultConfig()}, Processors: map[config.ComponentID]config.Processor{config.NewComponentID("nop"): componenttest.NewNopProcessorFactory().CreateDefaultConfig()}, diff --git a/service/internal/pipelines/pipelines_test.go b/service/internal/pipelines/pipelines_test.go index 120840b132e..0aec61fac35 100644 --- a/service/internal/pipelines/pipelines_test.go +++ b/service/internal/pipelines/pipelines_test.go @@ -26,11 +26,12 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config" + "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/internal/testdata" + "go.opentelemetry.io/collector/service/internal/configunmarshaler" "go.opentelemetry.io/collector/service/internal/testcomponents" - "go.opentelemetry.io/collector/service/servicetest" ) func TestBuild(t *testing.T) { @@ -87,8 +88,7 @@ func TestBuild(t *testing.T) { factories, err := testcomponents.ExampleComponents() assert.NoError(t, err) - cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata", test.name), factories) - require.NoError(t, err) + cfg := loadConfigAndValidate(t, filepath.Join("testdata", test.name), factories) // Build the pipeline pipelines, err := Build(context.Background(), toSettings(factories, cfg)) @@ -249,15 +249,14 @@ func TestBuildErrors(t *testing.T) { } // Need the unknown factories to do unmarshalling. - cfg, err := servicetest.LoadConfig(filepath.Join("testdata", test.configFile), factories) - require.NoError(t, err) + cfg := loadConfig(t, filepath.Join("testdata", test.configFile), factories) // Remove the unknown factories, so they are NOT available during building. delete(factories.Exporters, "unknown") delete(factories.Processors, "unknown") delete(factories.Receivers, "unknown") - _, err = Build(context.Background(), toSettings(factories, cfg)) + _, err := Build(context.Background(), toSettings(factories, cfg)) assert.Error(t, err) }) } @@ -464,3 +463,18 @@ func (e errComponent) Start(context.Context, component.Host) error { func (e errComponent) Shutdown(context.Context) error { return errors.New("my error") } + +func loadConfig(t *testing.T, fileName string, factories component.Factories) *config.Config { + // Read yaml config from file + conf, err := confmaptest.LoadConf(fileName) + require.NoError(t, err) + cfg, err := configunmarshaler.Unmarshal(conf, factories) + require.NoError(t, err) + return cfg +} + +func loadConfigAndValidate(t *testing.T, fileName string, factories component.Factories) *config.Config { + cfg := loadConfig(t, fileName, factories) + require.NoError(t, cfg.Validate()) + return cfg +} diff --git a/service/service_test.go b/service/service_test.go index 732111dd4a7..a7b579018e8 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -25,9 +25,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config" - "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/featuregate" - "go.opentelemetry.io/collector/service/internal/configunmarshaler" ) func TestService_GetFactory(t *testing.T) { @@ -99,9 +97,9 @@ func TestServiceTelemetryCleanupOnError(t *testing.T) { require.NoError(t, err) // Read invalid yaml config from file - invalidConf, err := confmaptest.LoadConf(filepath.Join("testdata", "otelcol-invalid.yaml")) + invalidProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-invalid.yaml")})) require.NoError(t, err) - invalidCfg, err := configunmarshaler.Unmarshal(invalidConf, factories) + invalidCfg, err := invalidProvider.Get(context.Background(), factories) require.NoError(t, err) // Read valid yaml config from file diff --git a/service/servicetest/configprovider.go b/service/servicetest/configprovider.go index 1b8250ef58a..227458ea93b 100644 --- a/service/servicetest/configprovider.go +++ b/service/servicetest/configprovider.go @@ -15,27 +15,47 @@ package servicetest // import "go.opentelemetry.io/collector/service/servicetest" import ( + "context" + "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config" - "go.opentelemetry.io/collector/confmap/confmaptest" - "go.opentelemetry.io/collector/service/internal/configunmarshaler" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/confmap/converter/expandconverter" + "go.opentelemetry.io/collector/confmap/provider/envprovider" + "go.opentelemetry.io/collector/confmap/provider/fileprovider" + "go.opentelemetry.io/collector/confmap/provider/httpprovider" + "go.opentelemetry.io/collector/confmap/provider/yamlprovider" + "go.opentelemetry.io/collector/service" ) // LoadConfig loads a config.Config from file, and does NOT validate the configuration. -func LoadConfig(fileName string, factories component.Factories) (*config.Config, error) { +func LoadConfig(fileName string, factories component.Factories) (*service.Config, error) { // Read yaml config from file - conf, err := confmaptest.LoadConf(fileName) + provider, err := service.NewConfigProvider(service.ConfigProviderSettings{ + ResolverSettings: confmap.ResolverSettings{ + URIs: []string{fileName}, + Providers: makeMapProvidersMap(fileprovider.New(), envprovider.New(), yamlprovider.New(), httpprovider.New()), + Converters: []confmap.Converter{expandconverter.New()}, + }, + }) if err != nil { return nil, err } - return configunmarshaler.Unmarshal(conf, factories) + return provider.Get(context.Background(), factories) } // LoadConfigAndValidate loads a config from the file, and validates the configuration. -func LoadConfigAndValidate(fileName string, factories component.Factories) (*config.Config, error) { +func LoadConfigAndValidate(fileName string, factories component.Factories) (*service.Config, error) { cfg, err := LoadConfig(fileName, factories) if err != nil { return nil, err } return cfg, cfg.Validate() } + +func makeMapProvidersMap(providers ...confmap.Provider) map[string]confmap.Provider { + ret := make(map[string]confmap.Provider, len(providers)) + for _, provider := range providers { + ret[provider.Scheme()] = provider + } + return ret +}