diff --git a/backend/src/__tests__/compose-doctor-service.test.ts b/backend/src/__tests__/compose-doctor-service.test.ts index 60a374ab..55de4dde 100644 --- a/backend/src/__tests__/compose-doctor-service.test.ts +++ b/backend/src/__tests__/compose-doctor-service.test.ts @@ -22,8 +22,13 @@ let nodeId: number; function db() { return DatabaseService.getInstance(); } function doctor() { return ComposeDoctorService.getInstance(); } -/** Mock the two Docker calls; render returns the given effective model JSON. */ -function stubDocker(rendered: object | null, stderr = '', snapshot = { containers: [], networks: [], volumes: [] }) { +/** Mock the two Docker calls; render returns the given effective model JSON. + * Pass `'reject'` as the snapshot to simulate an unreachable Docker daemon. */ +function stubDocker( + rendered: object | null, + stderr = '', + snapshot: { containers: unknown[]; networks: unknown[]; volumes: unknown[] } | 'reject' = { containers: [], networks: [], volumes: [] }, +) { vi.spyOn(ComposeService, 'getInstance').mockReturnValue({ renderConfig: vi.fn().mockResolvedValue({ rendered: rendered === null ? null : JSON.stringify(rendered), @@ -33,7 +38,9 @@ function stubDocker(rendered: object | null, stderr = '', snapshot = { container }), } as unknown as ComposeService); vi.spyOn(DockerController, 'getInstance').mockReturnValue({ - getDependencySnapshot: vi.fn().mockResolvedValue(snapshot), + getDependencySnapshot: snapshot === 'reject' + ? vi.fn().mockRejectedValue(new Error('docker down')) + : vi.fn().mockResolvedValue(snapshot), } as unknown as DockerController); } @@ -164,19 +171,55 @@ describe('runPreflight', () => { }); it('degrades to model-only findings when the node snapshot fails', async () => { - vi.spyOn(ComposeService, 'getInstance').mockReturnValue({ - renderConfig: vi.fn().mockResolvedValue({ - rendered: JSON.stringify({ name: STACK, services: { web: { image: 'nginx:latest' } }, networks: {}, volumes: {} }), - stderr: '', code: 0, timedOut: false, - }), - } as unknown as ComposeService); - vi.spyOn(DockerController, 'getInstance').mockReturnValue({ - getDependencySnapshot: vi.fn().mockRejectedValue(new Error('docker down')), - } as unknown as DockerController); + stubDocker({ name: STACK, services: { web: { image: 'nginx:latest' } }, networks: {}, volumes: {} }, '', 'reject'); const report = await doctor().runPreflight(nodeId, STACK, null); expect(report.renderable).toBe(true); expect(report.findings.map(f => f.ruleId)).toContain('image-latest'); // model rule still ran expect(report.findings.map(f => f.ruleId)).not.toContain('port-conflict-node'); // node-state skipped + expect(report.findings.map(f => f.ruleId)).toContain('node-state-unavailable'); // partial coverage is surfaced + }); + + it('does not flag external resources as missing when the snapshot is unavailable', async () => { + stubDocker( + { + name: STACK, + services: { web: { image: 'nginx:1.27', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } }, + networks: { ext: { name: 'shared', external: true } }, + volumes: { v: { name: 'data', external: true } }, + }, + '', + 'reject', + ); + const report = await doctor().runPreflight(nodeId, STACK, null); + expect(report.renderable).toBe(true); + const ruleIds = report.findings.map(f => f.ruleId); + // An empty snapshot must not be read as "the external resource is absent". + expect(ruleIds).not.toContain('external-network-missing'); + expect(ruleIds).not.toContain('external-volume-missing'); + expect(ruleIds).toContain('node-state-unavailable'); + }); + + it('marks the run partial (info) when only node state is unavailable', async () => { + stubDocker({ name: STACK, services: { web: { image: 'nginx:1.27', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } }, networks: {}, volumes: {} }, '', 'reject'); + const report = await doctor().runPreflight(nodeId, STACK, null); + // A clean model with the daemon down yields only the advisory, so the clean + // 'pass' becomes 'info': the operator sees the result is partial. + expect(report.findings.map(f => f.ruleId)).toEqual(['node-state-unavailable']); + expect(report.status).toBe('info'); + expect(report.highestSeverity).toBe('info'); + }); + + it('runs node-state rules against a collected snapshot', async () => { + stubDocker( + { name: STACK, services: { web: { image: 'nginx:1.27', container_name: 'dup', restart: 'always', healthcheck: { test: ['CMD', 'true'] } } }, networks: {}, volumes: {} }, + '', + { containers: [{ name: 'dup', stack: 'other', ports: [] }], networks: [], volumes: [] }, + ); + const report = await doctor().runPreflight(nodeId, STACK, null); + const ruleIds = report.findings.map(f => f.ruleId); + // A successful snapshot sets nodeStateAvailable true, so the node-state rule runs. + expect(ruleIds).toContain('container-name-collision'); + expect(ruleIds).not.toContain('node-state-unavailable'); }); it('orders findings by severity, highest first', async () => { diff --git a/backend/src/__tests__/preflight-rules.test.ts b/backend/src/__tests__/preflight-rules.test.ts index 23918782..e3a9fe54 100644 --- a/backend/src/__tests__/preflight-rules.test.ts +++ b/backend/src/__tests__/preflight-rules.test.ts @@ -29,7 +29,7 @@ function ctx(over: Partial = {}): PreflightContext { missingEnvFiles: [], sourceServiceNames: m ? m.services.map(s => s.name) : [], sourceReadable: true, nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(), - existingContainers: [], bindChecks: [], + existingContainers: [], nodeStateAvailable: true, bindChecks: [], stackIntent: null, serviceIntents: {}, accessUrlPorts: new Set(), hasAccessUrls: false, ...over, }; } @@ -342,6 +342,68 @@ describe('exposure-intent rules', () => { }); }); +describe('node-state availability', () => { + // When the node's Docker snapshot could not be collected, the empty sets must not + // be read as "resource absent" or "no conflict". Every node-state rule suppresses + // itself, and one advisory explains the partial coverage. + const externalRes = model([svc()], { + networks: { ext: { name: 'shared', external: true, internal: false } }, + volumes: { v: { name: 'data', external: true, internal: false } }, + }); + const newRes = model([svc()], { + networks: { backend: { name: 'backend', external: false, internal: false } }, + volumes: { data: { name: 'data', external: false, internal: false } }, + }); + const portModel = model([svc({ ports: [{ startPort: 8080, endPort: 8080, hostIp: '', protocol: 'tcp' }] })]); + const nameModel = model([svc({ containerName: 'taken' })]); + + it('does not assert an external network/volume is absent', () => { + const f = runRules(ctx({ model: externalRes, nodeStateAvailable: false })); + expect(ids(f, 'external-network-missing')).toHaveLength(0); + expect(ids(f, 'external-volume-missing')).toHaveLength(0); + }); + it('does not claim a network/volume is new', () => { + const f = runRules(ctx({ model: newRes, nodeStateAvailable: false })); + expect(ids(f, 'new-network')).toHaveLength(0); + expect(ids(f, 'new-volume')).toHaveLength(0); + }); + it('does not report a clean all-clear over a real port conflict', () => { + const f = runRules(ctx({ model: portModel, nodeStateAvailable: false, + nodePorts: [{ publishedPort: 8080, protocol: 'tcp', ip: '', stack: 'other' }] })); + expect(ids(f, 'port-conflict-node')).toHaveLength(0); + }); + it('does not report a clean all-clear over a real container_name collision', () => { + const f = runRules(ctx({ model: nameModel, nodeStateAvailable: false, + existingContainers: [{ name: 'taken', stack: 'other' }] })); + expect(ids(f, 'container-name-collision')).toHaveLength(0); + }); + it('still runs node-state rules when the snapshot is available', () => { + const f = runRules(ctx({ model: externalRes, nodeStateAvailable: true })); + expect(ids(f, 'external-network-missing')).toHaveLength(1); + expect(ids(f, 'external-volume-missing')).toHaveLength(1); + }); + it('does not suppress higher-severity model findings while node state is unavailable', () => { + const f = runRules(ctx({ model: model([svc({ privileged: true })]), nodeStateAvailable: false })); + expect(ids(f, 'node-state-unavailable')).toHaveLength(1); + expect(ids(f, 'privileged')).toHaveLength(1); // a real model finding is still reported alongside the advisory + }); + + describe('node-state-unavailable advisory', () => { + it('fires one info finding when the model rendered but node state is unavailable', () => { + const f = ids(runRules(ctx({ model: model([svc()]), nodeStateAvailable: false })), 'node-state-unavailable'); + expect(f).toHaveLength(1); + expect(f[0].severity).toBe('info'); + }); + it('stays silent when node state is available', () => { + expect(ids(runRules(ctx({ model: model([svc()]), nodeStateAvailable: true })), 'node-state-unavailable')).toHaveLength(0); + }); + it('stays silent when the model is unrenderable (render-failed already covers it)', () => { + const f = runRules(ctx({ model: null, renderable: false, renderError: 'boom', nodeStateAvailable: false })); + expect(ids(f, 'node-state-unavailable')).toHaveLength(0); + }); + }); +}); + describe('rule registry completeness', () => { // The canonical rule set. Adding or removing a rule must update this list, // which forces a deliberate pass over the docs and the frontend severity map. @@ -349,6 +411,7 @@ describe('rule registry completeness', () => { 'render-failed', 'env-unset', 'env-file-missing', 'port-conflict-node', 'port-conflict-internal', 'port-exposed-all-interfaces', 'bind-path-missing', 'bind-path-permission', 'docker-socket-mount', 'privileged', 'network-mode-host', 'uid-gid-risk', 'image-latest', 'no-restart-policy', 'no-healthcheck', 'deploy-swarm-only', + 'node-state-unavailable', 'external-network-missing', 'external-volume-missing', 'new-network', 'new-volume', 'anonymous-volume', 'container-name-internal-dup', 'container-name-collision', 'exposure-internal-published', 'sensitive-service-broad-exposure', 'exposure-unclassified', diff --git a/backend/src/services/ComposeDoctorService.ts b/backend/src/services/ComposeDoctorService.ts index 4f9e8420..69517172 100644 --- a/backend/src/services/ComposeDoctorService.ts +++ b/backend/src/services/ComposeDoctorService.ts @@ -175,7 +175,7 @@ export class ComposeDoctorService { || 'Sencho could not run docker compose on this node.'; } - const { nodePorts, existingNetworkNames, existingVolumeNames, existingContainers } = await this.nodeState(nodeId, fsSvc, stackName); + const { nodePorts, existingNetworkNames, existingVolumeNames, existingContainers, nodeStateAvailable } = await this.nodeState(nodeId, fsSvc, stackName); const bindChecks = model ? await this.resolveBindChecks(model, baseDir) : []; const { stackIntent, serviceIntents, accessUrlPorts, hasAccessUrls } = this.exposureState(nodeId, stackName); @@ -207,6 +207,7 @@ export class ComposeDoctorService { existingNetworkNames, existingVolumeNames, existingContainers, + nodeStateAvailable, bindChecks, stackIntent, serviceIntents, @@ -254,6 +255,7 @@ export class ComposeDoctorService { existingNetworkNames: Set; existingVolumeNames: Set; existingContainers: { name: string; stack: string | null }[]; + nodeStateAvailable: boolean; }> { try { const knownStacks = await fsSvc.getStacks(); @@ -265,11 +267,12 @@ export class ComposeDoctorService { existingNetworkNames: new Set(snapshot.networks.map(n => n.name)), existingVolumeNames: new Set(snapshot.volumes.map(v => v.name)), existingContainers: snapshot.containers.map(c => ({ name: c.name, stack: c.stack })), + nodeStateAvailable: true, }; } catch (error) { console.warn('[ComposeDoctor] Node snapshot unavailable for %s; node-state checks skipped:', sanitizeForLog(stackName), sanitizeForLog(getErrorMessage(error, 'unknown'))); - return { nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(), existingContainers: [] }; + return { nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(), existingContainers: [], nodeStateAvailable: false }; } } diff --git a/backend/src/services/preflight/rules.ts b/backend/src/services/preflight/rules.ts index c318108a..a8fe0ed6 100644 --- a/backend/src/services/preflight/rules.ts +++ b/backend/src/services/preflight/rules.ts @@ -105,7 +105,7 @@ const envFileMissing: PreflightRule = { const portConflictNode: PreflightRule = { id: 'port-conflict-node', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const byPort = new Map(); for (const b of ctx.nodePorts) { const list = byPort.get(b.publishedPort); @@ -388,10 +388,31 @@ const deploySwarmOnly: PreflightRule = { }, }; +const nodeStateUnavailable: PreflightRule = { + id: 'node-state-unavailable', + run(ctx) { + // Emit only when the model rendered but the node's Docker snapshot could not be + // read: an unrenderable model already raises its own render-failed blocker, so a + // second advisory there would be noise. The rules that read node state suppress + // themselves in this state; this finding tells the operator why, so a clean pass + // during an outage is not mistaken for full coverage. (The info-only + // new-network / new-volume notices are gated too, but left out of the message + // below: they preview a deploy action rather than flag a problem.) + if (!ctx.model || ctx.nodeStateAvailable) return []; + return [{ + ruleId: 'node-state-unavailable', + severity: 'info', + title: 'Node-state checks skipped', + message: 'The node\'s Docker state could not be read, so the external-resource, host-port, and container_name checks did not run. This result is partial.', + remediation: 'Confirm the Docker daemon is reachable on this node, then re-run preflight.', + }]; + }, +}; + const externalNetworkMissing: PreflightRule = { id: 'external-network-missing', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const findings: PreflightFinding[] = []; for (const [key, net] of Object.entries(ctx.model.networks)) { if (!net.external || ctx.existingNetworkNames.has(net.name)) continue; @@ -411,7 +432,7 @@ const externalNetworkMissing: PreflightRule = { const externalVolumeMissing: PreflightRule = { id: 'external-volume-missing', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const findings: PreflightFinding[] = []; for (const [key, vol] of Object.entries(ctx.model.volumes)) { if (!vol.external || ctx.existingVolumeNames.has(vol.name)) continue; @@ -431,7 +452,7 @@ const externalVolumeMissing: PreflightRule = { const newNetwork: PreflightRule = { id: 'new-network', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const findings: PreflightFinding[] = []; for (const [key, net] of Object.entries(ctx.model.networks)) { if (net.external || key === 'default') continue; @@ -452,7 +473,7 @@ const newNetwork: PreflightRule = { const newVolume: PreflightRule = { id: 'new-volume', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const findings: PreflightFinding[] = []; for (const [key, vol] of Object.entries(ctx.model.volumes)) { if (vol.external) continue; @@ -521,7 +542,7 @@ const containerNameInternalDup: PreflightRule = { const containerNameCollision: PreflightRule = { id: 'container-name-collision', run(ctx) { - if (!ctx.model) return []; + if (!ctx.model || !ctx.nodeStateAvailable) return []; const findings: PreflightFinding[] = []; for (const s of ctx.model.services) { if (!s.containerName) continue; @@ -723,6 +744,7 @@ export const PREFLIGHT_RULES: PreflightRule[] = [ noRestartPolicy, noHealthcheck, deploySwarmOnly, + nodeStateUnavailable, externalNetworkMissing, externalVolumeMissing, newNetwork, diff --git a/backend/src/services/preflight/types.ts b/backend/src/services/preflight/types.ts index 7aba3478..1f7f5cb0 100644 --- a/backend/src/services/preflight/types.ts +++ b/backend/src/services/preflight/types.ts @@ -100,6 +100,9 @@ export interface PreflightContext { existingNetworkNames: Set; existingVolumeNames: Set; existingContainers: { name: string; stack: string | null }[]; + /** Whether the node's Docker snapshot was collected; gates node-state checks so + * an unavailable snapshot cannot be mistaken for an empty node. */ + nodeStateAvailable: boolean; bindChecks: BindCheck[]; /** Stack-level exposure classification, or null when unset. */ stackIntent: ExposureIntent | null; diff --git a/docs/features/compose-doctor.mdx b/docs/features/compose-doctor.mdx index 3af6b6fe..174a79a5 100644 --- a/docs/features/compose-doctor.mdx +++ b/docs/features/compose-doctor.mdx @@ -60,6 +60,9 @@ Preflight runs against the **active node**, so selecting a remote node checks th Preflight ignores ports already held by the stack you are checking, so redeploying a running stack does not flag its own bindings. A conflict means a *different* stack, or an unmanaged container, holds that host port on this node. + + The node-state checks (external networks and volumes, host-port conflicts, and `container_name` collisions) read the node's live Docker state. When the Docker daemon on the target node is unreachable, Sencho cannot read that state, so those checks are skipped and the report shows an info notice rather than guessing. The model checks still run. Confirm the daemon is reachable on the node, then run preflight again for full coverage. + The tab appears when the active node reports that it supports Compose Doctor. A node running an older version of Sencho does not advertise it, so the tab is hidden for that node until it is updated.