Merge pull request #3396 from fosrl/dev

1.19.4-s.5

server/lib/rebuildClientAssociations.ts

@@ -582,10 +582,17 @@ async function rebuildClientAssociationsFromSiteResourceImpl(
                 ? mergedAllClientIds
                 : [];
 
+            // Note: we deliberately do NOT exclude clients covered by another
+            // site resource here (unlike clientSitesToRemove below). Doing so
+            // previously caused a permanent gap: if resource A saw resource B's
+            // cache row and skipped adding (assuming B would maintain it), and
+            // B's own rebuild made the same assumption about A, the site-level
+            // row could end up never inserted by anyone even though both
+            // resources' client associations were otherwise correct.
+            // onConflictDoNothing makes a redundant insert harmless, so there's
+            // no correctness reason to skip here.
             const clientSitesToAdd = expectedClientIdsForSite.filter(
-                (clientId) =>
+                (clientId) => !existingClientSiteIds.includes(clientId)
-                    !existingClientSiteIds.includes(clientId) &&
-                    !otherResourceClientIds.has(clientId) // dont add if already connected via another site resource
             );
 
             const clientSitesToInsert = clientSitesToAdd.map((clientId) => ({
@@ -700,7 +707,7 @@ async function handleMessagesForSiteClients(
     trx: Transaction | typeof db = db
 ): Promise<void> {
     if (!site.exitNodeId) {
-        logger.warn(
+        logger.debug(
             `Exit node ID not on site ${site.siteId} so there is no reason to update clients because it must be offline`
         );
         return;
@@ -714,14 +721,14 @@ async function handleMessagesForSiteClients(
         .limit(1);
 
     if (!exitNode) {
-        logger.warn(
+        logger.debug(
             `Exit node not found for site ${site.siteId} so there is no reason to update clients because it must be offline`
         );
         return;
     }
 
     if (!site.publicKey) {
-        logger.warn(
+        logger.debug(
             `Site publicKey not set for site ${site.siteId} so cannot add peers to clients`
         );
         return;
@@ -735,7 +742,7 @@ async function handleMessagesForSiteClients(
         .where(eq(newts.siteId, siteId))
         .limit(1);
     if (!newt) {
-        logger.warn(
+        logger.debug(
             `Newt not found for site ${siteId} so cannot add peers to clients`
         );
         return;

server/private/lib/orgRebuildCounter.ts

@@ -14,7 +14,7 @@
 import { redis } from "#private/lib/redis";
 import logger from "@server/logger";
 
-export const ORG_REBUILD_CONCURRENCY_LIMIT = 5;
+export const ORG_REBUILD_CONCURRENCY_LIMIT = 10;
 
 // Safety-net TTL: slightly longer than the rebuild lock TTL (120 s). If a
 // server process dies while holding a rebuild, this ensures the counter key

server/routers/newt/handleNewtDisconnectingMessage.ts

@@ -19,7 +19,7 @@ export const handleNewtDisconnectingMessage: MessageHandler = async (
     }
 
     if (!newt.siteId) {
-        logger.warn("Newt has no client ID!");
+        logger.warn("Newt has no site ID!");
         return;
     }
 
@@ -34,6 +34,12 @@ export const handleNewtDisconnectingMessage: MessageHandler = async (
                 .where(eq(sites.siteId, newt.siteId!))
                 .returning();
 
+            if (!site) {
+                throw new Error(
+                    `Could not find site ${newt.siteId} to update disconnection from disconnect message`
+                );
+            }
+
             await fireSiteOfflineAlert(
                 site.orgId,
                 site.siteId,

server/routers/site/createSite.ts

@@ -268,7 +268,11 @@ export async function createSite(
 
         let newSite: Site | undefined;
         try {
-            if (subnet && exitNodeId) {
+            if (type === "wireguard" && subnet && exitNodeId) {
+                // Only wireguard sites actually persist the provided subnet/exitNodeId.
+                // Newt sites have their subnet/exit node chosen (under a lock) when the
+                // newt connects, so validating them here is both unnecessary and racy,
+                // since pickSiteDefaults does not lock the subnet it suggests.
                 //make sure the subnet is in the range of the exit node if provided
                 const [exitNode] = await db
                     .select()