Skip to content

Commit 3346e40

Browse files
azdagronazdagron
authored
Fix plugin config parsing code (#3939)
* Fix plugin config parsing code Recent changes to the plugin config parsing to retain plugin order failed to account for the more verbose HCL or JSON forms. This PR fixes the code to handle these forms as well as prevents duplicate declarations, which was accidentally allowed as part of the previous change. Fixes: #3938 Signed-off-by: Andrew Harding <[email protected]> * Add another test case Signed-off-by: Andrew Harding <[email protected]> --------- Signed-off-by: Andrew Harding <[email protected]>
1 parent 7b0e1d5 commit 3346e40

File tree

2 files changed

+247
-93
lines changed

2 files changed

+247
-93
lines changed

‎pkg/common/catalog/config.go

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,37 +74,96 @@ func PluginConfigsFromHCLNode(pluginsNode ast.Node) (PluginConfigs, error) {
7474
if pluginsNode == nil {
7575
return nil, errors.New("plugins node is nil")
7676
}
77-
pluginsList, ok := pluginsNode.(*ast.ObjectList)
78-
if !ok {
79-
return nil, fmt.Errorf("expected plugins node type %T but got %T", pluginsList, pluginsNode)
77+
78+
// The seen set is used to detect multiple declarations for the same plugin.
79+
type key struct {
80+
Type string
81+
Name string
8082
}
83+
seen := make(map[key]struct{})
84+
8185
var pluginConfigs PluginConfigs
82-
for _, pluginObject := range pluginsList.Items {
83-
if len(pluginObject.Keys) != 2 {
84-
return nil, fmt.Errorf("plugin item expected to have two keys (type then name)")
86+
appendPlugin := func(pluginType, pluginName string, pluginNode ast.Node) error {
87+
// Ensure a plugin for a given type and name is only declared once.
88+
k := key{Type: pluginType, Name: pluginName}
89+
if _, ok := seen[k]; ok {
90+
return fmt.Errorf("plugin %q/%q declared more than once", pluginType, pluginName)
8591
}
92+
seen[k] = struct{}{}
8693

87-
pluginType, err := stringFromToken(pluginObject.Keys[0].Token)
88-
if err != nil {
89-
return nil, fmt.Errorf("invalid plugin type key %q: %w", pluginObject.Keys[0].Token.Text, err)
94+
var hclPluginConfig hclPluginConfig
95+
if err := hcl.DecodeObject(&hclPluginConfig, pluginNode); err != nil {
96+
return fmt.Errorf("failed to decode plugin config for %q/%q: %w", pluginType, pluginName, err)
9097
}
9198

92-
pluginName, err := stringFromToken(pluginObject.Keys[1].Token)
99+
pluginConfig, err := pluginConfigFromHCL(pluginType, pluginName, hclPluginConfig)
93100
if err != nil {
94-
return nil, fmt.Errorf("invalid plugin type name %q: %w", pluginObject.Keys[1].Token.Text, err)
101+
return fmt.Errorf("failed to create plugin config for %q/%q: %w", pluginType, pluginName, err)
95102
}
96103

97-
var hclPluginConfig hclPluginConfig
98-
if err := hcl.DecodeObject(&hclPluginConfig, pluginObject.Val); err != nil {
99-
return nil, fmt.Errorf("failed to decode plugin config for %q/%q: %w", pluginType, pluginName, err)
100-
}
104+
pluginConfigs = append(pluginConfigs, pluginConfig)
105+
return nil
106+
}
101107

102-
pluginConfig, err := pluginConfigFromHCL(pluginType, pluginName, hclPluginConfig)
108+
pluginsList, ok := pluginsNode.(*ast.ObjectList)
109+
if !ok {
110+
return nil, fmt.Errorf("expected plugins node type %T but got %T", pluginsList, pluginsNode)
111+
}
112+
113+
for _, pluginObject := range pluginsList.Items {
114+
pluginType, err := stringFromToken(pluginObject.Keys[0].Token)
103115
if err != nil {
104-
return nil, fmt.Errorf("failed to create plugin config for %q/%q: %w", pluginType, pluginName, err)
116+
return nil, fmt.Errorf("invalid plugin type key %q: %w", pluginObject.Keys[0].Token.Text, err)
105117
}
106118

107-
pluginConfigs = append(pluginConfigs, pluginConfig)
119+
var pluginName string
120+
switch len(pluginObject.Keys) {
121+
case 1:
122+
// One key is expected when using JSON formatted config:
123+
// "plugins": [
124+
// "NodeAttestor": [
125+
// {
126+
// "foo": ...
127+
// }
128+
// ]
129+
// ]
130+
//
131+
// Or the slightly more verbose HCL form:
132+
// plugins {
133+
// "NodeAttestor" = {
134+
// "foo" = { ... }
135+
// }
136+
// }
137+
pluginTypeList, ok := pluginObject.Val.(*ast.ObjectType)
138+
if !ok {
139+
return nil, fmt.Errorf("expected plugin object type %T but got %T", pluginTypeList, pluginObject.Val)
140+
}
141+
for _, pluginObject := range pluginTypeList.List.Items {
142+
pluginName, err = stringFromToken(pluginObject.Keys[0].Token)
143+
if err != nil {
144+
return nil, fmt.Errorf("invalid plugin type name %q: %w", pluginObject.Keys[0].Token.Text, err)
145+
}
146+
if err := appendPlugin(pluginType, pluginName, pluginObject.Val); err != nil {
147+
return nil, err
148+
}
149+
}
150+
case 2:
151+
// Two keys are expected for something like this (our typical config example)
152+
// plugins {
153+
// NodeAttestor "foo" {
154+
// ...
155+
// }
156+
// }
157+
pluginName, err = stringFromToken(pluginObject.Keys[1].Token)
158+
if err != nil {
159+
return nil, fmt.Errorf("invalid plugin type name %q: %w", pluginObject.Keys[1].Token.Text, err)
160+
}
161+
if err := appendPlugin(pluginType, pluginName, pluginObject.Val); err != nil {
162+
return nil, err
163+
}
164+
default:
165+
return nil, fmt.Errorf("expected one or two keys on the plugin item for type %q but got %d", pluginType, len(pluginObject.Keys))
166+
}
108167
}
109168
return pluginConfigs, nil
110169
}

‎pkg/common/catalog/config_test.go

Lines changed: 170 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -9,88 +9,183 @@ import (
99
)
1010

1111
func TestParsePluginConfigsFromHCLNode(t *testing.T) {
12-
root := struct {
13-
Plugins ast.Node
14-
}{}
15-
err := hcl.Decode(&root, `plugins {
16-
TYPE1 "NAME3" {
17-
plugin_data = "DATA3"
18-
enabled = false
19-
}
20-
TYPE4 "NAME4" {
21-
}
22-
TYPE1 "NAME1" {
23-
plugin_cmd = "CMD1"
24-
plugin_data = "DATA1"
25-
}
26-
TYPE2 "NAME2" {
27-
plugin_cmd = "CMD2"
28-
plugin_args = ["foo", "bar", "baz"]
29-
plugin_checksum = "CHECKSUM2"
30-
plugin_data = "DATA2"
31-
enabled = true
32-
}
33-
}`)
34-
require.NoError(t, err)
12+
test := func(t *testing.T, configIn string) {
13+
root := struct {
14+
Plugins ast.Node
15+
}{}
16+
err := hcl.Decode(&root, configIn)
17+
require.NoError(t, err)
3518

36-
configs, err := PluginConfigsFromHCLNode(root.Plugins)
37-
require.NoError(t, err)
19+
configs, err := PluginConfigsFromHCLNode(root.Plugins)
20+
require.NoError(t, err)
3821

39-
pluginA := PluginConfig{
40-
Name: "NAME3",
41-
Type: "TYPE1",
42-
Data: `"DATA3"`,
43-
Disabled: true,
44-
}
45-
pluginB := PluginConfig{
46-
Name: "NAME4",
47-
Type: "TYPE4",
48-
}
49-
pluginC := PluginConfig{
50-
Name: "NAME1",
51-
Type: "TYPE1",
52-
Path: "CMD1",
53-
Data: `"DATA1"`,
54-
Disabled: false,
55-
}
56-
pluginD := PluginConfig{
57-
Name: "NAME2",
58-
Type: "TYPE2",
59-
Path: "CMD2",
60-
Args: []string{"foo", "bar", "baz"},
61-
Checksum: "CHECKSUM2",
62-
Data: `"DATA2"`,
63-
Disabled: false,
64-
}
22+
pluginA := PluginConfig{
23+
Name: "NAME3",
24+
Type: "TYPE1",
25+
Data: `"DATA3"`,
26+
Disabled: true,
27+
}
28+
pluginB := PluginConfig{
29+
Name: "NAME4",
30+
Type: "TYPE4",
31+
}
32+
pluginC := PluginConfig{
33+
Name: "NAME1",
34+
Type: "TYPE1",
35+
Path: "CMD1",
36+
Data: `"DATA1"`,
37+
Disabled: false,
38+
}
39+
pluginD := PluginConfig{
40+
Name: "NAME2",
41+
Type: "TYPE2",
42+
Path: "CMD2",
43+
Args: []string{"foo", "bar", "baz"},
44+
Checksum: "CHECKSUM2",
45+
Data: `"DATA2"`,
46+
Disabled: false,
47+
}
6548

66-
// The declaration order should be preserved.
67-
require.Equal(t, PluginConfigs{
68-
pluginA,
69-
pluginB,
70-
pluginC,
71-
pluginD,
72-
}, configs)
49+
// The declaration order should be preserved.
50+
require.Equal(t, PluginConfigs{
51+
pluginA,
52+
pluginB,
53+
pluginC,
54+
pluginD,
55+
}, configs)
56+
57+
// Only A and C are of type TYPE1
58+
matching, remaining := configs.FilterByType("TYPE1")
59+
60+
require.Equal(t, PluginConfigs{
61+
pluginA,
62+
pluginC,
63+
}, matching)
64+
65+
require.Equal(t, PluginConfigs{
66+
pluginB,
67+
pluginD,
68+
}, remaining)
69+
70+
c, ok := configs.Find("TYPE1", "NAME1")
71+
require.Equal(t, pluginC, c)
72+
require.True(t, ok)
73+
74+
_, ok = configs.Find("WHATEVER", "NAME1")
75+
require.False(t, ok)
76+
77+
_, ok = configs.Find("TYPE1", "WHATEVER")
78+
require.False(t, ok)
79+
}
7380

74-
// Only A and C are of type TYPE1
75-
matching, remaining := configs.FilterByType("TYPE1")
81+
t.Run("HCL", func(t *testing.T) {
82+
config := `
83+
plugins {
84+
TYPE1 "NAME3" {
85+
plugin_data = "DATA3"
86+
enabled = false
87+
}
88+
TYPE4 "NAME4" {
89+
}
90+
TYPE1 "NAME1" {
91+
plugin_cmd = "CMD1"
92+
plugin_data = "DATA1"
93+
}
94+
TYPE2 "NAME2" {
95+
plugin_cmd = "CMD2"
96+
plugin_args = ["foo", "bar", "baz"]
97+
plugin_checksum = "CHECKSUM2"
98+
plugin_data = "DATA2"
99+
enabled = true
100+
}
101+
}
102+
`
103+
test(t, config)
104+
})
76105

77-
require.Equal(t, PluginConfigs{
78-
pluginA,
79-
pluginC,
80-
}, matching)
106+
t.Run("JSON", func(t *testing.T) {
107+
config := `{
108+
"plugins": {
109+
"TYPE1": [
110+
{
111+
"NAME3": {
112+
"plugin_data": "DATA3",
113+
"enabled": false
114+
}
115+
}
116+
],
117+
"TYPE4": [
118+
{
119+
"NAME4": [
120+
{
121+
}
122+
]
123+
}
124+
],
125+
"TYPE1": [
126+
{
127+
"NAME1": [
128+
{
129+
"plugin_cmd": "CMD1",
130+
"plugin_data": "DATA1"
131+
}
132+
]
133+
}
134+
],
135+
"TYPE2": [
136+
{
137+
"NAME2": [
138+
{
139+
"plugin_cmd": "CMD2",
140+
"plugin_args": ["foo", "bar", "baz"],
141+
"plugin_checksum": "CHECKSUM2",
142+
"plugin_data": "DATA2",
143+
"enabled": true
144+
}
145+
]
146+
}
147+
]
148+
}
149+
}`
150+
test(t, config)
151+
})
81152

82-
require.Equal(t, PluginConfigs{
83-
pluginB,
84-
pluginD,
85-
}, remaining)
153+
t.Run("Plugin declared more than once", func(t *testing.T) {
154+
config := `{
155+
"plugins": {
156+
"TYPE": [
157+
{
158+
"NAME": {}
159+
},
160+
{
161+
"NAME": {}
162+
}
163+
]
164+
}
165+
}`
166+
root := struct {
167+
Plugins ast.Node
168+
}{}
169+
err := hcl.Decode(&root, config)
170+
require.NoError(t, err)
86171

87-
c, ok := configs.Find("TYPE1", "NAME1")
88-
require.Equal(t, pluginC, c)
89-
require.True(t, ok)
172+
_, err = PluginConfigsFromHCLNode(root.Plugins)
173+
require.EqualError(t, err, `plugin "TYPE"/"NAME" declared more than once`)
174+
})
90175

91-
_, ok = configs.Find("WHATEVER", "NAME1")
92-
require.False(t, ok)
176+
t.Run("Unexpected key count", func(t *testing.T) {
177+
config := `
178+
plugins {
179+
"TYPE" "NAME" "BOGUS" {
180+
}
181+
}`
182+
root := struct {
183+
Plugins ast.Node
184+
}{}
185+
err := hcl.Decode(&root, config)
186+
require.NoError(t, err)
93187

94-
_, ok = configs.Find("TYPE1", "WHATEVER")
95-
require.False(t, ok)
188+
_, err = PluginConfigsFromHCLNode(root.Plugins)
189+
require.EqualError(t, err, `expected one or two keys on the plugin item for type "TYPE" but got 3`)
190+
})
96191
}

0 commit comments

Comments
 (0)