feat: Deleting a VulnerabilityReport should trigger rescan (#458)

Resolves: #414 Resolves: #451 Signed-off-by: Daniel Pacak <pacak.daniel@gmail.com>
aquasecurity · Mar 29, 2021 · 20182e2 · 20182e2
1 parent 2060f7b
commit 20182e2
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 251 deletions.
diff --git a/itest/matcher/matcher.go b/itest/matcher/matcher.go
@@ -67,10 +67,12 @@ func (m *vulnerabilityReportMatcher) Match(actual interface{}) (bool, error) {
 				kube.LabelResourceNamespace: Equal(m.owner.GetNamespace()),
 			}),
 			"OwnerReferences": ConsistOf(metav1.OwnerReference{
-				APIVersion: gvk.GroupVersion().Identifier(),
-				Kind:       gvk.Kind,
-				Name:       m.owner.GetName(),
-				UID:        m.owner.GetUID(),
+				APIVersion:         gvk.GroupVersion().Identifier(),
+				Kind:               gvk.Kind,
+				Name:               m.owner.GetName(),
+				UID:                m.owner.GetUID(),
+				Controller:         pointer.BoolPtr(true),
+				BlockOwnerDeletion: pointer.BoolPtr(true),
 			}),
 		}),
 		"Report": MatchFields(IgnoreExtras, Fields{

diff --git a/itest/matcher/matcher_test.go b/itest/matcher/matcher_test.go
@@ -44,10 +44,12 @@ func TestVulnerabilityReportMatcher(t *testing.T) {
 				},
 				OwnerReferences: []metav1.OwnerReference{
 					{
-						APIVersion: "v1",
-						Kind:       "Pod",
-						Name:       "nginx-pod",
-						UID:        "56d53a84-c81b-4620-81a1-e226c35d3983",
+						APIVersion:         "v1",
+						Kind:               "Pod",
+						Name:               "nginx-pod",
+						UID:                "56d53a84-c81b-4620-81a1-e226c35d3983",
+						Controller:         pointer.BoolPtr(true),
+						BlockOwnerDeletion: pointer.BoolPtr(true),
 					},
 				},
 			},

diff --git a/itest/starboard-operator/starboard_operator_test.go b/itest/starboard-operator/starboard_operator_test.go
@@ -187,8 +187,7 @@ var _ = Describe("Starboard Operator", func() {
 
 		It("Should create VulnerabilityReport and ConfigAuditReport", func() {
 			Eventually(HasConfigAuditReportOwnedBy(cronJob), assertionTimeout).Should(BeTrue())
-			// FIXME(issue: #415): Assign VulnerabilityReports to CronJob instead of Jobs. The PodSpec of a CronJob does not change so there's no point in rescanning individual Jobs.
-			// Eventually(HasVulnerabilityReportOwnedBy(cronJob), assertionTimeout).Should(BeTrue())
+			Eventually(HasVulnerabilityReportOwnedBy(cronJob), assertionTimeout).Should(BeTrue())
 		})
 
 		AfterEach(func() {

diff --git a/pkg/kube/resources.go b/pkg/kube/resources.go
@@ -8,7 +8,6 @@ import (
 	"github.com/davecgh/go-spew/spew"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/rand"
 )
 
@@ -41,44 +40,6 @@ func GetContainerImagesFromJob(job *batchv1.Job) (ContainerImages, error) {
 	return containerImages, nil
 }
 
-// GetImmediateOwnerReference returns the immediate owner of the specified pod.
-// For example, for a pod controlled by a Deployment it will return the active
-// ReplicaSet, whereas for an unmanaged pod the immediate owner is the pod
-// itself.
-//
-// Note that kubelet can manage pods independently by reading pod definition
-// files from a configured host directory (typically /etc/kubernetes/manifests).
-// Such pods are called *static pods* and are owned by a cluster Node.
-// In this case we treat them as unmanaged pods. (Otherwise we'd require
-// cluster-scoped permissions to get Nodes in order to set the owner reference
-// when we create an instance of custom security report.)
-//
-// Pods created and controlled by third party frameworks, such as Argo workflow
-// engine, are considered as unmanaged. Otherwise we'd need to maintain and
-// extend the list of RBAC permissions over time.
-// TODO Merge this method with ObjectResolver, which accepts kube.Object and resolves client.Object.
-func GetImmediateOwnerReference(pod *corev1.Pod) Object {
-	ownerRef := metav1.GetControllerOf(pod)
-
-	if ownerRef != nil {
-		switch ownerRef.Kind {
-		case "Pod", "ReplicaSet", "ReplicationController", "Deployment", "StatefulSet", "DaemonSet", "CronJob", "Job":
-			return Object{
-				Namespace: pod.Namespace,
-				Kind:      Kind(ownerRef.Kind),
-				Name:      ownerRef.Name,
-			}
-		}
-	}
-
-	// Pod owned by anything else is treated the same as an unmanaged pod
-	return Object{
-		Kind:      KindPod,
-		Namespace: pod.Namespace,
-		Name:      pod.Name,
-	}
-}
-
 // ComputeHash returns a hash value calculated from a given object.
 // The hash will be safe encoded to avoid bad words.
 func ComputeHash(obj interface{}) string {

diff --git a/pkg/kube/resources_test.go b/pkg/kube/resources_test.go
@@ -10,7 +10,6 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/utils/pointer"
 )
 
 func TestGetContainerImagesFromPodSpec(t *testing.T) {
@@ -66,104 +65,6 @@ func TestGetContainerImagesFromJob(t *testing.T) {
 	})
 }
 
-func TestGetImmediateOwnerReference(t *testing.T) {
-
-	testCases := []struct {
-		name          string
-		pod           *corev1.Pod
-		expectedOwner kube.Object
-	}{
-		{
-			name: "Should return pod as owner of unmanaged pod",
-			pod: &corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "foo",
-					Name:      "unmanaged-pod",
-				},
-			},
-			expectedOwner: kube.Object{
-				Kind:      "Pod",
-				Namespace: "foo",
-				Name:      "unmanaged-pod",
-			},
-		},
-		{
-			name: "Should return ReplicaSet as owner of pod managed by Deployment",
-			pod: &corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "bar",
-					Name:      "nginx-6d4cf56db6-8g9j6",
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: "apps/v1",
-							Kind:       "ReplicaSet",
-							Name:       "nginx-6d4cf56db6",
-							Controller: pointer.BoolPtr(true),
-						},
-					},
-				},
-			},
-			expectedOwner: kube.Object{
-				Kind:      "ReplicaSet",
-				Namespace: "bar",
-				Name:      "nginx-6d4cf56db6",
-			},
-		},
-		{
-			name: "Should return pod as owner of static pod managed by kubelet",
-			pod: &corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "kube-system",
-					Name:      "etcd-kind-control-plane",
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: "v1",
-							Kind:       "Node",
-							Name:       "kind-control-plane",
-							Controller: pointer.BoolPtr(true),
-						},
-					},
-				},
-			},
-			expectedOwner: kube.Object{
-				Kind:      "Pod",
-				Namespace: "kube-system",
-				Name:      "etcd-kind-control-plane",
-			},
-		},
-		{
-			name: "Should return pod as owner of pod managed by third party workload",
-			pod: &corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: "dev",
-					Name:      "hello-world-argo",
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: "argoproj.io/v1alpha1",
-							Kind:       "Workflow",
-							Name:       "hello-world-argo-r99sq",
-							Controller: pointer.BoolPtr(true),
-						},
-					},
-				},
-			},
-			expectedOwner: kube.Object{
-				Kind:      "Pod",
-				Namespace: "dev",
-				Name:      "hello-world-argo",
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			owner := kube.GetImmediateOwnerReference(tc.pod)
-			assert.Equal(t, tc.expectedOwner, owner)
-		})
-	}
-
-}
-
 func TestComputeHash(t *testing.T) {
 
 	booleanValue1 := true

diff --git a/pkg/operator/controller/ciskubebenchreport.go b/pkg/operator/controller/ciskubebenchreport.go
@@ -298,6 +298,9 @@ func (r *CISKubeBenchReportReconciler) processCompleteScanJob(ctx context.Contex
 func (r *CISKubeBenchReportReconciler) deleteJob(ctx context.Context, job *batchv1.Job) error {
 	err := r.Client.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground))
 	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
 		return fmt.Errorf("deleting job: %w", err)
 	}
 	return nil

diff --git a/pkg/operator/controller/configauditreport.go b/pkg/operator/controller/configauditreport.go
@@ -35,8 +35,13 @@ type ConfigAuditReportReconciler struct {
 	configauditreport.ReadWriter
 }
 
-var (
-	workloads = []struct {
+func (r *ConfigAuditReportReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	installModePredicate, err := InstallModePredicate(r.Config)
+	if err != nil {
+		return err
+	}
+
+	workloads := []struct {
 		kind       kube.Kind
 		forObject  client.Object
 		ownsObject client.Object
@@ -49,13 +54,6 @@ var (
 		{kind: kube.KindCronJob, forObject: &batchv1beta1.CronJob{}, ownsObject: &v1alpha1.ConfigAuditReport{}},
 		{kind: kube.KindJob, forObject: &batchv1.Job{}, ownsObject: &v1alpha1.ConfigAuditReport{}},
 	}
-)
-
-func (r *ConfigAuditReportReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	installModePredicate, err := InstallModePredicate(r.Config)
-	if err != nil {
-		return err
-	}
 
 	for _, workload := range workloads {
 		err = ctrl.NewControllerManagedBy(mgr).
@@ -97,7 +95,7 @@ func (r *ConfigAuditReportReconciler) reconcileWorkload(workloadKind kube.Kind)
 			return ctrl.Result{}, fmt.Errorf("getting %s from cache: %w", workloadKind, err)
 		}
 
-		// Skip processing if it's a Pod controlled by a standard K8s workload.
+		// Skip processing if it's a Pod controlled by a built-in K8s workload.
 		if workloadKind == kube.KindPod {
 			controller := metav1.GetControllerOf(workloadObj)
 			if kube.IsBuiltInWorkload(controller) {