Repository: aquasecurity/starboard
Pull request #432 (2021-03-16 15:34:50)

refactor: Remove redundant args passed to configauditreport.Plugin
Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/432?src=pr&el=h1) Report
> Merging [#432](https://codecov.io/gh/aquasecurity/starboard/pull/432?src=pr&el=desc) (a6f41e4) into [main](https://codecov.io/gh/aquasecurity/starboard/commit/0e234c174f46810eab72b21ab1fef4a94d92cd63?el=desc) (0e234c1) will **decrease** coverage by `37.33%`.
> The diff coverage is `30.76%`.

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

```diff
@@             Coverage Diff             @@
##             main     #432       +/-   ##
===========================================
- Coverage   68.74%   31.40%   -37.34%     
===========================================
  Files          57       56        -1     
  Lines        3369     3270       -99     
===========================================
- Hits         2316     1027     -1289     
- Misses        776     2175     +1399     
+ Partials      277       68      -209     
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/432?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/configauditreport/scanner.go](https://codecov.io/gh/aquasecurity/starboard/pull/432/diff?src=pr&el=tree#diff-cGtnL2NvbmZpZ2F1ZGl0cmVwb3J0L3NjYW5uZXIuZ28=) | `0.00% <0.00%> (-81.95%)` | :arrow_down: |
| [pkg/kube/object.go](https://codecov.io/gh/aquasecurity/starboard/pull/432/diff?src=pr&el=tree#diff-cGtnL2t1YmUvb2JqZWN0Lmdv) | `45.68% <0.00%> (-19.49%)` | :arrow_down: |
| [pkg/operator/controller/configauditreport.go](https://codecov.io/gh/aquasecurity/starboard/pull/432/diff?src=pr&el=tree#diff-cGtnL29wZXJhdG9yL2NvbnRyb2xsZXIvY29uZmlnYXVkaXRyZXBvcnQuZ28=) | `0.00% <0.00%> (-66.36%)` | :arrow_down: |
| [pkg/plugin/polaris/plugin.go](https://codecov.io/gh/aquasecurity/starboard/pull/432/diff?src=pr&el=tree#diff-cGtnL3BsdWdpbi9wb2xhcmlzL3BsdWdpbi5nbw==) | `91.24% <100.00%> (+0.06%)` | :arrow_up: |
| [pkg/cmd/get.go](https://codecov.io/gh/aquasecurity/starboard/pull/432/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/432/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/432/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/432/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/432/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/432/diff?src=pr&el=tree#diff-cGtnL2NtZC92ZXJzaW9uLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [35 more](https://codecov.io/gh/aquasecurity/starboard/pull/432/diff?src=pr&el=tree-more) | |

------

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


Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 13fa704..d18f504 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -198,6 +198,11 @@ started with a basic development workflow. For other install modes see [Operator
    This will create the `starboard-operator` namespace, and the `starboard-operator` service account. Beyond that,
    it will create the `starboard-operator` ClusterRole and bind it to the `starboard-operator` service account in the
    `starboard-operator` namespace via the `starboard-operator` ClusterRoleBinding.
+3. (Optional) Create configuration objects:
+
+   ```
+   $ kubectl create -f deploy/static/05-starboard-operator.config.yaml
+   ```
 
 ### In cluster
 
diff --git a/pkg/configauditreport/plugin.go b/pkg/configauditreport/plugin.go
index 5605d9c..2b0f56f 100644
--- a/pkg/configauditreport/plugin.go
+++ b/pkg/configauditreport/plugin.go
@@ -4,9 +4,7 @@ import (
 	"io"
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
-	"github.com/aquasecurity/starboard/pkg/kube"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -15,9 +13,18 @@ import (
 // first step to separate generic workloads discovery code and Polaris
 // implementation details.
 type Plugin interface {
-	GetScanJobSpec(workload kube.Object, obj client.Object, gvk schema.GroupVersionKind) (corev1.PodSpec, []*corev1.Secret, error)
 
-	GetContainerName() string
+	// GetScanJobSpec describes the pod that will be created by Starboard when
+	// it schedules a Kubernetes job to scan the specified workload client.Object.
+	// The plugin might return zero to many v1.Secret objects which will be
+	// created by Starboard and associated with the scan job.
+	GetScanJobSpec(obj client.Object) (corev1.PodSpec, []*corev1.Secret, error)
+
+	// ParseConfigAuditReportData is a callback to parse and convert logs of
+	// the container in a pod controlled by the scan job to v1alpha1.ConfigAuditResult.
+	ParseConfigAuditReportData(logsReader io.ReadCloser) (v1alpha1.ConfigAuditResult, error)
 
-	ParseConfigAuditResult(logsReader io.ReadCloser) (v1alpha1.ConfigAuditResult, error)
+	// GetContainerName returns the name of the container in a pod created by a scan job
+	// to read logs from.
+	GetContainerName() string
 }
diff --git a/pkg/configauditreport/scanner.go b/pkg/configauditreport/scanner.go
index c179766..a375810 100644
--- a/pkg/configauditreport/scanner.go
+++ b/pkg/configauditreport/scanner.go
@@ -56,7 +56,7 @@ func (s *Scanner) Scan(ctx context.Context, workload kube.Object, gvk schema.Gro
 	}
 
 	klog.V(3).Infof("Scanning with options: %+v", s.opts)
-	job, secrets, err := s.getScanJob(workload, owner, gvk)
+	job, secrets, err := s.getScanJob(workload, owner)
 	if err != nil {
 		return v1alpha1.ConfigAuditReport{}, err
 	}
@@ -87,7 +87,7 @@ func (s *Scanner) Scan(ctx context.Context, workload kube.Object, gvk schema.Gro
 		return v1alpha1.ConfigAuditReport{}, fmt.Errorf("getting logs: %w", err)
 	}
 
-	result, err := s.plugin.ParseConfigAuditResult(logsStream)
+	result, err := s.plugin.ParseConfigAuditReportData(logsStream)
 	defer func() {
 		_ = logsStream.Close()
 	}()
@@ -98,8 +98,8 @@ func (s *Scanner) Scan(ctx context.Context, workload kube.Object, gvk schema.Gro
 		Get()
 }
 
-func (s *Scanner) getScanJob(workload kube.Object, obj client.Object, gvk schema.GroupVersionKind) (*batchv1.Job, []*corev1.Secret, error) {
-	jobSpec, secrets, err := s.plugin.GetScanJobSpec(workload, obj, gvk)
+func (s *Scanner) getScanJob(workload kube.Object, obj client.Object) (*batchv1.Job, []*corev1.Secret, error) {
+	jobSpec, secrets, err := s.plugin.GetScanJobSpec(obj)
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/pkg/kube/object.go b/pkg/kube/object.go
index 71df570..116f4da 100644
--- a/pkg/kube/object.go
+++ b/pkg/kube/object.go
@@ -172,6 +172,11 @@ func (o *ObjectResolver) GetObjectFromPartialObject(ctx context.Context, workloa
 	if err != nil {
 		return nil, err
 	}
+	gvk, err := apiutil.GVKForObject(obj, o.Client.Scheme())
+	if err != nil {
+		return nil, err
+	}
+	obj.GetObjectKind().SetGroupVersionKind(gvk)
 	return obj, nil
 }
 
diff --git a/pkg/operator/controller/configauditreport.go b/pkg/operator/controller/configauditreport.go
index b5ab2a2..e0f36e8 100644
--- a/pkg/operator/controller/configauditreport.go
+++ b/pkg/operator/controller/configauditreport.go
@@ -20,7 +20,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
@@ -222,12 +221,7 @@ func (r *ConfigAuditReportReconciler) getScanJobName(workload kube.Object) strin
 }
 
 func (r *ConfigAuditReportReconciler) getScanJob(workload kube.Object, obj client.Object, hash string) (*batchv1.Job, []*corev1.Secret, error) {
-	gvk, err := apiutil.GVKForObject(obj, r.Client.Scheme())
-	if err != nil {
-		return nil, nil, err
-	}
-
-	jobSpec, secrets, err := r.Plugin.GetScanJobSpec(workload, obj, gvk)
+	jobSpec, secrets, err := r.Plugin.GetScanJobSpec(obj)
 
 	if err != nil {
 		return nil, nil, err
@@ -341,7 +335,7 @@ func (r *ConfigAuditReportReconciler) processCompleteScanJob(ctx context.Context
 		return fmt.Errorf("getting logs: %w", err)
 	}
 
-	result, err := r.Plugin.ParseConfigAuditResult(logsStream)
+	result, err := r.Plugin.ParseConfigAuditReportData(logsStream)
 	defer func() {
 		_ = logsStream.Close()
 	}()
diff --git a/pkg/plugin/polaris/plugin.go b/pkg/plugin/polaris/plugin.go
index e8bfff1..7034005 100644
--- a/pkg/plugin/polaris/plugin.go
+++ b/pkg/plugin/polaris/plugin.go
@@ -8,12 +8,10 @@ import (
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	"github.com/aquasecurity/starboard/pkg/configauditreport"
 	"github.com/aquasecurity/starboard/pkg/ext"
-	"github.com/aquasecurity/starboard/pkg/kube"
 	"github.com/aquasecurity/starboard/pkg/starboard"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -41,12 +39,12 @@ func NewPlugin(clock ext.Clock, config Config) configauditreport.Plugin {
 	}
 }
 
-func (p *plugin) GetScanJobSpec(workload kube.Object, obj client.Object, gvk schema.GroupVersionKind) (corev1.PodSpec, []*corev1.Secret, error) {
+func (p *plugin) GetScanJobSpec(obj client.Object) (corev1.PodSpec, []*corev1.Secret, error) {
 	imageRef, err := p.config.GetPolarisImageRef()
 	if err != nil {
 		return corev1.PodSpec{}, nil, err
 	}
-	sourceName := p.sourceNameFrom(workload, gvk)
+	sourceName := p.sourceNameFrom(obj)
 
 	return corev1.PodSpec{
 		ServiceAccountName:           starboard.ServiceAccountName,
@@ -118,7 +116,7 @@ func (p *plugin) GetContainerName() string {
 	return polarisContainerName
 }
 
-func (p *plugin) ParseConfigAuditResult(logsReader io.ReadCloser) (v1alpha1.ConfigAuditResult, error) {
+func (p *plugin) ParseConfigAuditReportData(logsReader io.ReadCloser) (v1alpha1.ConfigAuditResult, error) {
 	var report Report
 	err := json.NewDecoder(logsReader).Decode(&report)
 	if err != nil {
@@ -127,17 +125,18 @@ func (p *plugin) ParseConfigAuditResult(logsReader io.ReadCloser) (v1alpha1.Conf
 	return p.configAuditResultFrom(report.Results[0])
 }
 
-func (p *plugin) sourceNameFrom(workload kube.Object, gvk schema.GroupVersionKind) string {
+func (p *plugin) sourceNameFrom(obj client.Object) string {
+	gvk := obj.GetObjectKind().GroupVersionKind()
 	group := gvk.Group
 	if len(group) > 0 {
 		group = "." + group
 	}
 	return fmt.Sprintf("%s/%s%s/%s/%s",
-		workload.Namespace,
+		obj.GetNamespace(),
 		gvk.Kind,
 		group,
 		gvk.Version,
-		workload.Name,
+		obj.GetName(),
 	)
 }
 


Test patch:
diff --git a/pkg/plugin/polaris/plugin_test.go b/pkg/plugin/polaris/plugin_test.go
index 7d268a2..e0202b3 100644
--- a/pkg/plugin/polaris/plugin_test.go
+++ b/pkg/plugin/polaris/plugin_test.go
@@ -1,13 +1,14 @@
 package polaris_test
 
 import (
+	appsv1 "k8s.io/api/apps/v1"
 	"os"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"testing"
 	"time"
 
 	"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/plugin/polaris"
 	"github.com/aquasecurity/starboard/pkg/starboard"
 	"github.com/stretchr/testify/assert"
@@ -15,7 +16,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/utils/pointer"
 )
 
@@ -28,9 +28,8 @@ func TestPlugin_GetScanJobSpec(t *testing.T) {
 	testCases := []struct {
 		name string
 
-		config   starboard.ConfigData
-		workload kube.Object
-		gvk      schema.GroupVersionKind
+		config starboard.ConfigData
+		obj    client.Object
 
 		expectedJobSpec corev1.PodSpec
 	}{
@@ -39,15 +38,15 @@ func TestPlugin_GetScanJobSpec(t *testing.T) {
 			config: starboard.ConfigData{
 				"polaris.imageRef": "quay.io/fairwinds/polaris:3.0",
 			},
-			workload: kube.Object{
-				Name:      "nginx",
-				Namespace: corev1.NamespaceDefault,
-				Kind:      kube.KindDeployment,
-			},
-			gvk: schema.GroupVersionKind{
-				Group:   "apps",
-				Version: "v1",
-				Kind:    "Deployment",
+			obj: &appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "apps/v1",
+					Kind:       "Deployment",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "nginx",
+					Namespace: metav1.NamespaceDefault,
+				},
 			},
 			expectedJobSpec: corev1.PodSpec{
 				ServiceAccountName:           starboard.ServiceAccountName,
@@ -119,7 +118,7 @@ func TestPlugin_GetScanJobSpec(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			plugin := polaris.NewPlugin(fixedClock, tc.config)
-			jobSpec, secrets, err := plugin.GetScanJobSpec(tc.workload, &corev1.Pod{}, tc.gvk)
+			jobSpec, secrets, err := plugin.GetScanJobSpec(tc.obj)
 			require.NoError(t, err, tc.name)
 			assert.Nil(t, secrets)
 			assert.Equal(t, tc.expectedJobSpec, jobSpec, tc.name)
@@ -144,7 +143,7 @@ func TestPlugin_ParseConfigAuditResult(t *testing.T) {
 		"polaris.imageRef": "quay.io/fairwinds/polaris:3.0",
 	}
 	plugin := polaris.NewPlugin(fixedClock, config)
-	result, err := plugin.ParseConfigAuditResult(testReport)
+	result, err := plugin.ParseConfigAuditReportData(testReport)
 	require.NoError(t, err)
 	assert.Equal(t, metav1.NewTime(fixedTime), result.UpdateTimestamp)
 	assert.Equal(t, v1alpha1.Scanner{