Repository: aquasecurity/starboard
Pull request #598 (2021-05-24 19:23:44)

refactor(cli): Use deterministic names for vulnerability scan jobs
Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/598?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) Report
> Merging [#598](https://codecov.io/gh/aquasecurity/starboard/pull/598?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) (5e67a8f) into [main](https://codecov.io/gh/aquasecurity/starboard/commit/f0080b9522af9d038cb9c536df0a53ea45615a47?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) (f0080b9) will **decrease** coverage by `25.77%`.
> The diff coverage is `7.40%`.

[![Impacted file tree graph](https://codecov.io/gh/aquasecurity/starboard/pull/598/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/598?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity)

```diff
@@             Coverage Diff             @@
##             main     #598       +/-   ##
===========================================
- Coverage   71.18%   45.40%   -25.78%     
===========================================
  Files          57       41       -16     
  Lines        4216     3572      -644     
===========================================
- Hits         3001     1622     -1379     
- Misses        876     1855      +979     
+ Partials      339       95      -244     
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/598?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity) | Coverage Δ | |
|---|---|---|
| [pkg/operator/controller/vulnerabilityreport.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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%> (-69.73%)` | :arrow_down: |
| [pkg/vulnerabilityreport/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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%> (-72.65%)` | :arrow_down: |
| [pkg/vulnerabilityreport/builder.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity#diff-cGtnL3Z1bG5lcmFiaWxpdHlyZXBvcnQvYnVpbGRlci5nbw==) | `71.11% <28.57%> (-8.89%)` | :arrow_down: |
| [pkg/kubehunter/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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/598/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%> (-77.15%)` | :arrow_down: |
| [pkg/configauditreport/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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/operator/controller/configauditreport.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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%> (-66.67%)` | :arrow_down: |
| [pkg/kube/runnable\_job.go](https://codecov.io/gh/aquasecurity/starboard/pull/598/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/598/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/598/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/598?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/598?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aquasecurity). Last update [f0080b9...5e67a8f](https://codecov.io/gh/aquasecurity/starboard/pull/598?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/pkg/operator/controller/vulnerabilityreport.go b/pkg/operator/controller/vulnerabilityreport.go
index 5409067..ae7d4ee 100644
--- a/pkg/operator/controller/vulnerabilityreport.go
+++ b/pkg/operator/controller/vulnerabilityreport.go
@@ -395,11 +395,12 @@ func (r *VulnerabilityReportReconciler) processCompleteScanJob(ctx context.Conte
 		}
 		_ = logsStream.Close()
 
-		report, err := vulnerabilityreport.NewBuilder(r.Client.Scheme()).
-			Owner(ownerObj).
+		report, err := vulnerabilityreport.NewReportBuilder(r.Client.Scheme()).
+			Controller(ownerObj).
 			Container(containerName).
-			Result(scanResult).
-			PodSpecHash(hash).Get()
+			Data(scanResult).
+			PodSpecHash(hash).
+			Get()
 		if err != nil {
 			return err
 		}
diff --git a/pkg/vulnerabilityreport/builder.go b/pkg/vulnerabilityreport/builder.go
index c4c6f8f..e5c9104 100644
--- a/pkg/vulnerabilityreport/builder.go
+++ b/pkg/vulnerabilityreport/builder.go
@@ -10,24 +10,19 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/pointer"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
-type Builder interface {
-	Owner(owner metav1.Object) Builder
-	Container(name string) Builder
-	PodSpecHash(hash string) Builder
-	Result(result v1alpha1.VulnerabilityScanResult) Builder
-	Get() (v1alpha1.VulnerabilityReport, error)
+func GetScanJobName(obj client.Object) string {
+	return fmt.Sprintf("scan-vulnerabilityreport-%s", kube.ComputeHash(kube.Object{
+		Kind:      kube.Kind(obj.GetObjectKind().GroupVersionKind().Kind),
+		Namespace: obj.GetNamespace(),
+		Name:      obj.GetName(),
+	}))
 }
 
-func NewBuilder(scheme *runtime.Scheme) Builder {
-	return &builder{
-		scheme: scheme,
-	}
-}
-
-type builder struct {
+type ReportBuilder struct {
 	scheme    *runtime.Scheme
 	owner     metav1.Object
 	container string
@@ -35,27 +30,33 @@ type builder struct {
 	result    v1alpha1.VulnerabilityScanResult
 }
 
-func (b *builder) Owner(owner metav1.Object) Builder {
+func NewReportBuilder(scheme *runtime.Scheme) *ReportBuilder {
+	return &ReportBuilder{
+		scheme: scheme,
+	}
+}
+
+func (b *ReportBuilder) Controller(owner metav1.Object) *ReportBuilder {
 	b.owner = owner
 	return b
 }
 
-func (b *builder) Container(name string) Builder {
+func (b *ReportBuilder) Container(name string) *ReportBuilder {
 	b.container = name
 	return b
 }
 
-func (b *builder) PodSpecHash(hash string) Builder {
+func (b *ReportBuilder) PodSpecHash(hash string) *ReportBuilder {
 	b.hash = hash
 	return b
 }
 
-func (b *builder) Result(result v1alpha1.VulnerabilityScanResult) Builder {
+func (b *ReportBuilder) Data(result v1alpha1.VulnerabilityScanResult) *ReportBuilder {
 	b.result = result
 	return b
 }
 
-func (b *builder) reportName() (string, error) {
+func (b *ReportBuilder) reportName() (string, error) {
 	kind, err := kube.KindForObject(b.owner, b.scheme)
 	if err != nil {
 		return "", err
@@ -64,7 +65,7 @@ func (b *builder) reportName() (string, error) {
 		b.owner.GetName(), b.container), nil
 }
 
-func (b *builder) Get() (v1alpha1.VulnerabilityReport, error) {
+func (b *ReportBuilder) Get() (v1alpha1.VulnerabilityReport, error) {
 	kind, err := kube.KindForObject(b.owner, b.scheme)
 	if err != nil {
 		return v1alpha1.VulnerabilityReport{}, fmt.Errorf("getting kind for object: %w", err)
diff --git a/pkg/vulnerabilityreport/scanner.go b/pkg/vulnerabilityreport/scanner.go
index 20ee3e1..f9d5dc7 100644
--- a/pkg/vulnerabilityreport/scanner.go
+++ b/pkg/vulnerabilityreport/scanner.go
@@ -6,7 +6,6 @@ import (
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	"github.com/aquasecurity/starboard/pkg/docker"
-	"github.com/aquasecurity/starboard/pkg/ext"
 	"github.com/aquasecurity/starboard/pkg/kube"
 	"github.com/aquasecurity/starboard/pkg/runner"
 	"github.com/aquasecurity/starboard/pkg/starboard"
@@ -30,7 +29,6 @@ type Scanner struct {
 	logsReader     kube.LogsReader
 	config         starboard.ConfigData
 	opts           kube.ScannerOpts
-	idGenerator    ext.IDGenerator
 	secretsReader  kube.SecretsReader
 }
 
@@ -53,7 +51,6 @@ func NewScanner(
 		objectResolver: &kube.ObjectResolver{Client: client},
 		logsReader:     kube.NewLogsReader(clientset),
 		config:         config,
-		idGenerator:    ext.NewGoogleUUIDGenerator(),
 		secretsReader:  kube.NewSecretsReader(client),
 	}
 }
@@ -84,7 +81,7 @@ func (s *Scanner) Scan(ctx context.Context, workload kube.Object) ([]v1alpha1.Vu
 		return nil, err
 	}
 
-	job, secrets, err := s.prepareScanJob(workload, spec, credentials)
+	job, secrets, err := s.prepareScanJob(owner, spec, credentials)
 	if err != nil {
 		return nil, fmt.Errorf("preparing scan job: %w", err)
 	}
@@ -119,7 +116,7 @@ func (s *Scanner) getCredentials(ctx context.Context, spec corev1.PodSpec, ns st
 	return kube.MapContainerNamesToDockerAuths(kube.GetContainerImagesFromPodSpec(spec), imagePullSecrets)
 }
 
-func (s *Scanner) prepareScanJob(workload kube.Object, spec corev1.PodSpec, credentials map[string]docker.Auth) (*batchv1.Job, []*corev1.Secret, error) {
+func (s *Scanner) prepareScanJob(workload client.Object, spec corev1.PodSpec, credentials map[string]docker.Auth) (*batchv1.Job, []*corev1.Secret, error) {
 	templateSpec, secrets, err := s.plugin.GetScanJobSpec(spec, credentials)
 	if err != nil {
 		return nil, nil, err
@@ -143,15 +140,20 @@ func (s *Scanner) prepareScanJob(workload kube.Object, spec corev1.PodSpec, cred
 		return nil, nil, err
 	}
 
+	labels := map[string]string{
+		starboard.LabelResourceKind:      workload.GetObjectKind().GroupVersionKind().Kind,
+		starboard.LabelResourceName:      workload.GetName(),
+		starboard.LabelResourceNamespace: workload.GetNamespace(),
+		// TODO Add pod-spec-hash or resource-spec-hash
+		starboard.LabelK8SAppManagedBy:            starboard.AppStarboard,
+		starboard.LabelVulnerabilityReportScanner: "true",
+	}
+
 	return &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      s.idGenerator.GenerateID(),
+			Name:      GetScanJobName(workload),
 			Namespace: starboard.NamespaceName,
-			Labels: map[string]string{
-				starboard.LabelResourceKind:      string(workload.Kind),
-				starboard.LabelResourceName:      workload.Name,
-				starboard.LabelResourceNamespace: workload.Namespace,
-			},
+			Labels:    labels,
 			Annotations: map[string]string{
 				starboard.AnnotationContainerImages: containerImagesAsJSON,
 			},
@@ -162,11 +164,7 @@ func (s *Scanner) prepareScanJob(workload kube.Object, spec corev1.PodSpec, cred
 			ActiveDeadlineSeconds: kube.GetActiveDeadlineSeconds(s.opts.ScanJobTimeout),
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						starboard.LabelResourceKind:      string(workload.Kind),
-						starboard.LabelResourceName:      workload.Name,
-						starboard.LabelResourceNamespace: workload.Namespace,
-					},
+					Labels:      labels,
 					Annotations: scanJobAnnotations,
 				},
 				Spec: templateSpec,
@@ -199,12 +197,13 @@ func (s *Scanner) getVulnerabilityReportsByScanJob(ctx context.Context, job *bat
 
 		_ = logsStream.Close()
 
-		report, err := NewBuilder(s.scheme).
-			Owner(owner).
+		report, err := NewReportBuilder(s.scheme).
+			Controller(owner).
 			Container(containerName).
-			Result(result).
+			Data(result).
 			// TODO Add pod template hash like we do in the Operator
-			PodSpecHash("").Get()
+			PodSpecHash("").
+			Get()
 		if err != nil {
 			return nil, err
 		}


Test patch:
diff --git a/pkg/vulnerabilityreport/builder_test.go b/pkg/vulnerabilityreport/builder_test.go
index 3f2bfeb..9f7b1a4 100644
--- a/pkg/vulnerabilityreport/builder_test.go
+++ b/pkg/vulnerabilityreport/builder_test.go
@@ -15,8 +15,8 @@ import (
 
 func TestBuilder(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
-	report, err := vulnerabilityreport.NewBuilder(scheme.Scheme).
-		Owner(&appsv1.ReplicaSet{
+	report, err := vulnerabilityreport.NewReportBuilder(scheme.Scheme).
+		Controller(&appsv1.ReplicaSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "some-owner",
 				Namespace: "qa",
@@ -24,7 +24,8 @@ func TestBuilder(t *testing.T) {
 		}).
 		Container("my-container").
 		PodSpecHash("xyz").
-		Result(v1alpha1.VulnerabilityScanResult{}).Get()
+		Data(v1alpha1.VulnerabilityScanResult{}).
+		Get()
 
 	g.Expect(err).ToNot(gomega.HaveOccurred())
 	g.Expect(report).To(gomega.Equal(v1alpha1.VulnerabilityReport{