diff --git a/server/lib/rebuildClientAssociations.ts b/server/lib/rebuildClientAssociations.ts index abad6b7aa..efb856825 100644 --- a/server/lib/rebuildClientAssociations.ts +++ b/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) => - !existingClientSiteIds.includes(clientId) && - !otherResourceClientIds.has(clientId) // dont add if already connected via another site resource + (clientId) => !existingClientSiteIds.includes(clientId) ); const clientSitesToInsert = clientSitesToAdd.map((clientId) => ({ @@ -700,7 +707,7 @@ async function handleMessagesForSiteClients( trx: Transaction | typeof db = db ): Promise { 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; diff --git a/server/private/lib/orgRebuildCounter.ts b/server/private/lib/orgRebuildCounter.ts index 58d954f66..7107b9b8d 100644 --- a/server/private/lib/orgRebuildCounter.ts +++ b/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 diff --git a/server/routers/newt/handleNewtDisconnectingMessage.ts b/server/routers/newt/handleNewtDisconnectingMessage.ts index afc208ac8..811dc68a1 100644 --- a/server/routers/newt/handleNewtDisconnectingMessage.ts +++ b/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, diff --git a/server/routers/site/createSite.ts b/server/routers/site/createSite.ts index bddf5b251..b24608609 100644 --- a/server/routers/site/createSite.ts +++ b/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()