Repository: zegl/kube-score (go)

Issue:
kube-score/ignore ignored at pod level inside a statefulset
I am deploying Trivy with their Helm chart, so Trivy is deployed as a statefulset.

The Trivy chart only allows to add annotations at the pod level (i.e. not at the statefulset level) so I added the `kube-score/ignore`  annotation at the pod level:
```
apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: trivy
  namespace: trivy-staging
spec:
  podManagementPolicy: Parallel
  replicas: 3
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app.kubernetes.io/instance: trivy
      app.kubernetes.io/name: trivy
  serviceName: trivy
  template:
    metadata:
      annotations:
        kube-score/ignore: container-image-tag,pod-probes
      labels:
        app.kubernetes.io/instance: trivy
        app.kubernetes.io/name: trivy
    spec:
      automountServiceAccountToken: false
      containers:
      - args:
        - server
        envFrom:
        - configMapRef:
            name: trivy
        - secretRef:
            name: trivy
        image: aquasec/trivy:latest
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 10
          httpGet:
            path: /healthz
            port: trivy-http
            scheme: HTTP
          initialDelaySeconds: 5
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        name: main
        ports:
        - containerPort: 4954
          name: trivy-http
          protocol: TCP
        readinessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz
            port: trivy-http
            scheme: HTTP
          initialDelaySeconds: 5
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        resources:
          limits:
            cpu: "1"
            ephemeral-storage: 128Mi
            memory: 1Gi
          requests:
            cpu: 200m
            memory: 512Mi
        securityContext:
          privileged: false
          readOnlyRootFilesystem: true
          runAsGroup: 65534
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
        volumeMounts:
        - mountPath: /tmp
          name: tmp-data
        - mountPath: /home/scanner/.cache
          name: data
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext:
        fsGroup: 65534
        runAsNonRoot: true
        runAsUser: 65534
      serviceAccount: trivy
      serviceAccountName: trivy
      terminationGracePeriodSeconds: 30
      volumes:
      - emptyDir: {}
        name: tmp-data
  updateStrategy:
    rollingUpdate:
      partition: 0
    type: RollingUpdate
  volumeClaimTemplates:
  - apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      creationTimestamp: null
      name: data
    spec:
      accessModes:
      - ReadWriteOnce
      resources:
        requests:
          storage: 5Gi
      volumeMode: Filesystem
    status:
      phase: Pending
```

But then kube-score does not ignore the `container-image-tag` and `pod-probes` checks:
```
apps/v1/StatefulSet trivy in trivy-staging                                    💥
    [WARNING] Container Ephemeral Storage Request and Limit
        · main -> Ephemeral Storage request is not set
            Resource requests are recommended to make sure the application can
            start and run without crashing. Set
            resource.requests.ephemeral-storage
    [CRITICAL] Pod NetworkPolicy
        · The pod does not have a matching NetworkPolicy
            Create a NetworkPolicy that targets this pod to control who/what
            can communicate with this pod. Note, this feature needs to be
            supported by the CNI implementation used in the Kubernetes cluster
            to have an effect.
    [CRITICAL] Container Image Tag
        · main -> Image with latest tag
            Using a fixed tag is recommended to avoid accidental upgrades
    [CRITICAL] Pod Probes
        · Container has the same readiness and liveness probe
            Using the same probe for liveness and readiness is very likely
            dangerous. Generally it's better to avoid the livenessProbe than
            re-using the readinessProbe.
            More information: https://github.com/zegl/kube-score/blob/master/README_PROBES.md
    [WARNING] StatefulSet has host PodAntiAffinity
        · StatefulSet does not have a host podAntiAffinity set
            It's recommended to set a podAntiAffinity that stops multiple pods
            from a statefulset from being scheduled on the same node. This
            increases availability in case the node becomes unavailable.
    [CRITICAL] StatefulSet has ServiceName
        · StatefulSet does not have a valid serviceName
            StatefulSets currently require a Headless Service to be responsible
            for the network identity of the Pods. You are responsible for
            creating this Service.
            https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#limitations
    [CRITICAL] StatefulSet has PodDisruptionBudget
        · No matching PodDisruptionBudget was found
            It's recommended to define a PodDisruptionBudget to avoid
            unexpected downtime during Kubernetes maintenance operations, such
            as when draining a node.

```

Pull request description:
score: allow ignore/allow annotations on child template resources
```
RELNOTE: Adds support for the kube-score/ignore and kube-score/enable annotation on child resources such as Deployment and StatefulSet Pod Templates. The "child" annotations will override an annotations set on the root object.
```

Fixes #488

Patch:
diff --git a/parser/parse.go b/parser/parse.go
index ddf0908..37918c6 100644
--- a/parser/parse.go
+++ b/parser/parse.go
@@ -243,7 +243,11 @@ func detectFileLocation(fileName string, fileOffset int, fileContents []byte) ks
 func (p *Parser) decodeItem(cnf config.Configuration, s *parsedObjects, detectedVersion schema.GroupVersionKind, fileName string, fileOffset int, fileContents []byte) error {
 	addPodSpeccer := func(ps ks.PodSpecer) {
 		s.podspecers = append(s.podspecers, ps)
-		s.bothMetas = append(s.bothMetas, ks.BothMeta{TypeMeta: ps.GetTypeMeta(), ObjectMeta: ps.GetObjectMeta(), FileLocationer: ps})
+		s.bothMetas = append(s.bothMetas, ks.BothMeta{
+			TypeMeta:       ps.GetTypeMeta(),
+			ObjectMeta:     ps.GetObjectMeta(),
+			FileLocationer: ps,
+		})
 	}
 
 	fileLocation := detectFileLocation(fileName, fileOffset, fileContents)
diff --git a/score/score.go b/score/score.go
index 32f918b..a987969 100644
--- a/score/score.go
+++ b/score/score.go
@@ -54,14 +54,14 @@ func Score(allObjects ks.AllTypes, cnf config.Configuration) (*scorecard.Scoreca
 	for _, ingress := range allObjects.Ingresses() {
 		o := newObject(ingress.GetTypeMeta(), ingress.GetObjectMeta())
 		for _, test := range allChecks.Ingresses() {
-			o.Add(test.Fn(ingress), test.Check, ingress)
+			o.Add(test.Fn(ingress), test.Check, ingress, ingress.GetObjectMeta().Annotations)
 		}
 	}
 
 	for _, meta := range allObjects.Metas() {
 		o := newObject(meta.TypeMeta, meta.ObjectMeta)
 		for _, test := range allChecks.Metas() {
-			o.Add(test.Fn(meta), test.Check, meta)
+			o.Add(test.Fn(meta), test.Check, meta, meta.ObjectMeta.Annotations)
 		}
 	}
 
@@ -72,7 +72,7 @@ func Score(allObjects ks.AllTypes, cnf config.Configuration) (*scorecard.Scoreca
 				ObjectMeta: pod.Pod().ObjectMeta,
 				Spec:       pod.Pod().Spec,
 			}, pod.Pod().TypeMeta)
-			o.Add(score, test.Check, pod)
+			o.Add(score, test.Check, pod, pod.Pod().ObjectMeta.Annotations)
 		}
 	}
 
@@ -80,14 +80,17 @@ func Score(allObjects ks.AllTypes, cnf config.Configuration) (*scorecard.Scoreca
 		o := newObject(podspecer.GetTypeMeta(), podspecer.GetObjectMeta())
 		for _, test := range allChecks.Pods() {
 			score := test.Fn(podspecer.GetPodTemplateSpec(), podspecer.GetTypeMeta())
-			o.Add(score, test.Check, podspecer)
+			o.Add(score, test.Check, podspecer,
+				podspecer.GetObjectMeta().Annotations,
+				podspecer.GetPodTemplateSpec().Annotations,
+			)
 		}
 	}
 
 	for _, service := range allObjects.Services() {
 		o := newObject(service.Service().TypeMeta, service.Service().ObjectMeta)
 		for _, test := range allChecks.Services() {
-			o.Add(test.Fn(service.Service()), test.Check, service)
+			o.Add(test.Fn(service.Service()), test.Check, service, service.Service().Annotations)
 		}
 	}
 
@@ -98,7 +101,7 @@ func Score(allObjects ks.AllTypes, cnf config.Configuration) (*scorecard.Scoreca
 			if err != nil {
 				return nil, err
 			}
-			o.Add(res, test.Check, statefulset)
+			o.Add(res, test.Check, statefulset, statefulset.StatefulSet().ObjectMeta.Annotations)
 		}
 	}
 
@@ -109,35 +112,35 @@ func Score(allObjects ks.AllTypes, cnf config.Configuration) (*scorecard.Scoreca
 			if err != nil {
 				return nil, err
 			}
-			o.Add(res, test.Check, deployment)
+			o.Add(res, test.Check, deployment, deployment.Deployment().ObjectMeta.Annotations)
 		}
 	}
 
 	for _, netpol := range allObjects.NetworkPolicies() {
 		o := newObject(netpol.NetworkPolicy().TypeMeta, netpol.NetworkPolicy().ObjectMeta)
 		for _, test := range allChecks.NetworkPolicies() {
-			o.Add(test.Fn(netpol.NetworkPolicy()), test.Check, netpol)
+			o.Add(test.Fn(netpol.NetworkPolicy()), test.Check, netpol, netpol.NetworkPolicy().ObjectMeta.Annotations)
 		}
 	}
 
 	for _, cjob := range allObjects.CronJobs() {
 		o := newObject(cjob.GetTypeMeta(), cjob.GetObjectMeta())
 		for _, test := range allChecks.CronJobs() {
-			o.Add(test.Fn(cjob), test.Check, cjob)
+			o.Add(test.Fn(cjob), test.Check, cjob, cjob.GetObjectMeta().Annotations)
 		}
 	}
 
 	for _, hpa := range allObjects.HorizontalPodAutoscalers() {
 		o := newObject(hpa.GetTypeMeta(), hpa.GetObjectMeta())
 		for _, test := range allChecks.HorizontalPodAutoscalers() {
-			o.Add(test.Fn(hpa), test.Check, hpa)
+			o.Add(test.Fn(hpa), test.Check, hpa, hpa.GetObjectMeta().Annotations)
 		}
 	}
 
 	for _, pdb := range allObjects.PodDisruptionBudgets() {
 		o := newObject(pdb.GetTypeMeta(), pdb.GetObjectMeta())
 		for _, test := range allChecks.PodDisruptionBudgets() {
-			o.Add(test.Fn(pdb), test.Check, pdb)
+			o.Add(test.Fn(pdb), test.Check, pdb, pdb.GetObjectMeta().Annotations)
 		}
 	}
 
diff --git a/scorecard/enabled.go b/scorecard/enabled.go
new file mode 100644
index 0000000..839b80b
--- /dev/null
+++ b/scorecard/enabled.go
@@ -0,0 +1,42 @@
+package scorecard
+
+import (
+	"strings"
+
+	ks "github.com/zegl/kube-score/domain"
+)
+
+func (so *ScoredObject) isEnabled(check ks.Check, annotations, childAnnotations map[string]string) bool {
+	isIn := func(csv string, key string) bool {
+		for _, v := range strings.Split(csv, ",") {
+			if v == key {
+				return true
+			}
+			if _, ok := impliedIgnoreAnnotations[v]; ok {
+				return true
+			}
+		}
+		return false
+	}
+
+	if childAnnotations != nil && so.useIgnoreChecksAnnotation && isIn(childAnnotations[ignoredChecksAnnotation], check.ID) {
+		return false
+	}
+	if childAnnotations != nil && so.useOptionalChecksAnnotation && isIn(childAnnotations[optionalChecksAnnotation], check.ID) {
+		return true
+	}
+	if so.useIgnoreChecksAnnotation && isIn(annotations[ignoredChecksAnnotation], check.ID) {
+		return false
+	}
+	if so.useOptionalChecksAnnotation && isIn(annotations[optionalChecksAnnotation], check.ID) {
+		return true
+	}
+
+	// Optional checks are disabled unless explicitly allowed above
+	if check.Optional {
+		return false
+	}
+
+	// Enabled by default
+	return true
+}
diff --git a/scorecard/scorecard.go b/scorecard/scorecard.go
index f44b5e1..a1766a1 100644
--- a/scorecard/scorecard.go
+++ b/scorecard/scorecard.go
@@ -2,7 +2,6 @@ package scorecard
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/zegl/kube-score/config"
 	ks "github.com/zegl/kube-score/domain"
@@ -28,11 +27,13 @@ func New() Scorecard {
 
 func (s Scorecard) NewObject(typeMeta metav1.TypeMeta, objectMeta metav1.ObjectMeta, cnf config.Configuration) *ScoredObject {
 	o := &ScoredObject{
-		TypeMeta:       typeMeta,
-		ObjectMeta:     objectMeta,
-		Checks:         make([]TestScore, 0),
-		ignoredChecks:  make(map[string]struct{}),
-		optionalChecks: make(map[string]struct{}),
+		TypeMeta:   typeMeta,
+		ObjectMeta: objectMeta,
+		Checks:     make([]TestScore, 0),
+
+		useIgnoreChecksAnnotation:   cnf.UseIgnoreChecksAnnotation,
+		useOptionalChecksAnnotation: cnf.UseOptionalChecksAnnotation,
+		enabledOptionalTests:        cnf.EnabledOptionalTests,
 	}
 
 	// If this object already exists, return the previous version
@@ -40,17 +41,6 @@ func (s Scorecard) NewObject(typeMeta metav1.TypeMeta, objectMeta metav1.ObjectM
 		return object
 	}
 
-	if cnf.UseIgnoreChecksAnnotation {
-		o.setIgnoredTests()
-	}
-
-	if cnf.UseOptionalChecksAnnotation {
-		o.setOptionalTests()
-	}
-	for id, ot := range cnf.EnabledOptionalTests {
-		o.optionalChecks[id] = ot
-	}
-
 	s[o.resourceRefKey()] = o
 	return o
 }
@@ -70,12 +60,13 @@ type ScoredObject struct {
 	FileLocation ks.FileLocation
 	Checks       []TestScore
 
-	ignoredChecks  map[string]struct{}
-	optionalChecks map[string]struct{}
+	useIgnoreChecksAnnotation   bool
+	useOptionalChecksAnnotation bool
+	enabledOptionalTests        map[string]struct{}
 }
 
-func (s ScoredObject) AnyBelowOrEqualToGrade(threshold Grade) bool {
-	for _, o := range s.Checks {
+func (so *ScoredObject) AnyBelowOrEqualToGrade(threshold Grade) bool {
+	for _, o := range so.Checks {
 		if !o.Skipped && o.Grade <= threshold {
 			return true
 		}
@@ -83,36 +74,11 @@ func (s ScoredObject) AnyBelowOrEqualToGrade(threshold Grade) bool {
 	return false
 }
 
-func (so *ScoredObject) setIgnoredTests() {
-	ignoredMap := make(map[string]struct{})
-	if ignoredCSV, ok := so.ObjectMeta.Annotations[ignoredChecksAnnotation]; ok {
-		for _, ignored := range strings.Split(ignoredCSV, ",") {
-			ignoredMap[strings.TrimSpace(ignored)] = struct{}{}
-			if _, ok := impliedIgnoreAnnotations[ignored]; ok {
-				for _, impliedIgnore := range impliedIgnoreAnnotations[ignored] {
-					ignoredMap[impliedIgnore] = struct{}{}
-				}
-			}
-		}
-	}
-	so.ignoredChecks = ignoredMap
-}
-
-func (so *ScoredObject) setOptionalTests() {
-	optionalMap := make(map[string]struct{})
-	if optionalCSV, ok := so.ObjectMeta.Annotations[optionalChecksAnnotation]; ok {
-		for _, optional := range strings.Split(optionalCSV, ",") {
-			optionalMap[strings.TrimSpace(optional)] = struct{}{}
-		}
-	}
-	so.optionalChecks = optionalMap
-}
-
-func (so ScoredObject) resourceRefKey() string {
+func (so *ScoredObject) resourceRefKey() string {
 	return so.TypeMeta.Kind + "/" + so.TypeMeta.APIVersion + "/" + so.ObjectMeta.Namespace + "/" + so.ObjectMeta.Name
 }
 
-func (so ScoredObject) HumanFriendlyRef() string {
+func (so *ScoredObject) HumanFriendlyRef() string {
 	s := so.ObjectMeta.Name
 	if so.ObjectMeta.Namespace != "" {
 		s += "/" + so.ObjectMeta.Namespace
@@ -121,16 +87,22 @@ func (so ScoredObject) HumanFriendlyRef() string {
 	return s
 }
 
-func (so *ScoredObject) Add(ts TestScore, check ks.Check, locationer ks.FileLocationer) {
+func (so *ScoredObject) Add(ts TestScore, check ks.Check, locationer ks.FileLocationer, annotations ...map[string]string) {
 	ts.Check = check
 	so.FileLocation = locationer.FileLocation()
 
-	if _, ok := so.optionalChecks[check.ID]; ts.Check.Optional && !ok {
-		return
+	var skip bool
+	if annotations != nil {
+		if len(annotations) == 1 && !so.isEnabled(check, annotations[0], nil) {
+			skip = true
+		}
+		if len(annotations) == 2 && !so.isEnabled(check, annotations[0], annotations[1]) {
+			skip = true
+		}
 	}
 
 	// This test is ignored (via annotations), don't save the score
-	if _, ok := so.ignoredChecks[check.ID]; ok {
+	if skip {
 		ts.Skipped = true
 		ts.Comments = []TestScoreComment{{Summary: fmt.Sprintf("Skipped because %s is ignored", check.ID)}}
 	}


Test patch:
diff --git a/score/apps_test.go b/score/apps_test.go
index 4560766..bf08606 100644
--- a/score/apps_test.go
+++ b/score/apps_test.go
@@ -3,6 +3,9 @@ package score
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/zegl/kube-score/config"
+	ks "github.com/zegl/kube-score/domain"
 	"github.com/zegl/kube-score/scorecard"
 )
 
@@ -102,3 +105,23 @@ func TestStatefulsetSelectorLabels(t *testing.T) {
 	t.Parallel()
 	testExpectedScore(t, "statefulset-different-labels.yaml", "StatefulSet Pod Selector labels match template metadata labels", scorecard.GradeCritical)
 }
+
+func TestStatefulsetTemplateIgnores(t *testing.T) {
+	t.Parallel()
+	skipped := wasSkipped(t, config.Configuration{
+		UseIgnoreChecksAnnotation:   true,
+		UseOptionalChecksAnnotation: true,
+		AllFiles:                    []ks.NamedReader{testFile("statefulset-nested-ignores.yaml")},
+	}, "Container Image Tag")
+	assert.True(t, skipped)
+}
+
+func TestStatefulsetTemplateIgnoresNotIgnoredWhenFlagDisabled(t *testing.T) {
+	t.Parallel()
+	skipped := wasSkipped(t, config.Configuration{
+		UseIgnoreChecksAnnotation:   false,
+		UseOptionalChecksAnnotation: true,
+		AllFiles:                    []ks.NamedReader{testFile("statefulset-nested-ignores.yaml")},
+	}, "Container Image Tag")
+	assert.False(t, skipped)
+}
diff --git a/score/score_test.go b/score/score_test.go
index 719fbb3..296c37d 100644
--- a/score/score_test.go
+++ b/score/score_test.go
@@ -39,6 +39,21 @@ func testExpectedScoreWithConfig(t *testing.T, config config.Configuration, test
 	return nil
 }
 
+func wasSkipped(t *testing.T, config config.Configuration, testcase string) bool {
+	sc, err := testScore(config)
+	assert.NoError(t, err)
+	for _, objectScore := range sc {
+		for _, s := range objectScore.Checks {
+			if s.Check.Name == testcase {
+				return s.Skipped
+			}
+		}
+	}
+
+	assert.Fail(t, "test was not run")
+	return false
+}
+
 func testScore(config config.Configuration) (scorecard.Scorecard, error) {
 	p, err := parser.New()
 	if err != nil {
diff --git a/score/security_test.go b/score/security_test.go
index 5e9574d..1c45663 100644
--- a/score/security_test.go
+++ b/score/security_test.go
@@ -19,6 +19,15 @@ func TestContainerSeccompMissing(t *testing.T) {
 		AllFiles:             []ks.NamedReader{testFile("pod-seccomp-no-annotation.yaml")},
 		EnabledOptionalTests: structMap,
 	}, "Container Seccomp Profile", scorecard.GradeWarning)
+
+}
+
+func TestContainerSeccompMissingNotRunByDefault(t *testing.T) {
+	t.Parallel()
+	skipped := wasSkipped(t, config.Configuration{
+		AllFiles: []ks.NamedReader{testFile("pod-seccomp-no-annotation.yaml")},
+	}, "Container Seccomp Profile")
+	assert.True(t, skipped)
 }
 
 func TestContainerSeccompAllGood(t *testing.T) {
diff --git a/score/testdata/statefulset-nested-ignores.yaml b/score/testdata/statefulset-nested-ignores.yaml
new file mode 100644
index 0000000..ff45af9
--- /dev/null
+++ b/score/testdata/statefulset-nested-ignores.yaml
@@ -0,0 +1,109 @@
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: trivy
+  namespace: trivy-staging
+spec:
+  podManagementPolicy: Parallel
+  replicas: 3
+  revisionHistoryLimit: 10
+  selector:
+    matchLabels:
+      app.kubernetes.io/instance: trivy
+      app.kubernetes.io/name: trivy
+  serviceName: trivy
+  template:
+    metadata:
+      annotations:
+        kube-score/ignore: container-image-tag,pod-probes
+      labels:
+        app.kubernetes.io/instance: trivy
+        app.kubernetes.io/name: trivy
+    spec:
+      automountServiceAccountToken: false
+      containers:
+        - args:
+            - server
+          envFrom:
+            - configMapRef:
+                name: trivy
+            - secretRef:
+                name: trivy
+          image: aquasec/trivy:latest
+          imagePullPolicy: Always
+          livenessProbe:
+            failureThreshold: 10
+            httpGet:
+              path: /healthz
+              port: trivy-http
+              scheme: HTTP
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            successThreshold: 1
+            timeoutSeconds: 1
+          name: main
+          ports:
+            - containerPort: 4954
+              name: trivy-http
+              protocol: TCP
+          readinessProbe:
+            failureThreshold: 3
+            httpGet:
+              path: /healthz
+              port: trivy-http
+              scheme: HTTP
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            successThreshold: 1
+            timeoutSeconds: 1
+          resources:
+            limits:
+              cpu: "1"
+              ephemeral-storage: 128Mi
+              memory: 1Gi
+            requests:
+              cpu: 200m
+              memory: 512Mi
+          securityContext:
+            privileged: false
+            readOnlyRootFilesystem: true
+            runAsGroup: 65534
+          terminationMessagePath: /dev/termination-log
+          terminationMessagePolicy: File
+          volumeMounts:
+            - mountPath: /tmp
+              name: tmp-data
+            - mountPath: /home/scanner/.cache
+              name: data
+      dnsPolicy: ClusterFirst
+      restartPolicy: Always
+      schedulerName: default-scheduler
+      securityContext:
+        fsGroup: 65534
+        runAsNonRoot: true
+        runAsUser: 65534
+      serviceAccount: trivy
+      serviceAccountName: trivy
+      terminationGracePeriodSeconds: 30
+      volumes:
+        - emptyDir: {}
+          name: tmp-data
+  updateStrategy:
+    rollingUpdate:
+      partition: 0
+    type: RollingUpdate
+  volumeClaimTemplates:
+    - apiVersion: v1
+      kind: PersistentVolumeClaim
+      metadata:
+        creationTimestamp: null
+        name: data
+      spec:
+        accessModes:
+          - ReadWriteOnce
+        resources:
+          requests:
+            storage: 5Gi
+        volumeMode: Filesystem
+      status:
+        phase: Pending
\ No newline at end of file