Pick always a new port for newt

server/routers/target/createTarget.ts

@@ -7,10 +7,11 @@ import HttpCode from "@server/types/HttpCode";
 import createHttpError from "http-errors";
 import logger from "@server/logger";
 import { addPeer } from "../gerbil/peers";
-import { eq, and } from "drizzle-orm";
 import { isIpInCidr } from "@server/lib/ip";
 import { fromError } from "zod-validation-error";
 import { addTargets } from "../newt/targets";
+import { eq } from "drizzle-orm";
+import { pickPort } from "./ports";
 
 // Regular expressions for validation
 const DOMAIN_REGEX =
@@ -147,37 +148,7 @@ export async function createTarget(
                 );
             }
 
-            // Fetch resources for this site
+            const { internalPort, targetIps } = await pickPort(site.siteId!);
-            const resourcesRes = await db.query.resources.findMany({
-                where: eq(resources.siteId, site.siteId)
-            });
-
-            // TODO: is this all inefficient?
-            // Fetch targets for all resources of this site
-            let targetIps: string[] = [];
-            let targetInternalPorts: number[] = [];
-            await Promise.all(
-                resourcesRes.map(async (resource) => {
-                    const targetsRes = await db.query.targets.findMany({
-                        where: eq(targets.resourceId, resource.resourceId)
-                    });
-                    targetsRes.forEach((target) => {
-                        targetIps.push(`${target.ip}/32`);
-                        if (target.internalPort) {
-                            targetInternalPorts.push(target.internalPort);
-                        }
-                    });
-                })
-            );
-
-            let internalPort!: number;
-            // pick a port
-            for (let i = 40000; i < 65535; i++) {
-                if (!targetInternalPorts.includes(i)) {
-                    internalPort = i;
-                    break;
-                }
-            }
 
             if (!internalPort) {
                 return next(

server/routers/target/ports.ts

@@ -0,0 +1,48 @@
+import { db } from "@server/db";
+import { resources, targets } from "@server/db/schema";
+import { eq } from "drizzle-orm";
+
+let currentBannedPorts: number[] = [];
+
+export async function pickPort(siteId: number): Promise<{
+    internalPort: number;
+    targetIps: string[];
+}> {
+    // Fetch resources for this site
+    const resourcesRes = await db.query.resources.findMany({
+        where: eq(resources.siteId, siteId)
+    });
+
+    // TODO: is this all inefficient?
+    // Fetch targets for all resources of this site
+    let targetIps: string[] = [];
+    let targetInternalPorts: number[] = [];
+    await Promise.all(
+        resourcesRes.map(async (resource) => {
+            const targetsRes = await db.query.targets.findMany({
+                where: eq(targets.resourceId, resource.resourceId)
+            });
+            targetsRes.forEach((target) => {
+                targetIps.push(`${target.ip}/32`);
+                if (target.internalPort) {
+                    targetInternalPorts.push(target.internalPort);
+                }
+            });
+        })
+    );
+
+    let internalPort!: number;
+    // pick a port random port from 40000 to 65535 that is not in use
+    for (let i = 0; i < 1000; i++) {
+        internalPort = Math.floor(Math.random() * 25535) + 40000;
+        if (
+            !targetInternalPorts.includes(internalPort) &&
+            !currentBannedPorts.includes(internalPort)
+        ) {
+            break;
+        }
+    }
+    currentBannedPorts.push(internalPort);
+
+    return { internalPort, targetIps };
+}

server/routers/target/updateTarget.ts

@@ -10,6 +10,7 @@ import logger from "@server/logger";
 import { fromError } from "zod-validation-error";
 import { addPeer } from "../gerbil/peers";
 import { addTargets } from "../newt/targets";
+import { pickPort } from "./ports";
 
 // Regular expressions for validation
 const DOMAIN_REGEX =
@@ -84,15 +85,14 @@ export async function updateTarget(
         }
 
         const { targetId } = parsedParams.data;
-        const updateData = parsedBody.data;
 
-        const [updatedTarget] = await db
+        const [target] = await db
-            .update(targets)
+            .select()
-            .set(updateData)
+            .from(targets)
             .where(eq(targets.targetId, targetId))
-            .returning();
+            .limit(1);
 
-        if (!updatedTarget) {
+        if (!target) {
             return next(
                 createHttpError(
                     HttpCode.NOT_FOUND,
@@ -107,13 +107,13 @@ export async function updateTarget(
                 siteId: resources.siteId
             })
             .from(resources)
-            .where(eq(resources.resourceId, updatedTarget.resourceId!));
+            .where(eq(resources.resourceId, target.resourceId!));
 
         if (!resource) {
             return next(
                 createHttpError(
                     HttpCode.NOT_FOUND,
-                    `Resource with ID ${updatedTarget.resourceId} not found`
+                    `Resource with ID ${target.resourceId} not found`
                 )
             );
         }
@@ -132,24 +132,29 @@ export async function updateTarget(
                 )
             );
         }
+
+        const { internalPort, targetIps } = await pickPort(site.siteId!);
+
+        if (!internalPort) {
+            return next(
+                createHttpError(
+                    HttpCode.BAD_REQUEST,
+                    `No available internal port`
+                )
+            );
+        }
+
+        const [updatedTarget] = await db
+            .update(targets)
+            .set({
+                ...parsedBody.data,
+                internalPort
+            })
+            .where(eq(targets.targetId, targetId))
+            .returning();
+
         if (site.pubKey) {
             if (site.type == "wireguard") {
-                // TODO: is this all inefficient?
-                // Fetch resources for this site
-                const resourcesRes = await db.query.resources.findMany({
-                    where: eq(resources.siteId, site.siteId)
-                });
-
-                // Fetch targets for all resources of this site
-                const targetIps = await Promise.all(
-                    resourcesRes.map(async (resource) => {
-                        const targetsRes = await db.query.targets.findMany({
-                            where: eq(targets.resourceId, resource.resourceId)
-                        });
-                        return targetsRes.map((target) => `${target.ip}/32`);
-                    })
-                );
-
                 await addPeer(site.exitNodeId!, {
                     publicKey: site.pubKey,
                     allowedIps: targetIps.flat()