feat: save host run file output (#1376)

* feat: save cmd run output

* chore: schema changes

* chore: example hostCollector

* chore: add log messages to key actions

* fix: correctly inherit all parent env by default

* chore: do not save input file

the user invokes the input already got the input but those content could be sensitive to another user who received this bundle

* test: unit test for host run

* revert: "chore: do not save input file"

This reverts commit 6af77ad.

that commit is wrong

* chore: fix log msg and example yaml

* Ensure child cmd runs in its own working dir

* Check filename for slashes not content

* Update logging

* Add using relative path files as commands

---------

Co-authored-by: Evans Mungai <[email protected]>

config/crds/troubleshoot.sh_hostcollectors.yaml

@@ -1368,11 +1368,29 @@ spec:
                           type: string
                         command:
                           type: string
+                        env:
+                          items:
+                            type: string
+                          type: array
                         exclude:
                           type: BoolString
+                        ignoreParentEnvs:
+                          type: boolean
+                        inheritEnvs:
+                          items:
+                            type: string
+                          type: array
+                        input:
+                          additionalProperties:
+                            type: string
+                          type: object
+                        outputDir:
+                          type: string
                       required:
                       - args
                       - command
+                      - ignoreParentEnvs
+                      - inheritEnvs
                       type: object
                     subnetAvailable:
                       properties:

config/crds/troubleshoot.sh_hostpreflights.yaml

@@ -1368,11 +1368,29 @@ spec:
                           type: string
                         command:
                           type: string
+                        env:
+                          items:
+                            type: string
+                          type: array
                         exclude:
                           type: BoolString
+                        ignoreParentEnvs:
+                          type: boolean
+                        inheritEnvs:
+                          items:
+                            type: string
+                          type: array
+                        input:
+                          additionalProperties:
+                            type: string
+                          type: object
+                        outputDir:
+                          type: string
                       required:
                       - args
                       - command
+                      - ignoreParentEnvs
+                      - inheritEnvs
                       type: object
                     subnetAvailable:
                       properties:

config/crds/troubleshoot.sh_supportbundles.yaml

@@ -11699,11 +11699,29 @@ spec:
                           type: string
                         command:
                           type: string
+                        env:
+                          items:
+                            type: string
+                          type: array
                         exclude:
                           type: BoolString
+                        ignoreParentEnvs:
+                          type: boolean
+                        inheritEnvs:
+                          items:
+                            type: string
+                          type: array
+                        input:
+                          additionalProperties:
+                            type: string
+                          type: object
+                        outputDir:
+                          type: string
                       required:
                       - args
                       - command
+                      - ignoreParentEnvs
+                      - inheritEnvs
                       type: object
                     subnetAvailable:
                       properties:

examples/collect/host/run-and-save-output.yaml

@@ -0,0 +1,37 @@
+apiVersion: troubleshoot.sh/v1beta2
+kind: HostCollector
+metadata:
+  name: run-host-cmd-and-save-output
+spec:
+  collectors:
+    - run:
+        collectorName: "my-custom-run"
+        command: "sh"
+        # this is for demonstration purpose only -- you probably don't want to drop your input to the bundle!
+        args:
+          - "-c"
+          - "cat $TS_INPUT_DIR/dummy.yaml > $TS_WORKSPACE_DIR/dummy_content.yaml"
+        outputDir: "myCommandOutputs"
+        env:
+          - AWS_REGION=us-west-1
+        # if ignoreParentEnvs is true, it will not inherit envs from parent process.
+        # values specified in inheritEnv will not be used either
+        # ignoreParentEnvs: true
+        inheritEnvs:
+          - USER
+        input:
+          dummy.conf: |-
+            [hello]
+            hello = 1
+            
+            [bye]
+            bye = 2
+          dummy.yaml: |-
+            username: postgres
+            password: <my-pass>
+            dbHost: <hostname>
+            map:
+              key: value
+            list:
+              - val1
+              - val2

pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go

@@ -180,8 +180,13 @@ type HostServices struct {
 
 type HostRun struct {
 	HostCollectorMeta `json:",inline" yaml:",inline"`
-	Command           string   `json:"command"`
-	Args              []string `json:"args"`
+	Command           string            `json:"command"`
+	Args              []string          `json:"args"`
+	OutputDir         string            `json:"outputDir,omitempty" yaml:"outputDir,omitempty"`
+	Input             map[string]string `json:"input,omitempty" yaml:"input,omitempty"`
+	Env               []string          `json:"env,omitempty" yaml:"env,omitempty"`
+	InheritEnvs       []string          `json:"inheritEnvs" yaml:"inheritEnvs,omitempty"`
+	IgnoreParentEnvs  bool              `json:"ignoreParentEnvs" yaml:"ignoreParentEnvs,omitempty"`
 }
 
 type HostCollect struct {

pkg/collect/host_run.go

@@ -3,18 +3,24 @@ package collect
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"k8s.io/klog/v2"
 )
 
 type HostRunInfo struct {
-	Command  string `json:"command"`
-	ExitCode string `json:"exitCode"`
-	Error    string `json:"error"`
+	Command   string   `json:"command"`
+	ExitCode  string   `json:"exitCode"`
+	Error     string   `json:"error"`
+	OutputDir string   `json:"outputDir"`
+	Input     string   `json:"input"`
+	Env       []string `json:"env"`
 }
 
 type CollectHostRun struct {
@@ -31,20 +37,83 @@ func (c *CollectHostRun) IsExcluded() (bool, error) {
 }
 
 func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
-	runHostCollector := c.hostCollector
+	var (
+		cmdOutputTempDir         string
+		cmdInputTempDir          string
+		bundleOutputRelativePath string
+	)
 
-	cmd := exec.Command(runHostCollector.Command, runHostCollector.Args...)
+	runHostCollector := c.hostCollector
+	collectorName := runHostCollector.CollectorName
+	if collectorName == "" {
+		collectorName = "run-host"
+	}
 
-	var stdout, stderr bytes.Buffer
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
+	cmd := exec.Command(c.attemptToConvertCmdToAbsPath(), runHostCollector.Args...)
 
-	runInfo := HostRunInfo{
+	klog.V(2).Infof("Run host collector command: %q", cmd.String())
+	runInfo := &HostRunInfo{
 		Command:  cmd.String(),
 		ExitCode: "0",
 	}
 
-	err := cmd.Run()
+	err := c.processEnvVars(cmd)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to parse env variable")
+	}
+
+	// Create a working directory for the command
+	wkdir, err := os.MkdirTemp("", collectorName)
+	defer os.RemoveAll(wkdir)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to create temp dir for host run")
+	}
+	// Change the working directory for the command to ensure the command
+	// does not polute the parent/caller working directory
+	cmd.Dir = wkdir
+
+	// if we choose to save result for the command run
+	if runHostCollector.OutputDir != "" {
+		cmdOutputTempDir = filepath.Join(wkdir, runHostCollector.OutputDir)
+		err = os.MkdirAll(cmdOutputTempDir, 0755)
+		if err != nil {
+			return nil, errors.New(fmt.Sprintf("failed to create dir for: %s", runHostCollector.OutputDir))
+		}
+		cmd.Env = append(cmd.Env,
+			fmt.Sprintf("TS_WORKSPACE_DIR=%s", cmdOutputTempDir),
+		)
+	}
+
+	if runHostCollector.Input != nil {
+		cmdInputTempDir = filepath.Join(wkdir, "input")
+		err = os.MkdirAll(cmdInputTempDir, 0755)
+		if err != nil {
+			return nil, errors.New("failed to create temp dir for host run input")
+		}
+		for inFilename, inFileContent := range runHostCollector.Input {
+			if strings.Contains(inFilename, "/") {
+				return nil, errors.New("Input filename contains '/'")
+			}
+			cmdInputFilePath := filepath.Join(cmdInputTempDir, inFilename)
+			err = os.WriteFile(cmdInputFilePath, []byte(inFileContent), 0644)
+			if err != nil {
+				return nil, errors.Wrap(err, fmt.Sprintf("failed to write input file: %s to temp directory", inFilename))
+			}
+		}
+		cmd.Env = append(cmd.Env,
+			fmt.Sprintf("TS_INPUT_DIR=%s", cmdInputTempDir),
+		)
+	}
+
+	collectorRelativePath := filepath.Join("host-collectors/run-host", collectorName)
+
+	runInfo.Env = cmd.Env
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err = cmd.Run()
 	if err != nil {
 		if werr, ok := err.(*exec.ExitError); ok {
 			runInfo.ExitCode = strings.TrimPrefix(werr.Error(), "exit status ")
@@ -54,10 +123,7 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 		}
 	}
 
-	collectorName := c.hostCollector.CollectorName
-	if collectorName == "" {
-		collectorName = "run-host"
-	}
+	output := NewResult()
 	resultInfo := filepath.Join("host-collectors/run-host", collectorName+"-info.json")
 	result := filepath.Join("host-collectors/run-host", collectorName+".txt")
 
@@ -66,14 +132,89 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 		return nil, errors.Wrap(err, "failed to marshal run host result")
 	}
 
-	output := NewResult()
 	output.SaveResult(c.BundlePath, resultInfo, bytes.NewBuffer(b))
 	output.SaveResult(c.BundlePath, result, bytes.NewBuffer(stdout.Bytes()))
+	// walkthrough the output directory and save result for each file
+	if runHostCollector.OutputDir != "" {
+		runInfo.OutputDir = runHostCollector.OutputDir
+		bundleOutputRelativePath = filepath.Join(collectorRelativePath, runHostCollector.OutputDir)
+		klog.V(2).Infof("Saving command output to %q in bundle", bundleOutputRelativePath)
+		output.SaveResults(c.BundlePath, bundleOutputRelativePath, cmdOutputTempDir)
+	}
+
+	return output, nil
+}
+
+func (c *CollectHostRun) processEnvVars(cmd *exec.Cmd) error {
+	runHostCollector := c.hostCollector
+
+	if runHostCollector.IgnoreParentEnvs {
+		klog.V(2).Info("Not inheriting the environment variables!")
+		if runHostCollector.InheritEnvs != nil {
+			klog.V(2).Infof("The following environment variables will not be loaded to the command: [%s]",
+				strings.Join(runHostCollector.InheritEnvs, ","))
+		}
+		// clears the parent env vars
+		cmd.Env = []string{}
+		populateGuaranteedEnvVars(cmd)
+	} else if runHostCollector.InheritEnvs != nil {
+		for _, key := range runHostCollector.InheritEnvs {
+			envVal, found := os.LookupEnv(key)
+			if !found {
+				return errors.New(fmt.Sprintf("inherit env variable is not found: %s", key))
+			}
+			cmd.Env = append(cmd.Env,
+				fmt.Sprintf("%s=%s", key, envVal))
+		}
+		populateGuaranteedEnvVars(cmd)
+	} else {
+		cmd.Env = os.Environ()
+	}
 
-	runHostOutput := map[string][]byte{
-		resultInfo: b,
-		result:     stdout.Bytes(),
+	if runHostCollector.Env != nil {
+		for i := range runHostCollector.Env {
+			parts := strings.Split(runHostCollector.Env[i], "=")
+			if len(parts) == 2 && parts[0] != "" && parts[1] != "" {
+				cmd.Env = append(cmd.Env,
+					fmt.Sprintf("%s", runHostCollector.Env[i]))
+			} else {
+				return errors.New(fmt.Sprintf("env variable entry is missing '=' : %s", runHostCollector.Env[i]))
+			}
+		}
+	}
+
+	return nil
+}
+
+func populateGuaranteedEnvVars(cmd *exec.Cmd) {
+	guaranteedEnvs := []string{"PATH", "KUBECONFIG", "PWD"}
+	for _, key := range guaranteedEnvs {
+		guaranteedEnvVal, found := os.LookupEnv(key)
+		if found {
+			cmd.Env = append(cmd.Env,
+				fmt.Sprintf("%s=%s", key, guaranteedEnvVal))
+		}
+	}
+}
+
+// attemptToConvertCmdToAbsPath checks if the command is a file path or command name
+// If it is a file path, it will return the absolute path else
+// it will return the command name as is and leave the resolution to cmd.Run()
+// This enables passing commands using relative paths e.g. "./my-command"
+// which is not possible with cmd.Run() since the child process runs
+// in a different working directory
+func (c *CollectHostRun) attemptToConvertCmdToAbsPath() string {
+	// Attempt to check if the command is file path or command name
+	cmdAbsPath, err := filepath.Abs(c.hostCollector.Command)
+	if err != nil {
+		return c.hostCollector.Command
+	}
+
+	// Check if the file exists
+	_, err = os.Stat(cmdAbsPath)
+	if err != nil {
+		return c.hostCollector.Command
 	}
 
-	return runHostOutput, nil
+	return cmdAbsPath
 }