diff --git a/cfg/master.yaml b/cfg/master.yaml index 874b9cc..f54bf6a 100644 --- a/cfg/master.yaml +++ b/cfg/master.yaml @@ -596,10 +596,25 @@ groups: checks: - id: 1.4.1 text: "Ensure that the apiserver file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $apiserverconf; then stat -c %a $apiserverconf; fi" + # audit: "/bin/bash -c 'if test -e $apiserverconf; then stat -c %a $apiserverconf; fi'" + audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %a $apiserverconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chmod 644 $apiserverconf" @@ -607,10 +622,13 @@ groups: - id: 1.4.2 text: "Ensure that the apiserver file ownership is set to root:root (Scored)" - audit: "if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; fi" + audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: "root:root" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chown root:root $apiserverconf" @@ -618,10 +636,24 @@ groups: - id: 1.4.3 text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $config; then stat -c %a $config; fi" + audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chmod 644 $config" @@ -629,10 +661,13 @@ groups: - id: 1.4.4 text: "Ensure that the config file ownership is set to root:root (Scored)" - audit: "if test -e $config; then stat -c %U:%G $config; fi" + audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: "root:root" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chown root:root $config" @@ -640,10 +675,24 @@ groups: - id: 1.4.5 text: "Ensure that the scheduler file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $schedulerconf; then stat -c %a $schedulerconf; fi" + audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %a $schedulerconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chmod 644 $schedulerconf" @@ -651,10 +700,13 @@ groups: - id: 1.4.6 text: "Ensure that the scheduler file ownership is set to root:root (Scored)" - audit: "if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; fi" + audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: "root:root" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chown root:root $schedulerconf" @@ -662,10 +714,24 @@ groups: - id: 1.4.7 text: "Ensure that the etcd.conf file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $etcdconf; then stat -c %a $etcdconf; fi" + audit: "/bin/sh -c 'if test -e $etcdconf; then stat -c %a $etcdconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chmod 644 $etcdconf" @@ -673,10 +739,13 @@ groups: - id: 1.4.8 text: "Ensure that the etcd.conf file ownership is set to root:root (Scored)" - audit: "if test -e $etcdconf; then stat -c %U:%G $etcdconf; fi" + audit: "/bin/sh -c 'if test -e $etcdconf; then stat -c %U:%G $etcdconf; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: "root:root" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chown root:root $etcdconf" @@ -684,10 +753,24 @@ groups: - id: 1.4.9 text: "Ensure that the flanneld file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $flanneldconf; then stat -c %a $flanneldconf; fi" + audit: "/bin/sh -c 'if test -e $flanneldconf; then stat -c %a $flanneldconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chmod 644 $flanneldconf" @@ -695,10 +778,13 @@ groups: - id: 1.4.10 text: "Ensure that the flanneld file ownership is set to root:root (Scored)" - audit: "if test -e $flanneldconf; then stat -c %U:%G $flanneldconf; fi" + audit: "/bin/sh -c 'if test -e $flanneldconf; then stat -c %U:%G $flanneldconf; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: "root:root" set: true remediation: "Run the below command (based on the file location on your system) on the master node. \nFor example, chown root:root $flanneldconf" @@ -710,6 +796,9 @@ groups: tests: test_items: - flag: "700" + compare: + op: eq + value: "700" set: true remediation: "On the etcd server node, get the etcd data directory, passed as an argument --data-dir , from the below command:\n diff --git a/cfg/node.yaml b/cfg/node.yaml index 346ddd1..dfff376 100644 --- a/cfg/node.yaml +++ b/cfg/node.yaml @@ -221,10 +221,24 @@ groups: checks: - id: 2.2.1 text: "Ensure that the config file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $config; then stat -c %a $config; fi" + audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the each worker node. \nFor example, chmod 644 $config" @@ -232,10 +246,13 @@ groups: - id: 2.2.2 text: "Ensure that the config file ownership is set to root:root (Scored)" - audit: "if test -e $config; then stat -c %U:%G $config; fi" + audit: "/bin/sh -c 'if test -e $config; then stat -c %U:%G $config; fi'" tests: test_items: - flag: "root:root" + compare: + op: eq + value: root:root set: true remediation: "Run the below command (based on the file location on your system) on the each worker node. \nFor example, chown root:root $config" @@ -243,10 +260,24 @@ groups: - id: 2.2.3 text: "Ensure that the kubelet file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $kubeletconf; then stat -c %a $kubeletconf; fi" + audit: "/bin/sh -c 'if test -e $kubeletconf; then stat -c %a $kubeletconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: 644 + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the each worker node. \nFor example, chmod 644 $kubeletconf" @@ -254,7 +285,7 @@ groups: - id: 2.2.4 text: "Ensure that the kubelet file ownership is set to root:root (Scored)" - audit: "if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; fi" + audit: "/bin/sh -c 'if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; fi'" tests: test_items: - flag: "root:root" @@ -265,10 +296,24 @@ groups: - id: 2.2.5 text: "Ensure that the proxy file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $proxyconf; then stat -c %a $proxyconf; fi" + audit: "/bin/sh -c 'if test -e $proxyconf; then stat -c %a $proxyconf; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the below command (based on the file location on your system) on the each worker node. \nFor example, chmod 644 $proxyconf" @@ -276,7 +321,7 @@ groups: - id: 2.2.6 text: "Ensure that the proxy file ownership is set to root:root (Scored)" - audit: "if test -e $proxyconf; then stat -c %U:%G $proxyconf; fi" + audit: "/bin/sh -c 'if test -e $proxyconf; then stat -c %U:%G $proxyconf; fi'" tests: test_items: - flag: "root:root" @@ -288,10 +333,24 @@ groups: - id: 2.2.7 text: "Ensure that the certificate authorities file permissions are set to 644 or more restrictive (Scored)" - audit: "if test -e $ca-file; then stat -c %a $ca-file; fi" + audit: "/bin/sh -c 'if test -e $ca-file; then stat -c %a $ca-file; fi'" tests: + bin_op: or test_items: - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true remediation: "Run the following command to modify the file permissions of the --client-ca-file \nchmod 644 " @@ -299,7 +358,7 @@ groups: - id: 2.2.8 text: "Ensure that the client certificate authorities file ownership is set to root:root" - audit: "if test -e $ca-file; then stat -c %U:%G $ca-file; fi" + audit: "/bin/sh -c 'if test -e $ca-file; then stat -c %U:%G $ca-file; fi'" tests: test_items: - flag: "notexist:notexist" diff --git a/check/check.go b/check/check.go index 1f23a9e..4f91340 100644 --- a/check/check.go +++ b/check/check.go @@ -18,7 +18,9 @@ import ( "bytes" "fmt" "io" + "os" "os/exec" + "regexp" "strings" "github.com/golang/glog" @@ -83,8 +85,7 @@ func (c *Check) Run() { // Check if command exists or exit with WARN. for _, cmd := range c.Commands { - _, err := exec.LookPath(cmd.Path) - if err != nil { + if !isShellCommand(cmd.Path) { c.State = WARN return } @@ -119,7 +120,6 @@ func (c *Check) Run() { cs[i].Args, ), ) - i++ } @@ -166,18 +166,44 @@ func (c *Check) Run() { } } -// textToCommand transforms a text representation of commands to be +// textToCommand transforms an input text representation of commands to be // run into a slice of commands. // TODO: Make this more robust. func textToCommand(s string) []*exec.Cmd { cmds := []*exec.Cmd{} cp := strings.Split(s, "|") - // fmt.Println("check.toCommand:", cp) for _, v := range cp { v = strings.Trim(v, " ") - cs := strings.Split(v, " ") + + // TODO: + // GOAL: To split input text into arguments for exec.Cmd. + // + // CHALLENGE: The input text may contain quoted strings that + // must be passed as a unit to exec.Cmd. + // eg. bash -c 'foo bar' + // 'foo bar' must be passed as unit to exec.Cmd if not the command + // will fail when it is executed. + // eg. exec.Cmd("bash", "-c", "foo bar") + // + // PROBLEM: Current solution assumes the grouped string will always + // be at the end of the input text. + re := regexp.MustCompile(`^(.*)(['"].*['"])$`) + grps := re.FindStringSubmatch(v) + + var cs []string + if len(grps) > 0 { + s := strings.Trim(grps[1], " ") + cs = strings.Split(s, " ") + + s1 := grps[len(grps)-1] + s1 = strings.Trim(s1, "'\"") + + cs = append(cs, s1) + } else { + cs = strings.Split(v, " ") + } cmd := exec.Command(cs[0], cs[1:]...) cmds = append(cmds, cmd) @@ -185,3 +211,18 @@ func textToCommand(s string) []*exec.Cmd { return cmds } + +func isShellCommand(s string) bool { + cmd := exec.Command("/bin/sh", "-c", "command -v "+s) + + out, err := cmd.Output() + if err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) + os.Exit(1) + } + + if strings.Contains(string(out), s) { + return true + } + return false +} diff --git a/check/data b/check/data index 8e03efa..1e88841 100644 --- a/check/data +++ b/check/data @@ -7,59 +7,42 @@ groups: - id: 1.1 text: "Kube-apiserver" checks: - - id: 1.1.1 - text: "Ensure that the --allow-privileged argument is set (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 0 + text: "flag is set" tests: test_items: - - - flag: "--allow-privileged" + - flag: "--allow-privileged" set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.2 - text: "Ensure that the --basic-auth argument is not set (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 1 + text: "flag is not set" tests: test_item: - - - flag: "--basic-auth" + - flag: "--basic-auth" set: false - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.3 - text: "Ensure that the --insecure-port argument is set to 0 (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 2 + text: "flag value is set to some value" tests: test_items: - - - flag: "--insecure-port" + - flag: "--insecure-port" compare: op: eq value: 0 set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.4 - text: "Ensure that the --audit-log-maxage argument is set to 30 or appropriate (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 3 + text: "flag value is greater than or equal some number" tests: test_items: - - - flag: "--audit-log-maxage" + - flag: "--audit-log-maxage" compare: op: gte value: 30 set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.5 - text: "Ensure that the --max-backlog argument is set to 30 or less (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 4 + text: "flag value is less than some number" tests: test_items: - flag: "--max-backlog" @@ -67,26 +50,19 @@ groups: op: lt value: 30 set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.6 - text: "Ensure admission control does not include AlwaysAdmit (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 5 + text: "flag value does not have some value" tests: test_items: - - - flag: "--admission-control" + - flag: "--admission-control" compare: op: nothave value: AlwaysAdmit set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.7 - text: "Ensure that the --kubelet-client-certificate and --kubelet-clientkey arguments are set as appropriate (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 6 + text: "test AND binary operation" tests: bin_op: and test_items: @@ -94,17 +70,13 @@ groups: set: true - flag: "--kubelet-clientkey" set: true - remediation: "Edit the /etc/kubernetes/config file on the master node and set the KUBE_ALLOW_PRIV parameter to '--allow-privileged=false'" - scored: true - - id: 1.1.8 - text: "Ensure that the --secure-port argument is not set to 0 (Scored)" - audit: "ps -ef | grep kube-apiserver | grep -v grep" + - id: 7 + text: "test OR binary operation" tests: bin_op: or test_items: - - - flag: "--secure-port" + - flag: "--secure-port" compare: op: eq value: 0 @@ -112,28 +84,35 @@ groups: - flag: "--secure-port" set: false - remediation: "Edit the /etc/kubernetes/apiserver file on the master node and either remove the -secure-port argument from the KUBE_API_ARGS parameter or set it to a different desired port." - scored: true - - id: 1.4.1 - text: "Ensure that the apiserver file permissions are set to 644 or more restrictive (Scored)" - audit: "stat -c %a /etc/kubernetes/apiserver" + - id: 8 + text: "test flag with arbitrary text" tests: test_items: - flag: "644" - set: true - remediation: "Run the below command (based on the file location on your system) on the master node. For example, chmod 644 /etc/kubernetes/apiserver" - scored: true - - - id: 2.1.14 - text: "Ensure that the apiserver file permissions are set to 644 or more restrictive (Scored)" - audit: "ps -ef | grep kubelet | grep -v grep" - tests: - test_items: - - flag: "KubeletClient" compare: op: eq - value: true + value: "644" + set: true + + - id: 9 + text: "test permissions" + audit: "/bin/sh -c 'if test -e $config; then stat -c %a $config; fi'" + tests: + bin_op: or + test_items: + - flag: "644" + compare: + op: eq + value: "644" + set: true + - flag: "640" + compare: + op: eq + value: "640" + set: true + - flag: "600" + compare: + op: eq + value: "600" set: true - remediation: "Run the below command (based on the file location on your system) on the master node. For example, chmod 644 /etc/kubernetes/apiserver" - scored: true diff --git a/check/test.go b/check/test.go index f89e989..06c1b93 100644 --- a/check/test.go +++ b/check/test.go @@ -38,6 +38,7 @@ const ( type testItem struct { Flag string + Output string Value string Set bool Compare compare @@ -57,14 +58,22 @@ func (t *testItem) execute(s string) (result bool) { isset := match if isset && t.Compare.Op != "" { - pttn := t.Flag + `=([^\s,]*) *` + // Expects flags in the form; + // --flag=somevalue + // --flag + // somevalue + pttn := `(` + t.Flag + `)(=)*([^\s,]*) *` flagRe := regexp.MustCompile(pttn) vals := flagRe.FindStringSubmatch(s) if len(vals) > 0 { - flagVal = vals[1] + if vals[3] != "" { + flagVal = vals[3] + } else { + flagVal = vals[1] + } } else { - fmt.Fprintf(os.Stderr, "expected value for %s but none found\n", t.Flag) + fmt.Fprintf(os.Stderr, "invalid flag in testitem definition") os.Exit(1) } diff --git a/check/test_test.go b/check/test_test.go index 7fc5832..a0228c2 100644 --- a/check/test_test.go +++ b/check/test_test.go @@ -16,6 +16,8 @@ package check import ( "io/ioutil" + "os" + "strings" "testing" ) @@ -30,79 +32,74 @@ func init() { if err != nil { panic("Failed reading test data: " + err.Error()) } - controls, err = NewControls(MASTER, in) + + // substitute variables in data file + user := os.Getenv("USER") + s := strings.Replace(string(in), "$user", user, -1) + + controls, err = NewControls(MASTER, []byte(s)) + // controls, err = NewControls(MASTER, in) if err != nil { panic("Failed creating test controls: " + err.Error()) } } func TestTestExecute(t *testing.T) { + cases := []struct { - *tests - testfor string - str string + *Check + str string }{ { - controls.Groups[0].Checks[0].Tests, - "flag set", + controls.Groups[0].Checks[0], "2:45 ../kubernetes/kube-apiserver --allow-privileged=false --option1=20,30,40", }, { - controls.Groups[0].Checks[1].Tests, - "flag not set", + controls.Groups[0].Checks[1], "2:45 ../kubernetes/kube-apiserver --allow-privileged=false", }, { - controls.Groups[0].Checks[2].Tests, - "flag and value set", + controls.Groups[0].Checks[2], "niinai 13617 2635 99 19:26 pts/20 00:03:08 ./kube-apiserver --insecure-port=0 --anonymous-auth", }, { - controls.Groups[0].Checks[3].Tests, - "flag value greater than value", + controls.Groups[0].Checks[3], "2:45 ../kubernetes/kube-apiserver --secure-port=0 --audit-log-maxage=40 --option", }, { - controls.Groups[0].Checks[4].Tests, - "flag value less than value", + controls.Groups[0].Checks[4], "2:45 ../kubernetes/kube-apiserver --max-backlog=20 --secure-port=0 --audit-log-maxage=40 --option", }, { - controls.Groups[0].Checks[5].Tests, - "flag value does not have", + controls.Groups[0].Checks[5], "2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40", }, { - controls.Groups[0].Checks[6].Tests, - "AND multiple tests, all testitems pass", + controls.Groups[0].Checks[6], "2:45 .. --kubelet-clientkey=foo --kubelet-client-certificate=bar --admission-control=Webhook,RBAC", }, { - controls.Groups[0].Checks[7].Tests, - "OR multiple tests", + controls.Groups[0].Checks[7], "2:45 .. --secure-port=0 --kubelet-client-certificate=bar --admission-control=Webhook,RBAC", }, { - controls.Groups[0].Checks[8].Tests, - "text", + controls.Groups[0].Checks[8], "644", }, { - controls.Groups[0].Checks[9].Tests, - "flag value is comma-separated", - "2:35 ../kubelet --features-gates=KubeletClient=true,KubeletServer=true", + controls.Groups[0].Checks[9], + "640", }, { - controls.Groups[0].Checks[9].Tests, - "flag value is comma-separated", - "2:35 ../kubelet --features-gates=KubeletServer=true,KubeletClient=true", + controls.Groups[0].Checks[9], + "600", }, } for _, c := range cases { - res := c.tests.execute(c.str) + res := c.Tests.execute(c.str) if !res { - t.Errorf("%s, expected:%v, got:%v\n", c.testfor, true, res) + t.Errorf("%s, expected:%v, got:%v\n", c.Text, true, res) } } } diff --git a/cmd/common.go b/cmd/common.go index d27dc1f..89f45bc 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -17,7 +17,7 @@ package cmd import ( "fmt" "io/ioutil" - "strings" + "os" "github.com/aquasecurity/kube-bench/check" "github.com/spf13/viper" @@ -96,26 +96,26 @@ func runChecks(t check.NodeType) { } // Variable substitutions. Replace all occurrences of variables in controls files. - s := strings.Replace(string(in), "$apiserverbin", apiserverBin, -1) - s = strings.Replace(s, "$apiserverconf", apiserverConf, -1) - s = strings.Replace(s, "$schedulerbin", schedulerBin, -1) - 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, "$config", config, -1) + s := multiWordReplace(string(in), "$apiserverbin", apiserverBin) + s = multiWordReplace(s, "$apiserverconf", apiserverConf) + s = multiWordReplace(s, "$schedulerbin", schedulerBin) + s = multiWordReplace(s, "$schedulerconf", schedulerConf) + s = multiWordReplace(s, "$controllermanagerbin", controllerManagerBin) + s = multiWordReplace(s, "$controllermanagerconf", controllerManagerConf) + s = multiWordReplace(s, "$config", config) - s = strings.Replace(s, "$etcdbin", etcdBin, -1) - s = strings.Replace(s, "$etcdconf", etcdConf, -1) - s = strings.Replace(s, "$flanneldbin", flanneldBin, -1) - s = strings.Replace(s, "$flanneldconf", flanneldConf, -1) + s = multiWordReplace(s, "$etcdbin", etcdBin) + s = multiWordReplace(s, "$etcdconf", etcdConf) + s = multiWordReplace(s, "$flanneldbin", flanneldBin) + s = multiWordReplace(s, "$flanneldconf", flanneldConf) - s = strings.Replace(s, "$kubeletbin", kubeletBin, -1) - s = strings.Replace(s, "$kubeletconf", kubeletConf, -1) - s = strings.Replace(s, "$proxybin", proxyBin, -1) - s = strings.Replace(s, "$proxyconf", proxyConf, -1) + s = multiWordReplace(s, "$kubeletbin", kubeletBin) + s = multiWordReplace(s, "$kubeletconf", kubeletConf) + s = multiWordReplace(s, "$proxybin", proxyBin) + s = multiWordReplace(s, "$proxyconf", proxyConf) - s = strings.Replace(s, "$fedapiserverbin", fedApiserverBin, -1) - s = strings.Replace(s, "$fedcontrollermanagerbin", fedControllerManagerBin, -1) + s = multiWordReplace(s, "$fedapiserverbin", fedApiserverBin) + s = multiWordReplace(s, "$fedcontrollermanagerbin", fedControllerManagerBin) controls, err := check.NewControls(t, []byte(s)) if err != nil { @@ -150,15 +150,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 937e3e0..478ae21 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -64,64 +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(missing) > 0 { - missing = strings.Trim(missing, ", ") - printlnWarn(fmt.Sprintf("Missing kubernetes binaries: %s", missing)) - } + // Strip any quotes + bin = strings.Trim(bin, "'\"") - if len(notRunning) > 0 { - notRunning = strings.Trim(notRunning, ", ") - printlnWarn(fmt.Sprintf("Kubernetes binaries not running: %s", notRunning)) - } + // 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) { @@ -138,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) @@ -182,3 +150,12 @@ func versionMatch(r *regexp.Regexp, s string) string { } return match[1] } + +func multiWordReplace(s string, subname string, sub string) string { + f := strings.Fields(sub) + if len(f) > 1 { + sub = "'" + sub + "'" + } + + return strings.Replace(s, subname, sub, -1) +} diff --git a/cmd/util_test.go b/cmd/util_test.go index 74d955f..dbd434b 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,56 @@ 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) + } + }) + } +} + +func TestMultiWordReplace(t *testing.T) { + cases := []struct { + input string + sub string + subname string + output string + }{ + {input: "Here's a file with no substitutions", sub: "blah", subname: "blah", output: "Here's a file with no substitutions"}, + {input: "Here's a file with a substitution", sub: "blah", subname: "substitution", output: "Here's a file with a blah"}, + {input: "Here's a file with multi-word substitutions", sub: "multi word", subname: "multi-word", output: "Here's a file with 'multi word' substitutions"}, + {input: "Here's a file with several several substitutions several", sub: "blah", subname: "several", output: "Here's a file with blah blah substitutions blah"}, + } + for id, c := range cases { + t.Run(strconv.Itoa(id), func(t *testing.T) { + s := multiWordReplace(c.input, c.subname, c.sub) + if s != c.output { + t.Fatalf("Expected %s got %s", c.output, s) + } + }) + } +}