Merge pull request #107 from replicatedhq/laverya/rbac-wip

Check RBAC before running collectors

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")
 

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

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

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())

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
+}

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 "<none>"
+	}
+	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"
+}

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 "<none>"
-	}
-	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{}
 

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)

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
+}

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