Move ValidateConfig from configcheck to configtest

component/experimental/component/factory.go

@@ -44,7 +44,7 @@ type ConfigSourceFactory interface {
 	// configuration and should not cause side-effects that prevent the creation
 	// of multiple instances of the Source.
 	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have such check in the
+	// 'configtest.CheckConfigStruct'. It is recommended to have such check in the
 	// tests of any implementation of the ConfigSourceFactory interface.
 	CreateDefaultConfig() config.Source
 

component/exporter.go

@@ -70,7 +70,7 @@ type ExporterFactory interface {
 	// configuration and should not cause side-effects that prevent the creation
 	// of multiple instances of the Exporter.
 	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have these checks in the
+	// 'configtest.CheckConfigStruct'. It is recommended to have these checks in the
 	// tests of any implementation of the Factory interface.
 	CreateDefaultConfig() config.Exporter
 

component/extension.go

@@ -69,7 +69,7 @@ type ExtensionFactory interface {
 	// configuration and should not cause side-effects that prevent the creation
 	// of multiple instances of the Extension.
 	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have these checks in the
+	// 'configtest.CheckConfigStruct'. It is recommended to have these checks in the
 	// tests of any implementation of the Factory interface.
 	CreateDefaultConfig() config.Extension
 

component/processor.go

@@ -75,7 +75,7 @@ type ProcessorFactory interface {
 	// configuration and should not cause side-effects that prevent the creation
 	// of multiple instances of the Processor.
 	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have these checks in the
+	// 'configtest.CheckConfigStruct'. It is recommended to have these checks in the
 	// tests of any implementation of the Factory interface.
 	CreateDefaultConfig() config.Processor
 

component/receiver.go

@@ -79,7 +79,7 @@ type ReceiverFactory interface {
 	// configuration and should not cause side-effects that prevent the creation
 	// of multiple instances of the Receiver.
 	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have these checks in the
+	// 'configtest.CheckConfigStruct'. It is recommended to have these checks in the
 	// tests of any implementation of the Factory interface.
 	CreateDefaultConfig() config.Receiver
 

config/configcheck/configcheck.go

@@ -15,171 +15,36 @@
 package configcheck
 
 import (
-	"fmt"
-	"reflect"
-	"regexp"
-	"strings"
-
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config/configtest"
 	"go.opentelemetry.io/collector/consumer/consumererror"
 )
 
-// The regular expression for valid config field tag.
-var configFieldTagRegExp = regexp.MustCompile("^[a-z0-9][a-z0-9_]*$")
-
 // ValidateConfigFromFactories checks if all configurations for the given factories
 // are satisfying the patterns used by the collector.
 func ValidateConfigFromFactories(factories component.Factories) error {
 	var errs []error
 
 	for _, factory := range factories.Receivers {
-		if err := ValidateConfig(factory.CreateDefaultConfig()); err != nil {
+		if err := configtest.CheckConfigStruct(factory.CreateDefaultConfig()); err != nil {
 			errs = append(errs, err)
 		}
 	}
 	for _, factory := range factories.Processors {
-		if err := ValidateConfig(factory.CreateDefaultConfig()); err != nil {
+		if err := configtest.CheckConfigStruct(factory.CreateDefaultConfig()); err != nil {
 			errs = append(errs, err)
 		}
 	}
 	for _, factory := range factories.Exporters {
-		if err := ValidateConfig(factory.CreateDefaultConfig()); err != nil {
+		if err := configtest.CheckConfigStruct(factory.CreateDefaultConfig()); err != nil {
 			errs = append(errs, err)
 		}
 	}
 	for _, factory := range factories.Extensions {
-		if err := ValidateConfig(factory.CreateDefaultConfig()); err != nil {
+		if err := configtest.CheckConfigStruct(factory.CreateDefaultConfig()); err != nil {
 			errs = append(errs, err)
 		}
 	}
 
 	return consumererror.Combine(errs)
 }
-
-// ValidateConfig enforces that given configuration object is following the patterns
-// used by the collector. This ensures consistency between different implementations
-// of components and extensions. It is recommended for implementers of components
-// to call this function on their tests passing the default configuration of the
-// component factory.
-func ValidateConfig(config interface{}) error {
-	t := reflect.TypeOf(config)
-	if t.Kind() == reflect.Ptr {
-		t = t.Elem()
-	}
-
-	if t.Kind() != reflect.Struct {
-		return fmt.Errorf("config must be a struct or a pointer to one, the passed object is a %s", t.Kind())
-	}
-
-	return validateConfigDataType(t)
-}
-
-// validateConfigDataType performs a descending validation of the given type.
-// If the type is a struct it goes to each of its fields to check for the proper
-// tags.
-func validateConfigDataType(t reflect.Type) error {
-	var errs []error
-
-	switch t.Kind() {
-	case reflect.Ptr:
-		if err := validateConfigDataType(t.Elem()); err != nil {
-			errs = append(errs, err)
-		}
-	case reflect.Struct:
-		// Reflect on the pointed data and check each of its fields.
-		nf := t.NumField()
-		for i := 0; i < nf; i++ {
-			f := t.Field(i)
-			if err := checkStructFieldTags(f); err != nil {
-				errs = append(errs, err)
-			}
-		}
-	default:
-		// The config object can carry other types but they are not used when
-		// reading the configuration via koanf so ignore them. Basically ignore:
-		// reflect.Uintptr, reflect.Chan, reflect.Func, reflect.Interface, and
-		// reflect.UnsafePointer.
-	}
-
-	if err := consumererror.Combine(errs); err != nil {
-		return fmt.Errorf(
-			"type %q from package %q has invalid config settings: %v",
-			t.Name(),
-			t.PkgPath(),
-			err)
-	}
-
-	return nil
-}
-
-// checkStructFieldTags inspects the tags of a struct field.
-func checkStructFieldTags(f reflect.StructField) error {
-
-	tagValue := f.Tag.Get("mapstructure")
-	if tagValue == "" {
-
-		// Ignore special types.
-		switch f.Type.Kind() {
-		case reflect.Interface, reflect.Chan, reflect.Func, reflect.Uintptr, reflect.UnsafePointer:
-			// Allow the config to carry the types above, but since they are not read
-			// when loading configuration, just ignore them.
-			return nil
-		}
-
-		// Public fields of other types should be tagged.
-		chars := []byte(f.Name)
-		if len(chars) > 0 && chars[0] >= 'A' && chars[0] <= 'Z' {
-			return fmt.Errorf("mapstructure tag not present on field %q", f.Name)
-		}
-
-		// Not public field, no need to have a tag.
-		return nil
-	}
-
-	tagParts := strings.Split(tagValue, ",")
-	if tagParts[0] != "" {
-		if tagParts[0] == "-" {
-			// Nothing to do, as mapstructure decode skips this field.
-			return nil
-		}
-	}
-
-	// Check if squash is specified.
-	squash := false
-	for _, tag := range tagParts[1:] {
-		if tag == "squash" {
-			squash = true
-			break
-		}
-	}
-
-	if squash {
-		// Field was squashed.
-		if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
-			return fmt.Errorf(
-				"attempt to squash non-struct type on field %q", f.Name)
-		}
-	}
-
-	switch f.Type.Kind() {
-	case reflect.Struct:
-		// It is another struct, continue down-level.
-		return validateConfigDataType(f.Type)
-
-	case reflect.Map, reflect.Slice, reflect.Array:
-		// The element of map, array, or slice can be itself a configuration object.
-		return validateConfigDataType(f.Type.Elem())
-
-	default:
-		fieldTag := tagParts[0]
-		if !configFieldTagRegExp.MatchString(fieldTag) {
-			return fmt.Errorf(
-				"field %q has config tag %q which doesn't satisfy %q",
-				f.Name,
-				fieldTag,
-				configFieldTagRegExp.String())
-		}
-	}
-
-	return nil
-}

config/configcheck/configcheck_test.go

@@ -16,11 +16,8 @@ package configcheck
 
 import (
 	"context"
-	"io"
-	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/component"
@@ -48,141 +45,6 @@ func TestValidateConfigFromFactories_Failure(t *testing.T) {
 	require.Error(t, err)
 }
 
-func TestValidateConfigPointerAndValue(t *testing.T) {
-	config := struct {
-		SomeFiled string `mapstructure:"test"`
-	}{}
-	assert.NoError(t, ValidateConfig(config))
-	assert.NoError(t, ValidateConfig(&config))
-}
-
-func TestValidateConfig(t *testing.T) {
-	type BadConfigTag struct {
-		BadTagField int `mapstructure:"test-dash"`
-	}
-
-	tests := []struct {
-		name             string
-		config           interface{}
-		wantErrMsgSubStr string
-	}{
-		{
-			name: "typical_config",
-			config: struct {
-				MyPublicString string `mapstructure:"string"`
-			}{},
-		},
-		{
-			name: "private_fields_ignored",
-			config: struct {
-				// A public type with proper tag.
-				MyPublicString string `mapstructure:"string"`
-				// A public type with proper tag.
-				MyPublicInt string `mapstructure:"int"`
-				// A public type that should be ignored.
-				MyFunc func() error
-				// A public type that should be ignored.
-				Reader io.Reader
-				// private type not tagged.
-				myPrivateString string
-				_someInt        int
-			}{},
-		},
-		{
-			name: "not_struct_nor_pointer",
-			config: func(x int) int {
-				return x * x
-			},
-			wantErrMsgSubStr: "config must be a struct or a pointer to one, the passed object is a func",
-		},
-		{
-			name: "squash_on_non_struct",
-			config: struct {
-				MyInt int `mapstructure:",squash"`
-			}{},
-			wantErrMsgSubStr: "attempt to squash non-struct type on field \"MyInt\"",
-		},
-		{
-			name:             "invalid_tag_detected",
-			config:           BadConfigTag{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "public_field_must_have_tag",
-			config: struct {
-				PublicFieldWithoutMapstructureTag string
-			}{},
-			wantErrMsgSubStr: "mapstructure tag not present on field \"PublicFieldWithoutMapstructureTag\"",
-		},
-		{
-			name: "invalid_map_item",
-			config: struct {
-				Map map[string]BadConfigTag `mapstructure:"test_map"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "invalid_slice_item",
-			config: struct {
-				Slice []BadConfigTag `mapstructure:"test_slice"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "invalid_array_item",
-			config: struct {
-				Array [2]BadConfigTag `mapstructure:"test_array"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "invalid_map_item_ptr",
-			config: struct {
-				Map map[string]*BadConfigTag `mapstructure:"test_map"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "invalid_slice_item_ptr",
-			config: struct {
-				Slice []*BadConfigTag `mapstructure:"test_slice"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "invalid_array_item_ptr",
-			config: struct {
-				Array [2]*BadConfigTag `mapstructure:"test_array"`
-			}{},
-			wantErrMsgSubStr: "field \"BadTagField\" has config tag \"test-dash\" which doesn't satisfy",
-		},
-		{
-			name: "valid_map_item",
-			config: struct {
-				Map map[string]int `mapstructure:"test_map"`
-			}{},
-		},
-		{
-			name: "valid_slice_item",
-			config: struct {
-				Slice []string `mapstructure:"test_slice"`
-			}{},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := ValidateConfig(tt.config)
-			if tt.wantErrMsgSubStr == "" {
-				assert.NoError(t, err)
-			} else {
-				require.Error(t, err)
-				assert.True(t, strings.Contains(err.Error(), tt.wantErrMsgSubStr))
-			}
-		})
-	}
-}
-
 // badConfigExtensionFactory was created to force error path from factory returning
 // a config not satisfying the validation.
 type badConfigExtensionFactory struct{}