Repository: aquasecurity/starboard
Pull request #489 (2021-04-09 15:07:29)

refactor(conftest): Skip rescan when plugin ConfigMap is deleted
This wasn't an issue and we were handling ConfigMap deletion as
expected. However, I took this oportunity to refactor the code
a bit and use predicate instead of conditionally ingoring ConfigMap
events in the reconciliation loop. With predicate the events that
we're not interested in are not even added to the worker queue.

Resolves: #482

codecov[bot]: # [Codecov](https://codecov.io/gh/aquasecurity/starboard/pull/489?src=pr&el=h1) Report
> Merging [#489](https://codecov.io/gh/aquasecurity/starboard/pull/489?src=pr&el=desc) (98cad73) into [main](https://codecov.io/gh/aquasecurity/starboard/commit/8929137cd59fbb21e881b4ee14011ed6d1989c6d?el=desc) (8929137) will **decrease** coverage by `34.20%`.
> The diff coverage is `85.71%`.

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

```diff
@@             Coverage Diff             @@
##             main     #489       +/-   ##
===========================================
- Coverage   69.25%   35.05%   -34.21%     
===========================================
  Files          62       61        -1     
  Lines        3984     3860      -124     
===========================================
- Hits         2759     1353     -1406     
- Misses        928     2428     +1500     
+ Partials      297       79      -218     
```


| [Impacted Files](https://codecov.io/gh/aquasecurity/starboard/pull/489?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/operator/controller/plugins\_config.go](https://codecov.io/gh/aquasecurity/starboard/pull/489/diff?src=pr&el=tree#diff-cGtnL29wZXJhdG9yL2NvbnRyb2xsZXIvcGx1Z2luc19jb25maWcuZ28=) | `0.00% <0.00%> (-21.06%)` | :arrow_down: |
| [pkg/operator/predicate/predicate.go](https://codecov.io/gh/aquasecurity/starboard/pull/489/diff?src=pr&el=tree#diff-cGtnL29wZXJhdG9yL3ByZWRpY2F0ZS9wcmVkaWNhdGUuZ28=) | `70.37% <100.00%> (-21.63%)` | :arrow_down: |
| [pkg/plugin/conftest/plugin.go](https://codecov.io/gh/aquasecurity/starboard/pull/489/diff?src=pr&el=tree#diff-cGtnL3BsdWdpbi9jb25mdGVzdC9wbHVnaW4uZ28=) | `66.03% <100.00%> (ø)` | |
| [pkg/starboard/plugin.go](https://codecov.io/gh/aquasecurity/starboard/pull/489/diff?src=pr&el=tree#diff-cGtnL3N0YXJib2FyZC9wbHVnaW4uZ28=) | `80.95% <100.00%> (+0.95%)` | :arrow_up: |
| [pkg/cmd/get.go](https://codecov.io/gh/aquasecurity/starboard/pull/489/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/489/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/489/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/489/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/489/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/489/diff?src=pr&el=tree#diff-cGtnL2NtZC92ZXJzaW9uLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [36 more](https://codecov.io/gh/aquasecurity/starboard/pull/489/diff?src=pr&el=tree-more) | |

------

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


Patch:
diff --git a/pkg/operator/controller/plugins_config.go b/pkg/operator/controller/plugins_config.go
index 098f648..498b0ba 100644
--- a/pkg/operator/controller/plugins_config.go
+++ b/pkg/operator/controller/plugins_config.go
@@ -3,7 +3,6 @@ package controller
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1"
 	"github.com/aquasecurity/starboard/pkg/configauditreport"
@@ -32,6 +31,7 @@ func (r *PluginsConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.ConfigMap{}, builder.WithPredicates(
 			predicate.Not(predicate.IsBeingTerminated),
+			predicate.HasName(starboard.GetPluginConfigMapName(r.PluginContext.GetName())),
 			predicate.InNamespace(r.Config.Namespace))).
 		Complete(r)
 }
@@ -39,11 +39,6 @@ func (r *PluginsConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *PluginsConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := r.Logger.WithValues("configMap", req.NamespacedName)
 
-	// TODO Use Predicate instead
-	if req.Name != "starboard-"+strings.ToLower(r.PluginContext.GetName())+"-config" {
-		return ctrl.Result{}, nil
-	}
-
 	cm := &corev1.ConfigMap{}
 
 	err := r.Client.Get(ctx, req.NamespacedName, cm)
diff --git a/pkg/operator/predicate/predicate.go b/pkg/operator/predicate/predicate.go
index 3dd487b..7c1c60c 100644
--- a/pkg/operator/predicate/predicate.go
+++ b/pkg/operator/predicate/predicate.go
@@ -42,6 +42,14 @@ var InstallModePredicate = func(config etc.Config) (predicate.Predicate, error)
 	}), nil
 }
 
+// HasName is predicate.Predicate that returns true if the
+// specified client.Object has the desired name.
+var HasName = func(name string) predicate.Predicate {
+	return predicate.NewPredicateFuncs(func(obj client.Object) bool {
+		return name == obj.GetName()
+	})
+}
+
 // InNamespace is a predicate.Predicate that returns true if the
 // specified client.Object is in the desired namespace.
 var InNamespace = func(namespace string) predicate.Predicate {
@@ -68,8 +76,8 @@ var IsBeingTerminated = predicate.NewPredicateFuncs(func(obj client.Object) bool
 	return obj.GetDeletionTimestamp() != nil
 })
 
-// JobHasConditions is a predicate.Predicate that returns true if the
-// specified client.Object is a batchv1.Job with any batchv1.JobConditionType.
+// JobHasAnyCondition is a predicate.Predicate that returns true if the
+// specified client.Object is a v1.Job with any v1.JobConditionType.
 var JobHasAnyCondition = predicate.NewPredicateFuncs(func(obj client.Object) bool {
 	if job, ok := obj.(*batchv1.Job); ok {
 		return len(job.Status.Conditions) > 0
diff --git a/pkg/starboard/plugin.go b/pkg/starboard/plugin.go
index 26452d5..646da3e 100644
--- a/pkg/starboard/plugin.go
+++ b/pkg/starboard/plugin.go
@@ -26,6 +26,12 @@ type PluginContext interface {
 	GetServiceAccountName() string
 }
 
+// GetPluginConfigMapName returns the name of a ConfigMap used to configure a plugin
+// with the given name.
+func GetPluginConfigMapName(pluginName string) string {
+	return "starboard-" + strings.ToLower(pluginName) + "-config"
+}
+
 type pluginContext struct {
 	name               string
 	client             client.Client


Test patch:
diff --git a/pkg/operator/predicate/predicate_test.go b/pkg/operator/predicate/predicate_test.go
index c371e12..a60ea1c 100644
--- a/pkg/operator/predicate/predicate_test.go
+++ b/pkg/operator/predicate/predicate_test.go
@@ -121,6 +121,43 @@ var _ = Describe("Predicate", func() {
 		})
 	})
 
+	Describe("When checking a HasName predicate", func() {
+
+		Context("When object has desired name", func() {
+
+			It("Should return true", func() {
+				instance := predicate.HasName("starboard-polaris-config")
+				obj := &corev1.Pod{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "starboard-polaris-config",
+					},
+				}
+
+				Expect(instance.Create(event.CreateEvent{Object: obj})).To(BeTrue())
+				Expect(instance.Update(event.UpdateEvent{ObjectNew: obj})).To(BeTrue())
+				Expect(instance.Delete(event.DeleteEvent{Object: obj})).To(BeTrue())
+				Expect(instance.Generic(event.GenericEvent{Object: obj})).To(BeTrue())
+			})
+		})
+
+		Context("When object does not have desired name", func() {
+
+			It("Should return false", func() {
+				instance := predicate.HasName("starboard-conftest-config")
+				obj := &corev1.Pod{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "starboard",
+					},
+				}
+
+				Expect(instance.Create(event.CreateEvent{Object: obj})).To(BeFalse())
+				Expect(instance.Update(event.UpdateEvent{ObjectNew: obj})).To(BeFalse())
+				Expect(instance.Delete(event.DeleteEvent{Object: obj})).To(BeFalse())
+				Expect(instance.Generic(event.GenericEvent{Object: obj})).To(BeFalse())
+			})
+		})
+	})
+
 	Describe("When checking a InNamespace predicate", func() {
 
 		Context("When object is in desired namespace", func() {
diff --git a/pkg/plugin/conftest/plugin.go b/pkg/plugin/conftest/plugin.go
index 9f1968e..9cbed27 100644
--- a/pkg/plugin/conftest/plugin.go
+++ b/pkg/plugin/conftest/plugin.go
@@ -115,7 +115,7 @@ func (p *plugin) GetScanJobSpec(ctx starboard.PluginContext, obj client.Object)
 				VolumeSource: corev1.VolumeSource{
 					ConfigMap: &corev1.ConfigMapVolumeSource{
 						LocalObjectReference: corev1.LocalObjectReference{
-							Name: "starboard-conftest-config",
+							Name: starboard.GetPluginConfigMapName(ctx.GetName()),
 						},
 						Items: volumeItems,
 					},
diff --git a/pkg/starboard/plugin_test.go b/pkg/starboard/plugin_test.go
index 4e77a9f..050f3e9 100644
--- a/pkg/starboard/plugin_test.go
+++ b/pkg/starboard/plugin_test.go
@@ -10,6 +10,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+func TestGetPluginConfigMapName(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	name := starboard.GetPluginConfigMapName("Conftest")
+	g.Expect(name).To(gomega.Equal("starboard-conftest-config"))
+}
+
 func TestPluginContext_GetConfig(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)