From e561a2b86387a84c100034f064b7239223e85cee Mon Sep 17 00:00:00 2001 From: York Chen Date: Wed, 18 Oct 2023 13:45:26 -0400 Subject: [PATCH] fix: move the input/output part out from func also addressed bugs in passing env var value --- .../v1beta2/hostcollector_shared.go | 2 +- pkg/collect/host_run.go | 124 +++++++++--------- 2 files changed, 61 insertions(+), 65 deletions(-) diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index 918b52d78..913bb0cfc 100644 --- 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 index 769815cd6..63e5585c0 100644 --- 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 }