Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recursive validation check for configs #6545

Merged
merged 1 commit into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .chloggen/deepvalidate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add recursive validation check for configs

# One or more tracking issues or pull requests related to the change
issues: [4584]
69 changes: 66 additions & 3 deletions component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
package component // import "go.opentelemetry.io/collector/component"

import (
"reflect"

"go.uber.org/multierr"

"go.opentelemetry.io/collector/confmap"
)

Expand All @@ -24,6 +28,10 @@ type Config interface {
privateConfig()
}

// As interface types are only used for static typing, a common idiom to find the reflection Type
// for an interface type Foo is to use a *Foo value.
var configValidatorType = reflect.TypeOf((*ConfigValidator)(nil)).Elem()

// ConfigValidator defines an optional interface for configurations to implement to do validation.
type ConfigValidator interface {
// Validate the configuration and returns an error if invalid.
Expand All @@ -33,12 +41,67 @@ type ConfigValidator interface {
// ValidateConfig validates a config, by doing this:
// - Call Validate on the config itself if the config implements ConfigValidator.
func ValidateConfig(cfg Config) error {
validator, ok := cfg.(ConfigValidator)
if !ok {
return validate(reflect.ValueOf(cfg))
}

func validate(v reflect.Value) error {
// Validate the value itself.
switch v.Kind() {
case reflect.Invalid:
return nil
case reflect.Ptr:
return validate(v.Elem())
case reflect.Struct:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.NumField(); i++ {
if !v.Type().Field(i).IsExported() {
continue
}
errs = multierr.Append(errs, validate(v.Field(i)))
}
return errs
case reflect.Slice, reflect.Array:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
// Reflect on the pointed data and check each of its fields.
for i := 0; i < v.Len(); i++ {
errs = multierr.Append(errs, validate(v.Index(i)))
}
return errs
case reflect.Map:
var errs error
errs = multierr.Append(errs, callValidateIfPossible(v))
iter := v.MapRange()
for iter.Next() {
errs = multierr.Append(errs, validate(iter.Key()))
errs = multierr.Append(errs, validate(iter.Value()))
}
return errs
default:
return callValidateIfPossible(v)
}
}

func callValidateIfPossible(v reflect.Value) error {
// If the value type implements ConfigValidator just call Validate
if v.Type().Implements(configValidatorType) {
return v.Interface().(ConfigValidator).Validate()
}

// If the pointer type implements ConfigValidator call Validate on the pointer to the current value.
if reflect.PtrTo(v.Type()).Implements(configValidatorType) {
// If not addressable, then create a new *V pointer and set the value to current v.
if !v.CanAddr() {
pv := reflect.New(reflect.PtrTo(v.Type()).Elem())
pv.Elem().Set(v)
v = pv.Elem()
}
return v.Addr().Interface().(ConfigValidator).Validate()
}

return validator.Validate()
return nil
}

// Type is the component type as it is used in the config.
Expand Down
207 changes: 207 additions & 0 deletions component/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package component

import (
"errors"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
)

type configChildStruct struct {
Child errConfig
ChildPtr *errConfig
}

type configChildSlice struct {
Child []errConfig
ChildPtr []*errConfig
}

type configChildMapValue struct {
Child map[string]errConfig
ChildPtr map[string]*errConfig
}

type configChildMapKey struct {
Child map[errType]string
ChildPtr map[*errType]string
}

type configChildTypeDef struct {
Child errType
ChildPtr *errType
}

type errConfig struct {
err error
}

func (e *errConfig) Validate() error {
return e.err
}

type errType string

func (e errType) Validate() error {
if e == "" {
return nil
}
return errors.New(string(e))
}

func newErrType(etStr string) *errType {
et := errType(etStr)
return &et
}

type errMapType map[string]string

func (e errMapType) Validate() error {
return errors.New(e["err"])
}

func newErrMapType() *errMapType {
et := errMapType(nil)
return &et
}

func TestValidateConfig(t *testing.T) {
var tests = []struct {
name string
cfg any
expected error
}{
{
name: "struct",
cfg: errConfig{err: errors.New("struct")},
expected: errors.New("struct"),
},
{
name: "pointer struct",
cfg: &errConfig{err: errors.New("pointer struct")},
expected: errors.New("pointer struct"),
},
{
name: "type",
cfg: errType("type"),
expected: errors.New("type"),
},
{
name: "pointer child",
cfg: newErrType("pointer type"),
expected: errors.New("pointer type"),
},
{
name: "nil",
cfg: nil,
expected: nil,
},
{
name: "nil map type",
cfg: errMapType(nil),
expected: errors.New(""),
},
{
name: "nil pointer map type",
cfg: newErrMapType(),
expected: errors.New(""),
},
{
name: "child struct",
cfg: configChildStruct{Child: errConfig{err: errors.New("child struct")}},
expected: errors.New("child struct"),
},
{
name: "pointer child struct",
cfg: &configChildStruct{Child: errConfig{err: errors.New("pointer child struct")}},
expected: errors.New("pointer child struct"),
},
{
name: "child struct pointer",
cfg: &configChildStruct{ChildPtr: &errConfig{err: errors.New("child struct pointer")}},
expected: errors.New("child struct pointer"),
},
{
name: "child slice",
cfg: configChildSlice{Child: []errConfig{{}, {err: errors.New("child slice")}}},
expected: errors.New("child slice"),
},
{
name: "pointer child slice",
cfg: &configChildSlice{Child: []errConfig{{}, {err: errors.New("pointer child slice")}}},
expected: errors.New("pointer child slice"),
},
{
name: "child slice pointer",
cfg: &configChildSlice{ChildPtr: []*errConfig{{}, {err: errors.New("child slice pointer")}}},
expected: errors.New("child slice pointer"),
},
{
name: "child map value",
cfg: configChildMapValue{Child: map[string]errConfig{"test": {err: errors.New("child map")}}},
expected: errors.New("child map"),
},
{
name: "pointer child map value",
cfg: &configChildMapValue{Child: map[string]errConfig{"test": {err: errors.New("pointer child map")}}},
expected: errors.New("pointer child map"),
},
{
name: "child map value pointer",
cfg: &configChildMapValue{ChildPtr: map[string]*errConfig{"test": {err: errors.New("child map pointer")}}},
expected: errors.New("child map pointer"),
},
{
name: "child map key",
cfg: configChildMapKey{Child: map[errType]string{"child map key": ""}},
expected: errors.New("child map key"),
},
{
name: "pointer child map key",
cfg: &configChildMapKey{Child: map[errType]string{"pointer child map key": ""}},
expected: errors.New("pointer child map key"),
},
{
name: "child map key pointer",
cfg: &configChildMapKey{ChildPtr: map[*errType]string{newErrType("child map key pointer"): ""}},
expected: errors.New("child map key pointer"),
},
{
name: "child type",
cfg: configChildTypeDef{Child: "child type"},
expected: errors.New("child type"),
},
{
name: "pointer child type",
cfg: &configChildTypeDef{Child: "pointer child type"},
expected: errors.New("pointer child type"),
},
{
name: "child type pointer",
cfg: &configChildTypeDef{ChildPtr: newErrType("child type pointer")},
expected: errors.New("child type pointer"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validate(reflect.ValueOf(tt.cfg))
assert.Equal(t, tt.expected, err)
})
}
}