feat(installer): Prompt for the Device Agent port in interactive mode

installer/TESTING.md

@@ -131,16 +131,17 @@ Prereq: No installation present at the default directory
 
 Steps
 1) Run with no flags at all
-2) At `Where should the FlowFuse Device Agent be installed?`, press Enter
-3) Repeat, answering `some/relative/dir`, then `<dir>`
-4) Run: `--otc <OTC>`
-5) Run: `--dir <dir>` (no `--otc`)
+2) At `Which TCP port should the FlowFuse Device Agent listen on?`, press Enter
+3) At `Where should the FlowFuse Device Agent be installed?`, press Enter
+4) Repeat steps 1-2, answering `some/relative/dir`, then `<dir>`
+5) Run: `--otc <OTC>`
+6) Run: `--dir <dir>` (no `--otc`)
 
 Expect
-- The question appears after the welcome banner and after the permission check (sudo prompt first)
-- Step 2 installs into the default directory
-- Step 3 rejects the relative path with `Path must be absolute, for example ...`, re-asks, then installs into `<dir>`
-- Steps 4 and 5 show no directory question
+- The questions appear after the welcome banner and after the permission check (sudo prompt first), port first
+- Step 3 installs into the default directory
+- Step 4 rejects the relative path with `Path must be absolute, for example ...`, re-asks, then installs into `<dir>`
+- Steps 5 and 6 show no directory question
 
 ## N. Installation directory for uninstall and update
 Prereq for 1): An installation in the default directory
@@ -211,6 +212,31 @@ Expect
 
 ---
 
+## Q. Port prompt
+Prereq: No installation present; another process listening on `<busyPort>`
+
+A busy port must be rejected whichever address the other process listens on. Cover
+at least the wildcard and one interface-specific case, since they behave differently
+per platform:
+- `python3 -m http.server <busyPort>` (IPv4 wildcard, `0.0.0.0`)
+- `python3 -m http.server --bind :: <busyPort>` (IPv6 wildcard, dual-stack)
+- `python3 -m http.server --bind 127.0.0.1 <busyPort>` (loopback only)
+- `python3 -m http.server --bind <thisHostIP> <busyPort>` (one interface only)
+
+Steps
+1) Run with no flags at all; at the port question press Enter, then accept the directory
+2) Repeat, answering `80`, then `abc`, then `<busyPort>`, then `<port>`
+3) Run: `--port <port>` (no `--otc`)
+4) Run: `--otc <OTC>`
+
+Expect
+- Step 1 uses port 1880 and suggests the default directory
+- Step 2 rejects `80` and `abc` with `Port must be a number between 1025 and 65535.`, rejects `<busyPort>` with `port <busyPort> is in use...`, re-asks each time, then uses `<port>`
+- `<busyPort>` is rejected for every listener address above, and the installation never reaches the agent's own `Port <busyPort> is not available.` error
+- A non-default `<port>` makes the next question suggest `/opt/flowfuse-device-<port>` (`c:\opt\flowfuse-device-<port>` on Windows); the suggestion can still be overridden by typing a path
+- Steps 3 and 4 show no port question, only `The FlowFuse Device Agent will use port <port>.`
+- Step 4 installs into the plain default directory, with no port suffix
+
 ## OS-specific verification
 
 Linux — systemd

installer/go/cmd/install.go

@@ -17,18 +17,19 @@ import (
 //
 // The function performs the following steps:
 // 1. Checks if the process has sufficient permissions
-// 2. Asks for the installation directory when neither customWorkDir nor otc is set
-// 3. Runs the remaining pre-install validation
-// 4. Creates a working directory for the installation
-// 5. Ensures Node.js is installed at the required version
-// 6. Installs the Device Agent npm package
-// 7. Handles different installation modes based on OTC availability:
+// 2. Asks for the TCP port when neither the --port flag nor an OTC is given
+// 3. Asks for the installation directory when neither customWorkDir nor otc is set
+// 4. Runs the remaining pre-install validation
+// 5. Creates a working directory for the installation
+// 6. Ensures Node.js is installed at the required version
+// 7. Installs the Device Agent npm package
+// 8. Handles different installation modes based on OTC availability:
 //   - Traditional: With OTC, configures and starts service
 //   - Manual config: Without OTC, prompts for config and saves device.yml
 //   - Install-only: Without OTC and no config, installs but doesn't start service
 //
-// 8. Sets up the Device Agent to run as a system service
-// 9. Saves the installation configuration
+// 9. Sets up the Device Agent to run as a system service
+// 10. Saves the installation configuration
 //
 // Parameters:
 //   - nodeVersion: The version of Node.js to install or use
@@ -38,47 +39,79 @@ import (
 //   - customWorkDir: Optional custom working directory path. If empty and no OTC is
 //     given, the user is asked for it; otherwise the default path is used.
 //   - update: Whether this is an update operation
-//   - port: The TCP port number the device agent will use
+//   - requestedPort: The TCP port the device agent should use, or nil when it was
+//     not given on the command line. If nil and no OTC is given, the user is
+//     asked for it; otherwise utils.DefaultPort is used.
+//   - caCertPath: Optional path to a CA certificate bundle the agent should trust
 //
 // Returns:
 //   - error: An error object if any step of the installation fails, nil otherwise
 //
 // The function logs detailed information about each step of the process.
-func Install(nodeVersion, agentVersion, url, otc, customWorkDir string, update bool, port int, caCertPath string) error {
+func Install(nodeVersion, agentVersion, url, otc, customWorkDir string, update bool, requestedPort *int, caCertPath string) error {
 	logger.LogFunctionEntry("Install", map[string]interface{}{
 		"nodeVersion":   nodeVersion,
 		"agentVersion":  agentVersion,
 		"url":           url,
 		"otc":           otc,
 		"customWorkDir": customWorkDir,
-		"port":          port,
 	})
 
-	serviceName := fmt.Sprintf("flowfuse-device-agent-%d", port)
-
 	logger.Debug("Running permission check...")
 	if err := utils.CheckPermissions(); err != nil {
 		logger.LogFunctionExit("CheckPermissions", nil, err)
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 
+	// Settle the port before anything else is asked or created: the service name,
+	// the directory suggested for the installation and the pre-install checks all
+	// depend on it.
+	port := utils.DefaultPort
+	if requestedPort != nil {
+		port = *requestedPort
+	}
+
+	var err error
+	if requestedPort == nil && otc == "" {
+		port, err = utils.PromptPort(port)
+	} else {
+		err = utils.CheckUnusedPort(port)
+	}
+	if err != nil {
+		logger.Error("Failed to determine the port: %v", err)
+		logger.LogFunctionExit("Install", nil, err)
+		return fmt.Errorf("failed to determine the port: %w", err)
+	}
+
+	logger.Info("The FlowFuse Device Agent will use port %d.", port)
+
+	serviceName := fmt.Sprintf("flowfuse-device-agent-%d", port)
+
 	// Ask where to install when the directory was not given on the command line.
 	// With an OTC the installation is scripted, so it stays non-interactive and
 	// uses the default directory.
 	if customWorkDir == "" && otc == "" {
-		promptedDir, err := utils.PromptInstallDirectory()
+		promptedDir, err := utils.PromptInstallDirectory(port)
 		if err != nil {
 			logger.Error("Failed to determine the installation directory: %v", err)
 			logger.LogFunctionExit("Install", nil, err)
 			return fmt.Errorf("failed to determine the installation directory: %w", err)
 		}
 		customWorkDir = promptedDir
-		logger.Debug("Using installation directory: %s", customWorkDir)
 	}
 
+	customWorkDir, err = utils.GetWorkingDirectory(customWorkDir)
+	if err != nil {
+		logger.Error("Failed to determine the installation directory: %v", err)
+		logger.LogFunctionExit("Install", nil, err)
+		return fmt.Errorf("failed to determine the installation directory: %w", err)
+	}
+
+	logger.Info("The FlowFuse Device Agent will be installed in %s.", customWorkDir)
+
 	// Run pre-install validation
 	logger.Debug("Running pre-check...")
-	if err := validate.PreInstall(customWorkDir, port); err != nil {
+	if err := validate.PreInstall(customWorkDir); err != nil {
 		logger.LogFunctionExit("Install", nil, err)
 		return fmt.Errorf("pre-check failed: %w", err)
 	}

installer/go/main.go

@@ -29,7 +29,7 @@ var (
 	updateNode          bool
 	updateAgent         bool
 	debugMode           bool
-	port                int
+	port                utils.PortFlag
 )
 
 func init() {
@@ -39,7 +39,7 @@ func init() {
 	pflag.StringVarP(&flowfuseURL, "url", "u", "", "FlowFuse URL")
 	pflag.StringVarP(&flowfuseOneTimeCode, "otc", "o", "", "FlowFuse one time code for authentication (optional for interactive installation)")
 	pflag.StringVarP(&installDir, "dir", "d", "", "Custom installation directory (default: /opt/flowfuse-device on Unix, c:\\opt\\flowfuse-device on Windows)")
-	pflag.IntVarP(&port, "port", "p", 1880, "TCP port for the device agent (1-65535)")
+	pflag.VarP(&port, "port", "p", fmt.Sprintf("TCP port for the device agent (%d-%d) (default %d)", utils.MinPort, utils.MaxPort, utils.DefaultPort))
 	pflag.StringVar(&caCertPath, "ca-cert", "", "Path to a CA certificate bundle (PEM) the Device Agent should trust")
 	pflag.BoolVarP(&showVersion, "version", "v", false, "Display installer version")
 	pflag.BoolVarP(&help, "help", "h", false, "Display help information")
@@ -82,15 +82,9 @@ func init() {
 
 func main() {
 	utils.ServiceUsername = serviceUsername
-	utils.DefaultPort = port
 	var err error
 	var exitCode int
 
-	if port < 1025 || port > 65535 {
-		fmt.Println("Invalid --port value. Please specify a port in range 1025-65535.")
-		os.Exit(2)
-	}
-
 	// Initialize logger
 	if err := logger.Initialize(debugMode); err != nil {
 		fmt.Printf("Warning: Failed to initialize logger: %s\n", err)
@@ -112,8 +106,8 @@ func main() {
 	}()
 
 	// Log startup information
-	logger.Debug("Command line arguments: node=%s, agent=%s, user=%s, url=%s, debug=%v, customInstallDir=%s, port=%d, caCert=%s",
-		nodeVersion, agentVersion, serviceUsername, flowfuseURL, debugMode, installDir, port, caCertPath)
+	logger.Debug("Command line arguments: node=%s, agent=%s, user=%s, url=%s, debug=%v, customInstallDir=%s, port=%s, caCert=%s",
+		nodeVersion, agentVersion, serviceUsername, flowfuseURL, debugMode, installDir, port.String(), caCertPath)
 	operatingSystem, architecture := utils.GetOSDetails()
 	logger.Debug("Detected system: %s, detected architecture: %s", operatingSystem, architecture)
 
@@ -132,7 +126,7 @@ func main() {
 		logger.Info("")
 		logger.Info("Let's get your connected to FlowFuse.")
 		logger.Info("")
-		err = cmd.Install(nodeVersion, agentVersion, flowfuseURL, flowfuseOneTimeCode, installDir, false, port, caCertPath)
+		err = cmd.Install(nodeVersion, agentVersion, flowfuseURL, flowfuseOneTimeCode, installDir, false, port.Value, caCertPath)
 	}
 
 	if err != nil {

installer/go/pkg/utils/port.go

@@ -0,0 +1,150 @@
+package utils
+
+import (
+	"fmt"
+	"net"
+	"strconv"
+
+	"github.com/flowfuse/device-agent-installer/pkg/logger"
+)
+
+// DefaultPort is the TCP port the Device Agent uses unless told otherwise.
+const DefaultPort = 1880
+
+// MinPort and MaxPort bound the TCP ports the Device Agent may be given. Ports
+// below 1025 are privileged and the agent does not run as root.
+const (
+	MinPort = 1025
+	MaxPort = 65535
+)
+
+// PortFlag is the value behind the installer's --port flag. Value stays nil
+// until the flag is actually given, so an explicit "--port 1880" remains
+// distinguishable from an omitted flag: the former is the user's answer, the
+// latter means they have yet to be asked.
+//
+// It satisfies pflag.Value, which is a structural interface, so this package
+// needs no dependency on pflag.
+type PortFlag struct{ Value *int }
+
+// Type reports the value kind shown in the generated help output.
+func (p *PortFlag) Type() string { return "int" }
+
+// String renders the current value, and the empty string while unset. Reading as
+// a zero value when unset is what makes pflag omit its own "(default ...)" line,
+// so the real default belongs in the flag description instead.
+func (p *PortFlag) String() string {
+	if p.Value == nil {
+		return ""
+	}
+	return strconv.Itoa(*p.Value)
+}
+
+// Set parses and range-checks the port. Reporting the problem here lets pflag
+// print it and exit, so the flag needs no separate validation by the caller.
+func (p *PortFlag) Set(s string) error {
+	value, err := strconv.Atoi(s)
+	if err != nil {
+		return fmt.Errorf("must be a number between %d and %d", MinPort, MaxPort)
+	}
+	if value < MinPort || value > MaxPort {
+		return fmt.Errorf("must be between %d and %d", MinPort, MaxPort)
+	}
+	p.Value = &value
+	return nil
+}
+
+// portBindHosts returns every address a listener on this host could be occupying:
+// the IPv4 and IPv6 wildcards, plus each interface address (which includes the
+// loopback addresses).
+//
+// Testing a single address is not enough. On macOS and the BSDs a bind to
+// 127.0.0.1 succeeds while another process holds 0.0.0.0 on the same port, and
+// neither wildcard conflicts with a listener bound to one specific interface
+// address, so a port already taken would be offered as free.
+//
+// Link-local addresses are skipped: they need a zone identifier that
+// net.InterfaceAddrs does not report, so binding them fails for reasons that
+// have nothing to do with the port being taken.
+//
+// Returns:
+//   - []string: The host addresses to test, without a port
+func portBindHosts() []string {
+	hosts := []string{"0.0.0.0", "::"}
+
+	interfaceAddrs, err := net.InterfaceAddrs()
+	if err != nil {
+		logger.Debug("Could not list interface addresses, checking wildcards only: %v", err)
+		return hosts
+	}
+
+	for _, addr := range interfaceAddrs {
+		network, ok := addr.(*net.IPNet)
+		if !ok || network.IP.IsLinkLocalUnicast() || network.IP.IsLinkLocalMulticast() {
+			continue
+		}
+		hosts = append(hosts, network.IP.String())
+	}
+
+	return hosts
+}
+
+// CheckUnusedPort validates if specified TCP port is not in use by any process.
+// The port has to be free on every address a listener could occupy, so that the
+// Device Agent is never handed a port it cannot then bind.
+//
+// Parameters
+//   - port: The TCP port to validate for availability.
+//
+// Returns:
+//   - error: nil if the port is available, otherwise an error indicating the port is in use
+func CheckUnusedPort(port int) error {
+	logger.LogFunctionEntry("CheckUnusedPort", map[string]interface{}{
+		"port": port,
+	})
+
+	for _, host := range portBindHosts() {
+		address := net.JoinHostPort(host, strconv.Itoa(port))
+		listener, err := net.Listen("tcp", address)
+		if err != nil {
+			logger.Debug("Port %d is unavailable on %s: %v", port, address, err)
+			logger.LogFunctionExit("CheckUnusedPort", "error", err)
+			return fmt.Errorf("port %d is in use. Please select another port and try again", port)
+		}
+		listener.Close()
+	}
+
+	logger.LogFunctionExit("CheckUnusedPort", "success", nil)
+	return nil
+}
+
+// PromptPort asks the user which TCP port the Device Agent should listen on,
+// re-prompting until a free port in the allowed range is given.
+//
+// Parameters:
+//   - defaultPort: The port returned when the user provides no input
+//
+// Returns:
+//   - int: The selected port
+//   - error: An error if the input could not be read
+func PromptPort(defaultPort int) (int, error) {
+	for {
+		answer, err := PromptText("Which TCP port should the FlowFuse Device Agent listen on?", strconv.Itoa(defaultPort))
+		if err != nil {
+			return 0, err
+		}
+
+		port, err := strconv.Atoi(answer)
+		if err != nil || port < MinPort || port > MaxPort {
+			fmt.Printf("Port must be a number between %d and %d.\n", MinPort, MaxPort)
+			continue
+		}
+
+		if err := CheckUnusedPort(port); err != nil {
+			fmt.Printf("%v\n", err)
+			continue
+		}
+
+		return port, nil
+	}
+}