Compare commits
14 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 8f5a069e77 | |||
| 07717afd01 | |||
| d8f6981be1 | |||
| d7bb314087 | |||
| 7679fada0a | |||
| a164c24e8e | |||
| 6148ae6439 | |||
| f54a13fc8a | |||
| f7f90a71fc | |||
| f1b0a53520 | |||
| f32e9a6889 | |||
| a1b27b5501 | |||
| ae0ce3824f | |||
| f98c5ccaf7 |
@@ -65,8 +65,11 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
|
||||
| # | Scenario | Steps | Expected |
|
||||
|---|----------|-------|----------|
|
||||
| TC-API-0.1 | Unauthenticated health check | GET /api/health | 200 OK, `{"status":"ok"}` |
|
||||
| TC-API-0.2 | DB-touching readiness check — healthy (GRO-2678) | GET /api/readyz | 200 OK, `{"status":"ready"}` |
|
||||
| TC-API-0.3 | DB-touching readiness check — response body safe | GET /api/readyz and inspect body | Body contains only `status` and (on error) `check` fields — no raw SQL, driver messages, or stack traces |
|
||||
|
||||
> **Note (GRO-1544):** Health endpoint registered on `api` basePath before auth middleware at `/api/health`. The old path `/health` was incorrect (routed to web pod via HTTPRoute `/*` rule).
|
||||
> **Note (GRO-2678):** `/api/readyz` is a separate DB-touching endpoint for monitoring. It is intentionally NOT used for K8s liveness/readiness probes — those remain DB-less to avoid pod cycling on transient DB blips.
|
||||
|
||||
### 4.1 Authentication
|
||||
|
||||
|
||||
@@ -69,9 +69,14 @@ describe("auth init", () => {
|
||||
beforeEach(() => {
|
||||
dbSelectResult = [];
|
||||
vi.clearAllMocks();
|
||||
// Stub fetch so OIDC discovery requests resolve instantly during tests.
|
||||
// Without this, AbortSignal.timeout(5000) in auth.ts races with vitest's
|
||||
// 5000ms default test timeout and causes flaky failures.
|
||||
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 503 }));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
process.env = { ...originalEnv };
|
||||
});
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@
|
||||
*/
|
||||
import postgres from "postgres";
|
||||
import { drizzle } from "drizzle-orm/postgres-js";
|
||||
import { migrate } from "drizzle-orm/postgres-js/migrator";
|
||||
import { execSync } from "node:child_process";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import * as schema from "./schema.js";
|
||||
@@ -46,7 +46,6 @@ import {
|
||||
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const __dirname = dirname(__filename);
|
||||
const MIGRATIONS_FOLDER = resolve(__dirname, "../migrations");
|
||||
|
||||
async function reset() {
|
||||
const url = process.env.DATABASE_URL;
|
||||
@@ -73,7 +72,7 @@ async function reset() {
|
||||
// across processes regardless of how many connections the work uses —
|
||||
// it does NOT require the work to share the lock's session.
|
||||
//
|
||||
// Therefore `max` must be ≥ 2: 1 reserved for the lock + ≥1 free for
|
||||
// Therefore `max` must be >= 2: 1 reserved for the lock + >=1 free for
|
||||
// the work. `max: 1` would let `reserve()` consume the only connection
|
||||
// and every query inside the callback would block forever waiting for
|
||||
// a connection that never frees (connection-starvation deadlock). We
|
||||
@@ -122,7 +121,15 @@ async function reset() {
|
||||
console.log("✓ All tables and enums dropped\n");
|
||||
|
||||
console.log("Running migrations...");
|
||||
await migrate(db, { migrationsFolder: MIGRATIONS_FOLDER });
|
||||
// GRO-2672: drizzle-orm's migrate() has a high-water-mark bug that skips
|
||||
// migrations with stale `when` timestamps (0001, 0003, 0010, 0011). Use
|
||||
// drizzle-kit instead -- it applies migrations by hash, matching the K8s
|
||||
// migrate Job behaviour exactly.
|
||||
execSync("pnpm exec drizzle-kit migrate", {
|
||||
stdio: "inherit",
|
||||
env: { ...process.env },
|
||||
cwd: resolve(__dirname, ".."),
|
||||
});
|
||||
console.log("✓ Migrations applied\n");
|
||||
|
||||
console.log("Seeding database...");
|
||||
|
||||
@@ -69,9 +69,14 @@ describe("auth init", () => {
|
||||
beforeEach(() => {
|
||||
dbSelectResult = [];
|
||||
vi.clearAllMocks();
|
||||
// Stub fetch so OIDC discovery requests resolve instantly during tests.
|
||||
// Without this, AbortSignal.timeout(5000) in auth.ts races with vitest's
|
||||
// 5000ms default test timeout and causes flaky failures.
|
||||
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 503 }));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
process.env = { ...originalEnv };
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
|
||||
// Mock auth lib so getAuth() throws — non-bypass paths hit the 503 "not configured" guard.
|
||||
vi.mock("../lib/auth.js", () => ({
|
||||
getAuth: () => {
|
||||
throw new Error("auth not configured");
|
||||
},
|
||||
initAuth: vi.fn(),
|
||||
getActiveProviders: vi.fn(() => []),
|
||||
}));
|
||||
|
||||
describe("authMiddleware bypass: /api/readyz", () => {
|
||||
it("serves /api/readyz without auth (bypass before auth check)", async () => {
|
||||
// Ensure AUTH_DISABLED is not set so the bypass is exercised, not AUTH_DISABLED shortcut.
|
||||
const prev = process.env.AUTH_DISABLED;
|
||||
delete process.env.AUTH_DISABLED;
|
||||
|
||||
const { authMiddleware } = await import("../middleware/auth.js");
|
||||
const app = new Hono();
|
||||
app.use("/api/*", authMiddleware);
|
||||
app.get("/api/readyz", (c) => c.json({ status: "ok" }, 200));
|
||||
|
||||
const res = await app.request("/api/readyz", { method: "GET" });
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
if (prev !== undefined) process.env.AUTH_DISABLED = prev;
|
||||
});
|
||||
|
||||
it("blocks non-whitelisted /api/* paths when auth is not configured", async () => {
|
||||
const prev = process.env.AUTH_DISABLED;
|
||||
delete process.env.AUTH_DISABLED;
|
||||
|
||||
const { authMiddleware } = await import("../middleware/auth.js");
|
||||
const app = new Hono();
|
||||
app.use("/api/*", authMiddleware);
|
||||
app.get("/api/staff", (c) => c.json({ ok: true }, 200));
|
||||
|
||||
const res = await app.request("/api/staff", { method: "GET" });
|
||||
expect(res.status).toBe(503);
|
||||
|
||||
if (prev !== undefined) process.env.AUTH_DISABLED = prev;
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,87 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
|
||||
// ─── Mock db module ───────────────────────────────────────────────────────────
|
||||
|
||||
let selectImpl: () => Promise<unknown>;
|
||||
|
||||
vi.mock("@groombook/db", () => {
|
||||
const staff = new Proxy(
|
||||
{ _name: "staff" },
|
||||
{
|
||||
get(_target, prop) {
|
||||
if (prop === "_name") return "staff";
|
||||
return { table: "staff", column: prop };
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
select: (_fields: unknown) => ({
|
||||
from: (_table: unknown) => ({
|
||||
limit: (_n: number) => selectImpl(),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
staff,
|
||||
};
|
||||
});
|
||||
|
||||
// ─── Build test app ───────────────────────────────────────────────────────────
|
||||
|
||||
async function makeApp() {
|
||||
// Import after mocks are in place
|
||||
const { getDb, staff } = await import("@groombook/db");
|
||||
|
||||
const app = new Hono();
|
||||
app.get("/api/readyz", async (c) => {
|
||||
try {
|
||||
await getDb().select({ id: staff.id }).from(staff).limit(1);
|
||||
return c.json({ status: "ready" }, 200);
|
||||
} catch (err) {
|
||||
console.error("[readyz] DB check failed:", err);
|
||||
return c.json({ status: "degraded", check: "db" }, 503);
|
||||
}
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
// ─── Tests ────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("GET /api/readyz", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("returns 200 {status:'ready'} when DB query succeeds", async () => {
|
||||
selectImpl = () => Promise.resolve([{ id: "staff-1" }]);
|
||||
|
||||
const app = await makeApp();
|
||||
const res = await app.request("/api/readyz", { method: "GET" });
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(body.status).toBe("ready");
|
||||
});
|
||||
|
||||
it("returns 503 {status:'degraded',check:'db'} when DB query throws", async () => {
|
||||
selectImpl = () => Promise.reject(new Error("42P01: relation staff does not exist"));
|
||||
|
||||
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
const app = await makeApp();
|
||||
const res = await app.request("/api/readyz", { method: "GET" });
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
|
||||
expect(res.status).toBe(503);
|
||||
expect(body.status).toBe("degraded");
|
||||
expect(body.check).toBe("db");
|
||||
|
||||
// Raw SQL / driver error must NOT appear in the response body
|
||||
expect(JSON.stringify(body)).not.toContain("42P01");
|
||||
expect(JSON.stringify(body)).not.toContain("relation");
|
||||
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -65,6 +65,17 @@ app.use(
|
||||
app.get("/health", (c) => c.json({ status: "ok" }));
|
||||
// /api/health: used by Gateway HTTPRoute (/api/* → API pod)
|
||||
app.get("/api/health", (c) => c.json({ status: "ok" }));
|
||||
// /api/readyz: DB-touching deep health check consumed by monitoring (not K8s probes)
|
||||
// Distinct from /health so a dropped schema triggers an alert without cycling pods (GRO-2678)
|
||||
app.get("/api/readyz", async (c) => {
|
||||
try {
|
||||
await getDb().select({ id: staff.id }).from(staff).limit(1);
|
||||
return c.json({ status: "ready" }, 200);
|
||||
} catch (err) {
|
||||
console.error("[readyz] DB check failed:", err);
|
||||
return c.json({ status: "degraded", check: "db" }, 503);
|
||||
}
|
||||
});
|
||||
|
||||
// Public booking routes — no auth required, must be registered before auth middleware
|
||||
app.route("/api/book", bookRouter);
|
||||
|
||||
+6
-1
@@ -329,5 +329,10 @@ export async function initAuth(): Promise<void> {
|
||||
});
|
||||
})();
|
||||
|
||||
await authInitPromise;
|
||||
try {
|
||||
await authInitPromise;
|
||||
} catch (err) {
|
||||
authInitPromise = null; // allow retry on next call
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,7 +23,7 @@ if (process.env.AUTH_DISABLED === "true") {
|
||||
}
|
||||
|
||||
export const authMiddleware: MiddlewareHandler = async (c, next) => {
|
||||
if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health") {
|
||||
if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health" || c.req.path === "/api/readyz") {
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user