From d8e07c9c47ecd65a2f26cf941108b765771eac95 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Tue, 23 Jan 2024 14:19:33 +0000 Subject: [PATCH 1/3] tests: add tests for `cli-plugins/socket` Signed-off-by: Laura Brehm (cherry picked from commit 469bfc05ed27bf078e0d870bcbf1f855ce814802) Signed-off-by: Sebastiaan van Stijn --- cli-plugins/socket/socket_test.go | 129 ++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 cli-plugins/socket/socket_test.go diff --git a/cli-plugins/socket/socket_test.go b/cli-plugins/socket/socket_test.go new file mode 100644 index 000000000000..d5f001484d93 --- /dev/null +++ b/cli-plugins/socket/socket_test.go @@ -0,0 +1,129 @@ +package socket + +import ( + "io/fs" + "net" + "os" + "runtime" + "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") + checkDirClean(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) + }) +} + +func checkDirClean(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") + if 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") + } + }) + + 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)) +} From 2f6b5ada712308489ce9063160bc2db50d2c3c75 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Thu, 25 Jan 2024 15:07:19 +0000 Subject: [PATCH 2/3] scripts: don't hardcode architecture in e2e script Signed-off-by: Laura Brehm (cherry picked from commit 1c4d6d85dd2563e36b1687a84a687463f0302e5a) Signed-off-by: Sebastiaan van Stijn --- scripts/test/e2e/run | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/test/e2e/run b/scripts/test/e2e/run index 54e1d61e8a34..e2c651ea802c 100755 --- a/scripts/test/e2e/run +++ b/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-} } From 1cbc218c053e92767e6f28bf726318f8c204e8b4 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Thu, 25 Jan 2024 15:37:34 +0000 Subject: [PATCH 3/3] 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 (cherry picked from commit cfa9fef77d37cae7cb383dff77ddf1c977b09dd1) Signed-off-by: Sebastiaan van Stijn --- cli-plugins/socket/socket_test.go | 12 +- e2e/cli-plugins/plugins/presocket/main.go | 123 +++++++++++ e2e/cli-plugins/socket_test.go | 235 ++++++++++++++++++++++ 3 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 e2e/cli-plugins/plugins/presocket/main.go create mode 100644 e2e/cli-plugins/socket_test.go diff --git a/cli-plugins/socket/socket_test.go b/cli-plugins/socket/socket_test.go index d5f001484d93..409eb689485c 100644 --- a/cli-plugins/socket/socket_test.go +++ b/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) diff --git a/e2e/cli-plugins/plugins/presocket/main.go b/e2e/cli-plugins/plugins/presocket/main.go new file mode 100644 index 000000000000..6cdf87a42402 --- /dev/null +++ b/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 +} diff --git a/e2e/cli-plugins/socket_test.go b/e2e/cli-plugins/socket_test.go new file mode 100644 index 000000000000..5e0b1cbb7c8f --- /dev/null +++ b/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") + }) + }) +}