Clarify that users shouldn't provide more options when running a file (…

…#1053)

If using the --file option for `sonobuoy run` they shouldn't also
try and set other options.

This is because we are not able to take the file output and turn
that into a genConfig object which can be merged with the other
command line options.

When in the expanded file format, we are unsure if which parts
should be merged/how.

These are meant to be exclusive options.

Fixes #1027

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

cmd/sonobuoy/app/args.go

@@ -40,6 +40,8 @@ const (
 	pluginFlag          = "plugin"
 	timeoutFlag         = "timeout"
 	waitOutputFlag      = "wait-output"
+	kubeconfig          = "kubeconfig"
+	context             = "context"
 )
 
 // AddNamespaceFlag initialises a namespace flag.

cmd/sonobuoy/app/run.go

@@ -17,6 +17,7 @@ limitations under the License.
 package app
 
 import (
+	"fmt"
 	"os"
 	"time"
 
@@ -36,7 +37,11 @@ type runFlags struct {
 	genFile       string
 }
 
-var runflags runFlags
+var (
+	runflags runFlags
+
+	allowedGenFlagsWithRunFile = []string{kubeconfig, context}
+)
 
 func RunFlagSet(cfg *runFlags) *pflag.FlagSet {
 	runset := pflag.NewFlagSet("run", pflag.ExitOnError)
@@ -60,6 +65,10 @@ func (r *runFlags) Config() (*client.RunConfig, error) {
 		GenFile:    r.genFile,
 	}
 
+	if r.genFile != "" && givenAnyGenConfigFlags(&(r.genFlags), allowedGenFlagsWithRunFile) {
+		return nil, fmt.Errorf("setting the --file flag is incompatible with any other options")
+	}
+
 	if r.genFile == "" {
 		gencfg, err := r.genFlags.Config()
 		if err != nil {
@@ -71,6 +80,19 @@ func (r *runFlags) Config() (*client.RunConfig, error) {
 	return runcfg, nil
 }
 
+func givenAnyGenConfigFlags(gf *genFlags, whitelistFlagNames []string) bool {
+	changed := false
+	gf.genflags.Visit(func(f *pflag.Flag) {
+		if changed {
+			return
+		}
+		if f.Changed && !stringInList(whitelistFlagNames, f.Name) {
+			changed = true
+		}
+	})
+	return changed
+}
+
 func NewCmdRun() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "run",
@@ -116,3 +138,12 @@ func submitSonobuoyRun(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 }
+
+func stringInList(list []string, s string) bool {
+	for _, v := range list {
+		if v == s {
+			return true
+		}
+	}
+	return false
+}

cmd/sonobuoy/app/run_test.go

@@ -0,0 +1,69 @@
+/*
+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 (
+	"testing"
+)
+
+func TestGivenAnyGenConfigFlags(t *testing.T) {
+	getSampleFlagsWithChanged := func(s []string) *genFlags {
+		sampleFlags := &genFlags{}
+		GenFlagSet(sampleFlags, DetectRBACMode)
+		for _, v := range s {
+			sampleFlags.genflags.Set(v, "foo")
+		}
+		return sampleFlags
+	}
+
+	testCases := []struct {
+		desc      string
+		inFlags   *genFlags
+		whitelist []string
+		expect    bool
+	}{
+		{
+			desc:      "Nothing changed return true",
+			inFlags:   getSampleFlagsWithChanged(nil),
+			whitelist: []string{},
+			expect:    false,
+		}, {
+			desc:      "One changed flag return true",
+			inFlags:   getSampleFlagsWithChanged([]string{"kubeconfig"}),
+			whitelist: []string{},
+			expect:    true,
+		}, {
+			desc:      "One changed flag return false if in whitelist",
+			inFlags:   getSampleFlagsWithChanged([]string{"kubeconfig"}),
+			whitelist: []string{"kubeconfig"},
+			expect:    false,
+		}, {
+			desc:      "One changed flag return true if not in whitelist",
+			inFlags:   getSampleFlagsWithChanged([]string{"e2e-focus"}),
+			whitelist: []string{"flaga", "flagb", "flagc"},
+			expect:    true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			out := givenAnyGenConfigFlags(tc.inFlags, tc.whitelist)
+			if out != tc.expect {
+				t.Errorf("Expected %v but got %v", tc.expect, out)
+			}
+		})
+	}
+}