Implement fifo based shell env collector

.github/workflows/ci.yml

@@ -28,7 +28,7 @@ jobs:
       # to make sure that `make lint` also works.
       - name: Run lint from Makefile
         run: make lint
-      - uses: pre-commit/action@v3.0.0
+      - uses: pre-commit/action@v3.0.1
       - name: pre-commit
         run: pre-commit run --files $(git diff-tree --no-commit-id --name-only -r HEAD)
 

.pre-commit-config.yaml

@@ -3,23 +3,53 @@ repos:
     rev: v4.5.0
     hooks:
       - id: check-yaml
+        stages: [commit]
+      - id: check-json
+        exclude: "^.vscode/"
+        stages: [commit]
+      - id: check-merge-conflict
+        stages: [commit]
+      - id: check-case-conflict
+        stages: [commit]
+      - id: detect-private-key
+        stages: [commit]
       - id: end-of-file-fixer
+        stages: [commit]
       - id: trailing-whitespace
-  - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.4
+        stages: [commit]
+  - repo: https://github.com/crate-ci/committed
+    rev: v1.0.20
     hooks:
-      - id: codespell
-        args: ["-L=ndoes,nd,clen"] # 2x words that are preceeded with "\n"; cLen
-        exclude: (package.*\.json$|go\.sum|pkg/api/gen)
-  - repo: https://github.com/TekWizely/pre-commit-golang
-    rev: v1.0.0-rc.1
+      - id: committed
+        stages: [commit-msg]
+  - repo: https://github.com/crate-ci/typos
+    rev: v1.22.9
+    hooks:
+      - id: typos
+        args: ["--diff", "--force-exclude"]
+        exclude: "^.vscode/|go.mod"
+  - repo: local
     hooks:
-      - id: go-fumpt
-        exclude: (^pkg/api/gen/.*$|^internal/client/graphql/query/generated.go$)
-      - id: go-build-mod
       - id: go-mod-tidy
-      - id: go-revive-repo-mod
-      - id: go-staticcheck-mod
-        exclude: (^expect/.*$|^internal/client/graphql/query/generated.go$)
-      - id: go-sec-repo-mod
-        args: ["-exclude=G204,G304,G404", "-exclude-generated"]
+        name: go mod tidy
+        entry: go
+        args: ["mod", "tidy"]
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]
+      - id: lint
+        name: lint
+        entry: make
+        args: ["lint"]
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]
+      - id: build
+        name: build
+        entry: make
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]

.typos.toml

@@ -0,0 +1,6 @@
+[default]
+extend-ignore-re = [
+    "\"data\":\"\\b[0-9A-Za-z+/]+(=|==)?\\b\"", # base64 blobs in some structs
+    "\"id\":\"[0-9A-Z]+\"",                     # cell IDs
+    "thi\\.\\.\\.ice",
+]

Makefile

@@ -95,7 +95,14 @@ fmt:
 
 .PHONY: lint
 lint:
-	@revive -config revive.toml -formatter stylish -exclude integration/subject/... ./...
+	@gofumpt -d .
+	@revive \
+		-config revive.toml \
+		-formatter stylish \
+		-exclude integration/subject/... \
+		./...
+	@staticcheck ./...
+	@gosec -quiet -exclude=G204,G304,G404 -exclude-generated ./...
 
 .PHONY: pre-commit
 pre-commit: build wasm test lint

internal/cmd/beta/run_cmd.go

@@ -115,8 +115,12 @@ func runCodeBlock(
 	if err != nil {
 		return err
 	}
-	cmd := factory.Build(cfg, options)
-	if err := cmd.Start(ctx); err != nil {
+	cmd, err := factory.Build(cfg, options)
+	if err != nil {
+		return err
+	}
+	err = cmd.Start(ctx)
+	if err != nil {
 		return err
 	}
 	return cmd.Wait()

internal/cmd/environment.go

@@ -3,25 +3,28 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 	"time"
 
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/go-gh/pkg/tableprinter"
 	"github.com/pkg/errors"
-
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 
+	"github.com/stateful/runme/v3/internal/command"
 	"github.com/stateful/runme/v3/internal/owl"
 	"github.com/stateful/runme/v3/internal/runner/client"
 	runmetls "github.com/stateful/runme/v3/internal/tls"
 	runnerv1 "github.com/stateful/runme/v3/pkg/api/gen/proto/go/runme/runner/v1"
 )
 
-var osEnviron = os.Environ
+var newOSEnvironReader = func() (io.Reader, error) {
+	return command.NewEnvProducerFromEnv()
+}
 
 func environmentCmd() *cobra.Command {
 	cmd := cobra.Command{
@@ -204,9 +207,12 @@ func environmentDumpCmd() *cobra.Command {
 				return errors.New("must be run in insecure mode; enable by running with --insecure flag")
 			}
 
-			dumped := getDumpedEnvironment()
+			producer, err := newOSEnvironReader()
+			if err != nil {
+				return err
+			}
 
-			_, _ = cmd.OutOrStdout().Write([]byte(dumped))
+			_, _ = io.Copy(cmd.OutOrStdout(), producer)
 
 			return nil
 		},
@@ -269,7 +275,3 @@ func printStore(msgData *runnerv1.MonitorEnvStoreResponse_Snapshot, lines int, a
 
 	return table.Render()
 }
-
-func getDumpedEnvironment() string {
-	return strings.Join(osEnviron(), "\x00")
-}

internal/cmd/environment_test.go

@@ -2,48 +2,43 @@ package cmd
 
 import (
 	"bytes"
+	"io"
 	"os/exec"
 	"strings"
 	"testing"
 
+	"github.com/stateful/runme/v3/internal/command"
 	"github.com/stretchr/testify/assert"
 )
 
-func runEnvCommand(...string) *exec.Cmd {
-	return exec.Command("env", "-0")
-}
-
-func Test_cmdEnvironment(t *testing.T) {
-	t.Run("Dump", func(t *testing.T) {
-		t.Parallel()
-
-		cmd := runEnvCommand()
+func Test_cmdEnvironment_Dump(t *testing.T) {
+	t.Parallel()
 
-		stdout, stderr := new(bytes.Buffer), new(bytes.Buffer)
+	env := []string{
+		"A=1",
+		"B=2",
+		"C=3",
+	}
 
-		cmd.Stdout = stdout
-		cmd.Stderr = stderr
+	stdout, stderr := new(bytes.Buffer), new(bytes.Buffer)
 
-		env := []string{
-			"A=1",
-			"B=2",
-			"C=3",
-		}
+	cmd := runEnvCommand()
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	cmd.Env = env
 
-		cmd.Env = env
+	original := newOSEnvironReader
+	defer func() { newOSEnvironReader = original }()
+	newOSEnvironReader = func() (io.Reader, error) {
+		return strings.NewReader(command.EnvSliceToString(env)), nil
+	}
 
-		err := cmd.Run()
-
-		old := osEnviron
-		defer func() { osEnviron = old }()
-
-		osEnviron = func() []string {
-			return env
-		}
+	err := cmd.Run()
+	assert.NoError(t, err)
+	assert.Equal(t, "", stderr.String())
+	assert.Equal(t, true, strings.HasPrefix(stdout.String(), "A=1\x00B=2\x00C=3\x00"))
+}
 
-		assert.NoError(t, err)
-		assert.Equal(t, "", stderr.String())
-		assert.Equal(t, true, strings.HasPrefix(stdout.String(), "A=1\x00B=2\x00C=3\x00"))
-		assert.Equal(t, "A=1\x00B=2\x00C=3", getDumpedEnvironment())
-	})
+func runEnvCommand(...string) *exec.Cmd {
+	return exec.Command("env", "-0")
 }

internal/command/command.go

@@ -36,14 +36,13 @@ type internalCommand interface {
 }
 
 type base struct {
-	cfg         *ProgramConfig
-	project     *project.Project
-	runtime     runtime
-	session     *Session
-	stdin       io.Reader
-	stdinWriter io.Writer
-	stdout      io.Writer
-	stderr      io.Writer
+	cfg     *ProgramConfig
+	project *project.Project
+	runtime runtime
+	session *Session
+	stdin   io.Reader
+	stdout  io.Writer
+	stderr  io.Writer
 }
 
 var _ internalCommand = (*base)(nil)
@@ -174,10 +173,6 @@ func (c *base) Stdin() io.Reader {
 	return c.stdin
 }
 
-func (c *base) StdinWriter() io.Writer {
-	return c.stdinWriter
-}
-
 func (c *base) Stdout() io.Writer {
 	if c.stdout == nil {
 		c.stdout = io.Discard

internal/command/command_docker_test.go

@@ -25,7 +25,7 @@ func TestDockerCommand(t *testing.T) {
 
 	// This test case is treated as a warm up. Do not parallelize.
 	t.Run("NoOutput", func(t *testing.T) {
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "echo",
 				Arguments:   []string{"-n", "test"},
@@ -34,14 +34,15 @@ func TestDockerCommand(t *testing.T) {
 			},
 			CommandOptions{},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
 		require.NoError(t, cmd.Wait())
 	})
 
 	t.Run("Output", func(t *testing.T) {
 		t.Parallel()
 		stdout := bytes.NewBuffer(nil)
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "echo",
 				Arguments:   []string{"-n", "test"},
@@ -50,44 +51,42 @@ func TestDockerCommand(t *testing.T) {
 			},
 			CommandOptions{Stdout: stdout},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
 		require.NoError(t, cmd.Wait())
 		assert.Equal(t, "test", stdout.String())
 	})
 
 	t.Run("Running", func(t *testing.T) {
 		t.Parallel()
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "sleep",
 				Arguments:   []string{"1"},
 				Mode:        runnerv2alpha1.CommandMode_COMMAND_MODE_INLINE,
 			},
 			CommandOptions{},
 		)
-		err := cmd.Start(context.Background())
 		require.NoError(t, err)
+		require.NoError(t, cmd.Start(context.Background()))
 		require.True(t, cmd.Running())
 		require.Greater(t, cmd.Pid(), 0)
 		require.NoError(t, cmd.Wait())
 	})
 
 	t.Run("NonZeroExit", func(t *testing.T) {
-		t.Skip("enable when [envCollector] supports [Runtime]")
-
 		t.Parallel()
 
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "sh",
 				Arguments:   []string{"-c", "exit 11"},
 				Mode:        runnerv2alpha1.CommandMode_COMMAND_MODE_INLINE,
 			},
 			CommandOptions{},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
-		// TODO(adamb): wait should return non-nil error due to non-zero exit code.
-		require.NoError(t, cmd.Wait())
-		require.Equal(t, 11, cmd.(*dockerCommand).cmd.ProcessState.ExitCode)
+		require.Error(t, cmd.Wait(), "exit code 11 due to error \"\"")
 	})
 }