Skip to content

No GC Handler #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

No GC Handler #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ...ner-security-operator/1.0.1/container-security-operator.v1.0.1.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions labeller/labeller.go
View file
Open in desktop
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 @@ -37,6 +38,8 @@ var (
Defcon1Label = "Defcon1"
)

const skipGCAnnotation = "imagemanifestvuln.skip-gc"

type Labeller struct {
kclient kubernetes.Interface
sclient secscanclient.Interface
Expand Down Expand Up @@ -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{}) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not really a noop if it's updating the status.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, noop isn't the best wording. We're essentially trying to implement something that will pass the operator-scorecard test that assumes that a user will create an ImageManifestVuln from the alm-examples annotation on our CSV, and expects the status block to change if they update the spec. This Operator doesn't work like that, but we can add a special handler for it.

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
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions labeller/manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion labeller/manifest_test.go
View file
Open in desktop
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