fix: address code-review findings (startup validation, shutdown hooks, health check, deploy config)
Verified each finding against the real codebase first, since the review's diff was polluted by a stale leftover agent-worktree copy under .claude/worktrees/ (now removed) that made several already-fixed or never-broken issues look current (CORS, missing typecheck script, PWA icons, font CSS, Helmet/Swagger CSP - all confirmed fine live, not touched). Real fixes: - main.ts: fail-fast on missing DATABASE_URL/JWT_ACCESS_SECRET at boot, call enableShutdownHooks() so PrismaService.onModuleDestroy() actually runs on SIGTERM during k8s rolling deploys, PORT fallback 3000 -> 3001 to match every other place that documents the port. - health.controller.ts: log the real underlying DB error instead of discarding it, add a 5s timeout to the readiness query so a hung DB fails the probe fast instead of blocking indefinitely. - Frontend Dockerfile: declare ARG/ENV NEXT_PUBLIC_API_URL before the build step, since Next.js inlines NEXT_PUBLIC_* vars at build time and a runtime-only ConfigMap value never reaches the compiled bundle. - Backend ingress.yaml: drop the no-op rewrite-target regex for a plain prefix path (identical behavior, less confusing).
This commit is contained in:
1 parent
3f7c4ed404
commit
147417d7d4
4 files changed
+49
-6
No files matched your search
@@ -1,6 +1,7 @@
|
|||||||
import {
|
import {
|
||||||
Controller,
|
Controller,
|
||||||
Get,
|
Get,
|
||||||
|
Logger,
|
||||||
ServiceUnavailableException,
|
ServiceUnavailableException,
|
||||||
VERSION_NEUTRAL,
|
VERSION_NEUTRAL,
|
||||||
} from '@nestjs/common';
|
} from '@nestjs/common';
|
||||||
@@ -8,10 +9,29 @@ import { ApiTags } from '@nestjs/swagger';
|
|||||||
import { PrismaService } from '../prisma/prisma.service';
|
import { PrismaService } from '../prisma/prisma.service';
|
||||||
import { Public } from '../auth/decorators/public.decorator';
|
import { Public } from '../auth/decorators/public.decorator';
|
||||||
|
|
||||||
|
const READINESS_QUERY_TIMEOUT_MS = 5000;
|
||||||
|
|
||||||
|
async function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
|
||||||
|
let timer: NodeJS.Timeout;
|
||||||
|
const timeoutPromise = new Promise<never>((_, reject) => {
|
||||||
|
timer = setTimeout(
|
||||||
|
() => reject(new Error('Readiness query timed out')),
|
||||||
|
ms,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
try {
|
||||||
|
return await Promise.race([promise, timeoutPromise]);
|
||||||
|
} finally {
|
||||||
|
clearTimeout(timer!);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@ApiTags('health')
|
@ApiTags('health')
|
||||||
@Public()
|
@Public()
|
||||||
@Controller({ path: 'health', version: VERSION_NEUTRAL })
|
@Controller({ path: 'health', version: VERSION_NEUTRAL })
|
||||||
export class HealthController {
|
export class HealthController {
|
||||||
|
private readonly logger = new Logger(HealthController.name);
|
||||||
|
|
||||||
constructor(private readonly prisma: PrismaService) {}
|
constructor(private readonly prisma: PrismaService) {}
|
||||||
|
|
||||||
@Get()
|
@Get()
|
||||||
@@ -22,9 +42,13 @@ export class HealthController {
|
|||||||
@Get('ready')
|
@Get('ready')
|
||||||
async readiness(): Promise<{ status: 'ok' }> {
|
async readiness(): Promise<{ status: 'ok' }> {
|
||||||
try {
|
try {
|
||||||
await this.prisma.$queryRaw`SELECT 1`;
|
await withTimeout(
|
||||||
|
this.prisma.$queryRaw`SELECT 1`,
|
||||||
|
READINESS_QUERY_TIMEOUT_MS,
|
||||||
|
);
|
||||||
return { status: 'ok' };
|
return { status: 'ok' };
|
||||||
} catch {
|
} catch (error: unknown) {
|
||||||
|
this.logger.error('Readiness check failed', error);
|
||||||
throw new ServiceUnavailableException('Database not reachable');
|
throw new ServiceUnavailableException('Database not reachable');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,8 +4,22 @@ import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger';
|
|||||||
import { AppModule } from './app.module';
|
import { AppModule } from './app.module';
|
||||||
import { configureApp } from './app.config';
|
import { configureApp } from './app.config';
|
||||||
|
|
||||||
|
const REQUIRED_ENV_VARS = ['DATABASE_URL', 'JWT_ACCESS_SECRET'] as const;
|
||||||
|
|
||||||
|
function assertRequiredEnvVars(): void {
|
||||||
|
const missing = REQUIRED_ENV_VARS.filter((name) => !process.env[name]);
|
||||||
|
if (missing.length > 0) {
|
||||||
|
throw new Error(
|
||||||
|
`Missing required environment variable(s): ${missing.join(', ')}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function bootstrap() {
|
async function bootstrap() {
|
||||||
|
assertRequiredEnvVars();
|
||||||
|
|
||||||
const app = await NestFactory.create<NestExpressApplication>(AppModule);
|
const app = await NestFactory.create<NestExpressApplication>(AppModule);
|
||||||
|
app.enableShutdownHooks();
|
||||||
|
|
||||||
configureApp(app);
|
configureApp(app);
|
||||||
|
|
||||||
@@ -18,6 +32,6 @@ async function bootstrap() {
|
|||||||
const swaggerDocument = SwaggerModule.createDocument(app, swaggerConfig);
|
const swaggerDocument = SwaggerModule.createDocument(app, swaggerConfig);
|
||||||
SwaggerModule.setup('api/v1/docs', app, swaggerDocument);
|
SwaggerModule.setup('api/v1/docs', app, swaggerDocument);
|
||||||
|
|
||||||
await app.listen(process.env.PORT ?? 3000);
|
await app.listen(process.env.PORT ?? 3001);
|
||||||
}
|
}
|
||||||
void bootstrap();
|
void bootstrap();
|
||||||
@@ -2,6 +2,12 @@
|
|||||||
|
|
||||||
FROM node:22-alpine AS builder
|
FROM node:22-alpine AS builder
|
||||||
WORKDIR /app
|
WORKDIR /app
|
||||||
|
# NEXT_PUBLIC_* vars are inlined into the client bundle at build time, so
|
||||||
|
# this must be supplied per-environment via:
|
||||||
|
# docker build --build-arg NEXT_PUBLIC_API_URL=https://api.example.com .
|
||||||
|
# Setting it only in the runtime ConfigMap has no effect on the built bundle.
|
||||||
|
ARG NEXT_PUBLIC_API_URL
|
||||||
|
ENV NEXT_PUBLIC_API_URL=$NEXT_PUBLIC_API_URL
|
||||||
COPY package.json package-lock.json ./
|
COPY package.json package-lock.json ./
|
||||||
RUN npm ci
|
RUN npm ci
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ kind: Ingress
|
|||||||
metadata:
|
metadata:
|
||||||
name: apicore-backend
|
name: apicore-backend
|
||||||
annotations:
|
annotations:
|
||||||
nginx.ingress.kubernetes.io/rewrite-target: /$2
|
|
||||||
# Requires cert-manager with a ClusterIssuer named "letsencrypt-prod"
|
# Requires cert-manager with a ClusterIssuer named "letsencrypt-prod"
|
||||||
# (or swap for whatever issuer/cert mechanism the cluster already uses).
|
# (or swap for whatever issuer/cert mechanism the cluster already uses).
|
||||||
cert-manager.io/cluster-issuer: letsencrypt-prod
|
cert-manager.io/cluster-issuer: letsencrypt-prod
|
||||||
@@ -17,8 +16,8 @@ spec:
|
|||||||
- host: api.apicore.example.com
|
- host: api.apicore.example.com
|
||||||
http:
|
http:
|
||||||
paths:
|
paths:
|
||||||
- path: /()(.*)
|
- path: /
|
||||||
pathType: ImplementationSpecific
|
pathType: Prefix
|
||||||
backend:
|
backend:
|
||||||
service:
|
service:
|
||||||
name: apicore-backend
|
name: apicore-backend
|
||||||
|
|||||||
Reference in new issue
Block a user