diff --git a/installer/TESTING.md b/installer/TESTING.md index 4e397a28..9631b938 100644 --- a/installer/TESTING.md +++ b/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 `` -4) Run: `--otc ` -5) Run: `--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 `` +5) Run: `--otc ` +6) Run: `--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 `` -- 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 `` +- 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 `` + +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 ` (IPv4 wildcard, `0.0.0.0`) +- `python3 -m http.server --bind :: ` (IPv6 wildcard, dual-stack) +- `python3 -m http.server --bind 127.0.0.1 ` (loopback only) +- `python3 -m http.server --bind ` (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 ``, then `` +3) Run: `--port ` (no `--otc`) +4) Run: `--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 `` with `port is in use...`, re-asks each time, then uses `` +- `` is rejected for every listener address above, and the installation never reaches the agent's own `Port is not available.` error +- A non-default `` makes the next question suggest `/opt/flowfuse-device-` (`c:\opt\flowfuse-device-` 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 .` +- Step 4 installs into the plain default directory, with no port suffix + ## OS-specific verification Linux — systemd diff --git a/installer/go/cmd/install.go b/installer/go/cmd/install.go index 07c14b5a..f16c46d7 100644 --- a/installer/go/cmd/install.go +++ b/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) } diff --git a/installer/go/main.go b/installer/go/main.go index a6afdefc..e99b6ea7 100644 --- a/installer/go/main.go +++ b/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 { diff --git a/installer/go/pkg/utils/port.go b/installer/go/pkg/utils/port.go new file mode 100644 index 00000000..ccac6f87 --- /dev/null +++ b/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 + } +} diff --git a/installer/go/pkg/utils/utils.go b/installer/go/pkg/utils/utils.go index 2c0374a2..140a9fdf 100644 --- a/installer/go/pkg/utils/utils.go +++ b/installer/go/pkg/utils/utils.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strconv" "strings" "github.com/flowfuse/device-agent-installer/pkg/logger" @@ -23,9 +24,6 @@ import ( // Global variable to store the service username var ServiceUsername = "flowfuse" -// DefaultPort is the default TCP port for the device agent when not specified elsewhere -// This can be overridden at runtime by the CLI flag in main.go -var DefaultPort = 1880 // DeviceConfig represents the expected structure of the device.yml configuration file type DeviceConfig struct { @@ -97,7 +95,8 @@ func PromptYesNo(question string, defaultResponse bool) bool { // // Returns: // - string: The trimmed user input, or defaultValue when the input is empty -func PromptText(question, defaultValue string) string { +// - error: An error if the input could not be read +func PromptText(question, defaultValue string) (string, error) { style.FlushInput() reader := bufio.NewReader(os.Stdin) @@ -112,8 +111,7 @@ func PromptText(question, defaultValue string) string { response, err := reader.ReadString('\n') if err != nil { - logger.Error("Failed to read user input: %v", err) - return defaultValue + return "", fmt.Errorf("failed to read user input: %w", err) } // Collapse the prompt now that we have the answer. No-op on terminals that @@ -122,9 +120,9 @@ func PromptText(question, defaultValue string) string { response = strings.TrimSpace(response) if response == "" { - return defaultValue + return defaultValue, nil } - return response + return response, nil } // PromptMultilineInput prompts the user for multiline input until they enter an empty line @@ -342,6 +340,28 @@ func getDefaultWorkingDirectory() (string, error) { } } +// defaultInstallDirForPort returns the default working directory to suggest for +// a new installation on the given port. A port other than the default one gets +// the port appended, so Device Agents listening on different ports are not proposed the +// same directory. +// +// Parameters: +// - port: The TCP port the installation will use +// +// Returns: +// - string: The default path to suggest for this port +// - error: nil if successful, otherwise an error describing what went wrong +func defaultInstallDirForPort(port int) (string, error) { + dir, err := getDefaultWorkingDirectory() + if err != nil { + return "", err + } + if port != DefaultPort { + return dir + "-" + strconv.Itoa(port), nil + } + return dir, nil +} + // CreateWorkingDirectory creates and returns the working directory path for the FlowFuse device agent. // If customPath is provided and not empty, it uses that path; otherwise, it uses the default OS-specific path. // On Unix systems, the default is "/opt/flowfuse-device" with 0755 permissions. @@ -422,9 +442,8 @@ func cleanAbsolutePath(path string) (string, error) { // promptDirectory asks the user for a directory, offering defaultDir when the // user just presses Enter, and re-prompting until an absolute path is given. // -// defaultDir must be absolute, which is also what stops the loop from running -// forever on a closed stdin: PromptText falls back to the default value when -// input cannot be read, and that value is then accepted. +// defaultDir must be absolute, so that simply pressing Enter always yields a +// usable answer. // // Parameters: // - question: The prompt to display to the user @@ -440,7 +459,12 @@ func promptDirectory(question, defaultDir string) (string, error) { } for { - dir, err := cleanAbsolutePath(PromptText(question, cleanDefault)) + answer, err := PromptText(question, cleanDefault) + if err != nil { + return "", err + } + + dir, err := cleanAbsolutePath(answer) if err == nil { return dir, nil } @@ -449,13 +473,17 @@ func promptDirectory(question, defaultDir string) (string, error) { } // PromptInstallDirectory asks the user where a new Device Agent installation -// should be placed. +// should be placed. The directory offered by default carries the port, unless +// the installation uses the default one. +// +// Parameters: +// - port: The TCP port the installation will use // // Returns: // - string: The cleaned, absolute directory path // - error: An error if the default directory cannot be determined -func PromptInstallDirectory() (string, error) { - defaultDir, err := GetWorkingDirectory("") +func PromptInstallDirectory(port int) (string, error) { + defaultDir, err := defaultInstallDirForPort(port) if err != nil { return "", fmt.Errorf("failed to determine the default installation directory: %w", err) } @@ -608,7 +636,13 @@ func WaitForAgentProcesses(workDir string) bool { logger.Info("") logger.Info("Stop listed processes before continuing with the uninstall.") - answer := PromptText("Press Enter to check again, or type 's' to skip and keep the service account", "") + answer, err := PromptText("Press Enter to check again, or type 's' to skip and keep the service account", "") + if err != nil { + // Input that cannot be read is treated as a skip: the processes are + // still running, so the service account they use has to stay. + logger.Error("Could not read the response: %v", err) + return false + } if strings.EqualFold(answer, "s") { logger.Debug("User skipped waiting for Device Agent processes") return false diff --git a/installer/go/pkg/validate/validate.go b/installer/go/pkg/validate/validate.go index 7d74552d..730e2355 100644 --- a/installer/go/pkg/validate/validate.go +++ b/installer/go/pkg/validate/validate.go @@ -2,7 +2,6 @@ package validate import ( "fmt" - "net" "os" "path/filepath" "runtime" @@ -18,34 +17,27 @@ const minFreeDiskBytes uint64 = 500 * 1024 * 1024 // 500 MB // PreInstall performs validation steps before installation: // 1. Verifies there is enough free disk space for the installation -// 2. Verifies the requested TCP port is not already in use -// 3. Handles an existing installation found in the working directory -// 4. Verifies libstdc++ is present (Linux only) +// 2. Handles an existing installation found in the working directory +// 3. Verifies libstdc++ is present (Linux only) // // The caller is responsible for checking permissions (utils.CheckPermissions) -// before calling this function, as the working directory it validates may -// itself have to be resolved interactively first. +// and for settling the TCP port (utils.PromptPort or utils.CheckUnusedPort) +// before calling this function: the working directory it validates may itself +// have to be resolved interactively, and that needs the port first. // // Parameters: // - customWorkDir: Optional custom working directory path. If empty, uses default path. -// - port: The TCP port to validate for availability. // // Returns: // - nil if all checks pass // - error if any check fails -func PreInstall(customWorkDir string, port int) error { +func PreInstall(customWorkDir string) error { if err := checkFreeDiskSpace(customWorkDir, minFreeDiskBytes); err != nil { logger.Error("Disk space check failed: %v", err) logger.LogFunctionExit("PreInstall", nil, err) return fmt.Errorf("disk space check failed: %w", err) } - if err := checkUnusedPort(port); err != nil { - logger.Error("Port check failed: %v", err) - logger.LogFunctionExit("PreInstall", nil, err) - return fmt.Errorf("port check failed: %w", err) - } - if err := checkConfigFileExists(customWorkDir); err != nil { logger.LogFunctionExit("PreInstall", nil, err) return fmt.Errorf("configuration file pre-check failed: %w", err) @@ -217,28 +209,6 @@ func ValidateInstallationDirectory(workDir string) error { return nil } -// checkUnusedPort validates if specified TCP port is not in use by any process. -// -// 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, - }) - listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err != nil { - logger.LogFunctionExit("checkUnusedPort", "error", err) - logger.Debug("Port %d is in use: %v", port, err) - return fmt.Errorf("port %d is in use. Please select another port and try again", port) - } - defer listener.Close() - logger.LogFunctionExit("checkUnusedPort", "success", nil) - return nil -} - // checkFreeDiskSpace validates free disk space for the install directory and OS temp directory. // It requires at least requiredBytes free in each distinct location. // /