Merge pull request #4769 from laurazard/signal-handling-fix-tty

thaJeztah · web-flow · commit 688de6db1636 · 2024-01-12T22:06:23.000+01:00
plugins: run plugin with new process group ID
diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go
@@ -233,6 +233,7 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
 		cmd.Stdin = os.Stdin
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
+		configureOSSpecificCommand(cmd)
 
 		cmd.Env = os.Environ()
 		cmd.Env = append(cmd.Env, ReexecEnvvar+"="+os.Args[0])
diff --git a/cli-plugins/manager/manager_unix.go b/cli-plugins/manager/manager_unix.go
@@ -2,7 +2,21 @@
 
 package manager
 
+import (
+	"os/exec"
+	"syscall"
+)
+
 var defaultSystemPluginDirs = []string{
 	"/usr/local/lib/docker/cli-plugins", "/usr/local/libexec/docker/cli-plugins",
 	"/usr/lib/docker/cli-plugins", "/usr/libexec/docker/cli-plugins",
 }
+
+func configureOSSpecificCommand(cmd *exec.Cmd) {
+	// Spawn the plugin process in a new process group, so that signals are not forwarded by the OS.
+	// The foreground process group is e.g. sent a SIGINT when Ctrl-C is input to the TTY, but we
+	// implement our own job control for the plugin.
+	cmd.SysProcAttr = &syscall.SysProcAttr{
+		Setpgid: true,
+	}
+}
diff --git a/cli-plugins/manager/manager_windows.go b/cli-plugins/manager/manager_windows.go
@@ -2,10 +2,15 @@ package manager
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
 )
 
 var defaultSystemPluginDirs = []string{
 	filepath.Join(os.Getenv("ProgramData"), "Docker", "cli-plugins"),
 	filepath.Join(os.Getenv("ProgramFiles"), "Docker", "cli-plugins"),
 }
+
+func configureOSSpecificCommand(cmd *exec.Cmd) {
+	// no-op
+}
diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go
@@ -240,12 +240,16 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string,
 	// we send a SIGKILL to the plugin process and exit
 	go func() {
 		retries := 0
-		for range signals {
+		for s := range signals {
 			if conn != nil {
 				if err := conn.Close(); err != nil {
 					_, _ = fmt.Fprintf(dockerCli.Err(), "failed to signal plugin to close: %v\n", err)
 				}
 				conn = nil
+			} else {
+				// When the plugin is communicating via socket with the host CLI, we perform job control via the socket.
+				// However, if the plugin is an old version that is not socket-aware, we need to explicitly forward termination signals.
+				plugincmd.Process.Signal(s)
 			}
 			retries++
 			if retries >= exitLimit {

Original file line number	Diff line number	Diff line change
`@@ -2,10 +2,15 @@ package manager`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"os"`
	`5`	`+ "os/exec"`
`5`	`6`	`"path/filepath"`
`6`	`7`	`)`
`7`	`8`
`8`	`9`	`var defaultSystemPluginDirs = []string{`
`9`	`10`	`filepath.Join(os.Getenv("ProgramData"), "Docker", "cli-plugins"),`
`10`	`11`	`filepath.Join(os.Getenv("ProgramFiles"), "Docker", "cli-plugins"),`
`11`	`12`	`}`
	`13`	`+`
	`14`	`+func configureOSSpecificCommand(cmd *exec.Cmd) {`
	`15`	`+ // no-op`
	`16`	`+}`