From 71068a7bcda350510eb7423ba62149ee35ceb2e5 Mon Sep 17 00:00:00 2001 From: Alex Drankou Date: Thu, 14 May 2026 15:57:26 +0200 Subject: [PATCH] refactor: make kv redis optional --- .env.example | 10 +- README.md | 37 ++--- scripts/check-app-env.ts | 7 + src/app/api/health/route.ts | 88 +++++++---- .../server/functions/auth/validate-email.ts | 46 +++++- src/core/shared/clients/kv.ts | 84 ++++++++++- src/lib/env.ts | 4 +- tests/integration/health-route.test.ts | 121 +++++++++++++++ tests/integration/inspect-sandbox.test.ts | 20 ++- tests/unit/kv.test.ts | 53 +++++++ tests/unit/validate-email.test.ts | 141 ++++++++++++++++++ 11 files changed, 547 insertions(+), 64 deletions(-) create mode 100644 tests/integration/health-route.test.ts create mode 100644 tests/unit/kv.test.ts create mode 100644 tests/unit/validate-email.test.ts diff --git a/.env.example b/.env.example index 433bc4109..cacadb567 100644 --- a/.env.example +++ b/.env.example @@ -9,10 +9,6 @@ SUPABASE_SERVICE_ROLE_KEY=your_supabase_service_role_key # Resolves Infrastructure and Dashboard API + E2B SDK configuration NEXT_PUBLIC_E2B_DOMAIN=e2b.dev -### KV database configuration -KV_REST_API_TOKEN= -KV_REST_API_URL= - ### ================================= ### REQUIRED CLIENT ENVIRONMENT VARIABLES ### ================================= @@ -37,6 +33,12 @@ NEXT_PUBLIC_SUPABASE_ANON_KEY=your_supabase_anon_key ### ZeroBounce API key for email validation # ZEROBOUNCE_API_KEY= +### Optional KV database configuration +### Required only for KV-backed capabilities such as alternate-email warning dedupe. +### Must be Vercel/Upstash Redis REST compatible; raw redis://localhost:6379 is not supported by @vercel/kv. +# KV_REST_API_TOKEN= +# KV_REST_API_URL= + ### Plain API key for customer support integration # (Required if NEXT_PUBLIC_INCLUDE_REPORT_ISSUE=1) # PLAIN_API_KEY= diff --git a/README.md b/README.md index 5abfe3acb..18189ae6a 100644 --- a/README.md +++ b/README.md @@ -63,21 +63,7 @@ cp .env.example .env.local 4. Set up required services: -#### a. Key-Value Store Setup -This project requires a Redis-compatible key-value store. You'll need to: - -1. Set up a Redis instance (self-hosted or using a cloud provider) -2. Configure the following environment variables in your `.env.local` file: - ``` - KV_URL=your_redis_connection_string - KV_REST_API_URL=your_redis_rest_api_url - KV_REST_API_TOKEN=your_redis_api_write_token - KV_REST_API_READ_ONLY_TOKEN=your_redis_api_read_token - ``` - -> **Note**: For production deployments, we use Vercel KV Storage integration, which provides a managed Redis-compatible store and automatically configures these environment variables. You can add this integration through the Vercel dashboard when deploying your project. - -#### b. Supabase Setup +#### a. Supabase Setup 1. Create a new Supabase project 2. Go to Project Settings > API 3. Copy the `anon key` & `service_role key` to populate `.env.local` @@ -103,18 +89,33 @@ This project requires a Redis-compatible key-value store. You'll need to: {{ .SiteURL }}/api/auth/confirm?token_hash={{ .TokenHash }}&type=email&next={{ .RedirectTo }}&confirmation_url={{ .ConfirmationURL }} ``` -#### c. Database Setup +#### b. Database Setup 1. Apply the database migrations manually: - Navigate to the `/migrations` folder in the project - Execute each SQL migration file in sequential order against your Supabase database - You can run these migrations using the Supabase SQL Editor or a PostgreSQL client - Make sure to apply migrations in the correct order based on their timestamp prefixes -#### d. Supabase Storage Setup +#### c. Supabase Storage Setup 1. Go to Storage > Buckets 2. Create a new **public** bucket named `profile-pictures` -#### e. Start the development server +5. Optional services: + +#### a. Key-Value Store Setup +Redis/KV is optional for standard dashboard deployments, including local, enterprise, and on-prem environments. The dashboard can boot and run core auth and dashboard workflows without KV configured. + +KV is currently used for optional capability checks and for deduplicating ZeroBounce alternate-email warnings. If you need those capabilities, configure a Vercel/Upstash Redis REST-compatible store: + ``` + KV_REST_API_URL=your_redis_rest_api_url + KV_REST_API_TOKEN=your_redis_api_write_token + ``` + +> **Note**: `@vercel/kv` expects a Redis REST API. A raw Redis server such as `redis://localhost:6379` is not compatible without an Upstash-compatible REST proxy. + +> **Health check**: When `KV_REST_API_URL` and `KV_REST_API_TOKEN` are set, `/api/health` will report `503 degraded` if KV is unreachable. Leave both unset to opt out of the KV health check entirely. + +6. Start the development server: ```bash # Using Bun (recommended) bun run dev diff --git a/scripts/check-app-env.ts b/scripts/check-app-env.ts index 88871a756..367bdd401 100644 --- a/scripts/check-app-env.ts +++ b/scripts/check-app-env.ts @@ -6,6 +6,13 @@ loadEnvConfig(projectDir) const schema = serverSchema .merge(clientSchema) + .refine( + (data) => Boolean(data.KV_REST_API_URL) === Boolean(data.KV_REST_API_TOKEN), + { + message: 'KV_REST_API_URL and KV_REST_API_TOKEN must be set together', + path: ['KV_REST_API_URL'], + } + ) .refine( (data) => { if (data.NEXT_PUBLIC_INCLUDE_BILLING === '1') { diff --git a/src/app/api/health/route.ts b/src/app/api/health/route.ts index 8fc3aea0e..b1d882c43 100644 --- a/src/app/api/health/route.ts +++ b/src/app/api/health/route.ts @@ -1,42 +1,24 @@ import { NextResponse } from 'next/server' import { api } from '@/core/shared/clients/api' -import { kv } from '@/core/shared/clients/kv' +import { pingKv } from '@/core/shared/clients/kv' import { l, serializeErrorForLog } from '@/core/shared/clients/logger/logger' export const maxDuration = 10 -export async function GET() { - const checks = { - kv: false, - dashboardApi: false, - } - +async function checkDashboardApi(): Promise { try { - await kv.ping() - checks.kv = true - } catch (error) { + const { error } = await api.GET('/health', {}) + if (!error) { + return true + } + l.error( { - key: 'health_check:kv_error', - error: serializeErrorForLog(error), + key: 'health_check:dashboard_api_error', + error, }, - 'KV health check failed' + 'Dashboard API health check failed' ) - } - - try { - const { error } = await api.GET('/health', {}) - if (!error) { - checks.dashboardApi = true - } else { - l.error( - { - key: 'health_check:dashboard_api_error', - error, - }, - 'Dashboard API health check failed' - ) - } } catch (error) { l.error( { @@ -47,15 +29,59 @@ export async function GET() { ) } - const allHealthy = checks.kv && checks.dashboardApi + return false +} + +export async function GET() { + const [kvStatus, dashboardApi] = await Promise.all([ + pingKv(), + checkDashboardApi(), + ]) + + const checks: { kv?: boolean; dashboardApi: boolean } = { dashboardApi } + + if (kvStatus.configured) { + checks.kv = kvStatus.available + } + + if (kvStatus.status === 'misconfigured') { + // Surface misconfiguration in the response body so it's visible + // without scraping logs. + checks.kv = false + l.error( + { + key: 'health_check:kv_misconfigured', + }, + 'KV health check is misconfigured' + ) + } + + if (kvStatus.status === 'error') { + l.error( + { + key: 'health_check:kv_error', + error: serializeErrorForLog(kvStatus.error), + }, + 'KV health check failed' + ) + } + + // KV is required *only when configured*. If an operator has wired it up, + // they expect it to work — so failure or misconfiguration must degrade + // the overall status. When KV is intentionally absent (not_configured), + // it contributes nothing to the health check. + const kvRequiredAndHealthy = + kvStatus.status === 'not_configured' || kvStatus.status === 'ok' + + const allRequiredHealthy = dashboardApi && kvRequiredAndHealthy return NextResponse.json( { - status: allHealthy ? 'ok' : 'degraded', + status: allRequiredHealthy ? 'ok' : 'degraded', checks, }, { - status: allHealthy ? 200 : 503, + status: allRequiredHealthy ? 200 : 503, headers: { // vercel infra respects this to cache on cdn 'Cache-Control': 'public, max-age=30, must-revalidate', diff --git a/src/core/server/functions/auth/validate-email.ts b/src/core/server/functions/auth/validate-email.ts index eb27bb83e..bfe459a5b 100644 --- a/src/core/server/functions/auth/validate-email.ts +++ b/src/core/server/functions/auth/validate-email.ts @@ -1,5 +1,5 @@ -import { kv } from '@vercel/kv' import { KV_KEYS } from '@/configs/keys' +import { getKvValue, setKvValue } from '@/core/shared/clients/kv' import { l, serializeErrorForLog } from '@/core/shared/clients/logger/logger' /** @@ -102,16 +102,54 @@ export const shouldWarnAboutAlternateEmail = async ( validationResult: EmailValidationResponse ): Promise => { if (validationResult.sub_status === 'alternate') { - const warnedAlternateEmail = await kv.get( + const warnedAlternateEmail = await getKvValue( KV_KEYS.WARNED_ALTERNATE_EMAIL(validationResult.address) ) - if (!warnedAlternateEmail) { - await kv.set( + if (!warnedAlternateEmail.ok) { + l.warn( + { + key: 'validate_email:alternate_email_kv_unavailable', + error: + warnedAlternateEmail.reason === 'error' + ? serializeErrorForLog(warnedAlternateEmail.error) + : undefined, + context: { + email: validationResult.address, + reason: warnedAlternateEmail.reason, + }, + }, + 'Skipping alternate email warning because KV is unavailable' + ) + + return false + } + + if (!warnedAlternateEmail.value) { + const setResult = await setKvValue( KV_KEYS.WARNED_ALTERNATE_EMAIL(validationResult.address), true ) + if (!setResult.ok) { + l.warn( + { + key: 'validate_email:alternate_email_kv_set_unavailable', + error: + setResult.reason === 'error' + ? serializeErrorForLog(setResult.error) + : undefined, + context: { + email: validationResult.address, + reason: setResult.reason, + }, + }, + 'Skipping alternate email warning because KV could not persist state' + ) + + return false + } + return true } } diff --git a/src/core/shared/clients/kv.ts b/src/core/shared/clients/kv.ts index 7744ec452..a15cacb10 100644 --- a/src/core/shared/clients/kv.ts +++ b/src/core/shared/clients/kv.ts @@ -1 +1,83 @@ -export { kv } from '@vercel/kv' +import { kv } from '@vercel/kv' + +export type OptionalKvResult = + | { ok: true; configured: true; value: T } + | { + ok: false + configured: false + reason: 'not_configured' | 'misconfigured' + } + | { ok: false; configured: true; reason: 'error'; error: unknown } + +export type KvCapabilityStatus = + | { configured: false; available: false; status: 'not_configured' } + | { configured: false; available: false; status: 'misconfigured' } + | { configured: true; available: true; status: 'ok' } + | { configured: true; available: false; status: 'error'; error: unknown } + +function getKvConfigStatus() { + const hasUrl = Boolean(process.env.KV_REST_API_URL) + const hasToken = Boolean(process.env.KV_REST_API_TOKEN) + + if (!(hasUrl || hasToken)) { + return 'not_configured' + } + + if (hasUrl && hasToken) { + return 'configured' + } + + return 'misconfigured' +} + +export function isKvConfigured() { + return getKvConfigStatus() === 'configured' +} + +export async function pingKv(): Promise { + const configStatus = getKvConfigStatus() + + if (configStatus !== 'configured') { + return { configured: false, available: false, status: configStatus } + } + + try { + await kv.ping() + return { configured: true, available: true, status: 'ok' } + } catch (error) { + return { configured: true, available: false, status: 'error', error } + } +} + +export async function getKvValue( + key: string +): Promise> { + const configStatus = getKvConfigStatus() + + if (configStatus !== 'configured') { + return { ok: false, configured: false, reason: configStatus } + } + + try { + return { ok: true, configured: true, value: await kv.get(key) } + } catch (error) { + return { ok: false, configured: true, reason: 'error', error } + } +} + +export async function setKvValue( + key: string, + value: unknown +): Promise> { + const configStatus = getKvConfigStatus() + + if (configStatus !== 'configured') { + return { ok: false, configured: false, reason: configStatus } + } + + try { + return { ok: true, configured: true, value: await kv.set(key, value) } + } catch (error) { + return { ok: false, configured: true, reason: 'error', error } + } +} diff --git a/src/lib/env.ts b/src/lib/env.ts index 75f054c65..ab016be32 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -2,8 +2,8 @@ import { z } from 'zod' export const serverSchema = z.object({ SUPABASE_SERVICE_ROLE_KEY: z.string().min(1), - KV_REST_API_TOKEN: z.string().min(1), - KV_REST_API_URL: z.url(), + KV_REST_API_TOKEN: z.string().min(1).optional(), + KV_REST_API_URL: z.url().optional(), ENABLE_USER_BOOTSTRAP: z.string().optional(), DASHBOARD_API_ADMIN_TOKEN: z.string().min(1).optional(), diff --git a/tests/integration/health-route.test.ts b/tests/integration/health-route.test.ts new file mode 100644 index 000000000..4fa78e214 --- /dev/null +++ b/tests/integration/health-route.test.ts @@ -0,0 +1,121 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { apiGet, pingKv } = vi.hoisted(() => ({ + apiGet: vi.fn(), + pingKv: vi.fn(), +})) + +vi.mock('@/core/shared/clients/api', () => ({ + api: { + GET: apiGet, + }, +})) + +vi.mock('@/core/shared/clients/kv', () => ({ + pingKv, +})) + +vi.mock('@/core/shared/clients/logger/logger', () => ({ + l: { + error: vi.fn(), + }, + serializeErrorForLog: vi.fn((error) => error), +})) + +import { GET } from '@/app/api/health/route' + +describe('/api/health', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('omits checks.kv when KV is not configured', async () => { + pingKv.mockResolvedValue({ + configured: false, + available: false, + status: 'not_configured', + }) + apiGet.mockResolvedValue({ error: null }) + + const response = await GET() + const body = await response.json() + + expect(response.status).toBe(200) + expect(body).toEqual({ + status: 'ok', + checks: { dashboardApi: true }, + }) + expect(body.checks).not.toHaveProperty('kv') + }) + + it('returns degraded when KV is configured but ping fails', async () => { + pingKv.mockResolvedValue({ + configured: true, + available: false, + status: 'error', + error: new Error('kv unavailable'), + }) + apiGet.mockResolvedValue({ error: null }) + + const response = await GET() + const body = await response.json() + + expect(response.status).toBe(503) + expect(body).toEqual({ + status: 'degraded', + checks: { kv: false, dashboardApi: true }, + }) + }) + + it('returns degraded when KV env is misconfigured', async () => { + pingKv.mockResolvedValue({ + configured: false, + available: false, + status: 'misconfigured', + }) + apiGet.mockResolvedValue({ error: null }) + + const response = await GET() + const body = await response.json() + + expect(response.status).toBe(503) + expect(body).toEqual({ + status: 'degraded', + checks: { kv: false, dashboardApi: true }, + }) + }) + + it('includes checks.kv === true when KV is configured and healthy', async () => { + pingKv.mockResolvedValue({ + configured: true, + available: true, + status: 'ok', + }) + apiGet.mockResolvedValue({ error: null }) + + const response = await GET() + const body = await response.json() + + expect(response.status).toBe(200) + expect(body).toEqual({ + status: 'ok', + checks: { kv: true, dashboardApi: true }, + }) + }) + + it('returns degraded when required dashboard-api is unhealthy', async () => { + pingKv.mockResolvedValue({ + configured: true, + available: true, + status: 'ok', + }) + apiGet.mockResolvedValue({ error: { message: 'downstream unavailable' } }) + + const response = await GET() + const body = await response.json() + + expect(response.status).toBe(503) + expect(body.status).toBe('degraded') + expect(body.checks).toEqual({ kv: true, dashboardApi: false }) + }) +}) diff --git a/tests/integration/inspect-sandbox.test.ts b/tests/integration/inspect-sandbox.test.ts index 316df4547..25a12902d 100644 --- a/tests/integration/inspect-sandbox.test.ts +++ b/tests/integration/inspect-sandbox.test.ts @@ -35,10 +35,22 @@ vi.mock('@/core/modules/teams/user-teams-repository.server', () => ({ })) vi.mock('@/core/shared/clients/kv', () => ({ - kv: { - get: vi.fn(), - set: vi.fn(), - }, + isKvConfigured: vi.fn(() => false), + pingKv: vi.fn(async () => ({ + configured: false, + available: false, + status: 'not_configured' as const, + })), + getKvValue: vi.fn(async () => ({ + ok: false, + configured: false, + reason: 'not_configured' as const, + })), + setKvValue: vi.fn(async () => ({ + ok: false, + configured: false, + reason: 'not_configured' as const, + })), })) vi.mock('@/core/shared/clients/api', () => ({ diff --git a/tests/unit/kv.test.ts b/tests/unit/kv.test.ts new file mode 100644 index 000000000..47abfb881 --- /dev/null +++ b/tests/unit/kv.test.ts @@ -0,0 +1,53 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { ping } = vi.hoisted(() => ({ + ping: vi.fn(), +})) + +vi.mock('@vercel/kv', () => ({ + kv: { + ping, + }, +})) + +import { pingKv } from '@/core/shared/clients/kv' + +const originalKvEnv = { + KV_REST_API_TOKEN: process.env.KV_REST_API_TOKEN, + KV_REST_API_URL: process.env.KV_REST_API_URL, +} + +function resetKvEnv() { + if (originalKvEnv.KV_REST_API_TOKEN === undefined) { + delete process.env.KV_REST_API_TOKEN + } else { + process.env.KV_REST_API_TOKEN = originalKvEnv.KV_REST_API_TOKEN + } + + if (originalKvEnv.KV_REST_API_URL === undefined) { + delete process.env.KV_REST_API_URL + } else { + process.env.KV_REST_API_URL = originalKvEnv.KV_REST_API_URL + } +} + +describe('optional KV client', () => { + beforeEach(() => { + vi.clearAllMocks() + delete process.env.KV_REST_API_TOKEN + delete process.env.KV_REST_API_URL + }) + + afterEach(() => { + resetKvEnv() + }) + + it('reports KV as not configured when both env values are omitted', async () => { + await expect(pingKv()).resolves.toEqual({ + configured: false, + available: false, + status: 'not_configured', + }) + expect(ping).not.toHaveBeenCalled() + }) +}) diff --git a/tests/unit/validate-email.test.ts b/tests/unit/validate-email.test.ts new file mode 100644 index 000000000..39b60270e --- /dev/null +++ b/tests/unit/validate-email.test.ts @@ -0,0 +1,141 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { KV_KEYS } from '@/configs/keys' +import { shouldWarnAboutAlternateEmail } from '@/core/server/functions/auth/validate-email' + +const { getKvValue, setKvValue, warn } = vi.hoisted(() => ({ + getKvValue: vi.fn(), + setKvValue: vi.fn(), + warn: vi.fn(), +})) + +vi.mock('@/core/shared/clients/kv', () => ({ + getKvValue, + setKvValue, +})) + +vi.mock('@/core/shared/clients/logger/logger', () => ({ + l: { + warn, + error: vi.fn(), + }, + serializeErrorForLog: vi.fn((error) => error), +})) + +const alternateEmailValidation = { + address: 'user@example.com', + status: 'valid', + sub_status: 'alternate', + free_email: false, + account: 'user', + domain: 'example.com', + mx_found: true, + did_you_mean: null, + domain_age_days: null, + active_in_days: null, + smtp_provider: null, + mx_record: null, + firstname: null, + lastname: null, + gender: null, + country: null, + region: null, + city: null, + zipcode: null, + processed_at: '2026-05-13', +} + +describe('shouldWarnAboutAlternateEmail', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('keeps warn-once behavior when KV is available', async () => { + getKvValue.mockResolvedValue({ + ok: true, + configured: true, + value: null, + }) + setKvValue.mockResolvedValue({ + ok: true, + configured: true, + value: 'OK', + }) + + await expect( + shouldWarnAboutAlternateEmail(alternateEmailValidation) + ).resolves.toBe(true) + + expect(getKvValue).toHaveBeenCalledWith( + KV_KEYS.WARNED_ALTERNATE_EMAIL('user@example.com') + ) + expect(setKvValue).toHaveBeenCalledWith( + KV_KEYS.WARNED_ALTERNATE_EMAIL('user@example.com'), + true + ) + }) + + it('does not warn again when KV has already recorded the email', async () => { + getKvValue.mockResolvedValue({ + ok: true, + configured: true, + value: true, + }) + + await expect( + shouldWarnAboutAlternateEmail(alternateEmailValidation) + ).resolves.toBe(false) + + expect(setKvValue).not.toHaveBeenCalled() + }) + + it('allows sign-up flow to continue when KV is not configured', async () => { + getKvValue.mockResolvedValue({ + ok: false, + configured: false, + reason: 'not_configured', + }) + + await expect( + shouldWarnAboutAlternateEmail(alternateEmailValidation) + ).resolves.toBe(false) + + expect(warn).toHaveBeenCalled() + expect(setKvValue).not.toHaveBeenCalled() + }) + + it('allows sign-up flow to continue when KV read fails', async () => { + getKvValue.mockResolvedValue({ + ok: false, + configured: true, + reason: 'error', + error: new Error('kv unavailable'), + }) + + await expect( + shouldWarnAboutAlternateEmail(alternateEmailValidation) + ).resolves.toBe(false) + + expect(warn).toHaveBeenCalled() + expect(setKvValue).not.toHaveBeenCalled() + }) + + it('allows sign-up flow to continue when KV cannot persist warning state', async () => { + getKvValue.mockResolvedValue({ + ok: true, + configured: true, + value: null, + }) + setKvValue.mockResolvedValue({ + ok: false, + configured: true, + reason: 'error', + error: new Error('kv unavailable'), + }) + + await expect( + shouldWarnAboutAlternateEmail(alternateEmailValidation) + ).resolves.toBe(false) + + expect(warn).toHaveBeenCalled() + }) +})