From e482c0a46be059e976cb530db1c0c4540b2f1d42 Mon Sep 17 00:00:00 2001 From: John Lago <750845+Lagoja@users.noreply.github.com> Date: Tue, 24 Sep 2024 15:50:09 -0700 Subject: [PATCH 1/3] Let users set the port for process-compose --- internal/boxcli/services.go | 8 +++++++ internal/devbox/devopt/devboxopts.go | 1 + internal/devbox/services.go | 5 +++++ internal/services/manager.go | 10 ++++++--- internal/services/ports.go | 31 ++++++++++++++++++++++++++++ 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/internal/boxcli/services.go b/internal/boxcli/services.go index e1eb493afe1..c5c0d5ca1dc 100644 --- a/internal/boxcli/services.go +++ b/internal/boxcli/services.go @@ -20,6 +20,7 @@ type serviceUpFlags struct { background bool processComposeFile string processComposeFlags []string + pcport int } type serviceStopFlags struct { @@ -38,6 +39,8 @@ func (flags *serviceUpFlags) register(cmd *cobra.Command) { &flags.background, "background", "b", false, "run service in background") cmd.Flags().StringArrayVar( &flags.processComposeFlags, "pcflags", []string{}, "pass flags directly to process compose") + cmd.Flags().IntVarP( + &flags.pcport, "pcport", "p", 0, "specify the port for process-compose to use. You can also set the pcport by exporting PC_PORT_NUM") } func (flags *serviceStopFlags) register(cmd *cobra.Command) { @@ -245,6 +248,10 @@ func startProcessManager( return err } + if flags.pcport < 0 { + return errors.Errorf("invalid pcport %d: ports cannot be less than 0", flags.pcport) + } + box, err := devbox.Open(&devopt.Opts{ Dir: servicesFlags.config.path, Env: env, @@ -263,6 +270,7 @@ func startProcessManager( devopt.ProcessComposeOpts{ Background: flags.background, ExtraFlags: flags.processComposeFlags, + PCPort: flags.pcport, }, ) } diff --git a/internal/devbox/devopt/devboxopts.go b/internal/devbox/devopt/devboxopts.go index 8e4ec45c5b4..4f51b50d943 100644 --- a/internal/devbox/devopt/devboxopts.go +++ b/internal/devbox/devopt/devboxopts.go @@ -20,6 +20,7 @@ type Opts struct { type ProcessComposeOpts struct { ExtraFlags []string Background bool + PCPort int } type GenerateOpts struct { diff --git a/internal/devbox/services.go b/internal/devbox/services.go index ead19ded6e9..d45f85ac213 100644 --- a/internal/devbox/services.go +++ b/internal/devbox/services.go @@ -3,6 +3,7 @@ package devbox import ( "context" "fmt" + "strconv" "text/tabwriter" "go.jetpack.io/devbox/internal/boxcli/usererr" @@ -217,6 +218,9 @@ func (d *Devbox) StartProcessManager( for _, flag := range processComposeOpts.ExtraFlags { args = append(args, "--pcflags", flag) } + if processComposeOpts.PCPort != 0 { + args = append(args, "--pcport", strconv.Itoa(processComposeOpts.PCPort)) + } return d.runDevboxServicesScript(ctx, args) } @@ -257,6 +261,7 @@ func (d *Devbox) StartProcessManager( BinPath: processComposeBinPath, Background: processComposeOpts.Background, ExtraFlags: processComposeOpts.ExtraFlags, + PCPort: processComposeOpts.PCPort, }, ) } diff --git a/internal/services/manager.go b/internal/services/manager.go index df38f4c4bf6..6eb44a19d2e 100644 --- a/internal/services/manager.go +++ b/internal/services/manager.go @@ -44,6 +44,7 @@ type ProcessComposeOpts struct { BinPath string ExtraFlags []string Background bool + PCPort int } func newGlobalProcessComposeConfig() *globalProcessComposeConfig { @@ -128,10 +129,13 @@ func StartProcessManager( config := readGlobalProcessComposeJSON(configFile) config.File = configFile - // Get the port to use for this project - port, err := getAvailablePort() + port, err := selectPort(processComposeConfig.PCPort) if err != nil { - return err + return fmt.Errorf("failed to select port: %v", err) + } + + if !isPortAvailable(port) { + return fmt.Errorf("port %d is already in use", port) } // Start building the process-compose command diff --git a/internal/services/ports.go b/internal/services/ports.go index 1593576ee49..6af4617dad6 100644 --- a/internal/services/ports.go +++ b/internal/services/ports.go @@ -1,7 +1,10 @@ package services import ( + "fmt" "net" + "os" + "strconv" "github.com/pkg/errors" ) @@ -60,6 +63,34 @@ func getAvailablePort() (int, error) { return 0, errors.New("no available port") } +func selectPort(configPort int) (int, error) { + if configPort != 0 { + return configPort, nil + } + + if portStr, exists := os.LookupEnv("PC_PORT_NUM"); exists { + port, err := strconv.Atoi(portStr) + if err != nil { + return 0, fmt.Errorf("invalid PC_PORT_NUM environment variable: %v", err) + } + if port <= 0 { + return 0, fmt.Errorf("invalid PC_PORT_NUM environment variable: ports cannot be less than 0") + } + return port, nil + } + + return getAvailablePort() +} + func isAllowed(port int) bool { return port > 1024 && disallowedPorts[port] == "" } + +func isPortAvailable(port int) bool { + ln, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) + if err != nil { + return false + } + ln.Close() + return true +} From 34625e9825f20bf011cf983746ffddeffd20c007 Mon Sep 17 00:00:00 2001 From: John Lago <750845+Lagoja@users.noreply.github.com> Date: Wed, 25 Sep 2024 14:13:44 -0700 Subject: [PATCH 2/3] Fix feedback --- internal/boxcli/services.go | 8 ++++---- internal/devbox/devopt/devboxopts.go | 6 +++--- internal/devbox/services.go | 12 ++++++------ internal/services/manager.go | 14 +++++--------- internal/services/ports.go | 23 +++++++++++------------ 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/internal/boxcli/services.go b/internal/boxcli/services.go index c5c0d5ca1dc..372a2b669d8 100644 --- a/internal/boxcli/services.go +++ b/internal/boxcli/services.go @@ -40,7 +40,7 @@ func (flags *serviceUpFlags) register(cmd *cobra.Command) { cmd.Flags().StringArrayVar( &flags.processComposeFlags, "pcflags", []string{}, "pass flags directly to process compose") cmd.Flags().IntVarP( - &flags.pcport, "pcport", "p", 0, "specify the port for process-compose to use. You can also set the pcport by exporting PC_PORT_NUM") + &flags.pcport, "pcport", "p", 0, "specify the port for process-compose to use. You can also set the pcport by exporting DEVBOX_PC_PORT_NUM") } func (flags *serviceStopFlags) register(cmd *cobra.Command) { @@ -268,9 +268,9 @@ func startProcessManager( servicesFlags.runInCurrentShell, args, devopt.ProcessComposeOpts{ - Background: flags.background, - ExtraFlags: flags.processComposeFlags, - PCPort: flags.pcport, + Background: flags.background, + ExtraFlags: flags.processComposeFlags, + ProcessComposePort: flags.pcport, }, ) } diff --git a/internal/devbox/devopt/devboxopts.go b/internal/devbox/devopt/devboxopts.go index 4f51b50d943..df47b1c2174 100644 --- a/internal/devbox/devopt/devboxopts.go +++ b/internal/devbox/devopt/devboxopts.go @@ -18,9 +18,9 @@ type Opts struct { } type ProcessComposeOpts struct { - ExtraFlags []string - Background bool - PCPort int + ExtraFlags []string + Background bool + ProcessComposePort int } type GenerateOpts struct { diff --git a/internal/devbox/services.go b/internal/devbox/services.go index d45f85ac213..ff3585fa591 100644 --- a/internal/devbox/services.go +++ b/internal/devbox/services.go @@ -218,8 +218,8 @@ func (d *Devbox) StartProcessManager( for _, flag := range processComposeOpts.ExtraFlags { args = append(args, "--pcflags", flag) } - if processComposeOpts.PCPort != 0 { - args = append(args, "--pcport", strconv.Itoa(processComposeOpts.PCPort)) + if processComposeOpts.ProcessComposePort != 0 { + args = append(args, "--pcport", strconv.Itoa(processComposeOpts.ProcessComposePort)) } return d.runDevboxServicesScript(ctx, args) @@ -258,10 +258,10 @@ func (d *Devbox) StartProcessManager( svcs, d.projectDir, services.ProcessComposeOpts{ - BinPath: processComposeBinPath, - Background: processComposeOpts.Background, - ExtraFlags: processComposeOpts.ExtraFlags, - PCPort: processComposeOpts.PCPort, + BinPath: processComposeBinPath, + Background: processComposeOpts.Background, + ExtraFlags: processComposeOpts.ExtraFlags, + ProcessComposePort: processComposeOpts.ProcessComposePort, }, ) } diff --git a/internal/services/manager.go b/internal/services/manager.go index 6eb44a19d2e..7a71d18ac5d 100644 --- a/internal/services/manager.go +++ b/internal/services/manager.go @@ -41,10 +41,10 @@ type globalProcessComposeConfig struct { } type ProcessComposeOpts struct { - BinPath string - ExtraFlags []string - Background bool - PCPort int + BinPath string + ExtraFlags []string + Background bool + ProcessComposePort int } func newGlobalProcessComposeConfig() *globalProcessComposeConfig { @@ -129,15 +129,11 @@ func StartProcessManager( config := readGlobalProcessComposeJSON(configFile) config.File = configFile - port, err := selectPort(processComposeConfig.PCPort) + port, err := selectPort(processComposeConfig.ProcessComposePort) if err != nil { return fmt.Errorf("failed to select port: %v", err) } - if !isPortAvailable(port) { - return fmt.Errorf("port %d is already in use", port) - } - // Start building the process-compose command flags := []string{"-p", strconv.Itoa(port)} upCommand := []string{"up"} diff --git a/internal/services/ports.go b/internal/services/ports.go index 6af4617dad6..05e6700172d 100644 --- a/internal/services/ports.go +++ b/internal/services/ports.go @@ -41,12 +41,11 @@ func getAvailablePort() (int, error) { return 0, errors.WithStack(err) } - l, err := net.ListenTCP("tcp", addr) - if err != nil { + if isPortAvailable(addr.Port) != nil { return 0, errors.WithStack(err) } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port, nil + + return addr.Port, nil } for range 1000 { @@ -65,18 +64,18 @@ func getAvailablePort() (int, error) { func selectPort(configPort int) (int, error) { if configPort != 0 { - return configPort, nil + return configPort, isPortAvailable(configPort) } - if portStr, exists := os.LookupEnv("PC_PORT_NUM"); exists { + if portStr, exists := os.LookupEnv("DEVBOX_PC_PORT_NUM"); exists { port, err := strconv.Atoi(portStr) if err != nil { - return 0, fmt.Errorf("invalid PC_PORT_NUM environment variable: %v", err) + return 0, fmt.Errorf("invalid DEVBOX_PC_PORT_NUM environment variable: %v", err) } if port <= 0 { - return 0, fmt.Errorf("invalid PC_PORT_NUM environment variable: ports cannot be less than 0") + return 0, fmt.Errorf("invalid DEVBOX_PC_PORT_NUM environment variable: ports cannot be less than 0") } - return port, nil + return port, isPortAvailable(port) } return getAvailablePort() @@ -86,11 +85,11 @@ func isAllowed(port int) bool { return port > 1024 && disallowedPorts[port] == "" } -func isPortAvailable(port int) bool { +func isPortAvailable(port int) error { ln, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) if err != nil { - return false + return fmt.Errorf("port %d is already in use", port) } ln.Close() - return true + return nil } From b3e3c11af1960887453043f6ac0bea1fe71a30d9 Mon Sep 17 00:00:00 2001 From: John Lago <750845+Lagoja@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:29:57 -0700 Subject: [PATCH 3/3] Fix port availability check --- internal/services/ports.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/internal/services/ports.go b/internal/services/ports.go index 05e6700172d..b03f9792285 100644 --- a/internal/services/ports.go +++ b/internal/services/ports.go @@ -36,16 +36,11 @@ var disallowedPorts = map[int]string{ func getAvailablePort() (int, error) { get := func() (int, error) { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + port, err := isPortAvailable(0) if err != nil { return 0, errors.WithStack(err) } - - if isPortAvailable(addr.Port) != nil { - return 0, errors.WithStack(err) - } - - return addr.Port, nil + return port, nil } for range 1000 { @@ -64,7 +59,7 @@ func getAvailablePort() (int, error) { func selectPort(configPort int) (int, error) { if configPort != 0 { - return configPort, isPortAvailable(configPort) + return isPortAvailable(configPort) } if portStr, exists := os.LookupEnv("DEVBOX_PC_PORT_NUM"); exists { @@ -75,7 +70,7 @@ func selectPort(configPort int) (int, error) { if port <= 0 { return 0, fmt.Errorf("invalid DEVBOX_PC_PORT_NUM environment variable: ports cannot be less than 0") } - return port, isPortAvailable(port) + return isPortAvailable(port) } return getAvailablePort() @@ -85,11 +80,11 @@ func isAllowed(port int) bool { return port > 1024 && disallowedPorts[port] == "" } -func isPortAvailable(port int) error { - ln, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) +func isPortAvailable(port int) (int, error) { + ln, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", port)) if err != nil { - return fmt.Errorf("port %d is already in use", port) + return 0, fmt.Errorf("port %d is already in use", port) } - ln.Close() - return nil + defer ln.Close() + return ln.Addr().(*net.TCPAddr).Port, nil }