From dba8174b0c8ffd20765db3e4ac19e24838214196 Mon Sep 17 00:00:00 2001 From: alecmerdler Date: Tue, 17 Dec 2019 18:12:34 -0500 Subject: [PATCH] add logic for skip-gc behavior of ImageManifestVuln --- ...operator.v1.0.1.clusterserviceversion.yaml | 14 ++++++++++ labeller/labeller.go | 16 +++++++++++ labeller/labeller_test.go | 21 +++++++-------- labeller/manifest.go | 4 +++ labeller/manifest_test.go | 27 ++++++++++++++++++- 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/deploy/manifests/container-security-operator/1.0.1/container-security-operator.v1.0.1.clusterserviceversion.yaml b/deploy/manifests/container-security-operator/1.0.1/container-security-operator.v1.0.1.clusterserviceversion.yaml index aebcf7c..7d16e65 100644 --- a/deploy/manifests/container-security-operator/1.0.1/container-security-operator.v1.0.1.clusterserviceversion.yaml +++ b/deploy/manifests/container-security-operator/1.0.1/container-security-operator.v1.0.1.clusterserviceversion.yaml @@ -9,6 +9,20 @@ metadata: description: Identify image vulnerabilities in Kubernetes pods repository: https://github.com/quay/container-security-operator tectonic-visibility: ocs + alm-examples: |- + [ + { + "apiVersion": "secscan.quay.redhat.com/v1alpha1", + "kind": "ImageManifestVuln", + "metadata": { + "name": "example", + "annotations": { + "imagemanifestvuln.skip-gc": "true" + } + }, + "spec": {} + } + ] name: container-security-operator.v1.0.1 namespace: placeholder spec: diff --git a/labeller/labeller.go b/labeller/labeller.go index 9d6e863..de86b33 100644 --- a/labeller/labeller.go +++ b/labeller/labeller.go @@ -3,6 +3,7 @@ package labeller import ( "errors" "fmt" + "reflect" "strings" "time" @@ -37,6 +38,8 @@ var ( Defcon1Label = "Defcon1" ) +const skipGCAnnotation = "imagemanifestvuln.skip-gc" + type Labeller struct { kclient kubernetes.Interface sclient secscanclient.Interface @@ -251,6 +254,8 @@ func (l *Labeller) handleAddImageManifestVuln(obj interface{}) { level.Debug(l.logger).Log("msg", "ImageManifestVuln added", "key", key) prometheus.PromImageManifestVulnEventsTotal.WithLabelValues("add", imgmanifestvuln.Namespace).Inc() } + + l.handleNoGC(imgmanifestvuln, &secscanv1alpha1.ImageManifestVuln{Spec: secscanv1alpha1.ImageManifestVulnSpec{Image: "none"}}) } func (l *Labeller) handleDeleteImageManifestVuln(obj interface{}) { @@ -273,6 +278,8 @@ func (l *Labeller) handleUpdateImageManifestVuln(oldObj, newObj interface{}) { level.Debug(l.logger).Log("msg", "ImageManifestVuln updated", "key", key) prometheus.PromImageManifestVulnEventsTotal.WithLabelValues("update", imgmanifestvuln.Namespace).Inc() } + + l.handleNoGC(imgmanifestvuln, oldObj.(*secscanv1alpha1.ImageManifestVuln)) } func (l *Labeller) waitForCacheSync(stopc <-chan struct{}) error { @@ -506,3 +513,12 @@ func (l *Labeller) podInNamespaces(pod *corev1.Pod) bool { } return false } + +func (l *Labeller) handleNoGC(obj, oldObj *secscanv1alpha1.ImageManifestVuln) { + if _, ok := obj.GetAnnotations()[skipGCAnnotation]; ok && !reflect.DeepEqual(obj.Spec, oldObj.Spec) { + _, err := l.sclient.SecscanV1alpha1().ImageManifestVulns(obj.GetNamespace()).UpdateStatus(updateImageManifestVulnLastUpdate(obj)) + if err != nil { + level.Error(l.logger).Log("msg", "Error updating noop ImageManifestVuln", "err", err) + } + } +} diff --git a/labeller/labeller_test.go b/labeller/labeller_test.go index 7a6f461..5cd8ced 100644 --- a/labeller/labeller_test.go +++ b/labeller/labeller_test.go @@ -120,7 +120,6 @@ func newFakeLabeller(ctx context.Context, c *testClient, config *Config) *Labell resyncThreshold: config.ResyncThreshold, wellKnownEndpoint: config.WellknownEndpoint, prometheus: prometheus.NewServer(config.PrometheusAddr), - vulnerabilities: NewLockableVulnerabilites(), queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "fakeLabeller"), } @@ -229,7 +228,7 @@ func TestSkipNonRunningPod(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) for _, pod := range pods { - err := fakeLabeller.SecurityLabelPod(pod.Namespace + "/" + pod.Name) + err := fakeLabeller.Reconcile(pod.Namespace + "/" + pod.Name) if assert.Error(t, err) { if pod.Status.Phase == corev1.PodRunning { assert.Equal(t, err, fmt.Errorf("Pod condition not ready")) @@ -259,7 +258,7 @@ func TestNonVulnerablePod(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err := fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err := fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -289,7 +288,7 @@ func TestVulnerablePodCreateImageManifestVuln(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err := fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err := fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -319,7 +318,7 @@ func TestVulnerablePodUpdateImageManifestVuln(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err := fakeLabeller.SecurityLabelPod(runningPod1.Namespace + "/" + runningPod1.Name) + err := fakeLabeller.Reconcile(runningPod1.Namespace + "/" + runningPod1.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -332,7 +331,7 @@ func TestVulnerablePodUpdateImageManifestVuln(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err = fakeLabeller.SecurityLabelPod(runningPod2.Namespace + "/" + runningPod2.Name) + err = fakeLabeller.Reconcile(runningPod2.Namespace + "/" + runningPod2.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -364,7 +363,7 @@ func TestForcedResyncThreshold(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err := fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err := fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -374,7 +373,7 @@ func TestForcedResyncThreshold(t *testing.T) { initialTimestamp, _ := lastManfestUpdateTime(manifest) // Resyncing a pod too early should have no effects - err = fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err = fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -390,7 +389,7 @@ func TestForcedResyncThreshold(t *testing.T) { assert.NoError(t, err) // Rescan the pod and check for new lastUpdate - err = fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err = fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -419,7 +418,7 @@ func TestGarbageCollectManifest(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Scan the pod - err := fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err := fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) @@ -434,7 +433,7 @@ func TestGarbageCollectManifest(t *testing.T) { assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) // Delete event reconciliation - err = fakeLabeller.SecurityLabelPod(runningPod.Namespace + "/" + runningPod.Name) + err = fakeLabeller.Reconcile(runningPod.Namespace + "/" + runningPod.Name) assert.NoError(t, err) assert.NoError(t, fakeLabeller.waitForCacheSync(ctx.Done())) diff --git a/labeller/manifest.go b/labeller/manifest.go index 0ce3afb..0715b58 100644 --- a/labeller/manifest.go +++ b/labeller/manifest.go @@ -279,6 +279,10 @@ func garbageCollectManifests(podclient corev1.PodInterface, manifestclient secsc } for _, manifest := range manifestList.Items { + if _, ok := manifest.GetAnnotations()[skipGCAnnotation]; ok { + return nil + } + var ( updated bool updatedManifest *secscanv1alpha1.ImageManifestVuln diff --git a/labeller/manifest_test.go b/labeller/manifest_test.go index 43ba3e9..a623ee7 100644 --- a/labeller/manifest_test.go +++ b/labeller/manifest_test.go @@ -22,7 +22,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/kubernetes/typed/core/v1" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" ) func randString(n int) string { @@ -396,6 +396,31 @@ func TestGarbageCollectManifestDeletion(t *testing.T) { assert.Equal(t, sortedPodKeysFromPods(testPods), sortedAffectedPodsKeys(manifests[0])) } +func TestGarbageCollectManifestNoop(t *testing.T) { + ns := generateNamespaceForTest(t) + + c := newTestClient() + + noopManifest := &secscanv1alpha1.ImageManifestVuln{} + noopManifest.SetLabels(map[string]string{"imagemanifestvuln.example": "true"}) + if _, err := c.imageManifestVulnsClient(ns).Create(noopManifest); err != nil { + t.Fatal(err) + } + + // Garbage collecting manifest with noop label should not delete anything + err := garbageCollectManifests(c.podsClient(ns), c.imageManifestVulnsClient(ns)) + if err != nil { + t.Fatal(err) + } + + // Check that the number of manifests if the same + manifestList, err := c.imageManifestVulnsClient(ns).List(metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + assert.Len(t, manifestList.Items, 1) +} + func TestGarbageCollectManifestDanglingPods(t *testing.T) { ns := generateNamespaceForTest(t) testImageID := "quay.io/test/redis@sha256:94033a42da840b970fd9d2b04dae5fec56add2714ca674a758d030ce5acba27e"