Skip to content

Commit

Permalink
add logic for noop handler of ImageManifestVulns
Browse files Browse the repository at this point in the history
  • Loading branch information
alecmerdler committed Dec 18, 2019
1 parent 749c235 commit fdb47d0
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 12 deletions.
14 changes: 14 additions & 0 deletions labeller/labeller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package labeller
import (
"errors"
"fmt"
"reflect"
"strings"
"time"

Expand Down Expand Up @@ -251,6 +252,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.handleNoop(imgmanifestvuln, &secscanv1alpha1.ImageManifestVuln{Spec: secscanv1alpha1.ImageManifestVulnSpec{Image: "none"}})
}

func (l *Labeller) handleDeleteImageManifestVuln(obj interface{}) {
Expand All @@ -273,6 +276,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.handleNoop(imgmanifestvuln, oldObj.(*secscanv1alpha1.ImageManifestVuln))
}

func (l *Labeller) waitForCacheSync(stopc <-chan struct{}) error {
Expand Down Expand Up @@ -506,3 +511,12 @@ func (l *Labeller) podInNamespaces(pod *corev1.Pod) bool {
}
return false
}

func (l *Labeller) handleNoop(obj, oldObj *secscanv1alpha1.ImageManifestVuln) {
if _, ok := obj.GetLabels()["imagemanifestvuln.example"]; 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)
}
}
}
21 changes: 10 additions & 11 deletions labeller/labeller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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()))

Expand Down Expand Up @@ -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()))

Expand Down Expand Up @@ -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()))

Expand All @@ -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()))

Expand Down Expand Up @@ -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()))

Expand All @@ -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()))

Expand All @@ -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()))

Expand Down Expand Up @@ -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()))

Expand All @@ -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()))

Expand Down
5 changes: 5 additions & 0 deletions labeller/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ func garbageCollectManifests(podclient corev1.PodInterface, manifestclient secsc
)
updatedManifest, updated = removeDanglingPods(currentPodKeys, manifest)

// Noop label
if _, ok := updatedManifest.GetLabels()["imagemanifestvuln.example"]; ok {
return nil
}

if len(updatedManifest.Status.AffectedPods) == 0 {
if err := manifestclient.Delete(updatedManifest.Name, &metav1.DeleteOptions{}); err != nil {
return fmt.Errorf("Failed to delete unreferenced ImageManifestVuln: %w", err)
Expand Down
27 changes: 26 additions & 1 deletion labeller/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit fdb47d0

Please sign in to comment.