From 161508347b8ea228d7221430823e222bfb2f304f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 18 Aug 2021 11:13:28 +0000 Subject: [PATCH 01/15] chore: goimports --- internal/checks/kube/kubernetes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/checks/kube/kubernetes.go b/internal/checks/kube/kubernetes.go index 79dd642..486d817 100644 --- a/internal/checks/kube/kubernetes.go +++ b/internal/checks/kube/kubernetes.go @@ -5,8 +5,8 @@ import ( "io" "github.com/Masterminds/semver/v3" - "k8s.io/client-go/kubernetes" "golang.org/x/xerrors" + "k8s.io/client-go/kubernetes" "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" From 4d9da294a7ab537d344ce9f384dfc5fdedcff597 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 18 Aug 2021 11:38:11 +0000 Subject: [PATCH 02/15] feat: log current kubernetes context --- internal/cmd/check/kubernetes/kubernetes.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/cmd/check/kubernetes/kubernetes.go b/internal/cmd/check/kubernetes/kubernetes.go index bddd6d6..c3c9730 100644 --- a/internal/cmd/check/kubernetes/kubernetes.go +++ b/internal/cmd/check/kubernetes/kubernetes.go @@ -75,11 +75,17 @@ func run(cmd *cobra.Command, _ []string) error { return xerrors.Errorf("parse flags: %w", err) } - config, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides).ClientConfig() + configLoader := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides) + config, err := configLoader.ClientConfig() if err != nil { return xerrors.Errorf("creating NonInteractiveDeferredLoadingClientConfig: %w", err) } + rawConfig, err := configLoader.RawConfig() + if err != nil { + return xerrors.Errorf("creating RawConfig: %w", err) + } + clientset, err := kclient.NewForConfig(config) if err != nil { return xerrors.Errorf("creating kube client from config: %w", err) @@ -107,6 +113,13 @@ func run(cmd *cobra.Command, _ []string) error { log = log.Leveled(slog.LevelDebug) } + log.Info(cmd.Context(), "kubernetes config:", + slog.F("context", rawConfig.CurrentContext), + slog.F("cluster", rawConfig.Contexts[rawConfig.CurrentContext].Cluster), + slog.F("namespace", rawConfig.Contexts[rawConfig.CurrentContext].Namespace), + slog.F("authinfo", rawConfig.Contexts[rawConfig.CurrentContext].AuthInfo), + ) + checker := kube.NewKubernetesChecker( clientset, kube.WithLogger(log), From 1d276e334094029e306f5aa871318709befcbad8 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 18 Aug 2021 15:42:47 +0000 Subject: [PATCH 03/15] feat: kubernetes: check local helm version --- internal/api/skip.go | 11 ++ internal/api/skip_test.go | 19 +++ internal/api/types.go | 14 ++ internal/api/version.go | 15 ++ internal/checks/local/helm.go | 101 ++++++++++++++ internal/checks/local/helm_test.go | 144 ++++++++++++++++++++ internal/checks/local/local.go | 105 ++++++++++++++ internal/checks/local/local_test.go | 86 ++++++++++++ internal/cmd/check/kubernetes/kubernetes.go | 22 ++- 9 files changed, 513 insertions(+), 4 deletions(-) create mode 100644 internal/api/skip.go create mode 100644 internal/api/skip_test.go create mode 100644 internal/api/version.go create mode 100644 internal/checks/local/helm.go create mode 100644 internal/checks/local/helm_test.go create mode 100644 internal/checks/local/local.go create mode 100644 internal/checks/local/local_test.go diff --git a/internal/api/skip.go b/internal/api/skip.go new file mode 100644 index 0000000..06553e6 --- /dev/null +++ b/internal/api/skip.go @@ -0,0 +1,11 @@ +package api + +// SkippedResult returns a CheckResult indicating the check was skipped. +func SkippedResult(name string, summary string) *CheckResult { + return &CheckResult{ + Name: name, + State: StateSkipped, + Summary: summary, + Details: map[string]interface{}{}, + } +} diff --git a/internal/api/skip_test.go b/internal/api/skip_test.go new file mode 100644 index 0000000..b2a5bf1 --- /dev/null +++ b/internal/api/skip_test.go @@ -0,0 +1,19 @@ +package api_test + +import ( + "testing" + + "cdr.dev/slog/sloggers/slogtest/assert" + "github.com/cdr/coder-doctor/internal/api" +) + +func TestSkippedResult(t *testing.T) { + t.Parallel() + + checkName := "don't wanna" + checkSummary := "just because" + res := api.SkippedResult(checkName, checkSummary) + assert.Equal(t, "name matches", checkName, res.Name) + assert.Equal(t, "state matches", api.StateSkipped, res.State) + assert.Equal(t, "summary matches", checkSummary, res.Summary) +} diff --git a/internal/api/types.go b/internal/api/types.go index 04ed835..83f3c67 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -119,3 +119,17 @@ type CheckResult struct { Summary string `json:"summary"` Details map[string]interface{} `json:"details,omitempty"` } + +// CheckTarget indicates the subject of a Checker +type CheckTarget string + +const ( + // CheckTargetUndefined indicates that a Checker does not run against any specific target. + CheckTargetUndefined CheckTarget = "" + + // CheckTargetLocal indicates that a Checker runs against the local machine. + CheckTargetLocal CheckTarget = "local" + + // CheckTargetKubernetes indicates that a Checker runs against a Kubernetes cluster. + CheckTargetKubernetes = "kubernetes" +) diff --git a/internal/api/version.go b/internal/api/version.go new file mode 100644 index 0000000..409ffe6 --- /dev/null +++ b/internal/api/version.go @@ -0,0 +1,15 @@ +package api + +import ( + "github.com/Masterminds/semver/v3" + "golang.org/x/xerrors" +) + +func MustConstraint(s string) *semver.Constraints { + c, err := semver.NewConstraint(s) + if err != nil { + panic(xerrors.Errorf("parse constraint: %w", err)) + } + + return c +} diff --git a/internal/checks/local/helm.go b/internal/checks/local/helm.go new file mode 100644 index 0000000..4b1343f --- /dev/null +++ b/internal/checks/local/helm.go @@ -0,0 +1,101 @@ +package local + +import ( + "bytes" + "context" + "fmt" + "strings" + + "github.com/Masterminds/semver/v3" + + "cdr.dev/slog" + "github.com/cdr/coder-doctor/internal/api" +) + +const LocalHelmVersionCheck = "local-helm-version" + +type VersionRequirement struct { + Coder *semver.Version + HelmConstraint *semver.Constraints +} + +var versionRequirements = []VersionRequirement{ + { + Coder: semver.MustParse("1.21.0"), + HelmConstraint: api.MustConstraint(">= 3.6.0"), + }, + { + Coder: semver.MustParse("1.20.0"), + HelmConstraint: api.MustConstraint(">= 3.6.0"), + }, +} + +func (l *Checker) CheckLocalHelmVersion(ctx context.Context) *api.CheckResult { + if l.target != api.CheckTargetKubernetes { + return api.SkippedResult(LocalHelmVersionCheck, "not applicable for target "+string(l.target)) + } + + helmBin, err := l.lookPathF("helm") + if err != nil { + return api.ErrorResult(LocalHelmVersionCheck, "could not find helm binary in $PATH", err) + } + + helmVersionRaw, err := l.execF(ctx, helmBin, "version", "--short") + if err != nil { + return api.ErrorResult(LocalHelmVersionCheck, "failed to determine helm version", err) + } + + helmVersion, err := semver.NewVersion(string(bytes.TrimSpace(helmVersionRaw))) + if err != nil { + return api.ErrorResult(LocalHelmVersionCheck, "failed to parse helm version", err) + } + + 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{ + Name: LocalHelmVersionCheck, + Details: map[string]interface{}{ + "helm-bin": helmBin, + "helm-version": helmVersion.String(), + "helm-version-constraints": selectedVersion.HelmConstraint.String(), + }, + } + + if ok, cerrs := selectedVersion.HelmConstraint.Validate(helmVersion); !ok { + result.State = api.StateFailed + var b strings.Builder + _, err := fmt.Fprintf(&b, "Coder %s requires Helm version %s (installed: %s)\n", selectedVersion.Coder, selectedVersion.HelmConstraint, helmVersion) + if err != nil { + return api.ErrorResult(LocalHelmVersionCheck, "failed to write error result", err) + } + for _, cerr := range cerrs { + if _, err := fmt.Fprintf(&b, "constraint failed: %s\n", cerr); err != nil { + return api.ErrorResult(LocalHelmVersionCheck, "failed to write constraint error", err) + } + } + result.Summary = b.String() + } else { + result.State = api.StatePassed + result.Summary = fmt.Sprintf("Coder %s supports Helm %s", selectedVersion.Coder, selectedVersion.HelmConstraint) + } + + return result +} + +func findNearestHelmVersion(target *semver.Version) *VersionRequirement { + var selected *VersionRequirement + + for _, v := range versionRequirements { + v := v + if !v.Coder.GreaterThan(target) { + selected = &v + break + } + } + + return selected +} diff --git a/internal/checks/local/helm_test.go b/internal/checks/local/helm_test.go new file mode 100644 index 0000000..34575ff --- /dev/null +++ b/internal/checks/local/helm_test.go @@ -0,0 +1,144 @@ +package local + +import ( + "context" + "os" + "testing" + + "github.com/Masterminds/semver/v3" + + "cdr.dev/slog/sloggers/slogtest/assert" + "github.com/cdr/coder-doctor/internal/api" +) + +func Test_CheckLocalHelmVersion(t *testing.T) { + t.Parallel() + + type params struct { + W *api.CaptureWriter + EX *fakeExecer + LP *fakeLookPather + Opts []Option + Ctx context.Context + } + + run := func(t *testing.T, name string, fn func(t *testing.T, p *params)) { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + cw := &api.CaptureWriter{} + ex := newFakeExecer(t) + lp := newFakeLookPather(t) + opts := []Option{ + WithWriter(cw), + WithExecF(ex.ExecContext), + WithLookPathF(lp.LookPath), + WithTarget(api.CheckTargetKubernetes), // default + } + p := ¶ms{ + W: cw, + EX: ex, + LP: lp, + Opts: opts, + Ctx: ctx, + } + fn(t, p) + }) + } + + run(t, "helm: when not running against kubernetes", func(t *testing.T, p *params) { + p.Opts = append(p.Opts, WithTarget(api.CheckTargetUndefined)) + 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 skip helm check if not running against kubernetes", api.StateSkipped, res.State) + } + } + }) + + run(t, "helm: with version 3.6", func(t *testing.T, p *params) { + 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 pass", api.StatePassed, res.State) + } + } + }) + + run(t, "helm: with version 2", func(t *testing.T, p *params) { + p.LP.Handle("helm", "/usr/local/bin/helm", nil) + p.EX.Handle("/usr/local/bin/helm version --short", []byte("v2.0.0"), 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) + } + } + }) + + run(t, "helm: not in path", func(t *testing.T, p *params) { + p.LP.Handle("helm", "", os.ErrNotExist) + 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) + } + } + }) + + run(t, "helm: cannot be executed", func(t *testing.T, p *params) { + p.LP.Handle("helm", "/usr/local/bin/helm", nil) + p.EX.Handle("/usr/local/bin/helm version --short", []byte(""), os.ErrPermission) + 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) + } + } + }) + + run(t, "helm: returns garbage version", func(t *testing.T, p *params) { + p.LP.Handle("helm", "/usr/local/bin/helm", nil) + p.EX.Handle("/usr/local/bin/helm version --short", []byte(""), 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) + } + } + }) + + run(t, "helm: coder version is unsupported", func(t *testing.T, p *params) { + 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) + } + } + }) +} diff --git a/internal/checks/local/local.go b/internal/checks/local/local.go new file mode 100644 index 0000000..1ca5fd4 --- /dev/null +++ b/internal/checks/local/local.go @@ -0,0 +1,105 @@ +package local + +import ( + "context" + "io" + "os/exec" + + "github.com/Masterminds/semver/v3" + "golang.org/x/xerrors" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/sloghuman" + + "github.com/cdr/coder-doctor/internal/api" +) + +var _ api.Checker = &Checker{} + +type ExecF func(ctx context.Context, name string, args ...string) ([]byte, error) +type LookPathF func(string) (string, error) + +// local.Checker checks the local environment. +type Checker struct { + writer api.ResultWriter + coderVersion *semver.Version + log slog.Logger + target api.CheckTarget + execF ExecF + lookPathF LookPathF +} + +type Option func(*Checker) + +func NewChecker(opts ...Option) *Checker { + checker := &Checker{ + writer: &api.DiscardWriter{}, + coderVersion: semver.MustParse("100.0.0"), + log: slog.Make(sloghuman.Sink(io.Discard)), + execF: defaultExecCommand, + lookPathF: exec.LookPath, + } + + for _, opt := range opts { + opt(checker) + } + + return checker +} + +func WithTarget(t api.CheckTarget) Option { + return func(l *Checker) { + l.target = t + } +} + +func WithWriter(writer api.ResultWriter) Option { + return func(l *Checker) { + l.writer = writer + } +} + +func WithCoderVersion(version *semver.Version) Option { + return func(l *Checker) { + l.coderVersion = version + } +} + +func WithLogger(log slog.Logger) Option { + return func(l *Checker) { + l.log = log + } +} + +func WithExecF(f ExecF) Option { + return func(l *Checker) { + l.execF = f + } +} + +func WithLookPathF(f LookPathF) Option { + return func(l *Checker) { + l.lookPathF = f + } +} + +func (*Checker) Validate() error { + return nil +} + +func (l *Checker) Run(ctx context.Context) error { + if err := l.writer.WriteResult(l.CheckLocalHelmVersion(ctx)); err != nil { + return xerrors.Errorf("check local helm version: %w", err) + } + return nil +} + +func defaultExecCommand(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, name, args...) + out, err := cmd.CombinedOutput() + if err != nil { + return nil, xerrors.Errorf("exec %q %+q: %w", name, args, err) + } + + return out, nil +} diff --git a/internal/checks/local/local_test.go b/internal/checks/local/local_test.go new file mode 100644 index 0000000..f3b5570 --- /dev/null +++ b/internal/checks/local/local_test.go @@ -0,0 +1,86 @@ +package local + +import ( + "context" + "strings" + "testing" +) + +type execResult struct { + Output []byte + Err error +} + +func newFakeExecer(t *testing.T) *fakeExecer { + m := make(map[string]execResult) + return &fakeExecer{ + M: m, + T: t, + } +} + +type fakeExecer struct { + M map[string]execResult + T *testing.T +} + +func (f *fakeExecer) Handle(cmd string, output []byte, err error) { + f.M[cmd] = execResult{ + Output: output, + Err: err, + } +} + +func (f *fakeExecer) ExecContext(_ context.Context, name string, args ...string) ([]byte, error) { + var sb strings.Builder + _, _ = sb.WriteString(name) + for _, arg := range args { + _, _ = sb.WriteString(" ") + _, _ = sb.WriteString(arg) + } + + fullCmd := sb.String() + res, ok := f.M[fullCmd] + if !ok { + f.T.Logf("unhandled ExecContext: %s", fullCmd) + f.T.FailNow() + return nil, nil // should never happen + } + + return res.Output, res.Err +} + +type lookPathResult struct { + S string + Err error +} + +type fakeLookPather struct { + M map[string]lookPathResult + T *testing.T +} + +func (f *fakeLookPather) LookPath(name string) (string, error) { + res, ok := f.M[name] + if !ok { + f.T.Logf("unhandled LookPath: %s", name) + f.T.FailNow() + } + + return res.S, res.Err +} + +func (f *fakeLookPather) Handle(name string, path string, err error) { + f.M[name] = lookPathResult{ + S: path, + Err: err, + } +} + +func newFakeLookPather(t *testing.T) *fakeLookPather { + m := make(map[string]lookPathResult) + return &fakeLookPather{ + M: m, + T: t, + } +} diff --git a/internal/cmd/check/kubernetes/kubernetes.go b/internal/cmd/check/kubernetes/kubernetes.go index c3c9730..533fa95 100644 --- a/internal/cmd/check/kubernetes/kubernetes.go +++ b/internal/cmd/check/kubernetes/kubernetes.go @@ -19,7 +19,9 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth/openstack" "k8s.io/client-go/tools/clientcmd" + "github.com/cdr/coder-doctor/internal/api" "github.com/cdr/coder-doctor/internal/checks/kube" + "github.com/cdr/coder-doctor/internal/checks/local" "github.com/cdr/coder-doctor/internal/humanwriter" ) @@ -120,15 +122,27 @@ func run(cmd *cobra.Command, _ []string) error { slog.F("authinfo", rawConfig.Contexts[rawConfig.CurrentContext].AuthInfo), ) - checker := kube.NewKubernetesChecker( + hw := humanwriter.New(os.Stdout) + + localChecker := local.NewChecker( + local.WithLogger(log), + local.WithCoderVersion(cv), + local.WithWriter(hw), + local.WithTarget(api.CheckTargetKubernetes), + ) + + kubeChecker := kube.NewKubernetesChecker( clientset, kube.WithLogger(log), kube.WithCoderVersion(cv), - kube.WithWriter(humanwriter.New(os.Stdout)), + kube.WithWriter(hw), ) - err = checker.Run(cmd.Context()) - if err != nil { + if err := localChecker.Run(cmd.Context()); err != nil { + return xerrors.Errorf("run local checker: %w", err) + } + + if err := kubeChecker.Run(cmd.Context()); err != nil { return xerrors.Errorf("run kube checker: %w", err) } From cbe2bbbb05db6bc88b8cbddde7042f839d49d4fd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Aug 2021 14:15:04 +0000 Subject: [PATCH 04/15] chore: add api.PassResult convenience function --- internal/api/pass.go | 11 +++++++++++ internal/api/pass_test.go | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 internal/api/pass.go create mode 100644 internal/api/pass_test.go diff --git a/internal/api/pass.go b/internal/api/pass.go new file mode 100644 index 0000000..fb859eb --- /dev/null +++ b/internal/api/pass.go @@ -0,0 +1,11 @@ +package api + +// PassResults returns a CheckResult when everything is OK. +func PassResult(name string, summary string) *CheckResult { + return &CheckResult{ + Name: name, + State: StatePassed, + Summary: summary, + Details: map[string]interface{}{}, + } +} diff --git a/internal/api/pass_test.go b/internal/api/pass_test.go new file mode 100644 index 0000000..a4f06c6 --- /dev/null +++ b/internal/api/pass_test.go @@ -0,0 +1,19 @@ +package api_test + +import ( + "testing" + + "cdr.dev/slog/sloggers/slogtest/assert" + "github.com/cdr/coder-doctor/internal/api" +) + +func TestPassResult(t *testing.T) { + t.Parallel() + + res := api.PassResult("check-name", "check succeeded") + + assert.Equal(t, "name matches", "check-name", res.Name) + assert.Equal(t, "state matches", api.StatePassed, res.State) + assert.Equal(t, "summary matches", "check succeeded", res.Summary) + assert.Equal(t, "error matches", nil, res.Details["error"]) +} From e0a7efd9bd8f6c54ea5df0a085a22f7839094f2a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Aug 2021 14:16:16 +0000 Subject: [PATCH 05/15] add namespace to KubernetesChecker --- internal/checks/kube/kubernetes.go | 14 +++++++++++--- internal/cmd/check/kubernetes/kubernetes.go | 9 ++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/internal/checks/kube/kubernetes.go b/internal/checks/kube/kubernetes.go index 486d817..fc512da 100644 --- a/internal/checks/kube/kubernetes.go +++ b/internal/checks/kube/kubernetes.go @@ -17,6 +17,7 @@ import ( var _ = api.Checker(&KubernetesChecker{}) type KubernetesChecker struct { + namespace string client kubernetes.Interface writer api.ResultWriter coderVersion *semver.Version @@ -27,9 +28,10 @@ type Option func(k *KubernetesChecker) func NewKubernetesChecker(client kubernetes.Interface, opts ...Option) *KubernetesChecker { checker := &KubernetesChecker{ - client: client, - log: slog.Make(sloghuman.Sink(io.Discard)), - writer: &api.DiscardWriter{}, + namespace: "default", + client: client, + log: slog.Make(sloghuman.Sink(io.Discard)), + writer: &api.DiscardWriter{}, // Select the newest version by default coderVersion: semver.MustParse("100.0.0"), } @@ -59,6 +61,12 @@ func WithLogger(log slog.Logger) Option { } } +func WithNamespace(ns string) Option { + return func(k *KubernetesChecker) { + k.namespace = ns + } +} + func (*KubernetesChecker) Validate() error { return nil } diff --git a/internal/cmd/check/kubernetes/kubernetes.go b/internal/cmd/check/kubernetes/kubernetes.go index 533fa95..5048b90 100644 --- a/internal/cmd/check/kubernetes/kubernetes.go +++ b/internal/cmd/check/kubernetes/kubernetes.go @@ -115,11 +115,13 @@ func run(cmd *cobra.Command, _ []string) error { log = log.Leveled(slog.LevelDebug) } + cfgContext := rawConfig.Contexts[rawConfig.CurrentContext] + log.Info(cmd.Context(), "kubernetes config:", slog.F("context", rawConfig.CurrentContext), - slog.F("cluster", rawConfig.Contexts[rawConfig.CurrentContext].Cluster), - slog.F("namespace", rawConfig.Contexts[rawConfig.CurrentContext].Namespace), - slog.F("authinfo", rawConfig.Contexts[rawConfig.CurrentContext].AuthInfo), + slog.F("cluster", cfgContext.Cluster), + slog.F("namespace", cfgContext.Namespace), + slog.F("authinfo", cfgContext.AuthInfo), ) hw := humanwriter.New(os.Stdout) @@ -136,6 +138,7 @@ func run(cmd *cobra.Command, _ []string) error { kube.WithLogger(log), kube.WithCoderVersion(cv), kube.WithWriter(hw), + kube.WithNamespace(cfgContext.Namespace), ) if err := localChecker.Run(cmd.Context()); err != nil { From 70498f91c7fe17a282f81795c76c971e63750ba7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Aug 2021 15:11:31 +0000 Subject: [PATCH 06/15] chore: golangci.yml: fix importas settings for authorizationv1 and authorizationv1client --- .golangci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 3237faa..bdc5c79 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -64,8 +64,10 @@ linters-settings: alias: apps$1 - pkg: k8s.io/client-go/kubernetes/typed/authentication/(v[\w\d]+) alias: authentication$1 - - pkg: k8s.io/client-go/kubernetes/typed/authorization/(v[\w\d]+) + - pkg: k8s.io/api/authorization/(v[\w\d]+) alias: authorization$1 + - pkg: k8s.io/client-go/kubernetes/typed/authorization/(v[\w\d]+) + alias: authorization$1client - pkg: k8s.io/client-go/kubernetes/typed/autoscaling/(v[\w\d]+) alias: autoscaling$1 - pkg: k8s.io/client-go/kubernetes/typed/batch/(v[\w\d]+) From 9083c18b985e90711d01eeff75604f1cb71a36de Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 19 Aug 2021 15:16:13 +0000 Subject: [PATCH 07/15] feat: kubernetes: check RBAC requirements --- go.mod | 1 + internal/checks/kube/kubernetes.go | 6 ++ internal/checks/kube/rbac.go | 95 +++++++++++++++++++++++++++ internal/checks/kube/rbac_test.go | 101 +++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+) create mode 100644 internal/checks/kube/rbac_test.go diff --git a/go.mod b/go.mod index bdf1f6a..3ba1fcd 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/spf13/cobra v1.2.1 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 + k8s.io/api v0.19.14 k8s.io/apimachinery v0.19.14 k8s.io/client-go v0.19.14 k8s.io/klog/v2 v2.10.0 // indirect diff --git a/internal/checks/kube/kubernetes.go b/internal/checks/kube/kubernetes.go index fc512da..32c3a73 100644 --- a/internal/checks/kube/kubernetes.go +++ b/internal/checks/kube/kubernetes.go @@ -76,5 +76,11 @@ func (k *KubernetesChecker) Run(ctx context.Context) error { if err != nil { return xerrors.Errorf("check version: %w", err) } + + for _, res := range k.CheckRBAC(ctx) { + if err := k.writer.WriteResult(res); err != nil { + return xerrors.Errorf("check RBAC: %w", err) + } + } return nil } diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index f84e1e4..90e94e7 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -1 +1,96 @@ package kube + +import ( + "context" + "fmt" + "strings" + + "golang.org/x/xerrors" + + "github.com/cdr/coder-doctor/internal/api" + + authorizationv1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1" //nolint:importas +) + +type RBACRequirement struct { + Resource string + Verbs []string +} + +var VerbsCreateDeleteList = []string{"create", "delete", "list"} + +func NewRBACRequirement(resource string, verbs ...string) *RBACRequirement { + return &RBACRequirement{ + Resource: resource, + Verbs: verbs, + } +} + +var rbacRequirements = []*RBACRequirement{ + NewRBACRequirement("deployments", VerbsCreateDeleteList...), + NewRBACRequirement("serviceaccounts", VerbsCreateDeleteList...), + NewRBACRequirement("replicasets", VerbsCreateDeleteList...), + NewRBACRequirement("pods", VerbsCreateDeleteList...), + NewRBACRequirement("roles", VerbsCreateDeleteList...), + NewRBACRequirement("rolebindings", VerbsCreateDeleteList...), + NewRBACRequirement("ingresses", VerbsCreateDeleteList...), + NewRBACRequirement("secrets", VerbsCreateDeleteList...), + NewRBACRequirement("services", VerbsCreateDeleteList...), + NewRBACRequirement("statefulsets", VerbsCreateDeleteList...), + NewRBACRequirement("flabbagabbas", VerbsCreateDeleteList...), +} + +func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { + const checkName = "kubernetes-rbac" + authClient := k.client.AuthorizationV1() + results := make([]*api.CheckResult, 0, len(rbacRequirements)) + + for _, req := range 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) + results = append(results, api.ErrorResult(resName, summary, err)) + continue + } + + summary := fmt.Sprintf("%s: can %s", req.Resource, strings.Join(req.Verbs, ", ")) + results = append(results, api.PassResult(resName, summary)) + } + + return results +} + +func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authorizationv1client.AuthorizationV1Interface, req *RBACRequirement) error { + have := make([]string, 0, len(req.Verbs)) + for _, verb := range req.Verbs { + sar := &authorizationv1.SelfSubjectAccessReview{ + Spec: authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: k.namespace, + Resource: req.Resource, + Verb: verb, + }, + }, + } + + response, err := authClient.SelfSubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) + + if err != nil { + // should not fail - short-circuit + return xerrors.Errorf("failed to create SelfSubjectAccessReview request: %w", err) + } + + if response.Status.Allowed { + have = append(have, verb) + continue + } + } + + if len(have) != len(req.Verbs) { + return xerrors.Errorf(fmt.Sprintf("need: %+v have: %+v", req.Verbs, have)) + } + + return nil +} diff --git a/internal/checks/kube/rbac_test.go b/internal/checks/kube/rbac_test.go new file mode 100644 index 0000000..bc9d51c --- /dev/null +++ b/internal/checks/kube/rbac_test.go @@ -0,0 +1,101 @@ +package kube + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + authorizationv1 "k8s.io/api/authorization/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + + "cdr.dev/slog/sloggers/slogtest/assert" + + "github.com/cdr/coder-doctor/internal/api" +) + +func Test_CheckRBAC(t *testing.T) { + t.Parallel() + + tests := []struct { + Name string + Response *authorizationv1.SelfSubjectAccessReview + F func(*testing.T, []*api.CheckResult) + }{ + { + Name: "all allowed", + Response: &selfSubjectAccessReviewAllowed, + F: func(t *testing.T, results []*api.CheckResult) { + for _, result := range results { + assert.Success(t, result.Name+" should not error", result.Details["error"].(error)) + assert.True(t, result.Name+" should pass", result.State == api.StatePassed) + } + }, + }, + { + Name: "all denied", + Response: &selfSubjectAccessReviewDenied, + F: func(t *testing.T, results []*api.CheckResult) { + for _, result := range results { + assert.True(t, result.Name+" should have an error", result.Details["error"] != nil) + assert.True(t, result.Name+" should fail", result.State == api.StateFailed) + } + }, + }, + } + + for _, test := range tests { + t.Run(test.Name, func(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.StatusOK) + err := json.NewEncoder(w).Encode(test.Response) + assert.Success(t, "failed to encode response", err) + })) + defer server.Close() + + client, err := kubernetes.NewForConfig(&rest.Config{Host: server.URL}) + assert.Success(t, "failed to create client", err) + + checker := NewKubernetesChecker(client) + results := checker.CheckRBAC(context.Background()) + test.F(t, results) + }) + } +} + +func Test_CheckRBAC_ClientError(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) + results := checker.CheckRBAC(context.Background()) + for _, result := range results { + assert.ErrorContains(t, result.Name+" should show correct error", result.Details["error"].(error), "failed to create SelfSubjectAccessReview request") + assert.True(t, result.Name+" should fail", result.State == api.StateFailed) + } +} + +var selfSubjectAccessReviewAllowed authorizationv1.SelfSubjectAccessReview = authorizationv1.SelfSubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{ + Allowed: true, + }, +} + +var selfSubjectAccessReviewDenied authorizationv1.SelfSubjectAccessReview = authorizationv1.SelfSubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{ + Allowed: false, + }, +} From b0d6593aa0307d41a5e464fdf2e1d9bd4da9c8ea Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 20 Aug 2021 12:51:41 +0100 Subject: [PATCH 08/15] fixup! feat: kubernetes: check RBAC requirements --- internal/checks/kube/rbac.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index 90e94e7..01e887c 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -39,7 +39,6 @@ var rbacRequirements = []*RBACRequirement{ NewRBACRequirement("secrets", VerbsCreateDeleteList...), NewRBACRequirement("services", VerbsCreateDeleteList...), NewRBACRequirement("statefulsets", VerbsCreateDeleteList...), - NewRBACRequirement("flabbagabbas", VerbsCreateDeleteList...), } func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { From 6275eeabf1136e56036eaece86d0ab528c3a8684 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:32:30 +0100 Subject: [PATCH 09/15] internal/checks/kube/rbac_test.go: fix test --- internal/checks/kube/rbac_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/checks/kube/rbac_test.go b/internal/checks/kube/rbac_test.go index bc9d51c..31b9fbd 100644 --- a/internal/checks/kube/rbac_test.go +++ b/internal/checks/kube/rbac_test.go @@ -29,7 +29,7 @@ func Test_CheckRBAC(t *testing.T) { Response: &selfSubjectAccessReviewAllowed, F: func(t *testing.T, results []*api.CheckResult) { for _, result := range results { - assert.Success(t, result.Name+" should not error", result.Details["error"].(error)) + assert.True(t, result.Name+" should not error", result.Details["error"] == nil) assert.True(t, result.Name+" should pass", result.State == api.StatePassed) } }, @@ -47,6 +47,7 @@ func Test_CheckRBAC(t *testing.T) { } for _, test := range tests { + test := test t.Run(test.Name, func(t *testing.T) { t.Parallel() From 1235ba15e787ca663f79c71a825682050923ca52 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:36:56 +0100 Subject: [PATCH 10/15] chore: importas: work around aliasing issue --- .golangci.yml | 6 +++--- internal/checks/kube/rbac.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index bdc5c79..4c30f74 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -54,6 +54,8 @@ linters-settings: alias: - pkg: k8s.io/api/core/(v[\w\d]+) alias: core$1 + - pkg: k8s.io/api/authorization/(v[\w\d]+) + alias: authorization$1 - pkg: k8s.io/apimachinery/pkg/apis/meta/(v[\w\d]+) alias: meta$1 @@ -64,10 +66,8 @@ linters-settings: alias: apps$1 - pkg: k8s.io/client-go/kubernetes/typed/authentication/(v[\w\d]+) alias: authentication$1 - - pkg: k8s.io/api/authorization/(v[\w\d]+) - alias: authorization$1 - pkg: k8s.io/client-go/kubernetes/typed/authorization/(v[\w\d]+) - alias: authorization$1client + alias: authorizationclient$1 - pkg: k8s.io/client-go/kubernetes/typed/autoscaling/(v[\w\d]+) alias: autoscaling$1 - pkg: k8s.io/client-go/kubernetes/typed/batch/(v[\w\d]+) diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index 01e887c..35ae1ea 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -11,7 +11,7 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1" //nolint:importas + authorizationclientv1 "k8s.io/client-go/kubernetes/typed/authorization/v1" ) type RBACRequirement struct { @@ -61,7 +61,7 @@ func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { return results } -func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authorizationv1client.AuthorizationV1Interface, req *RBACRequirement) error { +func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authorizationclientv1.AuthorizationV1Interface, req *RBACRequirement) error { have := make([]string, 0, len(req.Verbs)) for _, verb := range req.Verbs { sar := &authorizationv1.SelfSubjectAccessReview{ From c74ab29274892a9580c70ff19129e9e6bea464be Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:37:31 +0100 Subject: [PATCH 11/15] internal/checks/kube/rbac.go: add APIGroup to requirements --- internal/checks/kube/rbac.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index 35ae1ea..e30752d 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -15,30 +15,32 @@ import ( ) type RBACRequirement struct { + APIGroup string Resource string Verbs []string } var VerbsCreateDeleteList = []string{"create", "delete", "list"} -func NewRBACRequirement(resource string, verbs ...string) *RBACRequirement { +func NewRBACRequirement(apiGroup, resource string, verbs ...string) *RBACRequirement { return &RBACRequirement{ + APIGroup: apiGroup, Resource: resource, Verbs: verbs, } } var rbacRequirements = []*RBACRequirement{ - NewRBACRequirement("deployments", VerbsCreateDeleteList...), - NewRBACRequirement("serviceaccounts", VerbsCreateDeleteList...), - NewRBACRequirement("replicasets", VerbsCreateDeleteList...), - NewRBACRequirement("pods", VerbsCreateDeleteList...), - NewRBACRequirement("roles", VerbsCreateDeleteList...), - NewRBACRequirement("rolebindings", VerbsCreateDeleteList...), - NewRBACRequirement("ingresses", VerbsCreateDeleteList...), - NewRBACRequirement("secrets", VerbsCreateDeleteList...), - NewRBACRequirement("services", VerbsCreateDeleteList...), - NewRBACRequirement("statefulsets", VerbsCreateDeleteList...), + NewRBACRequirement("", "pods", VerbsCreateDeleteList...), + NewRBACRequirement("", "roles", VerbsCreateDeleteList...), + NewRBACRequirement("", "rolebindings", VerbsCreateDeleteList...), + NewRBACRequirement("", "secrets", VerbsCreateDeleteList...), + NewRBACRequirement("", "serviceaccounts", VerbsCreateDeleteList...), + NewRBACRequirement("", "services", VerbsCreateDeleteList...), + NewRBACRequirement("apps", "deployments", VerbsCreateDeleteList...), + NewRBACRequirement("apps", "replicasets", VerbsCreateDeleteList...), + NewRBACRequirement("apps", "statefulsets", VerbsCreateDeleteList...), + NewRBACRequirement("extensions", "ingresses", VerbsCreateDeleteList...), } func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { @@ -68,6 +70,7 @@ func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authori Spec: authorizationv1.SelfSubjectAccessReviewSpec{ ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: k.namespace, + Group: req.APIGroup, Resource: req.Resource, Verb: verb, }, From ed0f873cf9473f40e18c1b7cfd54cedacca518f1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:38:05 +0100 Subject: [PATCH 12/15] internal/cmd/check/kubernetes/kubernetes.go: noop: rename variable --- internal/cmd/check/kubernetes/kubernetes.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/cmd/check/kubernetes/kubernetes.go b/internal/cmd/check/kubernetes/kubernetes.go index 5048b90..7f9323a 100644 --- a/internal/cmd/check/kubernetes/kubernetes.go +++ b/internal/cmd/check/kubernetes/kubernetes.go @@ -115,13 +115,13 @@ func run(cmd *cobra.Command, _ []string) error { log = log.Leveled(slog.LevelDebug) } - cfgContext := rawConfig.Contexts[rawConfig.CurrentContext] + currentContext := rawConfig.Contexts[rawConfig.CurrentContext] log.Info(cmd.Context(), "kubernetes config:", slog.F("context", rawConfig.CurrentContext), - slog.F("cluster", cfgContext.Cluster), - slog.F("namespace", cfgContext.Namespace), - slog.F("authinfo", cfgContext.AuthInfo), + slog.F("cluster", currentContext.Cluster), + slog.F("namespace", currentContext.Namespace), + slog.F("authinfo", currentContext.AuthInfo), ) hw := humanwriter.New(os.Stdout) @@ -138,7 +138,7 @@ func run(cmd *cobra.Command, _ []string) error { kube.WithLogger(log), kube.WithCoderVersion(cv), kube.WithWriter(hw), - kube.WithNamespace(cfgContext.Namespace), + kube.WithNamespace(currentContext.Namespace), ) if err := localChecker.Run(cmd.Context()); err != nil { From 6592f5592a5ade18de1d1a03298dd759844f147e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:39:35 +0100 Subject: [PATCH 13/15] internal/checks/kube/rbac.go: rename variable --- internal/checks/kube/rbac.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index e30752d..66d6349 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -20,7 +20,7 @@ type RBACRequirement struct { Verbs []string } -var VerbsCreateDeleteList = []string{"create", "delete", "list"} +var verbsCreateDeleteList = []string{"create", "delete", "list"} func NewRBACRequirement(apiGroup, resource string, verbs ...string) *RBACRequirement { return &RBACRequirement{ @@ -31,16 +31,16 @@ func NewRBACRequirement(apiGroup, resource string, verbs ...string) *RBACRequire } var rbacRequirements = []*RBACRequirement{ - NewRBACRequirement("", "pods", VerbsCreateDeleteList...), - NewRBACRequirement("", "roles", VerbsCreateDeleteList...), - NewRBACRequirement("", "rolebindings", VerbsCreateDeleteList...), - NewRBACRequirement("", "secrets", VerbsCreateDeleteList...), - NewRBACRequirement("", "serviceaccounts", VerbsCreateDeleteList...), - NewRBACRequirement("", "services", VerbsCreateDeleteList...), - NewRBACRequirement("apps", "deployments", VerbsCreateDeleteList...), - NewRBACRequirement("apps", "replicasets", VerbsCreateDeleteList...), - NewRBACRequirement("apps", "statefulsets", VerbsCreateDeleteList...), - NewRBACRequirement("extensions", "ingresses", VerbsCreateDeleteList...), + NewRBACRequirement("", "pods", verbsCreateDeleteList...), + NewRBACRequirement("", "roles", verbsCreateDeleteList...), + NewRBACRequirement("", "rolebindings", verbsCreateDeleteList...), + NewRBACRequirement("", "secrets", verbsCreateDeleteList...), + NewRBACRequirement("", "serviceaccounts", verbsCreateDeleteList...), + NewRBACRequirement("", "services", verbsCreateDeleteList...), + NewRBACRequirement("apps", "deployments", verbsCreateDeleteList...), + NewRBACRequirement("apps", "replicasets", verbsCreateDeleteList...), + NewRBACRequirement("apps", "statefulsets", verbsCreateDeleteList...), + NewRBACRequirement("extensions", "ingresses", verbsCreateDeleteList...), } func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { From de04989534a5781a1ec096e665879c797c2c8219 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 10:58:39 +0100 Subject: [PATCH 14/15] internal/checks/kube/rbac.go: support different RBAC requirements per coder version --- internal/checks/kube/rbac.go | 58 ++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index 66d6349..96f8e8a 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -7,6 +7,8 @@ import ( "golang.org/x/xerrors" + "github.com/Masterminds/semver/v3" + "github.com/cdr/coder-doctor/internal/api" authorizationv1 "k8s.io/api/authorization/v1" @@ -20,6 +22,11 @@ type RBACRequirement struct { Verbs []string } +type VersionedRBACRequirements struct { + VersionConstraints *semver.Constraints + RBACRequirements []*RBACRequirement +} + var verbsCreateDeleteList = []string{"create", "delete", "list"} func NewRBACRequirement(apiGroup, resource string, verbs ...string) *RBACRequirement { @@ -30,25 +37,41 @@ func NewRBACRequirement(apiGroup, resource string, verbs ...string) *RBACRequire } } -var rbacRequirements = []*RBACRequirement{ - NewRBACRequirement("", "pods", verbsCreateDeleteList...), - NewRBACRequirement("", "roles", verbsCreateDeleteList...), - NewRBACRequirement("", "rolebindings", verbsCreateDeleteList...), - NewRBACRequirement("", "secrets", verbsCreateDeleteList...), - NewRBACRequirement("", "serviceaccounts", verbsCreateDeleteList...), - NewRBACRequirement("", "services", verbsCreateDeleteList...), - NewRBACRequirement("apps", "deployments", verbsCreateDeleteList...), - NewRBACRequirement("apps", "replicasets", verbsCreateDeleteList...), - NewRBACRequirement("apps", "statefulsets", verbsCreateDeleteList...), - NewRBACRequirement("extensions", "ingresses", verbsCreateDeleteList...), +var allVersionedRBACRequirements = []VersionedRBACRequirements{ + { + VersionConstraints: api.MustConstraint(">= 1.20"), + RBACRequirements: []*RBACRequirement{ + NewRBACRequirement("", "pods", verbsCreateDeleteList...), + NewRBACRequirement("", "roles", verbsCreateDeleteList...), + NewRBACRequirement("", "rolebindings", verbsCreateDeleteList...), + NewRBACRequirement("", "secrets", verbsCreateDeleteList...), + NewRBACRequirement("", "serviceaccounts", verbsCreateDeleteList...), + NewRBACRequirement("", "services", verbsCreateDeleteList...), + NewRBACRequirement("apps", "deployments", verbsCreateDeleteList...), + NewRBACRequirement("apps", "replicasets", verbsCreateDeleteList...), + NewRBACRequirement("apps", "statefulsets", verbsCreateDeleteList...), + NewRBACRequirement("extensions", "ingresses", verbsCreateDeleteList...), + }, + }, } func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { const checkName = "kubernetes-rbac" authClient := k.client.AuthorizationV1() - results := make([]*api.CheckResult, 0, len(rbacRequirements)) + 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 rbacRequirements { + for _, req := range rbacReqs.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) @@ -96,3 +119,12 @@ func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authori return nil } + +func findClosestVersionRequirements(v *semver.Version) *VersionedRBACRequirements { + for _, vreqs := range allVersionedRBACRequirements { + if vreqs.VersionConstraints.Check(v) { + return &vreqs + } + } + return nil +} From e57e0cce9211dbd889cbcad5662bdfe6df00a8e5 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Aug 2021 11:01:45 +0100 Subject: [PATCH 15/15] internal/checks/kube/rbac_test.go: add test for unhandled version --- internal/checks/kube/rbac_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/checks/kube/rbac_test.go b/internal/checks/kube/rbac_test.go index 31b9fbd..d28c778 100644 --- a/internal/checks/kube/rbac_test.go +++ b/internal/checks/kube/rbac_test.go @@ -13,6 +13,8 @@ import ( "cdr.dev/slog/sloggers/slogtest/assert" + "github.com/Masterminds/semver/v3" + "github.com/cdr/coder-doctor/internal/api" ) @@ -89,6 +91,27 @@ 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,