From 9834667485256440515604a85eb058d931475b3c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 Aug 2021 10:00:39 +0000 Subject: [PATCH 1/2] internal/checks/kube/resources.go: skip instead of fail if api-resources fails --- internal/checks/kube/resources.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/checks/kube/resources.go b/internal/checks/kube/resources.go index 6aa583f..11edd8b 100644 --- a/internal/checks/kube/resources.go +++ b/internal/checks/kube/resources.go @@ -16,7 +16,7 @@ func (k *KubernetesChecker) CheckResources(_ context.Context) []*api.CheckResult dc := k.client.Discovery() lists, err := dc.ServerPreferredResources() if err != nil { - results = append(results, api.ErrorResult(checkName, "unable to fetch api resources from server", err)) + results = append(results, api.SkippedResult(checkName, "unable to fetch api resources from server: "+err.Error())) return results } From 468ee3b8b12b441f7f5bd48d326bf2052f0ef426 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 Aug 2021 14:11:19 +0000 Subject: [PATCH 2/2] internal/checks/kube: update resource requirements from current helm chart --- internal/checks/kube/kubernetes.go | 16 ++-- internal/checks/kube/rbac.go | 20 ++++- internal/checks/kube/resources.go | 3 +- internal/checks/kube/resources_list.go | 62 ++++++++++--- internal/checks/kube/resources_test.go | 117 +++++++++---------------- 5 files changed, 119 insertions(+), 99 deletions(-) diff --git a/internal/checks/kube/kubernetes.go b/internal/checks/kube/kubernetes.go index 669e9a4..7da135d 100644 --- a/internal/checks/kube/kubernetes.go +++ b/internal/checks/kube/kubernetes.go @@ -17,12 +17,12 @@ import ( var _ = api.Checker(&KubernetesChecker{}) type KubernetesChecker struct { - namespace string - client kubernetes.Interface - writer api.ResultWriter - coderVersion *semver.Version - log slog.Logger - rbacRequirements map[*ResourceRequirement]ResourceVerbs + namespace string + client kubernetes.Interface + writer api.ResultWriter + coderVersion *semver.Version + log slog.Logger + reqs *VersionedResourceRequirements } type Option func(k *KubernetesChecker) @@ -41,7 +41,7 @@ func NewKubernetesChecker(client kubernetes.Interface, opts ...Option) *Kubernet opt(checker) } - checker.rbacRequirements = findClosestVersionRequirements(checker.coderVersion) + checker.reqs = findClosestVersionRequirements(checker.coderVersion) if err := checker.Validate(); err != nil { panic(xerrors.Errorf("error validating kube checker: %w", err)) @@ -75,7 +75,7 @@ func WithNamespace(ns string) Option { } func (k *KubernetesChecker) Validate() error { - if k.rbacRequirements == nil { + if k.reqs == nil { return xerrors.Errorf("unhandled coder version: %s", k.coderVersion.String()) } return nil diff --git a/internal/checks/kube/rbac.go b/internal/checks/kube/rbac.go index 811c125..173637f 100644 --- a/internal/checks/kube/rbac.go +++ b/internal/checks/kube/rbac.go @@ -21,7 +21,21 @@ func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult { authClient := k.client.AuthorizationV1() results := make([]*api.CheckResult, 0) - for req, reqVerbs := range k.rbacRequirements { + for req, reqVerbs := range k.reqs.ResourceRequirements { + resName := fmt.Sprintf("%s-%s", checkName, req.Resource) + if err := k.checkOneRBAC(ctx, authClient, req, reqVerbs); 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(reqVerbs, ", ")) + results = append(results, api.PassResult(resName, summary)) + } + + // TODO: delete this when the enterprise-helm role no longer requests resources on things + // that don't exist. + for req, reqVerbs := range k.reqs.RoleOnlyResourceRequirements { resName := fmt.Sprintf("%s-%s", checkName, req.Resource) if err := k.checkOneRBAC(ctx, authClient, req, reqVerbs); err != nil { summary := fmt.Sprintf("missing permissions on resource %s: %s", req.Resource, err) @@ -70,10 +84,10 @@ func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authori return nil } -func findClosestVersionRequirements(v *semver.Version) map[*ResourceRequirement]ResourceVerbs { +func findClosestVersionRequirements(v *semver.Version) *VersionedResourceRequirements { for _, vreqs := range allRequirements { if vreqs.VersionConstraints.Check(v) { - return vreqs.ResourceRequirements + return &vreqs } } return nil diff --git a/internal/checks/kube/resources.go b/internal/checks/kube/resources.go index 11edd8b..f69b52d 100644 --- a/internal/checks/kube/resources.go +++ b/internal/checks/kube/resources.go @@ -45,8 +45,7 @@ func (k *KubernetesChecker) CheckResources(_ context.Context) []*api.CheckResult } } - versionReqs := findClosestVersionRequirements(k.coderVersion) - for versionReq := range versionReqs { + for versionReq := range k.reqs.ResourceRequirements { if !resourcesAvailable[*versionReq] { msg := fmt.Sprintf("missing required resource:%q group:%q version:%q", versionReq.Resource, versionReq.Group, versionReq.Version) errResult := api.ErrorResult(checkName, msg, xerrors.New(msg)) diff --git a/internal/checks/kube/resources_list.go b/internal/checks/kube/resources_list.go index a7d7d75..739729d 100644 --- a/internal/checks/kube/resources_list.go +++ b/internal/checks/kube/resources_list.go @@ -6,20 +6,53 @@ import ( "github.com/cdr/coder-doctor/internal/api" ) +// This is a list of all known Resource and/or RBAC requirements for each verison of Coder. +// Order by version DESCENDING. var allRequirements = []VersionedResourceRequirements{ { VersionConstraints: api.MustConstraint(">= 1.20"), ResourceRequirements: map[*ResourceRequirement]ResourceVerbs{ - NewResourceRequirement("", "v1", "pods"): verbsCreateDeleteList, - NewResourceRequirement("", "v1", "secrets"): verbsCreateDeleteList, - NewResourceRequirement("", "v1", "serviceaccounts"): verbsCreateDeleteList, - NewResourceRequirement("", "v1", "services"): verbsCreateDeleteList, - NewResourceRequirement("apps", "apps/v1", "deployments"): verbsCreateDeleteList, - NewResourceRequirement("apps", "apps/v1", "replicasets"): verbsCreateDeleteList, - NewResourceRequirement("apps", "apps/v1", "statefulsets"): verbsCreateDeleteList, - NewResourceRequirement("networking.k8s.io", "networking.k8s.io/v1", "ingresses"): verbsCreateDeleteList, - NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "roles"): verbsCreateDeleteList, - NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "rolebindings"): verbsCreateDeleteList, + NewResourceRequirement("", "v1", "events"): verbsAll, + NewResourceRequirement("", "v1", "persistentvolumeclaims"): verbsAll, + NewResourceRequirement("", "v1", "pods"): verbsAll, + NewResourceRequirement("", "v1", "secrets"): verbsAll, + NewResourceRequirement("", "v1", "serviceaccounts"): verbsAll, + NewResourceRequirement("", "v1", "services"): verbsAll, + NewResourceRequirement("apps", "apps/v1", "deployments"): verbsAll, + NewResourceRequirement("apps", "apps/v1", "replicasets"): verbsAll, + NewResourceRequirement("apps", "apps/v1", "statefulsets"): verbsAll, + NewResourceRequirement("metrics.k8s.io", "metrics.k8s.io/v1beta1", "pods"): verbsGetListWatch, + NewResourceRequirement("networking.k8s.io", "networking.k8s.io/v1", "ingresses"): verbsAll, + NewResourceRequirement("networking.k8s.io", "networking.k8s.io/v1", "networkpolicies"): verbsAll, + NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "roles"): verbsGetCreate, + NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "rolebindings"): verbsGetCreate, + NewResourceRequirement("storage.k8s.io", "storage.k8s.io/v1", "storageclasses"): verbsGetListWatch, + }, + RoleOnlyResourceRequirements: map[*ResourceRequirement]ResourceVerbs{ + // The below permissions are required by the default coder role created by the Helm chart. + // Installation will fail if these are not present in the role being used to install Coder. + NewResourceRequirement("", "v1", "deployments"): verbsAll, + NewResourceRequirement("", "v1", "networkpolicies"): verbsAll, + NewResourceRequirement("", "v1", "pods/exec"): verbsAll, + NewResourceRequirement("", "v1", "pods/log"): verbsAll, + NewResourceRequirement("apps", "v1", "events"): verbsAll, + NewResourceRequirement("apps", "v1", "networkpolicies"): verbsAll, + NewResourceRequirement("apps", "v1", "persistentvolumeclaims"): verbsAll, + NewResourceRequirement("apps", "v1", "pods"): verbsAll, + NewResourceRequirement("apps", "v1", "pods/exec"): verbsAll, + NewResourceRequirement("apps", "v1", "pods/log"): verbsAll, + NewResourceRequirement("apps", "v1", "secrets"): verbsAll, + NewResourceRequirement("apps", "v1", "services"): verbsAll, + NewResourceRequirement("metrics.k8s.io", "v1beta1", "storageclasses"): verbsGetListWatch, + NewResourceRequirement("networking.k8s.io", "v1", "deployments"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "events"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "persistentvolumeclaims"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "pods"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "pods/exec"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "pods/log"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "secrets"): verbsAll, + NewResourceRequirement("networking.k8s.io", "v1", "services"): verbsAll, + NewResourceRequirement("storage.k8s.io", "v1", "pods"): verbsGetListWatch, }, }, } @@ -39,11 +72,16 @@ type ResourceVerbs []string type VersionedResourceRequirements struct { VersionConstraints *semver.Constraints ResourceRequirements map[*ResourceRequirement]ResourceVerbs + // These are only required because the role in the Helm chart specifies broad swathes of permissions that + // don't necessarily exist in the real world. + RoleOnlyResourceRequirements map[*ResourceRequirement]ResourceVerbs } -var verbsCreateDeleteList ResourceVerbs = []string{"create", "delete", "list"} +var verbsAll ResourceVerbs = []string{"create", "delete", "deletecollection", "get", "list", "update", "patch", "watch"} +var verbsGetListWatch = []string{"get", "list", "watch"} +var verbsGetCreate = []string{"get", "create"} -// NewResourceRequirement is just a convenience function for creating ResourceRequirements. +// NewResourceRequirement is just a convenience function for creating ResourceRequirements for which the resource type must exist. func NewResourceRequirement(apiGroup, version, resource string) *ResourceRequirement { return &ResourceRequirement{ Group: apiGroup, diff --git a/internal/checks/kube/resources_test.go b/internal/checks/kube/resources_test.go index 4a5f9eb..ec539dc 100644 --- a/internal/checks/kube/resources_test.go +++ b/internal/checks/kube/resources_test.go @@ -69,6 +69,10 @@ func Test_KubernetesChecker_CheckResources(t *testing.T) { w.WriteHeader(http.StatusOK) err := json.NewEncoder(w).Encode(appsV1ResourceList) assert.Success(t, "failed to encode response", err) + case "/apis/metrics.k8s.io/v1": + w.WriteHeader(http.StatusOK) + err := json.NewEncoder(w).Encode(metricsV1ResourceLilst) + assert.Success(t, "failed to encode response", err) case "/apis/networking.k8s.io/v1": w.WriteHeader(http.StatusOK) err := json.NewEncoder(w).Encode(networkingV1ResourceList) @@ -77,6 +81,10 @@ func Test_KubernetesChecker_CheckResources(t *testing.T) { w.WriteHeader(http.StatusOK) err := json.NewEncoder(w).Encode(rbacV1ResourceList) assert.Success(t, "failed to encode response", err) + case "/apis/storage.k8s.io/v1": + w.WriteHeader(http.StatusOK) + err := json.NewEncoder(w).Encode(storageV1ResourceList) + assert.Success(t, "failed to encode response", err) default: w.WriteHeader(http.StatusNotFound) } @@ -124,6 +132,15 @@ var fullAPIGroupList *metav1.APIGroupList = &metav1.APIGroupList{ }, }, }, + { + Name: "metrics.k8s.io", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "metrics.k8s.io/v1beta1", + Version: "v1", + }, + }, + }, { Name: "networking.k8s.io", Versions: []metav1.GroupVersionForDiscovery{ @@ -142,84 +159,36 @@ var fullAPIGroupList *metav1.APIGroupList = &metav1.APIGroupList{ }, }, }, - }, -} - -var v1ResourceList = &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{ - Kind: "APIResourceList", - }, - GroupVersion: "v1", - APIResources: []metav1.APIResource{ - { - Name: "pods", - Verbs: []string{"get"}, - }, - { - Name: "secrets", - Verbs: []string{"get"}, - }, - { - Name: "serviceaccounts", - Verbs: []string{"get"}, - }, - { - Name: "services", - Verbs: []string{"get"}, - }, - }, -} - -var appsV1ResourceList = &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{ - Kind: "APIResourceList", - APIVersion: "v1", - }, - GroupVersion: "apps/v1", - APIResources: []metav1.APIResource{ - { - Name: "deployments", - Verbs: []string{"get"}, - }, { - Name: "replicasets", - Verbs: []string{"get"}, - }, - { - Name: "statefulsets", - Verbs: []string{"get"}, + Name: "storage.k8s.io", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "storage.k8s.io/v1", + Version: "v1", + }, + }, }, }, } -var networkingV1ResourceList = &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{ - Kind: "APIResourceList", - APIVersion: "v1", - }, - GroupVersion: "networking.k8s.io/v1", - APIResources: []metav1.APIResource{ - { - Name: "ingresses", - Verbs: []string{"get"}, - }, - }, -} +var v1ResourceList = makeResourceList("", "v1", "persistentvolumes", "persistentvolumeclaims", "events", "pods", "secrets", "serviceaccounts", "services") +var appsV1ResourceList = makeResourceList("v1", "apps/v1", "deployments", "replicasets", "statefulsets") +var metricsV1ResourceLilst = makeResourceList("v1", "metrics.k8s.io/v1beta1", "pods") +var networkingV1ResourceList = makeResourceList("v1", "networking.k8s.io/v1", "ingresses", "networkpolicies") +var rbacV1ResourceList = makeResourceList("v1", "rbac.authorization.k8s.io/v1", "roles", "rolebindings") +var storageV1ResourceList = makeResourceList("v1", "storage.k8s.io/v1", "roles", "storageclasses") -var rbacV1ResourceList = &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{ - Kind: "APIResourceList", - APIVersion: "v1", - }, - GroupVersion: "rbac.authorization.k8s.io/v1", - APIResources: []metav1.APIResource{ - { - Name: "roles", - Verbs: []string{"get"}, - }, - { - Name: "rolebindings", - Verbs: []string{"get"}, - }, - }, +func makeResourceList(version, groupVersion string, resources ...string) *metav1.APIResourceList { + rl := &metav1.APIResourceList{ + TypeMeta: metav1.TypeMeta{ + Kind: "APIResourceList", + APIVersion: version, + }, + GroupVersion: groupVersion, + APIResources: []metav1.APIResource{}, + } + for _, resource := range resources { + rl.APIResources = append(rl.APIResources, metav1.APIResource{Name: resource, Verbs: []string{"get"}}) + } + return rl }