cmd: add the ability to attach a pty to a local command (#6379)

Note that this doesn't wire anything up to
the tiltfile. That will come in a subsequent PR.

Signed-off-by: Nick Santos <[email protected]>

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/alessio/shellescape v1.4.1
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/compose-spec/compose-go v1.20.2
+	github.com/creack/pty v1.1.18
 	github.com/davecgh/go-spew v1.1.1
 	github.com/distribution/reference v0.5.0
 	github.com/docker/cli v24.0.6+incompatible

internal/controllers/core/cmd/controller.go

@@ -391,9 +391,10 @@ func (c *Controller) runInternal(ctx context.Context,
 	}
 
 	cmdModel := model.Cmd{
-		Argv: spec.Args,
-		Dir:  spec.Dir,
-		Env:  env,
+		Argv:      spec.Args,
+		Dir:       spec.Dir,
+		Env:       env,
+		StdinMode: spec.StdinMode,
 	}
 	statusCh := c.execer.Start(ctx, cmdModel, logger.Get(ctx).Writer(logger.InfoLvl))
 	proc.doneCh = make(chan struct{})

internal/controllers/core/cmd/controller_test.go

@@ -869,6 +869,23 @@ func TestCmdOnlyUsesButtonThatStartedIt(t *testing.T) {
 	require.Equal(t, expectedEnv, actualEnv)
 }
 
+func TestServeStdinPty(t *testing.T) {
+	f := newFixture(t)
+
+	t1 := time.Unix(1, 0)
+	c := model.ToHostCmd("my-server")
+	c.StdinMode = v1alpha1.StdinModePty
+	localTarget := model.NewLocalTarget(
+		model.TargetName("foo"), model.Cmd{}, c, nil)
+	f.resourceFromTarget("foo", localTarget, t1)
+	f.step()
+
+	f.fe.mu.Lock()
+	sm := f.fe.processes["my-server"].stdinMode
+	f.fe.mu.Unlock()
+	assert.Equal(t, v1alpha1.StdinModePty, sm)
+}
+
 type testStore struct {
 	*store.TestingStore
 	out     io.Writer

internal/controllers/core/cmd/execer.go

@@ -4,15 +4,18 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"os"
 	"os/exec"
 	"sync"
 	"syscall"
 	"testing"
 	"time"
 
+	"github.com/creack/pty"
 	"github.com/stretchr/testify/require"
 
 	"github.com/tilt-dev/tilt/internal/localexec"
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 	"github.com/tilt-dev/tilt/pkg/logger"
 	"github.com/tilt-dev/tilt/pkg/model"
 	"github.com/tilt-dev/tilt/pkg/procutil"
@@ -32,6 +35,7 @@ type fakeExecProcess struct {
 	workdir   string
 	env       []string
 	startTime time.Time
+	stdinMode v1alpha1.StdinMode
 }
 
 type FakeExecer struct {
@@ -69,6 +73,7 @@ func (e *FakeExecer) Start(ctx context.Context, cmd model.Cmd, w io.Writer) chan
 		workdir:   cmd.Dir,
 		startTime: time.Now(),
 		env:       cmd.Env,
+		stdinMode: cmd.StdinMode,
 	}
 	e.mu.Unlock()
 
@@ -176,7 +181,16 @@ func (e *processExecer) processRun(ctx context.Context, cmd model.Cmd, w io.Writ
 	c.Stderr = w
 	c.Stdout = w
 
-	err = c.Start()
+	if cmd.StdinMode == v1alpha1.StdinModePty {
+		var f *os.File
+		f, err = pty.StartWithAttrs(c, nil, c.SysProcAttr)
+		if f != nil {
+			defer f.Close()
+		}
+	} else {
+		err = c.Start()
+	}
+
 	if err != nil {
 		logger.Get(ctx).Errorf("%s failed to start: %v", cmd.String(), err)
 		statusCh <- statusAndMetadata{

internal/controllers/core/cmd/execer_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/testutils"
 	"github.com/tilt-dev/tilt/internal/testutils/bufsync"
 	"github.com/tilt-dev/tilt/internal/testutils/tempdir"
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 	"github.com/tilt-dev/tilt/pkg/logger"
 	"github.com/tilt-dev/tilt/pkg/model"
 )
@@ -38,7 +39,7 @@ func TestWorkdir(t *testing.T) {
 		cmd = "cd"
 	}
 
-	f.startWithWorkdir(cmd, d.Path())
+	f.withWorkdir(d.Path()).start(cmd)
 
 	f.assertCmdSucceeds()
 	f.assertLogContains(d.Path())
@@ -194,6 +195,8 @@ type processExecFixture struct {
 	execer     *processExecer
 	testWriter *bufsync.ThreadSafeBuffer
 	statusCh   chan statusAndMetadata
+	workdir    string
+	stdinMode  v1alpha1.StdinMode
 }
 
 func newProcessExecFixture(t *testing.T) *processExecFixture {
@@ -209,6 +212,8 @@ func newProcessExecFixture(t *testing.T) *processExecFixture {
 		cancel:     cancel,
 		execer:     execer,
 		testWriter: testWriter,
+		workdir:    ".",
+		stdinMode:  v1alpha1.StdinModeDefault,
 	}
 
 	t.Cleanup(ret.tearDown)
@@ -224,14 +229,21 @@ func (f *processExecFixture) startMalformedCommand() {
 	f.statusCh = f.execer.Start(f.ctx, c, f.testWriter)
 }
 
-func (f *processExecFixture) startWithWorkdir(cmd string, workdir string) {
-	c := model.ToHostCmd(cmd)
-	c.Dir = workdir
-	f.statusCh = f.execer.Start(f.ctx, c, f.testWriter)
+func (f *processExecFixture) withWorkdir(workdir string) *processExecFixture {
+	f.workdir = workdir
+	return f
+}
+
+func (f *processExecFixture) withStdinMode(stdinMode v1alpha1.StdinMode) *processExecFixture {
+	f.stdinMode = stdinMode
+	return f
 }
 
 func (f *processExecFixture) start(cmd string) {
-	f.startWithWorkdir(cmd, ".")
+	c := model.ToHostCmd(cmd)
+	c.Dir = f.workdir
+	c.StdinMode = f.stdinMode
+	f.statusCh = f.execer.Start(f.ctx, c, f.testWriter)
 }
 
 func (f *processExecFixture) assertCmdSucceeds() {

internal/controllers/core/cmd/execer_unix_test.go

@@ -4,8 +4,8 @@
 package cmd
 
 import (
+	"fmt"
 	"os"
-	"runtime"
 	"strconv"
 	"strings"
 	"syscall"
@@ -14,12 +14,31 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 )
 
+func TestStdinModeDefault(t *testing.T) {
+	f := newProcessExecFixture(t)
+	f.withStdinMode(v1alpha1.StdinModeDefault).start(
+		"echo terminal:$(tty)")
+	f.waitForStatus(Done)
+	f.assertLogContains("terminal:not a tty")
+}
+
+func TestStdinModePTY(t *testing.T) {
+	f := newProcessExecFixture(t)
+	f.withStdinMode(v1alpha1.StdinModePty).start(
+		"echo terminal:$(tty)")
+	f.waitForStatus(Done)
+
+	// Verify that it shows the same TTY as the TTY
+	// running the test.
+	fmt.Println("output:", f.testWriter.String())
+	f.assertLogContains("terminal:/dev")
+}
+
 func TestStopsBackgroundGrandchildren(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("no bash on windows")
-	}
 	f := newProcessExecFixture(t)
 
 	f.start(`bash -c 'sleep 100 &

internal/engine/local/servercontroller.go

@@ -138,6 +138,7 @@ func (c *ServerController) determineServers(ctx context.Context, st store.RStore
 				TriggerTime:    mt.State.LastSuccessfulDeployTime,
 				ReadinessProbe: lt.ReadinessProbe,
 				DisableSource:  lt.ServeCmdDisableSource,
+				StdinMode:      lt.ServeCmd.StdinMode,
 			},
 		}
 
@@ -265,6 +266,7 @@ func (c *ServerController) reconcile(ctx context.Context, server CmdServer, owne
 		Dir:            server.Spec.Dir,
 		Env:            server.Spec.Env,
 		ReadinessProbe: server.Spec.ReadinessProbe,
+		StdinMode:      server.Spec.StdinMode,
 	}
 
 	triggerTime := c.createdTriggerTime[name]
@@ -336,6 +338,7 @@ type CmdServerSpec struct {
 	Dir            string
 	Env            []string
 	ReadinessProbe *v1alpha1.Probe
+	StdinMode      v1alpha1.StdinMode
 
 	// Kubernetes tends to represent this as a "generation" field
 	// to force an update.

internal/tiltfile/v1alpha1/types.go

@@ -182,6 +182,7 @@ func (p Plugin) cmd(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tupl
 	var restartOn RestartOnSpec = RestartOnSpec{t: t}
 	var startOn StartOnSpec = StartOnSpec{t: t}
 	var disableSource DisableSource = DisableSource{t: t}
+	var stdinMode string
 	var labels value.StringStringMap
 	var annotations value.StringStringMap
 	err = starkit.UnpackArgs(t, fn.Name(), args, kwargs,
@@ -195,6 +196,7 @@ func (p Plugin) cmd(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tupl
 		"restart_on?", &restartOn,
 		"start_on?", &startOn,
 		"disable_source?", &disableSource,
+		"stdin_mode?", &stdinMode,
 	)
 	if err != nil {
 		return nil, err
@@ -215,6 +217,7 @@ func (p Plugin) cmd(t *starlark.Thread, fn *starlark.Builtin, args starlark.Tupl
 	if disableSource.isUnpacked {
 		obj.Spec.DisableSource = (*v1alpha1.DisableSource)(&disableSource.Value)
 	}
+	obj.Spec.StdinMode = v1alpha1.StdinMode(stdinMode)
 	obj.ObjectMeta.Labels = labels
 	obj.ObjectMeta.Annotations = annotations
 	return p.register(t, obj)

pkg/apis/core/v1alpha1/cmd_types.go

@@ -109,8 +109,26 @@ type CmdSpec struct {
 	//
 	// +optional
 	DisableSource *DisableSource `json:"disableSource,omitempty" protobuf:"bytes,7,opt,name=disableSource"`
+
+	// How stdin is attached to the command.
+	// +optional
+	StdinMode StdinMode `json:"stdinMode,omitempty" protobuf:"bytes,8,opt,name=stdinMode"`
 }
 
+type StdinMode string
+
+const (
+	// Attach a null device (e.g. /dev/null) to the command's stdin.
+	StdinModeDefault StdinMode = ""
+
+	// Attaches a pseudo-terminal to the command's stdin.
+	//
+	// If you're using Tilt's local() or local_resource() to run an interactive
+	// command, some commands may try to attach to the terminal and shutdown Tilt.
+	// Attaching a pty can help make it behave more reliably.
+	StdinModePty StdinMode = "pty"
+)
+
 var _ resource.Object = &Cmd{}
 var _ resourcerest.SingularNameProvider = &Cmd{}
 var _ resourcestrategy.Validater = &Cmd{}