Repository: tetratelabs/getenvoy
Pull request #279 (2021-06-11 10:03:52)

Makes code more OS portable
Almost ready for #105

codefromthecrypt: ok also backfilled a ln test, though it will need a slight adjustment when completing windows it shows why it is there.

Patch:
diff --git a/internal/cmd/run.go b/internal/cmd/run.go
index 706c008..5549220 100644
--- a/internal/cmd/run.go
+++ b/internal/cmd/run.go
@@ -90,6 +90,12 @@ state. On exit, these archive into ` + "`$GETENVOY_HOME/runs/$epochtime.tar.gz`"
 			defer stderrLog.Close() //nolint
 			r.Err = io.MultiWriter(c.App.ErrWriter, stderrLog)
 
+			// Ensure console redirect files close before we attempt to archive them
+			r.RegisterShutdownHook(func(ctx context.Context) error {
+				stdoutLog.Close() // nolint
+				return stderrLog.Close()
+			})
+
 			for _, enableShutdownHook := range shutdown.EnableHooks {
 				if err := enableShutdownHook(r); err != nil {
 					fmt.Fprintln(r.Out, "failed to enable shutdown hook:", err) //nolint
diff --git a/internal/envoy/install.go b/internal/envoy/install.go
index 3630b0c..79b44f5 100644
--- a/internal/envoy/install.go
+++ b/internal/envoy/install.go
@@ -23,11 +23,12 @@ import (
 	"time"
 
 	"github.com/tetratelabs/getenvoy/internal/globals"
+	"github.com/tetratelabs/getenvoy/internal/moreos"
 	"github.com/tetratelabs/getenvoy/internal/tar"
 	"github.com/tetratelabs/getenvoy/internal/version"
 )
 
-const binEnvoy = "bin/envoy"
+var binEnvoy = filepath.Join("bin", "envoy"+moreos.Exe)
 
 // InstallIfNeeded downloads an Envoy binary corresponding to the given version and returns a path to it or an error.
 func InstallIfNeeded(ctx context.Context, o *globals.GlobalOpts, p, v string) (string, error) {
@@ -77,7 +78,7 @@ func verifyEnvoy(installPath string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	if stat.Mode()&0111 == 0 {
+	if !moreos.IsExecutable(stat) {
 		return "", fmt.Errorf("envoy binary not executable at %q", envoyPath)
 	}
 	return envoyPath, nil
diff --git a/internal/envoy/shutdown.go b/internal/envoy/shutdown.go
index 45d9db7..fda9672 100644
--- a/internal/envoy/shutdown.go
+++ b/internal/envoy/shutdown.go
@@ -28,8 +28,8 @@ import (
 
 // RegisterShutdownHook registers the passed functions to be run after Envoy has started
 // and just before GetEnvoy instructs Envoy to exit
-func (r *Runtime) RegisterShutdownHook(f ...func(context.Context) error) {
-	r.shutdownHooks = append(r.shutdownHooks, f...)
+func (r *Runtime) RegisterShutdownHook(f func(context.Context) error) {
+	r.shutdownHooks = append(r.shutdownHooks, f)
 }
 
 func (r *Runtime) handleShutdown(ctx context.Context) {
diff --git a/internal/envoy/version.go b/internal/envoy/version.go
index 2f29571..cb79b92 100644
--- a/internal/envoy/version.go
+++ b/internal/envoy/version.go
@@ -81,7 +81,7 @@ func getCurrentVersion(homeDir string) (v, source string, err error) {
 	// Priority 2: $PWD/.envoy-version
 	data, err := os.ReadFile(".envoy-version")
 	if err == nil {
-		return trimNewline(data), CurrentVersionWorkingDirFile, nil
+		return strings.TrimSpace(string(data)), CurrentVersionWorkingDirFile, nil
 	} else if !os.IsNotExist(err) {
 		return "", CurrentVersionWorkingDirFile, err
 	}
@@ -96,7 +96,7 @@ func getHomeVersion(homeDir string) (v, homeVersionFile string, err error) {
 	homeVersionFile = filepath.Join(homeDir, "version")
 	var data []byte
 	if data, err = os.ReadFile(homeVersionFile); err == nil {
-		v = trimNewline(data)
+		v = strings.TrimSpace(string(data))
 	} else if os.IsNotExist(err) {
 		err = nil // ok on file-not-found
 	}
@@ -108,8 +108,3 @@ func getHomeVersion(homeDir string) (v, homeVersionFile string, err error) {
 func VersionUsageList() string {
 	return strings.Join([]string{currentVersionVar, CurrentVersionWorkingDirFile, CurrentVersionHomeDirFile}, ", ")
 }
-
-// trimNewline allows assignments like `echo 1.15.5 > $GETENVOY_HOME/version`
-func trimNewline(data []byte) string {
-	return strings.TrimSuffix(string(data), "\n")
-}
diff --git a/internal/moreos/moreos.go b/internal/moreos/moreos.go
index c7e773b..8178d65 100644
--- a/internal/moreos/moreos.go
+++ b/internal/moreos/moreos.go
@@ -19,6 +19,13 @@ import (
 	"syscall"
 )
 
+const (
+	// LineSeparator is the runtime.GOOS-specific new line or line feed. Ex. "\n"
+	LineSeparator = ln
+	// Exe is the runtime.GOOS-specific suffix for executables. Ex. "" unless windows which is ".exe"
+	Exe = exe
+)
+
 // ProcessGroupAttr sets attributes that ensure exec.Cmd doesn't propagate signals from getenvoy by default.
 // This is used to ensure shutdown hooks can apply
 func ProcessGroupAttr() *syscall.SysProcAttr {
@@ -29,3 +36,8 @@ func ProcessGroupAttr() *syscall.SysProcAttr {
 func Interrupt(p *os.Process) error {
 	return interrupt(p) // un-exported to prevent godoc drift
 }
+
+// IsExecutable returns true if the input can be run as an exec.Cmd
+func IsExecutable(f os.FileInfo) bool {
+	return isExecutable(f)
+}
diff --git a/internal/moreos/proc.go b/internal/moreos/proc_darwin.go
similarity index 88%
rename from internal/moreos/proc.go
rename to internal/moreos/proc_darwin.go
index 7308d4a..428f2a4 100644
--- a/internal/moreos/proc.go
+++ b/internal/moreos/proc_darwin.go
@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// +build !linux
-
 package moreos
 
 import (
@@ -21,6 +19,11 @@ import (
 	"syscall"
 )
 
+const (
+	ln  = "\n"
+	exe = ""
+)
+
 func processGroupAttr() *syscall.SysProcAttr {
 	return &syscall.SysProcAttr{Setpgid: true}
 }
@@ -28,3 +31,7 @@ func processGroupAttr() *syscall.SysProcAttr {
 func interrupt(p *os.Process) error {
 	return p.Signal(syscall.SIGINT)
 }
+
+func isExecutable(f os.FileInfo) bool {
+	return f.Mode()&0111 != 0
+}
diff --git a/internal/moreos/proc_linux.go b/internal/moreos/proc_linux.go
index 1a57ec8..72cc865 100644
--- a/internal/moreos/proc_linux.go
+++ b/internal/moreos/proc_linux.go
@@ -19,6 +19,11 @@ import (
 	"syscall"
 )
 
+const (
+	ln  = "\n"
+	exe = ""
+)
+
 func processGroupAttr() *syscall.SysProcAttr {
 	// Pdeathsig aims to ensure the process group is cleaned up even if this process dies
 	return &syscall.SysProcAttr{Setpgid: true, Pdeathsig: syscall.SIGTERM}
@@ -27,3 +32,7 @@ func processGroupAttr() *syscall.SysProcAttr {
 func interrupt(p *os.Process) error {
 	return p.Signal(syscall.SIGINT)
 }
+
+func isExecutable(f os.FileInfo) bool {
+	return f.Mode()&0111 != 0
+}
diff --git a/internal/tar/tar.go b/internal/tar/tar.go
index dcd4ab2..0e9cb77 100644
--- a/internal/tar/tar.go
+++ b/internal/tar/tar.go
@@ -125,6 +125,7 @@ func TarGz(dst, src string) error { //nolint dst, src order like io.Copy
 
 	// Recurse through the path including all files and directories
 	return fs.WalkDir(srcFS, basePath, func(path string, d os.DirEntry, err error) error {
+		path = filepath.ToSlash(path) // normalize to unix slashes
 		if err != nil {
 			return err
 		}
@@ -139,6 +140,10 @@ func TarGz(dst, src string) error { //nolint dst, src order like io.Copy
 			return err
 		}
 
+		if !info.IsDir() && header.Size == 0 { // skip empty files
+			return nil
+		}
+
 		// Ensure the destination file starts at the intended path
 		header.Name = path
 		if err := tw.WriteHeader(header); err != nil {
@@ -148,17 +153,20 @@ func TarGz(dst, src string) error { //nolint dst, src order like io.Copy
 		if info.IsDir() {
 			return nil // nothing to write
 		}
-		return copy(tw, srcFS, path)
+		if err := copy(tw, srcFS, header.Name, header.Size); err != nil {
+			return err
+		}
+		return tw.Flush()
 	})
 }
 
 // Copy the contents of the file into the tar without buffering
-func copy(dst io.Writer, src fs.FS, path string) error { // dst, src order like io.Copy
+func copy(dst io.Writer, src fs.FS, path string, n int64) error { // dst, src order like io.Copy
 	f, err := src.Open(path) //nolint:gosec
 	if err != nil {
 		return err
 	}
 	defer f.Close() //nolint
-	_, err = io.Copy(dst, f)
+	_, err = io.CopyN(dst, f, n)
 	return err
 }


Test patch:
diff --git a/e2e/getenvoy_use_test.go b/e2e/getenvoy_use_test.go
index 53fd419..9955a76 100644
--- a/e2e/getenvoy_use_test.go
+++ b/e2e/getenvoy_use_test.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/tetratelabs/getenvoy/internal/moreos"
 	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
 	"github.com/tetratelabs/getenvoy/internal/version"
 )
@@ -40,7 +41,8 @@ func TestGetEnvoyUse(t *testing.T) {
 		require.Empty(t, stderr)
 
 		// The binary was installed
-		require.FileExists(t, filepath.Join(homeDir, "versions", version.LastKnownEnvoy, "bin", "envoy"))
+		envoyBin := filepath.Join(homeDir, "versions", version.LastKnownEnvoy, "bin", "envoy"+moreos.Exe)
+		require.FileExists(t, envoyBin)
 
 		// The current version was written
 		f, err := os.ReadFile(filepath.Join(homeDir, "version"))
diff --git a/e2e/main_test.go b/e2e/main_test.go
index 62337af..cb081db 100644
--- a/e2e/main_test.go
+++ b/e2e/main_test.go
@@ -129,7 +129,7 @@ func readGetEnvoyPath() (string, error) {
 	}
 	// While "make bin" should result in correct permissions, double-check as some tools lose them, such as
 	// https://github.com/actions/upload-artifact#maintaining-file-permissions-and-case-sensitive-files
-	if stat.Mode()&0111 == 0 {
+	if !moreos.IsExecutable(stat) {
 		return "", fmt.Errorf("%s is not executable. Correct environment variable %s", path, getenvoyBinaryEnvKey)
 	}
 	return path, nil
diff --git a/e2e/static-filesystem.yaml b/e2e/static-filesystem.yaml
index 9f7e9a8..ac63369 100644
--- a/e2e/static-filesystem.yaml
+++ b/e2e/static-filesystem.yaml
@@ -1,5 +1,4 @@
 admin:
-  access_log_path: '/dev/stdout'
   address:
     socket_address:
       address: '127.0.0.1'
diff --git a/internal/cmd/run_test.go b/internal/cmd/run_test.go
index 2e33d72..68d63e2 100644
--- a/internal/cmd/run_test.go
+++ b/internal/cmd/run_test.go
@@ -19,7 +19,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -29,11 +28,14 @@ import (
 
 	rootcmd "github.com/tetratelabs/getenvoy/internal/cmd"
 	"github.com/tetratelabs/getenvoy/internal/globals"
+	"github.com/tetratelabs/getenvoy/internal/moreos"
 	"github.com/tetratelabs/getenvoy/internal/test"
 	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
 	"github.com/tetratelabs/getenvoy/internal/version"
 )
 
+const ln = moreos.LineSeparator
+
 // Runner allows us to not introduce dependency cycles on envoy.Runtime
 type runner struct {
 	c *cli.App
@@ -60,7 +62,7 @@ func TestGetEnvoyRun(t *testing.T) {
 
 	require.NoError(t, err)
 	require.Empty(t, stdout)
-	require.Equal(t, "initializing epoch 0\nstarting main dispatch loop\n", stderr.String())
+	require.Equal(t, fmt.Sprintf("initializing epoch 0%[1]sstarting main dispatch loop%[1]s", ln), stderr.String())
 }
 
 func TestGetEnvoyRun_TeesConsoleToLogs(t *testing.T) {
@@ -72,12 +74,12 @@ func TestGetEnvoyRun_TeesConsoleToLogs(t *testing.T) {
 	o.DontArchiveRunDir = true // we need to read-back the log files
 	runWithoutConfig(t, c)
 
-	have, err := ioutil.ReadFile(filepath.Join(o.RunDir, "stdout.log"))
+	have, err := os.ReadFile(filepath.Join(o.RunDir, "stdout.log"))
 	require.NoError(t, err)
-	require.NotEmpty(t, stdout.String()) // sanity check
-	require.Equal(t, stdout.String(), string(have))
+	require.NotEmpty(t, stdout.String())               // sanity check
+	require.Contains(t, stdout.String(), string(have)) // stdout will be more than in the log as getenvoy writes to it.
 
-	have, err = ioutil.ReadFile(filepath.Join(o.RunDir, "stderr.log"))
+	have, err = os.ReadFile(filepath.Join(o.RunDir, "stderr.log"))
 	require.NoError(t, err)
 	require.NotEmpty(t, stderr.String()) // sanity check
 	require.Equal(t, stderr.String(), string(have))
@@ -95,7 +97,7 @@ func TestGetEnvoyRun_ReadsHomeVersionFile(t *testing.T) {
 	runWithoutConfig(t, c)
 
 	// No implicit lookup
-	require.NotContains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version\n")
+	require.NotContains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version"+ln)
 	require.Equal(t, version.LastKnownEnvoy, o.EnvoyVersion)
 }
 
@@ -112,7 +114,7 @@ func TestGetEnvoyRun_CreatesHomeVersionFile(t *testing.T) {
 	runWithoutConfig(t, c)
 
 	// We logged the implicit lookup
-	require.Contains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version\n")
+	require.Contains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version"+ln)
 	require.FileExists(t, filepath.Join(o.HomeDir, "version"))
 	require.Equal(t, version.LastKnownEnvoy, o.EnvoyVersion)
 }
@@ -167,6 +169,6 @@ func TestGetEnvoyRun_ErrsWhenVersionsServerDown(t *testing.T) {
 	c, _, _ := newApp(o)
 	err := c.Run([]string{"getenvoy", "run"})
 
-	require.Contains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version\n")
+	require.Contains(t, o.Out.(*bytes.Buffer).String(), "looking up latest version"+ln)
 	require.Contains(t, err.Error(), fmt.Sprintf(`couldn't read latest version from %s`, o.EnvoyVersionsURL))
 }
diff --git a/internal/envoy/install_test.go b/internal/envoy/install_test.go
index 55c361f..1d270c7 100644
--- a/internal/envoy/install_test.go
+++ b/internal/envoy/install_test.go
@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -91,7 +90,7 @@ func TestInstallIfNeeded_ErrorOnIncorrectURL(t *testing.T) {
 
 	o.EnvoyVersionsURL += "/varsionz.json"
 
-	_, err := InstallIfNeeded(o.ctx, &o.GlobalOpts, runtime.GOOS, version.LastKnownEnvoy)
+	_, err := InstallIfNeeded(o.ctx, &o.GlobalOpts, globals.CurrentPlatform, version.LastKnownEnvoy)
 	require.EqualError(t, err, "received 404 status code from "+o.EnvoyVersionsURL)
 	require.Empty(t, o.Out.(*bytes.Buffer))
 }
@@ -109,9 +108,9 @@ func TestInstallIfNeeded_Validates(t *testing.T) {
 		},
 		{
 			name:        "unsupported OS",
-			goos:        "windows",
+			goos:        "solaris",
 			version:     version.LastKnownEnvoy,
-			expectedErr: fmt.Sprintf(`couldn't find version %q for platform "windows"`, version.LastKnownEnvoy),
+			expectedErr: fmt.Sprintf(`couldn't find version %q for platform "solaris"`, version.LastKnownEnvoy),
 		},
 	}
 
@@ -154,8 +153,8 @@ func TestInstallIfNeeded_NotFound(t *testing.T) {
 		require.EqualError(t, e, `couldn't find version "1.1.1" for platform "darwin/amd64"`)
 	})
 	t.Run("unknown platform", func(t *testing.T) {
-		_, e := InstallIfNeeded(o.ctx, &o.GlobalOpts, "windows/arm64", version.LastKnownEnvoy)
-		require.EqualError(t, e, fmt.Sprintf(`couldn't find version "%s" for platform "windows/arm64"`, version.LastKnownEnvoy))
+		_, e := InstallIfNeeded(o.ctx, &o.GlobalOpts, "solaris/amd64", version.LastKnownEnvoy)
+		require.EqualError(t, e, fmt.Sprintf(`couldn't find version "%s" for platform "solaris/amd64"`, version.LastKnownEnvoy))
 	})
 }
 
@@ -165,12 +164,12 @@ func TestInstallIfNeeded_AlreadyExists(t *testing.T) {
 	out := o.Out.(*bytes.Buffer)
 
 	require.NoError(t, os.MkdirAll(filepath.Dir(o.EnvoyPath), 0700))
-	require.NoError(t, ioutil.WriteFile(o.EnvoyPath, []byte("fake"), 0700))
+	require.NoError(t, os.WriteFile(o.EnvoyPath, []byte("fake"), 0700))
 
 	envoyStat, err := os.Stat(o.EnvoyPath)
 	require.NoError(t, err)
 
-	envoyPath, e := InstallIfNeeded(o.ctx, &o.GlobalOpts, runtime.GOOS, version.LastKnownEnvoy)
+	envoyPath, e := InstallIfNeeded(o.ctx, &o.GlobalOpts, globals.CurrentPlatform, version.LastKnownEnvoy)
 	require.NoError(t, e)
 	require.Equal(t, fmt.Sprintln(version.LastKnownEnvoy, "is already downloaded"), out.String())
 
@@ -190,23 +189,23 @@ func TestVerifyEnvoy(t *testing.T) {
 	t.Run("envoy binary doesn't exist", func(t *testing.T) {
 		EnvoyPath, e := verifyEnvoy(envoyPath)
 		require.Empty(t, EnvoyPath)
-		require.Contains(t, e.Error(), "no such file or directory")
-	})
-
-	expectedEnvoyPath := filepath.Join(envoyPath, "bin", "envoy")
-	require.NoError(t, os.WriteFile(expectedEnvoyPath, []byte{}, 0600))
-	t.Run("envoy binary not executable", func(t *testing.T) {
-		EnvoyPath, e := verifyEnvoy(envoyPath)
-		require.Empty(t, EnvoyPath)
-		require.EqualError(t, e, fmt.Sprintf(`envoy binary not executable at %q`, expectedEnvoyPath))
+		require.Contains(t, e.Error(), "file")
 	})
 
-	require.NoError(t, os.Chmod(expectedEnvoyPath, 0750))
+	expectedEnvoyPath := filepath.Join(envoyPath, binEnvoy)
+	require.NoError(t, os.WriteFile(expectedEnvoyPath, []byte{}, 0700))
 	t.Run("envoy binary ok", func(t *testing.T) {
 		EnvoyPath, e := verifyEnvoy(envoyPath)
 		require.Equal(t, expectedEnvoyPath, EnvoyPath)
 		require.Nil(t, e)
 	})
+
+	t.Run("envoy binary not executable", func(t *testing.T) {
+		require.NoError(t, os.Chmod(expectedEnvoyPath, 0600))
+		EnvoyPath, e := verifyEnvoy(envoyPath)
+		require.Empty(t, EnvoyPath)
+		require.EqualError(t, e, fmt.Sprintf(`envoy binary not executable at %q`, expectedEnvoyPath))
+	})
 }
 
 type installTest struct {
@@ -233,7 +232,7 @@ func setupInstallTest(t *testing.T) (*installTest, func()) {
 				EnvoyVersionsURL: versionsServer.URL + "/envoy-versions.json",
 				Out:              new(bytes.Buffer),
 				RunOpts: globals.RunOpts{
-					EnvoyPath: filepath.Join(tempDir, "versions", version.LastKnownEnvoy, "bin", "envoy"),
+					EnvoyPath: filepath.Join(tempDir, "versions", version.LastKnownEnvoy, binEnvoy),
 				},
 			},
 		}, func() {
diff --git a/internal/envoy/run_test.go b/internal/envoy/run_test.go
index 835bc55..419fd83 100644
--- a/internal/envoy/run_test.go
+++ b/internal/envoy/run_test.go
@@ -29,19 +29,22 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/tetratelabs/getenvoy/internal/globals"
+	"github.com/tetratelabs/getenvoy/internal/moreos"
 	"github.com/tetratelabs/getenvoy/internal/test"
 	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
 )
 
+const ln = moreos.LineSeparator
+
 func TestRuntime_Run(t *testing.T) {
 	tempDir, removeTempDir := morerequire.RequireNewTempDir(t)
 	defer removeTempDir()
 
 	runsDir := filepath.Join(tempDir, "runs")
 	runDir := filepath.Join(runsDir, "1619574747231823000") // fake a realistic value
-	adminFlag := fmt.Sprintf("--admin-address-path %s/admin-address.txt", runDir)
+	adminFlag := fmt.Sprint("--admin-address-path ", filepath.Join(runDir, "admin-address.txt"))
 
-	fakeEnvoy := filepath.Join(tempDir, "envoy")
+	fakeEnvoy := filepath.Join(tempDir, "envoy"+moreos.Exe)
 	test.RequireFakeEnvoy(t, fakeEnvoy)
 
 	tests := []struct {
@@ -56,8 +59,8 @@ func TestRuntime_Run(t *testing.T) {
 			name: "GetEnvoy Ctrl+C",
 			args: []string{"-c", "envoy.yaml"},
 			// Don't warn the user when they exited the process
-			expectedStdout:   fmt.Sprintln("starting:", fakeEnvoy, "-c", "envoy.yaml", adminFlag) + "GET /ready HTTP/1.1\n",
-			expectedStderr:   "initializing epoch 0\nstarting main dispatch loop\ncaught SIGINT\nexiting\n",
+			expectedStdout:   fmt.Sprintln("starting:", fakeEnvoy, "-c", "envoy.yaml", adminFlag) + "GET /ready HTTP/1.1" + ln,
+			expectedStderr:   fmt.Sprintf("initializing epoch 0%[1]sstarting main dispatch loop%[1]scaught SIGINT%[1]sexiting%[1]s", ln),
 			wantShutdownHook: true,
 		},
 		// We don't test envoy dying from an external signal as it isn't reported back to the getenvoy process and
@@ -67,7 +70,7 @@ func TestRuntime_Run(t *testing.T) {
 			shutdown:       func() { time.Sleep(time.Millisecond * 100) },
 			args:           []string{}, // no config file!
 			expectedStdout: fmt.Sprintln("starting:", fakeEnvoy, adminFlag),
-			expectedStderr: "initializing epoch 0\nexiting\nAt least one of --config-path or --config-yaml or Options::configProto() should be non-empty\n",
+			expectedStderr: fmt.Sprintf("initializing epoch 0%[1]sexiting%[1]sAt least one of --config-path or --config-yaml or Options::configProto() should be non-empty%[1]s", ln),
 			expectedErr:    "envoy exited with status: 1",
 		},
 	}
diff --git a/internal/envoy/version_test.go b/internal/envoy/version_test.go
index 63d6d18..afe7fcf 100644
--- a/internal/envoy/version_test.go
+++ b/internal/envoy/version_test.go
@@ -58,13 +58,6 @@ func TestGetHomeVersion_Empty(t *testing.T) {
 	}
 }
 
-func TestTrimNewline(t *testing.T) {
-	require.Equal(t, "", trimNewline([]byte{}))
-	require.Equal(t, "", trimNewline([]byte("\n")))
-	require.Equal(t, "1.15.5", trimNewline([]byte("1.15.5")))
-	require.Equal(t, "1.15.5", trimNewline([]byte("1.15.5\n")))
-}
-
 func TestGetHomeVersion(t *testing.T) {
 	homeDir, removeHomeDir := morerequire.RequireNewTempDir(t)
 	defer removeHomeDir()
@@ -86,7 +79,7 @@ func TestGetHomeVersion_Validates(t *testing.T) {
 	require.NoError(t, os.WriteFile(homeVersionFile, []byte("a.a.a"), 0600))
 
 	_, _, err := GetHomeVersion(homeDir)
-	require.EqualError(t, err, fmt.Sprintf(`invalid version in "$GETENVOY_HOME/version": "a.a.a" should look like "%s"`, version.LastKnownEnvoy))
+	require.EqualError(t, err, fmt.Sprintf(`invalid version in "%s": "a.a.a" should look like "%s"`, CurrentVersionHomeDirFile, version.LastKnownEnvoy))
 }
 
 func TestWriteCurrentVersion_HomeDir(t *testing.T) {
@@ -142,7 +135,7 @@ func TestCurrentVersion(t *testing.T) {
 	t.Run("defaults to home version", func(t *testing.T) {
 		v, source, err := CurrentVersion(homeDir)
 		require.Equal(t, "1.1.1", v)
-		require.Equal(t, "$GETENVOY_HOME/version", source)
+		require.Equal(t, CurrentVersionHomeDirFile, source)
 		require.NoError(t, err)
 	})
 
@@ -153,7 +146,7 @@ func TestCurrentVersion(t *testing.T) {
 	t.Run("prefers $PWD/.envoy-version over home version", func(t *testing.T) {
 		v, source, err := CurrentVersion(homeDir)
 		require.Equal(t, "2.2.2", v)
-		require.Equal(t, "$PWD/.envoy-version", source)
+		require.Equal(t, CurrentVersionWorkingDirFile, source)
 		require.NoError(t, err)
 	})
 
@@ -163,7 +156,7 @@ func TestCurrentVersion(t *testing.T) {
 	t.Run("prefers $ENVOY_VERSION over $PWD/.envoy-version", func(t *testing.T) {
 		v, source, err := CurrentVersion(homeDir)
 		require.Equal(t, "3.3.3", v)
-		require.Equal(t, "$ENVOY_VERSION", source)
+		require.Equal(t, currentVersionVar, source)
 		require.NoError(t, err)
 	})
 }
diff --git a/internal/moreos/moreos_test.go b/internal/moreos/moreos_test.go
new file mode 100644
index 0000000..9bbad8e
--- /dev/null
+++ b/internal/moreos/moreos_test.go
@@ -0,0 +1,84 @@
+// Copyright 2021 Tetrate
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package moreos
+
+import (
+	"bytes"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/shirou/gopsutil/v3/process"
+	"github.com/stretchr/testify/require"
+
+	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
+)
+
+func TestIsExecutable(t *testing.T) {
+	tempDir, removeTempDir := morerequire.RequireNewTempDir(t)
+	defer removeTempDir()
+
+	bin := filepath.Join(tempDir, "envoy"+Exe)
+	require.NoError(t, os.WriteFile(bin, []byte{}, 0700))
+
+	f, err := os.Stat(bin)
+	require.NoError(t, err)
+
+	require.True(t, isExecutable(f))
+}
+
+func TestIsExecutable_Not(t *testing.T) {
+	tempDir, removeTempDir := morerequire.RequireNewTempDir(t)
+	defer removeTempDir()
+
+	bin := filepath.Join(tempDir, "foo.txt")
+	require.NoError(t, os.WriteFile(bin, []byte{}, 0600))
+
+	f, err := os.Stat(bin)
+	require.NoError(t, err)
+
+	require.False(t, isExecutable(f))
+}
+
+func TestLineSeparator(t *testing.T) {
+	stdout := new(bytes.Buffer)
+	cmd := exec.Command("echo", "cats")
+	cmd.Stdout = stdout
+	require.NoError(t, cmd.Run())
+	require.Equal(t, "cats"+ln, stdout.String())
+}
+
+func TestProcessGroupAttr_Interrupt(t *testing.T) {
+	// Fork a process that hangs
+	cmd := exec.Command("cat" + Exe)
+	cmd.SysProcAttr = ProcessGroupAttr()
+	require.NoError(t, cmd.Start())
+
+	// Verify the process exists
+	require.NoError(t, findProcess(cmd.Process))
+
+	// Interrupt it
+	require.NoError(t, Interrupt(cmd.Process))
+
+	// Wait for the process to die; this could error due to the interrupt signal
+	cmd.Wait() //nolint
+	require.Error(t, findProcess(cmd.Process))
+}
+
+func findProcess(proc *os.Process) error {
+	_, err := process.NewProcess(int32(proc.Pid)) // because os.FindProcess is no-op in Linux!
+	return err
+}
diff --git a/internal/tar/tar_test.go b/internal/tar/tar_test.go
index d0414cc..39df230 100644
--- a/internal/tar/tar_test.go
+++ b/internal/tar/tar_test.go
@@ -132,7 +132,8 @@ func TestUntar(t *testing.T) {
 
 // requireTestFiles ensures the given directory includes the testdata/foo directory
 func requireTestFiles(t *testing.T, dst string) {
-	for _, path := range []string{"bar.sh", "bar/baz.txt"} {
+	// NOTE: this will not include empty.txt as we don't want to clutter the tar with empty files
+	for _, path := range []string{"bar.sh", filepath.Join("bar", "baz.txt")} {
 		want, e := os.Stat(filepath.Join("testdata", "foo", path))
 		require.NoError(t, e)
 		have, e := os.Stat(filepath.Join(dst, path))
diff --git a/internal/tar/testdata/foo/bar.sh b/internal/tar/testdata/foo/bar.sh
index e69de29..1a24852 100755
--- a/internal/tar/testdata/foo/bar.sh
+++ b/internal/tar/testdata/foo/bar.sh
@@ -0,0 +1,1 @@
+#!/bin/sh
diff --git a/internal/tar/testdata/foo/bar/baz.txt b/internal/tar/testdata/foo/bar/baz.txt
index e69de29..d00491f 100644
--- a/internal/tar/testdata/foo/bar/baz.txt
+++ b/internal/tar/testdata/foo/bar/baz.txt
@@ -0,0 +1,1 @@
+1
diff --git a/internal/tar/testdata/foo/bar/empty.txt b/internal/tar/testdata/foo/bar/empty.txt
new file mode 100644
index 0000000..e69de29
diff --git a/internal/test/envoy.go b/internal/test/envoy.go
index 2d16265..eee68a9 100644
--- a/internal/test/envoy.go
+++ b/internal/test/envoy.go
@@ -52,7 +52,7 @@ func RequireRun(t *testing.T, shutdown func(), r Runner, stderr io.Reader, args
 	reader := bufio.NewReader(stderr)
 	require.Eventually(t, func() bool {
 		b, err := reader.Peek(512)
-		return err != nil && strings.Contains(string(b), "initializing epoch 0\n")
+		return err != nil && strings.Contains(string(b), "initializing epoch 0")
 	}, 2*time.Second, 100*time.Millisecond, "never started process")
 
 	shutdown()
diff --git a/internal/test/server.go b/internal/test/server.go
index e93a8b2..1649f11 100644
--- a/internal/test/server.go
+++ b/internal/test/server.go
@@ -28,6 +28,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/tetratelabs/getenvoy/internal/moreos"
 	"github.com/tetratelabs/getenvoy/internal/tar"
 	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
 	"github.com/tetratelabs/getenvoy/internal/version"
@@ -106,7 +107,7 @@ func requireFakeEnvoyTarGz(t *testing.T, v string) []byte {
 	// construct the platform directory based on the input version
 	installDir := filepath.Join(tempDir, v)
 	require.NoError(t, os.MkdirAll(filepath.Join(installDir, "bin"), 0700)) //nolint:gosec
-	RequireFakeEnvoy(t, filepath.Join(installDir, "bin", "envoy"))
+	RequireFakeEnvoy(t, filepath.Join(installDir, "bin", "envoy"+moreos.Exe))
 
 	// tar.gz the platform dir
 	tempGz := filepath.Join(tempDir, "envoy.tar.gz")