Skip to content
This repository was archived by the owner on Nov 14, 2024. It is now read-only.

Commit 7362138

Browse files
authored
fix: validate kubernetes checker (#7)
* internal/checks/kube: validate rbac requirements * internal/checks/local: validate local helm requirements
1 parent 831e4d5 commit 7362138

File tree

10 files changed

+97
-59
lines changed

10 files changed

+97
-59
lines changed

internal/checks/kube/kubernetes.go

+16-6
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ import (
1717
var _ = api.Checker(&KubernetesChecker{})
1818

1919
type KubernetesChecker struct {
20-
namespace string
21-
client kubernetes.Interface
22-
writer api.ResultWriter
23-
coderVersion *semver.Version
24-
log slog.Logger
20+
namespace string
21+
client kubernetes.Interface
22+
writer api.ResultWriter
23+
coderVersion *semver.Version
24+
log slog.Logger
25+
rbacRequirements []*RBACRequirement
2526
}
2627

2728
type Option func(k *KubernetesChecker)
@@ -40,6 +41,12 @@ func NewKubernetesChecker(client kubernetes.Interface, opts ...Option) *Kubernet
4041
opt(checker)
4142
}
4243

44+
checker.rbacRequirements = findClosestVersionRequirements(checker.coderVersion)
45+
46+
if err := checker.Validate(); err != nil {
47+
panic(xerrors.Errorf("error validating kube checker: %w", err))
48+
}
49+
4350
return checker
4451
}
4552

@@ -67,7 +74,10 @@ func WithNamespace(ns string) Option {
6774
}
6875
}
6976

70-
func (*KubernetesChecker) Validate() error {
77+
func (k *KubernetesChecker) Validate() error {
78+
if k.rbacRequirements == nil {
79+
return xerrors.Errorf("unhandled coder version: %s", k.coderVersion.String())
80+
}
7181
return nil
7282
}
7383

internal/checks/kube/kubernetes_test.go

+21-3
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,32 @@ import (
66
"k8s.io/client-go/kubernetes/fake"
77

88
"cdr.dev/slog/sloggers/slogtest/assert"
9+
10+
"github.com/Masterminds/semver/v3"
911
)
1012

1113
func TestKubernetesOptions(t *testing.T) {
1214
t.Parallel()
1315

14-
clientset := fake.NewSimpleClientset()
15-
checker := NewKubernetesChecker(clientset)
16-
assert.Success(t, "validation successful", checker.Validate())
16+
t.Run("successful validation", func(t *testing.T) {
17+
clientset := fake.NewSimpleClientset()
18+
checker := NewKubernetesChecker(clientset)
19+
assert.Success(t, "validation successful", checker.Validate())
20+
})
21+
22+
t.Run("validation failed: unhandled coder version", func(t *testing.T) {
23+
defer func() {
24+
r := recover()
25+
if r == nil {
26+
t.Errorf("expected a panic")
27+
t.FailNow()
28+
}
29+
assert.ErrorContains(t, "KubernetesChecker with unknown version should fail to validate", r.(error), "unhandled coder version")
30+
}()
31+
32+
// This should panic
33+
_ = NewKubernetesChecker(fake.NewSimpleClientset(), WithCoderVersion(semver.MustParse("1.19.0")))
34+
})
1735

1836
// var buf bytes.Buffer
1937
// log := slog.Make(sloghuman.Sink(&buf)).Leveled(slog.LevelDebug)

internal/checks/kube/rbac.go

+3-14
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,9 @@ var allVersionedRBACRequirements = []VersionedRBACRequirements{
5858
func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult {
5959
const checkName = "kubernetes-rbac"
6060
authClient := k.client.AuthorizationV1()
61-
rbacReqs := findClosestVersionRequirements(k.coderVersion)
6261
results := make([]*api.CheckResult, 0)
63-
if rbacReqs == nil {
64-
results = append(results,
65-
api.ErrorResult(
66-
checkName,
67-
"unable to check RBAC requirements",
68-
xerrors.Errorf("unhandled coder version: %s", k.coderVersion.String()),
69-
),
70-
)
71-
return results
72-
}
7362

74-
for _, req := range rbacReqs.RBACRequirements {
63+
for _, req := range k.rbacRequirements {
7564
resName := fmt.Sprintf("%s-%s", checkName, req.Resource)
7665
if err := k.checkOneRBAC(ctx, authClient, req); err != nil {
7766
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
120109
return nil
121110
}
122111

123-
func findClosestVersionRequirements(v *semver.Version) *VersionedRBACRequirements {
112+
func findClosestVersionRequirements(v *semver.Version) []*RBACRequirement {
124113
for _, vreqs := range allVersionedRBACRequirements {
125114
if vreqs.VersionConstraints.Check(v) {
126-
return &vreqs
115+
return vreqs.RBACRequirements
127116
}
128117
}
129118
return nil

internal/checks/kube/rbac_test.go

-23
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import (
1313

1414
"cdr.dev/slog/sloggers/slogtest/assert"
1515

16-
"github.com/Masterminds/semver/v3"
17-
1816
"github.com/cdr/coder-doctor/internal/api"
1917
)
2018

@@ -91,27 +89,6 @@ func Test_CheckRBAC_ClientError(t *testing.T) {
9189
}
9290
}
9391

94-
func Test_CheckRBAC_UnknownCoderVerseion(t *testing.T) {
95-
t.Parallel()
96-
97-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
98-
w.Header().Set("Content-Type", "application/json")
99-
w.WriteHeader(http.StatusInternalServerError)
100-
}))
101-
defer server.Close()
102-
103-
client, err := kubernetes.NewForConfig(&rest.Config{Host: server.URL})
104-
assert.Success(t, "failed to create client", err)
105-
106-
checker := NewKubernetesChecker(client, WithCoderVersion(semver.MustParse("0.0.1")))
107-
108-
results := checker.CheckRBAC(context.Background())
109-
for _, result := range results {
110-
assert.ErrorContains(t, result.Name+" should show correct error", result.Details["error"].(error), "unhandled coder version")
111-
assert.True(t, result.Name+" should fail", result.State == api.StateFailed)
112-
}
113-
}
114-
11592
var selfSubjectAccessReviewAllowed authorizationv1.SelfSubjectAccessReview = authorizationv1.SelfSubjectAccessReview{
11693
Status: authorizationv1.SubjectAccessReviewStatus{
11794
Allowed: true,

internal/checks/local/helm.go

-3
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ func (l *Checker) CheckLocalHelmVersion(ctx context.Context) *api.CheckResult {
5151
}
5252

5353
selectedVersion := findNearestHelmVersion(l.coderVersion)
54-
if selectedVersion == nil {
55-
return api.ErrorResult(LocalHelmVersionCheck, fmt.Sprintf("checking coder version %s not supported", l.coderVersion.String()), nil)
56-
}
5754
l.log.Debug(ctx, "selected coder version", slog.F("requested", l.coderVersion), slog.F("selected", selectedVersion.Coder))
5855

5956
result := &api.CheckResult{

internal/checks/local/helm_test.go

+8-9
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,17 @@ func Test_CheckLocalHelmVersion(t *testing.T) {
127127
}
128128
})
129129

130-
run(t, "helm: coder version is unsupported", func(t *testing.T, p *params) {
130+
run(t, "helm: someone did not call validate", func(t *testing.T, p *params) {
131+
defer func() {
132+
if r := recover(); r == nil {
133+
t.Errorf("this code should have panicked")
134+
t.FailNow()
135+
}
136+
}()
131137
p.Opts = append(p.Opts, WithCoderVersion(semver.MustParse("v1.19")))
132138
p.LP.Handle("helm", "/usr/local/bin/helm", nil)
133139
p.EX.Handle("/usr/local/bin/helm version --short", []byte("v3.6.0+g7f2df64"), nil)
134140
lc := NewChecker(p.Opts...)
135-
err := lc.Run(p.Ctx)
136-
assert.Success(t, "run local checker", err)
137-
assert.False(t, "results should not be empty", p.W.Empty())
138-
for _, res := range p.W.Get() {
139-
if res.Name == LocalHelmVersionCheck {
140-
assert.Equal(t, "should fail", api.StateFailed, res.State)
141-
}
142-
}
141+
_ = lc.Run(p.Ctx)
143142
})
144143
}

internal/checks/local/local.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ func NewChecker(opts ...Option) *Checker {
4444
opt(checker)
4545
}
4646

47+
if err := checker.Validate(); err != nil {
48+
panic(xerrors.Errorf("error validating local checker: %w", err))
49+
}
50+
4751
return checker
4852
}
4953

@@ -83,7 +87,11 @@ func WithLookPathF(f LookPathF) Option {
8387
}
8488
}
8589

86-
func (*Checker) Validate() error {
90+
func (l *Checker) Validate() error {
91+
// Ensure we know the Helm version requirement for our Coder version.
92+
if findNearestHelmVersion(l.coderVersion) == nil {
93+
return xerrors.Errorf("unhandled coder version %s: compatible helm version not specified", l.coderVersion.String())
94+
}
8795
return nil
8896
}
8997

internal/checks/local/local_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,33 @@ import (
44
"context"
55
"strings"
66
"testing"
7+
8+
"cdr.dev/slog/sloggers/slogtest/assert"
9+
10+
"github.com/Masterminds/semver/v3"
711
)
812

13+
func Test_LocalChecker_Validate(t *testing.T) {
14+
t.Parallel()
15+
16+
t.Run("successful validation", func(t *testing.T) {
17+
lc := NewChecker()
18+
assert.Success(t, "local checker with defaults should be successful", lc.Validate())
19+
})
20+
21+
t.Run("validation failed: unsupported coder version", func(t *testing.T) {
22+
defer func() {
23+
r := recover()
24+
if r == nil {
25+
t.Errorf("expected a panic")
26+
t.FailNow()
27+
}
28+
assert.ErrorContains(t, "KubernetesChecker with unknown version should fail to validate", r.(error), "unhandled coder version")
29+
}()
30+
_ = NewChecker(WithCoderVersion(semver.MustParse("0.0.1")))
31+
})
32+
}
33+
934
type execResult struct {
1035
Output []byte
1136
Err error

internal/cmd/check/kubernetes/kubernetes.go

+8
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,18 @@ func run(cmd *cobra.Command, _ []string) error {
141141
kube.WithNamespace(currentContext.Namespace),
142142
)
143143

144+
if err := localChecker.Validate(); err != nil {
145+
return xerrors.Errorf("failed to validate local checks: %w", err)
146+
}
147+
144148
if err := localChecker.Run(cmd.Context()); err != nil {
145149
return xerrors.Errorf("run local checker: %w", err)
146150
}
147151

152+
if err := kubeChecker.Validate(); err != nil {
153+
return xerrors.Errorf("failed to validate kube checker: %w", err)
154+
}
155+
148156
if err := kubeChecker.Run(cmd.Context()); err != nil {
149157
return xerrors.Errorf("run kube checker: %w", err)
150158
}

main.go

+7
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,19 @@ package main
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67

78
"github.com/cdr/coder-doctor/internal/cmd"
89
)
910

1011
func main() {
12+
defer func() {
13+
if r := recover(); r != nil {
14+
_, _ = fmt.Fprintln(os.Stderr, "fatal:", r.(error))
15+
}
16+
os.Exit(1)
17+
}()
1118
command := cmd.NewDefaultDoctorCommand()
1219
err := command.ExecuteContext(context.Background())
1320
if err != nil {

0 commit comments

Comments
 (0)