feat: Create or update CISKubeBenchReports (#116)

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>

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

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"),
+				}))
 			}
 		})
 	})

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
 

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{

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"
 )
 

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
 }