Compare commits
2
Commits
f9eaa324dd
...
e8b9dd5b91
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e8b9dd5b91 | ||
|
|
baba762dab |
+8
-1
@@ -389,5 +389,12 @@ Tickets DC-046 through DC-049 implement pluggable auth + email magic link. Sami
|
||||
- **details:** Backend uses ad-hoc `if (!field) throw new ValidationError(...)` checks at every route entry point — 49 such checks across the codebase. They drift from the field semantics, allow unknown keys to flow through, and have no way to express structured types (CIDR, enum, port range). Tracked in `DC-PRODUCTION-GRADE-BACKLOG.md` as P1-1. Fix: `npm install joi@^18`, add `src/utilities/validate.js` exporting `validateBody(schema)` middleware factory + `schemas` object with reusable schemas. Apply to destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Add `__tests__/unit/validate.test.js` covering each schema's accept/reject/strip-unknown behaviour. Effort: ~2 hr.
|
||||
- **impact:** Closes P0-3 / P0-4 class of bugs at the schema layer instead of per-route. Prevents future routes from accepting arbitrary body fields. New routes copy-paste from `schemas.*` and get free validation.
|
||||
- **prerequisite:** None.
|
||||
- **result:** Shipped codex-graded B. New module `src/utilities/validate.js` (170 LOC) with `validateBody(schema, opts)` middleware + 9 Joi schemas. Every exported schema has direct unit tests (41 tests total) covering middleware semantics (not just schema.validate). Applied to 8 destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Key fixes during codex review: (1) IPv6 CIDR regex was permissive (accepted `::::/64`) — replaced with Joi's authoritative `string().ip({cidr: 'required'})`. (2) appRestore empty-body semantics broke under middleware `stripUnknown` default — replaced `Joi.object({}).max(0)` with `Joi.any().custom()` that enforces non-empty rejection even after strip. (3) appDeploy.config now uses `.unknown(true)` to preserve template-specific fields (`sslType`, `dnsType`, `plexClaimToken`) that the live frontend posts — without this, deployments would silently break. Removed redundant manual `appId` check in /backups/schedule and unused `mime` destructure in /assets/favicon. Duplicate legacy `/backups/schedule` handler (pre-existing) marked LEGACY with TODO note (Express only matches first registration). 1539/1539 Jest tests pass (was 1498, +41 new). ESLint warnings unchanged (416 total, all pre-existing).
|
||||
- **result:** Shipped codex-graded B. New module `src/utilities/validate.js` (170 LOC) with `validateBody(schema, opts)` middleware + 9 Joi schemas. Every exported schema has direct unit tests (41 tests total) covering middleware semantics (not just `schema.validate`). Applied to 8 destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Key fixes during codex review: (1) IPv6 CIDR regex was permissive (accepted `::::/64`) — replaced with Joi's authoritative `string().ip({cidr: 'required'})`. (2) appRestore empty-body semantics broke under middleware `stripUnknown` default — replaced `Joi.object({}).max(0)` with `Joi.any().custom()` that enforces non-empty rejection even after strip. (3) appDeploy.config now uses `.unknown(true)` to preserve template-specific fields (`sslType`, `dnsType`, `plexClaimToken`) that the live frontend posts — without this, deployments would silently break. Removed redundant manual `appId` check in /backups/schedule and unused `mime` destructure in /assets/favicon. Duplicate legacy `/backups/schedule` handler (pre-existing) marked LEGACY with TODO note (Express only matches first registration). 1539/1539 Jest tests pass (was 1498, +41 new). ESLint warnings unchanged (416 total, all pre-existing).
|
||||
|
||||
### DC-060: Console→logger sweep for `src/managers/update-manager.js` (49 sites)
|
||||
- **status:** in-progress
|
||||
- **owner:** hermes
|
||||
- **details:** Production code uses `console.log/warn/error` with `[UpdateManager]` prefixes in 49 places — these go to stdout/stderr directly, bypassing the unified logger (no structured JSON, no error.log file writes, no log-level filtering, no test capture). Tracked in `DC-PRODUCTION-GRADE-BACKLOG.md` as P1-2. Fix: import `log` from `../utils/logging`, replace every `console.log('[UpdateManager] X')` with `log.info('update', 'X')` (dropping the redundant `[UpdateManager]` tag), every `console.warn(...)` with `log.warn('update', ...)`, every `console.error('...', err.message)` with `log.error('update', err)` (passing the error object so it lands in error.log with stack + context). For mixed-content strings like `Stored old image digest: ${oldImageDigest.substring(0, 40)}...` extract the variable into the meta payload: `log.info('update', 'Stored old image digest', { digestPrefix })`. Effort: ~30 min. Risk: very low — pure logging refactor, no behavior change.
|
||||
- **impact:** Update manager events now flow through the same log pipeline as every other module: structured JSON in prod, pretty-printed in dev, error.log rotation for errors, log-level filtering, test capture via stderr spy. Operators get consistent log format and can grep across modules.
|
||||
- **prerequisite:** None.
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ const fs = require('fs');
|
||||
const path = require('path');
|
||||
const https = require('https');
|
||||
const platformPaths = require('../../platform-paths');
|
||||
const { log } = require('../utils/logging');
|
||||
|
||||
const docker = new Docker();
|
||||
|
||||
@@ -33,7 +34,7 @@ class UpdateManager extends EventEmitter {
|
||||
start() {
|
||||
if (this.checking) return;
|
||||
|
||||
console.log('[UpdateManager] Starting update checks');
|
||||
log.info('update', 'Starting update checks');
|
||||
this.checking = true;
|
||||
|
||||
// Initial check
|
||||
@@ -52,7 +53,7 @@ class UpdateManager extends EventEmitter {
|
||||
stop() {
|
||||
if (!this.checking) return;
|
||||
|
||||
console.log('[UpdateManager] Stopping update checks');
|
||||
log.info('update', 'Stopping update checks');
|
||||
this.checking = false;
|
||||
|
||||
if (this.checkInterval) {
|
||||
@@ -70,22 +71,22 @@ class UpdateManager extends EventEmitter {
|
||||
*/
|
||||
triggerWorkflows(eventType, eventData) {
|
||||
if (!this.workflowEngine) {
|
||||
console.log('[UpdateManager] Workflow engine not set, skipping workflow trigger');
|
||||
log.info('update', 'Workflow engine not set, skipping workflow trigger');
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
try {
|
||||
this.workflowEngine.triggerForEvent(eventType, eventData)
|
||||
.then(results => {
|
||||
if (results && results.length > 0) {
|
||||
console.log(`[UpdateManager] Triggered ${results.length} workflow(s) for ${eventType}`);
|
||||
log.info('update', `Triggered workflows for ${eventType}`, { count: results.length });
|
||||
}
|
||||
})
|
||||
.catch(err => {
|
||||
console.error('[UpdateManager] Workflow trigger error:', err.message);
|
||||
log.error('update', err);
|
||||
});
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error triggering workflows:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,7 +95,7 @@ class UpdateManager extends EventEmitter {
|
||||
*/
|
||||
setWorkflowEngine(workflowEngine) {
|
||||
this.workflowEngine = workflowEngine;
|
||||
console.log('[UpdateManager] Workflow engine configured');
|
||||
log.info('update', 'Workflow engine configured');
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -131,13 +132,13 @@ class UpdateManager extends EventEmitter {
|
||||
this.availableUpdates.delete(containerInfo.Id);
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(`[UpdateManager] Error checking ${containerInfo.Names[0]}:`, error.message);
|
||||
log.error('update', error, null, { containerName: containerInfo.Names[0] });
|
||||
}
|
||||
}
|
||||
|
||||
console.log(`[UpdateManager] Found ${this.availableUpdates.size} updates available`);
|
||||
|
||||
log.info('update', 'Checked for updates', { availableCount: this.availableUpdates.size });
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error checking for updates:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -168,10 +169,10 @@ class UpdateManager extends EventEmitter {
|
||||
}
|
||||
|
||||
// gcr.io / quay.io / registry.gitlab.com — currently unsupported
|
||||
console.warn(`[UpdateManager] Custom registry not yet supported: ${remainder}`);
|
||||
log.warn('update', 'Custom registry not yet supported', { remainder });
|
||||
return null;
|
||||
} catch (error) {
|
||||
console.error(`[UpdateManager] Error getting digest for ${imageName}:`, error.message);
|
||||
log.error('update', error, null, { imageName });
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -338,7 +339,7 @@ class UpdateManager extends EventEmitter {
|
||||
async updateContainer(containerId, options = {}) {
|
||||
const startTime = Date.now();
|
||||
|
||||
console.log(`[UpdateManager] Starting update for container ${containerId}`);
|
||||
log.info('update', 'Starting update for container', { containerId });
|
||||
this.emit('update-start', { containerId, timestamp: new Date().toISOString() });
|
||||
|
||||
try {
|
||||
@@ -355,9 +356,9 @@ class UpdateManager extends EventEmitter {
|
||||
const oldImage = docker.getImage(oldImageId);
|
||||
const oldImageInspect = await oldImage.inspect();
|
||||
oldImageDigest = oldImageInspect.RepoDigests?.[0] || oldImageId;
|
||||
console.log(`[UpdateManager] Stored old image digest: ${oldImageDigest.substring(0, 40)}...`);
|
||||
log.info('update', 'Stored old image digest', { digestPrefix: oldImageDigest.substring(0, 40) });
|
||||
} catch (error) {
|
||||
console.warn(`[UpdateManager] Could not get old image digest: ${error.message}`);
|
||||
log.warn('update', 'Could not get old image digest', { error: error.message });
|
||||
}
|
||||
|
||||
// Create backup of current state
|
||||
@@ -375,24 +376,24 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
// Emit pre-update event for bundled workflows (e.g., backup-before-update)
|
||||
this.emit('pre-update', { containerId, containerName, imageName, backup });
|
||||
|
||||
|
||||
// Also trigger workflows for pre-update event directly
|
||||
this.triggerWorkflows('pre-update', { containerId, containerName, appId: containerName, imageName });
|
||||
|
||||
// Pull latest image
|
||||
console.log(`[UpdateManager] Pulling latest image: ${imageName}`);
|
||||
log.info('update', 'Pulling latest image', { imageName });
|
||||
await this.pullImage(imageName);
|
||||
|
||||
// Stop container
|
||||
console.log(`[UpdateManager] Stopping container: ${containerName}`);
|
||||
log.info('update', 'Stopping container', { containerName });
|
||||
await container.stop();
|
||||
|
||||
// Remove old container
|
||||
console.log(`[UpdateManager] Removing old container: ${containerName}`);
|
||||
log.info('update', 'Removing old container', { containerName });
|
||||
await container.remove();
|
||||
|
||||
// Create new container with same configuration
|
||||
console.log(`[UpdateManager] Creating new container: ${containerName}`);
|
||||
log.info('update', 'Creating new container', { containerName });
|
||||
const newContainer = await docker.createContainer({
|
||||
name: containerName,
|
||||
Image: imageName,
|
||||
@@ -401,11 +402,11 @@ class UpdateManager extends EventEmitter {
|
||||
});
|
||||
|
||||
// Start new container
|
||||
console.log(`[UpdateManager] Starting new container: ${containerName}`);
|
||||
log.info('update', 'Starting new container', { containerName });
|
||||
await newContainer.start();
|
||||
|
||||
// Extended verification with health checks and port accessibility
|
||||
console.log(`[UpdateManager] Performing extended verification...`);
|
||||
log.info('update', 'Performing extended verification');
|
||||
await this.verifyContainerExtended(newContainer, inspect, options.verifyTimeout || 60000);
|
||||
|
||||
// Get new image ID
|
||||
@@ -415,12 +416,12 @@ class UpdateManager extends EventEmitter {
|
||||
// Remove old image only after successful verification
|
||||
if (oldImageId !== newImageId) {
|
||||
try {
|
||||
console.log(`[UpdateManager] Removing old image: ${oldImageId.substring(0, 12)}`);
|
||||
log.info('update', 'Removing old image', { oldImageIdPrefix: oldImageId.substring(0, 12) });
|
||||
const oldImage = docker.getImage(oldImageId);
|
||||
await oldImage.remove({ force: false });
|
||||
console.log(`[UpdateManager] Old image removed successfully`);
|
||||
log.info('update', 'Old image removed successfully');
|
||||
} catch (error) {
|
||||
console.warn(`[UpdateManager] Could not remove old image (may be in use): ${error.message}`);
|
||||
log.warn('update', 'Could not remove old image (may be in use)', { error: error.message });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -442,7 +443,7 @@ class UpdateManager extends EventEmitter {
|
||||
this.availableUpdates.delete(containerId);
|
||||
|
||||
this.emit('update-complete', historyEntry);
|
||||
console.log(`[UpdateManager] Update completed in ${duration}ms`);
|
||||
log.info('update', 'Update completed', { durationMs: duration });
|
||||
|
||||
return historyEntry;
|
||||
} catch (error) {
|
||||
@@ -461,11 +462,11 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
// Attempt rollback
|
||||
if (options.autoRollback !== false) {
|
||||
console.log(`[UpdateManager] Attempting rollback for ${containerId}`);
|
||||
log.info('update', 'Attempting rollback', { containerId });
|
||||
try {
|
||||
await this.rollbackUpdate(containerId);
|
||||
} catch (rollbackError) {
|
||||
console.error(`[UpdateManager] Rollback failed:`, rollbackError.message);
|
||||
log.error('update', rollbackError);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -538,7 +539,7 @@ class UpdateManager extends EventEmitter {
|
||||
const maxAttempts = Math.floor(timeout / 2000); // Check every 2 seconds
|
||||
let lastError = null;
|
||||
|
||||
console.log(`[UpdateManager] Extended verification with ${maxAttempts} attempts over ${timeout/1000}s`);
|
||||
log.info('update', 'Extended verification', { maxAttempts, timeoutSec: timeout / 1000 });
|
||||
|
||||
for (let attempt = 0; attempt < maxAttempts; attempt++) {
|
||||
try {
|
||||
@@ -553,14 +554,14 @@ class UpdateManager extends EventEmitter {
|
||||
// Step 2: Check Docker health check if available
|
||||
if (inspect.State.Health) {
|
||||
if (inspect.State.Health.Status === 'healthy') {
|
||||
console.log(`[UpdateManager] Container health check: healthy`);
|
||||
log.info('update', 'Container health check: healthy');
|
||||
return true;
|
||||
} else if (inspect.State.Health.Status === 'unhealthy') {
|
||||
lastError = 'Container health check failed (unhealthy)';
|
||||
throw new Error(lastError);
|
||||
}
|
||||
// Status is 'starting' - continue waiting
|
||||
console.log(`[UpdateManager] Health check status: ${inspect.State.Health.Status} (attempt ${attempt + 1}/${maxAttempts})`);
|
||||
log.info('update', 'Health check status', { status: inspect.State.Health.Status, attempt: attempt + 1, maxAttempts });
|
||||
} else {
|
||||
// Step 3: No Docker health check - verify HTTP port accessibility
|
||||
const ports = this.extractPorts(inspect);
|
||||
@@ -578,22 +579,22 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
// Accept 2xx, 3xx, 4xx as "accessible" (server is responding)
|
||||
if (response.status >= 200 && response.status < 500) {
|
||||
console.log(`[UpdateManager] Port ${primaryPort.hostPort} is accessible (HTTP ${response.status})`);
|
||||
log.info('update', 'Port accessible', { hostPort: primaryPort.hostPort, httpStatus: response.status });
|
||||
|
||||
// Wait a bit more to ensure stability
|
||||
if (attempt >= 2) {
|
||||
console.log(`[UpdateManager] Container verified successfully`);
|
||||
log.info('update', 'Container verified successfully');
|
||||
return true;
|
||||
}
|
||||
}
|
||||
} catch (fetchError) {
|
||||
lastError = `Port ${primaryPort.hostPort} not accessible: ${fetchError.message}`;
|
||||
console.log(`[UpdateManager] ${lastError} (attempt ${attempt + 1}/${maxAttempts})`);
|
||||
log.info('update', lastError, { attempt: attempt + 1, maxAttempts });
|
||||
}
|
||||
} else {
|
||||
// No ports exposed - just verify it's running for a few cycles
|
||||
if (attempt >= 5) {
|
||||
console.log(`[UpdateManager] Container running without exposed ports (verified)`);
|
||||
log.info('update', 'Container running without exposed ports (verified)');
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -605,7 +606,7 @@ class UpdateManager extends EventEmitter {
|
||||
}
|
||||
} catch (error) {
|
||||
lastError = error.message;
|
||||
console.log(`[UpdateManager] Verification attempt ${attempt + 1} failed: ${lastError}`);
|
||||
log.info('update', 'Verification attempt failed', { attempt: attempt + 1, error: lastError });
|
||||
|
||||
if (attempt < maxAttempts - 1) {
|
||||
await new Promise(resolve => setTimeout(resolve, 2000));
|
||||
@@ -649,8 +650,8 @@ class UpdateManager extends EventEmitter {
|
||||
* Rollback to previous version
|
||||
*/
|
||||
async rollbackUpdate(containerId) {
|
||||
console.log(`[UpdateManager] Rolling back container ${containerId}`);
|
||||
|
||||
log.info('update', 'Rolling back container', { containerId });
|
||||
|
||||
// Find last successful update in history
|
||||
const lastUpdate = this.history
|
||||
.filter(h => h.containerId === containerId && h.status === 'success' && h.backup)
|
||||
@@ -682,12 +683,12 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
await newContainer.start();
|
||||
|
||||
console.log(`[UpdateManager] Rollback completed for ${backup.containerName}`);
|
||||
log.info('update', 'Rollback completed', { containerName: backup.containerName });
|
||||
this.emit('rollback-complete', { containerId, containerName: backup.containerName });
|
||||
|
||||
|
||||
return true;
|
||||
} catch (error) {
|
||||
console.error(`[UpdateManager] Rollback failed:`, error.message);
|
||||
log.error('update', error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
@@ -697,18 +698,18 @@ class UpdateManager extends EventEmitter {
|
||||
*/
|
||||
scheduleUpdate(containerId, scheduledTime) {
|
||||
const delay = new Date(scheduledTime).getTime() - Date.now();
|
||||
|
||||
|
||||
if (delay < 0) {
|
||||
throw new Error('Scheduled time must be in the future');
|
||||
}
|
||||
|
||||
setTimeout(() => {
|
||||
this.updateContainer(containerId).catch(error => {
|
||||
console.error(`[UpdateManager] Scheduled update failed:`, error.message);
|
||||
log.error('update', error);
|
||||
});
|
||||
}, delay);
|
||||
|
||||
console.log(`[UpdateManager] Update scheduled for ${containerId} at ${scheduledTime}`);
|
||||
log.info('update', 'Update scheduled', { containerId, scheduledTime });
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -784,7 +785,7 @@ class UpdateManager extends EventEmitter {
|
||||
changelog: this.formatChangelog(repoInfo, tags, imageTag)
|
||||
};
|
||||
} catch (error) {
|
||||
console.error(`[UpdateManager] Error fetching changelog for ${imageName}:`, error.message);
|
||||
log.error('update', error, null, { imageName });
|
||||
|
||||
// Return basic info even on error
|
||||
const [fullRepo] = imageName.split(':');
|
||||
@@ -940,7 +941,7 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
const count = Object.values(this.config.autoUpdate || {}).filter(c => c.enabled).length;
|
||||
if (count > 0) {
|
||||
console.log(`[UpdateManager] Auto-update scheduler started (${count} container(s) configured)`);
|
||||
log.info('update', 'Auto-update scheduler started', { containerCount: count });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -989,17 +990,17 @@ class UpdateManager extends EventEmitter {
|
||||
const update = this.availableUpdates.get(containerId);
|
||||
if (!update) continue;
|
||||
|
||||
console.log(`[UpdateManager] Auto-updating ${update.containerName} (schedule: ${cfg.schedule})`);
|
||||
log.info('update', 'Auto-updating container', { containerName: update.containerName, schedule: cfg.schedule });
|
||||
this.emit('auto-update-start', { containerId, containerName: update.containerName, schedule: cfg.schedule });
|
||||
|
||||
try {
|
||||
const result = await this.updateContainer(containerId, { autoRollback: cfg.autoRollback !== false });
|
||||
cfg.lastAutoUpdate = now.toISOString();
|
||||
this.saveConfig();
|
||||
console.log(`[UpdateManager] Auto-update completed for ${update.containerName}`);
|
||||
log.info('update', 'Auto-update completed', { containerName: update.containerName });
|
||||
this.emit('auto-update-complete', { containerId, containerName: update.containerName, result });
|
||||
} catch (error) {
|
||||
console.error(`[UpdateManager] Auto-update failed for ${update.containerName}:`, error.message);
|
||||
log.error('update', error, null, { containerName: update.containerName });
|
||||
cfg.lastAutoUpdate = now.toISOString(); // Don't retry same day
|
||||
this.saveConfig();
|
||||
this.emit('auto-update-failed', { containerId, containerName: update.containerName, error: error.message });
|
||||
@@ -1056,7 +1057,7 @@ class UpdateManager extends EventEmitter {
|
||||
return JSON.parse(fs.readFileSync(UPDATE_CONFIG_FILE, 'utf8'));
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error loading config:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
return { autoUpdate: {} };
|
||||
}
|
||||
@@ -1068,7 +1069,7 @@ class UpdateManager extends EventEmitter {
|
||||
try {
|
||||
fs.writeFileSync(UPDATE_CONFIG_FILE, JSON.stringify(this.config, null, 2));
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error saving config:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1081,7 +1082,7 @@ class UpdateManager extends EventEmitter {
|
||||
return JSON.parse(fs.readFileSync(UPDATE_HISTORY_FILE, 'utf8'));
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error loading history:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
return [];
|
||||
}
|
||||
@@ -1093,7 +1094,7 @@ class UpdateManager extends EventEmitter {
|
||||
try {
|
||||
fs.writeFileSync(UPDATE_HISTORY_FILE, JSON.stringify(this.history, null, 2));
|
||||
} catch (error) {
|
||||
console.error('[UpdateManager] Error saving history:', error.message);
|
||||
log.error('update', error);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user