Add remediation information to all checks (#14)

viswajithiii · web-flow · commit 26a483c02d59 · 2020-10-23T14:21:16.000-07:00
* For each check, print information about how it can be remediated.
* Add a unit test to enforce that this is added for all built-in checks.
diff --git a/docs/generated/checks.md b/docs/generated/checks.md
@@ -1,20 +1,20 @@
 The following table enumerates built-in checks:
 
-| Name | Enabled by default | Description | Template | Parameters |
-| ---- | ------------------ | ----------- | -------- | ---------- |
- | dangling-service | Yes | Alert on services that don't have any matching deployments | dangling-service | `{}` |
- | default-service-account | No | Alert on pods that use the default service account | service-account | `{"serviceAccount":"^(|default)$"}` |
- | deprecated-service-account-field | Yes | Alert on deployments that use the deprecated serviceAccount field | deprecated-service-account-field | `{}` |
- | env-var-secret | Yes | Alert on objects using a secret in an environment variable | env-var | `{"name":".*secret.*"}` |
- | no-extensions-v1beta | Yes | Alert on objects using deprecated API versions under extensions v1beta | disallowed-api-obj | `{"group":"extensions","version":"v1beta.+"}` |
- | no-liveness-probe | No | Alert on containers which don't specify a liveness probe | liveness-probe | `{}` |
- | no-read-only-root-fs | Yes | Alert on containers not running with a read-only root filesystem | read-only-root-fs | `{}` |
- | no-readiness-probe | No | Alert on containers which don't specify a readiness probe | readiness-probe | `{}` |
- | non-existent-service-account | Yes | Alert on pods referencing a service account that isn't found | non-existent-service-account | `{}` |
- | privileged-container | Yes | Alert on deployments with containers running in privileged mode | privileged | `{}` |
- | required-annotation-email | No | Alert on objects without an 'email' annotation with a valid email | required-annotation | `{"key":"email","value":"[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+"}` |
- | required-label-owner | No | Alert on objects without the 'owner' label | required-label | `{"key":"owner"}` |
- | run-as-non-root | Yes | Alert on containers not set to runAsNonRoot | run-as-non-root | `{}` |
- | unset-cpu-requirements | Yes | Alert on containers without CPU requests and limits set | cpu-requirements | `{"lowerBoundMillis":0,"requirementsType":"any","upperBoundMillis":0}` |
- | unset-memory-requirements | Yes | Alert on containers without memory requests and limits set | memory-requirements | `{"lowerBoundMB":0,"requirementsType":"any","upperBoundMB":0}` |
- | writable-host-mount | No | Alert on containers that mount a host path as writable | writable-host-mount | `{}` |
+| Name | Enabled by default | Description | Remediation | Template | Parameters |
+| ---- | ------------------ | ----------- | ----------- | -------- | ---------- |
+ | dangling-service | Yes | Alert on services that don't have any matching deployments | Make sure your service's selector correctly matches the labels on one of your deployments. | dangling-service | `{}` |
+ | default-service-account | No | Alert on pods that use the default service account | Create a dedicated service account for your pod. See https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ for more details. | service-account | `{"serviceAccount":"^(|default)$"}` |
+ | deprecated-service-account-field | Yes | Alert on deployments that use the deprecated serviceAccount field | Use the serviceAccoutName field instead of the serviceAccount field. | deprecated-service-account-field | `{}` |
+ | env-var-secret | Yes | Alert on objects using a secret in an environment variable | Don't use raw secrets in an environment variable. Instead, either mount the secret as a file or use a secretKeyRef. See https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets for more details. | env-var | `{"name":"(?i).*secret.*"}` |
+ | no-extensions-v1beta | Yes | Alert on objects using deprecated API versions under extensions v1beta | Migrate to using the apps/v1 API versions for these objects. See https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/ for more details. | disallowed-api-obj | `{"group":"extensions","version":"v1beta.+"}` |
+ | no-liveness-probe | No | Alert on containers which don't specify a liveness probe | Specify a liveness probe in your container. See https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ for more details. | liveness-probe | `{}` |
+ | no-read-only-root-fs | Yes | Alert on containers not running with a read-only root filesystem | Set readOnlyRootFilesystem to true in your container's securityContext. | read-only-root-fs | `{}` |
+ | no-readiness-probe | No | Alert on containers which don't specify a readiness probe | Specify a readiness probe in your container. See https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ for more details. | readiness-probe | `{}` |
+ | non-existent-service-account | Yes | Alert on pods referencing a service account that isn't found | Make sure to create the service account, or to refer to an existing service account. | non-existent-service-account | `{}` |
+ | privileged-container | Yes | Alert on deployments with containers running in privileged mode | Don't run your container as privileged unless required. | privileged | `{}` |
+ | required-annotation-email | No | Alert on objects without an 'email' annotation with a valid email | Add an email annotation to your object with the contact information of the object's owner. | required-annotation | `{"key":"email","value":"[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+"}` |
+ | required-label-owner | No | Alert on objects without the 'owner' label | Add an email annotation to your object with information about the object's owner. | required-label | `{"key":"owner"}` |
+ | run-as-non-root | Yes | Alert on containers not set to runAsNonRoot | Set runAsUser to a non-zero number, and runAsNonRoot to true, in your pod or container securityContext. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ for more details. | run-as-non-root | `{}` |
+ | unset-cpu-requirements | Yes | Alert on containers without CPU requests and limits set | Set your container's CPU requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details. | cpu-requirements | `{"lowerBoundMillis":0,"requirementsType":"any","upperBoundMillis":0}` |
+ | unset-memory-requirements | Yes | Alert on containers without memory requests and limits set | Set your container's memory requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details. | memory-requirements | `{"lowerBoundMB":0,"requirementsType":"any","upperBoundMB":0}` |
+ | writable-host-mount | No | Alert on containers that mount a host path as writable | If you need to access files on the host, mount them as readOnly. | writable-host-mount | `{}` |
diff --git a/go.mod b/go.mod
@@ -4,6 +4,7 @@ go 1.14
 
 require (
 	github.com/Masterminds/sprig/v3 v3.1.0
+	github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d
 	github.com/fatih/color v1.9.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/gobuffalo/packr v1.30.1
diff --git a/go.sum b/go.sum
@@ -63,6 +63,8 @@ github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWX
 github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
 github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg=
 github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g=
+github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8=
+github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo=
 github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c=
 github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
diff --git a/internal/builtinchecks/built_in_checks_test.go b/internal/builtinchecks/built_in_checks_test.go
@@ -0,0 +1,20 @@
+package builtinchecks
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuiltInChecksWellFormed(t *testing.T) {
+	checks, err := List()
+	require.NoError(t, err)
+	for _, check := range checks {
+		t.Run(check.Name, func(t *testing.T) {
+			assert.NotEmpty(t, check.Remediation, "Please add remediation")
+			assert.True(t, strings.HasSuffix(check.Remediation, "."), "Please end your remediation texts with a period (got %q)", check.Remediation)
+		})
+	}
+}
diff --git a/internal/builtinchecks/yamls/dangling-service.yaml b/internal/builtinchecks/yamls/dangling-service.yaml
@@ -1,5 +1,6 @@
 name: "dangling-service"
 description: "Alert on services that don't have any matching deployments"
+remediation: "Make sure your service's selector correctly matches the labels on one of your deployments."
 scope:
   objectKinds:
     - Service
diff --git a/internal/builtinchecks/yamls/default-service-account.yaml b/internal/builtinchecks/yamls/default-service-account.yaml
@@ -1,6 +1,11 @@
-name: "deprecated-service-account-field"
-description: "Alert on deployments that use the deprecated serviceAccount field"
+name: "default-service-account"
+description: "Alert on pods that use the default service account"
+remediation: >-
+  Create a dedicated service account for your pod.
+  See https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ for more details.
 scope:
   objectKinds:
     - DeploymentLike
-template: "deprecated-service-account-field"
+template: "service-account"
+params:
+  serviceAccount: "^(|default)$"
diff --git a/internal/builtinchecks/yamls/deprecated-service-account.yaml b/internal/builtinchecks/yamls/deprecated-service-account.yaml
@@ -1,8 +1,7 @@
-name: "default-service-account"
-description: "Alert on pods that use the default service account"
+name: "deprecated-service-account-field"
+description: "Alert on deployments that use the deprecated serviceAccount field"
+remediation: "Use the serviceAccoutName field instead of the serviceAccount field."
 scope:
   objectKinds:
     - DeploymentLike
-template: "service-account"
-params:
-  serviceAccount: "^(|default)$"
+template: "deprecated-service-account-field"
diff --git a/internal/builtinchecks/yamls/env-var-secret.yaml b/internal/builtinchecks/yamls/env-var-secret.yaml
@@ -1,8 +1,11 @@
 name: "env-var-secret"
 description: "Alert on objects using a secret in an environment variable"
+remediation: >-
+  Don't use raw secrets in an environment variable. Instead, either mount the secret as a file or use a secretKeyRef.
+  See https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets for more details.
 scope:
   objectKinds:
     - DeploymentLike
 template: "env-var"
 params:
-  name: ".*secret.*"
+  name: "(?i).*secret.*"
diff --git a/internal/builtinchecks/yamls/liveness-probe.yaml b/internal/builtinchecks/yamls/liveness-probe.yaml
diff --git a/internal/builtinchecks/yamls/no-extensions-v1beta.yaml b/internal/builtinchecks/yamls/no-extensions-v1beta.yaml
@@ -1,5 +1,8 @@
 name: "no-extensions-v1beta"
 description: "Alert on objects using deprecated API versions under extensions v1beta"
+remediation: >-
+  Migrate to using the apps/v1 API versions for these objects.
+  See https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/ for more details.
 scope:
   objectKinds:
     - Any
diff --git a/internal/builtinchecks/yamls/no-liveness-probe.yaml b/internal/builtinchecks/yamls/no-liveness-probe.yaml
@@ -0,0 +1,9 @@
+name: "no-liveness-probe"
+description: "Alert on containers which don't specify a liveness probe"
+remediation: >-
+  Specify a liveness probe in your container.
+  See https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ for more details.
+scope:
+  objectKinds:
+    - DeploymentLike
+template: "liveness-probe"
diff --git a/internal/builtinchecks/yamls/no-readiness-probe.yaml b/internal/builtinchecks/yamls/no-readiness-probe.yaml
@@ -0,0 +1,9 @@
+name: "no-readiness-probe"
+description: "Alert on containers which don't specify a readiness probe"
+remediation: >-
+  Specify a readiness probe in your container.
+  See https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ for more details.
+scope:
+  objectKinds:
+    - DeploymentLike
+template: "readiness-probe"
diff --git a/internal/builtinchecks/yamls/non-existent-service-account.yaml b/internal/builtinchecks/yamls/non-existent-service-account.yaml
@@ -1,5 +1,6 @@
 name: "non-existent-service-account"
 description: "Alert on pods referencing a service account that isn't found"
+remediation: "Make sure to create the service account, or to refer to an existing service account."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/privileged.yaml b/internal/builtinchecks/yamls/privileged.yaml
@@ -1,5 +1,6 @@
 name: "privileged-container"
 description: "Alert on deployments with containers running in privileged mode"
+remediation: "Don't run your container as privileged unless required."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/read-only-root-fs.yaml b/internal/builtinchecks/yamls/read-only-root-fs.yaml
@@ -1,5 +1,6 @@
 name: "no-read-only-root-fs"
 description: "Alert on containers not running with a read-only root filesystem"
+remediation: "Set readOnlyRootFilesystem to true in your container's securityContext."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/readiness-probe.yaml b/internal/builtinchecks/yamls/readiness-probe.yaml
diff --git a/internal/builtinchecks/yamls/required-annotation-email.yaml b/internal/builtinchecks/yamls/required-annotation-email.yaml
@@ -1,5 +1,6 @@
 name: "required-annotation-email"
 description: "Alert on objects without an 'email' annotation with a valid email"
+remediation: "Add an email annotation to your object with the contact information of the object's owner."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/required-label-owner.yaml b/internal/builtinchecks/yamls/required-label-owner.yaml
@@ -1,5 +1,6 @@
 name: "required-label-owner"
 description: "Alert on objects without the 'owner' label"
+remediation: "Add an email annotation to your object with information about the object's owner."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/run-as-non-root.yaml b/internal/builtinchecks/yamls/run-as-non-root.yaml
@@ -1,5 +1,8 @@
 name: "run-as-non-root"
 description: "Alert on containers not set to runAsNonRoot"
+remediation: >-
+  Set runAsUser to a non-zero number, and runAsNonRoot to true, in your pod or container securityContext.
+  See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ for more details.
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/unset-cpu-requirements.yaml b/internal/builtinchecks/yamls/unset-cpu-requirements.yaml
@@ -3,6 +3,9 @@ description: "Alert on containers without CPU requests and limits set"
 scope:
   objectKinds:
     - DeploymentLike
+remediation: >-
+  Set your container's CPU requests and limits depending on its requirements.
+  See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details.
 template: "cpu-requirements"
 params:
   requirementsType: "any"
diff --git a/internal/builtinchecks/yamls/unset-memory-requirements.yaml b/internal/builtinchecks/yamls/unset-memory-requirements.yaml
@@ -1,5 +1,8 @@
 name: "unset-memory-requirements"
 description: "Alert on containers without memory requests and limits set"
+remediation: >-
+  Set your container's memory requests and limits depending on its requirements.
+  See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details.
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/builtinchecks/yamls/writable-host-mount.yaml b/internal/builtinchecks/yamls/writable-host-mount.yaml
@@ -1,5 +1,6 @@
 name: "writable-host-mount"
 description: "Alert on containers that mount a host path as writable"
+remediation: "If you need to access files on the host, mount them as readOnly."
 scope:
   objectKinds:
     - DeploymentLike
diff --git a/internal/check/check.go b/internal/check/check.go
@@ -4,6 +4,7 @@ package check
 type Check struct {
 	Name        string                 `json:"name"`
 	Description string                 `json:"description"`
+	Remediation string                 `json:"remediation"`
 	Scope       *ObjectKindsDesc       `json:"scope"`
 	Template    string                 `json:"template"`
 	Params      map[string]interface{} `json:"params,omitempty"`
diff --git a/internal/checkregistry/check_registry.go b/internal/checkregistry/check_registry.go
@@ -22,10 +22,10 @@ func (cr checkRegistry) Register(checks ...*check.Check) error {
 		if err != nil {
 			return errors.Wrapf(err, "invalid check %s", c.Name)
 		}
-		if _, ok := cr[instantiated.Name]; ok {
-			return errors.Errorf("duplicate check name: %s", instantiated.Name)
+		if _, ok := cr[instantiated.Spec.Name]; ok {
+			return errors.Errorf("duplicate check name: %s", instantiated.Spec.Name)
 		}
-		cr[instantiated.Name] = instantiated
+		cr[instantiated.Spec.Name] = instantiated
 	}
 	return nil
 }
diff --git a/internal/command/checks/command.go b/internal/command/checks/command.go
@@ -26,8 +26,8 @@ var (
 
 func renderPlain(checks []check.Check, out io.Writer) error { //nolint:unparam // The function signature is required to match formatToRenderFuncs
 	for i, chk := range checks {
-		fmt.Fprintf(out, "Name: %s\nDescription: %s\nTemplate: %s\nParameters: %v\nEnabled by default: %v\n",
-			chk.Name, chk.Description, chk.Template, chk.Params, defaultchecks.List.Contains(chk.Name))
+		fmt.Fprintf(out, "Name: %s\nDescription: %s\nRemediation: %s\nTemplate: %s\nParameters: %v\nEnabled by default: %v\n",
+			chk.Name, chk.Description, chk.Remediation, chk.Template, chk.Params, defaultchecks.List.Contains(chk.Name))
 		if i != len(checks)-1 {
 			fmt.Fprintf(out, "\n%s\n\n", dashes)
 		}
@@ -38,9 +38,9 @@ func renderPlain(checks []check.Check, out io.Writer) error { //nolint:unparam /
 const (
 	markDownTemplateStr = `The following table enumerates built-in checks:
 
-| Name | Enabled by default | Description | Template | Parameters |
-| ---- | ------------------ | ----------- | -------- | ---------- |
-{{ range . }} | {{ .Check.Name}} | {{ if .Default }}Yes{{ else }}No{{ end }} | {{.Check.Description}} | {{.Check.Template}} | {{ backtick }}{{ mustToJson (default (dict) .Check.Params ) }}{{ backtick }} |
+| Name | Enabled by default | Description | Remediation | Template | Parameters |
+| ---- | ------------------ | ----------- | ----------- | -------- | ---------- |
+{{ range . }} | {{ .Check.Name}} | {{ if .Default }}Yes{{ else }}No{{ end }} | {{.Check.Description}} | {{.Check.Remediation}} | {{.Check.Template}} | {{ backtick }}{{ mustToJson (default (dict) .Check.Params ) }}{{ backtick }} |
 {{ end -}}
 `
 )
diff --git a/internal/diagnostic/diagnostic.go b/internal/diagnostic/diagnostic.go
@@ -1,9 +1,11 @@
 package diagnostic
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 
+	"github.com/acarl005/stripansi"
 	"github.com/fatih/color"
 	"golang.stackrox.io/kube-linter/internal/k8sutil"
 	"golang.stackrox.io/kube-linter/internal/lintcontext"
@@ -20,9 +22,10 @@ type Diagnostic struct {
 // WithContext puts a diagnostic in the context of which check emitted it,
 // and which object it applied to.
 type WithContext struct {
-	Diagnostic Diagnostic
-	Check      string
-	Object     lintcontext.Object
+	Diagnostic  Diagnostic
+	Check       string
+	Remediation string
+	Object      lintcontext.Object
 }
 
 var (
@@ -36,12 +39,16 @@ func formatObj(obj k8sutil.Object) string {
 // FormatToTerminal writes the result to the given writer, which is expected to support
 // terminal-based formatting.
 func (w *WithContext) FormatToTerminal(out io.Writer) {
-	fmt.Fprintf(out, "%s %s", bold.Sprintf("%s:", w.Object.Metadata.FilePath), color.RedString(w.Diagnostic.Message))
-	fmt.Fprintf(out, " (check: %s, object: %s)\n\n", color.YellowString(w.Check),
-		color.YellowString(formatObj(w.Object.K8sObject)))
+	fmt.Fprintf(out, "%s %s",
+		bold.Sprintf("%s: (object: %s)", w.Object.Metadata.FilePath, formatObj(w.Object.K8sObject)), color.RedString(w.Diagnostic.Message),
+	)
+	fmt.Fprintf(out, " (check: %s, remediation: %s)\n\n", color.YellowString(w.Check),
+		color.YellowString(w.Remediation))
 }
 
 // FormatPlain prints out the result to the given writer, without colors/special formatting.
 func (w *WithContext) FormatPlain(out io.Writer) {
-	fmt.Fprintf(out, "%s %s (check: %s, object: %s)\n\n", w.Object.Metadata.FilePath, w.Diagnostic.Message, w.Check, formatObj(w.Object.K8sObject))
+	var buf bytes.Buffer
+	w.FormatToTerminal(&buf)
+	out.Write([]byte(stripansi.Strip(buf.String())))
 }
diff --git a/internal/instantiatedcheck/instantiated_check.go b/internal/instantiatedcheck/instantiated_check.go
@@ -13,9 +13,10 @@ import (
 // An InstantiatedCheck is the runtime instantiation of a check, which fuses the metadata in a check
 // spec with the runtime information from a template.
 type InstantiatedCheck struct {
-	Name    string
 	Func    check.Func
 	Matcher objectkinds.Matcher
+
+	Spec check.Check
 }
 
 var (
@@ -47,7 +48,7 @@ func ValidateAndInstantiate(c *check.Check) (*InstantiatedCheck, error) {
 		return nil, err
 	}
 
-	i := &InstantiatedCheck{Name: c.Name}
+	i := &InstantiatedCheck{Spec: *c}
 	var objectKinds check.ObjectKindsDesc
 	if c.Scope != nil {
 		objectKinds = *c.Scope
diff --git a/internal/run/run.go b/internal/run/run.go
diff --git a/internal/templates/nonexistentserviceaccount/template.go b/internal/templates/nonexistentserviceaccount/template.go
