From 34f8b8e980a438c1db8dbb7c0ec63037f86feed6 Mon Sep 17 00:00:00 2001 From: Liz Rice Date: Tue, 15 Aug 2017 16:44:40 +0100 Subject: [PATCH] Simplify verifying binaries and config files --- cmd/common.go | 31 ++++++++++++++++++---- cmd/util.go | 67 +++++++++++++++--------------------------------- cmd/util_test.go | 40 ++++++++++++++++++++++++++--- 3 files changed, 82 insertions(+), 56 deletions(-) diff --git a/cmd/common.go b/cmd/common.go index d27dc1f..fcacd8b 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -17,6 +17,7 @@ package cmd import ( "fmt" "io/ioutil" + "os" "strings" "github.com/aquasecurity/kube-bench/check" @@ -150,15 +151,35 @@ func runChecks(t check.NodeType) { // verifyNodeType checks the executables and config files are as expected // for the specified tests (master, node or federated). func verifyNodeType(t check.NodeType) { + var bins []string + var confs []string + switch t { case check.MASTER: - verifyBin(apiserverBin, schedulerBin, controllerManagerBin) - verifyConf(apiserverConf, schedulerConf, controllerManagerConf) + bins = []string{apiserverBin, schedulerBin, controllerManagerBin} + confs = []string{apiserverConf, schedulerConf, controllerManagerConf} case check.NODE: - verifyBin(kubeletBin, proxyBin) - verifyConf(kubeletConf, proxyConf) + bins = []string{kubeletBin, proxyBin} + confs = []string{kubeletConf, proxyConf} case check.FEDERATED: - verifyBin(fedApiserverBin, fedControllerManagerBin) + bins = []string{fedApiserverBin, fedControllerManagerBin} + } + + for _, bin := range bins { + if !verifyBin(bin, ps) { + printlnWarn(fmt.Sprintf("%s is not running", bin)) + } + } + + for _, conf := range confs { + _, err := os.Stat(conf) + if err != nil { + if os.IsNotExist(err) { + printlnWarn(fmt.Sprintf("Missing kubernetes config file: %s", conf)) + } else { + exitWithError(fmt.Errorf("error looking for file %s: %v", conf, err)) + } + } } } diff --git a/cmd/util.go b/cmd/util.go index c3d82cb..4967cbe 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -64,59 +64,30 @@ func cleanIDs(list string) []string { return ids } -func verifyConf(confPath ...string) { - var missing string - - for _, c := range confPath { - if _, err := os.Stat(c); err != nil && os.IsNotExist(err) { - continueWithError(err, "") - missing += c + ", " - } - } - - if len(missing) > 0 { - missing = strings.Trim(missing, ", ") - printlnWarn(fmt.Sprintf("Missing kubernetes config files: %s", missing)) - } - -} - -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 { - _, err := exec.LookPath(b) - bin = bin + "," + b - binSlice = append(binSlice, b) - if err != nil { - missing += b + ", " - continueWithError(err, "") - } - } - bin = strings.Trim(bin, ",") - - cmd := exec.Command("ps", "-C", bin, "-o", "cmd", "--no-headers") +// ps execs out to the ps command; it's separated into a function so we can write tests +func ps(proc string) string { + cmd := exec.Command("ps", "-C", proc, "-o", "cmd", "--no-headers") out, err := cmd.Output() if err != nil { continueWithError(fmt.Errorf("%s: %s", cmd.Args, err), "") } - for _, b := range binSlice { - matched := strings.Contains(string(out), b) + return string(out) +} - if !matched { - notRunning += b + ", " - } - } +// verifyBin checks that the binary specified is running +func verifyBin(bin string, psFunc func(string) string) bool { - if len(notRunning) > 0 { - notRunning = strings.Trim(notRunning, ", ") - printlnWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning)) - } + // Strip any quotes + bin = strings.Trim(bin, "'\"") + + // bin could consist of more than one word + // We'll search for running processes with the first word, and then check the whole + // proc as supplied is included in the results + proc := strings.Fields(bin)[0] + out := psFunc(proc) + + return strings.Contains(out, bin) } func verifyKubeVersion(major string, minor string) { @@ -133,7 +104,9 @@ func verifyKubeVersion(major string, minor string) { if err != nil { s := fmt.Sprintf("Kubernetes version check skipped with error %v", err) continueWithError(err, sprintlnWarn(s)) - return + if len(out) == 0 { + return + } } msg := checkVersion("Client", string(out), major, minor) diff --git a/cmd/util_test.go b/cmd/util_test.go index 74d955f..fe9e0b9 100644 --- a/cmd/util_test.go +++ b/cmd/util_test.go @@ -16,6 +16,7 @@ package cmd import ( "regexp" + "strconv" "testing" ) @@ -36,8 +37,8 @@ func TestCheckVersion(t *testing.T) { {t: "Server", s: "something unexpected", major: "2", minor: "0", exp: "Couldn't find Server version from kubectl output 'something unexpected'"}, } - for _, c := range cases { - t.Run("", func(t *testing.T) { + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { m := checkVersion(c.t, c.s, c.major, c.minor) if m != c.exp { t.Fatalf("Got: %s, expected: %s", m, c.exp) @@ -66,8 +67,8 @@ func TestVersionMatch(t *testing.T) { {r: minor}, // Checking that we don't fall over if the string is empty } - for _, c := range cases { - t.Run("", func(t *testing.T) { + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { m := versionMatch(c.r, c.s) if m != c.exp { t.Fatalf("Got %s expected %s", m, c.exp) @@ -75,3 +76,34 @@ func TestVersionMatch(t *testing.T) { }) } } + +var g string + +func fakeps(proc string) string { + return g +} +func TestVerifyBin(t *testing.T) { + cases := []struct { + proc string + psOut string + exp bool + }{ + {proc: "single", psOut: "single", exp: true}, + {proc: "single", psOut: "", exp: false}, + {proc: "two words", psOut: "two words", exp: true}, + {proc: "two words", psOut: "", exp: false}, + {proc: "cmd", psOut: "cmd param1 param2", exp: true}, + {proc: "cmd param", psOut: "cmd param1 param2", exp: true}, + {proc: "cmd param", psOut: "cmd", exp: false}, + } + + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + g = c.psOut + v := verifyBin(c.proc, fakeps) + if v != c.exp { + t.Fatalf("Expected %v got %v", c.exp, v) + } + }) + } +}