From f88de572f6ff4799636b86bb6237b491613211c0 Mon Sep 17 00:00:00 2001
From: Abubakr-Sadik Nii Nai Davis <dwa2pac@gmail.com>
Date: Tue, 25 Jul 2017 00:34:07 +0000
Subject: [PATCH] Improve error handling.
---
cfg/config.yaml | 2 +-
check/check.go | 9 ++--
check/controls.go | 8 ++--
cmd/common.go | 60 ++++++++----------------
cmd/root.go | 14 ++++--
cmd/util.go | 116 ++++++++++++++++++++++++++++++----------------
6 files changed, 115 insertions(+), 94 deletions(-)
diff --git a/cfg/config.yaml b/cfg/config.yaml
index c8c89c9..07548ac 100644
--- a/cfg/config.yaml
+++ b/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
diff --git a/check/check.go b/check/check.go
index e73c58f..5d8221a 100644
--- a/check/check.go
+++ b/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 {
diff --git a/check/controls.go b/check/controls.go
index 8f7530c..dfea006 100644
--- a/check/controls.go
+++ b/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.
diff --git a/cmd/common.go b/cmd/common.go
index c002945..50c9c41 100644
--- a/cmd/common.go
+++ b/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))
diff --git a/cmd/root.go b/cmd/root.go
index 5429c2c..aed8b42 100644
--- a/cmd/root.go
+++ b/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)
}
-
}
diff --git a/cmd/util.go b/cmd/util.go
index 6a1f790..88df3a7 100644
--- a/cmd/util.go
+++ b/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 printWarn(msg string) {
+ fmt.Fprintf(os.Stderr, "[%s] %s\n",
+ colors[check.WARN].Sprintf("%s", check.WARN),
+ msg,
+ )
+}
+
+func printfWarn(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, ", ")
+ printWarn(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, ", ")
+ printWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing))
+ }
+
+ if len(notRunning) > 0 {
+ notRunning = strings.Trim(notRunning, ", ")
+ printWarn(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, printfWarn("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, printfWarn("Kubernetes version check skipped"))
+ return
+ }
matched := strings.Contains(string(out), kubeVersion)
if !matched {
- w = append(w, fmt.Sprintf("%s unsupported version\n", b))
+ printWarn(fmt.Sprintf("Unsupported kubernetes version: %s", out))
}
-
- return w
}
--
GitLab