Merge pull request #89 from varshavaradarajan/clusterlint-panic-recovery

Recover from panic when running checks

checks/run_checks.go

@@ -19,8 +19,10 @@ package checks
 import (
 	"context"
 	"errors"
+	"fmt"
 	"sync"
 	"time"
+	"runtime/debug"
 
 	"github.com/digitalocean/clusterlint/kube"
 	"golang.org/x/sync/errgroup"
@@ -46,7 +48,12 @@ func Run(ctx context.Context, client *kube.Client, checkFilter CheckFilter, diag
 	checkDuration := make(map[string]time.Duration)
 	for _, check := range all {
 		check := check
-		g.Go(func() error {
+		g.Go(func() (err error) {
+			defer func() {
+				if r := recover(); r != nil {
+					err = fmt.Errorf("Recovered from panic in check '%s': %v", check.Name(), string(debug.Stack()))
+				}
+			}()
 			start := time.Now()
 			d, err := check.Run(objects)
 			elapsed := time.Since(start)

checks/run_checks_test.go

@@ -28,27 +28,54 @@ import (
 )
 
 func TestRun(t *testing.T) {
-	Register(&alwaysFail{})
-
-	filter := CheckFilter{
-		IncludeChecks: []string{"always-fail"},
-	}
-	client := &kube.Client{
-		KubeClient: fake.NewSimpleClientset(),
-	}
-	client.KubeClient.CoreV1().Namespaces().Create(context.Background(), &corev1.Namespace{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "kube-system",
+	tests := []struct{
+		name string
+		check string
+		expectedErr string
+		expectedDiagnostics int
+	}{
+		{
+			name: "test failure",
+			check: "always-fail",
+			expectedDiagnostics: 1,
+		},
+		{
+			name: "test panic",
+			check: "panic-check",
+			expectedErr: "Recovered from panic in check 'panic-check':",
 		},
-	}, metav1.CreateOptions{})
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			Register(&alwaysFail{})
+			Register(&panicCheck{})
+			filter := CheckFilter{
+				IncludeChecks: []string{test.check},
+			}
+
+			client := &kube.Client{
+				KubeClient: fake.NewSimpleClientset(),
+			}
+			client.KubeClient.CoreV1().Namespaces().Create(context.Background(), &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "kube-system",
+				},
+			}, metav1.CreateOptions{})
 
-	alwaysFailCheck, err := Get("always-fail")
-	assert.NoError(t, err)
+			check, err := Get(test.check)
+			assert.NoError(t, err)
 
-	result, err := Run(context.Background(), client, filter, DiagnosticFilter{}, kube.ObjectFilter{})
-	assert.NoError(t, err)
-	assert.Len(t, result.Diagnostics, 1)
-	assert.Equal(t, alwaysFailCheck.Name(), result.Diagnostics[0].Check)
+			result, err := Run(context.Background(), client, filter, DiagnosticFilter{}, kube.ObjectFilter{})
+			if test.expectedErr == "" {
+				assert.NoError(t, err)
+				assert.Len(t, result.Diagnostics, 1)
+				assert.Equal(t, check.Name(), result.Diagnostics[0].Check)
+			} else {
+				assert.Contains(t, err.Error(), test.expectedErr)
+				assert.Nil(t, result)
+			}
+		})
+	}
 }
 
 type alwaysFail struct{}
@@ -80,3 +107,33 @@ func (nc *alwaysFail) Run(*kube.Objects) ([]Diagnostic, error) {
 		Object:   &metav1.ObjectMeta{},
 	}}, nil
 }
+
+type panicCheck struct {}
+
+// Name returns a unique name for this check.
+func (nc *panicCheck) Name() string {
+	return "panic-check"
+}
+
+// Groups returns a list of group names this check should be part of.
+func (nc *panicCheck) Groups() []string {
+	return nil
+}
+
+// Description returns a detailed human-readable description of what this check
+// does.
+func (nc *panicCheck) Description() string {
+	return "Does not check anything. Panics.."
+}
+
+// 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 *panicCheck) Run(*kube.Objects) ([]Diagnostic, error) {
+	type some struct {
+		x int
+	}
+	var s *some
+	_ = s.x
+	return nil, nil
+}