stopCmd should use same environment as p.cmd.Env (#171, #172)

Append custom env vars instead of replace in Process (#168, #169)

PR #162 refactored the default configuration code. This
introduced a subtle bug where `env` became `[]string{}` instead of the
default of `nil`.

In golang, `exec.Cmd.Env == nil` means to use the "current process's
environment". By setting it to `[]string{}` as a default the Process's
environment was emptied out which caused an array of strange and
difficult to troubleshoot behaviour. See issues #168 and #169

This commit changes the behaviour to append model configured environment
variables to the default list rather than replace them.

.goreleaser.yaml

@@ -17,14 +17,16 @@ builds:
       - goos: windows
         goarch: arm64
 
-# use zip format for windows
 archives:
   - id: default
-    format: tar.gz
+    formats:
+      - tar.gz
     name_template: "{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}"
     builds_info:
       group: root
       owner: root
     format_overrides:
+      # use zip format for windows
       - goos: windows
-        format: zip
+        formats:
+          - zip

proxy/process.go

@@ -189,18 +189,19 @@ func (p *Process) start() error {
 	p.waitStarting.Add(1)
 	defer p.waitStarting.Done()
 	cmdContext, ctxCancelUpstream := context.WithCancel(context.Background())
-	p.proxyLogger.Debugf("<%s> Executing start command: %s", p.ID, strings.Join(args, " "))
+
 	p.cmd = exec.CommandContext(cmdContext, args[0], args[1:]...)
 	p.cmd.Stdout = p.processLogger
 	p.cmd.Stderr = p.processLogger
-	p.cmd.Env = p.config.Env
-
+	p.cmd.Env = append(p.cmd.Environ(), p.config.Env...)
 	p.cmd.Cancel = p.cmdStopUpstreamProcess
 	p.cmd.WaitDelay = p.gracefulStopTimeout
 	p.cancelUpstream = ctxCancelUpstream
 	p.cmdWaitChan = make(chan struct{})
 
 	p.failedStartCount++ // this will be reset to zero when the process has successfully started
+
+	p.proxyLogger.Debugf("<%s> Executing start command: %s, env: %s", p.ID, strings.Join(args, " "), strings.Join(p.config.Env, ", "))
 	err = p.cmd.Start()
 
 	// Set process state to failed
@@ -531,7 +532,7 @@ func (p *Process) cmdStopUpstreamProcess() error {
 		stopCmd := exec.Command(stopArgs[0], stopArgs[1:]...)
 		stopCmd.Stdout = p.processLogger
 		stopCmd.Stderr = p.processLogger
-		stopCmd.Env = p.config.Env
+		stopCmd.Env = p.cmd.Env
 
 		if err := stopCmd.Run(); err != nil {
 			p.proxyLogger.Errorf("<%s> Failed to exec stop command: %v", p.ID, err)

proxy/process_test.go

@@ -394,6 +394,9 @@ func TestProcess_StopImmediately(t *testing.T) {
 // Test that SIGKILL is sent when gracefulStopTimeout is reached and properly terminates
 // the upstream command
 func TestProcess_ForceStopWithKill(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping SIGTERM test on Windows ")
+	}
 
 	expectedMessage := "test_sigkill"
 	binaryPath := getSimpleResponderPath()
@@ -405,7 +408,6 @@ func TestProcess_ForceStopWithKill(t *testing.T) {
 		Cmd:           fmt.Sprintf("%s --port %d --respond %s --silent --ignore-sig-term", binaryPath, port, expectedMessage),
 		Proxy:         fmt.Sprintf("http://127.0.0.1:%d", port),
 		CheckEndpoint: "/health",
-		CmdStop:       "taskkill /f /t /pid ${PID}",
 	}
 
 	process := NewProcess("stop_immediate", 2, config, debugLogger, debugLogger)
@@ -465,3 +467,27 @@ func TestProcess_StopCmd(t *testing.T) {
 	process.StopImmediately()
 	assert.Equal(t, process.CurrentState(), StateStopped)
 }
+
+func TestProcess_EnvironmentSetCorrectly(t *testing.T) {
+	expectedMessage := "test_env_not_emptied"
+	config := getTestSimpleResponderConfig(expectedMessage)
+
+	// ensure that the the default config does not blank out the inherited environment
+	configWEnv := config
+
+	// ensure the additiona variables are appended to the process' environment
+	configWEnv.Env = append(configWEnv.Env, "TEST_ENV1=1", "TEST_ENV2=2")
+
+	process1 := NewProcess("env_test", 2, config, debugLogger, debugLogger)
+	process2 := NewProcess("env_test", 2, configWEnv, debugLogger, debugLogger)
+
+	process1.start()
+	defer process1.Stop()
+	process2.start()
+	defer process2.Stop()
+
+	assert.NotZero(t, len(process1.cmd.Environ()))
+	assert.NotZero(t, len(process2.cmd.Environ()))
+	assert.Equal(t, len(process1.cmd.Environ())+2, len(process2.cmd.Environ()), "process2 should have 2 more environment variables than process1")
+
+}