Merge pull request #28 from ttousai/errorhandling

Improve error handling.

cfg/config.yaml

@@ -29,7 +29,7 @@ installation:
       conf:
         apiserver: /etc/kubernetes/apiserver
         scheduler: /etc/kubernetes/scheduler
-        controller-manager: /etc/kubernetes/apiserver
+        controller-manager: /etc/kubernetes/controller-manager
     node:
       bin:
         kubelet: kubelet

check/check.go

@@ -18,9 +18,10 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"os"
 	"os/exec"
 	"strings"
+
+	"github.com/golang/glog"
 )
 
 // NodeType indicates the type of node (master, node, federated).
@@ -69,7 +70,7 @@ type Check struct {
 
 // Run executes the audit commands specified in a check and outputs
 // the results.
-func (c *Check) Run(verbose bool) {
+func (c *Check) Run() {
 	var out bytes.Buffer
 	var errmsgs string
 
@@ -148,9 +149,7 @@ func (c *Check) Run(verbose bool) {
 		i++
 	}
 
-	if verbose && errmsgs != "" {
-		fmt.Fprintf(os.Stderr, "%s\n", errmsgs)
-	}
+	glog.V(2).Info("%s\n", errmsgs)
 
 	res := c.Tests.execute(out.String())
 	if res {

check/controls.go

@@ -68,7 +68,7 @@ func NewControls(t NodeType, in []byte) (*Controls, error) {
 }
 
 // RunGroup runs all checks in a group.
-func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary {
+func (controls *Controls) RunGroup(gids ...string) Summary {
 	g := []*Group{}
 	controls.Summary.Pass, controls.Summary.Fail, controls.Summary.Warn = 0, 0, 0
 
@@ -82,7 +82,7 @@ func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary {
 		for _, gid := range gids {
 			if gid == group.ID {
 				for _, check := range group.Checks {
-					check.Run(verbose)
+					check.Run()
 					summarize(controls, check)
 				}
 
@@ -96,7 +96,7 @@ func (controls *Controls) RunGroup(verbose bool, gids ...string) Summary {
 }
 
 // RunChecks runs the checks with the supplied IDs.
-func (controls *Controls) RunChecks(verbose bool, ids ...string) Summary {
+func (controls *Controls) RunChecks(ids ...string) Summary {
 	g := []*Group{}
 	m := make(map[string]*Group)
 	controls.Summary.Pass, controls.Summary.Fail, controls.Summary.Warn = 0, 0, 0
@@ -110,7 +110,7 @@ func (controls *Controls) RunChecks(verbose bool, ids ...string) Summary {
 		for _, check := range group.Checks {
 			for _, id := range ids {
 				if id == check.ID {
-					check.Run(verbose)
+					check.Run()
 					summarize(controls, check)
 
 					// Check if we have already added this checks group.

cmd/common.go

@@ -17,7 +17,6 @@ package cmd
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strings"
 
 	"github.com/aquasecurity/kube-bench/check"
@@ -59,7 +58,7 @@ func runChecks(t check.NodeType) {
 	schedulerBin = viper.GetString("installation." + installation + ".master.bin.scheduler")
 	schedulerConf = viper.GetString("installation." + installation + ".master.conf.scheduler")
 	controllerManagerBin = viper.GetString("installation." + installation + ".master.bin.controller-manager")
-	controllerManagerConf = viper.GetString("installation." + installation + ".master.conf.controler-manager")
+	controllerManagerConf = viper.GetString("installation." + installation + ".master.conf.controller-manager")
 	config = viper.GetString("installation." + installation + ".config")
 
 	etcdBin = viper.GetString("etcd.bin")
@@ -78,7 +77,7 @@ func runChecks(t check.NodeType) {
 	fedControllerManagerBin = viper.GetString("installation." + installation + ".federated.bin.controller-manager")
 
 	// Run kubernetes installation validation checks.
-	warns := verifyNodeType(t)
+	verifyNodeType(t)
 
 	switch t {
 	case check.MASTER:
@@ -91,8 +90,7 @@ func runChecks(t check.NodeType) {
 
 	in, err := ioutil.ReadFile(file)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error opening %s controls file: %v\n", t, err)
-		os.Exit(1)
+		exitWithError(fmt.Errorf("error opening %s controls file: %v", t, err))
 	}
 
 	// Variable substitutions. Replace all occurrences of variables in controls files.
@@ -102,7 +100,6 @@ func runChecks(t check.NodeType) {
 	s = strings.Replace(s, "$schedulerconf", schedulerConf, -1)
 	s = strings.Replace(s, "$controllermanagerbin", controllerManagerBin, -1)
 	s = strings.Replace(s, "$controllermanagerconf", controllerManagerConf, -1)
-	s = strings.Replace(s, "$controllermanagerconf", controllerManagerConf, -1)
 	s = strings.Replace(s, "$config", config, -1)
 
 	s = strings.Replace(s, "$etcdbin", etcdBin, -1)
@@ -120,63 +117,50 @@ func runChecks(t check.NodeType) {
 
 	controls, err := check.NewControls(t, []byte(s))
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error setting up %s controls: %v\n", t, err)
-		os.Exit(1)
+		exitWithError(fmt.Errorf("error setting up %s controls: %v", t, err))
 	}
 
 	if groupList != "" && checkList == "" {
 		ids := cleanIDs(groupList)
-		summary = controls.RunGroup(verbose, ids...)
+		summary = controls.RunGroup(ids...)
 	} else if checkList != "" && groupList == "" {
 		ids := cleanIDs(checkList)
-		summary = controls.RunChecks(verbose, ids...)
+		summary = controls.RunChecks(ids...)
 	} else if checkList != "" && groupList != "" {
-		fmt.Fprintf(os.Stderr, "group option and check option can't be used together\n")
-		os.Exit(1)
+		exitWithError(fmt.Errorf("group option and check option can't be used together"))
 	} else {
-		summary = controls.RunGroup(verbose)
+		summary = controls.RunGroup()
 	}
 
 	// if we successfully ran some tests and it's json format, ignore the warnings
 	if (summary.Fail > 0 || summary.Warn > 0 || summary.Pass > 0) && jsonFmt {
 		out, err := controls.JSON()
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "failed to output in JSON format: %v\n", err)
-			os.Exit(1)
+			exitWithError(fmt.Errorf("failed to output in JSON format: %v", err))
 		}
 
 		fmt.Println(string(out))
 	} else {
-		prettyPrint(warns, controls, summary)
+		prettyPrint(controls, summary)
 	}
 }
 
 // verifyNodeType checks the executables and config files are as expected
 // for the specified tests (master, node or federated).
-func verifyNodeType(t check.NodeType) []string {
-	var w []string
-	// Always clear out error messages.
-	errmsgs = ""
-
+func verifyNodeType(t check.NodeType) {
 	switch t {
 	case check.MASTER:
-		w = append(w, verifyBin(apiserverBin, schedulerBin, controllerManagerBin)...)
-		w = append(w, verifyConf(apiserverConf, schedulerConf, controllerManagerConf)...)
-		w = append(w, verifyKubeVersion(apiserverBin)...)
+		verifyKubeVersion(apiserverBin)
+		verifyBin(apiserverBin, schedulerBin, controllerManagerBin)
+		verifyConf(apiserverConf, schedulerConf, controllerManagerConf)
 	case check.NODE:
-		w = append(w, verifyBin(kubeletBin, proxyBin)...)
-		w = append(w, verifyConf(kubeletConf, proxyConf)...)
-		w = append(w, verifyKubeVersion(kubeletBin)...)
+		verifyKubeVersion(kubeletBin)
+		verifyBin(kubeletBin, proxyBin)
+		verifyConf(kubeletConf, proxyConf)
 	case check.FEDERATED:
-		w = append(w, verifyBin(fedApiserverBin, fedControllerManagerBin)...)
-		w = append(w, verifyKubeVersion(fedApiserverBin)...)
-	}
-
-	if verbose {
-		fmt.Fprintf(os.Stderr, "%s\n", errmsgs)
+		verifyKubeVersion(fedApiserverBin)
+		verifyBin(fedApiserverBin, fedControllerManagerBin)
 	}
-
-	return w
 }
 
 // colorPrint outputs the state in a specific colour, along with a message string
@@ -186,13 +170,9 @@ func colorPrint(state check.State, s string) {
 }
 
 // prettyPrint outputs the results to stdout in human-readable format
-func prettyPrint(warnings []string, r *check.Controls, summary check.Summary) {
+func prettyPrint(r *check.Controls, summary check.Summary) {
 	colorPrint(check.INFO, fmt.Sprintf("Using config file: %s\n", viper.ConfigFileUsed()))
 
-	for _, w := range warnings {
-		colorPrint(check.WARN, w)
-	}
-
 	colorPrint(check.INFO, fmt.Sprintf("%s %s\n", r.ID, r.Text))
 	for _, g := range r.Groups {
 		colorPrint(check.INFO, fmt.Sprintf("%s %s\n", g.ID, g.Text))

cmd/root.go

@@ -15,6 +15,7 @@
 package cmd
 
 import (
+	goflag "flag"
 	"fmt"
 	"os"
 
@@ -34,11 +35,12 @@ var (
 	nodeFile      string
 	federatedFile string
 
+	loud bool
+
 	kubeConfDir     string
 	etcdConfDir     string
 	flanneldConfDir string
 
-	verbose      bool
 	installation string
 )
 
@@ -52,6 +54,9 @@ var RootCmd = &cobra.Command{
 // Execute adds all child commands to the root command sets flags appropriately.
 // This is called by main.main(). It only needs to happen once to the rootCmd.
 func Execute() {
+	goflag.Set("logtostderr", "true")
+	goflag.CommandLine.Parse([]string{})
+
 	if err := RootCmd.Execute(); err != nil {
 		fmt.Println(err)
 		os.Exit(-1)
@@ -83,7 +88,11 @@ func init() {
 		`Run all the checks under this comma-delimited list of groups. Example --group="1.1"`,
 	)
 	RootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is ./cfg/config.yaml)")
-	RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose output (default false)")
+
+	goflag.CommandLine.VisitAll(func(goflag *goflag.Flag) {
+		RootCmd.PersistentFlags().AddGoFlag(goflag)
+	})
+
 }
 
 // initConfig reads in config file and ENV variables if set.
@@ -103,5 +112,4 @@ func initConfig() {
 		colorPrint(check.FAIL, fmt.Sprintf("Failed to read config file: %v\n", err))
 		os.Exit(1)
 	}
-
 }

cmd/util.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/aquasecurity/kube-bench/check"
 	"github.com/fatih/color"
+	"github.com/golang/glog"
 )
 
 var (
@@ -20,11 +21,35 @@ var (
 	}
 )
 
-func handleError(err error, context string) (errmsg string) {
+func printlnWarn(msg string) {
+	fmt.Fprintf(os.Stderr, "[%s] %s\n",
+		colors[check.WARN].Sprintf("%s", check.WARN),
+		msg,
+	)
+}
+
+func sprintlnWarn(msg string) string {
+	return fmt.Sprintf("[%s] %s",
+		colors[check.WARN].Sprintf("%s", check.WARN),
+		msg,
+	)
+}
+
+func exitWithError(err error) {
+	fmt.Fprintf(os.Stderr, "\n%v\n", err)
+	os.Exit(1)
+}
+
+func continueWithError(err error, msg string) string {
 	if err != nil {
-		errmsg = fmt.Sprintf("%s, error: %s\n", context, err)
+		glog.V(1).Info(err)
 	}
-	return
+
+	if msg != "" {
+		fmt.Fprintf(os.Stderr, "%s\n", msg)
+	}
+
+	return ""
 }
 
 func cleanIDs(list string) []string {
@@ -38,76 +63,85 @@ func cleanIDs(list string) []string {
 	return ids
 }
 
-func verifyConf(confPath ...string) []string {
-	var w []string
+func verifyConf(confPath ...string) {
+	var missing string
+
 	for _, c := range confPath {
 		if _, err := os.Stat(c); err != nil && os.IsNotExist(err) {
-			w = append(w, fmt.Sprintf("config file %s does not exist\n", c))
+			continueWithError(err, "")
+			missing += c + ", "
 		}
 	}
 
-	return w
+	if len(missing) > 0 {
+		missing = strings.Trim(missing, ", ")
+		printlnWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing))
+	}
+
 }
 
-func verifyBin(binPath ...string) []string {
-	var w []string
-	var binList string
+func verifyBin(binPath ...string) {
+	var binSlice []string
+	var bin string
+	var missing string
+	var notRunning string
 
 	// Construct proc name for ps(1)
 	for _, b := range binPath {
-		binList += b + ","
 		_, err := exec.LookPath(b)
-		errmsgs += handleError(
-			err,
-			fmt.Sprintf("%s: command not found in path", b),
-		)
+		bin = bin + "," + b
+		binSlice = append(binSlice, b)
+		if err != nil {
+			missing += b + ", "
+			continueWithError(err, "")
+		}
 	}
+	bin = strings.Trim(bin, ",")
 
-	binList = strings.Trim(binList, ",")
-
-	// Run ps command
-	cmd := exec.Command("ps", "-C", binList, "-o", "cmd", "--no-headers")
+	cmd := exec.Command("ps", "-C", bin, "-o", "cmd", "--no-headers")
 	out, err := cmd.Output()
-	errmsgs += handleError(
-		err,
-		fmt.Sprintf("failed to run: %s", cmd.Args),
-	)
+	if err != nil {
+		continueWithError(fmt.Errorf("%s: %s", cmd.Args, err), "")
+	}
 
-	// Actual verification
-	for _, b := range binPath {
+	for _, b := range binSlice {
 		matched := strings.Contains(string(out), b)
 
 		if !matched {
-			w = append(w, fmt.Sprintf("%s is not running\n", b))
+			notRunning += b + ", "
 		}
 	}
 
-	return w
+	if len(missing) > 0 {
+		missing = strings.Trim(missing, ", ")
+		printlnWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing))
+	}
+
+	if len(notRunning) > 0 {
+		notRunning = strings.Trim(notRunning, ", ")
+		printlnWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning))
+	}
 }
 
-func verifyKubeVersion(b string) []string {
+func verifyKubeVersion(b string) {
 	// These executables might not be on the user's path.
 	// TODO! Check the version number using kubectl, which is more likely to be on the path.
-	var w []string
 
 	_, err := exec.LookPath(b)
-	errmsgs += handleError(
-		err,
-		fmt.Sprintf("%s: command not found on path - version check skipped", b),
-	)
+	if err != nil {
+		continueWithError(err, sprintlnWarn("Kubernetes version check skipped"))
+		return
+	}
 
-	// Check version
 	cmd := exec.Command(b, "--version")
 	out, err := cmd.Output()
-	errmsgs += handleError(
-		err,
-		fmt.Sprintf("failed to run:%s", cmd.Args),
-	)
+	if err != nil {
+		continueWithError(err, sprintlnWarn("Kubernetes version check skipped"))
+		return
+	}
 
 	matched := strings.Contains(string(out), kubeVersion)
 	if !matched {
-		w = append(w, fmt.Sprintf("%s unsupported version\n", b))
+		printlnWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out))
 	}
-
-	return w
 }