From 7120802731f7217ac1c41b863687a779124cf9ae Mon Sep 17 00:00:00 2001 From: Gustav Westling Date: Thu, 15 Sep 2022 22:07:19 +0200 Subject: [PATCH] score: allow ignore/allow annotations on child template resources --- parser/parse.go | 6 +- score/apps_test.go | 23 ++++ score/score.go | 25 ++-- score/score_test.go | 15 +++ score/security_test.go | 9 ++ .../testdata/statefulset-nested-ignores.yaml | 109 ++++++++++++++++++ scorecard/enabled.go | 42 +++++++ scorecard/scorecard.go | 76 ++++-------- 8 files changed, 241 insertions(+), 64 deletions(-) create mode 100644 score/testdata/statefulset-nested-ignores.yaml create mode 100644 scorecard/enabled.go diff --git a/parser/parse.go b/parser/parse.go index ddf09089..37918c64 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/apps_test.go b/score/apps_test.go index 45607660..bf086069 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.go b/score/score.go index 32f918b9..a9879698 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/score/score_test.go b/score/score_test.go index 719fbb3e..296c37de 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 5e9574df..1c45663a 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 00000000..ff45af96 --- /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 diff --git a/scorecard/enabled.go b/scorecard/enabled.go new file mode 100644 index 00000000..839b80bb --- /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 f44b5e12..a1766a19 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)}} }