Add plugin flag for custom, local plugins

The user can now load custom plugin manifests into the
YAML that Sonobuoy will run with the --plugin flag.

For backwards compatibility, if no plugins are specified
in this manner and instead a PluginSelection object is
still specified, we will fallback to adding the necessary
plugins.

When loading plugins using the flag, they should specify
either a filename which points to a plugin manifest or
one of the sentinal values representing our internal plugins:
e2e or systemd-logs.

Plugins must be uniquely defined by the plugin name.

Fixes #405

Signed-off-by: John Schnake <jschnake@vmware.com>

cmd/sonobuoy/app/args.go

@@ -281,6 +281,12 @@ func AddSSHUserFlag(user *string, flags *pflag.FlagSet) {
 	)
 }
 
+// AddPluginSetFlag adds the flag for gen/run which keeps track of which plugins
+// to run and loads them from local files if necessary.
+func AddPluginSetFlag(p *pluginList, flags *pflag.FlagSet) {
+	flags.VarP(p, "plugin", "p", "Which plugins to run. Can either point to a local file or be one of the known plugins (e2e or systemd-logs). Can be specified multiple times to run multiple plugins.")
+}
+
 // AddShortFlag adds a boolean flag to just print the Sonobuoy version and
 // nothing else. Useful in scripts.
 func AddShortFlag(flag *bool, flags *pflag.FlagSet) {

cmd/sonobuoy/app/gen.go

@@ -47,6 +47,10 @@ type genFlags struct {
 	imagePullPolicy             ImagePullPolicy
 	e2eRepoList                 string
 
+	// plugins will keep a list of the plugins we want. Custom type for
+	// flag support.
+	plugins pluginList
+
 	// These two fields are here since to properly squash settings down into nested
 	// configs we need to tell whether or not values are default values or the user
 	// provided them on the command line/config file.
@@ -72,6 +76,7 @@ func GenFlagSet(cfg *genFlags, rbac RBACMode) *pflag.FlagSet {
 	AddSSHKeyPathFlag(&cfg.sshKeyPath, genset)
 	AddSSHUserFlag(&cfg.sshUser, genset)
 
+	AddPluginSetFlag(&cfg.plugins, genset)
 	cfg.genflags = genset
 	return genset
 }
@@ -128,14 +133,16 @@ func (g *genFlags) Config() (*client.GenConfig, error) {
 
 	return &client.GenConfig{
 		E2EConfig:            e2ecfg,
-		Config:               g.getConfig(),
+		Config:               g.resolveConfig(),
 		Image:                g.sonobuoyImage,
 		Namespace:            g.namespace,
 		EnableRBAC:           rbacEnabled,
 		ImagePullPolicy:      g.imagePullPolicy.String(),
 		KubeConformanceImage: image,
 		SSHKeyPath:           g.sshKeyPath,
 		SSHUser:              g.sshUser,
+		DynamicPlugins:       g.plugins.DynamicPlugins,
+		StaticPlugins:        g.plugins.StaticPlugins,
 	}, nil
 }
 
@@ -209,31 +216,44 @@ func getClient(kubeconfig *Kubeconfig) (*kubernetes.Clientset, error) {
 	return client, kubeError
 }
 
-// getConfig creates a config with the following algorithm:
-// If no config is supplied defaults will be returned.
-// If a config is supplied then the default values will be merged into the supplied config
-//   in order to allow users to supply a minimal config that will still work.
-// Lastly, options provided on the command line will override
-//   any values in the config.
-func (g *genFlags) getConfig() *config.Config {
+// getConfig generates a config which has the the following rules:
+//  - command line options override config values
+//  - plugins specified manually via flags specifically override plugins implied by mode flag
+//  - config values override default values
+// NOTE: Since it mutates plugin values, it should be called before using them.
+func (g *genFlags) resolveConfig() *config.Config {
 	if g == nil {
 		return config.New()
 	}
 
 	conf := config.New()
-
 	suppliedConfig := g.sonobuoyConfig.Get()
 	if suppliedConfig != nil {
 		// Provide defaults but don't overwrite any customized configuration.
 		mergo.Merge(suppliedConfig, conf)
 		conf = suppliedConfig
 	}
 
-	// if there are no plugins yet, set some based on the mode, otherwise use whatever was supplied.
-	if len(conf.PluginSelections) == 0 {
-		modeConfig := g.mode.Get()
-		if modeConfig != nil {
-			conf.PluginSelections = modeConfig.Selectors
+	// Resolve plugins.
+	//  - If using the plugin flags, no actions needed.
+	//  - Otherwise use the supplied config and mode to figure out the plugins to run.
+	//    This only works for e2e/systemd-logs which are internal plugins so then "Set" them
+	//    as if they were provided on the cmdline.
+	// Gate the logic with a nil check because tests may not specify flags and intend the legacy logic.
+	if g.genflags == nil || !g.genflags.Changed("plugin") {
+		// Use legacy logic; conf.SelectedPlugins or mode if not set
+		if len(conf.PluginSelections) == 0 {
+			modeConfig := g.mode.Get()
+			if modeConfig != nil {
+				conf.PluginSelections = modeConfig.Selectors
+			}
+		}
+
+		// Set these values as if the user had requested the defaults.
+		if g.genflags != nil {
+			for _, v := range conf.PluginSelections {
+				g.genflags.Lookup("plugin").Value.Set(v.Name)
+			}
 		}
 	}
 

cmd/sonobuoy/app/gen_config.go

@@ -41,7 +41,7 @@ func genConfig() ([]byte, error) {
 	// as default plugin selection. We didn't want to wire this into
 	// the `config` package, but it will be a default value the CLI
 	// users expect.
-	c := genflags.getConfig()
+	c := genflags.resolveConfig()
 	b, err := json.Marshal(c)
 	return b, errors.Wrap(err, "unable to marshal configuration")
 }

cmd/sonobuoy/app/gen_test.go

@@ -77,7 +77,7 @@ func TestResolveConformanceImage(t *testing.T) {
 	}
 }
 
-func TestGetConfig(t *testing.T) {
+func TestResolveConfig(t *testing.T) {
 	defaultPluginSearchPath := config.New().PluginSearchPath
 
 	tcs := []struct {
@@ -205,6 +205,17 @@ func TestGetConfig(t *testing.T) {
 				},
 				PluginSearchPath: defaultPluginSearchPath,
 			},
+		}, {
+			name:     "Manually specified plugins should result in empty selection",
+			input:    &genFlags{},
+			cliInput: "--plugin e2e",
+			expected: &config.Config{
+				Namespace:        "heptio-sonobuoy",
+				WorkerImage:      "gcr.io/heptio-images/sonobuoy:" + buildinfo.Version,
+				ImagePullPolicy:  "IfNotPresent",
+				PluginSelections: nil,
+				PluginSearchPath: defaultPluginSearchPath,
+			},
 		},
 	}
 
@@ -219,7 +230,7 @@ func TestGetConfig(t *testing.T) {
 				}
 			}
 
-			conf := tc.input.getConfig()
+			conf := tc.input.resolveConfig()
 
 			if conf.Namespace != tc.expected.Namespace {
 				t.Errorf("Expected namespace %v but got %v", tc.expected.Namespace, conf.Namespace)

cmd/sonobuoy/app/pluginList.go

@@ -0,0 +1,94 @@
+/*
+Copyright Sonobuoy Contributors 2019
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package app
+
+import (
+	"io/ioutil"
+	"strings"
+
+	"github.com/heptio/sonobuoy/pkg/plugin/manifest"
+	"github.com/pkg/errors"
+	"github.com/spf13/pflag"
+
+	kuberuntime "k8s.io/apimachinery/pkg/runtime"
+)
+
+// pluginList represents a []manifest.Manifest objects describing plugins.
+type pluginList struct {
+	// StaticPlugins are plugins which do not depend on other values and can be
+	// written to YAML as-is.
+	StaticPlugins []*manifest.Manifest
+
+	// DynamicPlugins are ones which require all the other gen input in order to finalize.
+	// E.g. the e2e plugin was templated to use all those other values.
+	DynamicPlugins []string
+}
+
+const (
+	pluginE2E         = "e2e"
+	pluginSystemdLogs = "systemd-logs"
+)
+
+// Make sure pluginList implements Value properly
+var _ pflag.Value = &pluginList{}
+
+// String needed for pflag.Value
+func (p *pluginList) String() string {
+	pluginNames := make(
+		[]string,
+		len(p.DynamicPlugins)+len(p.StaticPlugins),
+		len(p.DynamicPlugins)+len(p.StaticPlugins),
+	)
+	for i := range p.StaticPlugins {
+		pluginNames[i] = p.StaticPlugins[i].SonobuoyConfig.PluginName
+	}
+	pluginNames = append(pluginNames, p.DynamicPlugins...)
+	return strings.Join(pluginNames, ",")
+}
+
+// Type needed for pflag.Value
+func (p *pluginList) Type() string { return "pluginList" }
+
+// Set sets the explicit path of the loader to the provided config file
+func (p *pluginList) Set(str string) error {
+	switch str {
+	case pluginE2E:
+		p.DynamicPlugins = append(p.DynamicPlugins, str)
+	case pluginSystemdLogs:
+		p.DynamicPlugins = append(p.DynamicPlugins, str)
+	default:
+		b, err := ioutil.ReadFile(str)
+		if err != nil {
+			return errors.Wrapf(err, "unable to read file '%v'", str)
+		}
+
+		newPlugin, err := loadManifest(b)
+		if err != nil {
+			return errors.Wrapf(err, "failed to load plugin file '%v'", str)
+		}
+
+		p.StaticPlugins = append(p.StaticPlugins, newPlugin)
+	}
+
+	return nil
+}
+
+func loadManifest(bytes []byte) (*manifest.Manifest, error) {
+	var def manifest.Manifest
+	err := kuberuntime.DecodeInto(manifest.Decoder, bytes, &def)
+	return &def, errors.Wrap(err, "couldn't decode yaml for plugin definition")
+}

cmd/sonobuoy/app/pluginList_test.go

@@ -0,0 +1,105 @@
+/*
+Copyright the Sonobuoy contributors 2019
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+     http://www.apache.org/licenses/LICENSE-2.0
+
+	 Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package app
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/heptio/sonobuoy/pkg/plugin/manifest"
+	"github.com/kylelemons/godebug/pretty"
+)
+
+func TestSetPluginList(t *testing.T) {
+	testCases := []struct {
+		desc      string
+		list      pluginList
+		input     string
+		expect    pluginList
+		expectErr string
+	}{
+		{
+			desc:      "empty filename",
+			expectErr: `unable to read file '': open : no such file or directory`,
+		}, {
+			desc:      "file does not exist",
+			input:     "no-file",
+			expectErr: `unable to read file 'no-file': open no-file: no such file or directory`,
+		}, {
+			desc:      "bad manifest",
+			input:     "testdata/badmanifest.yaml",
+			expectErr: `failed to load plugin file 'testdata/badmanifest.yaml': couldn't decode yaml for plugin definition: couldn't get version/kind; json parse error: json: cannot unmarshal string into Go value of type struct { APIVersion string "json:\"apiVersion,omitempty\""; Kind string "json:\"kind,omitempty\"" }`,
+		}, {
+			desc:   "loading e2e",
+			input:  "e2e",
+			list:   pluginList{},
+			expect: pluginList{DynamicPlugins: []string{"e2e"}},
+		}, {
+			desc:   "loading systemd-logs",
+			input:  "systemd-logs",
+			list:   pluginList{},
+			expect: pluginList{DynamicPlugins: []string{"systemd-logs"}},
+		}, {
+			desc:  "loading from file",
+			input: "testdata/goodmanifest.yaml",
+			list:  pluginList{},
+			expect: pluginList{StaticPlugins: []*manifest.Manifest{
+				&manifest.Manifest{SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "test"}},
+			}},
+		}, {
+			desc:  "dynamic and static",
+			input: "e2e",
+			list: pluginList{StaticPlugins: []*manifest.Manifest{
+				&manifest.Manifest{SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "test"}},
+			}},
+			expect: pluginList{
+				StaticPlugins: []*manifest.Manifest{
+					&manifest.Manifest{SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "test"}},
+				},
+				DynamicPlugins: []string{"e2e"},
+			},
+		}, {
+			desc:  "multiple dynamic",
+			input: "systemd-logs",
+			list:  pluginList{DynamicPlugins: []string{"e2e"}},
+			expect: pluginList{
+				DynamicPlugins: []string{"e2e", "systemd-logs"},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := tc.list.Set(tc.input)
+			switch {
+			case err != nil && len(tc.expectErr) == 0:
+				t.Fatalf("Expected nil error but got %q", err)
+			case err != nil && len(tc.expectErr) > 0:
+				if fmt.Sprint(err) != tc.expectErr {
+					t.Errorf("Expected error \n\t%q\nbut got\n\t%q", tc.expectErr, err)
+				}
+				return
+			case err == nil && len(tc.expectErr) > 0:
+				t.Fatalf("Expected error %q but got nil", tc.expectErr)
+			default:
+				// No error
+			}
+
+			if diff := pretty.Compare(tc.expect, tc.list); diff != "" {
+				t.Fatalf("\n\n%s\n", diff)
+			}
+		})
+	}
+}

cmd/sonobuoy/app/testdata/badmanifest.yaml

@@ -0,0 +1 @@
+not a valid manifest

cmd/sonobuoy/app/testdata/goodmanifest.yaml

@@ -0,0 +1,2 @@
+sonobuoy-config:
+  plugin-name: test