Repository: aquasecurity/starboard
Pull request #626 (2021-06-17 12:23:19)

feat: Add scope to config audit check
Add generic checks property to ConfigAuditReportData and
deprecate workload specific podChecks and containerChecks.
To provide the same semantics each check has scope to
identify the section of config that was audited, e.g.
container name.

Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/626?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) Report
> Merging [#626](https://codecov.io/gh/aquasecurity/starboard/pull/626?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) (1639889) into [main](https://codecov.io/gh/aquasecurity/starboard/commit/ea6c60a86714e5bb434b791cf465ebb7a34aba82?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) (ea6c60a) will **decrease** coverage by `23.35%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/aquasecurity/starboard/pull/626/graphs/tree.svg?width=650&height=150&src=pr&token=qqTGKAJ0Wb&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity)](https://codecov.io/gh/aquasecurity/starboard/pull/626?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity)

```diff
@@             Coverage Diff             @@
##             main     #626       +/-   ##
===========================================
- Coverage   68.61%   45.26%   -23.36%     
===========================================
  Files          58       41       -17     
  Lines        4435     3599      -836     
===========================================
- Hits         3043     1629     -1414     
- Misses       1042     1871      +829     
+ Partials      350       99      -251     
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/626?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) | Coverage Δ | |
|---|---|---|
| [pkg/plugin/conftest/plugin.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL3BsdWdpbi9jb25mdGVzdC9wbHVnaW4uZ28=) | `86.95% <100.00%> (+0.08%)` | :arrow_up: |
| [pkg/plugin/polaris/plugin.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL3BsdWdpbi9wb2xhcmlzL3BsdWdpbi5nbw==) | `89.26% <100.00%> (-0.88%)` | :arrow_down: |
| [pkg/kubehunter/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL2t1YmVodW50ZXIvc2Nhbm5lci5nbw==) | `0.00% <0.00%> (-80.51%)` | :arrow_down: |
| [pkg/plugin/factory.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL3BsdWdpbi9mYWN0b3J5Lmdv) | `0.00% <0.00%> (-80.49%)` | :arrow_down: |
| [pkg/configauditreport/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL2NvbmZpZ2F1ZGl0cmVwb3J0L3NjYW5uZXIuZ28=) | `0.00% <0.00%> (-70.77%)` | :arrow_down: |
| [pkg/vulnerabilityreport/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL3Z1bG5lcmFiaWxpdHlyZXBvcnQvc2Nhbm5lci5nbw==) | `0.00% <0.00%> (-70.54%)` | :arrow_down: |
| [pkg/operator/controller/vulnerabilityreport.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL29wZXJhdG9yL2NvbnRyb2xsZXIvdnVsbmVyYWJpbGl0eXJlcG9ydC5nbw==) | `0.00% <0.00%> (-70.09%)` | :arrow_down: |
| [pkg/operator/controller/configauditreport.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL29wZXJhdG9yL2NvbnRyb2xsZXIvY29uZmlnYXVkaXRyZXBvcnQuZ28=) | `0.00% <0.00%> (-67.13%)` | :arrow_down: |
| [pkg/kube/runnable\_job.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL2t1YmUvcnVubmFibGVfam9iLmdv) | `0.00% <0.00%> (-64.39%)` | :arrow_down: |
| [pkg/kubehunter/model.go](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL2t1YmVodW50ZXIvbW9kZWwuZ28=) | `0.00% <0.00%> (-61.54%)` | :arrow_down: |
| ... and [31 more](https://codecov.io/gh/aquasecurity/starboard/pull/626/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) | |

------

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


Patch:
diff --git a/docs/crds.md b/docs/crds.md
index 7c36611..e8162fd 100644
--- a/docs/crds.md
+++ b/docs/crds.md
@@ -126,34 +126,44 @@ report:
     dangerCount: 0
     passCount: 3
     warningCount: 2
-  podChecks:
+  checks:
     - category: Security
       checkID: hostNetworkSet
       message: Host network is not configured
       severity: warning
       success: true
-  containerChecks:
-    nginx:
-      - category: Security
-        checkID: dangerousCapabilities
-        message: Container does not have any dangerous capabilities
-        severity: danger
-        success: true
-      - category: Security
-        checkID: hostPortSet
-        message: Host port is not configured
-        severity: warning
-        success: true
-      - category: Security
-        checkID: insecureCapabilities
-        message: Container should not have insecure capabilities
-        severity: warning
-        success: false
-      - category: Security
-        checkID: notReadOnlyRootFilesystem
-        message: Filesystem should be read only
-        severity: warning
-        success: false
+    - category: Security
+      checkID: dangerousCapabilities
+      message: Container does not have any dangerous capabilities
+      severity: danger
+      success: true
+      scope:
+        type: Container
+        value: nginx
+    - category: Security
+      checkID: hostPortSet
+      message: Host port is not configured
+      severity: warning
+      success: true
+      scope:
+        type: Container
+        value: nginx
+    - category: Security
+      checkID: insecureCapabilities
+      message: Container should not have insecure capabilities
+      severity: warning
+      success: false
+      scope:
+        type: Container
+        value: nginx
+    - category: Security
+      checkID: notReadOnlyRootFilesystem
+      message: Filesystem should be read only
+      severity: warning
+      success: false
+      scope:
+        type: Container
+        value: nginx
 ```
 
 Third party Kubernetes configuration checkers, linters, and sanitizers that are compliant with the ConfigAuditReport
diff --git a/pkg/apis/aquasecurity/v1alpha1/config_audit_types.go b/pkg/apis/aquasecurity/v1alpha1/config_audit_types.go
index 43e55cd..8e1ca25 100644
--- a/pkg/apis/aquasecurity/v1alpha1/config_audit_types.go
+++ b/pkg/apis/aquasecurity/v1alpha1/config_audit_types.go
@@ -69,14 +69,48 @@ type ConfigAuditReportData struct {
 	UpdateTimestamp metav1.Time        `json:"updateTimestamp"`
 	Scanner         Scanner            `json:"scanner"`
 	Summary         ConfigAuditSummary `json:"summary"`
-	PodChecks       []Check            `json:"podChecks"`
+
+	// Checks provides results of conducting audit steps.
+	Checks []Check `json:"checks"`
+
+	// Deprecated in 0.12+ use Checks with CheckScope instead
+	PodChecks []Check `json:"podChecks"`
+	// Deprecated in 0.12+ use Checks with CheckScope instead
 	ContainerChecks map[string][]Check `json:"containerChecks"`
 }
 
+// CheckScope has Type and Value fields to further identify a given Check.
+// For example, we can use `Container` as Type and `nginx` as Value to indicate
+// that a particular check is relevant to the nginx container. Alternatively,
+// Type may be `JSONPath` and the Value would be JSONPath expression, e.g.
+// `.spec.container[0].securityContext.allowPrivilegeEscalation`.
+//
+// Another use case for CheckScope is to inspect a ConfigMap with many keys and
+// indicate a troublesome key. In this case the Type would be `ConfigMapKey`
+// and the Value will hold the name of a key, e.g. `myawsprivatekey`.
+type CheckScope struct {
+
+	// Type indicates type of this scope, e.g. Container, ConfigMapKey or JSONPath.
+	Type string `json:"type"`
+
+	// Value indicates value of this scope that depends on Type, e.g. container name, ConfigMap key or JSONPath expression
+	Value string `json:"value"`
+}
+
+// Check provides the result of conducting a single audit step.
 type Check struct {
-	ID       string `json:"checkID"`
-	Message  string `json:"message"`
+	ID      string `json:"checkID"`
+	Message string `json:"message"`
+
+	// Remediation provides description or links to external resources to remediate failing check.
+	// +optional
+	Remediation string `json:"remediation,omitempty"`
+
 	Success  bool   `json:"success"`
 	Severity string `json:"severity"`
 	Category string `json:"category"`
+
+	// Scope indicates the section of config that was audited.
+	// +optional
+	Scope *CheckScope `json:"scope,omitempty"`
 }
diff --git a/pkg/apis/aquasecurity/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/aquasecurity/v1alpha1/zz_generated.deepcopy.go
index cb73006..60f8e69 100644
--- a/pkg/apis/aquasecurity/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/aquasecurity/v1alpha1/zz_generated.deepcopy.go
@@ -189,6 +189,11 @@ func (in *CISKubeBenchTests) DeepCopy() *CISKubeBenchTests {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Check) DeepCopyInto(out *Check) {
 	*out = *in
+	if in.Scope != nil {
+		in, out := &in.Scope, &out.Scope
+		*out = new(CheckScope)
+		**out = **in
+	}
 	return
 }
 
@@ -202,6 +207,22 @@ func (in *Check) DeepCopy() *Check {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *CheckScope) DeepCopyInto(out *CheckScope) {
+	*out = *in
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CheckScope.
+func (in *CheckScope) DeepCopy() *CheckScope {
+	if in == nil {
+		return nil
+	}
+	out := new(CheckScope)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ClusterConfigAuditReport) DeepCopyInto(out *ClusterConfigAuditReport) {
 	*out = *in
@@ -295,10 +316,19 @@ func (in *ConfigAuditReportData) DeepCopyInto(out *ConfigAuditReportData) {
 	in.UpdateTimestamp.DeepCopyInto(&out.UpdateTimestamp)
 	out.Scanner = in.Scanner
 	out.Summary = in.Summary
+	if in.Checks != nil {
+		in, out := &in.Checks, &out.Checks
+		*out = make([]Check, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	if in.PodChecks != nil {
 		in, out := &in.PodChecks, &out.PodChecks
 		*out = make([]Check, len(*in))
-		copy(*out, *in)
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
 	}
 	if in.ContainerChecks != nil {
 		in, out := &in.ContainerChecks, &out.ContainerChecks
@@ -310,7 +340,9 @@ func (in *ConfigAuditReportData) DeepCopyInto(out *ConfigAuditReportData) {
 			} else {
 				in, out := &val, &outVal
 				*out = make([]Check, len(*in))
-				copy(*out, *in)
+				for i := range *in {
+					(*in)[i].DeepCopyInto(&(*out)[i])
+				}
 			}
 			(*out)[key] = outVal
 		}
diff --git a/pkg/plugin/polaris/plugin.go b/pkg/plugin/polaris/plugin.go
index b31366c..a293c60 100644
--- a/pkg/plugin/polaris/plugin.go
+++ b/pkg/plugin/polaris/plugin.go
@@ -340,32 +340,40 @@ func (p *plugin) sourceNameFrom(obj client.Object) string {
 }
 
 func (p *plugin) configAuditReportDataFrom(ctx starboard.PluginContext, result Result) (v1alpha1.ConfigAuditReportData, error) {
+	var checks []v1alpha1.Check
 	var podChecks []v1alpha1.Check
-	containerChecks := make(map[string][]v1alpha1.Check)
+	containerNameToChecks := make(map[string][]v1alpha1.Check)
 
 	for _, pr := range result.PodResult.Results {
-		podChecks = append(podChecks, v1alpha1.Check{
+		check := v1alpha1.Check{
 			ID:       pr.ID,
 			Message:  pr.Message,
 			Success:  pr.Success,
 			Severity: pr.Severity,
 			Category: pr.Category,
-		})
+		}
+		checks = append(checks, check)
+		podChecks = append(podChecks, check)
 	}
 
 	for _, cr := range result.PodResult.ContainerResults {
-		var checks []v1alpha1.Check
+		var containerChecks []v1alpha1.Check
 		for _, crr := range cr.Results {
-			checks = append(checks, v1alpha1.Check{
+			containerChecks = append(containerChecks, v1alpha1.Check{
 				ID:       crr.ID,
 				Message:  crr.Message,
 				Success:  crr.Success,
 				Severity: crr.Severity,
 				Category: crr.Category,
+				Scope: &v1alpha1.CheckScope{
+					Type:  "Container",
+					Value: cr.Name,
+				},
 			})
 
 		}
-		containerChecks[cr.Name] = checks
+		checks = append(checks, containerChecks...)
+		containerNameToChecks[cr.Name] = containerChecks
 	}
 
 	imageRef, err := p.getImageRef(ctx)
@@ -384,10 +392,12 @@ func (p *plugin) configAuditReportDataFrom(ctx starboard.PluginContext, result R
 			Vendor:  "Fairwinds Ops",
 			Version: version,
 		},
-		Summary:         p.configAuditSummaryFrom(podChecks, containerChecks),
+		Summary:         p.configAuditSummaryFrom(podChecks, containerNameToChecks),
 		UpdateTimestamp: metav1.NewTime(p.clock.Now()),
+		Checks:          checks,
+		// TODO Deprecate PodChecks and ContainerChecks in 0.12+
 		PodChecks:       podChecks,
-		ContainerChecks: containerChecks,
+		ContainerChecks: containerNameToChecks,
 	}, nil
 }
 


Test patch:
diff --git a/pkg/plugin/conftest/plugin.go b/pkg/plugin/conftest/plugin.go
index 9551b03..920cb82 100644
--- a/pkg/plugin/conftest/plugin.go
+++ b/pkg/plugin/conftest/plugin.go
@@ -253,6 +253,8 @@ func (p *plugin) ParseConfigAuditReportData(ctx starboard.PluginContext, logsRea
 			WarningCount: warningCount,
 			DangerCount:  dangerCount,
 		},
+		Checks: checks,
+		// TODO Deprecate PodChecks and ContainerChecks in 0.12+
 		PodChecks:       checks,
 		ContainerChecks: map[string][]v1alpha1.Check{},
 	}, nil
diff --git a/pkg/plugin/conftest/plugin_test.go b/pkg/plugin/conftest/plugin_test.go
index bfb6b6a..987144a 100644
--- a/pkg/plugin/conftest/plugin_test.go
+++ b/pkg/plugin/conftest/plugin_test.go
@@ -351,6 +351,53 @@ func TestPlugin_ParseConfigAuditReportData(t *testing.T) {
 			WarningCount: 0,
 			PassCount:    2,
 		}),
+		"Checks": MatchAllElements(groupChecksByMessage, Elements{
+			"container kubedns of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop": Equal(v1alpha1.Check{
+				ID:       "Default capabilities: some containers do not drop all",
+				Message:  "container kubedns of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+			"container dnsmasq of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop": Equal(v1alpha1.Check{
+				ID:       "Default capabilities: some containers do not drop all",
+				Message:  "container dnsmasq of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+			"container sidecar of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop": Equal(v1alpha1.Check{
+				ID:       "Default capabilities: some containers do not drop all",
+				Message:  "container sidecar of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+			"container prometheus-to-sd of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop": Equal(v1alpha1.Check{
+				ID:       "Default capabilities: some containers do not drop all",
+				Message:  "container prometheus-to-sd of deployment kube-dns in default namespace should add 'ALL' to securityContext.capabilities.drop",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+			"container dnsmasq of deployment kube-dns in default namespace should set securityContext.readOnlyRootFilesystem to true": Equal(v1alpha1.Check{
+				ID:       "Root file system is not read-only",
+				Message:  "container dnsmasq of deployment kube-dns in default namespace should set securityContext.readOnlyRootFilesystem to true",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+			"container prometheus-to-sd of deployment kube-dns in default namespace should set resources.requests.cpu": Equal(v1alpha1.Check{
+				// If the author of a Rego script does not provide the title property
+				// in the rule's response, which is then returned as metadata.type
+				// in Conftest output, the parser will fallback to a unique identifier.
+				ID:       "00000000-0000-0000-0000-000000000001",
+				Message:  "container prometheus-to-sd of deployment kube-dns in default namespace should set resources.requests.cpu",
+				Success:  false,
+				Severity: "danger",
+				Category: "Security",
+			}),
+		}),
 		// Most Rego scripts do not return structured response object to indicate
 		// container name. Therefore, the ContainerChecks map is empty.
 		"ContainerChecks": Equal(map[string][]v1alpha1.Check{}),
diff --git a/pkg/plugin/polaris/plugin_test.go b/pkg/plugin/polaris/plugin_test.go
index 30cac14..45020b2 100644
--- a/pkg/plugin/polaris/plugin_test.go
+++ b/pkg/plugin/polaris/plugin_test.go
@@ -251,12 +251,20 @@ func TestPlugin_ParseConfigAuditReportData(t *testing.T) {
 		Success:  false,
 		Severity: "warning",
 		Category: "Resources",
+		Scope: &v1alpha1.CheckScope{
+			Type:  "Container",
+			Value: "db",
+		},
 	}, v1alpha1.Check{
 		ID:       "cpuRequestsMissing",
 		Message:  "CPU requests are set",
 		Success:  true,
 		Severity: "warning",
 		Category: "Resources",
+		Scope: &v1alpha1.CheckScope{
+			Type:  "Container",
+			Value: "db",
+		},
 	}))
 }