diff --git a/lib/admin/__tests__/config-manager-reload.test.ts b/lib/admin/__tests__/config-manager-reload.test.ts new file mode 100644 index 000000000..ae8fcfe0a --- /dev/null +++ b/lib/admin/__tests__/config-manager-reload.test.ts @@ -0,0 +1,83 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { configManager } from '../config-manager'; + +// Several replicas share one config volume. An admin change saved by one has +// to reach the others without a restart. + +let dir: string; +let now: number; + +/** What another replica's save does: write a temp file, rename it over. */ +async function otherReplicaWrites(file: string, data: Record) { + const target = path.join(dir, file); + await writeFile(target + '.other', JSON.stringify(data), 'utf-8'); + await rename(target + '.other', target); +} + +beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), 'bw-config-reload-')); + vi.stubEnv('ADMIN_CONFIG_DIR', dir); + now = 1_000_000; + vi.spyOn(Date, 'now').mockImplementation(() => now); + await writeFile(path.join(dir, 'config.json'), JSON.stringify({ appName: 'First' }), 'utf-8'); + await writeFile(path.join(dir, 'policy.json'), JSON.stringify({ features: { calendarEnabled: true } }), 'utf-8'); + await configManager.load(); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + await rm(dir, { recursive: true, force: true }); +}); + +describe('config manager picks up changes on disk', () => { + it('reloads config.json changed by another replica after the check interval', async () => { + expect(configManager.get('appName')).toBe('First'); + await otherReplicaWrites('config.json', { appName: 'Second name' }); + + // Within the interval nothing is checked. + now += 1_000; + await configManager.ensureLoaded(); + expect(configManager.get('appName')).toBe('First'); + + now += 10_000; + await configManager.ensureLoaded(); + expect(configManager.get('appName')).toBe('Second name'); + }); + + it('reloads policy.json changed by another replica', async () => { + expect(configManager.getPolicy().features.calendarEnabled).toBe(true); + await otherReplicaWrites('policy.json', { features: { calendarEnabled: false } }); + + now += 10_000; + await configManager.ensureLoaded(); + expect(configManager.getPolicy().features.calendarEnabled).toBe(false); + }); + + it('a synchronous read starts the check and the next read sees the change', async () => { + await otherReplicaWrites('config.json', { appName: 'Second name' }); + now += 10_000; + configManager.get('appName'); + await vi.waitFor(() => expect(configManager.get('appName')).toBe('Second name')); + }); + + it('keeps the values in memory while the changed file is broken', async () => { + await writeFile(path.join(dir, 'config.json'), '{ not json', 'utf-8'); + vi.spyOn(console, 'error').mockImplementation(() => {}); + now += 10_000; + await configManager.ensureLoaded(); + expect(configManager.get('appName')).toBe('First'); + }); + + it("doesn't undo another replica's change when saving", async () => { + await otherReplicaWrites('config.json', { appName: 'Second name' }); + await configManager.setAdminConfig({ loginShowVersion: false }); + + const onDisk = JSON.parse(await readFile(path.join(dir, 'config.json'), 'utf-8')); + expect(onDisk).toEqual({ appName: 'Second name', loginShowVersion: false }); + }); +}); diff --git a/lib/admin/config-manager.ts b/lib/admin/config-manager.ts index 22c53f527..5f6697918 100644 --- a/lib/admin/config-manager.ts +++ b/lib/admin/config-manager.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile, rename } from 'node:fs/promises'; +import { readFile, writeFile, rename, stat } from 'node:fs/promises'; import { configureLogger, logger } from '@/lib/logger'; import { readFileEnv } from '@/lib/read-file-env'; import { CONFIG_ENV_MAP, DEFAULT_FEATURE_GATES, DEFAULT_POLICY, DEFAULT_THEME_POLICY, type SettingsPolicy } from './types'; @@ -28,23 +28,95 @@ function parseEnvValue(value: string, type: string): unknown { } } +/** The files load() reads; a change to either is picked up on the next check. */ +const WATCHED_FILES = ['config.json', 'policy.json'] as const; +type WatchedFile = typeof WATCHED_FILES[number]; + +/** + * How often, at most, the files' stat is compared with what was loaded. + * Replicas sharing one config volume see each other's admin changes within + * this time, without a restart. + */ +const CHANGE_CHECK_INTERVAL_MS = 5_000; + +/** Changes whenever the file is replaced or rewritten; null when it is missing. */ +async function fileStamp(filename: string): Promise { + try { + const s = await stat(getConfigPath(filename)); + return `${s.mtimeMs}:${s.size}:${s.ino}`; + } catch { + return null; + } +} + class ConfigManager { private adminConfig: Record = {}; private policyCache: SettingsPolicy = { ...DEFAULT_POLICY }; private loaded = false; + private fileStamps: Partial> = {}; + private lastChangeCheck = 0; + private changeCheck: Promise | null = null; /** Load admin config and policy from disk. Called once at startup and on reload. */ async load(): Promise { - this.adminConfig = await this.readJsonFile('config.json') || {}; - const policy = await this.readJsonFile('policy.json'); - if (policy) { - this.policyCache = ConfigManager.completePolicy(policy as Partial); - } else { - this.policyCache = { ...DEFAULT_POLICY }; - } + // Stamped before reading, so a write that lands mid-read is seen next time. + const stamps = await this.readStamps(); + this.apply(await this.readJsonFile('config.json'), await this.readJsonFile('policy.json')); + this.fileStamps = stamps; + this.lastChangeCheck = Date.now(); + logger.debug('ConfigManager loaded', { configKeys: Object.keys(this.adminConfig).length }); + } + + private apply(config: Record | null, policy: Record | null): void { + this.adminConfig = config || {}; + this.policyCache = policy + ? ConfigManager.completePolicy(policy as Partial) + : { ...DEFAULT_POLICY }; this.loaded = true; this.applyLoggerSettings(); - logger.debug('ConfigManager loaded', { configKeys: Object.keys(this.adminConfig).length }); + } + + private async readStamps(): Promise>> { + const stamps: Partial> = {}; + for (const file of WATCHED_FILES) stamps[file] = await fileStamp(file); + return stamps; + } + + /** + * Reload when config.json or policy.json changed on disk since the last + * load (another replica on the same volume saved, or someone edited the + * file). A file that can't be read or parsed keeps the values in memory + * and is tried again at the next check. + */ + private async reloadIfChanged(): Promise { + const stamps = await this.readStamps(); + if (WATCHED_FILES.every((file) => stamps[file] === this.fileStamps[file])) return; + try { + const [config, policy] = await Promise.all([ + this.readJsonFileOrThrow('config.json'), + this.readJsonFileOrThrow('policy.json'), + ]); + this.apply(config, policy); + this.fileStamps = stamps; + logger.info('Admin config changed on disk; reloaded'); + } catch (error) { + logger.warn('Admin config changed on disk but could not be read; keeping the current values', { + error: error instanceof Error ? error.message : 'Unknown error', + }); + } + } + + /** At most every CHANGE_CHECK_INTERVAL_MS: one stat per watched file. */ + private checkForChanges(): Promise { + if (!this.loaded) return Promise.resolve(); + if (this.changeCheck) return this.changeCheck; + const now = Date.now(); + if (now - this.lastChangeCheck < CHANGE_CHECK_INTERVAL_MS) return Promise.resolve(); + this.lastChangeCheck = now; + this.changeCheck = this.reloadIfChanged().finally(() => { + this.changeCheck = null; + }); + return this.changeCheck; } /** Hand the configured log level and format to the logger. */ @@ -55,15 +127,22 @@ class ConfigManager { }); } - /** Ensure config is loaded (no-op if already loaded). */ + /** + * Ensure config is loaded. Once loaded, waits for the periodic check for + * changes on disk when one is due (a stat per file, every few seconds). + */ async ensureLoaded(): Promise { if (!this.loaded) await this.load(); + else await this.checkForChanges(); } /** * Get a config value. Priority: admin override > env var > default. */ get(key: string, defaultValue?: T): T { + // Synchronous callers can't wait: a due check runs in the background and + // the next read sees its result. + void this.checkForChanges(); // Admin override (highest priority) const mapping = CONFIG_ENV_MAP[key]; if (key in this.adminConfig) { @@ -109,6 +188,7 @@ class ConfigManager { * Get all config values with source information (for admin UI). */ getAllWithSources(): Record { + void this.checkForChanges(); const result: Record = {}; for (const [key, mapping] of Object.entries(CONFIG_ENV_MAP)) { if (key in this.adminConfig) { @@ -145,6 +225,9 @@ class ConfigManager { */ async setAdminConfig(updates: Record): Promise { assertWritable('update admin config'); + // Start from what is on disk, so a save here doesn't undo a change another + // replica saved a moment ago. Same before every write below. + await this.reloadIfChanged(); Object.assign(this.adminConfig, updates); await this.writeJsonFile('config.json', this.adminConfig); this.applyLoggerSettings(); @@ -155,6 +238,7 @@ class ConfigManager { */ async removeAdminOverride(key: string): Promise { assertWritable('remove admin override'); + await this.reloadIfChanged(); delete this.adminConfig[key]; await this.writeJsonFile('config.json', this.adminConfig); this.applyLoggerSettings(); @@ -165,6 +249,7 @@ class ConfigManager { * /setup routes and the rest of the app. */ isSetupComplete(): boolean { + void this.checkForChanges(); return this.adminConfig.setupComplete === true; } @@ -174,6 +259,7 @@ class ConfigManager { */ async markSetupComplete(): Promise { assertWritable('mark setup complete'); + await this.reloadIfChanged(); this.adminConfig.setupComplete = true; await this.writeJsonFile('config.json', this.adminConfig); } @@ -182,6 +268,7 @@ class ConfigManager { * Get the current settings policy. */ getPolicy(): SettingsPolicy { + void this.checkForChanges(); return this.policyCache; } @@ -259,31 +346,41 @@ class ConfigManager { } /** - * Reload config from disk (for manual file edits or multi-instance). + * Reload config from disk now. Changes on disk are also picked up on their + * own within CHANGE_CHECK_INTERVAL_MS (see checkForChanges). */ async reload(): Promise { await this.load(); } private async readJsonFile(filename: string): Promise | null> { - const filePath = getConfigPath(filename); try { - const raw = await readFile(filePath, 'utf-8'); - return JSON.parse(raw); + return await this.readJsonFileOrThrow(filename); } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; logger.warn(`Failed to read ${filename}`, { error: error instanceof Error ? error.message : 'Unknown error' }); return null; } } - private async writeJsonFile(filename: string, data: Record): Promise { + /** null when the file doesn't exist; throws when it can't be read or parsed. */ + private async readJsonFileOrThrow(filename: string): Promise | null> { + try { + return JSON.parse(await readFile(getConfigPath(filename), 'utf-8')); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + throw error; + } + } + + private async writeJsonFile(filename: WatchedFile, data: Record): Promise { await ensureConfigDir(); const targetPath = getConfigPath(filename); const tmpPath = targetPath + '.tmp'; // Owner-only: config.json carries the session secret and OAuth client secrets. await writeFile(tmpPath, JSON.stringify(data, null, 2), { encoding: 'utf-8', mode: 0o600 }); await rename(tmpPath, targetPath); + // Our own write is not a change to pick up. + this.fileStamps[filename] = await fileStamp(filename); } }