[componenttest] Improve config struct checks (#12590)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Improves our config struct checks by allowing for a few more cases that
are possible with mapstructure.

---------

Co-authored-by: Evan Bradley <evan-bradley@users.noreply.github.com>
This commit is contained in:
Evan Bradley 2025-03-13 12:26:56 -04:00 committed by GitHub
parent 7ec72468f6
commit 29f0c4f35f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 87 additions and 19 deletions

View File

@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.
# 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: componenttest
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Improve config struct mapstructure field tag checks
# One or more tracking issues or pull requests related to the change
issues: [12590]
# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: "`remain` tags and `omitempty` tags without a custom field name will now pass validation."
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []

View File

@ -65,8 +65,8 @@ func validateConfigDataType(t reflect.Type) error {
// checkStructFieldTags inspects the tags of a struct field.
func checkStructFieldTags(f reflect.StructField) error {
tagValue := f.Tag.Get("mapstructure")
if tagValue == "" {
tagValue, ok := f.Tag.Lookup("mapstructure")
if !ok {
// Ignore special types.
switch f.Type.Kind() {
case reflect.Interface, reflect.Chan, reflect.Func, reflect.Uintptr, reflect.UnsafePointer:
@ -85,6 +85,10 @@ func checkStructFieldTags(f reflect.StructField) error {
return nil
}
if tagValue == "" {
return fmt.Errorf("mapstructure tag on field %q is empty", f.Name)
}
tagParts := strings.Split(tagValue, ",")
if tagParts[0] != "" {
if tagParts[0] == "-" {
@ -93,20 +97,17 @@ func checkStructFieldTags(f reflect.StructField) error {
}
}
// 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 tag {
case "squash":
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)
}
case "remain":
if f.Type.Kind() != reflect.Map && f.Type.Kind() != reflect.Interface {
return fmt.Errorf(`attempt to use "remain" on non-map or interface type field %q`, f.Name)
}
}
}
@ -121,10 +122,7 @@ func checkStructFieldTags(f reflect.StructField) error {
default:
fieldTag := tagParts[0]
if !configFieldTagRegExp.MatchString(fieldTag) {
if f.Name == "AdditionalProperties" {
return nil
}
if fieldTag != "" && !configFieldTagRegExp.MatchString(fieldTag) {
return fmt.Errorf(
"field %q has config tag %q which doesn't satisfy %q",
f.Name,

View File

@ -51,6 +51,30 @@ func TestCheckConfigStruct(t *testing.T) {
_someInt int
}{},
},
{
name: "remain_mapstructure_tag",
config: struct {
AdditionalProperties map[string]any `mapstructure:",remain"`
}{},
},
{
name: "remain_with_interface_type",
config: struct {
AdditionalProperties any `mapstructure:",remain"`
}{},
},
{
name: "omitempty_mapstructure_tag",
config: struct {
MyPublicString string `mapstructure:",omitempty"`
}{},
},
{
name: "named_omitempty_mapstructure_tag",
config: struct {
MyPublicString string `mapstructure:"my_public_string,omitempty"`
}{},
},
{
name: "not_struct_nor_pointer",
config: func(x int) int {
@ -65,6 +89,20 @@ func TestCheckConfigStruct(t *testing.T) {
}{},
wantErrMsgSubStr: "attempt to squash non-struct type on field \"MyInt\"",
},
{
name: "remain_on_non_map",
config: struct {
AdditionalProperties string `mapstructure:",remain"`
}{},
wantErrMsgSubStr: `attempt to use "remain" on non-map or interface type field "AdditionalProperties"`,
},
{
name: "bad_custom_field_name",
config: struct {
AdditionalProperties any `mapstructure:"Additional_Properties"`
}{},
wantErrMsgSubStr: `field "AdditionalProperties" has config tag "Additional_Properties" which doesn't satisfy "^[a-z0-9][a-z0-9_]*$"`,
},
{
name: "invalid_tag_detected",
config: BadConfigTag{},
@ -77,6 +115,13 @@ func TestCheckConfigStruct(t *testing.T) {
}{},
wantErrMsgSubStr: "mapstructure tag not present on field \"PublicFieldWithoutMapstructureTag\"",
},
{
name: "public_field_must_have_nonempty_tag",
config: struct {
PublicFieldWithoutMapstructureTag string `mapstructure:""`
}{},
wantErrMsgSubStr: "mapstructure tag on field \"PublicFieldWithoutMapstructureTag\" is empty",
},
{
name: "invalid_map_item",
config: struct {