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 <evans@replicated.com>

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
 }