Skip to content

Commit

Permalink
Add recursive validation check for configs (open-telemetry#6545)
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <[email protected]>

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored and jaronoff97 committed Dec 14, 2022
1 parent f8574d8 commit 6c6d238
Show file tree
Hide file tree
Showing 3 changed files with 284 additions and 3 deletions.
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)
})
}
}

0 comments on commit 6c6d238

Please sign in to comment.