tests: add plugin-socket-compatibility tests

Adds a new plugin to the e2e plugins that simulates an older
plugin binary and a test suite to ensure older plugin binaries
keep behaving the same with newer CLI versions.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>

cli-plugins/socket/socket_test.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -49,17 +50,17 @@ func TestSetupConn(t *testing.T) {
 		listener, err := SetupConn(&conn)
 		assert.NilError(t, err)
 		assert.Check(t, listener != nil, "returned nil listener but no error")
-		checkDirClean(t)
+		checkDirNoPluginSocket(t)
 
 		addr, err := net.ResolveUnixAddr("unix", listener.Addr().String())
 		assert.NilError(t, err, "failed to resolve listener address")
 		_, err = net.DialUnix("unix", nil, addr)
 		assert.NilError(t, err, "failed to dial returned listener")
-		checkDirClean(t)
+		checkDirNoPluginSocket(t)
 	})
 }
 
-func checkDirClean(t *testing.T) {
+func checkDirNoPluginSocket(t *testing.T) {
 	t.Helper()
 
 	files, err := os.ReadDir(".")
@@ -68,7 +69,8 @@ func checkDirClean(t *testing.T) {
 	for _, f := range files {
 		info, err := f.Info()
 		assert.NilError(t, err, "failed to check file info")
-		if info.Mode().Type() == fs.ModeSocket {
+		// check for a socket with `docker_cli_` in the name (from `SetupConn()`)
+		if strings.Contains(f.Name(), "docker_cli_") && info.Mode().Type() == fs.ModeSocket {
 			t.Fatal("found socket in a local directory")
 		}
 	}
@@ -96,6 +98,8 @@ func TestConnectAndWait(t *testing.T) {
 		}
 	})
 
+	// TODO: this test cannot be executed with `t.Parallel()`, due to
+	// relying on goroutine numbers to ensure correct behaviour
 	t.Run("connect goroutine exits after EOF", func(t *testing.T) {
 		var conn *net.UnixConn
 		listener, err := SetupConn(&conn)

e2e/cli-plugins/plugins/presocket/main.go

@@ -0,0 +1,123 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"os/signal"
+	"syscall"
+	"time"
+
+	"github.com/docker/cli/cli-plugins/manager"
+	"github.com/docker/cli/cli-plugins/plugin"
+	"github.com/docker/cli/cli/command"
+	"github.com/spf13/cobra"
+)
+
+func main() {
+	plugin.Run(RootCmd, manager.Metadata{
+		SchemaVersion: "0.1.0",
+		Vendor:        "Docker Inc.",
+		Version:       "test",
+	})
+}
+
+func RootCmd(dockerCli command.Cli) *cobra.Command {
+	cmd := cobra.Command{
+		Use:   "presocket",
+		Short: "testing plugin that does not connect to the socket",
+		// override PersistentPreRunE so that the plugin default
+		// PersistentPreRunE doesn't run, simulating a plugin built
+		// with a pre-socket-communication version of the CLI
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			return nil
+		},
+	}
+
+	cmd.AddCommand(&cobra.Command{
+		Use:   "test-no-socket",
+		Short: "test command that runs until it receives a SIGINT",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			go func() {
+				<-cmd.Context().Done()
+				fmt.Fprintln(dockerCli.Out(), "context cancelled")
+				os.Exit(2)
+			}()
+			signalCh := make(chan os.Signal, 10)
+			signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM)
+			go func() {
+				for range signalCh {
+					fmt.Fprintln(dockerCli.Out(), "received SIGINT")
+				}
+			}()
+			<-time.After(3 * time.Second)
+			fmt.Fprintln(dockerCli.Err(), "exit after 3 seconds")
+			return nil
+		},
+	})
+
+	cmd.AddCommand(&cobra.Command{
+		Use:   "test-socket",
+		Short: "test command that runs until it receives a SIGINT",
+		PreRunE: func(cmd *cobra.Command, args []string) error {
+			return plugin.PersistentPreRunE(cmd, args)
+		},
+		RunE: func(cmd *cobra.Command, args []string) error {
+			go func() {
+				<-cmd.Context().Done()
+				fmt.Fprintln(dockerCli.Out(), "context cancelled")
+				os.Exit(2)
+			}()
+			signalCh := make(chan os.Signal, 10)
+			signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM)
+			go func() {
+				for range signalCh {
+					fmt.Fprintln(dockerCli.Out(), "received SIGINT")
+				}
+			}()
+			<-time.After(3 * time.Second)
+			fmt.Fprintln(dockerCli.Err(), "exit after 3 seconds")
+			return nil
+		},
+	})
+
+	cmd.AddCommand(&cobra.Command{
+		Use:   "test-socket-ignore-context",
+		Short: "test command that runs until it receives a SIGINT",
+		PreRunE: func(cmd *cobra.Command, args []string) error {
+			return plugin.PersistentPreRunE(cmd, args)
+		},
+		RunE: func(cmd *cobra.Command, args []string) error {
+			signalCh := make(chan os.Signal, 10)
+			signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM)
+			go func() {
+				for range signalCh {
+					fmt.Fprintln(dockerCli.Out(), "received SIGINT")
+				}
+			}()
+			<-time.After(3 * time.Second)
+			fmt.Fprintln(dockerCli.Err(), "exit after 3 seconds")
+			return nil
+		},
+	})
+
+	cmd.AddCommand(&cobra.Command{
+		Use:   "tty",
+		Short: "test command that attempts to read from the TTY",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			done := make(chan struct{})
+			go func() {
+				b := make([]byte, 1)
+				_, _ = dockerCli.In().Read(b)
+				done <- struct{}{}
+			}()
+			select {
+			case <-done:
+			case <-time.After(2 * time.Second):
+				fmt.Fprint(dockerCli.Err(), "timeout after 2 seconds")
+			}
+			return nil
+		},
+	})
+
+	return &cmd
+}

e2e/cli-plugins/socket_test.go

@@ -0,0 +1,235 @@
+package cliplugins
+
+import (
+	"bytes"
+	"io"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/creack/pty"
+	"gotest.tools/v3/assert"
+)
+
+// TestPluginSocketBackwardsCompatible executes a plugin binary
+// that does not connect to the CLI plugin socket, simulating
+// a plugin compiled against an older version of the CLI, and
+// ensures that backwards compatibility is maintained.
+func TestPluginSocketBackwardsCompatible(t *testing.T) {
+	run, _, cleanup := prepare(t)
+	defer cleanup()
+
+	t.Run("attached", func(t *testing.T) {
+		t.Run("the plugin gets signalled if attached to a TTY", func(t *testing.T) {
+			cmd := run("presocket", "test-no-socket")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+
+			ptmx, err := pty.Start(command)
+			assert.NilError(t, err, "failed to launch command with fake TTY")
+
+			// send a SIGINT to the process group after 1 second, since
+			// we're simulating an "attached TTY" scenario and a TTY would
+			// send a signal to the process group
+			go func() {
+				<-time.After(time.Second)
+				err := syscall.Kill(-command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+			}()
+			bytes, err := io.ReadAll(ptmx)
+			if err != nil && !strings.Contains(err.Error(), "input/output error") {
+				t.Fatal("failed to get command output")
+			}
+
+			// the plugin is attached to the TTY, so the parent process
+			// ignores the received signal, and the plugin receives a SIGINT
+			// as well
+			assert.Equal(t, string(bytes), "received SIGINT\r\nexit after 3 seconds\r\n")
+		})
+
+		// ensure that we don't break plugins that attempt to read from the TTY
+		// (see: https://github.com/moby/moby/issues/47073)
+		// (remove me if/when we decide to break compatibility here)
+		t.Run("the plugin can read from the TTY", func(t *testing.T) {
+			cmd := run("presocket", "tty")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+
+			ptmx, err := pty.Start(command)
+			assert.NilError(t, err, "failed to launch command with fake TTY")
+			_, _ = ptmx.Write([]byte("hello!"))
+
+			done := make(chan error)
+			go func() {
+				<-time.After(time.Second)
+				_, err := io.ReadAll(ptmx)
+				done <- err
+			}()
+
+			select {
+			case cmdErr := <-done:
+				if cmdErr != nil && !strings.Contains(cmdErr.Error(), "input/output error") {
+					t.Fatal("failed to get command output")
+				}
+			case <-time.After(5 * time.Second):
+				t.Fatal("timed out! plugin process probably stuck")
+			}
+		})
+	})
+
+	t.Run("detached", func(t *testing.T) {
+		t.Run("the plugin does not get signalled", func(t *testing.T) {
+			cmd := run("presocket", "test-no-socket")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+			t.Log(strings.Join(command.Args, " "))
+			command.SysProcAttr = &syscall.SysProcAttr{
+				Setpgid: true,
+			}
+
+			go func() {
+				<-time.After(time.Second)
+				// we're signalling the parent process directly and not
+				// the process group, since we're testing the case where
+				// the process is detached and not simulating a CTRL-C
+				// from a TTY
+				err := syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+			}()
+			bytes, err := command.CombinedOutput()
+			t.Log("command output: " + string(bytes))
+			assert.NilError(t, err, "failed to run command")
+
+			// the plugin process does not receive a SIGINT
+			// so it exits after 3 seconds and prints this message
+			assert.Equal(t, string(bytes), "exit after 3 seconds\n")
+		})
+
+		t.Run("the main CLI exits after 3 signals", func(t *testing.T) {
+			cmd := run("presocket", "test-no-socket")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+			t.Log(strings.Join(command.Args, " "))
+			command.SysProcAttr = &syscall.SysProcAttr{
+				Setpgid: true,
+			}
+
+			go func() {
+				<-time.After(time.Second)
+				// we're signalling the parent process directly and not
+				// the process group, since we're testing the case where
+				// the process is detached and not simulating a CTRL-C
+				// from a TTY
+				err := syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+				// TODO: look into CLI signal handling, it's currently necessary
+				// to add a short delay between each signal in order for the CLI
+				// process to consistently pick them all up.
+				time.Sleep(50 * time.Millisecond)
+				err = syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+				time.Sleep(50 * time.Millisecond)
+				err = syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+			}()
+			bytes, err := command.CombinedOutput()
+			assert.ErrorContains(t, err, "exit status 1")
+
+			// the plugin process does not receive a SIGINT and does
+			// the CLI cannot cancel it over the socket, so it kills
+			// the plugin process and forcefully exits
+			assert.Equal(t, string(bytes), "got 3 SIGTERM/SIGINTs, forcefully exiting\n")
+		})
+	})
+}
+
+func TestPluginSocketCommunication(t *testing.T) {
+	run, _, cleanup := prepare(t)
+	defer cleanup()
+
+	t.Run("attached", func(t *testing.T) {
+		t.Run("the socket is not closed + the plugin receives a signal due to pgid", func(t *testing.T) {
+			cmd := run("presocket", "test-socket")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+
+			ptmx, err := pty.Start(command)
+			assert.NilError(t, err, "failed to launch command with fake TTY")
+
+			// send a SIGINT to the process group after 1 second, since
+			// we're simulating an "attached TTY" scenario and a TTY would
+			// send a signal to the process group
+			go func() {
+				<-time.After(time.Second)
+				err := syscall.Kill(-command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal process group")
+			}()
+			bytes, err := io.ReadAll(ptmx)
+			if err != nil && !strings.Contains(err.Error(), "input/output error") {
+				t.Fatal("failed to get command output")
+			}
+
+			// the plugin is attached to the TTY, so the parent process
+			// ignores the received signal, and the plugin receives a SIGINT
+			// as well
+			assert.Equal(t, string(bytes), "received SIGINT\r\nexit after 3 seconds\r\n")
+		})
+	})
+
+	t.Run("detached", func(t *testing.T) {
+		t.Run("the plugin does not get signalled", func(t *testing.T) {
+			cmd := run("presocket", "test-socket")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+			outB := bytes.Buffer{}
+			command.Stdout = &outB
+			command.Stderr = &outB
+			command.SysProcAttr = &syscall.SysProcAttr{
+				Setpgid: true,
+			}
+
+			// send a SIGINT to the process group after 1 second
+			go func() {
+				<-time.After(time.Second)
+				err := syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal CLI process")
+			}()
+			err := command.Run()
+			t.Log(outB.String())
+			assert.ErrorContains(t, err, "exit status 2")
+
+			// the plugin does not get signalled, but it does get it's
+			// context cancelled by the CLI through the socket
+			assert.Equal(t, outB.String(), "context cancelled\n")
+		})
+
+		t.Run("the main CLI exits after 3 signals", func(t *testing.T) {
+			cmd := run("presocket", "test-socket-ignore-context")
+			command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
+			command.SysProcAttr = &syscall.SysProcAttr{
+				Setpgid: true,
+			}
+
+			go func() {
+				<-time.After(time.Second)
+				// we're signalling the parent process directly and not
+				// the process group, since we're testing the case where
+				// the process is detached and not simulating a CTRL-C
+				// from a TTY
+				err := syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal CLI process")
+				// TODO: same as above TODO, CLI signal handling is not consistent
+				// with multiple signals without intervals
+				time.Sleep(50 * time.Millisecond)
+				err = syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal CLI process")
+				time.Sleep(50 * time.Millisecond)
+				err = syscall.Kill(command.Process.Pid, syscall.SIGINT)
+				assert.NilError(t, err, "failed to signal CLI process§")
+			}()
+			bytes, err := command.CombinedOutput()
+			assert.ErrorContains(t, err, "exit status 1")
+
+			// the plugin process does not receive a SIGINT and does
+			// not exit after having it's context cancelled, so the CLI
+			// kills the plugin process and forcefully exits
+			assert.Equal(t, string(bytes), "got 3 SIGTERM/SIGINTs, forcefully exiting\n")
+		})
+	})
+}