Skip to content

Commit

Permalink
Allow the --plugin flag to specify a directory of plugins (#911)
Browse files Browse the repository at this point in the history 
If someone specifies `sonobuoy run|gen --plugin X` then X ought to
be allowed to be either a single plugin file or a directory of plugins.

To avoid dropping errors intentionally or processing lots of unnecessary
files, when processing a directory, we only consider *.yaml files. We
also do not recurse deeper into the directory.

Fixes #906

Signed-off-by: John Schnake <[email protected]>
  • Loading branch information
@johnSchnake
johnSchnake authored Sep 27, 2019
1 parent d9ca4ba commit 06f638e
Show file tree
Hide file tree
Showing 8 changed files with 243 additions and 90 deletions.
2 changes: 1 addition & 1 deletion cmd/sonobuoy/app/gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (g *genFlags) Config() (*client.GenConfig, error) {

// the Enabled and Disabled modes of rbacmode don't need the client, so kubeclient can be nil.
// if kubeclient is needed, ErrRBACNoClient will be returned and that error can be reported back up.
rbacEnabled, err := genflags.rbacMode.Enabled(kubeclient)
rbacEnabled, err := g.rbacMode.Enabled(kubeclient)
if err != nil {
if errors.Cause(err) == ErrRBACNoClient {
return nil, errors.Wrap(err, kubeError.Error())
Expand Down
222 changes: 151 additions & 71 deletions cmd/sonobuoy/app/gen_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ limitations under the License.
package app

import (
"io/ioutil"
"os"
"reflect"
"strings"
"testing"
Expand All @@ -22,6 +24,10 @@ import (
"github.com/heptio/sonobuoy/pkg/client"
"github.com/heptio/sonobuoy/pkg/config"
"github.com/heptio/sonobuoy/pkg/plugin"
"github.com/heptio/sonobuoy/pkg/plugin/manifest"

"github.com/kylelemons/godebug/pretty"
v1 "k8s.io/api/core/v1"
)

const (
Expand Down Expand Up @@ -85,19 +91,30 @@ func TestResolveConformanceImage(t *testing.T) {
func TestResolveConfig(t *testing.T) {
defaultPluginSearchPath := config.New().PluginSearchPath
defaultAggr := plugin.AggregationConfig{TimeoutSeconds: 10800}
dynamicConfigFileName := "*determinedAtRuntime*"

tcs := []struct {
name string
input *genFlags
name string

// CLI input to parse.
input string

// Not every field of this will be tested at this time.
expected *config.Config
cliInput string

// If specified, will write the config to a temp file then append
// the `--config=tmpfile` to the input. This way we can keep the config files
// small and with the test rather than in a testdata file.
configFileContents string

// TODO(jschnake): This test previously was just testing the config.Config
// and only certain fields. We may consider expanding this to testing the entire
// genConfig object but for now I am expanding this just to ensure proper plugin loading.
expectedGenConfigPlugins *client.GenConfig
}{
{
name: "NonDisruptiveConformance mode when supplied config is nil (nothing interesting happens)",
input: &genFlags{
mode: client.NonDisruptiveConformance,
sonobuoyConfig: SonobuoyConfig{},
},
name: "NonDisruptiveConformance mode when supplied config is nil (nothing interesting happens)",
input: "",
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
Expand All @@ -110,19 +127,13 @@ func TestResolveConfig(t *testing.T) {
Aggregation: defaultAggr,
Resources: config.DefaultResources,
},
}, {
name: "Quick mode and a non-nil supplied config",
input: &genFlags{
mode: client.Quick,
sonobuoyConfig: SonobuoyConfig{
Config: config.Config{
Aggregation: plugin.AggregationConfig{
BindAddress: "10.0.0.1",
},
},
raw: rawInput,
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e", "systemd-logs"},
},
}, {
name: "Quick mode and a non-nil supplied config",
input: "--mode quick --config=" + dynamicConfigFileName,
configFileContents: `{"Server":{"bindaddress":"10.0.0.1"}}`,
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
Expand All @@ -138,19 +149,13 @@ func TestResolveConfig(t *testing.T) {
PluginSearchPath: defaultPluginSearchPath,
Resources: config.DefaultResources,
},
}, {
name: "NonDisruptiveConformance mode with plugin selection specified",
input: &genFlags{
mode: client.NonDisruptiveConformance,
sonobuoyConfig: SonobuoyConfig{
Config: config.Config{
PluginSelections: []plugin.Selection{
{Name: "systemd-logs"},
},
},
raw: "not empty",
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e"},
},
}, {
name: "NonDisruptiveConformance mode with plugin selection specified",
input: "--plugin systemd-logs --config=" + dynamicConfigFileName,
configFileContents: `{"Plugins":[{"name":"systemd-logs"}]}`,
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
Expand All @@ -166,14 +171,13 @@ func TestResolveConfig(t *testing.T) {
},
Resources: config.DefaultResources,
},
}, {
name: "Flags should override the config settings when set",
input: &genFlags{
sonobuoyConfig: SonobuoyConfig{
Config: config.Config{Namespace: "configNS"},
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"systemd-logs"},
},
cliInput: "--namespace=flagNS --sonobuoy-image=flagImage --image-pull-policy=Always --timeout 100",
}, {
name: "NS flag prioritized over config value",
configFileContents: `{"Namespace":"configNS","WorkerImage":"configImage","ImagePullPolicy":"IfNotPresent","Server":{"timeoutseconds":999}}`,
input: "--namespace=flagNS --sonobuoy-image=flagImage --image-pull-policy=Always --timeout 100 --config=" + dynamicConfigFileName,
expected: &config.Config{
Namespace: "flagNS",
WorkerImage: "flagImage",
Expand All @@ -186,14 +190,17 @@ func TestResolveConfig(t *testing.T) {
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 100},
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e", "systemd-logs"},
},
}, {
name: "Flags shouldn't override the config settings unless set",
input: &genFlags{},
cliInput: "--sonobuoy-image=flagImage --config testdata/sonobuoy.conf",
name: "Worker image and pull policy flags prioritized over config values",
input: "--sonobuoy-image=flagImage --image-pull-policy=Always --config " + dynamicConfigFileName,
configFileContents: `{"Namespace":"configNS","WorkerImage":"configImage","ImagePullPolicy":"Never","Server":{"timeoutseconds":500}}`,
expected: &config.Config{
Namespace: "configNS",
WorkerImage: "flagImage",
ImagePullPolicy: "Never",
ImagePullPolicy: "Always",
PluginSelections: []plugin.Selection{
{Name: "e2e"},
{Name: "systemd-logs"},
Expand All @@ -202,10 +209,13 @@ func TestResolveConfig(t *testing.T) {
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 500},
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e", "systemd-logs"},
},
}, {
name: "Flags shouldn't override the config settings unless set",
input: &genFlags{},
cliInput: "--config testdata/sonobuoy.conf",
name: "Default flag values dont override config values",
input: "--config " + dynamicConfigFileName,
configFileContents: `{"Namespace":"configNS","WorkerImage":"configImage","ImagePullPolicy":"Never","Server":{"timeoutseconds":500}}`,
expected: &config.Config{
Namespace: "configNS",
WorkerImage: "configImage",
Expand All @@ -218,10 +228,12 @@ func TestResolveConfig(t *testing.T) {
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 500},
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e", "systemd-logs"},
},
}, {
name: "Manually specified plugins should result in empty selection",
input: &genFlags{},
cliInput: "--plugin e2e",
name: "Manually specified plugins should result in empty selection",
input: "--plugin e2e",
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
Expand All @@ -231,10 +243,12 @@ func TestResolveConfig(t *testing.T) {
Aggregation: defaultAggr,
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
DynamicPlugins: []string{"e2e"},
},
}, {
name: "Empty, non-nil resources and plugins should be preserved",
input: &genFlags{},
cliInput: "--config testdata/emptyQueryAndPlugins.conf",
name: "Empty, non-nil resources and plugins should be preserved",
input: "--config testdata/emptyQueryAndPlugins.conf",
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
Expand All @@ -244,44 +258,110 @@ func TestResolveConfig(t *testing.T) {
PluginSelections: []plugin.Selection{},
Resources: []string{},
},
expectedGenConfigPlugins: &client.GenConfig{},
}, {
name: "Plugins can be loaded by directory",
input: "--plugin testdata/testPluginDir",
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
ImagePullPolicy: "IfNotPresent",
PluginSelections: nil,
PluginSearchPath: defaultPluginSearchPath,
Aggregation: defaultAggr,
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
StaticPlugins: []*manifest.Manifest{
{SonobuoyConfig: manifest.SonobuoyConfig{Driver: "Job", PluginName: "plugin1", SkipCleanup: false, ResultFormat: "raw"}, Spec: manifest.Container{Container: v1.Container{Name: "plugin", Image: "foo/bar:v1", Command: []string{"./run.sh"}, VolumeMounts: []v1.VolumeMount{{ReadOnly: false, Name: "results", MountPath: plugin.ResultsDir}}}}},
{SonobuoyConfig: manifest.SonobuoyConfig{Driver: "Job", PluginName: "plugin2", SkipCleanup: false, ResultFormat: "raw"}, Spec: manifest.Container{Container: v1.Container{Name: "plugin", Image: "foo/bar:v2", Command: []string{"./run.sh"}, VolumeMounts: []v1.VolumeMount{{ReadOnly: false, Name: "results", MountPath: plugin.ResultsDir}}}}},
},
},
}, {
name: "Plugins loaded by directory and by name",
input: "--plugin e2e --plugin testdata/testPluginDir --plugin testdata/testPluginDir/pluginNotYAML.ext",
expected: &config.Config{
Namespace: "sonobuoy",
WorkerImage: "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
ImagePullPolicy: "IfNotPresent",
PluginSelections: nil,
PluginSearchPath: defaultPluginSearchPath,
Aggregation: defaultAggr,
Resources: config.DefaultResources,
},
expectedGenConfigPlugins: &client.GenConfig{
StaticPlugins: []*manifest.Manifest{
{SonobuoyConfig: manifest.SonobuoyConfig{Driver: "Job", PluginName: "plugin1", SkipCleanup: false, ResultFormat: "raw"}, Spec: manifest.Container{Container: v1.Container{Name: "plugin", Image: "foo/bar:v1", Command: []string{"./run.sh"}, VolumeMounts: []v1.VolumeMount{{ReadOnly: false, Name: "results", MountPath: plugin.ResultsDir}}}}},
{SonobuoyConfig: manifest.SonobuoyConfig{Driver: "Job", PluginName: "plugin2", SkipCleanup: false, ResultFormat: "raw"}, Spec: manifest.Container{Container: v1.Container{Name: "plugin", Image: "foo/bar:v2", Command: []string{"./run.sh"}, VolumeMounts: []v1.VolumeMount{{ReadOnly: false, Name: "results", MountPath: plugin.ResultsDir}}}}},
{SonobuoyConfig: manifest.SonobuoyConfig{Driver: "Job", PluginName: "plugin3", SkipCleanup: false, ResultFormat: "raw"}, Spec: manifest.Container{Container: v1.Container{Name: "plugin", Image: "foo/bar:v3", Command: []string{"./run.sh"}, VolumeMounts: []v1.VolumeMount{{ReadOnly: false, Name: "results", MountPath: plugin.ResultsDir}}}}},
},
DynamicPlugins: []string{"e2e"},
},
},
}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
// Simulate parsing of input via CLI. Making this optional to avoid complicating
// setup for other tests which just explicitly set the values.
if len(tc.cliInput) > 0 {
fs := GenFlagSet(tc.input, EnabledRBACMode)
if err := fs.Parse(strings.Split(tc.cliInput, " ")); err != nil {
t.Fatalf("Failed to parse CLI input %q: %v", tc.cliInput, err)
// Write and load a config file
if len(tc.configFileContents) > 0 {
tmpFile, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("Failed to create temp file for test: %v", err)
}
defer os.Remove(tmpFile.Name())
err = ioutil.WriteFile(tmpFile.Name(), []byte(tc.configFileContents), 0666)
if err != nil {
t.Fatalf("Failed to write temp config file for test: %v", err)
}
tc.input = strings.Replace(tc.input, dynamicConfigFileName, tmpFile.Name(), -1)
}

// Simulate parsing of input via CLI.
gflagset := &genFlags{}
fs := GenFlagSet(gflagset, EnabledRBACMode)
if err := fs.Parse(strings.Split(tc.input, " ")); err != nil {
t.Fatalf("Failed to parse CLI input %q: %v", tc.input, err)
}

conf := tc.input.resolveConfig()
// Manually set KubeConformanceImage for all tests so that it does not error due to having 'auto' image
// without a real cluster to target for version info
gflagset.kubeConformanceImage = "testOnly"

genConfig, err := gflagset.Config()
if err != nil {
t.Fatalf("Failed to generate GenConfig: %v", err)
}

if genConfig.Config.Namespace != tc.expected.Namespace {
t.Errorf("Expected namespace %v but got %v", tc.expected.Namespace, genConfig.Config.Namespace)
}

if genConfig.Config.WorkerImage != tc.expected.WorkerImage {
t.Errorf("Expected worker image %v but got %v", tc.expected.WorkerImage, genConfig.Config.WorkerImage)
}

if conf.Namespace != tc.expected.Namespace {
t.Errorf("Expected namespace %v but got %v", tc.expected.Namespace, conf.Namespace)
if genConfig.Config.ImagePullPolicy != tc.expected.ImagePullPolicy {
t.Errorf("Expected image pull policy %v but got %v", tc.expected.ImagePullPolicy, genConfig.Config.ImagePullPolicy)
}

if conf.WorkerImage != tc.expected.WorkerImage {
t.Errorf("Expected worker image %v but got %v", tc.expected.WorkerImage, conf.WorkerImage)
if genConfig.Config.Aggregation.TimeoutSeconds != tc.expected.Aggregation.TimeoutSeconds {
t.Errorf("Expected timeout %v but got %v", tc.expected.Aggregation.TimeoutSeconds, genConfig.Config.Aggregation.TimeoutSeconds)
}

if conf.ImagePullPolicy != tc.expected.ImagePullPolicy {
t.Errorf("Expected image pull policy %v but got %v", tc.expected.ImagePullPolicy, conf.ImagePullPolicy)
if !reflect.DeepEqual(genConfig.Config.PluginSelections, tc.expected.PluginSelections) {
t.Errorf("expected PluginSelections %v but got %v", tc.expected.PluginSelections, genConfig.Config.PluginSelections)
}

if conf.Aggregation.TimeoutSeconds != tc.expected.Aggregation.TimeoutSeconds {
t.Errorf("Expected timeout %v but got %v", tc.expected.Aggregation.TimeoutSeconds, conf.Aggregation.TimeoutSeconds)
if !reflect.DeepEqual(genConfig.Config.Resources, tc.expected.Resources) {
t.Errorf("expected resources %v but got %v", tc.expected.Resources, genConfig.Config.Resources)
}

if !reflect.DeepEqual(conf.PluginSelections, tc.expected.PluginSelections) {
t.Errorf("expected PluginSelections %v but got %v", tc.expected.PluginSelections, conf.PluginSelections)
if diff := pretty.Compare(genConfig.StaticPlugins, tc.expectedGenConfigPlugins.StaticPlugins); diff != "" {
t.Errorf("expected static plugins to match but got diff:\n%s\n", diff)
}

if !reflect.DeepEqual(conf.Resources, tc.expected.Resources) {
t.Errorf("expected resources %v but got %v", tc.expected.Resources, conf.Resources)
if diff := pretty.Compare(genConfig.DynamicPlugins, tc.expectedGenConfigPlugins.DynamicPlugins); diff != "" {
t.Errorf("expected dynamic plugins to match but got diff:\n%s\n", diff)
}
})
}
Expand Down
Loading

0 comments on commit 06f638e

Please sign in to comment.