Skip to content

Commit 29f0c4f

Browse files
authored
[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 <[email protected]>
1 parent 7ec7246 commit 29f0c4f

File tree

3 files changed

+87
-19
lines changed

3 files changed

+87
-19
lines changed
+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: componenttest
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Improve config struct mapstructure field tag checks
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12590]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: "`remain` tags and `omitempty` tags without a custom field name will now pass validation."
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: []

component/componenttest/configtest.go

+17-19
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ func validateConfigDataType(t reflect.Type) error {
6565

6666
// checkStructFieldTags inspects the tags of a struct field.
6767
func checkStructFieldTags(f reflect.StructField) error {
68-
tagValue := f.Tag.Get("mapstructure")
69-
if tagValue == "" {
68+
tagValue, ok := f.Tag.Lookup("mapstructure")
69+
if !ok {
7070
// Ignore special types.
7171
switch f.Type.Kind() {
7272
case reflect.Interface, reflect.Chan, reflect.Func, reflect.Uintptr, reflect.UnsafePointer:
@@ -85,6 +85,10 @@ func checkStructFieldTags(f reflect.StructField) error {
8585
return nil
8686
}
8787

88+
if tagValue == "" {
89+
return fmt.Errorf("mapstructure tag on field %q is empty", f.Name)
90+
}
91+
8892
tagParts := strings.Split(tagValue, ",")
8993
if tagParts[0] != "" {
9094
if tagParts[0] == "-" {
@@ -93,20 +97,17 @@ func checkStructFieldTags(f reflect.StructField) error {
9397
}
9498
}
9599

96-
// Check if squash is specified.
97-
squash := false
98100
for _, tag := range tagParts[1:] {
99-
if tag == "squash" {
100-
squash = true
101-
break
102-
}
103-
}
104-
105-
if squash {
106-
// Field was squashed.
107-
if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
108-
return fmt.Errorf(
109-
"attempt to squash non-struct type on field %q", f.Name)
101+
switch tag {
102+
case "squash":
103+
if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
104+
return fmt.Errorf(
105+
"attempt to squash non-struct type on field %q", f.Name)
106+
}
107+
case "remain":
108+
if f.Type.Kind() != reflect.Map && f.Type.Kind() != reflect.Interface {
109+
return fmt.Errorf(`attempt to use "remain" on non-map or interface type field %q`, f.Name)
110+
}
110111
}
111112
}
112113

@@ -121,10 +122,7 @@ func checkStructFieldTags(f reflect.StructField) error {
121122

122123
default:
123124
fieldTag := tagParts[0]
124-
if !configFieldTagRegExp.MatchString(fieldTag) {
125-
if f.Name == "AdditionalProperties" {
126-
return nil
127-
}
125+
if fieldTag != "" && !configFieldTagRegExp.MatchString(fieldTag) {
128126
return fmt.Errorf(
129127
"field %q has config tag %q which doesn't satisfy %q",
130128
f.Name,

component/componenttest/configtest_test.go

+45
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,30 @@ func TestCheckConfigStruct(t *testing.T) {
5151
_someInt int
5252
}{},
5353
},
54+
{
55+
name: "remain_mapstructure_tag",
56+
config: struct {
57+
AdditionalProperties map[string]any `mapstructure:",remain"`
58+
}{},
59+
},
60+
{
61+
name: "remain_with_interface_type",
62+
config: struct {
63+
AdditionalProperties any `mapstructure:",remain"`
64+
}{},
65+
},
66+
{
67+
name: "omitempty_mapstructure_tag",
68+
config: struct {
69+
MyPublicString string `mapstructure:",omitempty"`
70+
}{},
71+
},
72+
{
73+
name: "named_omitempty_mapstructure_tag",
74+
config: struct {
75+
MyPublicString string `mapstructure:"my_public_string,omitempty"`
76+
}{},
77+
},
5478
{
5579
name: "not_struct_nor_pointer",
5680
config: func(x int) int {
@@ -65,6 +89,20 @@ func TestCheckConfigStruct(t *testing.T) {
6589
}{},
6690
wantErrMsgSubStr: "attempt to squash non-struct type on field \"MyInt\"",
6791
},
92+
{
93+
name: "remain_on_non_map",
94+
config: struct {
95+
AdditionalProperties string `mapstructure:",remain"`
96+
}{},
97+
wantErrMsgSubStr: `attempt to use "remain" on non-map or interface type field "AdditionalProperties"`,
98+
},
99+
{
100+
name: "bad_custom_field_name",
101+
config: struct {
102+
AdditionalProperties any `mapstructure:"Additional_Properties"`
103+
}{},
104+
wantErrMsgSubStr: `field "AdditionalProperties" has config tag "Additional_Properties" which doesn't satisfy "^[a-z0-9][a-z0-9_]*$"`,
105+
},
68106
{
69107
name: "invalid_tag_detected",
70108
config: BadConfigTag{},
@@ -77,6 +115,13 @@ func TestCheckConfigStruct(t *testing.T) {
77115
}{},
78116
wantErrMsgSubStr: "mapstructure tag not present on field \"PublicFieldWithoutMapstructureTag\"",
79117
},
118+
{
119+
name: "public_field_must_have_nonempty_tag",
120+
config: struct {
121+
PublicFieldWithoutMapstructureTag string `mapstructure:""`
122+
}{},
123+
wantErrMsgSubStr: "mapstructure tag on field \"PublicFieldWithoutMapstructureTag\" is empty",
124+
},
80125
{
81126
name: "invalid_map_item",
82127
config: struct {

0 commit comments

Comments
 (0)