fix: move the input/output part out from func

also addressed bugs in passing env var value
replicatedhq · Oct 18, 2023 · e561a2b · e561a2b
1 parent 48ee607
commit e561a2b
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 65 deletions.
diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go
@@ -186,7 +186,7 @@ type HostRun struct {
 	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"`
-	InheritParentEnvs bool              `json:"inheritParentEnvs" yaml:"inheritParentEnvs,omitempty"`
+	IgnoreParentEnvs  bool              `json:"inheritParentEnvs" yaml:"ignoreParentEnvs,omitempty"`
 }
 
 type HostCollect struct {

diff --git a/pkg/collect/host_run.go b/pkg/collect/host_run.go
@@ -36,15 +36,17 @@ func (c *CollectHostRun) IsExcluded() (bool, error) {
 }
 
 func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
+	var (
+		cmdOutputTempDir         string
+		cmdInputTempDir          string
+		bundleOutputRelativePath string
+	)
+
 	runHostCollector := c.hostCollector
 	collectorName := runHostCollector.CollectorName
 	if collectorName == "" {
 		collectorName = "run-host"
 	}
-	var (
-		bundleOutputRelativePath string
-		err                      error
-	)
 
 	cmd := exec.Command(runHostCollector.Command, runHostCollector.Args...)
 
@@ -53,11 +55,44 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 		ExitCode: "0",
 	}
 
-	cmdOutputTempDir, err := c.processEnvVars(cmd)
+	err := c.processEnvVars(cmd)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to parse env variable")
 	}
 
+	// if we choose to save result for the command run
+	if runHostCollector.OutputDir != "" {
+		cmdOutputTempDir, err = os.MkdirTemp("", runHostCollector.OutputDir)
+		defer os.RemoveAll(cmdOutputTempDir)
+		if err != nil {
+			return nil, errors.New(fmt.Sprintf("failed to created temp dir for: %s", runHostCollector.OutputDir))
+		}
+		cmd.Env = append(cmd.Env,
+			fmt.Sprintf("TS_WORKSPACE_DIR=%s", cmdOutputTempDir),
+		)
+	}
+
+	if runHostCollector.Input != nil {
+		cmdInputTempDir, err = os.MkdirTemp("", "input")
+		defer os.RemoveAll(cmdInputTempDir)
+		if err != nil {
+			return nil, errors.New("failed to created temp dir for host run input")
+		}
+		for inFilename, inFileContent := range runHostCollector.Input {
+			if strings.Contains(inFileContent, "/") {
+				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
@@ -97,13 +132,23 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 	return output, nil
 }
 
-func (c *CollectHostRun) processEnvVars(cmd *exec.Cmd) (string, error) {
-	var (
-		cmdOutputTempDir string
-		err              error
-	)
-	// clears the parent env vars
-	cmd.Env = []string{}
+func (c *CollectHostRun) processEnvVars(cmd *exec.Cmd) error {
+	runHostCollector := c.hostCollector
+
+	if runHostCollector.IgnoreParentEnvs {
+		// clears the parent env vars
+		cmd.Env = []string{}
+	} 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))
+		}
+	}
+
 	guaranteedEnvs := []string{"PATH", "KUBECONFIG"}
 	for _, key := range guaranteedEnvs {
 		guaranteedEnvVal, found := os.LookupEnv(key)
@@ -113,66 +158,17 @@ func (c *CollectHostRun) processEnvVars(cmd *exec.Cmd) (string, error) {
 		}
 	}
 
-	runHostCollector := c.hostCollector
-	if runHostCollector.InheritParentEnvs {
-		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))
-			}
-		} else {
-			cmd.Env = append(cmd.Env, os.Environ()...)
-		}
-	}
-
 	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=%s", runHostCollector.Env[i], os.Getenv(runHostCollector.Env[i])))
+					fmt.Sprintf("%s", runHostCollector.Env[i]))
 			} else {
-				return "", errors.New(fmt.Sprintf("env variable entry is missing '=' : %s", runHostCollector.Env[i]))
+				return errors.New(fmt.Sprintf("env variable entry is missing '=' : %s", runHostCollector.Env[i]))
 			}
 		}
 	}
 
-	// if we choose to save result for the command run
-	if runHostCollector.OutputDir != "" {
-		cmdOutputTempDir, err = os.MkdirTemp("", runHostCollector.OutputDir)
-		defer os.RemoveAll(cmdOutputTempDir)
-		if err != nil {
-			return "", errors.New(fmt.Sprintf("failed to created temp dir for: %s", runHostCollector.OutputDir))
-		}
-		cmd.Env = append(cmd.Env,
-			fmt.Sprintf("TS_WORKSPACE_DIR=%s", cmdOutputTempDir),
-		)
-	}
-
-	if runHostCollector.Input != nil {
-		cmdInputTempDir, err := os.MkdirTemp("", "input")
-		defer os.RemoveAll(cmdInputTempDir)
-		if err != nil {
-			return "", errors.New("failed to created temp dir for host run input")
-		}
-		for inFilename, inFileContent := range runHostCollector.Input {
-			if strings.Contains(inFileContent, "/") {
-				return "", errors.New("Input filename contains '/'")
-			}
-			cmdInputFilePath := filepath.Join(cmdInputTempDir, inFilename)
-			err = os.WriteFile(cmdInputFilePath, []byte(inFileContent), 0644)
-			if err != nil {
-				return "", 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),
-		)
-	}
-
-	return cmdOutputTempDir, nil
+	return nil
 }