Skip to content

Commit

Permalink
Remove ConfigValidate and add Validate on the Config struct
Browse files Browse the repository at this point in the history 
Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
@bogdandrutu
bogdandrutu committed Mar 10, 2021
1 parent 2c017a0 commit 3d7e63d
Show file tree
Hide file tree
Showing 33 changed files with 324 additions and 365 deletions.
175 changes: 0 additions & 175 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/spf13/cast"
"github.com/spf13/viper"
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configmodels"
Expand All @@ -42,15 +41,6 @@ const (
errInvalidTypeAndNameKey
errUnknownType
errDuplicateName
errMissingPipelines
errPipelineMustHaveReceiver
errPipelineMustHaveExporter
errExtensionNotExists
errPipelineReceiverNotExists
errPipelineProcessorNotExists
errPipelineExporterNotExists
errMissingReceivers
errMissingExporters
errUnmarshalTopLevelStructureError
)

Expand Down Expand Up @@ -478,171 +468,6 @@ func loadPipelines(pipelinesConfig map[string]pipelineSettings) (configmodels.Pi
return pipelines, nil
}

// ValidateConfig validates config.
func ValidateConfig(cfg *configmodels.Config, _ *zap.Logger) error {
// This function performs basic validation of configuration. There may be more subtle
// invalid cases that we currently don't check for but which we may want to add in
// the future (e.g. disallowing receiving and exporting on the same endpoint).

if err := validateReceivers(cfg); err != nil {
return err
}

if err := validateExporters(cfg); err != nil {
return err
}

if err := validateService(cfg); err != nil {
return err
}

return nil
}

func validateService(cfg *configmodels.Config) error {
if err := validatePipelines(cfg); err != nil {
return err
}

return validateServiceExtensions(cfg)
}

func validateServiceExtensions(cfg *configmodels.Config) error {
if len(cfg.Service.Extensions) == 0 {
return nil
}

// Validate extensions.
for _, ref := range cfg.Service.Extensions {
// Check that the name referenced in the Service extensions exists in the top-level extensions
if cfg.Extensions[ref] == nil {
return &configError{
code: errExtensionNotExists,
msg: fmt.Sprintf("Service references extension %q which does not exist", ref),
}
}
}

return nil
}

func validatePipelines(cfg *configmodels.Config) error {
// Must have at least one pipeline.
if len(cfg.Service.Pipelines) == 0 {
return &configError{code: errMissingPipelines, msg: "must have at least one pipeline"}
}

// Validate pipelines.
for _, pipeline := range cfg.Service.Pipelines {
if err := validatePipeline(cfg, pipeline); err != nil {
return err
}
}
return nil
}

func validatePipeline(cfg *configmodels.Config, pipeline *configmodels.Pipeline) error {
if err := validatePipelineReceivers(cfg, pipeline); err != nil {
return err
}

if err := validatePipelineExporters(cfg, pipeline); err != nil {
return err
}

if err := validatePipelineProcessors(cfg, pipeline); err != nil {
return err
}

return nil
}

func validatePipelineReceivers(cfg *configmodels.Config, pipeline *configmodels.Pipeline) error {
if len(pipeline.Receivers) == 0 {
return &configError{
code: errPipelineMustHaveReceiver,
msg: fmt.Sprintf("pipeline %q must have at least one receiver", pipeline.Name),
}
}

// Validate pipeline receiver name references.
for _, ref := range pipeline.Receivers {
// Check that the name referenced in the pipeline's receivers exists in the top-level receivers
if cfg.Receivers[ref] == nil {
return &configError{
code: errPipelineReceiverNotExists,
msg: fmt.Sprintf("pipeline %q references receiver %q which does not exist", pipeline.Name, ref),
}
}
}

return nil
}

func validatePipelineExporters(cfg *configmodels.Config, pipeline *configmodels.Pipeline) error {
if len(pipeline.Exporters) == 0 {
return &configError{
code: errPipelineMustHaveExporter,
msg: fmt.Sprintf("pipeline %q must have at least one exporter", pipeline.Name),
}
}

// Validate pipeline exporter name references.
for _, ref := range pipeline.Exporters {
// Check that the name referenced in the pipeline's Exporters exists in the top-level Exporters
if cfg.Exporters[ref] == nil {
return &configError{
code: errPipelineExporterNotExists,
msg: fmt.Sprintf("pipeline %q references exporter %q which does not exist", pipeline.Name, ref),
}
}
}

return nil
}

func validatePipelineProcessors(cfg *configmodels.Config, pipeline *configmodels.Pipeline) error {
if len(pipeline.Processors) == 0 {
return nil
}

// Validate pipeline processor name references
for _, ref := range pipeline.Processors {
// Check that the name referenced in the pipeline's processors exists in the top-level processors.
if cfg.Processors[ref] == nil {
return &configError{
code: errPipelineProcessorNotExists,
msg: fmt.Sprintf("pipeline %q references processor %s which does not exist", pipeline.Name, ref),
}
}
}

return nil
}

func validateReceivers(cfg *configmodels.Config) error {
// Currently there is no default receiver enabled. The configuration must specify at least one enabled receiver to
// be valid.
if len(cfg.Receivers) == 0 {
return &configError{
code: errMissingReceivers,
msg: "no enabled receivers specified in config",
}
}
return nil
}

func validateExporters(cfg *configmodels.Config) error {
// There must be at least one enabled exporter to be considered a valid configuration.
if len(cfg.Exporters) == 0 {
return &configError{
code: errMissingExporters,
msg: "no enabled exporters specified in config",
}
}
return nil
}

// expandEnvConfig creates a new viper config with expanded values for all the values (simple, list or map value).
// It does not expand the keys.
func expandEnvConfig(v *viper.Viper) {
Expand Down
79 changes: 30 additions & 49 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
Expand Down Expand Up @@ -414,47 +413,40 @@ func TestDecodeConfig_Invalid(t *testing.T) {
expected configErrorCode // expected error (if nil any error is acceptable)
expectedMessage string // string that the error must contain
}{
{name: "empty-config"},
{name: "missing-all-sections"},
{name: "missing-exporters", expected: errMissingExporters},
{name: "missing-receivers", expected: errMissingReceivers},
{name: "missing-processors"},
{name: "invalid-extension-name", expected: errExtensionNotExists},
{name: "invalid-receiver-name"},
{name: "invalid-receiver-reference", expected: errPipelineReceiverNotExists},
{name: "missing-extension-type", expected: errInvalidTypeAndNameKey},
{name: "missing-receiver-type", expected: errInvalidTypeAndNameKey},
{name: "missing-exporter-name-after-slash", expected: errInvalidTypeAndNameKey},
{name: "missing-processor-type", expected: errInvalidTypeAndNameKey},
{name: "missing-pipelines", expected: errMissingPipelines},
{name: "pipeline-must-have-exporter-logs", expected: errPipelineMustHaveExporter},
{name: "pipeline-must-have-exporter-metrics", expected: errPipelineMustHaveExporter},
{name: "pipeline-must-have-exporter-traces", expected: errPipelineMustHaveExporter},
{name: "pipeline-must-have-receiver-logs", expected: errPipelineMustHaveReceiver},
{name: "pipeline-must-have-receiver-metrics", expected: errPipelineMustHaveReceiver},
{name: "pipeline-must-have-receiver-traces", expected: errPipelineMustHaveReceiver},
{name: "pipeline-exporter-not-exists", expected: errPipelineExporterNotExists},
{name: "pipeline-processor-not-exists", expected: errPipelineProcessorNotExists},
{name: "invalid-extension-type", expected: errInvalidTypeAndNameKey},
{name: "invalid-receiver-type", expected: errInvalidTypeAndNameKey},
{name: "invalid-exporter-type", expected: errInvalidTypeAndNameKey},
{name: "invalid-processor-type", expected: errInvalidTypeAndNameKey},
{name: "invalid-pipeline-type", expected: errInvalidTypeAndNameKey},

{name: "invalid-extension-name-after-slash", expected: errInvalidTypeAndNameKey},
{name: "invalid-receiver-name-after-slash", expected: errInvalidTypeAndNameKey},
{name: "invalid-exporter-name-after-slash", expected: errInvalidTypeAndNameKey},
{name: "invalid-processor-name-after-slash", expected: errInvalidTypeAndNameKey},
{name: "invalid-pipeline-name-after-slash", expected: errInvalidTypeAndNameKey},

{name: "unknown-extension-type", expected: errUnknownType, expectedMessage: "extensions"},
{name: "unknown-receiver-type", expected: errUnknownType, expectedMessage: "receivers"},
{name: "unknown-exporter-type", expected: errUnknownType, expectedMessage: "exporters"},
{name: "unknown-processor-type", expected: errUnknownType, expectedMessage: "processors"},
{name: "invalid-service-extensions-value", expected: errUnmarshalTopLevelStructureError, expectedMessage: "service"},
{name: "invalid-sequence-value", expected: errUnmarshalTopLevelStructureError, expectedMessage: "pipelines"},
{name: "invalid-pipeline-type", expected: errUnknownType, expectedMessage: "pipelines"},
{name: "invalid-pipeline-type-and-name", expected: errInvalidTypeAndNameKey},
{name: "unknown-pipeline-type", expected: errUnknownType, expectedMessage: "pipelines"},

{name: "duplicate-extension", expected: errDuplicateName, expectedMessage: "extensions"},
{name: "duplicate-receiver", expected: errDuplicateName, expectedMessage: "receivers"},
{name: "duplicate-exporter", expected: errDuplicateName, expectedMessage: "exporters"},
{name: "duplicate-processor", expected: errDuplicateName, expectedMessage: "processors"},
{name: "duplicate-pipeline", expected: errDuplicateName, expectedMessage: "pipelines"},

{name: "invalid-top-level-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "top level"},
{name: "invalid-extension-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "extensions"},
{name: "invalid-service-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "service"},
{name: "invalid-receiver-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "receivers"},
{name: "invalid-processor-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "processors"},
{name: "invalid-exporter-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "exporters"},
{name: "invalid-service-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "service"},
{name: "invalid-service-extensions-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "service"},
{name: "invalid-pipeline-section", expected: errUnmarshalTopLevelStructureError, expectedMessage: "pipelines"},
{name: "invalid-sequence-value", expected: errUnmarshalTopLevelStructureError, expectedMessage: "pipelines"},

{name: "invalid-extension-sub-config", expected: errUnmarshalTopLevelStructureError},
{name: "invalid-exporter-sub-config", expected: errUnmarshalTopLevelStructureError},
{name: "invalid-processor-sub-config", expected: errUnmarshalTopLevelStructureError},
Expand All @@ -468,9 +460,8 @@ func TestDecodeConfig_Invalid(t *testing.T) {
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
_, err := loadConfigFile(t, path.Join(".", "testdata", test.name+".yaml"), factories)
if err == nil {
t.Error("expected error but succeeded")
} else if test.expected != 0 {
require.Error(t, err)
if test.expected != 0 {
cfgErr, ok := err.(*configError)
if !ok {
t.Errorf("expected config error code %v but got a different error '%v'", test.expected, err)
Expand All @@ -486,25 +477,19 @@ func TestDecodeConfig_Invalid(t *testing.T) {
}
}

func TestLoadEmptyConfig(t *testing.T) {
func TestLoadEmpty(t *testing.T) {
factories, err := componenttest.ExampleComponents()
assert.NoError(t, err)

// Open the file for reading.
file, err := os.Open(path.Join(".", "testdata", "empty-config.yaml"))
require.NoError(t, err)

defer func() {
require.NoError(t, file.Close())
}()
_, err = loadConfigFile(t, path.Join(".", "testdata", "empty-config.yaml"), factories)
assert.NoError(t, err)
}

// Read yaml config from file
v := NewViper()
v.SetConfigType("yaml")
require.NoError(t, v.ReadConfig(file))
func TestLoadEmptyAllSections(t *testing.T) {
factories, err := componenttest.ExampleComponents()
assert.NoError(t, err)

// Load the config from viper using the given factories.
_, err = Load(v, factories)
_, err = loadConfigFile(t, path.Join(".", "testdata", "empty-all-sections.yaml"), factories)
assert.NoError(t, err)
}

Expand All @@ -515,11 +500,7 @@ func loadConfigFile(t *testing.T, fileName string, factories component.Factories
require.NoErrorf(t, v.ReadInConfig(), "unable to read the file %v", fileName)

// Load the config from viper using the given factories.
cfg, err := Load(v, factories)
if err != nil {
return nil, err
}
return cfg, ValidateConfig(cfg, zap.NewNop())
return Load(v, factories)
}

type nestedConfig struct {
Expand Down
Loading

0 comments on commit 3d7e63d

Please sign in to comment.