Better handling of parameters and config audits (#674)

* read-only-port defaults are correct

* Tests that should catch good read-only-port

* Rework checks & tests

* Linting on issue template YAML

* More explicit test for 4.2.4

.github/ISSUE_TEMPLATE/config.yml

@@ -1,3 +1,4 @@
+---
 blank_issues_enabled: false
 contact_links:
   - name: Feature request

cfg/cis-1.5/node.yaml

@@ -236,6 +236,7 @@ groups:
         audit: "/bin/ps -fC $kubeletbin"
         audit_config: "/bin/cat $kubeletconf"
         tests:
+          bin_op: or
           test_items:
             - flag: "--read-only-port"
               path: '{.readOnlyPort}'
@@ -243,6 +244,9 @@ groups:
               compare:
                 op: eq
                 value: 0
+            - flag: "--read-only-port"
+              path: '{.readOnlyPort}'
+              set: false
         remediation: |
           If using a Kubelet config file, edit the file to set readOnlyPort to 0.
           If using command line arguments, edit the kubelet service file

check/check.go

@@ -62,21 +62,23 @@ const (
 // Check contains information about a recommendation in the
 // CIS Kubernetes document.
 type Check struct {
-	ID             string   `yaml:"id" json:"test_number"`
+	ID                string   `yaml:"id" json:"test_number"`
-	Text           string   `json:"test_desc"`
+	Text              string   `json:"test_desc"`
-	Audit          string   `json:"audit"`
+	Audit             string   `json:"audit"`
-	AuditConfig    string   `yaml:"audit_config"`
+	AuditConfig       string   `yaml:"audit_config"`
-	Type           string   `json:"type"`
+	Type              string   `json:"type"`
-	Tests          *tests   `json:"omit"`
+	Tests             *tests   `json:"omit"`
-	Set            bool     `json:"omit"`
+	Set               bool     `json:"omit"`
-	Remediation    string   `json:"remediation"`
+	Remediation       string   `json:"remediation"`
-	TestInfo       []string `json:"test_info"`
+	TestInfo          []string `json:"test_info"`
-	State          `json:"status"`
+	State             `json:"status"`
-	ActualValue    string `json:"actual_value"`
+	ActualValue       string `json:"actual_value"`
-	Scored         bool   `json:"scored"`
+	Scored            bool   `json:"scored"`
-	IsMultiple     bool   `yaml:"use_multiple_values"`
+	IsMultiple        bool   `yaml:"use_multiple_values"`
-	ExpectedResult string `json:"expected_result"`
+	ExpectedResult    string `json:"expected_result"`
-	Reason         string `json:"reason,omitempty"`
+	Reason            string `json:"reason,omitempty"`
+	AuditOutput       string `json:"omit"`
+	AuditConfigOutput string `json:"omit"`
 }
 
 // Runner wraps the basic Run method.
@@ -123,63 +125,46 @@ func (c *Check) run() State {
 		return c.State
 	}
 
-	lastCommand := c.Audit
+	// If there aren't any tests defined this is a FAIL or WARN
-	hasAuditConfig := c.AuditConfig != ""
+	if c.Tests == nil || len(c.Tests.TestItems) == 0 {
-
+		c.Reason = "No tests defined"
-	state, finalOutput, retErrmsgs := performTest(c.Audit, c.Tests, c.IsMultiple)
+		if c.Scored {
-	if len(state) > 0 {
+			c.State = FAIL
-		c.Reason = retErrmsgs
+		} else {
-		c.State = state
+			c.State = WARN
+		}
 		return c.State
 	}
-	errmsgs := retErrmsgs
-
-	// If something went wrong with the 'Audit' command
-	// and an 'AuditConfig' command was provided, use it to
-	// execute tests
-	if (finalOutput == nil || !finalOutput.testResult) && hasAuditConfig {
-		lastCommand = c.AuditConfig
-
-		nItems := len(c.Tests.TestItems)
-		// The reason we're creating a copy of the "tests"
-		// is so that tests can executed
-		// with the AuditConfig command
-		// against the Path only
-		currentTests := &tests{
-			BinOp:     c.Tests.BinOp,
-			TestItems: make([]*testItem, nItems),
-		}
 
-		for i := 0; i < nItems; i++ {
+	// Command line parameters override the setting in the config file, so if we get a good result from the Audit command that's all we need to run
-			ti := c.Tests.TestItems[i]
+	var finalOutput *testOutput
-			nti := &testItem{
+	var lastCommand string
-				// Path is used to test Command Param values
-				// AuditConfig ==> Path
-				Path:    ti.Path,
-				Set:     ti.Set,
-				Compare: ti.Compare,
-			}
-			currentTests.TestItems[i] = nti
-		}
 
-		state, finalOutput, retErrmsgs = performTest(c.AuditConfig, currentTests, c.IsMultiple)
+	lastCommand, err := c.runAuditCommands()
-		if len(state) > 0 {
+	if err == nil {
-			c.Reason = retErrmsgs
+		finalOutput, err = c.execute()
-			c.State = state
-			return c.State
-		}
-		errmsgs += retErrmsgs
 	}
 
-	if finalOutput != nil && finalOutput.testResult {
+	if finalOutput != nil {
-		c.State = PASS
+		if finalOutput.testResult {
+			c.State = PASS
+		} else {
+			if c.Scored {
+				c.State = FAIL
+			} else {
+				c.State = WARN
+			}
+		}
+
 		c.ActualValue = finalOutput.actualResult
 		c.ExpectedResult = finalOutput.ExpectedResult
-	} else {
+	}
+
+	if err != nil {
+		c.Reason = err.Error()
 		if c.Scored {
 			c.State = FAIL
 		} else {
-			c.Reason = errmsgs
 			c.State = WARN
 		}
 	}
@@ -190,39 +175,97 @@ func (c *Check) run() State {
 		glog.V(3).Infof("Check.ID: %s Command: %q TestResult: <<EMPTY>> \n", c.ID, lastCommand)
 	}
 
-	if errmsgs != "" {
+	if c.Reason != "" {
-		glog.V(2).Info(errmsgs)
+		glog.V(2).Info(c.Reason)
 	}
 	return c.State
 }
 
-func performTest(audit string, tests *tests, isMultipleOutput bool) (State, *testOutput, string) {
+func (c *Check) runAuditCommands() (lastCommand string, err error) {
-	if len(strings.TrimSpace(audit)) == 0 {
+	// Run the audit command and auditConfig commands, if present
-		return "", failTestItem("missing command"), "missing audit command"
+	c.AuditOutput, err = runAudit(c.Audit)
+	if err != nil {
+		return c.Audit, err
 	}
 
-	var out bytes.Buffer
+	c.AuditConfigOutput, err = runAudit(c.AuditConfig)
-	errmsgs := runAudit(audit, &out)
+	return c.AuditConfig, err
+}
+
+func (c *Check) execute() (finalOutput *testOutput, err error) {
+	finalOutput = &testOutput{}
+
+	ts := c.Tests
+	res := make([]testOutput, len(ts.TestItems))
+	expectedResultArr := make([]string, len(res))
+
+	glog.V(3).Infof("%d tests", len(ts.TestItems))
+	for i, t := range ts.TestItems {
+
+		t.isMultipleOutput = c.IsMultiple
 
-	finalOutput := tests.execute(out.String(), isMultipleOutput)
+		// Try with the auditOutput first, and if that's not found, try the auditConfigOutput
-	if finalOutput == nil {
+		t.isConfigSetting = false
-		errmsgs += fmt.Sprintf("Final output is <<EMPTY>>. Failed to run: %s\n", audit)
+		result := *(t.execute(c.AuditOutput))
+		if !result.flagFound {
+			t.isConfigSetting = true
+			result = *(t.execute(c.AuditConfigOutput))
+		}
+		res[i] = result
+		expectedResultArr[i] = res[i].ExpectedResult
 	}
 
-	return "", finalOutput, errmsgs
+	var result bool
+	// If no binary operation is specified, default to AND
+	switch ts.BinOp {
+	default:
+		glog.V(2).Info(fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp))
+		finalOutput.actualResult = fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp)
+		return finalOutput, fmt.Errorf("unknown binary operator for tests %s", ts.BinOp)
+	case and, "":
+		result = true
+		for i := range res {
+			result = result && res[i].testResult
+		}
+		// Generate an AND expected result
+		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " AND ")
+
+	case or:
+		result = false
+		for i := range res {
+			result = result || res[i].testResult
+		}
+		// Generate an OR expected result
+		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " OR ")
+	}
+
+	finalOutput.testResult = result
+	finalOutput.actualResult = res[0].actualResult
+
+	glog.V(3).Infof("Returning from execute on tests: finalOutput %#v", finalOutput)
+	return finalOutput, nil
 }
 
-func runAudit(audit string, out *bytes.Buffer) string {
+func runAudit(audit string) (output string, err error) {
-	errmsgs := ""
+	var out bytes.Buffer
+
+	audit = strings.TrimSpace(audit)
+	if len(audit) == 0 {
+		return output, err
+	}
 
 	cmd := exec.Command("/bin/sh")
 	cmd.Stdin = strings.NewReader(audit)
-	cmd.Stdout = out
+	cmd.Stdout = &out
-	cmd.Stderr = out
+	cmd.Stderr = &out
-	if err := cmd.Run(); err != nil {
+	err = cmd.Run()
-		errmsgs += fmt.Sprintf("failed to run: %q, output: %q, error: %s\n", audit, out.String(), err)
+	output = out.String()
-	}
+
+	if err != nil {
+		err = fmt.Errorf("failed to run: %q, output: %q, error: %s", audit, output, err)
+	} else {
+		glog.V(3).Infof("Command %q\n - Output:\n %q", audit, output)
 
-	glog.V(3).Infof("Command %q - Output:\n\n %q\n - Error Messages:%q \n", audit, out.String(), errmsgs)
+	}
-	return errmsgs
+	return output, err
 }

check/check_test.go

@@ -15,38 +15,57 @@
 package check
 
 import (
-	"bytes"
 	"strings"
 	"testing"
 )
 
 func TestCheck_Run(t *testing.T) {
 	type TestCase struct {
+		name     string
 		check    Check
 		Expected State
 	}
 
 	testCases := []TestCase{
-		{check: Check{Type: MANUAL}, Expected: WARN},
+		{name: "Manual check should WARN", check: Check{Type: MANUAL}, Expected: WARN},
-		{check: Check{Type: "skip"}, Expected: INFO},
+		{name: "Skip check should INFO", check: Check{Type: "skip"}, Expected: INFO},
-
+		{name: "Unscored check (with no type) should WARN on failure", check: Check{Scored: false}, Expected: WARN},
-		{check: Check{Scored: false}, Expected: WARN}, // Not scored checks with no type, or not scored failing tests are marked warn
 		{
-			check: Check{ // Not scored checks with passing tests are marked pass
+			name: "Unscored check that pass should PASS",
+			check: Check{
 				Scored: false,
-				Audit:  ":",
+				Audit:  "echo hello",
-				Tests:  &tests{TestItems: []*testItem{&testItem{}}},
+				Tests: &tests{TestItems: []*testItem{{
+					Flag: "hello",
+					Set:  true,
+				}}},
 			},
 			Expected: PASS,
 		},
 
-		{check: Check{Scored: true}, Expected: WARN},                  // If there are no tests in the check, warn
+		{name: "Check with no tests should WARN", check: Check{Scored: true}, Expected: WARN},
-		{check: Check{Scored: true, Tests: &tests{}}, Expected: FAIL}, // If there are tests that are not passing, fail
+		{name: "Scored check with empty tests should FAIL", check: Check{Scored: true, Tests: &tests{}}, Expected: FAIL},
 		{
-			check: Check{ // Scored checks with passing tests are marked pass
+			name: "Scored check that doesn't pass should FAIL",
+			check: Check{
 				Scored: true,
-				Audit:  ":",
+				Audit:  "echo hello",
-				Tests:  &tests{TestItems: []*testItem{&testItem{}}},
+				Tests: &tests{TestItems: []*testItem{{
+					Flag: "hello",
+					Set:  false,
+				}},
+				}},
+			Expected: FAIL,
+		},
+		{
+			name: "Scored checks that pass should PASS",
+			check: Check{
+				Scored: true,
+				Audit:  "echo hello",
+				Tests: &tests{TestItems: []*testItem{{
+					Flag: "hello",
+					Set:  true,
+				}}},
 			},
 			Expected: PASS,
 		},
@@ -56,7 +75,7 @@ func TestCheck_Run(t *testing.T) {
 		testCase.check.run()
 
 		if testCase.check.State != testCase.Expected {
-			t.Errorf("test failed, expected %s, actual %s\n", testCase.Expected, testCase.check.State)
+			t.Errorf("%s: expected %s, actual %s\n", testCase.name, testCase.Expected, testCase.check.State)
 		}
 	}
 }
@@ -115,6 +134,26 @@ func TestCheckAuditConfig(t *testing.T) {
 			controls.Groups[1].Checks[11],
 			"FAIL",
 		},
+		{
+			controls.Groups[1].Checks[12],
+			"FAIL",
+		},
+		{
+			controls.Groups[1].Checks[13],
+			"FAIL",
+		},
+		{
+			controls.Groups[1].Checks[14],
+			"FAIL",
+		},
+		{
+			controls.Groups[1].Checks[15],
+			"PASS",
+		},
+		{
+			controls.Groups[1].Checks[16],
+			"FAIL",
+		},
 	}
 
 	for _, c := range cases {
@@ -128,7 +167,6 @@ func TestCheckAuditConfig(t *testing.T) {
 func Test_runAudit(t *testing.T) {
 	type args struct {
 		audit  string
-		out    *bytes.Buffer
 		output string
 	}
 	tests := []struct {
@@ -141,7 +179,6 @@ func Test_runAudit(t *testing.T) {
 			name: "run success",
 			args: args{
 				audit: "echo 'hello world'",
-				out:   &bytes.Buffer{},
 			},
 			errMsg: "",
 			output: "hello world\n",
@@ -156,7 +193,6 @@ hello() {
 
 hello
 `,
-				out: &bytes.Buffer{},
 			},
 			errMsg: "",
 			output: "hello world\n",
@@ -165,7 +201,6 @@ hello
 			name: "run failed",
 			args: args{
 				audit: "unknown_command",
-				out:   &bytes.Buffer{},
 			},
 			errMsg: "failed to run: \"unknown_command\", output: \"/bin/sh: ",
 			output: "not found\n",
@@ -173,16 +208,19 @@ hello
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			errMsg := runAudit(tt.args.audit, tt.args.out)
+			var errMsg string
+			output, err := runAudit(tt.args.audit)
+			if err != nil {
+				errMsg = err.Error()
+			}
 			if errMsg != "" && !strings.Contains(errMsg, tt.errMsg) {
-				t.Errorf("runAudit() errMsg = %q, want %q", errMsg, tt.errMsg)
+				t.Errorf("name %s errMsg = %q, want %q", tt.name, errMsg, tt.errMsg)
 			}
-			output := tt.args.out.String()
 			if errMsg == "" && output != tt.output {
-				t.Errorf("runAudit() output = %q, want %q", output, tt.output)
+				t.Errorf("name %s output = %q, want %q", tt.name, output, tt.output)
 			}
 			if errMsg != "" && !strings.Contains(output, tt.output) {
-				t.Errorf("runAudit() output = %q, want %q", output, tt.output)
+				t.Errorf("name %s output = %q, want %q", tt.name, output, tt.output)
 			}
 		})
 	}

check/data

@@ -166,7 +166,7 @@ groups:
               op: eq
               value: some-val
             set: true
-            
+
     - id: 15
       text: "jsonpath correct value on field"
       tests:
@@ -476,3 +476,84 @@ groups:
               value: "600"
             set: true
       scored: true
+    - id: 12
+      text: "audit is present and wrong, audit_config is right -> fail (command line parameters override config file)"
+      audit: "echo flag=wrong"
+      audit_config: "echo 'flag: correct'"
+      tests:
+        test_items:
+          - flag: "flag"
+            path: "{.flag}"
+            compare:
+              op: eq
+              value: "correct"
+            set: true
+      scored: true
+    - id: 13
+      text: "parameter and config file don't have same default - parameter has failing value"
+      audit: "echo '--read-only-port=1'"
+      audit_config: "echo 'readOnlyPort: 0'"
+      tests:
+        bin_op: and
+        test_items:
+          - flag: "--read-only-port"
+            path: "{.readOnlyPort}"
+            set: true
+            compare:
+              op: eq
+              value: 0
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: false
+      scored: true
+    - id: 14
+      text: "parameter and config file don't have same default - config file has failing value"
+      audit: "echo ''"
+      audit_config: "echo 'readOnlyPort: 1'"
+      tests:
+        bin_op: or
+        test_items:
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: true
+            compare:
+              op: eq
+              value: 0
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: false
+      scored: true
+    - id: 15
+      text: "parameter and config file don't have same default - passing"
+      audit: "echo ''"
+      audit_config: "echo ''"
+      tests:
+        bin_op: or
+        test_items:
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: true
+            compare:
+              op: eq
+              value: 0
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: false
+      scored: true
+    - id: 15
+      text: "parameter and config file don't have same default - parameter has bad value and config is not present - failing"
+      audit: "echo '--read-only-port=1'"
+      audit_config: "echo ''"
+      tests:
+        bin_op: or
+        test_items:
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: true
+            compare:
+              op: eq
+              value: 0
+          - flag: "--read-only-port"
+            path: '{.readOnlyPort}'
+            set: false
+      scored: true

check/test.go

@@ -43,15 +43,25 @@ const (
 	defaultArraySeparator       = ","
 )
 
+type tests struct {
+	TestItems []*testItem `yaml:"test_items"`
+	BinOp     binOp       `yaml:"bin_op"`
+}
+
 type testItem struct {
-	Flag    string
+	Flag             string
-	Path    string
+	Path             string
-	Output  string
+	Output           string
-	Value   string
+	Value            string
-	Set     bool
+	Set              bool
-	Compare compare
+	Compare          compare
+	isMultipleOutput bool
+	isConfigSetting  bool
 }
 
+type pathTestItem testItem
+type flagTestItem testItem
+
 type compare struct {
 	Op    string
 	Value string
@@ -59,6 +69,7 @@ type compare struct {
 
 type testOutput struct {
 	testResult     bool
+	flagFound      bool
 	actualResult   string
 	ExpectedResult string
 }
@@ -67,99 +78,124 @@ func failTestItem(s string) *testOutput {
 	return &testOutput{testResult: false, actualResult: s}
 }
 
-func (t *testItem) execute(s string, isMultipleOutput bool) *testOutput {
+func (t testItem) flagValue() string {
-	result := &testOutput{}
+	if t.isConfigSetting {
-	s = strings.TrimRight(s, " \n")
+		return t.Path
+	}
 
-	// If the test has output that should be evaluated for each row
+	return t.Flag
-	if isMultipleOutput {
+}
-		output := strings.Split(s, "\n")
+
-		for _, op := range output {
+func (t testItem) findValue(s string) (match bool, value string, err error) {
-			result = t.evaluate(op)
+	if t.isConfigSetting {
-			// If the test failed for the current row, no need to keep testing for this output
+		pt := pathTestItem(t)
-			if !result.testResult {
+		return pt.findValue(s)
-				break
+	}
+
+	ft := flagTestItem(t)
+	return ft.findValue(s)
+}
+
+func (t flagTestItem) findValue(s string) (match bool, value string, err error) {
+	if s == "" || t.Flag == "" {
+		return
+	}
+	match = strings.Contains(s, t.Flag)
+	if match {
+		// Expects flags in the form;
+		// --flag=somevalue
+		// flag: somevalue
+		// --flag
+		// somevalue
+		pttn := `(` + t.Flag + `)(=|: *)*([^\s]*) *`
+		flagRe := regexp.MustCompile(pttn)
+		vals := flagRe.FindStringSubmatch(s)
+
+		if len(vals) > 0 {
+			if vals[3] != "" {
+				value = vals[3]
+			} else {
+				// --bool-flag
+				if strings.HasPrefix(t.Flag, "--") {
+					value = "true"
+				} else {
+					value = vals[1]
+				}
 			}
+		} else {
+			err = fmt.Errorf("invalid flag in testItem definition: %s", s)
 		}
-	} else {
-		result = t.evaluate(s)
 	}
+	glog.V(3).Infof("In flagTestItem.findValue %s, match %v, s %s, t.Flag %s", value, match, s, t.Flag)
 
-	return result
+	return match, value, err
+}
+
+func (t pathTestItem) findValue(s string) (match bool, value string, err error) {
+	var jsonInterface interface{}
+
+	err = unmarshal(s, &jsonInterface)
+	if err != nil {
+		return false, "", fmt.Errorf("failed to load YAML or JSON from input \"%s\": %v", s, err)
+	}
+
+	value, err = executeJSONPath(t.Path, &jsonInterface)
+	if err != nil {
+		return false, "", fmt.Errorf("unable to parse path expression \"%s\": %v", t.Path, err)
+	}
+
+	glog.V(3).Infof("In pathTestItem.findValue %s", value)
+	match = (value != "")
+	return match, value, err
 }
 
-func (t *testItem) evaluate(s string) *testOutput {
+func (t testItem) execute(s string) *testOutput {
 	result := &testOutput{}
-	var match bool
+	s = strings.TrimRight(s, " \n")
-	var flagVal string
 
-	if t.Flag != "" {
+	// If the test has output that should be evaluated for each row
-		// Flag comparison: check if the flag is present in the input
+	var output []string
-		match = strings.Contains(s, t.Flag)
+	if t.isMultipleOutput {
+		output = strings.Split(s, "\n")
 	} else {
-		// Path != "" - we don't know whether it's YAML or JSON but
+		output = []string{s}
-		// we can just try one then the other
+	}
-		var jsonInterface interface{}
-
-		if t.Path != "" {
-			err := unmarshal(s, &jsonInterface)
-			if err != nil {
-				fmt.Fprintf(os.Stderr, "failed to load YAML or JSON from provided input \"%s\": %v\n", s, err)
-				return failTestItem("failed to load YAML or JSON")
-			}
 
+	for _, op := range output {
+		result = t.evaluate(op)
+		// If the test failed for the current row, no need to keep testing for this output
+		if !result.testResult {
+			break
 		}
+	}
 
-		jsonpathResult, err := executeJSONPath(t.Path, &jsonInterface)
+	return result
-		if err != nil {
+}
-			fmt.Fprintf(os.Stderr, "unable to parse path expression \"%s\": %v\n", t.Path, err)
+
-			return failTestItem("error executing path expression")
+func (t testItem) evaluate(s string) *testOutput {
-		}
+	result := &testOutput{}
-		match = (jsonpathResult != "")
+
-		flagVal = jsonpathResult
+	match, value, err := t.findValue(s)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, err.Error())
+		return failTestItem(err.Error())
 	}
 
 	if t.Set {
-		isset := match
+		if match && t.Compare.Op != "" {
-
+			result.ExpectedResult, result.testResult = compareOp(t.Compare.Op, value, t.Compare.Value)
-		if isset && t.Compare.Op != "" {
-			if t.Flag != "" {
-				// Expects flags in the form;
-				// --flag=somevalue
-				// flag: somevalue
-				// --flag
-				// somevalue
-				pttn := `(` + t.Flag + `)(=|: *)*([^\s]*) *`
-				flagRe := regexp.MustCompile(pttn)
-				vals := flagRe.FindStringSubmatch(s)
-
-				if len(vals) > 0 {
-					if vals[3] != "" {
-						flagVal = vals[3]
-					} else {
-						// --bool-flag
-						if strings.HasPrefix(t.Flag, "--") {
-							flagVal = "true"
-						} else {
-							flagVal = vals[1]
-						}
-					}
-				} else {
-					glog.V(1).Infof(fmt.Sprintf("invalid flag in testitem definition"))
-					return failTestItem("error invalid flag in testitem definition")
-				}
-			}
-
-			result.ExpectedResult, result.testResult = compareOp(t.Compare.Op, flagVal, t.Compare.Value)
 		} else {
-			result.ExpectedResult = fmt.Sprintf("'%s' is present", t.Flag)
+			result.ExpectedResult = fmt.Sprintf("'%s' is present", t.flagValue())
-			result.testResult = isset
+			result.testResult = match
 		}
 	} else {
-		result.ExpectedResult = fmt.Sprintf("'%s' is not present", t.Flag)
+		result.ExpectedResult = fmt.Sprintf("'%s' is not present", t.flagValue())
-		notset := !match
+		result.testResult = !match
-		result.testResult = notset
 	}
+
+	result.flagFound = match
+	glog.V(3).Info(fmt.Sprintf("flagFound %v", result.flagFound))
+
 	return result
 }
 
@@ -326,66 +362,6 @@ func splitAndRemoveLastSeparator(s, sep string) []string {
 	return ts
 }
 
-type tests struct {
-	TestItems []*testItem `yaml:"test_items"`
-	BinOp     binOp       `yaml:"bin_op"`
-}
-
-func (ts *tests) execute(s string, isMultipleOutput bool) *testOutput {
-	finalOutput := &testOutput{}
-
-	// If no tests are defined return with empty finalOutput.
-	// This may be the case for checks of type: "skip".
-	if ts == nil {
-		return finalOutput
-	}
-
-	res := make([]testOutput, len(ts.TestItems))
-	if len(res) == 0 {
-		return finalOutput
-	}
-
-	expectedResultArr := make([]string, len(res))
-
-	for i, t := range ts.TestItems {
-		res[i] = *(t.execute(s, isMultipleOutput))
-		expectedResultArr[i] = res[i].ExpectedResult
-	}
-
-	var result bool
-	// If no binary operation is specified, default to AND
-	switch ts.BinOp {
-	default:
-		glog.V(2).Info(fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp))
-		finalOutput.actualResult = fmt.Sprintf("unknown binary operator for tests %s\n", ts.BinOp)
-		return finalOutput
-	case and, "":
-		result = true
-		for i := range res {
-			result = result && res[i].testResult
-		}
-		// Generate an AND expected result
-		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " AND ")
-
-	case or:
-		result = false
-		for i := range res {
-			result = result || res[i].testResult
-		}
-		// Generate an OR expected result
-		finalOutput.ExpectedResult = strings.Join(expectedResultArr, " OR ")
-	}
-
-	finalOutput.testResult = result
-	finalOutput.actualResult = res[0].actualResult
-
-	if finalOutput.actualResult == "" {
-		finalOutput.actualResult = s
-	}
-
-	return finalOutput
-}
-
 func toNumeric(a, b string) (c, d int, err error) {
 	c, err = strconv.Atoi(strings.TrimSpace(a))
 	if err != nil {

check/test_test.go

@@ -48,143 +48,181 @@ func TestTestExecute(t *testing.T) {
 
 	cases := []struct {
 		*Check
-		str string
+		str       string
+		strConfig string
 	}{
 		{
 			controls.Groups[0].Checks[0],
 			"2:45 ../kubernetes/kube-apiserver --allow-privileged=false --option1=20,30,40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[1],
 			"2:45 ../kubernetes/kube-apiserver --allow-privileged=false",
+			"",
 		},
 		{
 			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],
 			"2:45 ../kubernetes/kube-apiserver --secure-port=0 --audit-log-maxage=40 --option",
+			"",
 		},
 		{
 			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],
 			"2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[6],
 			"2:45 .. --kubelet-clientkey=foo --kubelet-client-certificate=bar --admission-control=Webhook,RBAC",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[7],
 			"2:45 ..  --secure-port=0 --kubelet-client-certificate=bar --admission-control=Webhook,RBAC",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[8],
 			"644",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[9],
 			"640",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[9],
 			"600",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[10],
 			"2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[11],
 			"2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,RBAC ---audit-log-maxage=40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[12],
 			"2:45 ../kubernetes/kube-apiserver --option --admission-control=WebHook,Something,RBAC ---audit-log-maxage=40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[13],
 			"2:45 ../kubernetes/kube-apiserver --option --admission-control=Something ---audit-log-maxage=40",
+			"",
 		},
 		{
 			// check for ':' as argument-value separator, with space between arg and val
 			controls.Groups[0].Checks[14],
 			"2:45 kube-apiserver some-arg: some-val --admission-control=Something ---audit-log-maxage=40",
+			"",
 		},
 		{
 			// check for ':' as argument-value separator, with no space between arg and val
 			controls.Groups[0].Checks[14],
 			"2:45 kube-apiserver some-arg:some-val --admission-control=Something ---audit-log-maxage=40",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[15],
+			"",
 			"{\"readOnlyPort\": 15000}",
 		},
 		{
 			controls.Groups[0].Checks[16],
+			"",
 			"{\"stringValue\": \"WebHook,Something,RBAC\"}",
 		},
 		{
 			controls.Groups[0].Checks[17],
+			"",
 			"{\"trueValue\": true}",
 		},
 		{
 			controls.Groups[0].Checks[18],
+			"",
 			"{\"readOnlyPort\": 15000}",
 		},
 		{
 			controls.Groups[0].Checks[19],
+			"",
 			"{\"authentication\": { \"anonymous\": {\"enabled\": false}}}",
 		},
 		{
 			controls.Groups[0].Checks[20],
+			"",
 			"readOnlyPort: 15000",
 		},
 		{
 			controls.Groups[0].Checks[21],
+			"",
 			"readOnlyPort: 15000",
 		},
 		{
 			controls.Groups[0].Checks[22],
+			"",
 			"authentication:\n  anonymous:\n    enabled: false",
 		},
 		{
 			controls.Groups[0].Checks[26],
+			"",
 			"currentMasterVersion: 1.12.7",
 		},
 		{
 			controls.Groups[0].Checks[27],
 			"--peer-client-cert-auth",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[27],
 			"--abc=true --peer-client-cert-auth --efg=false",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[27],
 			"--abc --peer-client-cert-auth --efg",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[27],
 			"--peer-client-cert-auth=true",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[27],
 			"--abc --peer-client-cert-auth=true --efg",
+			"",
 		},
 		{
 			controls.Groups[0].Checks[28],
 			"--abc --peer-client-cert-auth=false --efg",
+			"",
 		},
 	}
 
 	for _, c := range cases {
-		res := c.Tests.execute(c.str, c.IsMultiple).testResult
+		c.Check.AuditOutput = c.str
-		if !res {
+		c.Check.AuditConfigOutput = c.strConfig
+		res, err := c.Check.execute()
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		if !res.testResult {
 			t.Errorf("%s, expected:%v, got:%v\n", c.Text, true, res)
 		}
 	}
@@ -219,8 +257,12 @@ func TestTestExecuteExceptions(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		res := c.Tests.execute(c.str, c.IsMultiple).testResult
+		c.Check.AuditConfigOutput = c.str
-		if res {
+		res, err := c.Check.execute()
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		if res.testResult {
 			t.Errorf("%s, expected:%v, got:%v\n", c.Text, false, res)
 		}
 	}

integration/testdata/cis-1.5/job-node.data

@@ -14,7 +14,7 @@
 [PASS] 4.2.1 Ensure that the --anonymous-auth argument is set to false (Scored)
 [PASS] 4.2.2 Ensure that the --authorization-mode argument is not set to AlwaysAllow (Scored)
 [PASS] 4.2.3 Ensure that the --client-ca-file argument is set as appropriate (Scored)
-[FAIL] 4.2.4 Ensure that the --read-only-port argument is set to 0 (Scored)
+[PASS] 4.2.4 Ensure that the --read-only-port argument is set to 0 (Scored)
 [PASS] 4.2.5 Ensure that the --streaming-connection-idle-timeout argument is not set to 0 (Scored)
 [FAIL] 4.2.6 Ensure that the --protect-kernel-defaults argument is set to true (Scored)
 [PASS] 4.2.7 Ensure that the --make-iptables-util-chains argument is set to true (Scored)
@@ -33,15 +33,6 @@ chmod 644 /etc/kubernetes/proxy.conf
 4.1.4 Run the below command (based on the file location on your system) on the each worker node.
 For example, chown root:root /etc/kubernetes/proxy.conf
 
-4.2.4 If using a Kubelet config file, edit the file to set readOnlyPort to 0.
-If using command line arguments, edit the kubelet service file
-/etc/systemd/system/kubelet.service.d/10-kubeadm.conf on each worker node and
-set the below parameter in KUBELET_SYSTEM_PODS_ARGS variable.
---read-only-port=0
-Based on your system, restart the kubelet service. For example:
-systemctl daemon-reload
-systemctl restart kubelet.service
-
 4.2.6 If using a Kubelet config file, edit the file to set protectKernelDefaults: true.
 If using command line arguments, edit the kubelet service file
 /etc/systemd/system/kubelet.service.d/10-kubeadm.conf on each worker node and
@@ -80,7 +71,7 @@ systemctl restart kubelet.service
 
 
 == Summary ==
-16 checks PASS
+17 checks PASS
-6 checks FAIL
+5 checks FAIL
 1 checks WARN
 0 checks INFO

integration/testdata/cis-1.5/job.data

@@ -227,7 +227,7 @@ minimum.
 [PASS] 4.2.1 Ensure that the --anonymous-auth argument is set to false (Scored)
 [PASS] 4.2.2 Ensure that the --authorization-mode argument is not set to AlwaysAllow (Scored)
 [PASS] 4.2.3 Ensure that the --client-ca-file argument is set as appropriate (Scored)
-[FAIL] 4.2.4 Ensure that the --read-only-port argument is set to 0 (Scored)
+[PASS] 4.2.4 Ensure that the --read-only-port argument is set to 0 (Scored)
 [PASS] 4.2.5 Ensure that the --streaming-connection-idle-timeout argument is not set to 0 (Scored)
 [FAIL] 4.2.6 Ensure that the --protect-kernel-defaults argument is set to true (Scored)
 [PASS] 4.2.7 Ensure that the --make-iptables-util-chains argument is set to true (Scored)
@@ -246,15 +246,6 @@ chmod 644 /etc/kubernetes/proxy.conf
 4.1.4 Run the below command (based on the file location on your system) on the each worker node.
 For example, chown root:root /etc/kubernetes/proxy.conf
 
-4.2.4 If using a Kubelet config file, edit the file to set readOnlyPort to 0.
-If using command line arguments, edit the kubelet service file
-/etc/systemd/system/kubelet.service.d/10-kubeadm.conf on each worker node and
-set the below parameter in KUBELET_SYSTEM_PODS_ARGS variable.
---read-only-port=0
-Based on your system, restart the kubelet service. For example:
-systemctl daemon-reload
-systemctl restart kubelet.service
-
 4.2.6 If using a Kubelet config file, edit the file to set protectKernelDefaults: true.
 If using command line arguments, edit the kubelet service file
 /etc/systemd/system/kubelet.service.d/10-kubeadm.conf on each worker node and
@@ -293,8 +284,8 @@ systemctl restart kubelet.service
 
 
 == Summary ==
-16 checks PASS
+17 checks PASS
-6 checks FAIL
+5 checks FAIL
 1 checks WARN
 0 checks INFO
 [INFO] 5 Kubernetes Policies