From 4dec027ef70251ffbf925b82c3346dc74003ff16 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 10 Sep 2024 14:36:27 -0700 Subject: [PATCH] [builder] Remove builder defaulr confmap providers Signed-off-by: Bogdan Drutu --- .chloggen/fixbuilderproviders.yaml | 20 +++++++++++ cmd/builder/internal/builder/config.go | 39 +++------------------ cmd/builder/internal/builder/config_test.go | 9 +---- cmd/builder/internal/builder/main.go | 2 +- cmd/builder/internal/builder/main_test.go | 26 +++++++++++--- cmd/builder/test/core.builder.yaml | 7 ++++ 6 files changed, 56 insertions(+), 47 deletions(-) create mode 100644 .chloggen/fixbuilderproviders.yaml diff --git a/.chloggen/fixbuilderproviders.yaml b/.chloggen/fixbuilderproviders.yaml new file mode 100644 index 00000000000..91c6bd67868 --- /dev/null +++ b/.chloggen/fixbuilderproviders.yaml @@ -0,0 +1,20 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'breaking' + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: 'builder' + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Remove default config providers. This will require users to always specify the providers. + +# One or more tracking issues or pull requests related to the change +issues: [11126] + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index 8de7ed3d82c..2050213f208 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -39,7 +39,7 @@ type Config struct { Receivers []Module `mapstructure:"receivers"` Processors []Module `mapstructure:"processors"` Connectors []Module `mapstructure:"connectors"` - Providers *[]Module `mapstructure:"providers"` + Providers []Module `mapstructure:"providers"` Replaces []string `mapstructure:"replaces"` Excludes []string `mapstructure:"excludes"` @@ -114,17 +114,13 @@ func NewDefaultConfig() Config { // Validate checks whether the current configuration is valid func (c *Config) Validate() error { - var providersError error - if c.Providers != nil { - providersError = validateModules("provider", *c.Providers) - } return multierr.Combine( validateModules("extension", c.Extensions), validateModules("receiver", c.Receivers), validateModules("exporter", c.Exporters), validateModules("processor", c.Processors), validateModules("connector", c.Connectors), - providersError, + validateModules("provider", c.Providers), ) } @@ -207,34 +203,9 @@ func (c *Config) ParseModules() error { return err } - if c.Providers != nil { - providers, err := parseModules(*c.Providers) - if err != nil { - return err - } - c.Providers = &providers - } else { - providers, err := parseModules([]Module{ - { - GoMod: "go.opentelemetry.io/collector/confmap/provider/envprovider v" + c.Distribution.OtelColVersion, - }, - { - GoMod: "go.opentelemetry.io/collector/confmap/provider/fileprovider v" + c.Distribution.OtelColVersion, - }, - { - GoMod: "go.opentelemetry.io/collector/confmap/provider/httpprovider v" + c.Distribution.OtelColVersion, - }, - { - GoMod: "go.opentelemetry.io/collector/confmap/provider/httpsprovider v" + c.Distribution.OtelColVersion, - }, - { - GoMod: "go.opentelemetry.io/collector/confmap/provider/yamlprovider v" + c.Distribution.OtelColVersion, - }, - }) - if err != nil { - return err - } - c.Providers = &providers + c.Providers, err = parseModules(c.Providers) + if err != nil { + return err } return nil diff --git a/cmd/builder/internal/builder/config_test.go b/cmd/builder/internal/builder/config_test.go index e1fcd2fb575..5f3f42145ab 100644 --- a/cmd/builder/internal/builder/config_test.go +++ b/cmd/builder/internal/builder/config_test.go @@ -89,7 +89,7 @@ func TestMissingModule(t *testing.T) { { cfg: Config{ Logger: zap.NewNop(), - Providers: &[]Module{{ + Providers: []Module{{ Import: "invalid", }}, }, @@ -312,13 +312,6 @@ func TestConfmapFactoryVersions(t *testing.T) { } } -func TestAddsDefaultProviders(t *testing.T) { - cfg := NewDefaultConfig() - cfg.Providers = nil - assert.NoError(t, cfg.ParseModules()) - assert.Len(t, *cfg.Providers, 5) -} - func TestSkipsNilFieldValidation(t *testing.T) { cfg := NewDefaultConfig() cfg.Providers = nil diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index ba54c40e591..143188fa894 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -223,7 +223,7 @@ func (c *Config) coreModuleAndVersion() (string, string) { func (c *Config) allComponents() []Module { return slices.Concat[[]Module](c.Exporters, c.Receivers, c.Processors, - c.Extensions, c.Connectors, *c.Providers) + c.Extensions, c.Connectors, c.Providers) } func (c *Config) readGoModFile() (string, map[string]string, error) { diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 400740610cf..e9bcc85766c 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -194,7 +194,7 @@ func TestVersioning(t *testing.T) { }, }) require.NoError(t, err) - cfg.Providers = &providers + cfg.Providers = providers return cfg }, expectedErr: nil, @@ -209,7 +209,7 @@ func TestVersioning(t *testing.T) { GoMod: "go.opentelemetry.io/collector/exporter/otlpexporter v0.97.0", }, } - cfg.Providers = &[]Module{} + cfg.Providers = []Module{} cfg.Replaces = append(cfg.Replaces, replaces...) return cfg }, @@ -226,7 +226,7 @@ func TestVersioning(t *testing.T) { GoMod: "go.opentelemetry.io/collector/exporter/otlpexporter v0.97.0", }, } - cfg.Providers = &[]Module{} + cfg.Providers = []Module{} cfg.Replaces = append(cfg.Replaces, replaces...) return cfg }, @@ -310,7 +310,7 @@ func TestGenerateAndCompile(t *testing.T) { require.NoError(t, err) cfg.Distribution.OutputPath = t.TempDir() cfg.Replaces = append(cfg.Replaces, replaces...) - cfg.Providers = &[]Module{} + cfg.Providers = []Module{} return cfg }, }, @@ -443,6 +443,24 @@ func TestReplaceStatementsAreComplete(t *testing.T) { }, }) require.NoError(t, err) + cfg.Providers, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/confmap/provider/envprovider v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/confmap/provider/fileprovider v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/confmap/provider/httpprovider v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/confmap/provider/httpsprovider v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/confmap/provider/yamlprovider v1.9999.9999", + }, + }) + require.NoError(t, err) require.NoError(t, cfg.SetBackwardsCompatibility()) require.NoError(t, cfg.Validate()) diff --git a/cmd/builder/test/core.builder.yaml b/cmd/builder/test/core.builder.yaml index 458bb6a0eb4..1dbc86e3890 100644 --- a/cmd/builder/test/core.builder.yaml +++ b/cmd/builder/test/core.builder.yaml @@ -17,6 +17,13 @@ exporters: gomod: go.opentelemetry.io/collector v0.109.0 path: ${WORKSPACE_DIR} +providers: + - gomod: go.opentelemetry.io/collector/confmap/provider/envprovider v1.15.0 + - gomod: go.opentelemetry.io/collector/confmap/provider/fileprovider v1.15.0 + - gomod: go.opentelemetry.io/collector/confmap/provider/httpprovider v0.109.0 + - gomod: go.opentelemetry.io/collector/confmap/provider/httpsprovider v0.109.0 + - gomod: go.opentelemetry.io/collector/confmap/provider/yamlprovider v0.109.0 + replaces: - go.opentelemetry.io/collector => ${WORKSPACE_DIR} - go.opentelemetry.io/collector/client => ${WORKSPACE_DIR}/client