From 1dc12f7da8c33bbab5d4cb8402d1b9ff41c62c0a Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 28 Jun 2026 16:33:01 -0400 Subject: [PATCH] fix: bind fleet stop-by-label to the exact confirmed nodes and stacks (#1506) A fleet stop re-matched stacks by label name at execution, so a stack that gained the label between the operator's preview and confirmation could be stopped even though it never appeared in the confirmation. A confirmed node that was deleted after the preview also vanished from the results, letting the remaining successes read as a clean stop. The confirm flow now sends the exact node and stack list resolved in the preview. Each node's stop is bound to that set: only stacks that are still label-matched and confirmed are stopped, and a confirmed node missing from the registry is reported as an explicit failure rather than dropped. --- .../fleet-action-card-endpoints.test.ts | 39 ++--- backend/src/__tests__/fleet-actions.test.ts | 147 ++++++++++++++++++ backend/src/helpers/fleetLabelStop.ts | 37 ++++- backend/src/routes/fleet.ts | 90 ++++++++--- backend/src/routes/fleetActions.ts | 15 +- .../cards/LabelFleetStopCard.test.tsx | 42 ++++- .../FleetActions/cards/LabelFleetStopCard.tsx | 22 +-- 7 files changed, 333 insertions(+), 59 deletions(-) diff --git a/backend/src/__tests__/fleet-action-card-endpoints.test.ts b/backend/src/__tests__/fleet-action-card-endpoints.test.ts index c5021038..01eb8313 100644 --- a/backend/src/__tests__/fleet-action-card-endpoints.test.ts +++ b/backend/src/__tests__/fleet-action-card-endpoints.test.ts @@ -757,11 +757,12 @@ describe('POST /api/fleet/labels/fleet-stop remote leg', () => { }); }); -describe('POST /api/fleet/labels/fleet-stop nodeIds allowlist', () => { - // Guards the wrong-node drift fix: the real stop carries the node ids the - // operator confirmed in the preview, so a node that was unreachable then and - // reconnects before execution cannot silently enter the fan-out. - it('contacts a confirmed remote in the allowlist and excludes the unconfirmed local node', async () => { +describe('POST /api/fleet/labels/fleet-stop confirmed-target allowlist', () => { + // Guards the drift fix: the real stop carries the exact node + stack list the + // operator confirmed in the preview. A node that was unreachable then and + // reconnects cannot enter the fan-out, and a stack labelled after the preview + // is not stopped (the per-node receiver intersects against the sent stacks). + it('contacts a confirmed remote and forwards its confirmed stacks, excluding the unconfirmed local node', async () => { const remoteId = db.addNode({ name: 'confirmed-remote', type: 'remote', api_url: 'http://confirmed.example:1852', api_token: 'tok', compose_dir: '/app/compose', is_default: false, @@ -773,14 +774,16 @@ describe('POST /api/fleet/labels/fleet-stop nodeIds allowlist', () => { const res = await request(app) .post('/api/fleet/labels/fleet-stop') .set('Authorization', authHeader) - .send({ labelName: 'any-label', nodeIds: [remoteId] }); + .send({ labelName: 'any-label', targets: [{ nodeId: remoteId, stackNames: ['alpha'] }] }); expect(res.status).toBe(200); // Only the confirmed remote is in the fan-out: its local-stop receiver is // contacted and the unconfirmed local node is absent from the results. expect(res.body.results).toHaveLength(1); expect(res.body.results[0].nodeId).toBe(remoteId); - const urls = fetchSpy.mock.calls.map(c => String(c[0])); - expect(urls.some(u => u.endsWith('/api/fleet-actions/labels/local-stop'))).toBe(true); + const stopCall = fetchSpy.mock.calls.find(c => String(c[0]).endsWith('/api/fleet-actions/labels/local-stop')); + expect(stopCall).toBeTruthy(); + // The confirmed stacks are forwarded so the remote binds its stop to them. + expect(JSON.parse((stopCall![1] as RequestInit).body as string)).toMatchObject({ labelName: 'any-label', stackNames: ['alpha'] }); } finally { db.deleteNode(remoteId); } @@ -798,7 +801,7 @@ describe('POST /api/fleet/labels/fleet-stop nodeIds allowlist', () => { const res = await request(app) .post('/api/fleet/labels/fleet-stop') .set('Authorization', authHeader) - .send({ labelName: label.name, nodeIds: [localId] }); + .send({ labelName: label.name, targets: [{ nodeId: localId, stackNames: ['alpha'] }] }); expect(res.status).toBe(200); // The excluded remote has a proxy target and would otherwise be contacted; // the allowlist keeps it out of execution entirely. @@ -811,36 +814,36 @@ describe('POST /api/fleet/labels/fleet-stop nodeIds allowlist', () => { } }); - it('treats an empty allowlist as zero target nodes, stopping nothing', async () => { + it('treats empty targets as zero target nodes, stopping nothing', async () => { const label = await createAssignedLabel('empty-allow', ['alpha']); const fetchSpy = vi.spyOn(globalThis, 'fetch'); const res = await request(app) .post('/api/fleet/labels/fleet-stop') .set('Authorization', authHeader) - .send({ labelName: label.name, nodeIds: [] }); + .send({ labelName: label.name, targets: [] }); expect(res.status).toBe(200); - // An empty allowlist filters to no nodes: a fail-safe no-op rather than a + // Empty targets filter to no nodes: a fail-safe no-op rather than a // full-fleet stop. The local node is not acted on and no remote is contacted. expect(res.body.results).toEqual([]); expect(fetchSpy).not.toHaveBeenCalled(); }); - it('rejects a non-array nodeIds with 400', async () => { + it('rejects a non-array targets with 400', async () => { const res = await request(app) .post('/api/fleet/labels/fleet-stop') .set('Authorization', authHeader) - .send({ labelName: 'whatever', nodeIds: 'oops' }); + .send({ labelName: 'whatever', targets: 'oops' }); expect(res.status).toBe(400); - expect(res.body.error).toMatch(/nodeIds/); + expect(res.body.error).toMatch(/targets/); }); - it('rejects a non-integer nodeIds entry with 400', async () => { + it('rejects a non-integer target.nodeId with 400', async () => { const res = await request(app) .post('/api/fleet/labels/fleet-stop') .set('Authorization', authHeader) - .send({ labelName: 'whatever', nodeIds: [1, 2.5] }); + .send({ labelName: 'whatever', targets: [{ nodeId: 2.5, stackNames: ['x'] }] }); expect(res.status).toBe(400); - expect(res.body.error).toMatch(/nodeIds/); + expect(res.body.error).toMatch(/nodeId/); }); }); diff --git a/backend/src/__tests__/fleet-actions.test.ts b/backend/src/__tests__/fleet-actions.test.ts index 25cf58c6..b4e058d0 100644 --- a/backend/src/__tests__/fleet-actions.test.ts +++ b/backend/src/__tests__/fleet-actions.test.ts @@ -111,6 +111,24 @@ describe('Fleet Actions input validation', () => { expect(res.status).toBe(400); }); + it('POST /api/fleet/labels/fleet-stop rejects a non-object target entry', async () => { + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'prod', targets: ['oops'] }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/each target must be an object/); + }); + + it('POST /api/fleet/labels/fleet-stop rejects a target with non-array stackNames', async () => { + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'prod', targets: [{ nodeId: 1, stackNames: 'oops' }] }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/target.stackNames must be an array/); + }); + it('POST /api/fleet/labels/bulk-assign rejects an invalid label color', async () => { const res = await request(app) .post('/api/fleet/labels/bulk-assign') @@ -326,6 +344,135 @@ describe('local-stop behavior', () => { expect(res.status).toBe(200); expect(res.body).toEqual({ matched: true, results: [] }); }); + + it('rejects a non-array stackNames allowlist', async () => { + const res = await request(app) + .post('/api/fleet-actions/labels/local-stop') + .set('Authorization', authHeader) + .send({ labelName: 'whatever', stackNames: 'oops' }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/stackNames must be an array/); + }); + + it('stops only the stacks in the stackNames allowlist, not every labelled stack', async () => { + makeStack('allow-a'); + makeStack('allow-b'); + const label = db.createLabel(nodeId, 'allow-label', 'slate'); + db.setStackLabels('allow-a', nodeId, [label.id]); + db.setStackLabels('allow-b', nodeId, [label.id]); + const res = await request(app) + .post('/api/fleet-actions/labels/local-stop') + .set('Authorization', authHeader) + .send({ labelName: 'allow-label', dryRun: true, stackNames: ['allow-a'] }); + expect(res.status).toBe(200); + // allow-b carries the label too but was not confirmed, so it is excluded. + expect(res.body.results).toEqual([{ stackName: 'allow-a', success: true, dryRun: true }]); + }); + + it('reports a confirmed stack that no longer carries the label as a failure', async () => { + makeStack('drop-kept'); + makeStack('drop-gone'); + const label = db.createLabel(nodeId, 'drop-label', 'teal'); + db.setStackLabels('drop-kept', nodeId, [label.id]); + // drop-gone was confirmed but does not carry the label at execution time. + const res = await request(app) + .post('/api/fleet-actions/labels/local-stop') + .set('Authorization', authHeader) + .send({ labelName: 'drop-label', dryRun: true, stackNames: ['drop-kept', 'drop-gone'] }); + expect(res.status).toBe(200); + const byName = Object.fromEntries(res.body.results.map((r: { stackName: string }) => [r.stackName, r])); + expect(byName['drop-kept']).toEqual({ stackName: 'drop-kept', success: true, dryRun: true }); + // The confirmed stack that fell out of scope is reported, not silently dropped. + expect(byName['drop-gone']).toEqual({ stackName: 'drop-gone', success: false, error: 'No longer carries this label' }); + }); + + it('reports a confirmed labelled stack that is missing on disk as a failure', async () => { + const label = db.createLabel(nodeId, 'ghostdisk-label', 'rose'); + db.setStackLabels('ghostdisk-stack', nodeId, [label.id]); // labelled but never created on disk + const res = await request(app) + .post('/api/fleet-actions/labels/local-stop') + .set('Authorization', authHeader) + .send({ labelName: 'ghostdisk-label', dryRun: true, stackNames: ['ghostdisk-stack'] }); + expect(res.status).toBe(200); + expect(res.body.results).toEqual([{ stackName: 'ghostdisk-stack', success: false, error: 'Stack not found on this node' }]); + }); +}); + +// Orchestrator-level binding: the control sends the exact node + stack list the +// operator confirmed, and the fan-out must neither widen the stack set nor drop +// a confirmed node that has since disappeared from the registry. +describe('fleet-stop confirmed-target binding', () => { + let db: import('../services/DatabaseService').DatabaseService; + let localNodeId: number; + + beforeAll(async () => { + const { DatabaseService } = await import('../services/DatabaseService'); + const { NodeRegistry } = await import('../services/NodeRegistry'); + db = DatabaseService.getInstance(); + localNodeId = NodeRegistry.getInstance().getDefaultNodeId(); + }); + + afterEach(() => vi.restoreAllMocks()); + + it('stops only the confirmed stacks even when others share the label', async () => { + makeStack('bind-a'); + makeStack('bind-late'); + const label = db.createLabel(localNodeId, 'bind-label', 'teal'); + db.setStackLabels('bind-a', localNodeId, [label.id]); + // bind-late carries the label but was not in the confirmed preview (it + // stands in for a stack labelled between preview and execution). + db.setStackLabels('bind-late', localNodeId, [label.id]); + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'bind-label', dryRun: true, targets: [{ nodeId: localNodeId, stackNames: ['bind-a'] }] }); + expect(res.status).toBe(200); + const local = res.body.results.find((r: { nodeId: number }) => r.nodeId === localNodeId); + expect(local.matched).toBe(true); + expect(local.stackResults.map((s: { stackName: string }) => s.stackName)).toEqual(['bind-a']); + }); + + it('surfaces a confirmed node that no longer exists as an unreachable failure', async () => { + makeStack('mix-a'); + const label = db.createLabel(localNodeId, 'mix-label', 'rose'); + db.setStackLabels('mix-a', localNodeId, [label.id]); + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ + labelName: 'mix-label', + dryRun: true, + targets: [ + { nodeId: localNodeId, stackNames: ['mix-a'] }, + { nodeId: 999999, stackNames: ['ghost'] }, + ], + }); + expect(res.status).toBe(200); + const local = res.body.results.find((r: { nodeId: number }) => r.nodeId === localNodeId); + const missing = res.body.results.find((r: { nodeId: number }) => r.nodeId === 999999); + expect(local.stackResults.map((s: { stackName: string }) => s.stackName)).toEqual(['mix-a']); + expect(missing).toBeTruthy(); + expect(missing.reachable).toBe(false); + expect(missing.error).toMatch(/no longer exists/i); + // The missing node's confirmed stacks are marked failed so the run reports + // as partial rather than a clean stop of the surviving node alone. + expect(missing.stackResults).toEqual([{ stackName: 'ghost', success: false, error: 'Node no longer exists' }]); + }); + + it('without targets, scans the whole label (no per-stack binding)', async () => { + makeStack('nobind-a'); + makeStack('nobind-b'); + const label = db.createLabel(localNodeId, 'nobind-label', 'amber'); + db.setStackLabels('nobind-a', localNodeId, [label.id]); + db.setStackLabels('nobind-b', localNodeId, [label.id]); + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'nobind-label', dryRun: true }); + expect(res.status).toBe(200); + const local = res.body.results.find((r: { nodeId: number }) => r.nodeId === localNodeId); + expect(local.stackResults.map((s: { stackName: string }) => s.stackName).sort()).toEqual(['nobind-a', 'nobind-b']); + }); }); describe('local-assign receiver behavior', () => { diff --git a/backend/src/helpers/fleetLabelStop.ts b/backend/src/helpers/fleetLabelStop.ts index 45458af8..657ae427 100644 --- a/backend/src/helpers/fleetLabelStop.ts +++ b/backend/src/helpers/fleetLabelStop.ts @@ -62,23 +62,36 @@ export function isLabelLocalStopResponse(value: unknown): value is LabelLocalSto * * Shares the per-node `bulk:` lock with `POST /api/labels/:id/action` so * a fleet-stop and a per-label action cannot double-stop the same containers. + * + * `allowedStacks`, when provided, binds the stop to exactly the stacks the + * operator confirmed in the preview: only stacks that are still label-matched + * AND in this set are stopped, so a stack that gained the label between preview + * and execution is never stopped. A confirmed stack that went the other way (it + * lost the label or left disk) is reported as a failure rather than dropped, so + * the stop never reads as clean for a stack that silently fell out of scope. + * Omitted (e.g. a dry run) means stop every currently label-matched stack. */ export async function runLocalLabelStop( nodeId: number, labelName: string, dryRun: boolean, + allowedStacks?: ReadonlySet, ): Promise { const db = DatabaseService.getInstance(); const label = db.getLabels(nodeId).find(l => l.name === labelName); if (!label) return { matched: false, stackResults: [] }; const stackNames = db.getStacksForLabel(label.id, nodeId); - if (stackNames.length === 0) return { matched: true, stackResults: [] }; + // With no confirmed allowlist this is the unbound path: an unassigned label is + // a clean no-op. When stacks were confirmed we fall through so any that left + // scope are reported rather than silently dropped (see the reconcile below). + if (stackNames.length === 0 && !allowedStacks) return { matched: true, stackResults: [] }; const lockKey = `bulk:${nodeId}`; if (activeBulkActions.has(lockKey)) { + const busyStacks = allowedStacks ? stackNames.filter(name => allowedStacks.has(name)) : stackNames; return { matched: true, - stackResults: stackNames.map(stackName => ({ + stackResults: busyStacks.map(stackName => ({ stackName, success: false, error: 'A bulk action is already running on this node', @@ -89,7 +102,9 @@ export async function runLocalLabelStop( try { const fsStacks = await FileSystemService.getInstance(nodeId).getStacks(); const fsStackSet = new Set(fsStacks); - const validStacks = stackNames.filter(name => fsStackSet.has(name)); + const validStacks = stackNames.filter(name => + fsStackSet.has(name) && (!allowedStacks || allowedStacks.has(name)), + ); const stackResults: StackStopResult[] = []; for (const stackName of validStacks) { if (dryRun) { @@ -101,6 +116,22 @@ export async function runLocalLabelStop( else if (outcome.kind === 'no-containers') stackResults.push({ stackName, success: false, error: 'No containers found for this stack' }); else stackResults.push({ stackName, success: false, error: outcome.message }); } + // Reconcile the confirmed set against what was actually stoppable. A stack + // the operator confirmed that has since lost the label or left disk is + // reported as a failure, so the stop never reads as clean for a stack that + // silently dropped out between preview and execution. + if (allowedStacks) { + const acted = new Set(validStacks); + const labelled = new Set(stackNames); + for (const stackName of allowedStacks) { + if (acted.has(stackName)) continue; + stackResults.push({ + stackName, + success: false, + error: labelled.has(stackName) ? 'Stack not found on this node' : 'No longer carries this label', + }); + } + } if (!dryRun && stackResults.some(r => r.success)) invalidateNodeCaches(nodeId); return { matched: true, stackResults }; } finally { diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index eaf2ee26..732896a6 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -1379,37 +1379,56 @@ type FleetStopNodeResult = { // Tier: requireAdmin (admin-only fleet plumbing; available on every license). fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: Response): Promise => { if (!requireAdmin(req, res)) return; - const body = req.body as { labelName?: unknown; dryRun?: unknown; nodeIds?: unknown } | undefined; + const body = req.body as { labelName?: unknown; dryRun?: unknown; targets?: unknown } | undefined; if (!body || typeof body !== 'object') { res.status(400).json({ error: 'Request body is required' }); return; } - const { labelName, dryRun, nodeIds } = body; + const { labelName, dryRun, targets } = body; if (typeof labelName !== 'string' || labelName.trim().length === 0) { res.status(400).json({ error: 'labelName is required' }); return; } - // Optional allowlist binding execution to the preview the operator confirmed. - // The confirm flow sends exactly the nodes shown in the resolved blast radius, - // so a node that was unreachable during preview and reconnects before the stop - // cannot silently enter execution and have unlisted stacks stopped. Absent - // (e.g. a dry run) the fan-out scans the whole fleet as before. - let allowedNodeIds: Set | null = null; - if (nodeIds !== undefined) { - if (!Array.isArray(nodeIds) || !nodeIds.every(n => typeof n === 'number' && Number.isInteger(n))) { - res.status(400).json({ error: 'nodeIds must be an array of integers' }); + // Optional per-node allowlist binding execution to exactly the preview the + // operator confirmed: each entry names a node and the stacks to stop on it. + // This binds both axes of drift between preview and execution: a node that was + // unreachable during preview and reconnects cannot enter the stop (it is not + // in the map), and a stack that gained the label after preview is never + // stopped (the executor intersects the live label match against this set). + // Absent (e.g. a dry run) the fan-out scans the whole fleet as before. + let confirmedStacksByNode: Map> | null = null; + if (targets !== undefined) { + if (!Array.isArray(targets)) { + res.status(400).json({ error: 'targets must be an array' }); return; } - allowedNodeIds = new Set(nodeIds as number[]); + confirmedStacksByNode = new Map(); + for (const raw of targets) { + if (!raw || typeof raw !== 'object') { + res.status(400).json({ error: 'each target must be an object' }); + return; + } + const { nodeId, stackNames } = raw as { nodeId?: unknown; stackNames?: unknown }; + if (typeof nodeId !== 'number' || !Number.isInteger(nodeId)) { + res.status(400).json({ error: 'target.nodeId must be an integer' }); + return; + } + if (!Array.isArray(stackNames) || !stackNames.every(s => typeof s === 'string')) { + res.status(400).json({ error: 'target.stackNames must be an array of strings' }); + return; + } + confirmedStacksByNode.set(nodeId, new Set(stackNames as string[])); + } } const trimmed = labelName.trim(); const isDryRun = dryRun === true; try { const db = DatabaseService.getInstance(); const nodes = db.getNodes(); - const targetNodes = allowedNodeIds ? nodes.filter(n => allowedNodeIds.has(n.id)) : nodes; - if (isDebugEnabled()) console.debug('[Fleet:debug] fleet-stop:', { labelName: trimmed, dryRun: isDryRun, nodes: targetNodes.length, scoped: allowedNodeIds !== null }); + const targetNodes = confirmedStacksByNode ? nodes.filter(n => confirmedStacksByNode.has(n.id)) : nodes; + if (isDebugEnabled()) console.debug('[Fleet:debug] fleet-stop:', { labelName: trimmed, dryRun: isDryRun, nodes: targetNodes.length, scoped: confirmedStacksByNode !== null }); const results = await Promise.all(targetNodes.map(async (node): Promise => { + const allowedStacks = confirmedStacksByNode?.get(node.id); if (node.type === 'local') { // Match + stop runs in-process against the control's own Docker. The // helper shares the per-node `bulk:` lock with the per-label action @@ -1417,12 +1436,13 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: // failure (e.g. the compose dir is unreadable) degrades to a per-stack // error for this node only; the node is reachable, the stop just failed. try { - const outcome = await runLocalLabelStop(node.id, trimmed, isDryRun); + const outcome = await runLocalLabelStop(node.id, trimmed, isDryRun, allowedStacks); return { nodeId: node.id, nodeName: node.name, reachable: true, matched: outcome.matched, stackResults: outcome.stackResults }; } catch (err) { const errorMsg = getErrorMessage(err, 'Failed to stop local stacks'); const localLabel = db.getLabels(node.id).find(l => l.name === trimmed); - const localStacks = localLabel ? db.getStacksForLabel(localLabel.id, node.id) : []; + const localStacks = (localLabel ? db.getStacksForLabel(localLabel.id, node.id) : []) + .filter(s => !allowedStacks || allowedStacks.has(s)); return { nodeId: node.id, nodeName: node.name, reachable: true, matched: !!localLabel, stackResults: failAllStacks(localStacks, errorMsg), @@ -1445,7 +1465,11 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: const response = await fetch(`${target.apiUrl.replace(/\/$/, '')}/api/fleet-actions/labels/local-stop`, { method: 'POST', headers, - body: JSON.stringify({ labelName: trimmed, dryRun: isDryRun }), + body: JSON.stringify({ + labelName: trimmed, + dryRun: isDryRun, + ...(allowedStacks ? { stackNames: [...allowedStacks] } : {}), + }), signal: AbortSignal.timeout(60000), }); if (!response.ok) { @@ -1470,13 +1494,33 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: getErrorMessage(err, 'Failed to reach remote node') }; } })); - if (isDebugEnabled()) { - const matched = results.filter(r => r.matched).length; - const stopped = results.reduce((n, r) => n + r.stackResults.filter(s => s.success).length, 0); - const failed = results.reduce((n, r) => n + r.stackResults.filter(s => !s.success).length, 0); - console.debug('[Fleet:debug] fleet-stop complete:', { matched, stopped, failed }); + // A confirmed node that was deleted or unregistered between preview and + // execution would otherwise vanish from the response (the node-list filter + // above drops it), letting the remaining successes read as a clean stop. + // Surface each missing confirmed node as an unreachable failure, with its + // confirmed stacks marked failed so the run is reported as partial, not + // clean. + const missingResults: FleetStopNodeResult[] = []; + if (confirmedStacksByNode) { + const present = new Set(nodes.map(n => n.id)); + for (const [nodeId, stacks] of confirmedStacksByNode) { + if (!present.has(nodeId)) { + missingResults.push({ + nodeId, nodeName: `Node ${nodeId}`, reachable: false, matched: false, + stackResults: failAllStacks([...stacks], 'Node no longer exists'), + error: 'Node no longer exists', + }); + } + } } - res.json({ results }); + const allResults = [...results, ...missingResults]; + if (isDebugEnabled()) { + const matched = allResults.filter(r => r.matched).length; + const stopped = allResults.reduce((n, r) => n + r.stackResults.filter(s => s.success).length, 0); + const failed = allResults.reduce((n, r) => n + r.stackResults.filter(s => !s.success).length, 0); + console.debug('[Fleet:debug] fleet-stop complete:', { matched, stopped, failed, missing: missingResults.length }); + } + res.json({ results: allResults }); } catch (error) { console.error('[Fleet] fleet-stop error:', error); res.status(500).json({ error: getErrorMessage(error, 'Failed to run fleet stop') }); diff --git a/backend/src/routes/fleetActions.ts b/backend/src/routes/fleetActions.ts index 6be99f57..699196a6 100644 --- a/backend/src/routes/fleetActions.ts +++ b/backend/src/routes/fleetActions.ts @@ -27,15 +27,26 @@ fleetActionsRouter.post( async (req: Request, res: Response): Promise => { if (!requireAdmin(req, res)) return; if (!requireBody(req, res)) return; - const { labelName, dryRun } = req.body as { labelName?: unknown; dryRun?: unknown }; + const { labelName, dryRun, stackNames } = req.body as { labelName?: unknown; dryRun?: unknown; stackNames?: unknown }; if (typeof labelName !== 'string' || labelName.trim().length === 0) { res.status(400).json({ error: 'labelName is required' }); return; } + // Optional allowlist binding the stop to the stacks the control confirmed in + // its preview, so a stack labelled on this node after the preview is not + // stopped. Absent (e.g. a dry run) stops every currently label-matched stack. + let allowedStacks: Set | undefined; + if (stackNames !== undefined) { + if (!Array.isArray(stackNames) || !stackNames.every(s => typeof s === 'string')) { + res.status(400).json({ error: 'stackNames must be an array of strings' }); + return; + } + allowedStacks = new Set(stackNames as string[]); + } const nodeId = req.nodeId ?? 0; const trimmedLabel = labelName.trim(); try { - const outcome = await runLocalLabelStop(nodeId, trimmedLabel, dryRun === true); + const outcome = await runLocalLabelStop(nodeId, trimmedLabel, dryRun === true, allowedStacks); if (isDebugEnabled()) console.debug('[FleetActions:debug] local-stop:', { nodeId, dryRun: dryRun === true, matched: outcome.matched, stacks: outcome.stackResults.length }); const body: LabelLocalStopResponse = { matched: outcome.matched, results: outcome.stackResults }; res.json(body); diff --git a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx index 4588c57b..f243c3cf 100644 --- a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx @@ -230,9 +230,10 @@ it('gates Stop fleet on a resolved preview, carries the node/stack list into the await waitFor(() => { const stopCall = mockedFetch.mock.calls.find(c => c[0] === '/fleet/labels/fleet-stop'); - // The real stop binds execution to the nodes resolved in the preview: it - // carries their ids so a node that was unreachable then cannot be added later. - expect(JSON.parse(stopCall![1].body)).toEqual({ labelName: 'prod', dryRun: false, nodeIds: [1] }); + // The real stop binds execution to the exact node + stack list resolved in + // the preview, so neither a reconnecting node nor a newly labelled stack can + // be added later. + expect(JSON.parse(stopCall![1].body)).toEqual({ labelName: 'prod', dryRun: false, targets: [{ nodeId: 1, stackNames: ['web'] }] }); }); // Per-node results render in the card (the "ยท 1 stack" label is unique to the // results list, distinguishing it from the preview well). @@ -527,3 +528,38 @@ it('renders an unreachable fleet-stop node as unreachable, not "no matching labe expect(await screen.findByText(/edge-1 \(unreachable\)/)).toBeInTheDocument(); await waitFor(() => expect(toastWarning).toHaveBeenCalled()); }); + +it('reports a confirmed node that vanished as a failure, not a clean success', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/match-preview') { + return Promise.resolve(jsonResponse(200, { + matchedNodes: 1, matchedStacks: 1, unreachableNodes: 0, + perNode: [{ nodeId: 1, nodeName: 'central', reachable: true, labelExists: true, stackCount: 1, stackNames: ['web'] }], + })); + } + if (url === '/fleet/labels/fleet-stop') { + // One stopped node plus a confirmed node that disappeared from the + // registry: the backend returns its confirmed stacks as failures. + return Promise.resolve(jsonResponse(200, { + results: [ + { nodeId: 1, nodeName: 'central', reachable: true, matched: true, stackResults: [{ stackName: 'web', success: true }] }, + { nodeId: 2, nodeName: 'Node 2', reachable: false, matched: false, error: 'Node no longer exists', stackResults: [{ stackName: 'api', success: false, error: 'Node no longer exists' }] }, + ], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); + const stopBtn = screen.getByRole('button', { name: 'Stop fleet' }); + await waitFor(() => expect(stopBtn).toBeEnabled(), { timeout: 2000 }); + await user.click(stopBtn); + const dialog = await screen.findByRole('alertdialog'); + await user.click(within(dialog).getByRole('button', { name: 'Stop fleet' })); + // The vanished confirmed node renders as an unreachable failure and flips the + // outcome to a warning, so a partial stop never reads as a clean success. + expect(await screen.findByText(/Node 2 \(unreachable\)/)).toBeInTheDocument(); + await waitFor(() => expect(toastWarning).toHaveBeenCalled()); + expect(toastSuccess).not.toHaveBeenCalled(); +}); diff --git a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx index 372810f2..c2ece3ee 100644 --- a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx @@ -190,7 +190,7 @@ export function LabelFleetStopCard() { const blastTone = preview.kind === 'loading' || preview.kind === 'unavailable' ? 'muted' as const : undefined; - async function run(opts: { dryRun: boolean; nodeIds?: number[] }) { + async function run(opts: { dryRun: boolean; targets?: { nodeId: number; stackNames: string[] }[] }) { const trimmed = labelName.trim(); if (!trimmed) return; const verb = opts.dryRun ? 'Dry-running' : 'Stopping'; @@ -198,12 +198,13 @@ export function LabelFleetStopCard() { setRunning(true); setResults([]); try { - // The real stop carries the node ids the operator confirmed in the preview - // so execution cannot expand to a node that was unreachable then and has - // since reconnected. The dry run sends no allowlist and scans the fleet. + // The real stop carries the exact node + stack list the operator confirmed + // in the preview, so execution cannot expand to a node that has since + // reconnected nor to a stack that gained the label after the preview. The + // dry run sends no allowlist and scans the fleet. const res = await apiFetch('/fleet/labels/fleet-stop', { method: 'POST', - body: JSON.stringify({ labelName: trimmed, dryRun: opts.dryRun, ...(opts.nodeIds ? { nodeIds: opts.nodeIds } : {}) }), + body: JSON.stringify({ labelName: trimmed, dryRun: opts.dryRun, ...(opts.targets ? { targets: opts.targets } : {}) }), }); const body = await res.json().catch(() => ({})); toast.dismiss(toastId); @@ -373,7 +374,7 @@ export function LabelFleetStopCard() { description="Sencho will stop the stacks listed below. Node labels are not used by this action. Services will be unavailable until restarted." confirmLabel="Stop fleet" confirming={running} - onConfirm={() => run({ dryRun: false, nodeIds: (resolvedTargets ?? []).map(t => t.nodeId) })} + onConfirm={() => run({ dryRun: false, targets: (resolvedTargets ?? []).map(t => ({ nodeId: t.nodeId, stackNames: t.stackNames })) })} > {resolvedTargets && resolvedTargets.length > 0 && } @@ -431,15 +432,16 @@ function renderPreviewSection(preview: PreviewState, trimmed: string) { // The resolved node/stack list carried into the destructive confirm modal, so // the operator confirms against the concrete blast radius rather than a label -// name. The node set is bound to this list (the stop sends these node ids and -// the backend acts on no others); only the per-node stacks are re-matched by -// label at execution, where state can still drift between preview and confirm. +// name. Both axes are bound to this list: the stop sends these node ids and the +// exact stacks per node, and the backend stops only stacks that are still +// label-matched AND in this set, so drift between preview and confirm (a node +// reconnecting, a stack newly labelled) cannot widen the blast radius. function ResolvedTargetsList({ targets }: { targets: ResolvedTarget[] }) { const stackCount = targets.reduce((n, t) => n + t.stackNames.length, 0); return (
- Will stop {stackCount} stack{stackCount === 1 ? '' : 's'} across {targets.length} node{targets.length === 1 ? '' : 's'}. The stacks on each node are re-matched by label at execution; the node set is fixed to those listed. + Will stop {stackCount} stack{stackCount === 1 ? '' : 's'} across {targets.length} node{targets.length === 1 ? '' : 's'}. Only these stacks are stopped, and only while they still carry the label.