From 0cb302761c669128cdec9c136b69e677359314a5 Mon Sep 17 00:00:00 2001
From: Yoav Rotem <yoavrotems97@gmail.com>
Date: Mon, 22 Mar 2021 17:33:53 +0200
Subject: [PATCH] Add logging (#822)

* Add more logging

The old logging could was lacking and in some cases misleading

* Add Logging

Add more logs and change some old messages, the important part is make each test log more readable by adding ------ test id ------ section in logs

* Fix typos

* more info

add more info in comment about the function and it use cases

Co-authored-by: Liz Rice <liz@lizrice.com>

* Use switch case

Change the logic from if to switch and tidy up the code
---
 cfg/cis-1.5/policies.yaml             |  2 +-
 cfg/cis-1.6/policies.yaml             |  2 +-
 cfg/gke-1.0/policies.yaml             |  2 +-
 check/check.go                        | 17 ++++++++++++-----
 check/test.go                         | 26 +++++++++++++++++++++-----
 integration/testdata/cis-1.5/job.data |  2 +-
 integration/testdata/cis-1.6/job.data |  4 ++--
 7 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/cfg/cis-1.5/policies.yaml b/cfg/cis-1.5/policies.yaml
index ace5282..5ba7090 100644
--- a/cfg/cis-1.5/policies.yaml
+++ b/cfg/cis-1.5/policies.yaml
@@ -131,7 +131,7 @@ groups:
         text: "Minimize the admission of containers with capabilities assigned (Not Scored)"
         type: "manual"
         remediation: |
-          Review the use of capabilites in applications runnning on your cluster. Where a namespace
+          Review the use of capabilites in applications running on your cluster. Where a namespace
           contains applicaions which do not require any Linux capabities to operate consider adding
           a PSP which forbids the admission of containers which do not drop all capabilities.
         scored: false
diff --git a/cfg/cis-1.6/policies.yaml b/cfg/cis-1.6/policies.yaml
index 14b5ffa..980762a 100644
--- a/cfg/cis-1.6/policies.yaml
+++ b/cfg/cis-1.6/policies.yaml
@@ -131,7 +131,7 @@ groups:
         text: "Minimize the admission of containers with capabilities assigned (Manual)"
         type: "manual"
         remediation: |
-          Review the use of capabilites in applications runnning on your cluster. Where a namespace
+          Review the use of capabilites in applications running on your cluster. Where a namespace
           contains applicaions which do not require any Linux capabities to operate consider adding
           a PSP which forbids the admission of containers which do not drop all capabilities.
         scored: false
diff --git a/cfg/gke-1.0/policies.yaml b/cfg/gke-1.0/policies.yaml
index 2f26fbd..54ce12b 100644
--- a/cfg/gke-1.0/policies.yaml
+++ b/cfg/gke-1.0/policies.yaml
@@ -131,7 +131,7 @@ groups:
         text: "Minimize the admission of containers with capabilities assigned (Scored) "
         type: "manual"
         remediation: |
-          Review the use of capabilites in applications runnning on your cluster. Where a namespace
+          Review the use of capabilites in applications running on your cluster. Where a namespace
           contains applications which do not require any Linux capabities to operate consider adding
           a PSP which forbids the admission of containers which do not drop all capabilities.
         scored: true
diff --git a/check/check.go b/check/check.go
index da621d7..6e91f23 100644
--- a/check/check.go
+++ b/check/check.go
@@ -107,12 +107,14 @@ func (r *defaultRunner) Run(c *Check) State {
 // Run executes the audit commands specified in a check and outputs
 // the results.
 func (c *Check) run() State {
+	glog.V(3).Infof("-----   Running check %v   -----", c.ID)
 	// Since this is an Scored check
 	// without tests return a 'WARN' to alert
 	// the user that this check needs attention
 	if c.Scored && strings.TrimSpace(c.Type) == "" && c.Tests == nil {
 		c.Reason = "There are no tests"
 		c.State = WARN
+		glog.V(3).Info(c.Reason)
 		return c.State
 	}
 
@@ -120,6 +122,7 @@ func (c *Check) run() State {
 	if c.Type == SKIP {
 		c.Reason = "Test marked as skip"
 		c.State = INFO
+		glog.V(3).Info(c.Reason)
 		return c.State
 	}
 
@@ -127,6 +130,7 @@ func (c *Check) run() State {
 	if c.Type == MANUAL {
 		c.Reason = "Test marked as a manual test"
 		c.State = WARN
+		glog.V(3).Info(c.Reason)
 		return c.State
 	}
 
@@ -138,6 +142,7 @@ func (c *Check) run() State {
 		} else {
 			c.State = WARN
 		}
+		glog.V(3).Info(c.Reason)
 		return c.State
 	}
 
@@ -172,12 +177,13 @@ func (c *Check) run() State {
 		} else {
 			c.State = WARN
 		}
+		glog.V(3).Info(c.Reason)
 	}
 
 	if finalOutput != nil {
-		glog.V(3).Infof("Check.ID: %s Command: %q TestResult: %t State: %q \n", c.ID, lastCommand, finalOutput.testResult, c.State)
+		glog.V(3).Infof("Command: %q TestResult: %t State: %q \n", lastCommand, finalOutput.testResult, c.State)
 	} else {
-		glog.V(3).Infof("Check.ID: %s Command: %q TestResult: <<EMPTY>> \n", c.ID, lastCommand)
+		glog.V(3).Infof("Command: %q TestResult: <<EMPTY>> \n", lastCommand)
 	}
 
 	if c.Reason != "" {
@@ -212,7 +218,7 @@ func (c *Check) execute() (finalOutput *testOutput, err error) {
 	res := make([]testOutput, len(ts.TestItems))
 	expectedResultArr := make([]string, len(res))
 
-	glog.V(3).Infof("%d tests", len(ts.TestItems))
+	glog.V(3).Infof("Running %d test_items", len(ts.TestItems))
 	for i, t := range ts.TestItems {
 
 		t.isMultipleOutput = c.IsMultiple
@@ -236,6 +242,7 @@ func (c *Check) execute() (finalOutput *testOutput, err error) {
 			t.auditUsed = AuditEnv
 			result = *(t.execute(c.AuditEnvOutput))
 		}
+		glog.V(2).Infof("Used %s", t.auditUsed)
 		res[i] = result
 		expectedResultArr[i] = res[i].ExpectedResult
 	}
@@ -289,8 +296,8 @@ func runAudit(audit string) (output string, err error) {
 	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", audit)
+		glog.V(3).Infof("Output:\n %q", output)
 	}
 	return output, err
 }
diff --git a/check/test.go b/check/test.go
index b5abd30..3c63d45 100644
--- a/check/test.go
+++ b/check/test.go
@@ -126,6 +126,9 @@ func (t flagTestItem) findValue(s string) (match bool, value string, err error)
 		// flag: somevalue
 		// --flag
 		// somevalue
+		// DOESN'T COVER - use pathTestItem implementation of findValue() for this
+		// flag:
+		//	 - wehbook
 		pttn := `(` + t.Flag + `)(=|: *)*([^\s]*) *`
 		flagRe := regexp.MustCompile(pttn)
 		vals := flagRe.FindStringSubmatch(s)
@@ -145,7 +148,7 @@ func (t flagTestItem) findValue(s string) (match bool, value string, err error)
 			err = fmt.Errorf("invalid flag in testItem definition: %s", s)
 		}
 	}
-	glog.V(3).Infof("In flagTestItem.findValue %s, match %v, s %s, t.Flag %s", value, match, s, t.Flag)
+	glog.V(3).Infof("In flagTestItem.findValue %s", value)
 
 	return match, value, err
 }
@@ -183,6 +186,7 @@ func (t envTestItem) findValue(s string) (match bool, value string, err error) {
 			value = ""
 		}
 	}
+	glog.V(3).Infof("In envTestItem.findValue %s", value)
 	return match, value, nil
 }
 
@@ -232,10 +236,22 @@ func (t testItem) evaluate(s string) *testOutput {
 	}
 
 	result.flagFound = match
-	glog.V(3).Info(fmt.Sprintf("found %v", result.flagFound))
-
-
-	return result
+	var isExist = "exists"
+	if !result.flagFound{
+		isExist = "does not exist"
+	}
+	switch t.auditUsed {
+        case "auditCommand": 
+			glog.V(3).Infof("Flag '%s' %s", t.Flag, isExist)
+        case "auditConfig":
+			glog.V(3).Infof("Path '%s' %s", t.Path, isExist)
+		case "auditEnv":
+			glog.V(3).Infof("Env '%s' %s", t.Env, isExist)
+		default: 
+		glog.V(3).Infof("Error with identify audit used %s", t.auditUsed)
+    	}
+	
+    	return result
 }
 
 func compareOp(tCompareOp string, flagVal string, tCompareValue string, flagName string) (string, bool) {
diff --git a/integration/testdata/cis-1.5/job.data b/integration/testdata/cis-1.5/job.data
index 984d113..9c2e7a0 100644
--- a/integration/testdata/cis-1.5/job.data
+++ b/integration/testdata/cis-1.5/job.data
@@ -353,7 +353,7 @@ UIDs not including 0.
 5.2.8 Ensure that allowedCapabilities is not present in PSPs for the cluster unless
 it is set to an empty array.
 
-5.2.9 Review the use of capabilites in applications runnning on your cluster. Where a namespace
+5.2.9 Review the use of capabilites in applications running on your cluster. Where a namespace
 contains applicaions which do not require any Linux capabities to operate consider adding
 a PSP which forbids the admission of containers which do not drop all capabilities.
 
diff --git a/integration/testdata/cis-1.6/job.data b/integration/testdata/cis-1.6/job.data
index 54e79a4..54ec2b0 100644
--- a/integration/testdata/cis-1.6/job.data
+++ b/integration/testdata/cis-1.6/job.data
@@ -356,7 +356,7 @@ UIDs not including 0.
 5.2.8 Ensure that allowedCapabilities is not present in PSPs for the cluster unless
 it is set to an empty array.
 
-5.2.9 Review the use of capabilites in applications runnning on your cluster. Where a namespace
+5.2.9 Review the use of capabilites in applications running on your cluster. Where a namespace
 contains applicaions which do not require any Linux capabities to operate consider adding
 a PSP which forbids the admission of containers which do not drop all capabilities.
 
@@ -416,4 +416,4 @@ resources and that all new resources are created in a specific namespace.
 72 checks PASS
 11 checks FAIL
 39 checks WARN
-0 checks INFO
\ No newline at end of file
+0 checks INFO
-- 
GitLab