diff --git a/server/openapi.json b/server/openapi.json index 32f5013a1..237bcea29 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -13307,11 +13307,6 @@ }, "maxItems": 10 }, - "egressPollIntervalSeconds": { - "type": "integer", - "minimum": 5, - "maximum": 600 - }, "egressNotifications": { "type": "array", "items": { diff --git a/server/src/api/validation/settingsValidation.ts b/server/src/api/validation/settingsValidation.ts index 5bf635054..51ea3df5a 100644 --- a/server/src/api/validation/settingsValidation.ts +++ b/server/src/api/validation/settingsValidation.ts @@ -1,5 +1,5 @@ import { CHECK_TTL_SENTINEL } from "@/domain/checks/check.type.js"; -import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "@/domain/egress/egress.type.js"; +import { MAX_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; import { parseEgressTarget } from "@/utils/egressTarget.js"; import { z } from "zod"; @@ -56,7 +56,6 @@ export const updateAppSettingsBodyValidation = z egressCheckEnabled: z.boolean().optional(), egressCheckTargets: z.array(egressTargetValidation).max(MAX_EGRESS_TARGETS).optional(), // empty list falls back to DEFAULT_EGRESS_TARGETS - egressPollIntervalSeconds: z.number().int().min(MIN_EGRESS_POLL_INTERVAL_SECONDS).max(MAX_EGRESS_POLL_INTERVAL_SECONDS).optional(), egressNotifications: z.array(z.string().regex(/^[0-9a-fA-F]{24}$/, "Invalid notification id")).optional(), }) .strip() diff --git a/server/src/domain/app-settings/app-settings.model.ts b/server/src/domain/app-settings/app-settings.model.ts index dfe138b8f..194ee44a2 100644 --- a/server/src/domain/app-settings/app-settings.model.ts +++ b/server/src/domain/app-settings/app-settings.model.ts @@ -1,6 +1,6 @@ import { Schema, model, type Types } from "mongoose"; import type { Settings, SettingsThresholds } from "@/domain/app-settings/app-settings.type.js"; -import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; +import { DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; interface AppSettingsDocument extends Omit { _id: Types.ObjectId; @@ -45,7 +45,6 @@ const AppSettingsSchema = new Schema( globalProxyId: { type: String, default: null }, egressCheckEnabled: { type: Boolean, default: false }, egressCheckTargets: { type: [String], default: () => [...DEFAULT_EGRESS_TARGETS] }, - egressPollIntervalSeconds: { type: Number, default: DEFAULT_EGRESS_POLL_INTERVAL_SECONDS }, egressNotifications: { type: [String], default: () => [] }, // Notification ids, stored as strings like globalProxyId }, { timestamps: true } diff --git a/server/src/domain/app-settings/app-settings.repository.mongo.ts b/server/src/domain/app-settings/app-settings.repository.mongo.ts index 795a2db35..4527c4245 100644 --- a/server/src/domain/app-settings/app-settings.repository.mongo.ts +++ b/server/src/domain/app-settings/app-settings.repository.mongo.ts @@ -3,7 +3,7 @@ import { ISettingsRepository } from "@/domain/app-settings/app-settings-reposito import type { Settings, SettingsUpdate } from "@/domain/app-settings/app-settings.type.js"; import { AppSettingsModel, type AppSettingsDocument } from "@/domain/app-settings/app-settings.model.js"; import { toStringId, toDateString } from "@/utils/mongoMappers.js"; -import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; +import { DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; class MongoSettingsRepository implements ISettingsRepository { private toEntity = (doc: AppSettingsDocument): Settings => { @@ -33,7 +33,6 @@ class MongoSettingsRepository implements ISettingsRepository { globalProxyId: doc.globalProxyId ?? undefined, egressCheckEnabled: doc.egressCheckEnabled ?? false, egressCheckTargets: doc.egressCheckTargets ?? [...DEFAULT_EGRESS_TARGETS], - egressPollIntervalSeconds: doc.egressPollIntervalSeconds ?? DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, egressNotifications: doc.egressNotifications ?? [], createdAt: toDateString(doc.createdAt), updatedAt: toDateString(doc.updatedAt), diff --git a/server/src/domain/app-settings/app-settings.type.ts b/server/src/domain/app-settings/app-settings.type.ts index 9dc4eae45..dbb2d0665 100644 --- a/server/src/domain/app-settings/app-settings.type.ts +++ b/server/src/domain/app-settings/app-settings.type.ts @@ -54,7 +54,6 @@ export interface Settings { // Egress self-check (see domain/egress) egressCheckEnabled: boolean; egressCheckTargets: string[]; - egressPollIntervalSeconds: number; // recovery poll cadence while degraded, not a general schedule egressNotifications: string[]; // Notification ids to receive the "egress recovered" alert createdAt: string; updatedAt: string; diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index 1088356ff..4e6b0e515 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -7,7 +7,7 @@ import type { INetworkService } from "@/service/networkService.js"; import type { IProxyResolver } from "@/service/network/ProxyResolver.js"; import type { ILogger } from "@/utils/logger.js"; import { - DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, + EGRESS_RECOVERY_POLL_SECONDS, DEFAULT_EGRESS_TARGETS, type EgressAssessment, type EgressProbeResult, @@ -136,11 +136,6 @@ export class EgressService implements IEgressService { return targets.length > 0 ? targets : [...DEFAULT_EGRESS_TARGETS]; }; - private toIntervalMs = (seconds: number | undefined): number => { - const valid = typeof seconds === "number" && Number.isFinite(seconds) && seconds > 0; - return (valid ? seconds : DEFAULT_EGRESS_POLL_INTERVAL_SECONDS) * 1000; - }; - // **************************** // Event-triggered assessment // **************************** @@ -181,7 +176,7 @@ export class EgressService implements IEgressService { const state = await this.egressStateRepository.findSingleton(); if (state.status === "degraded") { // Recovery is detected by the scheduled job. Make sure the row exists without touching its schedule. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, false); + await this.scheduleRecoveryCheck(false); return "degraded"; } @@ -195,7 +190,7 @@ export class EgressService implements IEgressService { // Schedule the recovery job before recording the transition: a crash between the two then leaves a stray row // while ok, which the job's first run removes, rather than a degraded state that nothing is polling. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, true); + await this.scheduleRecoveryCheck(true); const degraded = await this.egressStateRepository.markDegraded(results, now); this.logger.warn({ message: degraded @@ -224,8 +219,8 @@ export class EgressService implements IEgressService { // reschedule=true sets nextScheduledAt (start of an episode); false only inserts the row if it is missing, // so re-arming while degraded never pushes a pending run back. - private scheduleRecoveryCheck = async (pollIntervalSeconds: number | undefined, reschedule: boolean) => { - const intervalMs = this.toIntervalMs(pollIntervalSeconds); + private scheduleRecoveryCheck = async (reschedule: boolean) => { + const intervalMs = EGRESS_RECOVERY_POLL_SECONDS * 1000; const seed: JobSeed = { id: RECOVERY_JOB_ID, type: RECOVERY_JOB_TYPE, diff --git a/server/src/domain/egress/egress.type.ts b/server/src/domain/egress/egress.type.ts index 49c8d462f..080202ce1 100644 --- a/server/src/domain/egress/egress.type.ts +++ b/server/src/domain/egress/egress.type.ts @@ -9,11 +9,13 @@ export type EgressAssessment = EgressStatus | "disabled"; // Sensible defaults for a fresh install. Bare hosts are ICMP-pinged, host:port is a TCP connect, // http(s):// URLs are fetched. Any one target being reachable means egress is fine. export const DEFAULT_EGRESS_TARGETS = ["1.1.1.1", "8.8.8.8"] as const; -export const DEFAULT_EGRESS_POLL_INTERVAL_SECONDS = 30; -export const MIN_EGRESS_POLL_INTERVAL_SECONDS = 5; -export const MAX_EGRESS_POLL_INTERVAL_SECONDS = 600; export const MAX_EGRESS_TARGETS = 10; +// While the instance is marked degraded, every failing check is flagged from stored state without probing, so +// this interval is the window in which a recovered instance still discards its checks. It is deliberately fixed +// and not configurable: the only effect of raising it is a longer blind window. +export const EGRESS_RECOVERY_POLL_SECONDS = 5; + // Instance-global singleton persisted in MongoDB so that every worker process and the API // process share one view of the state (the queue worker may run separately from the API). export interface EgressState { diff --git a/server/src/domain/monitors/monitor.type.ts b/server/src/domain/monitors/monitor.type.ts index 5f40d6d80..bacaedb47 100644 --- a/server/src/domain/monitors/monitor.type.ts +++ b/server/src/domain/monitors/monitor.type.ts @@ -50,13 +50,15 @@ export const UptimeDetailsSupportedTypes = ["http", "ping", "port", "game", "grp export type UptimeDetailsSupportedType = (typeof UptimeDetailsSupportedTypes)[number]; export const supportsUptimeDetails = (type: MonitorType): type is UptimeDetailsSupportedType => UptimeDetailsSupportedTypes.some((t) => t === type); -// Types whose transport failure can be the instance's own loss of egress. Hardware and docker are left out: -// a Capture agent or a docker socket is normally reached locally, and a docker monitor pointed at a remote -// host over TCP is deliberately out of scope rather than assumed local. +// Types whose check leaves the instance, and whose failure to reach the target can therefore be the instance's +// own loss of egress. A hardware check is an ordinary outbound HTTP request to the Capture agent, so it counts +// however far away the agent is. `unknown` is excluded because no request is made for it. export const EgressAttributableTypes = [ "http", "ping", "pagespeed", + "hardware", + "docker", "port", "game", "grpc", @@ -64,7 +66,14 @@ export const EgressAttributableTypes = [ "dns", ] as const satisfies readonly MonitorType[]; export type EgressAttributableType = (typeof EgressAttributableTypes)[number]; -export const isEgressAttributable = (type: MonitorType): type is EgressAttributableType => EgressAttributableTypes.some((t) => t === type); + +// A Docker daemon reached over a unix socket is local IPC and cannot fail through egress; one reached over +// TCP or TLS is as remote as any other target. The URL, not the type, settles it. +const isLocalDockerSocket = (monitor: Pick): boolean => + monitor.type === "docker" && (monitor.url?.trim().startsWith("unix://") ?? false); + +export const isEgressAttributable = (monitor: Pick): boolean => + EgressAttributableTypes.some((t) => t === monitor.type) && !isLocalDockerSocket(monitor); export const MonitorStatuses = ["up", "down", "paused", "initializing", "maintenance", "breached"] as const; export type MonitorStatus = (typeof MonitorStatuses)[number]; diff --git a/server/src/service/network/DNSProvider.ts b/server/src/service/network/DNSProvider.ts index 10eff8d74..475026c54 100644 --- a/server/src/service/network/DNSProvider.ts +++ b/server/src/service/network/DNSProvider.ts @@ -1,5 +1,6 @@ import { DNSStatusPayload, MonitorStatusResponse } from "@/types/network.js"; import { IStatusProvider } from "@/service/network/IStatusProvider.js"; +import dns from "dns"; import type { Resolver } from "dns/promises"; import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; import { AppError } from "@/utils/AppError.js"; @@ -8,6 +9,16 @@ import { NETWORK_ERROR } from "@/types/network.js"; const SERVICE_NAME = "DNSProvider"; +// c-ares codes where the DNS server replied: the name does not exist, holds no such record, or the server +// declined or answered badly. Anything else (timeout, connection refused, a local resolver fault) means we +// never got an answer, so the failure could be our own loss of egress rather than the target's. +const ANSWERED_DNS_CODES: ReadonlySet = new Set([dns.NODATA, dns.FORMERR, dns.SERVFAIL, dns.NOTFOUND, dns.NOTIMP, dns.REFUSED, dns.BADRESP]); + +const dnsServerAnswered = (error: unknown): boolean => { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return typeof code === "string" && ANSWERED_DNS_CODES.has(code); +}; + export class DNSProvider implements IStatusProvider { readonly type = "dns"; constructor(private createResolver: () => Resolver) {} @@ -63,6 +74,7 @@ export class DNSProvider implements IStatusProvider { type: monitor.type, status: false, code: NETWORK_ERROR, + peerResponded: dnsServerAnswered(error), message: error instanceof Error ? error.message : String(error), responseTime, payload: { @@ -81,6 +93,7 @@ export class DNSProvider implements IStatusProvider { type: monitor.type, status: true, code: 200, + peerResponded: true, message: "Success", responseTime, payload: { diff --git a/server/src/service/network/GrpcProvider.ts b/server/src/service/network/GrpcProvider.ts index 340fdc01a..b651344cb 100644 --- a/server/src/service/network/GrpcProvider.ts +++ b/server/src/service/network/GrpcProvider.ts @@ -25,6 +25,17 @@ export class GrpcProvider implements IStatusProvider { return type === "grpc"; } + // gRPC codes that mean the call never reached a server. Everything else in 0-16 is the server's own answer, + // including NOT_SERVING, which shares NETWORK_ERROR with a connection failure and so cannot be read from `code`. + // A call that fails with no gRPC code at all never got as far as the protocol. + private peerAnswered(code: number | undefined): boolean { + const UNREACHABLE_GRPC_CODES: ReadonlySet = new Set([ + 4, // DEADLINE_EXCEEDED + 14, // UNAVAILABLE + ]); + return typeof code === "number" && !UNREACHABLE_GRPC_CODES.has(code); + } + private getGrpcStatusName(code: number): string { const statusNames: Record = { 0: "OK", @@ -148,6 +159,7 @@ export class GrpcProvider implements IStatusProvider { type: type, status: false, code: grpcError.grpcCode ?? 5000, + peerResponded: this.peerAnswered(grpcError.grpcCode), message: grpcError.message ?? "gRPC health check failed", responseTime, payload: payload ?? null, @@ -163,6 +175,8 @@ export class GrpcProvider implements IStatusProvider { type: type, status: isServing, code: isServing ? 200 : 5000, + // The server replied, whatever it said, so this failure is never the instance's own egress. + peerResponded: true, message: isServing ? `gRPC service healthy (${grpcPayload.servingStatus})` : `gRPC service unhealthy (${grpcPayload.servingStatus})`, responseTime, payload: grpcPayload, diff --git a/server/src/types/network.ts b/server/src/types/network.ts index 1f8afe267..b7a3d885a 100644 --- a/server/src/types/network.ts +++ b/server/src/types/network.ts @@ -33,6 +33,12 @@ export interface MonitorStatusResponse< status: boolean; code: number; message: string; + // Whether the peer answered, for failures where `code` alone cannot say. `code` carries an HTTP status for + // providers that speak HTTP, but DNS and gRPC report their own codes, and gRPC reuses NETWORK_ERROR for a + // server that replied NOT_SERVING. Providers that can tell the two apart set this; the rest leave it unset + // and the HTTP-status heuristic stands. Consumed by the egress check, which must only blame the instance's + // own connectivity when nothing answered. + peerResponded?: boolean; responseTime?: number; payload?: T | string | null; timings?: GotTimings; diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index 3f09e998d..d7b93ae3d 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -46,10 +46,15 @@ export class CheckProducer implements ICheckProducer { return dockerTlsKey ?? undefined; } - // Only a failure to reach the target at all can be the instance's own fault. Any HTTP response (4xx, 5xx, - // or a 200 with a content mismatch) proves the target was reached, and hardware/docker failures are local. + // Only a failure to reach the target at all can be the instance's own fault. A provider that can tell the + // two apart says so outright; for the rest, any HTTP status code proves the target answered (a 4xx, a 5xx, + // or a 200 with a content mismatch). + private peerAnswered(status: MonitorStatusResponse): boolean { + return status.peerResponded ?? isHttpStatusCode(status.code); + } + private isTransportFailure(monitor: Monitor, status: MonitorStatusResponse): boolean { - return status.status === false && isEgressAttributable(monitor.type) && !isHttpStatusCode(status.code); + return status.status === false && isEgressAttributable(monitor) && !this.peerAnswered(status); } // The egress check must never stop a check being recorded, so a rejection is logged and treated as unknown. @@ -89,6 +94,13 @@ export class CheckProducer implements ICheckProducer { return null; } + // Leaving the window is explicit rather than a side effect of evaluating the resulting check, which the + // degraded-egress short-circuit skips. "initializing" is resolved to up or down by the first check that is + // evaluated, so the monitor cannot sit in "maintenance" for the length of an egress outage. + if (monitor.status === "maintenance") { + monitor = await this.monitorsRepository.updateById(monitor.id, monitor.teamId, { status: "initializing" }); + } + // Step 1b: Acquire status const proxyUrl = await this.proxyResolver.resolve(monitor); const dockerTlsKey = await this.resolveDockerTlsKey(monitor); @@ -98,15 +110,12 @@ export class CheckProducer implements ICheckProducer { throw new Error("No network response"); } - // Step 1c: On a transport failure, ask whether the instance itself can reach anything before blaming the target. - // Null means the egress check is disabled (or failed internally) and the check is treated as usual. - const egressStatus = this.isTransportFailure(monitor, status) ? await this.assessEgress(monitor.id) : null; - // **************************** // Step 2: Record // **************************** - // Step 2a: Create & record a check, return null if fail + // Step 2a: Create & record a check, return null if fail. + // Built before the egress assessment below so that the probe's duration stays out of `createdAt`. const check = this.checkService.toCheck(status); if (!check) { this.logger.warn({ @@ -117,13 +126,17 @@ export class CheckProducer implements ICheckProducer { }); return null; } + + // Step 2b: On a transport failure, ask whether the instance itself can reach anything before blaming the + // target. Null means the egress check is disabled (or failed internally) and the check is treated as usual. + const egressStatus = this.isTransportFailure(monitor, status) ? await this.assessEgress(monitor.id) : null; if (egressStatus !== null) { check.egressStatus = egressStatus; } - // Step 2b: Add to buffer + // Step 2c: Add to buffer this.bufferService.addToBuffer(check); - // Step 2c: Handle docker logs + // Step 2d: Handle docker logs if (status.type === "docker") { const dockerLogs = await this.dockerLogsService.buildDockerLogs(status as MonitorStatusResponse); for (const dockerLog of dockerLogs) { diff --git a/server/test/unit/providers/network/dnsProvider.test.ts b/server/test/unit/providers/network/dnsProvider.test.ts index d485ca599..c6cfff23b 100644 --- a/server/test/unit/providers/network/dnsProvider.test.ts +++ b/server/test/unit/providers/network/dnsProvider.test.ts @@ -209,6 +209,49 @@ describe("DNSProvider", () => { expect(mockResolverInstance.resolve4).toHaveBeenCalledWith("example.com"); }); + // ── Peer reachability ──────────────────────────────────────────────────── + // The egress check must only blame the instance's own connectivity when nothing answered. Every DNS failure + // carries NETWORK_ERROR, so `peerResponded` is the only thing separating an authoritative answer from silence. + + it.each([ + ["ENOTFOUND", "NXDOMAIN: the server answered that the name does not exist"], + ["ENODATA", "the server answered with no record of that type"], + ["ESERVFAIL", "the server answered with a failure"], + ["EREFUSED", "the server declined to answer"], + ["EFORMERR", "the server rejected the query as malformed"], + ["ENOTIMP", "the server does not implement the query"], + ["EBADRESP", "the server replied, badly"], + ])("reports the peer as having answered for %s", async (code) => { + const error = Object.assign(new Error("dns failure"), { code }); + mockResolverInstance.resolve4.mockRejectedValueOnce(error); + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(NETWORK_ERROR); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ETIMEOUT"], ["ECONNREFUSED"], ["ECANCELLED"], [undefined]])("reports the peer as unreachable for %s", async (code) => { + const error = code ? Object.assign(new Error("dns failure"), { code }) : new Error("dns failure"); + mockResolverInstance.resolve4.mockRejectedValueOnce(error); + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered on success", async () => { + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + it("stringifies non-Error throws from outside timeRequest", async () => { mockResolverInstance.setServers.mockImplementationOnce(() => { throw 42; diff --git a/server/test/unit/providers/network/grpcProvider.test.ts b/server/test/unit/providers/network/grpcProvider.test.ts index 63e8859f2..0d53943d2 100644 --- a/server/test/unit/providers/network/grpcProvider.test.ts +++ b/server/test/unit/providers/network/grpcProvider.test.ts @@ -362,6 +362,59 @@ describe("GrpcProvider", () => { [99, "UNKNOWN"], ]; + // ── Peer reachability ──────────────────────────────────────────────── + // NOT_SERVING and a connection failure both report NETWORK_ERROR, and gRPC codes are not HTTP statuses, + // so `peerResponded` is what stops the egress check reading a server's answer as a transport failure. + + it("reports the peer as having answered when the server replies NOT_SERVING", async () => { + const { provider } = createProvider({ behavior: "not-serving" }); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(5000); + expect(result.peerResponded).toBe(true); + }); + + it.each([[5], [7], [12], [16], [2]])("reports the peer as having answered for gRPC code %i", async (code) => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + + it.each([[4], [14]])("reports the peer as unreachable for gRPC code %i", async (code) => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as unreachable when the failure carries no gRPC code", async () => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ message: "socket hang up" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + it.each(statusCodes)("maps gRPC code %i to %s", async (code, expectedName) => { const client = { Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), diff --git a/server/test/unit/services/checkProducer.test.ts b/server/test/unit/services/checkProducer.test.ts index bb2a0b536..c83a890cc 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -79,6 +79,31 @@ describe("CheckProducer", () => { expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); }); + it("moves a monitor out of maintenance explicitly once the window is over", async () => { + const monitor = makeMonitor({ status: "maintenance" }); + const { producer, defaults } = createProducer({ + monitorsRepository: { + updateById: jest.fn().mockResolvedValue({ ...monitor, status: "initializing" }), + findDockerTlsKeyById: jest.fn().mockResolvedValue(null), + }, + }); + + await producer.produce(monitor); + + // The exit used to be a side effect of evaluating the resulting check, which the degraded-egress + // short-circuit skips. Left implicit, a monitor could sit in "maintenance" for a whole egress outage. + expect(defaults.monitorsRepository.updateById).toHaveBeenCalledWith("m1", "team", { status: "initializing" }); + expect(defaults.networkService.requestStatus).toHaveBeenCalled(); + }); + + it("does not touch the status of a monitor that was not in maintenance", async () => { + const { producer, defaults } = createProducer(); + + await producer.produce(makeMonitor({ status: "up" })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + it("proceeds with the check when the maintenance window is inactive", async () => { const { producer, defaults } = createProducer({ maintenanceWindowsRepository: { findByMonitorId: jest.fn().mockResolvedValue([{ ...activeWindow(), active: false }]) }, @@ -265,23 +290,73 @@ describe("CheckProducer", () => { expect(check.egressStatus).toBe("degraded"); }); - it("never consults egress for a failing docker or hardware check", async () => { - for (const type of ["docker", "hardware"] as const) { + it("never consults egress for a docker daemon on a local socket", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type: "docker" }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "docker", url: "unix:///var/run/docker.sock" })); + + // Local IPC never leaves the instance, so it cannot fail through egress. + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + }); + + it("consults egress for a hardware check and for a docker daemon reached over TCP", async () => { + for (const monitor of [ + makeMonitor({ type: "hardware", url: "http://capture.example.com:59232/api/v1/metrics" }), + makeMonitor({ type: "docker", url: "tcp://docker.example.com:2376" }), + ]) { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ - networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type }) }, + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type: monitor.type }) }, checkService: { toCheck: jest.fn().mockReturnValue(check) }, egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, }); - await producer.produce(makeMonitor({ type })); + await producer.produce(monitor); - expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); - expect(check).not.toHaveProperty("egressStatus"); - expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + // Both requests leave the instance, so our own loss of egress can be the cause. + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); } }); + it("does not consult egress when the provider reports that the peer answered", async () => { + // gRPC NOT_SERVING and an authoritative DNS NXDOMAIN both carry NETWORK_ERROR, so only this flag separates + // them from a connection failure. + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, peerResponded: true }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "dns" })); + + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + }); + + it("consults egress when the provider reports that nothing answered, despite an HTTP-shaped code", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, code: 503, peerResponded: false }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "grpc" })); + + // An explicit report from the provider outranks the HTTP-status heuristic. + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); + }); + it("flags a failing check as degraded when the egress service reports degraded egress", async () => { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts index e339ef7ec..adc79feb4 100644 --- a/server/test/unit/services/egressService.test.ts +++ b/server/test/unit/services/egressService.test.ts @@ -21,7 +21,6 @@ const makeState = (overrides?: Partial): EgressState => ({ const makeSettings = (overrides?: Record) => ({ egressCheckEnabled: true, egressCheckTargets: ["1.1.1.1", "8.8.8.8"], - egressPollIntervalSeconds: 30, egressNotifications: ["notif-1"], ...overrides, }); @@ -33,7 +32,7 @@ const makeJob = (overrides?: Partial): Job => ({ refId: null, isActive: true, nextScheduledAt: 1_000, - intervalMs: 30_000, + intervalMs: 5_000, lockedBy: "worker-1", lockedUntil: 61_000, runCount: 0, @@ -245,7 +244,7 @@ describe("EgressService", () => { refId: null, isActive: true, nextScheduledAt: expect.any(Number), - intervalMs: 30_000, + intervalMs: 5_000, }); expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled(); // Job first, transition second: a crash in between leaves a stray row, never an unpolled degraded state @@ -266,7 +265,7 @@ describe("EgressService", () => { const result = await service.assessAfterFailure(); expect(result).toBe("degraded"); - expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 5_000 })); expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("already recorded") })); }); @@ -293,21 +292,10 @@ describe("EgressService", () => { expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); - expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 5_000 })); expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled(); }); - it("uses the default poll interval for the recovery job when the setting is missing or invalid", async () => { - const { service, defaults } = createService({ - settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: undefined })) }, - networkService: { requestStatus: statusFor([]) }, - }); - - await service.assessAfterFailure(); - - expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ intervalMs: 30_000 })); - }); - it("falls back to the default targets when the configured list is empty", async () => { const { service, defaults } = createService({ settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckTargets: [] })) }, diff --git a/server/test/unit/validation/settingsValidation.test.ts b/server/test/unit/validation/settingsValidation.test.ts index 745cbd81e..506090783 100644 --- a/server/test/unit/validation/settingsValidation.test.ts +++ b/server/test/unit/validation/settingsValidation.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "@jest/globals"; import { updateAppSettingsBodyValidation } from "../../../src/api/validation/settingsValidation.ts"; -import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "../../../src/domain/egress/egress.type.ts"; +import { MAX_EGRESS_TARGETS } from "../../../src/domain/egress/egress.type.ts"; const validNotificationId = "64b7f0c2a1d2e3f4a5b6c7d8"; @@ -10,13 +10,11 @@ describe("settingsValidation", () => { const parsed = updateAppSettingsBodyValidation.parse({ egressCheckEnabled: true, egressCheckTargets: ["1.1.1.1", "dns.google:53", "https://example.com/health"], - egressPollIntervalSeconds: 30, egressNotifications: [validNotificationId], }); expect(parsed.egressCheckEnabled).toBe(true); expect(parsed.egressCheckTargets).toEqual(["1.1.1.1", "dns.google:53", "https://example.com/health"]); - expect(parsed.egressPollIntervalSeconds).toBe(30); expect(parsed.egressNotifications).toEqual([validNotificationId]); }); @@ -62,21 +60,6 @@ describe("settingsValidation", () => { } }); - it("accepts the poll interval boundaries", () => { - expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MIN_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( - MIN_EGRESS_POLL_INTERVAL_SECONDS - ); - expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MAX_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( - MAX_EGRESS_POLL_INTERVAL_SECONDS - ); - }); - - it("rejects out-of-range and non-integer poll intervals", () => { - for (const interval of [MIN_EGRESS_POLL_INTERVAL_SECONDS - 1, MAX_EGRESS_POLL_INTERVAL_SECONDS + 1, 0, -5, 30.5, "30"]) { - expect(() => updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: interval })).toThrow(); - } - }); - it("accepts an empty notification list", () => { expect(updateAppSettingsBodyValidation.parse({ egressNotifications: [] }).egressNotifications).toEqual([]); });