Skip to content

Commit

Permalink
[builder] Remove builder defaulr confmap providers
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed Oct 9, 2024
1 parent 151f837 commit 98b2dc1
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 45 deletions.
20 changes: 20 additions & 0 deletions .chloggen/fixbuilderproviders.yaml
Original file line number Diff line number Diff line change
@@ -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]
61 changes: 25 additions & 36 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down Expand Up @@ -79,6 +79,13 @@ type Module struct {
Path string `mapstructure:"path"` // an optional path to the local version of this module
}

func (mod *Module) Validate() error {
if mod.GoMod == "" {
return ErrMissingGoMod
}
return nil
}

type retry struct {
numRetries int
wait time.Duration
Expand Down Expand Up @@ -114,17 +121,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,
validateProviders(c.Providers),
)
}

Expand Down Expand Up @@ -207,43 +210,29 @@ 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
}

func validateProviders(providers []Module) error {
if err := validateModules("provider", providers); err != nil {
return err
}
if len(providers) == 0 {
return errors.New("at least one provider is required")
}
return nil
}

func validateModules(name string, mods []Module) error {
var errs error
for i, mod := range mods {
if mod.GoMod == "" {
return fmt.Errorf("%s module at index %v: %w", name, i, ErrMissingGoMod)
if err := mod.Validate(); err != nil {
errs = multierr.Append(errs, fmt.Errorf("%s module at index %v: %w", name, i, err))
}
}
return nil
Expand Down
7 changes: 3 additions & 4 deletions cmd/builder/internal/builder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestMissingModule(t *testing.T) {
{
cfg: Config{
Logger: zap.NewNop(),
Providers: &[]Module{{
Providers: []Module{{
Import: "invalid",
}},
},
Expand Down Expand Up @@ -324,11 +324,10 @@ func TestConfmapFactoryVersions(t *testing.T) {
}
}

func TestAddsDefaultProviders(t *testing.T) {
func TestNoProviders(t *testing.T) {
cfg := NewDefaultConfig()
cfg.Providers = nil
require.NoError(t, cfg.ParseModules())
assert.Len(t, *cfg.Providers, 5)
require.EqualError(t, cfg.Validate(), "at least one provider is required")
}

func TestSkipsNilFieldValidation(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/builder/internal/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
26 changes: 22 additions & 4 deletions cmd/builder/internal/builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestVersioning(t *testing.T) {
},
})
require.NoError(t, err)
cfg.Providers = &providers
cfg.Providers = providers
return cfg
},
expectedErr: nil,
Expand All @@ -210,7 +210,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
},
Expand All @@ -227,7 +227,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
},
Expand Down Expand Up @@ -323,7 +323,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
},
},
Expand Down Expand Up @@ -456,6 +456,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())
Expand Down
7 changes: 7 additions & 0 deletions cmd/builder/test/core.builder.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ receivers:
exporters:
- gomod: go.opentelemetry.io/collector/exporter/debugexporter v0.111.0

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
Expand Down

0 comments on commit 98b2dc1

Please sign in to comment.