Skip to content

Commit

Permalink
plugins/discovery: Improve error message about prohibited config
Browse files Browse the repository at this point in the history
Previously the discovery plugin would just log a generic error if any
plugins were enabled. With this commit, it will log an error that
mentions which plugins were enabled, making the source of the problem
a bit more obvious.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall authored and anderseknert committed Dec 2, 2021
1 parent 37d2025 commit b5212fc
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 2 deletions.
21 changes: 21 additions & 0 deletions config/config.go
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"os"
"path/filepath"
"sort"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/internal/ref"
Expand Down Expand Up @@ -44,7 +45,27 @@ func ParseConfig(raw []byte, id string) (*Config, error) {
return &result, result.validateAndInjectDefaults(id)
}

// PluginNames returns a sorted list of names of enabled plugins.
func (c Config) PluginNames() (result []string) {
if c.Bundle != nil || c.Bundles != nil {
result = append(result, "bundles")
}
if c.Status != nil {
result = append(result, "status")
}
if c.DecisionLogs != nil {
result = append(result, "decision_logs")
}
for name := range c.Plugins {
result = append(result, name)
}
sort.Strings(result)
return result
}

// PluginsEnabled returns true if one or more plugin features are enabled.
//
// Deprecated. Use PluginNames instead.
func (c Config) PluginsEnabled() bool {
return c.Bundle != nil || c.Bundles != nil || c.DecisionLogs != nil || c.Status != nil || len(c.Plugins) > 0
}
Expand Down
68 changes: 68 additions & 0 deletions config/config_test.go
Expand Up @@ -16,6 +16,74 @@ import (
"github.com/open-policy-agent/opa/version"
)

func TestConfigPluginNames(t *testing.T) {
tests := []struct {
name string
conf Config
expected []string
}{
{
name: "empty config",
conf: Config{},
expected: nil,
},
{
name: "bundle",
conf: Config{
Bundle: []byte(`{"bundle": {"name": "test-bundle"}}`),
},
expected: []string{"bundles"},
},
{
name: "bundles",
conf: Config{
Bundles: []byte(`{"bundles": {"test-bundle": {}}`),
},
expected: []string{"bundles"},
},
{
name: "decision_logs",
conf: Config{
DecisionLogs: []byte(`{decision_logs: {}}`),
},
expected: []string{"decision_logs"},
},
{
name: "status",
conf: Config{
Status: []byte(`{status: {}}`),
},
expected: []string{"status"},
},
{
name: "plugins",
conf: Config{
Plugins: map[string]json.RawMessage{
"some-plugin": {},
},
},
expected: []string{"some-plugin"},
},
{
name: "sorted",
conf: Config{
DecisionLogs: []byte(`{decision_logs: {}}`),
Status: []byte(`{status: {}}`),
},
expected: []string{"decision_logs", "status"},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := test.conf.PluginNames()
if !reflect.DeepEqual(actual, test.expected) {
t.Errorf("Expected %v but got %v", test.expected, actual)
}
})
}
}

func TestConfigPluginsEnabled(t *testing.T) {
tests := []struct {
name string
Expand Down
5 changes: 3 additions & 2 deletions plugins/discovery/discovery.go
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"encoding/json"
"fmt"
"strings"
"sync"

"github.com/open-policy-agent/opa/ast"
Expand Down Expand Up @@ -84,8 +85,8 @@ func New(manager *plugins.Manager, opts ...func(*Discovery)) (*Discovery, error)
return result, nil
}

if manager.Config.PluginsEnabled() {
return nil, fmt.Errorf("plugins cannot be specified in the bootstrap configuration when discovery enabled")
if names := manager.Config.PluginNames(); len(names) > 0 {
return nil, fmt.Errorf("discovery prohibits manual configuration of %v", strings.Join(names, " and "))
}

result.config = config
Expand Down

0 comments on commit b5212fc

Please sign in to comment.