Merge pull request #65 from digitalocean/awg/fill-check-names

Add check names to diagnostics from the check runner

checks/basic/bare_pods.go

@@ -50,7 +50,6 @@ func (b *barePodCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) {
 		pod := pod
 		if len(pod.ObjectMeta.OwnerReferences) == 0 {
 			d := checks.Diagnostic{
-				Check:    b.Name(),
 				Severity: checks.Warning,
 				Message:  "Avoid using bare pods in clusters",
 				Kind:     checks.Pod,

checks/basic/bare_pods_test.go

@@ -67,7 +67,6 @@ func TestBarePodError(t *testing.T) {
 			expected: []checks.Diagnostic{
 				{
 					Severity: "warning",
-					Check:    "bare-pods",
 					Kind:     checks.Pod,
 					Message:  "Avoid using bare pods in clusters",
 					Object:   GetObjectMeta(),
@@ -81,15 +80,13 @@ func TestBarePodError(t *testing.T) {
 			expected: []checks.Diagnostic{
 				{
 					Severity: "warning",
-					Check:    "bare-pods",
 					Kind:     checks.Pod,
 					Message:  "Avoid using bare pods in clusters",
 					Object:   &metav1.ObjectMeta{Name: "pod_1", Namespace: "k8s"},
 					Owners:   nil,
 				},
 				{
 					Severity: "warning",
-					Check:    "bare-pods",
 					Kind:     checks.Pod,
 					Message:  "Avoid using bare pods in clusters",
 					Object:   &metav1.ObjectMeta{Name: "pod_2", Namespace: "k8s"},

checks/basic/fully_qualified_image.go

@@ -71,7 +71,6 @@ func (fq *fullyQualifiedImageCheck) checkImage(containers []corev1.Container, po
 		value, err := reference.ParseAnyReference(container.Image)
 		if err != nil {
 			d := checks.Diagnostic{
-				Check:    fq.Name(),
 				Severity: checks.Warning,
 				Message:  fmt.Sprintf("Malformed image name for container '%s'", container.Name),
 				Kind:     checks.Pod,
@@ -82,7 +81,6 @@ func (fq *fullyQualifiedImageCheck) checkImage(containers []corev1.Container, po
 		} else {
 			if value.String() != container.Image {
 				d := checks.Diagnostic{
-					Check:    fq.Name(),
 					Severity: checks.Warning,
 					Message:  fmt.Sprintf("Use fully qualified image for container '%s'", container.Name),
 					Kind:     checks.Pod,

checks/basic/helper_test.go

@@ -92,7 +92,6 @@ func initContainer(image string) *kube.Objects {
 func issues(severity checks.Severity, message string, kind checks.Kind, check string) []checks.Diagnostic {
 	d := []checks.Diagnostic{
 		{
-			Check:    check,
 			Severity: severity,
 			Message:  message,
 			Kind:     kind,

checks/basic/hostpath.go

@@ -55,7 +55,6 @@ func (h *hostPathCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error)
 			pod := pod
 			if volume.VolumeSource.HostPath != nil {
 				d := checks.Diagnostic{
-					Check:    h.Name(),
 					Severity: checks.Warning,
 					Message:  fmt.Sprintf("Avoid using hostpath for volume '%s'.", volume.Name),
 					Kind:     checks.Pod,

checks/basic/hostpath_test.go

@@ -69,7 +69,6 @@ func TestHostpathVolumeError(t *testing.T) {
 			}),
 			expected: []checks.Diagnostic{
 				{
-					Check:    "hostpath-volume",
 					Severity: checks.Warning,
 					Message:  "Avoid using hostpath for volume 'bar'.",
 					Kind:     checks.Pod,

checks/basic/latest_tag.go

@@ -70,7 +70,6 @@ func (l *latestTagCheck) checkTags(containers []corev1.Container, pod corev1.Pod
 		tagNameOnly := reference.TagNameOnly(namedRef)
 		if strings.HasSuffix(tagNameOnly.String(), ":latest") {
 			d := checks.Diagnostic{
-				Check:    l.Name(),
 				Severity: checks.Warning,
 				Message:  fmt.Sprintf("Avoid using latest tag for container '%s'", container.Name),
 				Kind:     checks.Pod,

checks/basic/namespace.go

@@ -50,7 +50,6 @@ func (alert *alert) SetDiagnostics(d []checks.Diagnostic) {
 // warn adds warnings for k8s objects that should not be in the default namespace
 func (alert *alert) warn(k8stype checks.Kind, itemMeta metav1.ObjectMeta) {
 	d := checks.Diagnostic{
-		Check:    "default-namespace",
 		Severity: checks.Warning,
 		Message:  "Avoid using the default namespace",
 		Kind:     k8stype,

checks/basic/namespace_test.go

@@ -100,55 +100,48 @@ func errors(n defaultNamespaceCheck) []checks.Diagnostic {
 	d := []checks.Diagnostic{
 
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.Pod,
 			Object:   &pod.ObjectMeta,
 			Owners:   pod.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.PodTemplate,
 			Object:   &template.ObjectMeta,
 			Owners:   template.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.PersistentVolumeClaim,
 			Object:   &pvc.ObjectMeta,
 			Owners:   pvc.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.ConfigMap,
 			Object:   &cm.ObjectMeta,
 			Owners:   cm.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.Service,
 			Object:   &service.ObjectMeta,
 			Owners:   service.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.Secret,
 			Object:   &secret.ObjectMeta,
 			Owners:   secret.ObjectMeta.GetOwnerReferences(),
 		},
 		{
-			Check:    n.Name(),
 			Severity: checks.Warning,
 			Message:  "Avoid using the default namespace",
 			Kind:     checks.ServiceAccount,

checks/basic/pod_status.go

@@ -55,7 +55,6 @@ func (p *podStatusCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error)
 	for _, pod := range objects.Pods.Items {
 		if corev1.PodFailed == pod.Status.Phase || corev1.PodUnknown == pod.Status.Phase {
 			d := checks.Diagnostic{
-				Check:    p.Name(),
 				Severity: checks.Warning,
 				Message:  fmt.Sprintf("Unhealthy pod. State: `%s`. Pod state should be `Running`, `Pending` or `Succeeded`.", pod.Status.Phase),
 				Kind:     checks.Pod,

checks/basic/pod_status_test.go

@@ -71,7 +71,6 @@ func TestPodStateError(t *testing.T) {
 			objs: status(corev1.PodFailed),
 			expected: []checks.Diagnostic{
 				{
-					Check:    podStatusCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unhealthy pod. State: `Failed`. Pod state should be `Running`, `Pending` or `Succeeded`.",
 					Kind:     checks.Pod,
@@ -85,7 +84,6 @@ func TestPodStateError(t *testing.T) {
 			objs: status(corev1.PodUnknown),
 			expected: []checks.Diagnostic{
 				{
-					Check:    podStatusCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unhealthy pod. State: `Unknown`. Pod state should be `Running`, `Pending` or `Succeeded`.",
 					Kind:     checks.Pod,

checks/basic/resource_requests.go

@@ -69,7 +69,6 @@ func (r *resourceRequirementsCheck) checkResourceRequirements(containers []corev
 	for _, container := range containers {
 		if container.Resources.Size() == 0 {
 			d := checks.Diagnostic{
-				Check:    r.Name(),
 				Severity: checks.Warning,
 				Message:  fmt.Sprintf("Set resource requests and limits for container `%s` to prevent resource contention", container.Name),
 				Kind:     checks.Pod,

checks/basic/resource_requests_test.go

@@ -60,7 +60,6 @@ func TestResourceRequestsWarning(t *testing.T) {
 			objs: container("alpine"),
 			expected: []checks.Diagnostic{
 				{
-					Check:    resourceRequirementsCheck.Name(),
 					Severity: checks.Warning,
 					Message:  message,
 					Kind:     checks.Pod,
@@ -74,7 +73,6 @@ func TestResourceRequestsWarning(t *testing.T) {
 			objs: initContainer("alpine"),
 			expected: []checks.Diagnostic{
 				{
-					Check:    resourceRequirementsCheck.Name(),
 					Severity: checks.Warning,
 					Message:  message,
 					Kind:     checks.Pod,

checks/basic/unused_config_map.go

@@ -71,7 +71,6 @@ func (c *unusedCMCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error)
 		if _, ok := used[kube.Identifier{Name: cm.GetName(), Namespace: cm.GetNamespace()}]; !ok {
 			cm := cm
 			d := checks.Diagnostic{
-				Check:    c.Name(),
 				Severity: checks.Warning,
 				Message:  "Unused config map",
 				Kind:     checks.ConfigMap,

checks/basic/unused_config_map_test.go

@@ -80,7 +80,6 @@ func TestUnusedConfigMapWarning(t *testing.T) {
 			objs: initConfigMap(),
 			expected: []checks.Diagnostic{
 				{
-					Check:    unusedCMCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unused config map",
 					Kind:     checks.ConfigMap,

checks/basic/unused_pv.go

@@ -53,7 +53,6 @@ func (p *unusedPVCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error)
 	for _, pv := range objects.PersistentVolumes.Items {
 		if pv.Spec.ClaimRef == nil {
 			d := checks.Diagnostic{
-				Check:    p.Name(),
 				Severity: checks.Warning,
 				Message:  fmt.Sprintf("Unused Persistent Volume '%s'.", pv.GetName()),
 				Kind:     checks.PersistentVolume,

checks/basic/unused_pv_test.go

@@ -63,7 +63,6 @@ func TestUnusedPVWarning(t *testing.T) {
 			objs: unused(),
 			expected: []checks.Diagnostic{
 				{
-					Check:    unusedPVCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unused Persistent Volume 'pv_foo'.",
 					Kind:     checks.PersistentVolume,

checks/basic/unused_pvc.go

@@ -62,7 +62,6 @@ func (c *unusedClaimCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, erro
 	for _, claim := range objects.PersistentVolumeClaims.Items {
 		if _, ok := used[kube.Identifier{Name: claim.GetName(), Namespace: claim.GetNamespace()}]; !ok {
 			d := checks.Diagnostic{
-				Check:    c.Name(),
 				Severity: checks.Warning,
 				Message:  "Unused persistent volume claim",
 				Kind:     checks.PersistentVolumeClaim,

checks/basic/unused_pvc_test.go

@@ -67,7 +67,6 @@ func TestUnusedPVCWarning(t *testing.T) {
 			objs: initPVC(),
 			expected: []checks.Diagnostic{
 				{
-					Check:    unusedClaimCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unused persistent volume claim",
 					Kind:     checks.PersistentVolumeClaim,

checks/basic/unused_secrets.go

@@ -66,7 +66,6 @@ func (s *unusedSecretCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, err
 		if _, ok := used[kube.Identifier{Name: secret.GetName(), Namespace: secret.GetNamespace()}]; !ok {
 			secret := secret
 			d := checks.Diagnostic{
-				Check:    s.Name(),
 				Severity: checks.Warning,
 				Message:  "Unused secret",
 				Kind:     checks.Secret,

checks/basic/unused_secrets_test.go

@@ -78,7 +78,6 @@ func TestUnusedSecretWarning(t *testing.T) {
 			objs: initSecret(),
 			expected: []checks.Diagnostic{
 				{
-					Check:    unusedSecretCheck.Name(),
 					Severity: checks.Warning,
 					Message:  "Unused secret",
 					Kind:     checks.Secret,

checks/doks/node_labels_taints.go

@@ -53,7 +53,6 @@ func (c *nodeLabelsTaintsCheck) Run(objects *kube.Objects) ([]checks.Diagnostic,
 		for labelKey := range node.Labels {
 			if !isKubernetesLabel(labelKey) && !isDOKSLabel(labelKey) {
 				d := checks.Diagnostic{
-					Check:    c.Name(),
 					Severity: checks.Warning,
 					Message:  "Custom node labels will be lost if node is replaced or upgraded.",
 					Kind:     checks.Node,
@@ -67,7 +66,6 @@ func (c *nodeLabelsTaintsCheck) Run(objects *kube.Objects) ([]checks.Diagnostic,
 		for _, taint := range node.Spec.Taints {
 			if !isDOKSTaint(taint) {
 				d := checks.Diagnostic{
-					Check:    c.Name(),
 					Severity: checks.Warning,
 					Message:  "Custom node taints will be lost if node is replaced or upgraded.",
 					Kind:     checks.Node,

checks/doks/node_labels_taints_test.go

@@ -74,7 +74,6 @@ func TestNodeLabels(t *testing.T) {
 				"region": "tor1",
 			},
 			expectedDiagnostics: []checks.Diagnostic{{
-				Check:    "node-labels-and-taints",
 				Severity: checks.Warning,
 				Message:  "Custom node labels will be lost if node is replaced or upgraded.",
 				Kind:     checks.Node,
@@ -134,7 +133,6 @@ func TestNodeTaints(t *testing.T) {
 				Effect: corev1.TaintEffectNoSchedule,
 			}},
 			expectedDiagnostics: []checks.Diagnostic{{
-				Check:    "node-labels-and-taints",
 				Severity: checks.Warning,
 				Message:  "Custom node taints will be lost if node is replaced or upgraded.",
 				Kind:     checks.Node,

checks/doks/node_name_pod_selector.go

@@ -53,7 +53,6 @@ func (p *podSelectorCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, erro
 		nodeSelectorMap := pod.Spec.NodeSelector
 		if _, ok := nodeSelectorMap[corev1.LabelHostname]; ok {
 			d := checks.Diagnostic{
-				Check:    p.Name(),
 				Severity: checks.Warning,
 				Message:  "Avoid node name label for node selector.",
 				Kind:     checks.Pod,

checks/doks/node_name_pod_selector_test.go

@@ -93,7 +93,6 @@ func expectedWarnings(objs *kube.Objects, name string) []checks.Diagnostic {
 	pod := objs.Pods.Items[0]
 	diagnostics := []checks.Diagnostic{
 		{
-			Check:    name,
 			Severity: checks.Warning,
 			Message:  "Avoid node name label for node selector.",
 			Kind:     checks.Pod,

checks/run_checks.go

@@ -54,6 +54,12 @@ func Run(ctx context.Context, client *kube.Client, checkFilter CheckFilter, diag
 				return err
 			}
 			mu.Lock()
+			// Fill in the check names for the diagnostics. Doing this here
+			// absolves checks of needing to do it and also ensures they're
+			// consistent.
+			for i := 0; i < len(d); i++ {
+				d[i].Check = check.Name()
+			}
 			diagnostics = append(diagnostics, d...)
 			checkDuration[check.Name()] = elapsed
 			mu.Unlock()

checks/run_checks_test.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2019 DigitalOcean
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package checks
+
+import (
+	"context"
+	"testing"
+
+	"github.com/digitalocean/clusterlint/kube"
+	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestRun(t *testing.T) {
+	Register(&alwaysFail{})
+
+	filter := CheckFilter{
+		IncludeChecks: []string{"always-fail"},
+	}
+	client := &kube.Client{
+		KubeClient: fake.NewSimpleClientset(),
+	}
+	client.KubeClient.CoreV1().Namespaces().Create(&corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "kube-system",
+		},
+	})
+
+	alwaysFailCheck, err := Get("always-fail")
+	assert.NoError(t, err)
+
+	result, err := Run(context.Background(), client, filter, DiagnosticFilter{})
+	assert.NoError(t, err)
+	assert.Len(t, result.Diagnostics, 1)
+	assert.Equal(t, alwaysFailCheck.Name(), result.Diagnostics[0].Check)
+}
+
+type alwaysFail struct{}
+
+// Name returns a unique name for this check.
+func (nc *alwaysFail) Name() string {
+	return "always-fail"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (nc *alwaysFail) Groups() []string {
+	return nil
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (nc *alwaysFail) Description() string {
+	return "Does not check anything. Always returns an error.."
+}
+
+// Run runs this check on a set of Kubernetes objects. It can return warnings
+// (low-priority problems) and errors (high-priority problems) as well as an
+// error value indicating that the check failed to run.
+func (nc *alwaysFail) Run(*kube.Objects) ([]Diagnostic, error) {
+	return []Diagnostic{{
+		Message:  "This check always produces an error.",
+		Severity: Error,
+		Kind:     Pod,
+		Object:   &metav1.ObjectMeta{},
+	}}, nil
+}