coder · johnstcn · Aug 25, 2021 · Aug 24, 2021 · Aug 24, 2021 · Aug 25, 2021
diff --git a/internal/checks/kube/kubernetes.go b/internal/checks/kube/kubernetes.go
@@ -17,11 +17,12 @@ import (
 var _ = api.Checker(&KubernetesChecker{})
 
 type KubernetesChecker struct {
-	namespace    string
-	client       kubernetes.Interface
-	writer       api.ResultWriter
-	coderVersion *semver.Version
-	log          slog.Logger
+	namespace        string
+	client           kubernetes.Interface
+	writer           api.ResultWriter
+	coderVersion     *semver.Version
+	log              slog.Logger
+	rbacRequirements []*RBACRequirement
 }
 
 type Option func(k *KubernetesChecker)
@@ -40,6 +41,12 @@ func NewKubernetesChecker(client kubernetes.Interface, opts ...Option) *Kubernet
 		opt(checker)
 	}
 
+	checker.rbacRequirements = findClosestVersionRequirements(checker.coderVersion)
+
+	if err := checker.Validate(); err != nil {
+		panic(xerrors.Errorf("error validating kube checker: %w", err))
+	}
+
 	return checker
 }
 
@@ -67,7 +74,10 @@ func WithNamespace(ns string) Option {
 	}
 }
 
-func (*KubernetesChecker) Validate() error {
+func (k *KubernetesChecker) Validate() error {
+	if k.rbacRequirements == nil {
+		return xerrors.Errorf("unhandled coder version: %s", k.coderVersion.String())
+	}
 	return nil
 }
 

diff --git a/internal/checks/kube/kubernetes_test.go b/internal/checks/kube/kubernetes_test.go
@@ -6,14 +6,32 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 
 	"cdr.dev/slog/sloggers/slogtest/assert"
+
+	"github.com/Masterminds/semver/v3"
 )
 
 func TestKubernetesOptions(t *testing.T) {
 	t.Parallel()
 
-	clientset := fake.NewSimpleClientset()
-	checker := NewKubernetesChecker(clientset)
-	assert.Success(t, "validation successful", checker.Validate())
+	t.Run("successful validation", func(t *testing.T) {
+		clientset := fake.NewSimpleClientset()
+		checker := NewKubernetesChecker(clientset)
+		assert.Success(t, "validation successful", checker.Validate())
+	})
+
+	t.Run("validation failed: unhandled coder version", func(t *testing.T) {
+		defer func() {
+			r := recover()
+			if r == nil {
+				t.Errorf("expected a panic")
+				t.FailNow()
+			}
+			assert.ErrorContains(t, "KubernetesChecker with unknown version should fail to validate", r.(error), "unhandled coder version")
+		}()
+
+		// This should panic
+		_ = NewKubernetesChecker(fake.NewSimpleClientset(), WithCoderVersion(semver.MustParse("1.19.0")))
+	})
 
 	// var buf bytes.Buffer
 	// log := slog.Make(sloghuman.Sink(&buf)).Leveled(slog.LevelDebug)

diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go
@@ -58,20 +58,9 @@ var allVersionedRBACRequirements = []VersionedRBACRequirements{
 func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult {
 	const checkName = "kubernetes-rbac"
 	authClient := k.client.AuthorizationV1()
-	rbacReqs := findClosestVersionRequirements(k.coderVersion)
 	results := make([]*api.CheckResult, 0)
-	if rbacReqs == nil {
-		results = append(results,
-			api.ErrorResult(
-				checkName,
-				"unable to check RBAC requirements",
-				xerrors.Errorf("unhandled coder version: %s", k.coderVersion.String()),
-			),
-		)
-		return results
-	}
 
-	for _, req := range rbacReqs.RBACRequirements {
+	for _, req := range k.rbacRequirements {
 		resName := fmt.Sprintf("%s-%s", checkName, req.Resource)
 		if err := k.checkOneRBAC(ctx, authClient, req); err != nil {
 			summary := fmt.Sprintf("missing permissions on resource %s: %s", req.Resource, err)
@@ -120,10 +109,10 @@ func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authori
 	return nil
 }
 
-func findClosestVersionRequirements(v *semver.Version) *VersionedRBACRequirements {
+func findClosestVersionRequirements(v *semver.Version) []*RBACRequirement {
 	for _, vreqs := range allVersionedRBACRequirements {
 		if vreqs.VersionConstraints.Check(v) {
-			return &vreqs
+			return vreqs.RBACRequirements
 		}
 	}
 	return nil

diff --git a/internal/checks/kube/rbac_test.go b/internal/checks/kube/rbac_test.go
@@ -13,8 +13,6 @@ import (
 
 	"cdr.dev/slog/sloggers/slogtest/assert"
 
-	"github.com/Masterminds/semver/v3"
-
 	"github.com/cdr/coder-doctor/internal/api"
 )
 
@@ -91,27 +89,6 @@ func Test_CheckRBAC_ClientError(t *testing.T) {
 	}
 }
 
-func Test_CheckRBAC_UnknownCoderVerseion(t *testing.T) {
-	t.Parallel()
-
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusInternalServerError)
-	}))
-	defer server.Close()
-
-	client, err := kubernetes.NewForConfig(&rest.Config{Host: server.URL})
-	assert.Success(t, "failed to create client", err)
-
-	checker := NewKubernetesChecker(client, WithCoderVersion(semver.MustParse("0.0.1")))
-
-	results := checker.CheckRBAC(context.Background())
-	for _, result := range results {
-		assert.ErrorContains(t, result.Name+" should show correct error", result.Details["error"].(error), "unhandled coder version")
-		assert.True(t, result.Name+" should fail", result.State == api.StateFailed)
-	}
-}
-
 var selfSubjectAccessReviewAllowed authorizationv1.SelfSubjectAccessReview = authorizationv1.SelfSubjectAccessReview{
 	Status: authorizationv1.SubjectAccessReviewStatus{
 		Allowed: true,

diff --git a/internal/checks/local/helm.go b/internal/checks/local/helm.go
@@ -51,9 +51,6 @@ func (l *Checker) CheckLocalHelmVersion(ctx context.Context) *api.CheckResult {
 	}
 
 	selectedVersion := findNearestHelmVersion(l.coderVersion)
-	if selectedVersion == nil {
-		return api.ErrorResult(LocalHelmVersionCheck, fmt.Sprintf("checking coder version %s not supported", l.coderVersion.String()), nil)
-	}
 	l.log.Debug(ctx, "selected coder version", slog.F("requested", l.coderVersion), slog.F("selected", selectedVersion.Coder))
 
 	result := &api.CheckResult{

diff --git a/internal/checks/local/helm_test.go b/internal/checks/local/helm_test.go
@@ -127,18 +127,17 @@ func Test_CheckLocalHelmVersion(t *testing.T) {
 		}
 	})
 
-	run(t, "helm: coder version is unsupported", func(t *testing.T, p *params) {
+	run(t, "helm: someone did not call validate", func(t *testing.T, p *params) {
+		defer func() {
+			if r := recover(); r == nil {
+				t.Errorf("this code should have panicked")
+				t.FailNow()
+			}
+		}()
 		p.Opts = append(p.Opts, WithCoderVersion(semver.MustParse("v1.19")))
 		p.LP.Handle("helm", "/usr/local/bin/helm", nil)
 		p.EX.Handle("/usr/local/bin/helm version --short", []byte("v3.6.0+g7f2df64"), nil)
 		lc := NewChecker(p.Opts...)
-		err := lc.Run(p.Ctx)
-		assert.Success(t, "run local checker", err)
-		assert.False(t, "results should not be empty", p.W.Empty())
-		for _, res := range p.W.Get() {
-			if res.Name == LocalHelmVersionCheck {
-				assert.Equal(t, "should fail", api.StateFailed, res.State)
-			}
-		}
+		_ = lc.Run(p.Ctx)
 	})
 }
diff --git a/internal/checks/local/local.go b/internal/checks/local/local.go
@@ -44,6 +44,10 @@ func NewChecker(opts ...Option) *Checker {
 		opt(checker)
 	}
 
+	if err := checker.Validate(); err != nil {
+		panic(xerrors.Errorf("error validating local checker: %w", err))
+	}
+
 	return checker
 }
 
@@ -83,7 +87,11 @@ func WithLookPathF(f LookPathF) Option {
 	}
 }
 
-func (*Checker) Validate() error {
+func (l *Checker) Validate() error {
+	// Ensure we know the Helm version requirement for our Coder version.
+	if findNearestHelmVersion(l.coderVersion) == nil {
+		return xerrors.Errorf("unhandled coder version %s: compatible helm version not specified", l.coderVersion.String())
+	}
 	return nil
 }
 

diff --git a/internal/checks/local/local_test.go b/internal/checks/local/local_test.go
@@ -4,8 +4,33 @@ import (
 	"context"
 	"strings"
 	"testing"
+
+	"cdr.dev/slog/sloggers/slogtest/assert"
+
+	"github.com/Masterminds/semver/v3"
 )
 
+func Test_LocalChecker_Validate(t *testing.T) {
+	t.Parallel()
+
+	t.Run("successful validation", func(t *testing.T) {
+		lc := NewChecker()
+		assert.Success(t, "local checker with defaults should be successful", lc.Validate())
+	})
+
+	t.Run("validation failed: unsupported coder version", func(t *testing.T) {
+		defer func() {
+			r := recover()
+			if r == nil {
+				t.Errorf("expected a panic")
+				t.FailNow()
+			}
+			assert.ErrorContains(t, "KubernetesChecker with unknown version should fail to validate", r.(error), "unhandled coder version")
+		}()
+		_ = NewChecker(WithCoderVersion(semver.MustParse("0.0.1")))
+	})
+}
+
 type execResult struct {
 	Output []byte
 	Err    error

diff --git a/internal/cmd/check/kubernetes/kubernetes.go b/internal/cmd/check/kubernetes/kubernetes.go
@@ -141,10 +141,18 @@ func run(cmd *cobra.Command, _ []string) error {
 		kube.WithNamespace(currentContext.Namespace),
 	)
 
+	if err := localChecker.Validate(); err != nil {
+		return xerrors.Errorf("failed to validate local checks: %w", err)
+	}
+
 	if err := localChecker.Run(cmd.Context()); err != nil {
 		return xerrors.Errorf("run local checker: %w", err)
 	}
 
+	if err := kubeChecker.Validate(); err != nil {
+		return xerrors.Errorf("failed to validate kube checker: %w", err)
+	}
+
 	if err := kubeChecker.Run(cmd.Context()); err != nil {
 		return xerrors.Errorf("run kube checker: %w", err)
 	}

diff --git a/main.go b/main.go
@@ -2,12 +2,19 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"os"
 
 	"github.com/cdr/coder-doctor/internal/cmd"
 )
 
 func main() {
+	defer func() {
+		if r := recover(); r != nil {
+			_, _ = fmt.Fprintln(os.Stderr, "fatal:", r.(error))
+		}
+		os.Exit(1)
+	}()
 	command := cmd.NewDefaultDoctorCommand()
 	err := command.ExecuteContext(context.Background())
 	if err != nil {