Repository: aquasecurity/starboard
Pull request #378 (2021-02-05 14:39:48)

fix(kube-bench): Auto-detect CIS benchmark sections to run
Resolves: #369

Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=h1) Report
> Merging [#378](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=desc) (4fea6a6) into [main](https://codecov.io/gh/aquasecurity/starboard/commit/39126780c680c8191137917dd417402d4114e856?el=desc) (3912678) will **decrease** coverage by `37.73%`.
> The diff coverage is `63.15%`.

[![Impacted file tree graph](https://codecov.io/gh/aquasecurity/starboard/pull/378/graphs/tree.svg?width=650&height=150&src=pr&token=qqTGKAJ0Wb)](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##             main     #378       +/-   ##
===========================================
- Coverage   68.34%   30.61%   -37.74%     
===========================================
  Files          58       57        -1     
  Lines        3064     2976       -88     
===========================================
- Hits         2094      911     -1183     
- Misses        714     2002     +1288     
+ Partials      256       63      -193     
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/kubebench/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2t1YmViZW5jaC9zY2FubmVyLmdv) | `0.00% <0.00%> (-92.62%)` | :arrow_down: |
| [pkg/kubebench/converter.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2t1YmViZW5jaC9jb252ZXJ0ZXIuZ28=) | `88.88% <91.66%> (+0.65%)` | :arrow_up: |
| [pkg/starboard/config.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL3N0YXJib2FyZC9jb25maWcuZ28=) | `71.07% <100.00%> (ø)` | |
| [pkg/cmd/get.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9nZXQuZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/find.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9maW5kLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/root.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9yb290Lmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/scan.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9zY2FuLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/polaris.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9wb2xhcmlzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/version.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC92ZXJzaW9uLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/cmd/kube\_bench.go](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree#diff-cGtnL2NtZC9rdWJlX2JlbmNoLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/aquasecurity/starboard/pull/378/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=footer). Last update [17aa68d...bf7bb15](https://codecov.io/gh/aquasecurity/starboard/pull/378?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/settings.md b/docs/settings.md
index c37f800..0b6160f 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -65,7 +65,7 @@ The following tables list available configuration settings with their default va
 | `trivy.serverTokenHeader`      | `Trivy-Token`                                          | The name of the HTTP header to send the authentication token to Trivy server. Only application in `ClientServer` mode when `trivy.serverToken` is specified. |
 | `aqua.imageRef`                | `docker.io/aquasec/scanner:5.3`                        | Aqua scanner image reference. The tag determines the version of the `scanner` binary executable and it must be compatible with version of Aqua console. |
 | `aqua.serverURL`               | N/A                                                    | The endpoint URL of Aqua management console |
-| `kube-bench.imageRef`          | `docker.io/aquasec/kube-bench:0.4.0`                   | kube-bench image reference |
+| `kube-bench.imageRef`          | `docker.io/aquasec/kube-bench:0.5.0`                   | kube-bench image reference |
 | `kube-hunter.imageRef`         | `docker.io/aquasec/kube-hunter:0.4.0`                  | kube-hunter image reference |
 | `kube-hunter.quick`            | `"false"`                                              | Whether to use kube-hunter's "quick" scanning mode (subnet 24). Set to `"true"` to enable. |
 | `polaris.imageRef`             | `quay.io/fairwinds/polaris:3.0`                        | Polaris image reference |
diff --git a/pkg/kubebench/converter.go b/pkg/kubebench/converter.go
index e88d154..972bc7f 100644
--- a/pkg/kubebench/converter.go
+++ b/pkg/kubebench/converter.go
@@ -3,54 +3,51 @@ package kubebench
 import (
 	"encoding/json"
 	"io"
-	"time"
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
+	"github.com/aquasecurity/starboard/pkg/ext"
 	"github.com/aquasecurity/starboard/pkg/starboard"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-type Converter interface {
-	Convert(config Config, reader io.Reader) (v1alpha1.CISKubeBenchOutput, error)
+type Converter struct {
+	ext.Clock
+	Config
 }
 
-var DefaultConverter Converter = &converter{}
+func (c *Converter) Convert(reader io.Reader) (v1alpha1.CISKubeBenchOutput, error) {
+	output := &struct {
+		Controls []v1alpha1.CISKubeBenchSection `json:"Controls"`
+	}{}
 
-type converter struct {
-}
-
-func (c *converter) Convert(config Config, reader io.Reader) (report v1alpha1.CISKubeBenchOutput, err error) {
 	decoder := json.NewDecoder(reader)
-	var section []v1alpha1.CISKubeBenchSection
-	err = decoder.Decode(&section)
+	err := decoder.Decode(output)
 	if err != nil {
-		return
+		return v1alpha1.CISKubeBenchOutput{}, err
 	}
 
-	imageRef, err := config.GetKubeBenchImageRef()
+	imageRef, err := c.Config.GetKubeBenchImageRef()
 	if err != nil {
-		return report, err
+		return v1alpha1.CISKubeBenchOutput{}, err
 	}
 	version, err := starboard.GetVersionFromImageRef(imageRef)
 	if err != nil {
 		return v1alpha1.CISKubeBenchOutput{}, err
 	}
 
-	report = v1alpha1.CISKubeBenchOutput{
+	return v1alpha1.CISKubeBenchOutput{
 		Scanner: v1alpha1.Scanner{
 			Name:    "kube-bench",
 			Vendor:  "Aqua Security",
 			Version: version,
 		},
-		Summary:         c.summary(section),
-		UpdateTimestamp: metav1.NewTime(time.Now()),
-		Sections:        section,
-	}
-
-	return
+		Summary:         c.summary(output.Controls),
+		UpdateTimestamp: metav1.NewTime(c.Clock.Now()),
+		Sections:        output.Controls,
+	}, nil
 }
 
-func (c *converter) summary(sections []v1alpha1.CISKubeBenchSection) v1alpha1.CISKubeBenchSummary {
+func (c *Converter) summary(sections []v1alpha1.CISKubeBenchSection) v1alpha1.CISKubeBenchSummary {
 	totalPass := 0
 	totalInfo := 0
 	totalWarn := 0
diff --git a/pkg/kubebench/scanner.go b/pkg/kubebench/scanner.go
index d28fff9..62a1042 100644
--- a/pkg/kubebench/scanner.go
+++ b/pkg/kubebench/scanner.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
+	"github.com/aquasecurity/starboard/pkg/ext"
 	"github.com/aquasecurity/starboard/pkg/kube"
 	"github.com/aquasecurity/starboard/pkg/kube/pod"
 	"github.com/aquasecurity/starboard/pkg/runner"
@@ -23,7 +24,6 @@ import (
 
 const (
 	kubeBenchContainerName = "kube-bench"
-	masterNodeLabel        = "node-role.kubernetes.io/master"
 )
 
 type Config interface {
@@ -37,7 +37,7 @@ type Scanner struct {
 	clientset  kubernetes.Interface
 	pods       *pod.Manager
 	logsReader kube.LogsReader
-	converter  Converter
+	converter  *Converter
 }
 
 func NewScanner(
@@ -53,7 +53,10 @@ func NewScanner(
 		clientset:  clientset,
 		pods:       pod.NewPodManager(clientset),
 		logsReader: kube.NewLogsReader(clientset),
-		converter:  DefaultConverter,
+		converter: &Converter{
+			Clock:  ext.NewSystemClock(),
+			Config: config,
+		},
 	}
 }
 
@@ -95,7 +98,7 @@ func (s *Scanner) Scan(ctx context.Context, node corev1.Node) (v1alpha1.CISKubeB
 	}()
 
 	// 4. Parse the CISBenchmarkReport from the logs Reader
-	return s.converter.Convert(s.config, logsStream)
+	return s.converter.Convert(logsStream)
 }
 
 func (s *Scanner) prepareKubeBenchJob(node corev1.Node) (*batchv1.Job, error) {
@@ -103,10 +106,6 @@ func (s *Scanner) prepareKubeBenchJob(node corev1.Node) (*batchv1.Job, error) {
 	if err != nil {
 		return nil, err
 	}
-	target := "node"
-	if _, ok := node.Labels[masterNodeLabel]; ok {
-		target = "master"
-	}
 	return &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      uuid.New().String(),
@@ -183,8 +182,8 @@ func (s *Scanner) prepareKubeBenchJob(node corev1.Node) (*batchv1.Job, error) {
 							Image:                    imageRef,
 							ImagePullPolicy:          corev1.PullIfNotPresent,
 							TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
-							Command:                  []string{"kube-bench", target},
-							Args:                     []string{"--json"},
+							Command:                  []string{"sh"},
+							Args:                     []string{"-c", "kube-bench --json 2> /dev/null"},
 							Resources: corev1.ResourceRequirements{
 								Limits: corev1.ResourceList{
 									corev1.ResourceCPU:    resource.MustParse("300m"),
diff --git a/pkg/starboard/config.go b/pkg/starboard/config.go
index ce40abb..906e9f8 100644
--- a/pkg/starboard/config.go
+++ b/pkg/starboard/config.go
@@ -265,7 +265,7 @@ func GetDefaultConfig() ConfigData {
 		keyTrivyMode:     string(Standalone),
 		"aqua.imageRef":  "docker.io/aquasec/scanner:5.3",
 
-		"kube-bench.imageRef":  "docker.io/aquasec/kube-bench:0.4.0",
+		"kube-bench.imageRef":  "docker.io/aquasec/kube-bench:0.5.0",
 		"kube-hunter.imageRef": "docker.io/aquasec/kube-hunter:0.4.0",
 		"kube-hunter.quick":    "false",
 


Test patch:
diff --git a/itest/starboard/starboard_cli_test.go b/itest/starboard/starboard_cli_test.go
index 1e0bae6..2ecc0ea 100644
--- a/itest/starboard/starboard_cli_test.go
+++ b/itest/starboard/starboard_cli_test.go
@@ -978,7 +978,7 @@ var _ = Describe("Starboard CLI", func() {
 						"Scanner": Equal(v1alpha1.Scanner{
 							Name:    "kube-bench",
 							Vendor:  "Aqua Security",
-							Version: "0.4.0",
+							Version: "0.5.0",
 						}),
 					}),
 				}))
diff --git a/pkg/kubebench/converter_test.go b/pkg/kubebench/converter_test.go
index 2f8829f..6055136 100644
--- a/pkg/kubebench/converter_test.go
+++ b/pkg/kubebench/converter_test.go
@@ -3,6 +3,7 @@ package kubebench_test
 import (
 	"encoding/json"
 	"errors"
+	"github.com/aquasecurity/starboard/pkg/ext"
 	"os"
 	"testing"
 	"time"
@@ -18,11 +19,16 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var (
+	fixedTime  = time.Now()
+	fixedClock = ext.NewFixedClock(fixedTime)
+)
+
 func TestConverter_Convert(t *testing.T) {
 	config := starboard.ConfigData{
 		"kube-bench.imageRef": "aquasec/kube-bench:0.3.1",
 	}
-	var testcases = []struct {
+	var testCases = []struct {
 		name string
 		in   string // input File
 		op   string // golden file
@@ -37,7 +43,7 @@ func TestConverter_Convert(t *testing.T) {
 		{
 			name: "invalid json object",
 			in:   "testdata/invalid.json",
-			err:  errors.New("json: cannot unmarshal object into Go value of type []v1alpha1.CISKubeBenchSection"),
+			err:  errors.New("invalid character 'I' looking for beginning of value"),
 		},
 		{
 			name: "Valid multiple json object in array",
@@ -47,7 +53,7 @@ func TestConverter_Convert(t *testing.T) {
 		},
 	}
 
-	for _, tc := range testcases {
+	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			inFile, err := os.Open(tc.in)
 			require.NoError(t, err)
@@ -55,29 +61,34 @@ func TestConverter_Convert(t *testing.T) {
 				_ = inFile.Close()
 			}()
 
-			var r starboardv1alpha1.CISKubeBenchOutput
-			r, err = kubebench.DefaultConverter.Convert(config, inFile)
+			converter := &kubebench.Converter{Clock: fixedClock, Config: config}
+			output, err := converter.Convert(inFile)
 
 			switch {
 			case tc.err == nil:
 				require.NoError(t, err)
-				gFile, err := os.Open(tc.op)
-				require.NoError(t, err)
-				dec := json.NewDecoder(gFile)
-				var kbop starboardv1alpha1.CISKubeBenchOutput
-				err = dec.Decode(&kbop)
-				require.NoError(t, err)
-				defer func() {
-					_ = gFile.Close()
-				}()
-
-				fakeTime := metav1.NewTime(time.Now())
-				kbop.UpdateTimestamp = fakeTime
-				r.UpdateTimestamp = fakeTime
-				assert.Equal(t, kbop, r, "Converted report does not match expected report")
+				expectedOutput := expectedOutputFrom(t, tc.op)
+				assert.Equal(t, expectedOutput, output, "Converted report does not match expected report")
 			default:
 				assert.EqualError(t, err, tc.err.Error())
 			}
 		})
 	}
 }
+
+func expectedOutputFrom(t *testing.T, fileName string) starboardv1alpha1.CISKubeBenchOutput {
+	t.Helper()
+
+	file, err := os.Open(fileName)
+	require.NoError(t, err)
+	defer file.Close()
+
+	var expectedOutput starboardv1alpha1.CISKubeBenchOutput
+	err = json.NewDecoder(file).Decode(&expectedOutput)
+	require.NoError(t, err)
+
+	// Override time read from file
+	expectedOutput.UpdateTimestamp = metav1.NewTime(fixedTime)
+
+	return expectedOutput
+}
diff --git a/pkg/kubebench/testdata/invalid.json b/pkg/kubebench/testdata/invalid.json
index 31d9226..48de5d9 100644
--- a/pkg/kubebench/testdata/invalid.json
+++ b/pkg/kubebench/testdata/invalid.json
@@ -1,37 +1,1 @@
-{
-    "id": "1",
-    "version": "1.5",
-    "text": "Master Node Security Configuration",
-    "node_type": "master",
-    "tests": [
-        {
-            "section": "1.1",
-            "pass": 15,
-            "fail": 1,
-            "warn": 5,
-            "info": 0,
-            "desc": "Master Node Configuration Files ",
-            "results": [
-                {
-                    "test_number": "1.1.1",
-                    "test_desc": "test desc",
-                    "audit": "audit",
-                    "AuditConfig": "",
-                    "type": "",
-                    "remediation": "remeds",
-                    "test_info": [
-                        "test-info"
-                    ],
-                    "status": "PASS",
-                    "actual_value": "permissions=600\n",
-                    "scored": true,
-                    "expected_result": "exp-result"
-                }
-            ]
-        }
-    ],
-    "total_pass": 41,
-    "total_fail": 13,
-    "total_warn": 11,
-    "total_info": 0
-}
\ No newline at end of file
+INVALIDJSON
diff --git a/pkg/kubebench/testdata/multiObjects.json b/pkg/kubebench/testdata/multiObjects.json
index 60d89eb..7f59d8f 100644
--- a/pkg/kubebench/testdata/multiObjects.json
+++ b/pkg/kubebench/testdata/multiObjects.json
@@ -1,67 +1,78 @@
-[{
-    "id": "1",
-    "version": "1.5",
-    "text": "Master Node Security Configuration",
-    "node_type": "master",
-    "tests": [{
-        "section": "1.1",
-        "pass": 15,
-        "fail": 1,
-        "warn": 5,
-        "info": 0,
-        "desc": "Master Node Configuration Files ",
-        "results": [{
-            "test_number": "1.1.1",
-            "test_desc": "test desc",
-            "audit": "audit",
-            "AuditConfig": "",
-            "type": "",
-            "remediation": "remeds",
-            "test_info": [
-                "test-info"
-            ],
-            "status": "PASS",
-            "actual_value": "permissions=600\n",
-            "scored": true,
-            "expected_result": "exp-result"
-        }]
-    }],
-    "total_pass": 41,
-    "total_fail": 13,
-    "total_warn": 11,
-    "total_info": 0
-},
 {
-    "id": "2",
-    "version": "1.5",
-    "text": "Master Node Security Configuration",
-    "node_type": "master",
-    "tests": [{
-        "section": "1.1",
-        "pass": 15,
-        "fail": 1,
-        "warn": 5,
-        "info": 0,
-        "desc": "Master Node Configuration Files ",
-        "results": [{
-            "test_number": "1.1.1",
-            "test_desc": "test desc",
-            "audit": "audit",
-            "AuditConfig": "",
-            "type": "",
-            "remediation": "remeds",
-            "test_info": [
+  "Controls": [
+    {
+      "id": "1",
+      "version": "1.5",
+      "text": "Master Node Security Configuration",
+      "node_type": "master",
+      "tests": [
+        {
+          "section": "1.1",
+          "pass": 15,
+          "fail": 1,
+          "warn": 5,
+          "info": 0,
+          "desc": "Master Node Configuration Files ",
+          "results": [
+            {
+              "test_number": "1.1.1",
+              "test_desc": "test desc",
+              "audit": "audit",
+              "AuditConfig": "",
+              "type": "",
+              "remediation": "remeds",
+              "test_info": [
+                "test-info"
+              ],
+              "status": "PASS",
+              "actual_value": "permissions=600\n",
+              "scored": true,
+              "expected_result": "exp-result"
+            }
+          ]
+        }
+      ],
+      "total_pass": 41,
+      "total_fail": 13,
+      "total_warn": 11,
+      "total_info": 0
+    },
+    {
+      "id": "2",
+      "version": "1.5",
+      "text": "Master Node Security Configuration",
+      "node_type": "master",
+      "tests": [
+        {
+          "section": "1.1",
+          "pass": 15,
+          "fail": 1,
+          "warn": 5,
+          "info": 0,
+          "desc": "Master Node Configuration Files ",
+          "results": [
+            {
+              "test_number": "1.1.1",
+              "test_desc": "test desc",
+              "audit": "audit",
+              "AuditConfig": "",
+              "type": "",
+              "remediation": "remeds",
+              "test_info": [
                 "test-info"
-            ],
-            "status": "PASS",
-            "actual_value": "permissions=600\n",
-            "scored": true,
-            "expected_result": "exp-result"
-        }]
-    }],
-    "total_pass": 41,
-    "total_fail": 13,
-    "total_warn": 11,
-    "total_info": 0
+              ],
+              "status": "PASS",
+              "actual_value": "permissions=600\n",
+              "scored": true,
+              "expected_result": "exp-result"
+            }
+          ]
+        }
+      ],
+      "total_pass": 41,
+      "total_fail": 13,
+      "total_warn": 11,
+      "total_info": 0
+    }
+  ]
 }
-]
\ No newline at end of file
diff --git a/pkg/kubebench/testdata/valid.json b/pkg/kubebench/testdata/valid.json
index 45b684e..58c216a 100644
--- a/pkg/kubebench/testdata/valid.json
+++ b/pkg/kubebench/testdata/valid.json
@@ -1,37 +1,41 @@
-[{
-    "id": "1",
-    "version": "1.5",
-    "text": "Master Node Security Configuration",
-    "node_type": "master",
-    "tests": [
+{
+  "Controls": [
+    {
+      "id": "1",
+      "version": "1.5",
+      "text": "Master Node Security Configuration",
+      "node_type": "master",
+      "tests": [
         {
-            "section": "1.1",
-            "pass": 15,
-            "fail": 1,
-            "warn": 5,
-            "info": 0,
-            "desc": "Master Node Configuration Files ",
-            "results": [
-                {
-                    "test_number": "1.1.1",
-                    "test_desc": "test desc",
-                    "audit": "audit",
-                    "AuditConfig": "",
-                    "type": "",
-                    "remediation": "remeds",
-                    "test_info": [
-                        "test-info"
-                    ],
-                    "status": "PASS",
-                    "actual_value": "permissions=600\n",
-                    "scored": true,
-                    "expected_result": "exp-result"
-                }
-            ]
+          "section": "1.1",
+          "pass": 15,
+          "fail": 1,
+          "warn": 5,
+          "info": 0,
+          "desc": "Master Node Configuration Files ",
+          "results": [
+            {
+              "test_number": "1.1.1",
+              "test_desc": "test desc",
+              "audit": "audit",
+              "AuditConfig": "",
+              "type": "",
+              "remediation": "remeds",
+              "test_info": [
+                "test-info"
+              ],
+              "status": "PASS",
+              "actual_value": "permissions=600\n",
+              "scored": true,
+              "expected_result": "exp-result"
+            }
+          ]
         }
-    ],
-    "total_pass": 41,
-    "total_fail": 13,
-    "total_warn": 11,
-    "total_info": 0
-}]
\ No newline at end of file
+      ],
+      "total_pass": 41,
+      "total_fail": 13,
+      "total_warn": 11,
+      "total_info": 0
+    }
+  ]
+}