From 55f2ed44bfec43efb981830b2e29c7d9ad7c378d Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Tue, 17 Dec 2019 08:13:04 -0800 Subject: [PATCH] Check RBAC before running collectors --- cmd/preflight/cli/root.go | 1 + cmd/preflight/cli/run.go | 89 +++++-- cmd/preflight/cli/upload_results.go | 46 +++- cmd/troubleshoot/cli/root.go | 1 + cmd/troubleshoot/cli/run.go | 56 +++-- .../troubleshoot/v1beta1/collector_shared.go | 227 ++++++++++++++++++ pkg/collect/collector.go | 99 ++++---- pkg/collect/logs.go | 7 +- pkg/collect/rbac.go | 26 ++ pkg/collect/run.go | 36 ++- 10 files changed, 481 insertions(+), 107 deletions(-) create mode 100644 pkg/collect/rbac.go diff --git a/cmd/preflight/cli/root.go b/cmd/preflight/cli/root.go index 88178c7c..555d3b83 100644 --- a/cmd/preflight/cli/root.go +++ b/cmd/preflight/cli/root.go @@ -42,6 +42,7 @@ that a cluster meets the requirements to run an application.`, cmd.Flags().String("pullpolicy", "", "the pull policy of the preflight image") cmd.Flags().String("collector-image", "", "the full name of the collector image to use") cmd.Flags().String("collector-pullpolicy", "", "the pull policy of the collector image") + cmd.Flags().Bool("collect-without-permissions", false, "always run preflight checks even if some require permissions that preflight does not have") cmd.Flags().String("serviceaccount", "", "name of the service account to use. if not provided, one will be created") diff --git a/cmd/preflight/cli/run.go b/cmd/preflight/cli/run.go index ae624a5e..d21bdf21 100644 --- a/cmd/preflight/cli/run.go +++ b/cmd/preflight/cli/run.go @@ -12,6 +12,7 @@ import ( "time" "github.com/ahmetalpbalkan/go-cursor" + "github.com/fatih/color" "github.com/pkg/errors" analyzerunner "github.com/replicatedhq/troubleshoot/pkg/analyze" troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" @@ -65,22 +66,35 @@ func runPreflights(v *viper.Viper, arg string) error { s := spin.New() finishedCh := make(chan bool, 1) + progressChan := make(chan interface{}, 0) // non-zero buffer will result in missed messages go func() { for { select { - case <-finishedCh: - fmt.Printf("\r") - return + case msg, ok := <-progressChan: + if !ok { + continue + } + switch msg := msg.(type) { + case error: + c := color.New(color.FgHiRed) + c.Println(fmt.Sprintf("%s\r * %v", cursor.ClearEntireLine(), msg)) + case string: + c := color.New(color.FgCyan) + c.Println(fmt.Sprintf("%s\r * %s", cursor.ClearEntireLine(), msg)) + } case <-time.After(time.Millisecond * 100): fmt.Printf("\r \033[36mRunning Preflight checks\033[m %s ", s.Next()) + case <-finishedCh: + fmt.Printf("\r%s\r", cursor.ClearEntireLine()) + return } } }() defer func() { - finishedCh <- true + close(finishedCh) }() - allCollectedData, err := runCollectors(v, preflight) + allCollectedData, err := runCollectors(v, preflight, progressChan) if err != nil { return err } @@ -117,25 +131,30 @@ func runPreflights(v *viper.Viper, arg string) error { } } + if preflight.Spec.UploadResultsTo != "" { + err := uploadResults(preflight.Spec.UploadResultsTo, analyzeResults) + if err != nil { + progressChan <- err + } + } + finishedCh <- true - if preflight.Spec.UploadResultsTo != "" { - tryUploadResults(preflight.Spec.UploadResultsTo, preflight.Name, analyzeResults) - } if v.GetBool("interactive") { + if len(analyzeResults) == 0 { + return errors.New("no data has been collected") + } return showInteractiveResults(preflight.Name, analyzeResults) } return showStdoutResults(v.GetString("format"), preflight.Name, analyzeResults) } -func runCollectors(v *viper.Viper, preflight troubleshootv1beta1.Preflight) (map[string][]byte, error) { - desiredCollectors := make([]*troubleshootv1beta1.Collect, 0, 0) - for _, definedCollector := range preflight.Spec.Collectors { - desiredCollectors = append(desiredCollectors, definedCollector) - } - desiredCollectors = ensureCollectorInList(desiredCollectors, troubleshootv1beta1.Collect{ClusterInfo: &troubleshootv1beta1.ClusterInfo{}}) - desiredCollectors = ensureCollectorInList(desiredCollectors, troubleshootv1beta1.Collect{ClusterResources: &troubleshootv1beta1.ClusterResources{}}) +func runCollectors(v *viper.Viper, preflight troubleshootv1beta1.Preflight, progressChan chan interface{}) (map[string][]byte, error) { + collectSpecs := make([]*troubleshootv1beta1.Collect, 0, 0) + collectSpecs = append(collectSpecs, preflight.Spec.Collectors...) + collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterInfo: &troubleshootv1beta1.ClusterInfo{}}) + collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterResources: &troubleshootv1beta1.ClusterResources{}}) allCollectedData := make(map[string][]byte) @@ -144,24 +163,56 @@ func runCollectors(v *viper.Viper, preflight troubleshootv1beta1.Preflight) (map return nil, errors.Wrap(err, "failed to convert kube flags to rest config") } - // Run preflights collectors synchronously - for _, desiredCollector := range desiredCollectors { + var collectors collect.Collectors + for _, desiredCollector := range collectSpecs { collector := collect.Collector{ Redact: true, Collect: desiredCollector, ClientConfig: config, Namespace: v.GetString("namespace"), } + collectors = append(collectors, &collector) + } + + if err := collectors.CheckRBAC(); err != nil { + return nil, errors.Wrap(err, "failed to check RBAC for collectors") + } + + foundForbidden := false + for _, c := range collectors { + for _, e := range c.RBACErrors { + foundForbidden = true + progressChan <- e + } + } + + if foundForbidden && !v.GetBool("collect-without-permissions") { + if preflight.Spec.UploadResultsTo != "" { + err := uploadErrors(preflight.Spec.UploadResultsTo, collectors) + if err != nil { + progressChan <- err + } + } + return nil, errors.New("insufficient permissions to run all collectors") + } + + // Run preflights collectors synchronously + for _, collector := range collectors { + if len(collector.RBACErrors) > 0 { + continue + } result, err := collector.RunCollectorSync() if err != nil { - return nil, errors.Wrap(err, "failed to run collector") + progressChan <- errors.Errorf("failed to run collector %s: %v\n", collector.GetDisplayName(), err) + continue } if result != nil { output, err := parseCollectorOutput(string(result)) if err != nil { - return nil, errors.Wrap(err, "failed to parse collector output") + progressChan <- errors.Errorf("failed to parse collector output %s: %v\n", collector.GetDisplayName(), err) + continue } for k, v := range output { allCollectedData[k] = v diff --git a/cmd/preflight/cli/upload_results.go b/cmd/preflight/cli/upload_results.go index bca88e0d..7168c7cf 100644 --- a/cmd/preflight/cli/upload_results.go +++ b/cmd/preflight/cli/upload_results.go @@ -5,7 +5,9 @@ import ( "encoding/json" "net/http" + "github.com/pkg/errors" analyzerunner "github.com/replicatedhq/troubleshoot/pkg/analyze" + "github.com/replicatedhq/troubleshoot/pkg/collect" ) type UploadPreflightResult struct { @@ -18,12 +20,17 @@ type UploadPreflightResult struct { URI string `json:"uri,omitempty"` } -type UploadPreflightResults struct { - Results []*UploadPreflightResult `json:"results"` +type UploadPreflightError struct { + Error string `json:"error"` } -func tryUploadResults(uri string, preflightName string, analyzeResults []*analyzerunner.AnalyzeResult) error { - uploadPreflightResults := UploadPreflightResults{ +type UploadPreflightResults struct { + Results []*UploadPreflightResult `json:"results,omitempty"` + Errors []*UploadPreflightError `json:"errors,omitempty"` +} + +func uploadResults(uri string, analyzeResults []*analyzerunner.AnalyzeResult) error { + uploadPreflightResults := &UploadPreflightResults{ Results: []*UploadPreflightResult{}, } for _, analyzeResult := range analyzeResults { @@ -39,14 +46,35 @@ func tryUploadResults(uri string, preflightName string, analyzeResults []*analyz uploadPreflightResults.Results = append(uploadPreflightResults.Results, uploadPreflightResult) } - b, err := json.Marshal(uploadPreflightResults) + return upload(uri, uploadPreflightResults) +} + +func uploadErrors(uri string, collectors collect.Collectors) error { + errors := []*UploadPreflightError{} + for _, collector := range collectors { + for _, e := range collector.RBACErrors { + errors = append(errors, &UploadPreflightError{ + Error: e.Error(), + }) + } + } + + results := &UploadPreflightResults{ + Errors: errors, + } + + return upload(uri, results) +} + +func upload(uri string, payload *UploadPreflightResults) error { + b, err := json.Marshal(payload) if err != nil { - return err + return errors.Wrap(err, "failed to marshal payload") } req, err := http.NewRequest("POST", uri, bytes.NewBuffer(b)) if err != nil { - return err + return errors.Wrap(err, "failed to create request") } req.Header.Set("Content-Type", "application/json") @@ -54,11 +82,11 @@ func tryUploadResults(uri string, preflightName string, analyzeResults []*analyz client := http.DefaultClient resp, err := client.Do(req) if err != nil { - return err + return errors.Wrap(err, "failed to execute request") } if resp.StatusCode > 290 { - return err + return errors.Errorf("unexpected status code: %d", resp.StatusCode) } return nil diff --git a/cmd/troubleshoot/cli/root.go b/cmd/troubleshoot/cli/root.go index 13cc9401..d2220ac1 100644 --- a/cmd/troubleshoot/cli/root.go +++ b/cmd/troubleshoot/cli/root.go @@ -43,6 +43,7 @@ from a server that can be used to assist when troubleshooting a server.`, cmd.Flags().String("image", "", "the full name of the collector image to use") cmd.Flags().String("pullpolicy", "", "the pull policy of the collector image") cmd.Flags().Bool("redact", true, "enable/disable default redactions") + cmd.Flags().Bool("collect-without-permissions", false, "always run troubleshoot collectors even if some require permissions that troubleshoot does not have") cmd.Flags().String("serviceaccount", "", "name of the service account to use. if not provided, one will be created") viper.BindPFlags(cmd.Flags()) diff --git a/cmd/troubleshoot/cli/run.go b/cmd/troubleshoot/cli/run.go index 05a13720..4bbf5fce 100644 --- a/cmd/troubleshoot/cli/run.go +++ b/cmd/troubleshoot/cli/run.go @@ -11,15 +11,16 @@ import ( "path/filepath" "time" - "github.com/ahmetalpbalkan/go-cursor" + cursor "github.com/ahmetalpbalkan/go-cursor" "github.com/fatih/color" "github.com/mholt/archiver" "github.com/pkg/errors" + "github.com/spf13/viper" + spin "github.com/tj/go-spin" + "gopkg.in/yaml.v2" + troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" "github.com/replicatedhq/troubleshoot/pkg/collect" - "github.com/spf13/viper" - "github.com/tj/go-spin" - "gopkg.in/yaml.v2" ) func runTroubleshoot(v *viper.Viper, arg string) error { @@ -65,7 +66,7 @@ func runTroubleshoot(v *viper.Viper, arg string) error { s := spin.New() finishedCh := make(chan bool, 1) - progressChan := make(chan interface{}, 1) + progressChan := make(chan interface{}, 0) // non-zero buffer can result in missed messages go func() { currentDir := "" for { @@ -91,7 +92,7 @@ func runTroubleshoot(v *viper.Viper, arg string) error { } }() defer func() { - finishedCh <- true + close(finishedCh) }() archivePath, err := runCollectors(v, collector, progressChan) @@ -150,26 +151,48 @@ func runCollectors(v *viper.Viper, collector troubleshootv1beta1.Collector, prog return "", errors.Wrap(err, "write version file") } - desiredCollectors := make([]*troubleshootv1beta1.Collect, 0, 0) - for _, definedCollector := range collector.Spec.Collectors { - desiredCollectors = append(desiredCollectors, definedCollector) - } - desiredCollectors = ensureCollectorInList(desiredCollectors, troubleshootv1beta1.Collect{ClusterInfo: &troubleshootv1beta1.ClusterInfo{}}) - desiredCollectors = ensureCollectorInList(desiredCollectors, troubleshootv1beta1.Collect{ClusterResources: &troubleshootv1beta1.ClusterResources{}}) + collectSpecs := make([]*troubleshootv1beta1.Collect, 0, 0) + collectSpecs = append(collectSpecs, collector.Spec.Collectors...) + collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterInfo: &troubleshootv1beta1.ClusterInfo{}}) + collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterResources: &troubleshootv1beta1.ClusterResources{}}) config, err := KubernetesConfigFlags.ToRESTConfig() if err != nil { return "", errors.Wrap(err, "failed to convert kube flags to rest config") } - // Run preflights collectors synchronously - for _, desiredCollector := range desiredCollectors { + var collectors collect.Collectors + for _, desiredCollector := range collectSpecs { collector := collect.Collector{ Redact: true, Collect: desiredCollector, ClientConfig: config, Namespace: v.GetString("namespace"), } + collectors = append(collectors, &collector) + } + + if err := collectors.CheckRBAC(); err != nil { + return "", errors.Wrap(err, "failed to check RBAC for collectors") + } + + foundForbidden := false + for _, c := range collectors { + for _, e := range c.RBACErrors { + foundForbidden = true + progressChan <- e + } + } + + if foundForbidden && !v.GetBool("collect-without-permissions") { + return "", errors.New("insufficient permissions to run all collectors") + } + + // Run preflights collectors synchronously + for _, collector := range collectors { + if len(collector.RBACErrors) > 0 { + continue + } progressChan <- collector.GetDisplayName() @@ -338,3 +361,8 @@ func tarSupportBundleDir(inputDir, outputFilename string) error { return nil } + +type CollectorFailure struct { + Collector *troubleshootv1beta1.Collect + Failure string +} diff --git a/pkg/apis/troubleshoot/v1beta1/collector_shared.go b/pkg/apis/troubleshoot/v1beta1/collector_shared.go index e437a056..eec5974e 100644 --- a/pkg/apis/troubleshoot/v1beta1/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta1/collector_shared.go @@ -1,5 +1,12 @@ package v1beta1 +import ( + "fmt" + "strings" + + authorizationv1 "k8s.io/api/authorization/v1" +) + type CollectorMeta struct { CollectorName string `json:"collectorName,omitempty" yaml:"collectorName,omitempty"` Exclude bool `json:"when,omitmempty" yaml:"when,omitempty"` @@ -111,3 +118,223 @@ type Collect struct { Copy *Copy `json:"copy,omitempty" yaml:"copy,omitempty"` HTTP *HTTP `json:"http,omitempty" yaml:"http,omitempty"` } + +func (c *Collect) AccessReviewSpecs(overrideNS string) []authorizationv1.SelfSubjectAccessReviewSpec { + result := make([]authorizationv1.SelfSubjectAccessReviewSpec, 0) + + if c.ClusterInfo != nil { + // NOOP + } else if c.ClusterResources != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: "", + Verb: "list", + Group: "", + Version: "", + Resource: "Namespace", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: "", + Verb: "list", + Group: "", + Version: "", + Resource: "Node", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: "", + Verb: "list", + Group: "apiextensions.k8s.io", + Version: "", + Resource: "CustomResourceDefinition", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: "", + Verb: "list", + Group: "storage.k8s.io", + Version: "", + Resource: "StorageClasses", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + } else if c.Secret != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Secret.Namespace, overrideNS), + Verb: "get", + Group: "", + Version: "", + Resource: "Secret", + Subresource: "", + Name: c.Secret.SecretName, + }, + NonResourceAttributes: nil, + }) + } else if c.Logs != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Logs.Namespace, overrideNS), + Verb: "list", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Logs.Namespace, overrideNS), + Verb: "get", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "log", + Name: "", + }, + NonResourceAttributes: nil, + }) + } else if c.Run != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Run.Namespace, overrideNS), + Verb: "create", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + } else if c.Exec != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Exec.Namespace, overrideNS), + Verb: "list", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Exec.Namespace, overrideNS), + Verb: "get", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "exec", + Name: "", + }, + NonResourceAttributes: nil, + }) + } else if c.Copy != nil { + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Copy.Namespace, overrideNS), + Verb: "list", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "", + Name: "", + }, + NonResourceAttributes: nil, + }) + result = append(result, authorizationv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Namespace: pickNamespaceOrDefault(c.Copy.Namespace, overrideNS), + Verb: "get", + Group: "", + Version: "", + Resource: "Pod", + Subresource: "exec", + Name: "", + }, + NonResourceAttributes: nil, + }) + } else if c.HTTP != nil { + // NOOP + } + + return result +} + +func (c *Collect) GetName() string { + var collector, name, selector string + if c.ClusterInfo != nil { + collector = "cluster-info" + } + if c.ClusterResources != nil { + collector = "cluster-resources" + } + if c.Secret != nil { + collector = "secret" + name = c.Secret.CollectorName + } + if c.Logs != nil { + collector = "logs" + name = c.Logs.CollectorName + selector = strings.Join(c.Logs.Selector, ",") + } + if c.Run != nil { + collector = "run" + name = c.Run.CollectorName + } + if c.Exec != nil { + collector = "exec" + name = c.Exec.CollectorName + selector = strings.Join(c.Exec.Selector, ",") + } + if c.Copy != nil { + collector = "copy" + name = c.Copy.CollectorName + selector = strings.Join(c.Copy.Selector, ",") + } + if c.HTTP != nil { + collector = "http" + name = c.HTTP.CollectorName + } + + if collector == "" { + return "" + } + if name != "" { + return fmt.Sprintf("%s/%s", collector, name) + } + if selector != "" { + return fmt.Sprintf("%s/%s", collector, selector) + } + return collector +} + +func pickNamespaceOrDefault(collectorNS string, overrideNS string) string { + if overrideNS != "" { + return overrideNS + } + if collectorNS != "" { + return collectorNS + } + return "default" +} diff --git a/pkg/collect/collector.go b/pkg/collect/collector.go index 1f9f2ed0..30c1a35a 100644 --- a/pkg/collect/collector.go +++ b/pkg/collect/collector.go @@ -1,22 +1,24 @@ package collect import ( - "errors" - "fmt" - "strings" - + "github.com/pkg/errors" troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" "gopkg.in/yaml.v2" + authorizationv1 "k8s.io/api/authorization/v1" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ) type Collector struct { Collect *troubleshootv1beta1.Collect Redact bool + RBACErrors []error ClientConfig *rest.Config Namespace string } +type Collectors []*Collector + type Context struct { Redact bool ClientConfig *rest.Config @@ -83,51 +85,7 @@ func (c *Collector) RunCollectorSync() ([]byte, error) { } func (c *Collector) GetDisplayName() string { - var collector, name, selector string - if c.Collect.ClusterInfo != nil { - collector = "cluster-info" - } - if c.Collect.ClusterResources != nil { - collector = "cluster-resources" - } - if c.Collect.Secret != nil { - collector = "secret" - name = c.Collect.Secret.CollectorName - } - if c.Collect.Logs != nil { - collector = "logs" - name = c.Collect.Logs.CollectorName - selector = strings.Join(c.Collect.Logs.Selector, ",") - } - if c.Collect.Run != nil { - collector = "run" - name = c.Collect.Run.CollectorName - } - if c.Collect.Exec != nil { - collector = "exec" - name = c.Collect.Exec.CollectorName - selector = strings.Join(c.Collect.Exec.Selector, ",") - } - if c.Collect.Copy != nil { - collector = "copy" - name = c.Collect.Copy.CollectorName - selector = strings.Join(c.Collect.Copy.Selector, ",") - } - if c.Collect.HTTP != nil { - collector = "http" - name = c.Collect.HTTP.CollectorName - } - - if collector == "" { - return "" - } - if name != "" { - return fmt.Sprintf("%s/%s", collector, name) - } - if selector != "" { - return fmt.Sprintf("%s/%s", collector, selector) - } - return collector + return c.Collect.GetName() } func (c *Collector) GetContext() *Context { @@ -138,6 +96,49 @@ func (c *Collector) GetContext() *Context { } } +func (c *Collector) CheckRBAC() error { + client, err := kubernetes.NewForConfig(c.ClientConfig) + if err != nil { + return errors.Wrap(err, "failed to create client from config") + } + + forbidden := make([]error, 0) + + specs := c.Collect.AccessReviewSpecs(c.Namespace) + for _, spec := range specs { + + sar := &authorizationv1.SelfSubjectAccessReview{ + Spec: spec, + } + + resp, err := client.AuthorizationV1().SelfSubjectAccessReviews().Create(sar) + if err != nil { + return errors.Wrap(err, "failed to run subject review") + } + + if !resp.Status.Allowed { // all other fields of Status are empty... + forbidden = append(forbidden, RBACError{ + DisplayName: c.GetDisplayName(), + Namespace: spec.ResourceAttributes.Namespace, + Resource: spec.ResourceAttributes.Resource, + Verb: spec.ResourceAttributes.Verb, + }) + } + } + c.RBACErrors = forbidden + + return nil +} + +func (cs Collectors) CheckRBAC() error { + for _, c := range cs { + if err := c.CheckRBAC(); err != nil { + return errors.Wrap(err, "failed to check RBAC") + } + } + return nil +} + func ParseSpec(specContents string) (*troubleshootv1beta1.Collect, error) { collect := troubleshootv1beta1.Collect{} diff --git a/pkg/collect/logs.go b/pkg/collect/logs.go index 2810eea5..a84df601 100644 --- a/pkg/collect/logs.go +++ b/pkg/collect/logs.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/pkg/errors" troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" "github.com/replicatedhq/troubleshoot/pkg/logger" corev1 "k8s.io/api/core/v1" @@ -29,7 +30,7 @@ func Logs(ctx *Context, logsCollector *troubleshootv1beta1.Logs) ([]byte, error) if len(podsErrors) > 0 { errorBytes, err := marshalNonNil(podsErrors) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to list pods") } logsOutput[getLogsErrorsFileName(logsCollector)] = errorBytes } @@ -127,14 +128,14 @@ func getPodLogs(client *kubernetes.Clientset, pod corev1.Pod, name, container st req := client.CoreV1().Pods(pod.Namespace).GetLogs(pod.Name, &podLogOpts) podLogs, err := req.Stream() if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to get log stream") } defer podLogs.Close() buf := new(bytes.Buffer) _, err = io.Copy(buf, podLogs) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to copy log") } fileKey := fmt.Sprintf("%s/%s.txt", name, pod.Name) diff --git a/pkg/collect/rbac.go b/pkg/collect/rbac.go new file mode 100644 index 00000000..9d9af736 --- /dev/null +++ b/pkg/collect/rbac.go @@ -0,0 +1,26 @@ +package collect + +import ( + "fmt" + + "github.com/pkg/errors" +) + +type RBACError struct { + DisplayName string + Namespace string + Resource string + Verb string +} + +func (e RBACError) Error() string { + if e.Namespace == "" { + return fmt.Sprintf("cannot collect %s: action %q is not allowed on resource %q at the cluster scope", e.DisplayName, e.Verb, e.Resource) + } + return fmt.Sprintf("cannot collect %s: action %q is not allowed on resource %q in the %q namespace", e.DisplayName, e.Verb, e.Resource, e.Namespace) +} + +func IsRBACError(err error) bool { + _, ok := errors.Cause(err).(RBACError) + return ok +} diff --git a/pkg/collect/run.go b/pkg/collect/run.go index 6c341fb0..27695b50 100644 --- a/pkg/collect/run.go +++ b/pkg/collect/run.go @@ -2,9 +2,9 @@ package collect import ( "encoding/json" - "errors" "time" + "github.com/pkg/errors" troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" "github.com/replicatedhq/troubleshoot/pkg/logger" corev1 "k8s.io/api/core/v1" @@ -17,12 +17,12 @@ type RunOutput map[string][]byte func Run(ctx *Context, runCollector *troubleshootv1beta1.Run) ([]byte, error) { client, err := kubernetes.NewForConfig(ctx.ClientConfig) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to create client from config") } - pod, err := runPod(client, runCollector) + pod, err := runPod(client, runCollector, ctx.Namespace) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to run pod") } defer func() { @@ -37,7 +37,7 @@ func Run(ctx *Context, runCollector *troubleshootv1beta1.Run) ([]byte, error) { timeout, err := time.ParseDuration(runCollector.Timeout) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to parse timeout") } errCh := make(chan error, 1) @@ -64,13 +64,13 @@ func Run(ctx *Context, runCollector *troubleshootv1beta1.Run) ([]byte, error) { func runWithoutTimeout(ctx *Context, pod *corev1.Pod, runCollector *troubleshootv1beta1.Run) ([]byte, error) { client, err := kubernetes.NewForConfig(ctx.ClientConfig) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed create client from config") } for { status, err := client.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{}) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to get pod") } if status.Status.Phase == corev1.PodRunning || status.Status.Phase == corev1.PodFailed || @@ -86,6 +86,9 @@ func runWithoutTimeout(ctx *Context, pod *corev1.Pod, runCollector *troubleshoot MaxLines: 10000, } podLogs, err := getPodLogs(client, *pod, runCollector.Name, "", &limits, true) + if err != nil { + return nil, errors.Wrap(err, "failed to get pod logs") + } for k, v := range podLogs { runOutput[k] = v @@ -94,19 +97,19 @@ func runWithoutTimeout(ctx *Context, pod *corev1.Pod, runCollector *troubleshoot if ctx.Redact { runOutput, err = runOutput.Redact() if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to redact pod logs") } } b, err := json.MarshalIndent(runOutput, "", " ") if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to marshal logs output") } return b, nil } -func runPod(client *kubernetes.Clientset, runCollector *troubleshootv1beta1.Run) (*corev1.Pod, error) { +func runPod(client *kubernetes.Clientset, runCollector *troubleshootv1beta1.Run, namespace string) (*corev1.Pod, error) { podLabels := make(map[string]string) podLabels["troubleshoot-role"] = "run-collector" @@ -115,10 +118,17 @@ func runPod(client *kubernetes.Clientset, runCollector *troubleshootv1beta1.Run) pullPolicy = corev1.PullPolicy(runCollector.ImagePullPolicy) } + if namespace == "" { + namespace = runCollector.Namespace + } + if namespace == "" { + namespace = "default" + } + pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: runCollector.CollectorName, - Namespace: runCollector.Namespace, + Namespace: namespace, Labels: podLabels, }, TypeMeta: metav1.TypeMeta{ @@ -139,9 +149,9 @@ func runPod(client *kubernetes.Clientset, runCollector *troubleshootv1beta1.Run) }, } - created, err := client.CoreV1().Pods(runCollector.Namespace).Create(&pod) + created, err := client.CoreV1().Pods(namespace).Create(&pod) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to create pod") } return created, nil