add test for required fields on builtin checks (#965)

* fix checks category

* add changelog

checks/automountServiceAccountToken.yaml

@@ -5,7 +5,6 @@ target: PodSpec
 schema:
   '$schema': http://json-schema.org/draft-07/schema
   type: object
-  required: ["serviceAccountName"]
   properties:
     serviceAccountName:
       type: string
@@ -15,12 +14,11 @@ schema:
         const: true
 additionalSchemaStrings:
   ServiceAccount: |
+    {{ if not (eq .Polaris.PodSpec.automountServiceAccountToken false) }}
     type: object
     required:
       - metadata
-      {{ if not (eq .Polaris.PodSpec.automountServiceAccountToken false) }}
       - automountServiceAccountToken
-      {{ end }}
     properties:
       metadata:
         type: object
@@ -34,3 +32,4 @@ additionalSchemaStrings:
         type: boolean
         const: false
       {{ end }}
+    {{ end }}

checks/clusterrolebindingClusterAdmin.yaml

@@ -39,6 +39,7 @@ additionalSchemaStrings:
   rbac.authorization.k8s.io/ClusterRole: |
     type: object
     # Do not alert on default ClusterRoleBindings.
+    {{ if (ne .roleRef.name "view") }}
     {{ if and (ne .metadata.name "cluster-admin") (not (hasPrefix .metadata.name "system:")) (ne .metadata.name "gce:podsecuritypolicy:calico-sa") }}
     required: ["metadata", "rules"]
     allOf:
@@ -86,3 +87,4 @@ additionalSchemaStrings:
                             - "patch"
                             - "delete"
     {{ end }}
+    {{ end }}

checks/clusterrolebindingPodExecAttach.yaml

@@ -37,6 +37,7 @@ additionalSchemaStrings:
   rbac.authorization.k8s.io/ClusterRole: |
     type: object
     # Do not alert on default ClusterRoleBindings.
+    {{ if (ne .roleRef.name "view") }}
     {{ if and (ne .metadata.name "cluster-admin") (not (hasPrefix .metadata.name "system:")) (ne .metadata.name "gce:podsecuritypolicy:calico-sa") }}
     required: ["metadata", "rules"]
     allOf:
@@ -80,3 +81,4 @@ additionalSchemaStrings:
                         - const: 'get'
                         - const: 'create'
     {{ end }}
+    {{ end }}

checks/metadataAndNameMismatched.yaml

@@ -1,5 +1,6 @@
 successMessage: Label app.kubernetes.io/name matches metadata.name
 failureMessage: Label app.kubernetes.io/name must match metadata.name
+category: Reliability
 target: Controller
 schema:
   '$schema': http://json-schema.org/draft-07/schema

checks/priorityClassNotSet.yaml

@@ -1,6 +1,6 @@
 successMessage: Priority class has been set
 failureMessage: Priority class should be set
-category: Security
+category: Reliability
 target: PodSpec
 schema:
   '$schema': http://json-schema.org/draft-07/schema

checks/sensitiveContainerEnvVar.yaml

@@ -10,7 +10,9 @@ schemaString: |
       type: array
       items:
         type: object
-        oneOf:
+        anyOf:
+          - not:
+              required: ["value"]
           - required: ["name", "value"]
             properties:
               name:

cmd/polaris/audit.go

@@ -173,7 +173,7 @@ var auditCmd = &cobra.Command{
 				os.Exit(1)
 			}
 			logrus.Println("Success! You can see your results at:")
-			logrus.Printf("%s/orgs/%s/clusters/%s\n", insightsHost, auth.Organization, clusterName)
+			logrus.Printf("%s/orgs/%s/clusters/%s/action-items\n", insightsHost, auth.Organization, clusterName)
 		} else {
 			outputAudit(auditData, auditOutputFile, auditOutputURL, auditOutputFormat, useColor, onlyShowFailedTests)
 		}

cmd/polaris/webhook.go

@@ -53,6 +53,7 @@ var webhookCmd = &cobra.Command{
 			CertDir: certDir,
 			Port:    webhookPort,
 			WebhookServer: webhook.NewServer(webhook.Options{
+				CertDir: certDir,
 				CertName: "tls.crt",
 				KeyName:  "tls.key",
 			}),

docs/admission-controller.md

@@ -53,7 +53,7 @@ This means Polaris will remediate the issue it finds, rather than rejecting
 the deployment.
 
 To enable the mutating webhook, add `--set webhook.mutate=true` to your
-Helm instlallation command.
+Helm installation command.
 
 The following default checks currently have mutation support enabled:
 * `hostPIDSet`

docs/changelog.md

@@ -6,6 +6,10 @@ meta:
 
 ---
 
+## 8.1.1
+* Add category for `metadataAndNameMismatched`.
+* Fix category for `priorityClassNotSet`.
+
 ## 8.1.0
 * Add `insights-host` global flag to configure Fairwinds Insights host (defaults to `https://insights.fairwinds.com`).
 * Add new `auth` sub-commands be able to authenticate on Polaris using Fairwinds Insights credentials

docs/checks/reliability.md

@@ -16,8 +16,8 @@ key | default | description
 `pullPolicyNotAlways` | `warning` | Fails when an image pull policy is not `always`.
 `priorityClassNotSet` | `warning` | Fails when a priorityClassName is not set for a pod.
 `deploymentMissingReplicas` | `warning` | Fails when there is only one replica for a deployment.
-`missingPodDisruptionBudget` | `warning`
-`metadataAndNameMismatched` | `warning`
+`missingPodDisruptionBudget` | `warning` | Fails when PDB is missing.
+`metadataAndNameMismatched` | `warning` | Fails when label `app.kubernetes.io/name` and `metadata.name` mismatch
 `topologySpreadConstraint` | `warning` | Fails when there is no topology spread constraint on the pod
 
 ## Background
@@ -70,4 +70,4 @@ spec:
 - [Kubernetes Docs: Configure Liveness and Readiness Probes](https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-probes/)
 - [Utilizing Kubernetes Liveness and Readiness Probes to Automatically Recover From Failure](https://medium.com/spire-labs/utilizing-kubernetes-liveness-and-readiness-probes-to-automatically-recover-from-failure-2fe0314f2b2e)
 - [Kubernetes Liveness and Readiness Probes: How to Avoid Shooting Yourself in the Foot](https://blog.colinbreck.com/kubernetes-liveness-and-readiness-probes-how-to-avoid-shooting-yourself-in-the-foot/)
-- [Topology Spread Cosntraints](https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/)
+- [Topology Spread Constraints](https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/)

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/gorilla/mux v1.8.0
 	github.com/pkg/errors v0.9.1
 	github.com/qri-io/jsonschema v0.1.2
-	github.com/sirupsen/logrus v1.9.2
+	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.7.0
 	github.com/stretchr/testify v1.8.4
 	github.com/thoas/go-funk v0.9.3

go.sum

@@ -334,8 +334,8 @@ github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
 github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
-github.com/sirupsen/logrus v1.9.2 h1:oxx1eChJGI6Uks2ZC4W1zpLlVgqB8ner4EuQwV4Ik1Y=
-github.com/sirupsen/logrus v1.9.2/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
+github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
+github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
 github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
 github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA=
 github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=

pkg/auth/login.go

@@ -85,7 +85,7 @@ func HandleLogin(insightsHost string) error {
 
 	var user, token, organization string
 	if answer == loginUsingBrowser {
-		listener, err := net.Listen("tcp", ":0")
+		listener, err := net.Listen("tcp", "localhost:0")
 		if err != nil {
 			panic(err)
 		}

pkg/config/checks.go

@@ -22,7 +22,6 @@ import (
 var (
 	// BuiltInChecks contains the checks that come pre-installed w/ Polaris
 	BuiltInChecks = map[string]SchemaCheck{}
-	schemaBox     = (*packr.Box)(nil)
 	// We explicitly set the order to avoid thrash in the
 	// tests as we migrate toward JSON schema
 	checkOrder = []string{
@@ -72,7 +71,7 @@ var (
 )
 
 func init() {
-	schemaBox = packr.New("Schemas", "../../checks")
+	schemaBox := packr.New("Schemas", "../../checks")
 	for _, checkID := range checkOrder {
 		contents, err := schemaBox.Find(checkID + ".yaml")
 		if err != nil {

pkg/config/checks_test.go

@@ -0,0 +1,16 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRequiredFieldsOnBuiltInChecks(t *testing.T) {
+	for _, v := range BuiltInChecks {
+		assert.NotEmpty(t, v.SuccessMessage)
+		assert.NotEmpty(t, v.FailureMessage)
+		assert.NotEmpty(t, v.Category)
+		assert.NotEmpty(t, v.Target)
+	}
+}

pkg/config/schema.go

@@ -236,11 +236,15 @@ func (check SchemaCheck) TemplateForResource(res interface{}) (*SchemaCheck, err
 		if err != nil {
 			return nil, err
 		}
+		templated := w.String()
+		if strings.TrimSpace(templated) == "" {
+			continue
+		}
 
 		if kind == "" {
-			newCheck.SchemaString = w.String()
+			newCheck.SchemaString = templated
 		} else {
-			newCheck.AdditionalSchemaStrings[kind] = w.String()
+			newCheck.AdditionalSchemaStrings[kind] = templated
 		}
 	}
 

test/checks/automountServiceAccountToken/success.no_serviceaccount.yaml

@@ -0,0 +1,12 @@
+# This succeeds because automounting is disabled on the pod, and there is no specified service account
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-pod
+spec:
+  automountServiceAccountToken: false
+  containers:
+  - name: nginx-foo
+    image: nginx-foo
+    ports:
+    - containerPort: 80

test/checks/insecureCapabilities/failure.add.yaml

@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx
+  labels:
+    env: test
+spec:
+  containers:
+  - name: nginx
+    image: nginx
+    securityContext:
+      capabilities:
+        add:
+          - ALL

test/checks/insecureCapabilities/mutated.add.yaml

@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx
+  labels:
+    env: test
+spec:
+  containers:
+    - name: nginx
+      image: nginx
+      securityContext:
+        capabilities:
+          drop:
+            - ALL

test/mutation_test.go

@@ -19,7 +19,7 @@ checks:
   hostNetworkSet: danger
   hostPortSet: warning
   deploymentMissingReplicas: warning
-  priorityClassNotSet: ignore
+  priorityClassNotSet: warning
   runAsRootAllowed: danger
   cpuRequestsMissing: warning
   cpuLimitsMissing: warning
@@ -27,6 +27,7 @@ checks:
   memoryLimitsMissing: warning
   readinessProbeMissing: warning
   livenessProbeMissing: warning
+  insecureCapabilities: warning
 `
 
 func TestMutations(t *testing.T) {