From d621219340681a34fe3f70a8cf9fbbe94b38528d Mon Sep 17 00:00:00 2001 From: Richard Trinh Date: Fri, 25 Jul 2025 11:46:31 -0700 Subject: [PATCH] bug: fix multiple connections to same listen port hanging CLI (#5939) * fix: return error when listen port already in use * add err handling for proxy start * add listen port and DefaultClientPort tests * add err handling for listen info --- internal/cmd/commands/connect/connect.go | 16 ++- .../target_tcp_connect_listen_port_test.go | 134 ++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go diff --git a/internal/cmd/commands/connect/connect.go b/internal/cmd/commands/connect/connect.go index 41a660c7ea..a6df08865e 100644 --- a/internal/cmd/commands/connect/connect.go +++ b/internal/cmd/commands/connect/connect.go @@ -434,6 +434,7 @@ func (c *Command) Run(args []string) (retCode int) { c.PrintCliError(fmt.Errorf("Error parsing listen address: %w", err)) return base.CommandCliError } + listenAddr = netip.AddrPortFrom(addr, uint16(c.flagListenPort)) connsLeftCh := make(chan int32) @@ -458,7 +459,10 @@ func (c *Command) Run(args []string) (retCode int) { proxyError := new(atomic.Error) go func() { defer close(clientProxyCloseCh) - proxyError.Store(clientProxy.Start()) + if err = clientProxy.Start(); err != nil { + c.proxyCancel() + proxyError.Store(err) + } }() go func() { defer close(connCountCloseCh) @@ -482,7 +486,15 @@ func (c *Command) Run(args []string) (retCode int) { // The only way a user will be able to connect to the session is by // connecting directly to the port and address we report to them here. - proxyAddr := clientProxy.ListenerAddress(context.Background()) + proxyAddr := clientProxy.ListenerAddress(c.proxyCtx) + if proxyAddr == "" { + if err := proxyError.Load(); err != nil { + c.PrintCliError(fmt.Errorf("Error starting proxy: %w", err)) + return base.CommandCliError + } + c.PrintCliError(fmt.Errorf("Error starting proxy: no address returned")) + return base.CommandCliError + } var clientProxyHost, clientProxyPort string clientProxyHost, clientProxyPort, err = util.SplitHostPort(proxyAddr) if err != nil && !errors.Is(err, util.ErrMissingPort) { diff --git a/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go b/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go new file mode 100644 index 0000000000..90bef7d63f --- /dev/null +++ b/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go @@ -0,0 +1,134 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package base_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/hashicorp/boundary/internal/session" + "github.com/hashicorp/boundary/internal/target" + "github.com/hashicorp/boundary/testing/internal/e2e" + "github.com/hashicorp/boundary/testing/internal/e2e/boundary" +) + +// TestCliTcpTargetConnectListenPort uses the boundary cli to create a target and +// connect to it multiple times on the same listen port. The cli should return an error when +// a port is already in use. +func TestCliTcpTargetConnectListenPort(t *testing.T) { + e2e.MaybeSkipTest(t) + c, err := loadTestConfig() + require.NoError(t, err) + + ctx := context.Background() + boundary.AuthenticateAdminCli(t, ctx) + orgId, err := boundary.CreateOrgCli(t, ctx) + require.NoError(t, err) + t.Cleanup(func() { + ctx := context.Background() + boundary.AuthenticateAdminCli(t, ctx) + output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("scopes", "delete", "-id", orgId)) + require.NoError(t, output.Err, string(output.Stderr)) + }) + projectId, err := boundary.CreateProjectCli(t, ctx, orgId) + require.NoError(t, err) + targetId, err := boundary.CreateTargetCli(t, ctx, projectId, c.TargetPort, + target.WithAddress(c.TargetAddress), + ) + require.NoError(t, err) + + // Connect to target with client port 3333 + const ListenPort = "3333" + + ctxCancel, cancel := context.WithCancel(ctx) + sessionChannel := make(chan *e2e.CommandResult) + + go func() { + sessionChannel <- e2e.RunCommand(ctxCancel, "boundary", + e2e.WithArgs( + "connect", + "-target-id", targetId, + "-listen-port", ListenPort, + ), + ) + }() + t.Cleanup(cancel) + + s := boundary.WaitForSessionCli(t, ctx, projectId) + boundary.WaitForSessionStatusCli(t, ctx, s.Id, session.StatusPending.String()) + t.Logf("Successfully connected to target with listen port %s", ListenPort) + + t.Logf("Starting new connection to target with same listen port %s", ListenPort) + output := e2e.RunCommand(ctx, "boundary", + e2e.WithArgs( + "connect", + "-target-id", targetId, + "-listen-port", ListenPort, + ), + ) + + require.Error(t, output.Err, "Expected error when connecting to target with same listen port") + require.Contains(t, string(output.Stderr), "address already in use") + require.Equal(t, 2, output.ExitCode) +} + +// TestCliTcpTargetConnectDefaultClientPort uses the boundary cli to create a target and +// connect to it multiple times on the same client port. The test is similar to the one above, +// but sets DefaultClientPort instead of ListenPort +func TestCliTcpTargetConnectDefaultClientPort(t *testing.T) { + const DefaultClientPort = 3333 + e2e.MaybeSkipTest(t) + c, err := loadTestConfig() + require.NoError(t, err) + + ctx := context.Background() + boundary.AuthenticateAdminCli(t, ctx) + orgId, err := boundary.CreateOrgCli(t, ctx) + require.NoError(t, err) + t.Cleanup(func() { + ctx := context.Background() + boundary.AuthenticateAdminCli(t, ctx) + output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("scopes", "delete", "-id", orgId)) + require.NoError(t, output.Err, string(output.Stderr)) + }) + projectId, err := boundary.CreateProjectCli(t, ctx, orgId) + require.NoError(t, err) + targetId, err := boundary.CreateTargetCli(t, ctx, projectId, c.TargetPort, + target.WithAddress(c.TargetAddress), + target.WithDefaultClientPort(DefaultClientPort), + ) + require.NoError(t, err) + + // Connect to target with DefaultClientPort set + ctxCancel, cancel := context.WithCancel(ctx) + sessionChannel := make(chan *e2e.CommandResult) + + go func() { + sessionChannel <- e2e.RunCommand(ctxCancel, "boundary", + e2e.WithArgs( + "connect", + "-target-id", targetId, + ), + ) + }() + t.Cleanup(cancel) + + s := boundary.WaitForSessionCli(t, ctx, projectId) + boundary.WaitForSessionStatusCli(t, ctx, s.Id, session.StatusPending.String()) + t.Logf("Successfully connected to target with client port %d", DefaultClientPort) + + t.Logf("Starting new connection to target on client port %d", DefaultClientPort) + output := e2e.RunCommand(ctx, "boundary", + e2e.WithArgs( + "connect", + "-target-id", targetId, + ), + ) + + require.Error(t, output.Err, "Expected error when connecting to target with same listen port") + require.Contains(t, string(output.Stderr), "address already in use") + require.Equal(t, 2, output.ExitCode) +}