fix: emissary detects tty and wraps command in pseudo terminal

Signed-off-by: Dana Pieluszczak <[email protected]>

cmd/argoexec/commands/emissary.go

@@ -18,8 +18,10 @@ import (
 
 	"github.com/argoproj/argo-workflows/v3/util/errors"
 
+	"github.com/creack/pty"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"golang.org/x/term"
 	"k8s.io/client-go/util/retry"
 
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
@@ -111,26 +113,24 @@ func NewEmissaryCommand() *cobra.Command {
 					break
 				}
 			}
-
 			backoff, err := template.GetRetryStrategy()
 			if err != nil {
 				return fmt.Errorf("failed to get retry strategy: %w", err)
 			}
 
 			cmdErr := retry.OnError(backoff, func(error) bool { return true }, func() error {
-				command, stdout, combined, err := createCommand(name, args, template)
-				if err != nil {
-					return fmt.Errorf("failed to create command: %w", err)
-				}
-				defer stdout.Close()
-				defer combined.Close()
+				// setup signal handlers
 				signals := make(chan os.Signal, 1)
 				defer close(signals)
 				signal.Notify(signals)
 				defer signal.Reset()
-				if err := command.Start(); err != nil {
-					return err
+
+				command, closer, err := startCommand(name, args, template)
+				if err != nil {
+					return fmt.Errorf("failed to start command: %w", err)
 				}
+				defer closer()
+
 				go func() {
 					for s := range signals {
 						if !osspecific.IsSIGCHLD(s) {
@@ -209,32 +209,96 @@ func NewEmissaryCommand() *cobra.Command {
 	}
 }
 
-func createCommand(name string, args []string, template *wfv1.Template) (*exec.Cmd, *os.File, *os.File, error) {
+func startCommand(name string, args []string, template *wfv1.Template) (*exec.Cmd, func(), error) {
+	tty := term.IsTerminal(int(os.Stdin.Fd()))
+
 	command := exec.Command(name, args...)
 	command.Env = os.Environ()
 	command.SysProcAttr = &syscall.SysProcAttr{}
-	osspecific.Setpgid(command.SysProcAttr)
-	command.Stdout = os.Stdout
-	command.Stderr = os.Stderr
+	if !tty {
+		// avoid the error "Inappropriate ioctl for device" when
+		// running in tty
+		//
+		// pty.Start uses setsid internally, which makes the process
+		// the group leader already
+		osspecific.Setpgid(command.SysProcAttr)
+	}
+
+	var closer func() = func() {}
+	var stdout io.Writer = os.Stdout
+	var stderr io.Writer = os.Stderr
 
-	var stdout *os.File
-	var combined *os.File
-	var err error
 	// this may not be that important an optimisation, except for very long logs we don't want to capture
 	if includeScriptOutput || template.SaveLogsAsArtifact() {
 		logger.Info("capturing logs")
-		stdout, err = os.OpenFile(varRunArgo+"/ctr/"+containerName+"/stdout", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
+		stdoutf, err := os.OpenFile(varRunArgo+"/ctr/"+containerName+"/stdout", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("failed to open stdout: %w", err)
+			return nil, nil, fmt.Errorf("failed to open stdout: %w", err)
 		}
-		combined, err = os.OpenFile(varRunArgo+"/ctr/"+containerName+"/combined", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
+		combinedf, err := os.OpenFile(varRunArgo+"/ctr/"+containerName+"/combined", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("failed to open combined: %w", err)
+			return nil, nil, fmt.Errorf("failed to open combined: %w", err)
+		}
+		stdout = io.MultiWriter(stdout, stdoutf, combinedf)
+		stderr = io.MultiWriter(stderr, combinedf)
+
+		closer = func() {
+			_ = stdoutf.Close()
+			_ = combinedf.Close()
 		}
-		command.Stdout = io.MultiWriter(os.Stdout, stdout, combined)
-		command.Stderr = io.MultiWriter(os.Stderr, combined)
 	}
-	return command, stdout, combined, nil
+
+	if tty {
+		logger.Info("container requested TTY")
+		ptmx, err := pty.Start(command)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		// Handle pty size
+		sigWinchCh := make(chan os.Signal, 1)
+		signal.Notify(sigWinchCh, syscall.SIGWINCH)
+		go func() {
+			for range sigWinchCh {
+				if ierr := pty.InheritSize(os.Stdin, ptmx); ierr != nil {
+					logger.WithField("error", ierr).Warn("error resizing pty")
+				}
+			}
+		}()
+		// Initial resize
+		sigWinchCh <- syscall.SIGWINCH
+		// Set stdin in raw mode
+		oldState, err := term.MakeRaw(int(os.Stdin.Fd()))
+		if err != nil {
+			return nil, nil, err
+		}
+
+		// copy from stdin to the pty
+		go func() { _, _ = io.Copy(ptmx, os.Stdin) }()
+		// copy from pty to stdout
+		go func() { _, _ = io.Copy(stdout, ptmx) }()
+		// copy from pty to stderr
+		go func() { _, _ = io.Copy(stderr, ptmx) }()
+
+		origCloser := closer
+		closer = func() {
+			signal.Stop(sigWinchCh)
+			close(sigWinchCh)
+
+			_ = term.Restore(int(os.Stdin.Fd()), oldState)
+			_ = ptmx.Close()
+			origCloser()
+		}
+	} else {
+		command.Stdout = stdout
+		command.Stderr = stderr
+
+		if err := command.Start(); err != nil {
+			return nil, nil, err
+		}
+	}
+
+	return command, closer, nil
 }
 
 func saveArtifact(srcPath string) error {

go.mod

@@ -116,6 +116,7 @@ require (
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
 	github.com/chrismellard/docker-credential-acr-env v0.0.0-20220119192733-fe33c00cee21 // indirect
 	github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
+	github.com/creack/pty v1.1.18
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dimchansky/utfbom v1.1.1 // indirect
 	github.com/docker/cli v20.10.17+incompatible // indirect
@@ -204,7 +205,7 @@ require (
 	github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 // indirect
 	go.opencensus.io v0.23.0 // indirect
 	golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 // indirect
-	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
+	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
 	golang.org/x/text v0.4.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.7 // indirect

go.sum

@@ -309,6 +309,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHH
 github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
+github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
 github.com/daixiang0/gci v0.2.9/go.mod h1:+4dZ7TISfSmqfAGv59ePaHfNzgGtIkHAhhdKggP1JAc=
 github.com/danieljoos/wincred v1.1.0/go.mod h1:XYlo+eRTsVA9aHGp7NGjFkPla4m+DCL7hqDjlFjiygg=
 github.com/dave/jennifer v1.4.1/go.mod h1:7jEdnm+qBcxl8PC0zyp7vxcpSRnzXSt9r39tpTVGlwA=

test/e2e/functional/tty.yaml

@@ -0,0 +1,35 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: tty-
+spec:
+  entrypoint: entrypoint
+  templates:
+    - name: entrypoint
+      steps:
+        - - name: tty-true
+            template: tty-true
+          - name: tty-false
+            template: tty-false
+
+    - name: tty-true
+      script:
+        tty: true
+        stdin: true
+        image: alpine:latest
+        command: [sh]
+        source: |
+          if [[ ! -t 0 ]]; then
+            echo "I should be in a terminal but I'm not"
+            exit 1
+          fi
+
+    - name: tty-false
+      script:
+        image: alpine:latest
+        command: [sh]
+        source: |
+          if [[ -t 0 ]]; then
+            echo "I should not be in a terminal but I am"
+            exit 1
+          fi

test/e2e/functional_test.go

@@ -1154,3 +1154,11 @@ spec:
 		SubmitWorkflow().
 		WaitForWorkflow(fixtures.ToBeFailed)
 }
+
+func (s *FunctionalSuite) TestTTY() {
+	s.Given().
+		Workflow(`@functional/tty.yaml`).
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow(fixtures.ToBeSucceeded)
+}