Repository: aquasecurity/kube-bench
Pull request #529 (2019-11-29 15:22:19)

Add run subcommand 
Adds `run` subcommand to partially cover what is discussed [here](https://github.com/aquasecurity/kube-bench/issues/494#issuecomment-559183325)

This works with the existing config files as follows:

* `kube-bench run` is equivalent to `kube-bench`
* `kube-bench run --sections master,node` is also equivalent to `kube-bench` 
* `kube-bench run --sections master` is equivalent to `kube-bench master`
* `kube-bench run --sections node` is equivalent to `kube-bench node`

I think that when we have new section files for CIS 1.5 we will be able to run, for example `kube-bench run --sections etcd` to run the tests in the etcd file. However, there will need to be settings in the `config.yaml` file(s) to cover `etcd` (just as we currently need settings for `master` and `node`. 

This PR does not attempt to do the auto-detection part described in the proposal in #494 

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=h1) Report
> Merging [#529](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=desc) into [master](https://codecov.io/gh/aquasecurity/kube-bench/commit/8780e5cb5920bf03a1ccd50becc80be59887845f?src=pr&el=desc) will **decrease** coverage by `1.3%`.
> The diff coverage is `36%`.

[![Impacted file tree graph](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/graphs/tree.svg?width=650&token=MVaDEmdgi9&height=150&src=pr)](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #529      +/-   ##
==========================================
- Coverage    65.8%   64.49%   -1.31%     
==========================================
  Files          11       12       +1     
  Lines        1000     1059      +59     
==========================================
+ Hits          658      683      +25     
- Misses        305      337      +32     
- Partials       37       39       +2
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cmd/master.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL21hc3Rlci5nbw==) | `66.66% <0%> (-6.07%)` | :arrow_down: |
| [cmd/root.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL3Jvb3QuZ28=) | `59.09% <0%> (-1.85%)` | :arrow_down: |
| [cmd/common.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL2NvbW1vbi5nbw==) | `32.48% <0%> (-0.68%)` | :arrow_down: |
| [cmd/node.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL25vZGUuZ28=) | `66.66% <0%> (-6.07%)` | :arrow_down: |
| [cmd/run.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL3J1bi5nbw==) | `42.85% <42.85%> (ø)` | |
| [cmd/util.go](https://codecov.io/gh/aquasecurity/kube-bench/pull/529/diff?src=pr&el=tree#diff-Y21kL3V0aWwuZ28=) | `73.36% <81.81%> (+0.2%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=footer). Last update [8780e5c...63d20e9](https://codecov.io/gh/aquasecurity/kube-bench/pull/529?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/cmd/common.go b/cmd/common.go
index 23957c7..de3c6e8 100644
--- a/cmd/common.go
+++ b/cmd/common.go
@@ -62,7 +62,7 @@ func NewRunFilter(opts FilterOpts) (check.Predicate, error) {
 	}, nil
 }
 
-func runChecks(nodetype check.NodeType) {
+func runChecks(nodetype check.NodeType, testYamlFile string) {
 	var summary check.Summary
 
 	// Verify config file was loaded into Viper during Cobra sub-command initialization.
@@ -71,19 +71,24 @@ func runChecks(nodetype check.NodeType) {
 		os.Exit(1)
 	}
 
-	def := loadConfig(nodetype)
-	in, err := ioutil.ReadFile(def)
+	in, err := ioutil.ReadFile(testYamlFile)
 	if err != nil {
-		exitWithError(fmt.Errorf("error opening %s controls file: %v", nodetype, err))
+		exitWithError(fmt.Errorf("error opening %s test file: %v", testYamlFile, err))
 	}
 
-	glog.V(1).Info(fmt.Sprintf("Using benchmark file: %s\n", def))
+	glog.V(1).Info(fmt.Sprintf("Using test file: %s\n", testYamlFile))
 
-	// Get the set of executables and config files we care about on this type of node.
+	// Get the viper config for this section of tests
 	typeConf := viper.Sub(string(nodetype))
+	if typeConf == nil {
+		colorPrint(check.FAIL, fmt.Sprintf("No config settings for %s\n", string(nodetype)))
+		os.Exit(1)
+	}
+
+	// Get the set of executables we need for this section of the tests
 	binmap, err := getBinaries(typeConf, nodetype)
 
-	// Checks that the executables we need for the node type are running.
+	// Checks that the executables we need for the section are running.
 	if err != nil {
 		exitWithError(err)
 	}
@@ -226,19 +231,26 @@ func loadConfig(nodetype check.NodeType) string {
 		exitWithError(fmt.Errorf("can't find %s controls file in %s: %v", nodetype, cfgDir, err))
 	}
 
-	// Merge kubernetes version specific config if any.
+	// Merge version-specific config if any.
+	mergeConfig(path)
+
+	return filepath.Join(path, file)
+}
+
+func mergeConfig(path string) error {
 	viper.SetConfigFile(path + "/config.yaml")
-	err = viper.MergeInConfig()
+	err := viper.MergeInConfig()
 	if err != nil {
 		if os.IsNotExist(err) {
 			glog.V(2).Info(fmt.Sprintf("No version-specific config.yaml file in %s", path))
 		} else {
-			exitWithError(fmt.Errorf("couldn't read config file %s: %v", path+"/config.yaml", err))
+			return fmt.Errorf("couldn't read config file %s: %v", path+"/config.yaml", err)
 		}
-	} else {
-		glog.V(1).Info(fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed()))
 	}
-	return filepath.Join(path, file)
+
+	glog.V(1).Info(fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed()))
+
+	return nil
 }
 
 func mapToBenchmarkVersion(kubeToBenchmarkMap map[string]string, kv string) (string, error) {
diff --git a/cmd/master.go b/cmd/master.go
index 1659d38..fd903a7 100644
--- a/cmd/master.go
+++ b/cmd/master.go
@@ -1,4 +1,4 @@
-// Copyright © 2017 Aqua Security Software Ltd. <info@aquasec.com>
+// Copyright © 2017-2019 Aqua Security Software Ltd. <info@aquasec.com>
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,10 +22,11 @@ import (
 // masterCmd represents the master command
 var masterCmd = &cobra.Command{
 	Use:   "master",
-	Short: "Run benchmark checks for a Kubernetes master node.",
-	Long:  `Run benchmark checks for a Kubernetes master node.`,
+	Short: "Run Kubernetes benchmark checks from the master.yaml file.",
+	Long:  `Run Kubernetes benchmark checks from the master.yaml file in cfg/<version>.`,
 	Run: func(cmd *cobra.Command, args []string) {
-		runChecks(check.MASTER)
+		filename := loadConfig(check.MASTER)
+		runChecks(check.MASTER, filename)
 	},
 }
 
diff --git a/cmd/node.go b/cmd/node.go
index b07ed7e..d09b47b 100644
--- a/cmd/node.go
+++ b/cmd/node.go
@@ -1,4 +1,4 @@
-// Copyright © 2017 Aqua Security Software Ltd. <info@aquasec.com>
+// Copyright © 2017-2019 Aqua Security Software Ltd. <info@aquasec.com>
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,10 +22,11 @@ import (
 // nodeCmd represents the node command
 var nodeCmd = &cobra.Command{
 	Use:   "node",
-	Short: "Run benchmark checks for a Kubernetes node.",
-	Long:  `Run benchmark checks for a Kubernetes node.`,
+	Short: "Run Kubernetes benchmark checks from the node.yaml file.",
+	Long:  `Run Kubernetes benchmark checks from the node.yaml file in cfg/<version>.`,
 	Run: func(cmd *cobra.Command, args []string) {
-		runChecks(check.NODE)
+		filename := loadConfig(check.NODE)
+		runChecks(check.NODE, filename)
 	},
 }
 
diff --git a/cmd/root.go b/cmd/root.go
index 3eac65f..a94faf1 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -61,10 +61,12 @@ var RootCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		if isMaster() {
 			glog.V(1).Info("== Running master checks ==\n")
-			runChecks(check.MASTER)
+			filename := loadConfig(check.MASTER)
+			runChecks(check.MASTER, filename)
 		}
 		glog.V(1).Info("== Running node checks ==\n")
-		runChecks(check.NODE)
+		filename := loadConfig(check.NODE)
+		runChecks(check.NODE, filename)
 	},
 }
 
diff --git a/cmd/run.go b/cmd/run.go
new file mode 100644
index 0000000..9566311
--- /dev/null
+++ b/cmd/run.go
@@ -0,0 +1,91 @@
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/aquasecurity/kube-bench/check"
+	"github.com/golang/glog"
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+)
+
+func init() {
+	RootCmd.AddCommand(runCmd)
+	runCmd.Flags().StringSliceP("sections", "s", []string{},
+		`Specify sections of the benchmark to run. These names need to match the filenames in the cfg/<version> directory.
+	For example, to run the tests specified in master.yaml and etcd.yaml, specify --sections=master,etcd 
+	If no sections are specified, run tests from all files in the cfg/<version> directory.
+	`)
+}
+
+// runCmd represents the run command
+var runCmd = &cobra.Command{
+	Use:   "run",
+	Short: "Run tests",
+	Long:  `Run tests. If no arguments are specified, runs tests from all files`,
+	Run: func(cmd *cobra.Command, args []string) {
+		sections, err := cmd.Flags().GetStringSlice("sections")
+		if err != nil {
+			exitWithError(err)
+		}
+
+		benchmarkVersion, err := getBenchmarkVersion(kubeVersion, benchmarkVersion, viper.GetViper())
+		if err != nil {
+			exitWithError(err)
+		}
+
+		// Merge version-specific config if any.
+		path := filepath.Join(cfgDir, benchmarkVersion)
+		mergeConfig(path)
+
+		err = run(sections, benchmarkVersion)
+		if err != nil {
+			fmt.Printf("Error in run: %v\n", err)
+		}
+	},
+}
+
+func run(sections []string, benchmarkVersion string) (err error) {
+
+	yamlFiles, err := getTestYamlFiles(sections, benchmarkVersion)
+	if err != nil {
+		return err
+	}
+
+	glog.V(3).Infof("Running tests from files %v\n", yamlFiles)
+
+	for _, yamlFile := range yamlFiles {
+		_, name := filepath.Split(yamlFile)
+		testType := check.NodeType(strings.Split(name, ".")[0])
+		runChecks(testType, yamlFile)
+	}
+
+	return nil
+}
+
+func getTestYamlFiles(sections []string, benchmarkVersion string) (yamlFiles []string, err error) {
+
+	// Check that the specified sections have corresponding YAML files in the config directory
+	configFileDirectory := filepath.Join(cfgDir, benchmarkVersion)
+	for _, section := range sections {
+		filename := section + ".yaml"
+		file := filepath.Join(configFileDirectory, filename)
+		if _, err := os.Stat(file); err != nil {
+			return nil, fmt.Errorf("file %s not found for version %s", filename, benchmarkVersion)
+		}
+		yamlFiles = append(yamlFiles, file)
+	}
+
+	// If no sections were specified, we will run tests from all the files in the directory
+	if len(yamlFiles) == 0 {
+		yamlFiles, err = getYamlFilesFromDir(configFileDirectory)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return yamlFiles, err
+}
diff --git a/cmd/util.go b/cmd/util.go
index 2c2967e..61ab7f8 100644
--- a/cmd/util.go
+++ b/cmd/util.go
@@ -123,21 +123,39 @@ func getBinaries(v *viper.Viper, nodetype check.NodeType) (map[string]string, er
 	return binmap, nil
 }
 
-// getConfigFilePath locates the config files we should be using CIS version
+// getConfigFilePath locates the config files we should be using for CIS version
 func getConfigFilePath(benchmarkVersion string, filename string) (path string, err error) {
 	glog.V(2).Info(fmt.Sprintf("Looking for config specific CIS version %q", benchmarkVersion))
 
 	path = filepath.Join(cfgDir, benchmarkVersion)
 	file := filepath.Join(path, string(filename))
-	glog.V(2).Info(fmt.Sprintf("Looking for config file: %s", file))
+	glog.V(2).Info(fmt.Sprintf("Looking for file: %s", file))
 
-	if _, err = os.Stat(file); os.IsNotExist(err) {
+	if _, err := os.Stat(file); err != nil {
 		glog.V(2).Infof("error accessing config file: %q error: %v\n", file, err)
 		return "", fmt.Errorf("no test files found <= benchmark version: %s", benchmarkVersion)
 	}
+
 	return path, nil
 }
 
+// getYamlFilesFromDir returns a list of yaml files in the specified directory, ignoring config.yaml
+func getYamlFilesFromDir(path string) (names []string, err error) {
+	err = filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		_, name := filepath.Split(path)
+		if name != "" && name != "config.yaml" && filepath.Ext(name) == ".yaml" {
+			names = append(names, path)
+		}
+
+		return nil
+	})
+	return names, err
+}
+
 // decrementVersion decrements the version number
 // We want to decrement individually even through versions where we don't supply test files
 // just in case someone wants to specify their own test files for that version


Test patch:
diff --git a/cmd/run_test.go b/cmd/run_test.go
new file mode 100644
index 0000000..dd330c1
--- /dev/null
+++ b/cmd/run_test.go
@@ -0,0 +1,84 @@
+package cmd
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestGetTestYamlFiles(t *testing.T) {
+	cases := []struct {
+		name      string
+		sections  []string
+		benchmark string
+		succeed   bool
+		expCount  int
+	}{
+		{
+			name:      "Specify two sections",
+			sections:  []string{"one", "two"},
+			benchmark: "benchmark",
+			succeed:   true,
+			expCount:  2,
+		},
+		{
+			name:      "Specify a section that doesn't exist",
+			sections:  []string{"one", "missing"},
+			benchmark: "benchmark",
+			succeed:   false,
+		},
+		{
+			name:      "No sections specified - should return everything except config.yaml",
+			sections:  []string{},
+			benchmark: "benchmark",
+			succeed:   true,
+			expCount:  3,
+		},
+		{
+			name:      "Specify benchmark that doesn't exist",
+			sections:  []string{"one"},
+			benchmark: "missing",
+			succeed:   false,
+		},
+	}
+
+	// Set up temp config directory
+	var err error
+	cfgDir, err = ioutil.TempDir("", "kube-bench-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp directory")
+	}
+	defer os.RemoveAll(cfgDir)
+
+	d := filepath.Join(cfgDir, "benchmark")
+	err = os.Mkdir(d, 0766)
+	if err != nil {
+		t.Fatalf("Failed to create temp dir")
+	}
+
+	// We never expect config.yaml to be returned
+	for _, filename := range []string{"one.yaml", "two.yaml", "three.yaml", "config.yaml"} {
+		err = ioutil.WriteFile(filepath.Join(d, filename), []byte("hello world"), 0666)
+		if err != nil {
+			t.Fatalf("error writing temp file %s: %v", filename, err)
+		}
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			yamlFiles, err := getTestYamlFiles(c.sections, c.benchmark)
+			if err != nil && c.succeed {
+				t.Fatalf("Error %v", err)
+			}
+
+			if err == nil && !c.succeed {
+				t.Fatalf("Expected failure")
+			}
+
+			if len(yamlFiles) != c.expCount {
+				t.Fatalf("Expected %d, got %d", c.expCount, len(yamlFiles))
+			}
+		})
+	}
+}
diff --git a/cmd/util_test.go b/cmd/util_test.go
index 3e327c1..ea9045b 100644
--- a/cmd/util_test.go
+++ b/cmd/util_test.go
@@ -410,11 +410,14 @@ func TestGetConfigFilePath(t *testing.T) {
 	}
 	defer os.RemoveAll(cfgDir)
 	d := filepath.Join(cfgDir, "cis-1.4")
-	err = os.Mkdir(d, 0666)
+	err = os.Mkdir(d, 0766)
 	if err != nil {
-		t.Fatalf("Failed to create temp file")
+		t.Fatalf("Failed to create temp dir")
+	}
+	err = ioutil.WriteFile(filepath.Join(d, "master.yaml"), []byte("hello world"), 0666)
+	if err != nil {
+		t.Logf("Failed to create temp file")
 	}
-	ioutil.WriteFile(filepath.Join(d, "master.yaml"), []byte("hello world"), 0666)
 
 	cases := []struct {
 		benchmarkVersion string
@@ -471,3 +474,38 @@ func TestDecrementVersion(t *testing.T) {
 		}
 	}
 }
+
+func TestGetYamlFilesFromDir(t *testing.T) {
+	cfgDir, err := ioutil.TempDir("", "kube-bench-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp directory")
+	}
+	defer os.RemoveAll(cfgDir)
+
+	d := filepath.Join(cfgDir, "cis-1.4")
+	err = os.Mkdir(d, 0766)
+	if err != nil {
+		t.Fatalf("Failed to create temp dir")
+	}
+
+	err = ioutil.WriteFile(filepath.Join(d, "something.yaml"), []byte("hello world"), 0666)
+	if err != nil {
+		t.Fatalf("error writing file %v", err)
+	}
+	err = ioutil.WriteFile(filepath.Join(d, "config.yaml"), []byte("hello world"), 0666)
+	if err != nil {
+		t.Fatalf("error writing file %v", err)
+	}
+
+	files, err := getYamlFilesFromDir(d)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("Expected to find one file, found %d", len(files))
+	}
+
+	if files[0] != filepath.Join(d, "something.yaml") {
+		t.Fatalf("Expected to find something.yaml, found %s", files[0])
+	}
+}