Repository: aquasecurity/starboard
Pull request #116 (2020-08-12 08:31:03)

feat: Create or update CISKubeBenchReports
In the initial version of Starbaord we wanted to store historical
benchmark reports. However, we now think that it's more appropriate
to store only the latest report.

Resolves: #101

Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/116?src=pr&el=h1) Report
> Merging [#116](https://codecov.io/gh/aquasecurity/starboard/pull/116?src=pr&el=desc) into [master](https://codecov.io/gh/aquasecurity/starboard/commit/f807c83a74e85c66819377e3520325cc3903ff2a&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `0.00%`.

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

```diff
@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   10.69%   10.39%   -0.30%     
==========================================
  Files          29       30       +1     
  Lines        1375     1414      +39     
==========================================
  Hits          147      147              
- Misses       1221     1260      +39     
  Partials        7        7              
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/116?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/cmd/kube\_bench.go](https://codecov.io/gh/aquasecurity/starboard/pull/116/diff?src=pr&el=tree#diff-cGtnL2NtZC9rdWJlX2JlbmNoLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/kubebench/crd/writer.go](https://codecov.io/gh/aquasecurity/starboard/pull/116/diff?src=pr&el=tree#diff-cGtnL2t1YmViZW5jaC9jcmQvd3JpdGVyLmdv) | `0.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/116?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/116?src=pr&el=footer). Last update [f807c83...53a5cfb](https://codecov.io/gh/aquasecurity/starboard/pull/116?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 5c1c3ee..23785b5 100644
--- a/README.md
+++ b/README.md
@@ -8,6 +8,7 @@
 [![Coverage Status][cov-img]][cov]
 [![Go Report Card][report-card-img]][report-card]
 [![License][license-img]][license]
+[![GitHub All Releases][github-all-releases-img]][release]
 
 ## Table of Contents
 
@@ -284,6 +285,7 @@ This repository is available under the [Apache License 2.0][license].
 [report-card]: https://goreportcard.com/report/github.com/aquasecurity/starboard
 [license-img]: https://img.shields.io/github/license/aquasecurity/starboard.svg
 [license]: https://github.com/aquasecurity/starboard/blob/master/LICENSE
+[github-all-releases-img]: https://img.shields.io/github/downloads/aquasecurity/starboard/total?logo=github
 
 [aqua-starboard-blog]: https://blog.aquasec.com/starboard-kubernetes-tools
 [discussions]: https://github.com/aquasecurity/starboard/discussions
diff --git a/pkg/cmd/kube_bench.go b/pkg/cmd/kube_bench.go
index b4f49f9..5462934 100644
--- a/pkg/cmd/kube_bench.go
+++ b/pkg/cmd/kube_bench.go
@@ -7,7 +7,6 @@ import (
 
 	core "k8s.io/api/core/v1"
 
-	"github.com/aquasecurity/starboard/pkg/ext"
 	starboard "github.com/aquasecurity/starboard/pkg/generated/clientset/versioned"
 	"github.com/aquasecurity/starboard/pkg/kubebench"
 	"github.com/aquasecurity/starboard/pkg/kubebench/crd"
@@ -46,7 +45,7 @@ func NewKubeBenchCmd(cf *genericclioptions.ConfigFlags) *cobra.Command {
 				return
 			}
 			scanner := kubebench.NewScanner(opts, kubernetesClientset)
-			writer := crd.NewWriter(ext.NewSystemClock(), starboardClientset)
+			writer := crd.NewReadWriter(starboardClientset)
 
 			var wg sync.WaitGroup
 
diff --git a/pkg/kube/starboard.go b/pkg/kube/starboard.go
index 4920ac4..cfe8cc7 100644
--- a/pkg/kube/starboard.go
+++ b/pkg/kube/starboard.go
@@ -24,11 +24,12 @@ const (
 	LabelScannerName   = "starboard.scanner.name"
 	LabelScannerVendor = "starboard.scanner.vendor"
 
+	// Deprecated We don't want to store historical reports in Starboard, only the current state.
+	// We should remove this label once we update the Octant plugin to not take it into account.
 	LabelHistoryLatest = "starboard.history.latest"
 )
 
 const (
-	AnnotationHistoryLimit    = "starboard.history.limit"
 	AnnotationContainerImages = "starboard.container-images"
 )
 
diff --git a/pkg/kubebench/crd/writer.go b/pkg/kubebench/crd/writer.go
index 6a20767..8a7a9b0 100644
--- a/pkg/kubebench/crd/writer.go
+++ b/pkg/kubebench/crd/writer.go
@@ -2,143 +2,72 @@ package crd
 
 import (
 	"context"
-	"fmt"
-	"strconv"
-
-	"github.com/aquasecurity/starboard/pkg/ext"
-	"github.com/aquasecurity/starboard/pkg/kube"
 
+	"github.com/aquasecurity/starboard/pkg/generated/clientset/versioned/typed/aquasecurity/v1alpha1"
 	"k8s.io/klog"
 
+	"k8s.io/apimachinery/pkg/api/errors"
+
+	"github.com/aquasecurity/starboard/pkg/kube"
+
 	core "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/utils/pointer"
 
 	starboard "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	starboardapi "github.com/aquasecurity/starboard/pkg/generated/clientset/versioned"
-	"github.com/aquasecurity/starboard/pkg/kubebench"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	defaultHistoryLimit = 10
-)
-
-type writer struct {
-	clock     ext.Clock
-	clientset starboardapi.Interface
+type ReadWriter struct {
+	reports v1alpha1.CISKubeBenchReportInterface
 }
 
-func NewWriter(clock ext.Clock, clientset starboardapi.Interface) kubebench.Writer {
-	return &writer{
-		clock:     clock,
-		clientset: clientset,
+func NewReadWriter(clientset starboardapi.Interface) *ReadWriter {
+	return &ReadWriter{
+		reports: clientset.AquasecurityV1alpha1().CISKubeBenchReports(),
 	}
 }
 
-func (w *writer) Write(ctx context.Context, report starboard.CISKubeBenchOutput, node *core.Node) (err error) {
-	reports, err := w.getReportsByNodeName(ctx, node.GetName())
-	if err != nil {
-		return
-	}
-	err = w.removeHistoryLatestLabel(ctx, reports)
-	if err != nil {
-		return
-	}
-	err = w.removeReportsWithHistoryLimitExceeded(ctx, reports)
-	if err != nil {
-		return
-	}
-
-	_, err = w.clientset.AquasecurityV1alpha1().CISKubeBenchReports().Create(ctx, &starboard.CISKubeBenchReport{
-		ObjectMeta: meta.ObjectMeta{
-			Name: fmt.Sprintf("%s-%d", node.Name, w.clock.Now().Unix()),
-			Labels: map[string]string{
-				kube.LabelResourceKind:  "Node", // TODO Why node.Kind is nil?
-				kube.LabelResourceName:  node.Name,
-				kube.LabelScannerName:   "kube-bench",
-				kube.LabelScannerVendor: "aqua",
-				kube.LabelHistoryLatest: "true",
-			},
-			Annotations: map[string]string{
-				// TODO Make this history limit configurable somehow, e.g. $ starboard kube-bench --history-limit=7
-				kube.AnnotationHistoryLimit: strconv.Itoa(10),
-			},
-			OwnerReferences: []meta.OwnerReference{
-				{
-					APIVersion: "v1",   // TODO Why node.APIVersion is nil?
-					Kind:       "Node", // TODO Why node.Kind is nil?
-					Name:       node.Name,
-					UID:        node.UID,
-					Controller: pointer.BoolPtr(false),
+func (w *ReadWriter) Write(ctx context.Context, report starboard.CISKubeBenchOutput, node *core.Node) error {
+	reportExisting, err := w.reports.Get(ctx, node.Name, meta.GetOptions{})
+	if err != nil && errors.IsNotFound(err) {
+		klog.V(3).Infof("Creating CISKubeBenchReport for %s node", node.Name)
+		_, err = w.reports.Create(ctx, &starboard.CISKubeBenchReport{
+			ObjectMeta: meta.ObjectMeta{
+				Name: node.Name,
+				Labels: map[string]string{
+					kube.LabelResourceKind:  string(kube.KindNode),
+					kube.LabelResourceName:  node.Name,
+					kube.LabelHistoryLatest: "true",
+				},
+				OwnerReferences: []meta.OwnerReference{
+					{
+						APIVersion: "v1",
+						Kind:       string(kube.KindNode),
+						Name:       node.Name,
+						UID:        node.UID,
+						Controller: pointer.BoolPtr(false),
+					},
 				},
 			},
-		},
-		Report: report,
-	}, meta.CreateOptions{})
-	return
-}
-
-func (w *writer) getReportsByNodeName(ctx context.Context, name string) (reports []starboard.CISKubeBenchReport, err error) {
-	list, err := w.clientset.AquasecurityV1alpha1().CISKubeBenchReports().List(ctx, meta.ListOptions{
-		LabelSelector: labels.Set{
-			kube.LabelResourceKind: "Node",
-			kube.LabelResourceName: name,
-		}.String(),
-	})
-	if err != nil {
-		return
-	}
-	reports = list.Items
-	return
-}
-
-func (w *writer) removeHistoryLatestLabel(ctx context.Context, reports []starboard.CISKubeBenchReport) (err error) {
-	for _, report := range reports {
-		if value, ok := report.Labels[kube.LabelHistoryLatest]; !ok || value != "true" {
-			continue
-		}
-		clone := report.DeepCopy()
-		delete(clone.Labels, kube.LabelHistoryLatest)
-		klog.V(3).Infof("Removing %s label from %s report", kube.LabelHistoryLatest, clone.Name)
-		_, err = w.clientset.AquasecurityV1alpha1().CISKubeBenchReports().Update(ctx, clone, meta.UpdateOptions{})
-		if err != nil {
-			return
-		}
-	}
-	return
-}
-
-func (w *writer) removeReportsWithHistoryLimitExceeded(ctx context.Context, reports []starboard.CISKubeBenchReport) (err error) {
-	limit := w.getHistoryLimit(reports)
-	diff := len(reports) - limit
-	if diff < 0 {
-		return
+			Report: report,
+		}, meta.CreateOptions{})
+		return err
 	}
-	for _, r := range reports[0 : diff+1] {
-		klog.V(3).Infof("Removing %s report which exceeded history limit of %d", r.GetName(), limit)
-		err = w.clientset.AquasecurityV1alpha1().CISKubeBenchReports().Delete(ctx, r.GetName(), meta.DeleteOptions{
-			GracePeriodSeconds: pointer.Int64Ptr(0),
-		})
-		if err != nil {
-			return
-		}
+	if err != nil {
+		return err
 	}
-	return
+	klog.V(3).Infof("Updating existing CISKubeBenchReport for %s node", node.Name)
+	reportCopied := reportExisting.DeepCopy()
+	reportCopied.Report = report
+	_, err = w.reports.Update(ctx, reportCopied, meta.UpdateOptions{})
+	return err
 }
 
-func (w *writer) getHistoryLimit(reports []starboard.CISKubeBenchReport) int {
-	if len(reports) == 0 {
-		return defaultHistoryLimit
-	}
-	latestReport := reports[len(reports)-1]
-	if value, ok := latestReport.Annotations[kube.AnnotationHistoryLimit]; ok {
-		limit, err := strconv.Atoi(value)
-		if err != nil {
-			klog.V(3).Infof("Error while parsing value %s of %s annotation", value, kube.AnnotationHistoryLimit)
-			return defaultHistoryLimit
-		}
-		return limit
+func (w *ReadWriter) Read(ctx context.Context, node kube.Object) (starboard.CISKubeBenchOutput, error) {
+	report, err := w.reports.Get(ctx, node.Name, meta.GetOptions{})
+	if err != nil {
+		return starboard.CISKubeBenchOutput{}, err
 	}
-	return defaultHistoryLimit
+	return report.Report, nil
 }
diff --git a/pkg/kubebench/writer.go b/pkg/kubebench/writer.go
index 265dc96..01c1cec 100644
--- a/pkg/kubebench/writer.go
+++ b/pkg/kubebench/writer.go
@@ -3,6 +3,8 @@ package kubebench
 import (
 	"context"
 
+	"github.com/aquasecurity/starboard/pkg/kube"
+
 	starboard "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	core "k8s.io/api/core/v1"
 )
@@ -10,3 +12,12 @@ import (
 type Writer interface {
 	Write(ctx context.Context, report starboard.CISKubeBenchOutput, node *core.Node) error
 }
+
+type Reader interface {
+	Read(ctx context.Context, node kube.Object) (starboard.CISKubeBenchOutput, error)
+}
+
+type ReadWriter interface {
+	Writer
+	Reader
+}


Test patch:
diff --git a/itest/starboard_cli_test.go b/itest/starboard_cli_test.go
index 46e26b9..9468c04 100644
--- a/itest/starboard_cli_test.go
+++ b/itest/starboard_cli_test.go
@@ -201,14 +201,13 @@ var _ = Describe("Starboard CLI", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			for _, nodeName := range nodeNames {
-				reportList, err := starboardClientset.AquasecurityV1alpha1().CISKubeBenchReports().List(context.TODO(), metav1.ListOptions{
-					LabelSelector: labels.Set{
-						kube.LabelResourceKind: string(kube.KindNode),
-						kube.LabelResourceName: nodeName,
-					}.String(),
-				})
-				Expect(err).ToNot(HaveOccurred())
-				Expect(reportList.Items).To(HaveLen(1), "Expected CISKubeBenchReport for node %s but not found", nodeName)
+				report, err := starboardClientset.AquasecurityV1alpha1().CISKubeBenchReports().Get(context.TODO(), nodeName, metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred(), "Expected CISKubeBenchReport for node %s but not found", nodeName)
+				Expect(report.Labels).To(MatchAllKeys(Keys{
+					kube.LabelResourceKind:  Equal(string(kube.KindNode)),
+					kube.LabelResourceName:  Equal(nodeName),
+					kube.LabelHistoryLatest: Equal("true"),
+				}))
 			}
 		})
 	})
diff --git a/pkg/find/vulnerabilities/crd/writer_test.go b/pkg/find/vulnerabilities/crd/writer_test.go
index 03c1cd2..c85ca2b 100644
--- a/pkg/find/vulnerabilities/crd/writer_test.go
+++ b/pkg/find/vulnerabilities/crd/writer_test.go
@@ -11,7 +11,7 @@ import (
 	"github.com/aquasecurity/starboard/pkg/kube"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var (
@@ -29,11 +29,11 @@ var (
 
 func TestReadWriter_Read(t *testing.T) {
 	clientset := fake.NewSimpleClientset(&v1alpha1.Vulnerability{
-		TypeMeta: meta.TypeMeta{
+		TypeMeta: metav1.TypeMeta{
 			Kind:       "Vulnerability",
 			APIVersion: "v1alpha1",
 		},
-		ObjectMeta: meta.ObjectMeta{
+		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "my-namespace",
 			Name:      "my-deploy-my-container-01",
 			Labels: map[string]string{
@@ -45,11 +45,11 @@ func TestReadWriter_Read(t *testing.T) {
 		},
 		Report: vulnerabilityReport01,
 	}, &v1alpha1.Vulnerability{
-		TypeMeta: meta.TypeMeta{
+		TypeMeta: metav1.TypeMeta{
 			Kind:       "Vulnerability",
 			APIVersion: "v1alpha1",
 		},
-		ObjectMeta: meta.ObjectMeta{
+		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "my-namespace",
 			Name:      "my-deploy-my-container-02",
 			Labels: map[string]string{
@@ -61,11 +61,11 @@ func TestReadWriter_Read(t *testing.T) {
 		},
 		Report: vulnerabilityReport02,
 	}, &v1alpha1.Vulnerability{
-		TypeMeta: meta.TypeMeta{
+		TypeMeta: metav1.TypeMeta{
 			Kind:       "Vulnerability",
 			APIVersion: "v1alpha1",
 		},
-		ObjectMeta: meta.ObjectMeta{
+		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "my-namespace",
 			Name:      "my-sts",
 			Labels: map[string]string{
diff --git a/pkg/kubebench/crd/writer_test.go b/pkg/kubebench/crd/writer_test.go
new file mode 100644
index 0000000..a5f50ed
--- /dev/null
+++ b/pkg/kubebench/crd/writer_test.go
@@ -0,0 +1,146 @@
+package crd_test
+
+import (
+	"context"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/labels"
+
+	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
+	starboardClientset "github.com/aquasecurity/starboard/pkg/generated/clientset/versioned"
+	starboardClientsetFake "github.com/aquasecurity/starboard/pkg/generated/clientset/versioned/fake"
+	"github.com/aquasecurity/starboard/pkg/kube"
+	"github.com/aquasecurity/starboard/pkg/kubebench/crd"
+	. "github.com/onsi/gomega/gstruct"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestReadWriter(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "ReadWriter Suite")
+}
+
+var _ = Describe("ReadWriter", func() {
+
+	var (
+		workerNodeName = "worker-node"
+		workerNode     = &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: workerNodeName,
+			},
+		}
+		workerNodeReport = v1alpha1.CISKubeBenchOutput{
+			Scanner: v1alpha1.Scanner{
+				Name: "worker-node-report",
+			},
+		}
+	)
+
+	var (
+		objects   []runtime.Object
+		clientset starboardClientset.Interface
+		rw        *crd.ReadWriter
+	)
+
+	BeforeEach(func() {
+		objects = []runtime.Object{}
+		clientset = starboardClientsetFake.NewSimpleClientset(objects...)
+		rw = crd.NewReadWriter(clientset)
+	})
+
+	Describe("Writing report", func() {
+
+		Context("when report does not exist", func() {
+
+			It("should create a report", func() {
+				err := rw.Write(context.TODO(), workerNodeReport, workerNode)
+				Expect(err).ToNot(HaveOccurred())
+
+				report, err := clientset.AquasecurityV1alpha1().CISKubeBenchReports().
+					Get(context.TODO(), workerNodeName, metav1.GetOptions{})
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(report.Labels).To(MatchAllKeys(Keys{
+					kube.LabelResourceKind:  Equal(string(kube.KindNode)),
+					kube.LabelResourceName:  Equal(workerNodeName),
+					kube.LabelHistoryLatest: Equal("true"),
+				}))
+			})
+
+		})
+
+		Context("when report already exists", func() {
+
+			BeforeEach(func() {
+				err := rw.Write(context.TODO(), workerNodeReport, workerNode)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			It("should update a report", func() {
+				err := rw.Write(context.TODO(), workerNodeReport, workerNode)
+				Expect(err).ToNot(HaveOccurred())
+
+				report, err := clientset.AquasecurityV1alpha1().CISKubeBenchReports().
+					Get(context.TODO(), workerNodeName, metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(report.Labels).To(MatchAllKeys(Keys{
+					kube.LabelResourceKind:  Equal(string(kube.KindNode)),
+					kube.LabelResourceName:  Equal(workerNodeName),
+					kube.LabelHistoryLatest: Equal("true"),
+				}))
+
+				reportList, err := clientset.AquasecurityV1alpha1().CISKubeBenchReports().
+					List(context.TODO(), metav1.ListOptions{
+						LabelSelector: labels.Set{
+							kube.LabelResourceKind: string(kube.KindNode),
+							kube.LabelResourceName: workerNodeName,
+						}.String(),
+					})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reportList.Items).To(HaveLen(1))
+			})
+		})
+
+	})
+
+	Describe("Reading report", func() {
+
+		Context("when report does not exist", func() {
+
+			It("should return error not found", func() {
+				_, err := rw.Read(context.TODO(), kube.Object{
+					Kind: kube.KindNode,
+					Name: workerNodeName,
+				})
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(Equal("ciskubebenchreports.aquasecurity.github.io \"worker-node\" not found"))
+			})
+
+		})
+
+		Context("when report exists", func() {
+
+			BeforeEach(func() {
+				err := rw.Write(context.TODO(), workerNodeReport, workerNode)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			It("should return report", func() {
+				report, err := rw.Read(context.TODO(), kube.Object{
+					Kind: kube.KindNode,
+					Name: workerNodeName,
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(report).To(Equal(workerNodeReport))
+			})
+
+		})
+	})
+
+})