From 6412d73a5a420c8fef70af68ee8a4b6a040662a7 Mon Sep 17 00:00:00 2001
From: Alexis Deruelle <alexis.deruelle@laposte.net>
Date: Tue, 5 May 2020 06:31:58 +0200
Subject: [PATCH] Fix port availability test (#333)

* Let OS allocate port number

Port availability might be tricky if some port is already in use on the
'all' interface '0.0.0.0'.

The proposed solution is to let the OS allocate the port for us using 0
as port specifier :

- first create a server instance to allocate a port number
- save port number
- close the server
- return the port number to the caller

This should be safe granted the OS doesn't reuse the port numbers on
consecutive port allocations.

see :
- about Node.js Net module : https://nodejs.org/docs/latest-v12.x/api/net.html#net_server_listen_port_host_backlog_callback
- about safety around reusing port number : https://unix.stackexchange.com/a/132524

Signed-off-by: Alexis Deruelle <alexis.deruelle@gmail.com>
---
 spec/src/main/port_spec.ts      | 13 +++---------
 src/main/context-handler.ts     |  2 +-
 src/main/index.ts               |  2 +-
 src/main/port.ts                | 37 ++++++++++++++++++---------------
 src/main/routes/port-forward.ts |  2 +-
 5 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/spec/src/main/port_spec.ts b/spec/src/main/port_spec.ts
index 58c62c73b0..8c7566e02c 100644
--- a/spec/src/main/port_spec.ts
+++ b/spec/src/main/port_spec.ts
@@ -2,13 +2,10 @@ import { EventEmitter } from 'events'
 
 class MockServer extends EventEmitter {
   listen = jest.fn((obj) => {
-    if(obj.port < 9003) {
-      this.emit('error', new Error("fail!"))
-    } else {
-      this.emit('listening', {})
-    }
+    this.emit('listening', {})
     return this
   })
+  address = () => { return { port: 12345 }}
   unref = jest.fn()
   close = jest.fn((cb) => {
     cb()
@@ -29,11 +26,7 @@ describe("getFreePort", () => {
     jest.clearAllMocks()
   })
 
-  it("fails for an invalid range", async () => {
-    return expect(port.getFreePort(1, 2)).rejects.toMatch('free port')
-  })
-
   it("finds the next free port", async () => {
-    return expect(port.getFreePort(9000, 9005)).resolves.toBe(9003)
+    return expect(port.getFreePort()).resolves.toEqual(expect.any(Number))
   })
 })
diff --git a/src/main/context-handler.ts b/src/main/context-handler.ts
index 431f8c450a..817b684d00 100644
--- a/src/main/context-handler.ts
+++ b/src/main/context-handler.ts
@@ -135,7 +135,7 @@ export class ContextHandler {
 
     let serverPort: number = null
     try {
-      serverPort = await getFreePort(49901, 65535)
+      serverPort = await getFreePort()
     } catch(error) {
       logger.error(error)
       throw(error)
diff --git a/src/main/index.ts b/src/main/index.ts
index 700b44a4e3..8bf99ca583 100644
--- a/src/main/index.ts
+++ b/src/main/index.ts
@@ -48,7 +48,7 @@ async function main() {
   let port: number = null
   // find free port
   try {
-    port = await getFreePort(49152, 65535)
+    port = await getFreePort()
   } catch (error) {
     logger.error(error)
     await dialog.showErrorBox("Lens Error", "Could not find a free port for the cluster proxy")
diff --git a/src/main/port.ts b/src/main/port.ts
index 6240575ee1..f419f4e78c 100644
--- a/src/main/port.ts
+++ b/src/main/port.ts
@@ -1,27 +1,30 @@
 import logger from "./logger"
 import { createServer } from "net"
+import { AddressInfo } from "net"
 
-// Adapted from https://gist.github.com/mikeal/1840641#gistcomment-2896667
-function checkPort(port: number) {
+const getNextAvailablePort = () => {
+  logger.debug("getNextAvailablePort() start")
   const server = createServer()
   server.unref()
-  return new Promise((resolve, reject) =>
+  return new Promise<number>((resolve, reject) =>
     server
-      .on('error', error => reject(error))
-      .on('listening', () => server.close(() => resolve(port)))
-      .listen({host: "127.0.0.1", port: port}))
+      .on('error', (error: any) => reject(error))
+      .on('listening', () => {
+        logger.debug("*** server listening event ***")
+        const _port = (server.address() as AddressInfo).port
+        server.close(() => resolve(_port))
+      })
+      .listen({host: "127.0.0.1", port: 0}))
 }
 
-export async function getFreePort(firstPort: number, lastPort: number): Promise<number> {
-  let port = firstPort
-
-  while(true) {
-    try {
-      logger.debug("Checking port " + port + " availability ...")
-      await checkPort(port)
-      return(port)
-    } catch(error) {
-      if(++port > lastPort) throw("Could not find a free port")
-    }
+export const getFreePort = async () => {
+  logger.debug("getFreePort() start")
+  let freePort: number = null
+  try {
+    freePort = await getNextAvailablePort()
+    logger.debug("got port : " + freePort)
+  } catch(error) {
+    throw("getNextAvailablePort() threw: '" + error + "'")
   }
+  return freePort
 }
diff --git a/src/main/routes/port-forward.ts b/src/main/routes/port-forward.ts
index f79cc9228b..4b0a4077f0 100644
--- a/src/main/routes/port-forward.ts
+++ b/src/main/routes/port-forward.ts
@@ -36,7 +36,7 @@ class PortForward {
   }
 
   public async start() {
-    this.localPort = await getFreePort(8000, 9999)
+    this.localPort = await getFreePort()
     const kubectlBin = await bundledKubectl.kubectlPath()
     const args = [
       "--kubeconfig", this.kubeConfig,