Use strict mode to read config (#375)

* Use strict mode to read config

Reading the config using exact matches causes errors for unknown sections reducing the changes of accidental errors on the configuration files.

* Add comments and fix testbed config files

Author: Paulo Janotti

cmd/otelcol/main.go

@@ -34,6 +34,6 @@ func main() {
 	handleErr(err)
 
 	svc := service.New(factories)
-	err = svc.StartUnified()
+	err = svc.Start()
 	handleErr(err)
 }

config/config.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/viper"
 	"go.uber.org/zap"
 
@@ -58,9 +59,15 @@ const (
 	errPipelineReceiverNotExists
 	errPipelineProcessorNotExists
 	errPipelineExporterNotExists
-	errUnmarshalError
 	errMissingReceivers
 	errMissingExporters
+	errUnmarshalErrorOnTopLevelSection
+	errUnmarshalErrorOnExtension
+	errUnmarshalErrorOnService
+	errUnmarshalErrorOnReceiver
+	errUnmarshalErrorOnProcessor
+	errUnmarshalErrorOnExporter
+	errUnmarshalErrorOnPipeline
 )
 
 type configError struct {
@@ -123,6 +130,23 @@ func Load(
 
 	// Load the config.
 
+	// Struct to validate top level sections.
+	var topLevelSections struct {
+		Extensions map[string]interface{} `mapstructure:"extensions"`
+		Service    map[string]interface{} `mapstructure:"service"`
+		Receivers  map[string]interface{} `mapstructure:"receivers"`
+		Processors map[string]interface{} `mapstructure:"processors"`
+		Exporters  map[string]interface{} `mapstructure:"exporters"`
+		Pipelines  map[string]interface{} `mapstructure:"pipelines"`
+	}
+
+	if err := v.UnmarshalExact(&topLevelSections); err != nil {
+		return nil, &configError{
+			code: errUnmarshalErrorOnTopLevelSection,
+			msg:  fmt.Sprintf("error reading top level sections: %s", err.Error()),
+		}
+	}
+
 	// Start with extensions and service.
 
 	extensions, err := loadExtensions(v, factories.Extensions)
@@ -250,9 +274,9 @@ func loadExtensions(v *viper.Viper, factories map[string]extension.Factory) (con
 
 		// Now that the default config struct is created we can Unmarshal into it
 		// and it will apply user-defined config on top of the default.
-		if err := sv.Unmarshal(extensionCfg); err != nil {
+		if err := sv.UnmarshalExact(extensionCfg); err != nil {
 			return nil, &configError{
-				code: errUnmarshalError,
+				code: errUnmarshalErrorOnExtension,
 				msg:  fmt.Sprintf("error reading settings for extension type %q: %v", typeStr, err),
 			}
 		}
@@ -272,9 +296,9 @@ func loadExtensions(v *viper.Viper, factories map[string]extension.Factory) (con
 
 func loadService(v *viper.Viper) (configmodels.Service, error) {
 	var service configmodels.Service
-	if err := v.UnmarshalKey(serviceKeyName, &service); err != nil {
+	if err := v.UnmarshalKey(serviceKeyName, &service, errorOnUnused); err != nil {
 		return service, &configError{
-			code: errUnmarshalError,
+			code: errUnmarshalErrorOnService,
 			msg:  fmt.Sprintf("error reading settings for %q: %v", serviceKeyName, err),
 		}
 	}
@@ -336,16 +360,12 @@ func loadReceivers(v *viper.Viper, factories map[string]receiver.Factory) (confi
 			// This configuration requires a custom unmarshaler, use it.
 			err = customUnmarshaler(subViper, key, receiverCfg)
 		} else {
-			// Standard viper unmarshaler is fine.
-			// TODO(ccaraman): UnmarshallExact should be used to catch erroneous config entries.
-			// 	This leads to quickly identifying config values that are not supported and reduce confusion for
-			// 	users.
-			err = sv.Unmarshal(receiverCfg)
+			err = sv.UnmarshalExact(receiverCfg)
 		}
 
 		if err != nil {
 			return nil, &configError{
-				code: errUnmarshalError,
+				code: errUnmarshalErrorOnReceiver,
 				msg:  fmt.Sprintf("error reading settings for receiver type %q: %v", typeStr, err),
 			}
 		}
@@ -410,9 +430,9 @@ func loadExporters(v *viper.Viper, factories map[string]exporter.Factory) (confi
 
 		// Now that the default config struct is created we can Unmarshal into it
 		// and it will apply user-defined config on top of the default.
-		if err := sv.Unmarshal(exporterCfg); err != nil {
+		if err := sv.UnmarshalExact(exporterCfg); err != nil {
 			return nil, &configError{
-				code: errUnmarshalError,
+				code: errUnmarshalErrorOnExporter,
 				msg:  fmt.Sprintf("error reading settings for exporter type %q: %v", typeStr, err),
 			}
 		}
@@ -470,9 +490,9 @@ func loadProcessors(v *viper.Viper, factories map[string]processor.Factory) (con
 
 		// Now that the default config struct is created we can Unmarshal into it
 		// and it will apply user-defined config on top of the default.
-		if err := sv.Unmarshal(processorCfg); err != nil {
+		if err := sv.UnmarshalExact(processorCfg); err != nil {
 			return nil, &configError{
-				code: errUnmarshalError,
+				code: errUnmarshalErrorOnProcessor,
 				msg:  fmt.Sprintf("error reading settings for processor type %q: %v", typeStr, err),
 			}
 		}
@@ -529,9 +549,9 @@ func loadPipelines(v *viper.Viper) (configmodels.Pipelines, error) {
 
 		// Now that the default config struct is created we can Unmarshal into it
 		// and it will apply user-defined config on top of the default.
-		if err := subViper.UnmarshalKey(key, &pipelineCfg); err != nil {
+		if err := subViper.UnmarshalKey(key, &pipelineCfg, errorOnUnused); err != nil {
 			return nil, &configError{
-				code: errUnmarshalError,
+				code: errUnmarshalErrorOnPipeline,
 				msg:  fmt.Sprintf("error reading settings for pipeline type %q: %v", typeStr, err),
 			}
 		}
@@ -876,3 +896,12 @@ func expandStringValues(value interface{}) interface{} {
 		return nmap
 	}
 }
+
+// errorOnUnused sets the decoder configuration to error in case of unused sections
+// are present in the configuration.
+func errorOnUnused(decoderCfg *mapstructure.DecoderConfig) {
+	// If ErrorUnused is true, then it is an error for there to exist
+	// keys in the original map that were unused in the decoding process
+	// (extra keys).
+	decoderCfg.ErrorUnused = true
+}

config/config_test.go

@@ -371,19 +371,26 @@ func TestDecodeConfig_Invalid(t *testing.T) {
 		{name: "unknown-receiver-type", expected: errUnknownReceiverType},
 		{name: "unknown-exporter-type", expected: errUnknownExporterType},
 		{name: "unknown-processor-type", expected: errUnknownProcessorType},
-		{name: "invalid-extension-disabled-value", expected: errUnmarshalError},
-		{name: "invalid-service-extensions-value", expected: errUnmarshalError},
-		{name: "invalid-bool-value", expected: errUnmarshalError},
-		{name: "invalid-sequence-value", expected: errUnmarshalError},
-		{name: "invalid-disabled-bool-value", expected: errUnmarshalError},
-		{name: "invalid-disabled-bool-value2", expected: errUnmarshalError},
+		{name: "invalid-extension-disabled-value", expected: errUnmarshalErrorOnExtension},
+		{name: "invalid-service-extensions-value", expected: errUnmarshalErrorOnService},
+		{name: "invalid-bool-value", expected: errUnmarshalErrorOnProcessor},
+		{name: "invalid-sequence-value", expected: errUnmarshalErrorOnPipeline},
+		{name: "invalid-disabled-bool-value", expected: errUnmarshalErrorOnExporter},
+		{name: "invalid-disabled-bool-value2", expected: errUnmarshalErrorOnReceiver},
 		{name: "invalid-pipeline-type", expected: errInvalidPipelineType},
 		{name: "invalid-pipeline-type-and-name", expected: errInvalidTypeAndNameKey},
 		{name: "duplicate-extension", expected: errDuplicateExtensionName},
 		{name: "duplicate-receiver", expected: errDuplicateReceiverName},
 		{name: "duplicate-exporter", expected: errDuplicateExporterName},
 		{name: "duplicate-processor", expected: errDuplicateProcessorName},
 		{name: "duplicate-pipeline", expected: errDuplicatePipelineName},
+		{name: "invalid-top-level-section", expected: errUnmarshalErrorOnTopLevelSection},
+		{name: "invalid-extension-section", expected: errUnmarshalErrorOnExtension},
+		{name: "invalid-service-section", expected: errUnmarshalErrorOnService},
+		{name: "invalid-receiver-section", expected: errUnmarshalErrorOnReceiver},
+		{name: "invalid-processor-section", expected: errUnmarshalErrorOnProcessor},
+		{name: "invalid-exporter-section", expected: errUnmarshalErrorOnExporter},
+		{name: "invalid-pipeline-section", expected: errUnmarshalErrorOnPipeline},
 	}
 
 	factories, err := ExampleComponents()
@@ -400,8 +407,8 @@ func TestDecodeConfig_Invalid(t *testing.T) {
 					test.expected, err, test.name)
 			} else {
 				if cfgErr.code != test.expected {
-					t.Errorf("expected config error code %v but got error code %v on invalid config case: %s",
-						test.expected, cfgErr.code, test.name)
+					t.Errorf("expected config error code %v but got error code %v (msg: %q) on invalid config case: %s",
+						test.expected, cfgErr.code, cfgErr.msg, test.name)
 				}
 
 				if cfgErr.Error() == "" {

config/testdata/invalid-exporter-section.yaml

@@ -0,0 +1,20 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+    unknown_section: exporter
+extensions:
+  exampleextension:
+service:
+  extensions:
+    - exampleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter

config/testdata/invalid-extension-section.yaml

@@ -0,0 +1,21 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+    unknown_section:
+      a_num: 2
+service:
+  extensions:
+    - examapleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter

config/testdata/invalid-pipeline-section.yaml

@@ -0,0 +1,20 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+service:
+  extensions:
+    - exampleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter
+    unknown_section: 1

config/testdata/invalid-processor-section.yaml

@@ -0,0 +1,21 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+    unknown_section:
+      a_num: 2
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+service:
+  extensions:
+    - examapleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter

config/testdata/invalid-receiver-section.yaml

@@ -0,0 +1,21 @@
+receivers:
+  examplereceiver:
+    unknown_section:
+      a_num: 2
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+service:
+  extensions:
+    - examapleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter

config/testdata/invalid-service-section.yaml

@@ -0,0 +1,21 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+service:
+  extenstions:
+    - examapleextension
+  unknown_section:
+    a_num: 2
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter

config/testdata/invalid-top-level-section.yaml

@@ -0,0 +1,21 @@
+receivers:
+  examplereceiver:
+processors:
+  exampleprocessor:
+exporters:
+  exampleexporter:
+extensions:
+  exampleextension:
+service:
+  extenstions:
+    - examapleextension
+pipelines:
+  traces:
+    receivers:
+      - examplereceiver
+    processors:
+      - exampleprocessor
+    exporters:
+      - exampleexporter
+unknown_section:
+  a_num: 2