Fixes Issue #331: Changes the Error Message When Programs are Missing (#497)

* changed error description for missing kubectl/kubelet execs

* adds function to generate error message for missing components

* adds function to generate error message for missing components

* adds function to generate error message for missing components

* Update cmd/util.go

Co-Authored-By: Liz Rice <liz@lizrice.com>

* Update cmd/util.go

Co-Authored-By: Liz Rice <liz@lizrice.com>

* Update cmd/util.go

Co-Authored-By: Liz Rice <liz@lizrice.com>

* Update cmd/util.go

Co-Authored-By: Liz Rice <liz@lizrice.com>

* Update cmd/util.go

Co-Authored-By: Liz Rice <liz@lizrice.com>

* fixed error message

* changes are per PR review

cmd/common.go

@@ -81,7 +81,7 @@ func runChecks(nodetype check.NodeType) {
 
 	// Get the set of executables and config files we care about on this type of node.
 	typeConf := viper.Sub(string(nodetype))
-	binmap, err := getBinaries(typeConf)
+	binmap, err := getBinaries(typeConf, nodetype)
 
 	// Checks that the executables we need for the node type are running.
 	if err != nil {
@@ -213,7 +213,7 @@ func loadConfig(nodetype check.NodeType) string {
 	if kubeVersion == "" {
 		runningVersion, err = getKubeVersion()
 		if err != nil {
-			exitWithError(fmt.Errorf("Version check failed: %s\nAlternatively, you can specify the version with --version", err))
+			exitWithError(fmt.Errorf("Version check failed: \n%s", err))
 		}
 	}
 
@@ -245,7 +245,7 @@ func isMaster() bool {
 		glog.V(2).Info("No master components found to be running")
 		return false
 	}
-	components, err := getBinariesFunc(masterConf)
+	components, err := getBinariesFunc(masterConf, check.MASTER)
 
 	if err != nil {
 		glog.V(2).Info(err)

cmd/common_test.go

@@ -16,10 +16,11 @@ package cmd
 
 import (
 	"errors"
+	"testing"
+
 	"github.com/aquasecurity/kube-bench/check"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 func TestNewRunFilter(t *testing.T) {
@@ -114,44 +115,44 @@ func TestNewRunFilter(t *testing.T) {
 }
 
 func TestIsMaster(t *testing.T) {
-	testCases := []struct{
-		name string
-		cfgFile string
-		getBinariesFunc func(*viper.Viper) (map[string]string, error)
-		isMaster bool
+	testCases := []struct {
+		name            string
+		cfgFile         string
+		getBinariesFunc func(*viper.Viper, check.NodeType) (map[string]string, error)
+		isMaster        bool
 	}{
 		{
-			name: "valid config, is master and all components are running",
+			name:    "valid config, is master and all components are running",
 			cfgFile: "../cfg/config.yaml",
-			getBinariesFunc: func(viper *viper.Viper) (strings map[string]string, i error) {
+			getBinariesFunc: func(viper *viper.Viper, nt check.NodeType) (strings map[string]string, i error) {
 				return map[string]string{"apiserver": "kube-apiserver"}, nil
 			},
 			isMaster: true,
 		},
 		{
-			name: "valid config, is master and but not all components are running",
+			name:    "valid config, is master and but not all components are running",
 			cfgFile: "../cfg/config.yaml",
-			getBinariesFunc: func(viper *viper.Viper) (strings map[string]string, i error) {
+			getBinariesFunc: func(viper *viper.Viper, nt check.NodeType) (strings map[string]string, i error) {
 				return map[string]string{}, nil
 			},
 			isMaster: false,
 		},
 		{
-			name: "valid config, is master, not all components are running and fails to find all binaries",
+			name:    "valid config, is master, not all components are running and fails to find all binaries",
 			cfgFile: "../cfg/config.yaml",
-			getBinariesFunc: func(viper *viper.Viper) (strings map[string]string, i error) {
+			getBinariesFunc: func(viper *viper.Viper, nt check.NodeType) (strings map[string]string, i error) {
 				return map[string]string{}, errors.New("failed to find binaries")
 			},
 			isMaster: false,
 		},
 		{
-			name: "valid config, does not include master",
-			cfgFile: "../cfg/node_only.yaml",
+			name:     "valid config, does not include master",
+			cfgFile:  "../cfg/node_only.yaml",
 			isMaster: false,
 		},
 	}
 
-	for _, tc := range testCases{
+	for _, tc := range testCases {
 		cfgFile = tc.cfgFile
 		initConfig()
 

cmd/util.go

@@ -27,7 +27,7 @@ var (
 
 var psFunc func(string) string
 var statFunc func(string) (os.FileInfo, error)
-var getBinariesFunc func(*viper.Viper) (map[string]string, error)
+var getBinariesFunc func(*viper.Viper, check.NodeType) (map[string]string, error)
 var TypeMap = map[string][]string{
 	"ca":         []string{"cafile", "defaultcafile"},
 	"kubeconfig": []string{"kubeconfig", "defaultkubeconfig"},
@@ -89,7 +89,7 @@ func ps(proc string) string {
 
 // getBinaries finds which of the set of candidate executables are running.
 // It returns an error if one mandatory executable is not running.
-func getBinaries(v *viper.Viper) (map[string]string, error) {
+func getBinaries(v *viper.Viper, nodetype check.NodeType) (map[string]string, error) {
 	binmap := make(map[string]string)
 
 	for _, component := range v.GetStringSlice("components") {
@@ -103,7 +103,8 @@ func getBinaries(v *viper.Viper) (map[string]string, error) {
 		if len(bins) > 0 {
 			bin, err := findExecutable(bins)
 			if err != nil && !optional {
-				return nil, fmt.Errorf("need %s executable but none of the candidates are running", component)
+				glog.Warning(buildComponentMissingErrorMessage(nodetype, component, bins))
+				return nil, fmt.Errorf("unable to detect running programs for component %q", component)
 			}
 
 			// Default the executable name that we'll substitute to the name of the component
@@ -269,6 +270,25 @@ func multiWordReplace(s string, subname string, sub string) string {
 	return strings.Replace(s, subname, sub, -1)
 }
 
+const missingKubectlKubeletMessage = `
+Unable to find the programs kubectl or kubelet in the PATH.
+These programs are used to determine which version of Kubernetes is running.
+Make sure the /usr/bin directory is mapped to the container, 
+either in the job.yaml file, or Docker command.
+
+For job.yaml:
+...
+- name: usr-bin
+  mountPath: /usr/bin
+...
+
+For docker command:
+   docker -v $(which kubectl):/usr/bin/kubectl ....
+
+Alternatively, you can specify the version with --version
+   kube-bench --version <VERSION> ...
+`
+
 func getKubeVersion() (string, error) {
 	// These executables might not be on the user's path.
 	_, err := exec.LookPath("kubectl")
@@ -282,7 +302,9 @@ func getKubeVersion() (string, error) {
 			if err == nil {
 				return getVersionFromKubeletOutput(string(out)), nil
 			}
-			return "", fmt.Errorf("need kubectl or kubelet binaries to get kubernetes version")
+
+			glog.Warning(missingKubectlKubeletMessage)
+			return "", fmt.Errorf("unable to find the programs kubectl or kubelet in the PATH")
 		}
 		return getKubeVersionFromKubelet(), nil
 	}
@@ -344,3 +366,29 @@ func makeSubstitutions(s string, ext string, m map[string]string) string {
 
 	return s
 }
+
+func buildComponentMissingErrorMessage(nodetype check.NodeType, component string, bins []string) string {
+
+	errMessageTemplate := `
+Unable to detect running programs for component %q
+The following %q programs have been searched, but none of them have been found:
+%s
+
+These program names are provided in the config.yaml, section '%s.%s.bins'
+`
+
+	componentRoleName := "master node"
+	componentType := "master"
+
+	if nodetype == check.NODE {
+		componentRoleName = "worker node"
+		componentType = "node"
+	}
+
+	binList := ""
+	for _, bin := range bins {
+		binList = fmt.Sprintf("%s\t- %s\n", binList, bin)
+	}
+
+	return fmt.Sprintf(errMessageTemplate, component, componentRoleName, binList, componentType, component)
+}

cmd/util_test.go

@@ -22,6 +22,7 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/aquasecurity/kube-bench/check"
 	"github.com/spf13/viper"
 )
 
@@ -166,7 +167,7 @@ func TestGetBinaries(t *testing.T) {
 			for k, val := range c.config {
 				v.Set(k, val)
 			}
-			m, err := getBinaries(v)
+			m, err := getBinaries(v, check.MASTER)
 			if c.expectErr {
 				if err == nil {
 					t.Fatal("Got nil Expected error")