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.

Sign up for GitHub

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

Fix plugin config parsing code #3939

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Fix plugin config parsing code #3939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a6c1866
Fix plugin config parsing code
azdagron Mar 2, 2023
39c8293
Add another test case
azdagron Mar 2, 2023
c499d4e
Merge branch 'main' into fix-plugin-parsing
evan2645 Mar 2, 2023
3bded89
Merge branch 'main' into fix-plugin-parsing
MarcosDY Mar 2, 2023
d56282d
Merge branch 'main' into fix-plugin-parsing
azdagron Mar 2, 2023
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
95 changes: 77 additions & 18 deletions pkg/common/catalog/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,37 +74,96 @@ func PluginConfigsFromHCLNode(pluginsNode ast.Node) (PluginConfigs, error) {
if pluginsNode == nil {
return nil, errors.New("plugins node is nil")
}
pluginsList, ok := pluginsNode.(*ast.ObjectList)
if !ok {
return nil, fmt.Errorf("expected plugins node type %T but got %T", pluginsList, pluginsNode)

// The seen set is used to detect multiple declarations for the same plugin.
type key struct {
Type string
Name string
}
seen := make(map[key]struct{})

var pluginConfigs PluginConfigs
for _, pluginObject := range pluginsList.Items {
if len(pluginObject.Keys) != 2 {
return nil, fmt.Errorf("plugin item expected to have two keys (type then name)")
appendPlugin := func(pluginType, pluginName string, pluginNode ast.Node) error {
// Ensure a plugin for a given type and name is only declared once.
k := key{Type: pluginType, Name: pluginName}
if _, ok := seen[k]; ok {
return fmt.Errorf("plugin %q/%q declared more than once", pluginType, pluginName)
}
seen[k] = struct{}{}

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

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

var hclPluginConfig hclPluginConfig
if err := hcl.DecodeObject(&hclPluginConfig, pluginObject.Val); err != nil {
return nil, fmt.Errorf("failed to decode plugin config for %q/%q: %w", pluginType, pluginName, err)
}
pluginConfigs = append(pluginConfigs, pluginConfig)
return nil
}

pluginConfig, err := pluginConfigFromHCL(pluginType, pluginName, hclPluginConfig)
pluginsList, ok := pluginsNode.(*ast.ObjectList)
if !ok {
return nil, fmt.Errorf("expected plugins node type %T but got %T", pluginsList, pluginsNode)
}

for _, pluginObject := range pluginsList.Items {
pluginType, err := stringFromToken(pluginObject.Keys[0].Token)
if err != nil {
return nil, fmt.Errorf("failed to create plugin config for %q/%q: %w", pluginType, pluginName, err)
return nil, fmt.Errorf("invalid plugin type key %q: %w", pluginObject.Keys[0].Token.Text, err)
}

pluginConfigs = append(pluginConfigs, pluginConfig)
var pluginName string
switch len(pluginObject.Keys) {
case 1:
// One key is expected when using JSON formatted config:
// "plugins": [
// "NodeAttestor": [
// {
// "foo": ...
// }
// ]
// ]
//
// Or the slightly more verbose HCL form:
// plugins {
// "NodeAttestor" = {
// "foo" = { ... }
// }
// }
pluginTypeList, ok := pluginObject.Val.(*ast.ObjectType)
if !ok {
return nil, fmt.Errorf("expected plugin object type %T but got %T", pluginTypeList, pluginObject.Val)
}
for _, pluginObject := range pluginTypeList.List.Items {
pluginName, err = stringFromToken(pluginObject.Keys[0].Token)
if err != nil {
return nil, fmt.Errorf("invalid plugin type name %q: %w", pluginObject.Keys[0].Token.Text, err)
}
if err := appendPlugin(pluginType, pluginName, pluginObject.Val); err != nil {
return nil, err
}
}
case 2:
// Two keys are expected for something like this (our typical config example)
// plugins {
// NodeAttestor "foo" {
// ...
// }
// }
pluginName, err = stringFromToken(pluginObject.Keys[1].Token)
if err != nil {
return nil, fmt.Errorf("invalid plugin type name %q: %w", pluginObject.Keys[1].Token.Text, err)
}
if err := appendPlugin(pluginType, pluginName, pluginObject.Val); err != nil {
return nil, err
}
default:
return nil, fmt.Errorf("expected one or two keys on the plugin item for type %q but got %d", pluginType, len(pluginObject.Keys))
}
}
return pluginConfigs, nil
}
Expand Down
245 changes: 170 additions & 75 deletions pkg/common/catalog/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,88 +9,183 @@ import (
)

func TestParsePluginConfigsFromHCLNode(t *testing.T) {
root := struct {
Plugins ast.Node
}{}
err := hcl.Decode(&root, `plugins {
TYPE1 "NAME3" {
plugin_data = "DATA3"
enabled = false
}
TYPE4 "NAME4" {
}
TYPE1 "NAME1" {
plugin_cmd = "CMD1"
plugin_data = "DATA1"
}
TYPE2 "NAME2" {
plugin_cmd = "CMD2"
plugin_args = ["foo", "bar", "baz"]
plugin_checksum = "CHECKSUM2"
plugin_data = "DATA2"
enabled = true
}
}`)
require.NoError(t, err)
test := func(t *testing.T, configIn string) {
root := struct {
Plugins ast.Node
}{}
err := hcl.Decode(&root, configIn)
require.NoError(t, err)

configs, err := PluginConfigsFromHCLNode(root.Plugins)
require.NoError(t, err)
configs, err := PluginConfigsFromHCLNode(root.Plugins)
require.NoError(t, err)

pluginA := PluginConfig{
Name: "NAME3",
Type: "TYPE1",
Data: `"DATA3"`,
Disabled: true,
}
pluginB := PluginConfig{
Name: "NAME4",
Type: "TYPE4",
}
pluginC := PluginConfig{
Name: "NAME1",
Type: "TYPE1",
Path: "CMD1",
Data: `"DATA1"`,
Disabled: false,
}
pluginD := PluginConfig{
Name: "NAME2",
Type: "TYPE2",
Path: "CMD2",
Args: []string{"foo", "bar", "baz"},
Checksum: "CHECKSUM2",
Data: `"DATA2"`,
Disabled: false,
}
pluginA := PluginConfig{
Name: "NAME3",
Type: "TYPE1",
Data: `"DATA3"`,
Disabled: true,
}
pluginB := PluginConfig{
Name: "NAME4",
Type: "TYPE4",
}
pluginC := PluginConfig{
Name: "NAME1",
Type: "TYPE1",
Path: "CMD1",
Data: `"DATA1"`,
Disabled: false,
}
pluginD := PluginConfig{
Name: "NAME2",
Type: "TYPE2",
Path: "CMD2",
Args: []string{"foo", "bar", "baz"},
Checksum: "CHECKSUM2",
Data: `"DATA2"`,
Disabled: false,
}

// The declaration order should be preserved.
require.Equal(t, PluginConfigs{
pluginA,
pluginB,
pluginC,
pluginD,
}, configs)
// The declaration order should be preserved.
require.Equal(t, PluginConfigs{
pluginA,
pluginB,
pluginC,
pluginD,
}, configs)

// Only A and C are of type TYPE1
matching, remaining := configs.FilterByType("TYPE1")

require.Equal(t, PluginConfigs{
pluginA,
pluginC,
}, matching)

require.Equal(t, PluginConfigs{
pluginB,
pluginD,
}, remaining)

c, ok := configs.Find("TYPE1", "NAME1")
require.Equal(t, pluginC, c)
require.True(t, ok)

_, ok = configs.Find("WHATEVER", "NAME1")
require.False(t, ok)

_, ok = configs.Find("TYPE1", "WHATEVER")
require.False(t, ok)
}

// Only A and C are of type TYPE1
matching, remaining := configs.FilterByType("TYPE1")
t.Run("HCL", func(t *testing.T) {
config := `
plugins {
TYPE1 "NAME3" {
plugin_data = "DATA3"
enabled = false
}
TYPE4 "NAME4" {
}
TYPE1 "NAME1" {
plugin_cmd = "CMD1"
plugin_data = "DATA1"
}
TYPE2 "NAME2" {
plugin_cmd = "CMD2"
plugin_args = ["foo", "bar", "baz"]
plugin_checksum = "CHECKSUM2"
plugin_data = "DATA2"
enabled = true
}
}
`
test(t, config)
})

require.Equal(t, PluginConfigs{
pluginA,
pluginC,
}, matching)
t.Run("JSON", func(t *testing.T) {
config := `{
"plugins": {
"TYPE1": [
{
"NAME3": {
"plugin_data": "DATA3",
"enabled": false
}
}
],
"TYPE4": [
{
"NAME4": [
{
}
]
}
],
"TYPE1": [
{
"NAME1": [
{
"plugin_cmd": "CMD1",
"plugin_data": "DATA1"
}
]
}
],
"TYPE2": [
{
"NAME2": [
{
"plugin_cmd": "CMD2",
"plugin_args": ["foo", "bar", "baz"],
"plugin_checksum": "CHECKSUM2",
"plugin_data": "DATA2",
"enabled": true
}
]
}
]
}
}`
test(t, config)
})

require.Equal(t, PluginConfigs{
pluginB,
pluginD,
}, remaining)
t.Run("Plugin declared more than once", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @azdagron what do you think about having an extra test case for an invalid number of keys here?

	t.Run("Invalid number of keys", func(t *testing.T) {
		config := `
			plugins {
				KEY1 "KEY2" KEY3 {
					plugin_data = "DATA3"
					enabled = false
				}
			}
		`
		root := struct {
			Plugins ast.Node
		}{}
		err := hcl.Decode(&root, config)
		require.NoError(t, err)

		_, err = PluginConfigsFromHCLNode(root.Plugins)
		require.EqualError(t, err, "expected one or two keys on the plugin item but got 3")
	})

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. Added.

config := `{
"plugins": {
"TYPE": [
{
"NAME": {}
},
{
"NAME": {}
}
]
}
}`
root := struct {
Plugins ast.Node
}{}
err := hcl.Decode(&root, config)
require.NoError(t, err)

c, ok := configs.Find("TYPE1", "NAME1")
require.Equal(t, pluginC, c)
require.True(t, ok)
_, err = PluginConfigsFromHCLNode(root.Plugins)
require.EqualError(t, err, `plugin "TYPE"/"NAME" declared more than once`)
})

_, ok = configs.Find("WHATEVER", "NAME1")
require.False(t, ok)
t.Run("Unexpected key count", func(t *testing.T) {
config := `
plugins {
"TYPE" "NAME" "BOGUS" {
}
}`
root := struct {
Plugins ast.Node
}{}
err := hcl.Decode(&root, config)
require.NoError(t, err)

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