Merge pull request #4828 from laurazard/plugin-socket-tests

Add tests for CLI/plugin communication (i.e. let's not break everyone's plugins 🥲)

cli-plugins/socket/socket_test.go

@@ -0,0 +1,133 @@
+package socket
+
+import (
+	"io/fs"
+	"net"
+	"os"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/poll"
+)
+
+func TestSetupConn(t *testing.T) {
+	t.Run("updates conn when connected", func(t *testing.T) {
+		var conn *net.UnixConn
+		listener, err := SetupConn(&conn)
+		assert.NilError(t, err)
+		assert.Check(t, listener != nil, "returned nil listener but no error")
+		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")
+
+		pollConnNotNil(t, &conn)
+	})
+
+	t.Run("allows reconnects", func(t *testing.T) {
+		var conn *net.UnixConn
+		listener, err := SetupConn(&conn)
+		assert.NilError(t, err)
+		assert.Check(t, listener != nil, "returned nil listener but no error")
+		addr, err := net.ResolveUnixAddr("unix", listener.Addr().String())
+		assert.NilError(t, err, "failed to resolve listener address")
+
+		otherConn, err := net.DialUnix("unix", nil, addr)
+		assert.NilError(t, err, "failed to dial returned listener")
+
+		otherConn.Close()
+
+		_, err = net.DialUnix("unix", nil, addr)
+		assert.NilError(t, err, "failed to redial listener")
+	})
+
+	t.Run("does not leak sockets to local directory", func(t *testing.T) {
+		var conn *net.UnixConn
+		listener, err := SetupConn(&conn)
+		assert.NilError(t, err)
+		assert.Check(t, listener != nil, "returned nil listener but no error")
+		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")
+		checkDirNoPluginSocket(t)
+	})
+}
+
+func checkDirNoPluginSocket(t *testing.T) {
+	t.Helper()
+
+	files, err := os.ReadDir(".")
+	assert.NilError(t, err, "failed to list files in dir to check for leaked sockets")
+
+	for _, f := range files {
+		info, err := f.Info()
+		assert.NilError(t, err, "failed to check file info")
+		// 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")
+		}
+	}
+}
+
+func TestConnectAndWait(t *testing.T) {
+	t.Run("calls cancel func on EOF", func(t *testing.T) {
+		var conn *net.UnixConn
+		listener, err := SetupConn(&conn)
+		assert.NilError(t, err, "failed to setup listener")
+
+		done := make(chan struct{})
+		t.Setenv(EnvKey, listener.Addr().String())
+		cancelFunc := func() {
+			done <- struct{}{}
+		}
+		ConnectAndWait(cancelFunc)
+		pollConnNotNil(t, &conn)
+		conn.Close()
+
+		select {
+		case <-done:
+		case <-time.After(10 * time.Millisecond):
+			t.Fatal("cancel function not closed after 10ms")
+		}
+	})
+
+	// 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)
+		assert.NilError(t, err, "failed to setup listener")
+		t.Setenv(EnvKey, listener.Addr().String())
+		numGoroutines := runtime.NumGoroutine()
+
+		ConnectAndWait(func() {})
+		assert.Equal(t, runtime.NumGoroutine(), numGoroutines+1)
+
+		pollConnNotNil(t, &conn)
+		conn.Close()
+		poll.WaitOn(t, func(t poll.LogT) poll.Result {
+			if runtime.NumGoroutine() > numGoroutines+1 {
+				return poll.Continue("waiting for connect goroutine to exit")
+			}
+			return poll.Success()
+		}, poll.WithDelay(1*time.Millisecond), poll.WithTimeout(10*time.Millisecond))
+	})
+}
+
+func pollConnNotNil(t *testing.T, conn **net.UnixConn) {
+	t.Helper()
+
+	poll.WaitOn(t, func(t poll.LogT) poll.Result {
+		if *conn == nil {
+			return poll.Continue("waiting for conn to not be nil")
+		}
+		return poll.Success()
+	}, poll.WithDelay(1*time.Millisecond), poll.WithTimeout(10*time.Millisecond))
+}

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")
+		})
+	})
+}

scripts/test/e2e/run

@@ -2,6 +2,8 @@
 # Run integration tests against the latest docker-ce dind
 set -eu -o pipefail
 
+source ./scripts/build/.variables
+
 container_ip() {
     local cid=$1
     local network=$2
@@ -69,7 +71,7 @@ runtests() {
         GOPATH="$GOPATH" \
         PATH="$PWD/build/:/usr/bin:/usr/local/bin:/usr/local/go/bin" \
         HOME="$HOME" \
-        DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS="$PWD/build/plugins-linux-amd64" \
+        DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS="$PWD/build/plugins-linux-${GOARCH}" \
         GO111MODULE=auto \
         "$(command -v gotestsum)" -- ${TESTDIRS:-./e2e/...} ${TESTFLAGS-}
 }