refactor: make ssh port stable if already in .ssh/config

FabianKramm · FabianKramm · commit 69cb5bf48508 · 2022-03-31T14:52:10.000+02:00
diff --git a/pkg/devspace/services/proxycommands/commands.go b/pkg/devspace/services/proxycommands/commands.go
@@ -51,12 +51,12 @@ func startProxyCommands(ctx *devspacecontext.Context, devContainer *latest.DevCo
 	}
 
 	// get a local port
-	port, err := ssh.LockPort()
+	port, err := ssh.GetInstance(ctx.Log).LockPort()
 	if err != nil {
 		return errors.Wrap(err, "find port")
 	}
 
-	defer ssh.ReleasePort(port)
+	defer ssh.GetInstance(ctx.Log).ReleasePort(port)
 
 	// get remote port
 	defaultRemotePort := DefaultRemotePort
diff --git a/pkg/devspace/services/ssh/config.go b/pkg/devspace/services/ssh/config.go
@@ -9,12 +9,18 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
 )
 
 var configLock sync.Mutex
 
+var (
+	MarkerStartPrefix = "# DevSpace Start "
+	MarkerEndPrefix   = "# DevSpace End "
+)
+
 func configureSSHConfig(host, port string, log log.Logger) error {
 	configLock.Lock()
 	defer configLock.Unlock()
@@ -25,7 +31,7 @@ func configureSSHConfig(host, port string, log log.Logger) error {
 	}
 
 	sshConfigPath := filepath.Join(homeDir, ".ssh", "config")
-	newFile, err := replaceHost(sshConfigPath, host, port)
+	newFile, err := addHost(sshConfigPath, host, port)
 	if err != nil {
 		return errors.Wrap(err, "parse ssh config")
 	}
@@ -43,7 +49,65 @@ func configureSSHConfig(host, port string, log log.Logger) error {
 	return nil
 }
 
-func replaceHost(path, host, port string) (string, error) {
+type DevSpaceSSHEntry struct {
+	Host     string
+	Hostname string
+	Port     int
+}
+
+func ParseDevSpaceHosts(path string) ([]DevSpaceSSHEntry, error) {
+	var reader io.Reader
+	f, err := os.Open(path)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return nil, err
+		}
+
+		reader = strings.NewReader("")
+	} else {
+		reader = f
+		defer f.Close()
+	}
+
+	configScanner := scanner.NewScanner(reader)
+	inSection := false
+
+	entries := []DevSpaceSSHEntry{}
+	current := &DevSpaceSSHEntry{}
+	for configScanner.Scan() {
+		text := strings.TrimSpace(configScanner.Text())
+		if strings.HasPrefix(text, MarkerStartPrefix) {
+			inSection = true
+		} else if strings.HasPrefix(text, MarkerEndPrefix) {
+			if current.Host != "" && current.Port > 0 && current.Hostname != "" {
+				entries = append(entries, *current)
+			}
+			current = &DevSpaceSSHEntry{}
+			inSection = false
+		} else if inSection {
+			if strings.HasPrefix(text, "Host ") {
+				current.Host = strings.TrimPrefix(text, "Host ")
+			}
+			if strings.HasPrefix(text, "Port ") {
+				port := strings.TrimPrefix(text, "Port ")
+				intPort, err := strconv.Atoi(port)
+				if err == nil {
+					current.Port = intPort
+				}
+			}
+			if strings.HasPrefix(text, "HostName ") {
+				current.Hostname = strings.TrimPrefix(text, "HostName ")
+			}
+		}
+	}
+	if configScanner.Err() != nil {
+		return nil, errors.Wrap(err, "parse ssh config")
+	}
+
+	return entries, nil
+}
+
+func addHost(path, host, port string) (string, error) {
 	var reader io.Reader
 	f, err := os.Open(path)
 	if err != nil {
@@ -77,7 +141,6 @@ func replaceHost(path, host, port string) (string, error) {
 	}
 
 	// add new section
-	newLines = append(newLines, "")
 	newLines = append(newLines, startMarker)
 	newLines = append(newLines, "Host "+host)
 	newLines = append(newLines, "  HostName localhost")
@@ -87,7 +150,5 @@ func replaceHost(path, host, port string) (string, error) {
 	newLines = append(newLines, "  UserKnownHostsFile /dev/null")
 	newLines = append(newLines, "  User devspace")
 	newLines = append(newLines, endMarker)
-	newLines = append(newLines, "")
-
 	return strings.Join(newLines, "\n"), nil
 }
diff --git a/pkg/devspace/services/ssh/port.go b/pkg/devspace/services/ssh/port.go
@@ -3,45 +3,114 @@ package ssh
 import (
 	"fmt"
 	"github.com/loft-sh/devspace/helper/util/port"
+	"github.com/loft-sh/devspace/pkg/util/log"
+	"github.com/mitchellh/go-homedir"
+	"github.com/pkg/errors"
 	"math/rand"
+	"path/filepath"
 	"sync"
 )
 
+type PortManager interface {
+	LockPort() (int, error)
+	LockSpecificPort(p int) error
+	ReleasePort(p int)
+}
+
 var (
-	portRangeStart = 10000
-	portRangeEnd   = 12000
-	portMap        = map[int]bool{}
-	portMutex      sync.Mutex
+	portManager     PortManager
+	portManagerOnce sync.Once
 )
 
-func LockPort() (int, error) {
-	portMutex.Lock()
-	defer portMutex.Unlock()
+func GetInstance(log log.Logger) PortManager {
+	portManagerOnce.Do(func() {
+		portManager = NewManager(log)
+	})
+	return portManager
+}
+
+func NewManager(log log.Logger) PortManager {
+	homeDir, err := homedir.Dir()
+	if err != nil {
+		log.Errorf("%v", errors.Wrap(err, "get home dir"))
+	}
+
+	sshConfigPath := filepath.Join(homeDir, ".ssh", "config")
+	hosts, err := ParseDevSpaceHosts(sshConfigPath)
+	if err != nil {
+		log.Errorf("error parsing %s: %v", sshConfigPath, err)
+	}
+
+	reservedPorts := map[int]bool{}
+	for _, h := range hosts {
+		reservedPorts[h.Port] = true
+	}
+
+	return &manager{
+		reservedPorts:  reservedPorts,
+		portRangeStart: 10000,
+		portRangeEnd:   12000,
+		portMap:        map[int]bool{},
+	}
+}
+
+type manager struct {
+	m sync.Mutex
+
+	reservedPorts map[int]bool
+
+	portRangeStart int
+	portRangeEnd   int
+	portMap        map[int]bool
+}
+
+func (m *manager) LockSpecificPort(p int) error {
+	m.m.Lock()
+	defer m.m.Unlock()
+
+	if m.portMap[p] {
+		return fmt.Errorf("port %d already in use", p)
+	}
+
+	available, err := port.IsAvailable(fmt.Sprintf(":%d", p))
+	if err != nil {
+		return err
+	} else if !available {
+		return fmt.Errorf("port %d is already in use %v", p, err)
+	}
+
+	m.portMap[p] = true
+	return nil
+}
+
+func (m *manager) LockPort() (int, error) {
+	m.m.Lock()
+	defer m.m.Unlock()
 
 	var (
 		available bool
 		err       error
 	)
 	for i := 0; i < 10; i++ {
-		p := rand.Intn(portRangeEnd-portRangeStart+1) + portRangeStart
-		if portMap[p] {
+		p := rand.Intn(m.portRangeEnd-m.portRangeStart+1) + m.portRangeStart
+		if m.portMap[p] || m.reservedPorts[p] {
 			i--
 			continue
 		}
 
 		available, err = port.IsAvailable(fmt.Sprintf(":%d", p))
 		if available {
-			portMap[p] = true
+			m.portMap[p] = true
 			return p, nil
 		}
 	}
 
 	return 0, fmt.Errorf("couldn't find an open port: %v", err)
 }
 
-func ReleasePort(p int) {
-	portMutex.Lock()
-	defer portMutex.Unlock()
+func (m *manager) ReleasePort(p int) {
+	m.m.Lock()
+	defer m.m.Unlock()
 
-	delete(portMap, p)
+	delete(m.portMap, p)
 }
diff --git a/pkg/devspace/services/ssh/ssh.go b/pkg/devspace/services/ssh/ssh.go
@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"fmt"
 	"github.com/mgutz/ansi"
+	"github.com/mitchellh/go-homedir"
 	"io"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -56,24 +58,60 @@ func startSSH(ctx *devspacecontext.Context, name, arch string, sshConfig *latest
 		return nil
 	}
 
-	// get a local port
+	// configure ssh host
+	sshHost := name + "." + ctx.Config.Config().Name + ".devspace"
+	if sshConfig.LocalHostname != "" {
+		sshHost = sshConfig.LocalHostname
+	}
+
+	// try to find host port
+	homeDir, err := homedir.Dir()
+	if err != nil {
+		return errors.Wrap(err, "get home dir")
+	}
+
+	// get port
 	port := sshConfig.LocalPort
-	var err error
 	if port == 0 {
-		port, err = LockPort()
+		sshConfigPath := filepath.Join(homeDir, ".ssh", "config")
+		hosts, err := ParseDevSpaceHosts(sshConfigPath)
 		if err != nil {
-			return errors.Wrap(err, "find port")
+			ctx.Log.Debugf("error parsing %s: %v", sshConfigPath, err)
+		} else {
+			for _, h := range hosts {
+				if h.Host == sshHost {
+					port = h.Port
+				}
+			}
 		}
 
-		defer ReleasePort(port)
-	}
+		if port == 0 {
+			port, err = GetInstance(ctx.Log).LockPort()
+			if err != nil {
+				return errors.Wrap(err, "find port")
+			}
 
-	// configure ssh host
-	sshHost := name + "." + ctx.Config.Config().Name + ".devspace"
-	if sshConfig.LocalHostname != "" {
-		sshHost = sshConfig.LocalHostname
+			// update ssh config
+			err = configureSSHConfig(sshHost, strconv.Itoa(port), ctx.Log)
+			if err != nil {
+				return errors.Wrap(err, "update ssh config")
+			}
+		}
+	} else {
+		err = GetInstance(ctx.Log).LockSpecificPort(port)
+		if err != nil {
+			return errors.Wrap(err, "find port")
+		}
+
+		// update ssh config
+		err = configureSSHConfig(sshHost, strconv.Itoa(port), ctx.Log)
+		if err != nil {
+			return errors.Wrap(err, "update ssh config")
+		}
 	}
+	defer GetInstance(ctx.Log).ReleasePort(port)
 
+	// get a local port
 	// get remote port
 	defaultRemotePort := helperssh.DefaultPort
 	if sshConfig.RemoteAddress != "" {
@@ -99,12 +137,6 @@ func startSSH(ctx *devspacecontext.Context, name, arch string, sshConfig *latest
 		return errors.Wrap(err, "start ssh port forwarding")
 	}
 
-	// update ssh config
-	err = configureSSHConfig(sshHost, strconv.Itoa(port), ctx.Log)
-	if err != nil {
-		return errors.Wrap(err, "update ssh config")
-	}
-
 	// start ssh
 	return startSSHWithRestart(ctx, arch, sshConfig.RemoteAddress, sshHost, selector, parent)
 }

Original file line number	Diff line number	Diff line change
`@@ -51,12 +51,12 @@ func startProxyCommands(ctx devspacecontext.Context, devContainer latest.DevCo`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`// get a local port`
`54`		`- port, err := ssh.LockPort()`
	`54`	`+ port, err := ssh.GetInstance(ctx.Log).LockPort()`
`55`	`55`	`if err != nil {`
`56`	`56`	`return errors.Wrap(err, "find port")`
`57`	`57`	`}`
`58`	`58`
`59`		`- defer ssh.ReleasePort(port)`
	`59`	`+ defer ssh.GetInstance(ctx.Log).ReleasePort(port)`
`60`	`60`
`61`	`61`	`// get remote port`
`62`	`62`	`defaultRemotePort := DefaultRemotePort`