feat(board): remove random port selection (#698)

Author: lucarin91

pkg/board/remote/adb/adb.go

@@ -18,7 +18,7 @@ import (
 	"github.com/arduino/go-paths-helper"
 
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote"
-	"github.com/bcmi-labs/orchestrator/pkg/board/remote/common"
+	"github.com/bcmi-labs/orchestrator/pkg/x/ports"
 )
 
 const username = "arduino"
@@ -56,27 +56,27 @@ func FromHost(host string, adbPath string) (*ADBConnection, error) {
 	return FromSerial(host, adbPath)
 }
 
-func (a *ADBConnection) Forward(ctx context.Context, remotePort int) (int, error) {
-	hostAvailablePort, err := common.GetAvailablePort()
-	if err != nil {
-		return 0, fmt.Errorf("failed to find an available port: %w", err)
+func (a *ADBConnection) Forward(ctx context.Context, localPort int, remotePort int) error {
+	if !ports.IsAvailable(localPort) {
+		return remote.ErrPortAvailable
 	}
 
-	local := fmt.Sprintf("tcp:%d", hostAvailablePort)
+	local := fmt.Sprintf("tcp:%d", localPort)
 	remote := fmt.Sprintf("tcp:%d", remotePort)
 	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "forward", local, remote)
 	if err != nil {
-		return hostAvailablePort, err
+		return err
 	}
 	if err := cmd.RunWithinContext(ctx); err != nil {
-		return hostAvailablePort, fmt.Errorf(
+		return fmt.Errorf(
 			"failed to forward ADB port %s to %s: %w",
 			local,
 			remote,
 			err,
 		)
 	}
-	return hostAvailablePort, nil
+
+	return nil
 }
 
 func (a *ADBConnection) ForwardKillAll(ctx context.Context) error {

pkg/board/remote/local/local.go

@@ -18,9 +18,9 @@ type LocalConnection struct{}
 // Ensures LocalConnection implements the RemoteConn interface at compile time.
 var _ remote.RemoteConn = (*LocalConnection)(nil)
 
-func (a *LocalConnection) Forward(ctx context.Context, remotePort int) (int, error) {
+func (a *LocalConnection) Forward(ctx context.Context, localPort int, remotePort int) error {
 	// Locally we don't need to forward ports.
-	return remotePort, nil
+	return nil
 }
 
 func (a *LocalConnection) ForwardKillAll(ctx context.Context) error {

pkg/board/remote/remote.go

@@ -2,9 +2,12 @@ package remote
 
 import (
 	"context"
+	"fmt"
 	"io"
 )
 
+var ErrPortAvailable = fmt.Errorf("port is not available")
+
 type FileInfo struct {
 	Name  string
 	IsDir bool
@@ -30,7 +33,7 @@ type RemoteShell interface {
 }
 
 type Forwarder interface {
-	Forward(ctx context.Context, remotePort int) (int, error)
+	Forward(ctx context.Context, localPort int, remotePort int) error
 	ForwardKillAll(ctx context.Context) error
 }
 

pkg/board/remote/remote_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote/adb"
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote/local"
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote/ssh"
+	"github.com/bcmi-labs/orchestrator/pkg/x/ports"
 	"github.com/bcmi-labs/orchestrator/pkg/x/testtools"
 )
 
@@ -182,7 +183,10 @@ func TestSSHForwarder(t *testing.T) {
 		ctx, cancel := context.WithCancel(t.Context())
 		defer cancel()
 
-		forwardPort, err := conn.Forward(ctx, 5555)
+		forwardPort, err := ports.GetAvailable()
+		require.NoError(t, err)
+
+		err = conn.Forward(ctx, forwardPort, 5555)
 		if err != nil {
 			t.Errorf("Forward failed: %v", err)
 		}
@@ -213,7 +217,10 @@ func TestSSHKillForwarder(t *testing.T) {
 		ctx, cancel := context.WithCancel(t.Context())
 		defer cancel()
 
-		forwardPort, err := conn.Forward(ctx, 5555)
+		forwardPort, err := ports.GetAvailable()
+		require.NoError(t, err)
+
+		err = conn.Forward(ctx, forwardPort, 5555)
 		if err != nil {
 			t.Errorf("Forward failed: %v", err)
 		}

pkg/board/remote/ssh/ssh.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
-	"log"
 	"log/slog"
 	"net"
 	"path"
@@ -17,7 +16,7 @@ import (
 	"golang.org/x/crypto/ssh"
 
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote"
-	"github.com/bcmi-labs/orchestrator/pkg/board/remote/common"
+	"github.com/bcmi-labs/orchestrator/pkg/x/ports"
 )
 
 var ErrAuthFailed = errors.New("ssh authentication failed")
@@ -57,18 +56,14 @@ func FromHost(user, password, address string) (*SSHConnection, error) {
 	}, nil
 }
 
-func (a *SSHConnection) Forward(ctx context.Context, remotePort int) (int, error) {
-
-	// Get a random available port for remote connection
-	randomPort, err := common.GetAvailablePort()
-	if err != nil {
-		log.Printf("failed to get available port: %v", err)
-		return 0, err
+func (a *SSHConnection) Forward(ctx context.Context, localPort int, remotePort int) error {
+	if !ports.IsAvailable(localPort) {
+		return remote.ErrPortAvailable
 	}
-	// Listen locally on remote port
-	listener, err := net.Listen("tcp", fmt.Sprintf("%s:%d", "localhost", randomPort))
+
+	listener, err := net.Listen("tcp", fmt.Sprintf("%s:%d", "localhost", localPort))
 	if err != nil {
-		return 0, err
+		return err
 	}
 
 	a.mu.Lock()
@@ -112,8 +107,7 @@ func (a *SSHConnection) Forward(ctx context.Context, remotePort int) (int, error
 		}
 	}()
 
-	return randomPort, nil
-
+	return nil
 }
 
 func copyAndLog(dst io.Writer, src io.Reader) {

pkg/board/remote/common/common.go renamed to pkg/x/ports/ports.go

@@ -1,28 +1,14 @@
-package common
+package ports
 
 import (
 	"fmt"
 	"math/rand/v2"
 	"net"
 )
 
-func isPortAvailable(port int) bool {
-	listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port))
-	if err != nil {
-		return false
-	}
-	listener.Close()
-	return true
-}
-
-func getRandomPort() int {
-	port := 1000 + rand.IntN(9000) // nolint:gosec
-	return port
-}
-
 const forwardPortAttempts = 10
 
-func GetAvailablePort() (int, error) {
+func GetAvailable() (int, error) {
 	tried := make(map[int]any, forwardPortAttempts)
 	for len(tried) < forwardPortAttempts {
 		port := getRandomPort()
@@ -31,9 +17,23 @@ func GetAvailablePort() (int, error) {
 		}
 		tried[port] = struct{}{}
 
-		if isPortAvailable(port) {
+		if IsAvailable(port) {
 			return port, nil
 		}
 	}
 	return 0, fmt.Errorf("no available port found in range 1000-9999 after %d attempts", forwardPortAttempts)
 }
+
+func IsAvailable(port int) bool {
+	listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port))
+	if err != nil {
+		return false
+	}
+	listener.Close()
+	return true
+}
+
+func getRandomPort() int {
+	port := 1000 + rand.IntN(9000) // nolint:gosec
+	return port
+}