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

Check RBAC before running collectors

Author: divolgin

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 {
 		}
 	}
 
-	finishedCh <- true
-
 	if preflight.Spec.UploadResultsTo != "" {
-		tryUploadResults(preflight.Spec.UploadResultsTo, preflight.Name, analyzeResults)
+		err := uploadResults(preflight.Spec.UploadResultsTo, analyzeResults)
+		if err != nil {
+			progressChan <- err
+		}
 	}
+
+	finishedCh <- true
+
 	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 UploadPreflightError struct {
+	Error string `json:"error"`
+}
+
 type UploadPreflightResults struct {
-	Results []*UploadPreflightResult `json:"results"`
+	Results []*UploadPreflightResult `json:"results,omitempty"`
+	Errors  []*UploadPreflightError  `json:"errors,omitempty"`
 }
 
-func tryUploadResults(uri string, preflightName string, analyzeResults []*analyzerunner.AnalyzeResult) error {
-	uploadPreflightResults := UploadPreflightResults{
+func uploadResults(uri string, analyzeResults []*analyzerunner.AnalyzeResult) error {
+	uploadPreflightResults := &UploadPreflightResults{
 		Results: []*UploadPreflightResult{},
 	}
 	for _, analyzeResult := range analyzeResults {
@@ -39,26 +46,47 @@ 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")
 
 	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"
-	troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
-	"github.com/replicatedhq/troubleshoot/pkg/collect"
 	"github.com/spf13/viper"
-	"github.com/tj/go-spin"
+	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"
 )
 
 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
+}