diff --git a/src/game-server-node/game-server-node.controller.spec.ts b/src/game-server-node/game-server-node.controller.spec.ts index f382b53d1..67e2e7d7c 100644 --- a/src/game-server-node/game-server-node.controller.spec.ts +++ b/src/game-server-node/game-server-node.controller.spec.ts @@ -70,3 +70,108 @@ describe("GameServerNodeController ping disk alerts", () => { expect(notifications.send).not.toHaveBeenCalled(); }); }); + +// RCON does not reach a hibernating server, so its own ping is answered instead. +describe("GameServerNodeController server ping reply", () => { + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let cache: { put: jest.Mock; forget: jest.Mock }; + let controller: GameServerNodeController; + + const server = (currentMatchId: string | null) => ({ + plugin_version: "1.0.0", + plugin_runtime: "swiftlys2", + connected: true, + enabled: true, + steam_relay: null as null, + is_dedicated: true, + game_server_node_id: null as null, + current_match: currentMatchId + ? { + id: currentMatchId, + current_match_map_id: null as null, + match_maps: [] as unknown[], + } + : null, + }); + + const ping = (query: Record) => + controller.ping({ + params: { serverId: "server-1" }, + query: { map: "de_overpass", pluginVersion: "1.0.0", ...query }, + } as any); + + beforeEach(() => { + hasura = { + query: jest.fn(), + mutation: jest.fn().mockResolvedValue({}), + }; + cache = { put: jest.fn(), forget: jest.fn() }; + const queue = { add: jest.fn(), remove: jest.fn() }; + + controller = new GameServerNodeController( + { warn: jest.fn(), log: jest.fn() } as any, + {} as any, + { get: jest.fn().mockReturnValue({}) } as any, + hasura as any, + cache as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + queue as any, + queue as any, + {} as any, + queue as any, + queue as any, + {} as any, + {} as any, + ); + }); + + it("tells a server with nothing loaded that a match is waiting", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server("match-1") }); + + expect(await ping({ matchId: "", hibernating: "true" })).toEqual({ + get_match: true, + }); + }); + + it("has nothing to say once the server has that match loaded", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server("match-1") }); + + expect(await ping({ matchId: "match-1" })).toEqual({ get_match: false }); + }); + + it("has nothing to say to an idle server with no match", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server(null) }); + + expect(await ping({ matchId: "" })).toEqual({ get_match: false }); + }); + + it("tells a server to drop a match it no longer has", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server(null) }); + + expect(await ping({ matchId: "match-1" })).toEqual({ get_match: true }); + }); + + it("never asks a plugin that does not name its match", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server("match-1") }); + + expect(await ping({})).toEqual({ get_match: false }); + }); + + it("remembers a hibernating server only for as long as it says so", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: server(null) }); + + await ping({ hibernating: "true" }); + expect(cache.put).toHaveBeenCalledWith( + "server:server-1:hibernating", + true, + expect.any(Number), + ); + + await ping({ hibernating: "false" }); + expect(cache.forget).toHaveBeenCalledWith("server:server-1:hibernating"); + }); +}); diff --git a/src/game-server-node/game-server-node.controller.ts b/src/game-server-node/game-server-node.controller.ts index c45ac80f9..bff989696 100644 --- a/src/game-server-node/game-server-node.controller.ts +++ b/src/game-server-node/game-server-node.controller.ts @@ -830,6 +830,12 @@ UNIT pluginRuntime: string; }; + // Plugins that can hibernate say so, and name the match they have loaded. + const { hibernating, matchId: loadedMatchId } = request.query as { + hibernating?: string; + matchId?: string; + }; + if (steamRelay && !steamID) { return; } @@ -857,6 +863,7 @@ UNIT is_dedicated: true, game_server_node_id: true, current_match: { + id: true, current_match_map_id: true, match_maps: { id: true, @@ -873,6 +880,27 @@ UNIT throw Error("server not found"); } + if (hibernating === "true") { + await this.cache.put( + RconService.hibernatingCacheKey(String(serverId)), + true, + RconService.HIBERNATING_SECONDS, + ); + } else if (hibernating === "false") { + await this.cache.forget( + RconService.hibernatingCacheKey(String(serverId)), + ); + } + + // RCON cannot push `get_match` to a hibernating server, so the ping it + // sends anyway is answered instead: it fetches its match when the one it + // has loaded is not the one it has been given. + const reply = { + get_match: + loadedMatchId !== undefined && + (server.current_match?.id ?? "") !== loadedMatchId, + }; + // A disabled node-managed server is being torn down; refuse to bring it // back online. External servers keep running independently, so a disabled // one that's still heartbeating is genuinely online. @@ -928,7 +956,7 @@ UNIT map !== currentMap?.map.workshop_map_id ) { this.logger.warn(`server is still loading the map`); - return; + return reply; } } @@ -994,6 +1022,8 @@ UNIT jobId, }, ); + + return reply; } // SwiftlyS2 routes `sw plugins list` to its own log sink and answers RCON with diff --git a/src/matches/match-server-middleware/match-server-middleware.middleware.spec.ts b/src/matches/match-server-middleware/match-server-middleware.middleware.spec.ts new file mode 100644 index 000000000..df963b364 --- /dev/null +++ b/src/matches/match-server-middleware/match-server-middleware.middleware.spec.ts @@ -0,0 +1,111 @@ +import { MatchServerMiddlewareMiddleware } from "./match-server-middleware.middleware"; + +const OWN_SERVER = "11111111-1111-4111-8111-111111111111"; +const OTHER_SERVER = "22222222-2222-4222-8222-222222222222"; +const OTHER_MATCH = "33333333-3333-4333-8333-333333333333"; + +describe("MatchServerMiddlewareMiddleware", () => { + let hasura: { checkSecret: jest.Mock; query: jest.Mock }; + let middleware: MatchServerMiddlewareMiddleware; + let next: jest.Mock; + let status: jest.Mock; + + const passwords: Record = { + [OWN_SERVER]: "own-password", + [OTHER_SERVER]: "other-password", + }; + + const run = (request: { + params: Record; + body?: Record; + password: string; + }) => + middleware.use( + { + headers: { authorization: `Bearer ${request.password}` }, + params: request.params, + body: request.body, + } as any, + { status } as any, + next, + ); + + beforeEach(() => { + next = jest.fn(); + status = jest.fn().mockReturnValue({ end: jest.fn() }); + hasura = { + checkSecret: jest.fn().mockReturnValue(false), + query: jest.fn(async (query: any) => { + if (query.servers_by_pk) { + const id = query.servers_by_pk.__args.id; + return { + servers_by_pk: passwords[id] + ? { api_password: passwords[id] } + : null, + }; + } + + return { + matches_by_pk: { + id: OTHER_MATCH, + server: { + api_password: passwords[OTHER_SERVER], + current_match: { id: OTHER_MATCH }, + }, + }, + }; + }), + }; + middleware = new MatchServerMiddlewareMiddleware( + hasura as any, + { warn: jest.fn() } as any, + ); + }); + + it("lets a server through to its own route", async () => { + await run({ params: { serverId: OWN_SERVER }, password: "own-password" }); + + expect(next).toHaveBeenCalled(); + }); + + it("turns away the wrong password", async () => { + await run({ params: { serverId: OWN_SERVER }, password: "nope" }); + + expect(next).not.toHaveBeenCalled(); + expect(status).toHaveBeenCalledWith(401); + }); + + // The handler acts on the server in the route, so that is the one whose + // password has to match -- not one the caller names in the body. + it("authorizes the server the route names, whatever the body says", async () => { + await run({ + params: { serverId: OTHER_SERVER }, + body: { serverId: OWN_SERVER }, + password: "own-password", + }); + + expect(next).not.toHaveBeenCalled(); + expect(status).toHaveBeenCalledWith(401); + }); + + it("authorizes the match the route names, whatever the body says", async () => { + await run({ + params: { matchId: OTHER_MATCH }, + body: { serverId: OWN_SERVER }, + password: "own-password", + }); + + expect(next).not.toHaveBeenCalled(); + expect(status).toHaveBeenCalledWith(401); + }); + + it("still reads the body when the route names nothing", async () => { + await run({ + params: {}, + body: { serverId: OWN_SERVER }, + password: "own-password", + }); + + expect(next).toHaveBeenCalled(); + }); +}); diff --git a/src/matches/match-server-middleware/match-server-middleware.middleware.ts b/src/matches/match-server-middleware/match-server-middleware.middleware.ts index d32b6acc5..3b5b68a89 100644 --- a/src/matches/match-server-middleware/match-server-middleware.middleware.ts +++ b/src/matches/match-server-middleware/match-server-middleware.middleware.ts @@ -22,11 +22,15 @@ export class MatchServerMiddlewareMiddleware implements NestMiddleware { return next(); } - let matchId: string | undefined; - let serverId: string | undefined; + // The handler acts on the server or match in its route, so that is the one + // to authorize. The body only names it when the route does not. + const named = + request.params.matchId || request.params.serverId + ? request.params + : request.body; - matchId = request.body?.matchId || (request.params.matchId as string); - serverId = request.body?.serverId || (request.params.serverId as string); + const matchId = named?.matchId as string | undefined; + const serverId = named?.serverId as string | undefined; if (!matchId && !serverId) { return response.status(401).end(); diff --git a/src/rcon/rcon.service.spec.ts b/src/rcon/rcon.service.spec.ts index 1ea33734f..2f6bb34b6 100644 --- a/src/rcon/rcon.service.spec.ts +++ b/src/rcon/rcon.service.spec.ts @@ -17,6 +17,7 @@ describe("RconService connect failure", () => { let hasura: { query: jest.Mock; mutation: jest.Mock }; let notifications: { send: jest.Mock }; let service: RconService; + let hibernating: boolean; const dedicatedServer = (enabled: boolean, type = "Ranked") => ({ host: "10.0.0.1", @@ -35,6 +36,7 @@ describe("RconService connect failure", () => { beforeEach(() => { graceRemaining = -2; + hibernating = false; hasura = { query: jest.fn(), mutation: jest.fn().mockResolvedValue({}), @@ -49,7 +51,12 @@ describe("RconService connect failure", () => { { getConnection: () => ({ pttl: jest.fn(async () => graceRemaining) }), } as any, - {} as any, + { + has: jest.fn( + async (key: string) => + hibernating && key === RconService.hibernatingCacheKey("server-1"), + ), + } as any, ); }); @@ -101,6 +108,17 @@ describe("RconService connect failure", () => { expect(hasura.mutation).toHaveBeenCalled(); expect(notifications.send).not.toHaveBeenCalled(); }); + + // A hibernating server answers no RCON at all, and says so on its ping. + it("does not count a hibernating server's silence as a failure", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: dedicatedServer(true) }); + hibernating = true; + + await service.connect("server-1"); + + expect(hasura.mutation).not.toHaveBeenCalled(); + expect(notifications.send).not.toHaveBeenCalled(); + }); }); describe("RconService.listCvars", () => { diff --git a/src/rcon/rcon.service.ts b/src/rcon/rcon.service.ts index be1dcfe76..e92752dfc 100644 --- a/src/rcon/rcon.service.ts +++ b/src/rcon/rcon.service.ts @@ -26,6 +26,14 @@ export class RconService { private CONNECTION_TIMEOUT = 3 * 1000; private readonly GENERATE_CVARS_CACHE_KEY = "generate_cvars"; + // Set by the server's own ping. A hibernating server does not answer RCON at + // all, which is how it is meant to behave rather than a fault to report. + public static readonly HIBERNATING_SECONDS = 45; + + public static hibernatingCacheKey(serverId: string) { + return `server:${serverId}:hibernating`; + } + private connections: Record = {}; private connectTimeouts: Record = {}; @@ -198,7 +206,11 @@ export class RconService { this.logger.warn("Error during RCON cleanup:", cleanupError); } - if (server.rcon_status && server.is_dedicated) { + if ( + server.rcon_status && + server.is_dedicated && + !(await this.cache.has(RconService.hibernatingCacheKey(serverId))) + ) { void this.hasuraService.mutation({ update_servers_by_pk: { __args: {