fix: condition --volumes in downStack() on the removeVolumes option (#1764)

* fix: condition --volumes in downStack() on the removeVolumes option

ComposeService.downStack() hardcoded --volumes on every stack delete,
ignoring the "Also remove associated volumes" checkbox and destroying
volumes the operator asked to keep. The sibling Take-down path (runDown)
already conditions --volumes correctly.

- Add options?: { removeVolumes?: boolean } to downStack()
- Default to data-preserving (no --volumes when option absent)
- DeletedStackDeletionService reads the persisted intent flag
- Templates rollback passes removeVolumes: true (clean up failed deploy)
- Blueprint withdraw passes removeVolumes: false (volumes preserved)

* docs: update Delete row to reflect conditional volume removal

The Delete row now describes that volumes are removed only when the
operator opts in, matching the behavior introduced by the downStack fix.

* fix: add capability gate for delete pruneVolumes and fix QA findings

Four P0 issues found in live QA:

P0-1/P0-4 - No capability gate on delete's pruneVolumes:
  Add stack-delete-prune-volumes capability so the frontend hides the
  "Also remove associated volumes" checkbox on nodes that don't support
  conditional volume removal on delete. Without this, an operator on an
  old node sees a VOLUMES KEPT promise the old node silently breaks.
  Frontend-only gate: no API or proxy gate because the old node's
  fallback (always destroy) is correct for the checked case.

P0-2 - Checkbox state leaked across dialogs:
  Reset pruneVolumes in onConfirm before calling the parent, so a
  previously checked box doesn't appear pre-checked when the dialog
  opens for a different stack.

P0-3 - Delete not bound to the active node:
  Capture activeNode.id at delete time and pass it as an explicit
  nodeId to apiFetch, matching the Take Down pattern. Without this,
  switching the active node while the dialog is open silently deletes
  the wrong stack on the wrong node.

* fix: update test assertions for nodeId binding and showVolumeOption gate

P0-3 added nodeId to apiFetch DELETE calls — two useStackActions tests
now expect the parameter. P0-1 gated the volume checkbox behind
showVolumeOption — the confirming test now passes the prop.

* fix: gate volume hint on showVolumeOption to prevent false promise

On nodes without stack-delete-prune-volumes, volumes are always
destroyed. Showing VOLUMES KEPT was a lie. Now the hint is hidden
entirely when the capability is absent.

* fix: gate delete against nodes that cannot guarantee volume preservation

Hiding the checkbox and the misleading hint stopped the false promise but
not the data loss: an unchecked delete against a node lacking
stack-delete-prune-volumes still reached that node and its downStack()
still destroyed volumes unconditionally, now with no warning at all.

- remoteNodeProxy.ts: block an unacknowledged DELETE /stacks/:name
  (no pruneVolumes=true) to a remote lacking the capability, mirroring
  the existing removeVolumes gate on the down route. An explicit
  pruneVolumes=true always proxies through since that matches what an
  unsupported remote does anyway.
- DeleteStackDialog: rework around a three-state model (supported /
  unsupported / unknown) instead of a boolean. A node whose capabilities
  have not been confirmed (meta not yet fetched, or a failed probe) is
  now treated like a supported node, not forced onto the destructive
  path just because its state is unresolved.
- Fix deleteStack's error toast, which surfaced the raw JSON response
  body instead of the parsed error message.
- Fix CreateStackDialog's orphan-stack rollback (docker-run import),
  which silently no-op'd against a node requiring acknowledgement.
- Update node-compatibility.mdx and stack-management.mdx to describe
  the new gate.

* test: advertise stack-delete-prune-volumes on the scoped-evidence fixtures

These mock remotes simulate nodes capable enough to run scoped-stack-auth-evidence
RBAC and were pinned before stack-delete-prune-volumes existed, so the new delete
gate now blocked their unacknowledged DELETE calls before reaching the mock server,
failing the grant-tuple-cleanup assertions the tests actually check.
This commit is contained in:
Anso
2026-08-04 13:04:21 -04:00
committed by GitHub
parent 0b046bfa52
commit 92d974b13e
21 changed files with 523 additions and 40 deletions
+15 -1
View File
@@ -11,6 +11,7 @@ import { CreateStackDialog, type CreateMode } from './EditorLayout/CreateStackDi
import { AdoptExistingDialog } from './EditorLayout/AdoptExistingDialog';
import { EditorView } from './EditorLayout/EditorView';
import { ShellOverlays } from './EditorLayout/ShellOverlays';
import type { VolumePreservationOnDelete } from './EditorLayout/DeleteStackDialog';
import { classifyFailedGate } from './EditorLayout/failed-gate-recovery';
import { useEditorViewState } from './EditorLayout/hooks/useEditorViewState';
import { useStackListState } from './EditorLayout/hooks/useStackListState';
@@ -37,7 +38,7 @@ import {
import { SENCHO_OPEN_LOGS_EVENT, SENCHO_OPEN_STACK_EVENT } from '@/lib/events';
import type { SenchoOpenLogsDetail, SenchoOpenStackDetail } from '@/lib/events';
import { useNodes } from '@/context/NodeContext';
import { STACK_DOWN_REMOVE_VOLUMES_CAPABILITY } from '@/lib/capabilities';
import { STACK_DOWN_REMOVE_VOLUMES_CAPABILITY, STACK_DELETE_PRUNE_VOLUMES_CAPABILITY } from '@/lib/capabilities';
import { useAuth } from '@/context/AuthContext';
import { useDeployFeedback } from '@/context/DeployFeedbackContext';
import { useTrivyStatus } from '@/hooks/useTrivyStatus';
@@ -85,6 +86,17 @@ const ResourcesView = lazy(() => import('./ResourcesView'));
const NetworkingView = lazy(() => import('./networking/NetworkingView').then(m => ({ default: m.NetworkingView })));
const GlobalObservabilityView = lazy(() => import('./GlobalObservabilityView').then(m => ({ default: m.GlobalObservabilityView })));
/**
* NodeContext records an unfetched or failed /api/meta as an empty capability list, so an
* empty list means "not confirmed either way", never "confirmed without the capability".
* Reporting that as 'unsupported' would force the destructive delete default onto a node
* that may well preserve volumes, so it maps to 'unknown' instead.
*/
function resolveDeleteVolumePreservation(capabilities: string[] | undefined): VolumePreservationOnDelete {
if (capabilities == null || capabilities.length === 0) return 'unknown';
return capabilities.includes(STACK_DELETE_PRUNE_VOLUMES_CAPABILITY) ? 'supported' : 'unsupported';
}
export default function EditorLayout() {
const { isAdmin, can, permissions, permissionsStatus } = useAuth();
const { status: trivy } = useTrivyStatus();
@@ -168,6 +180,7 @@ export default function EditorLayout() {
const { nodes, activeNode, setActiveNode, hasCapability, activeNodeMeta, isLoading: nodesLoading } = useNodes();
const canOfferVolumeRemoval =
activeNodeMeta?.capabilities.includes(STACK_DOWN_REMOVE_VOLUMES_CAPABILITY) === true;
const deleteVolumePreservation = resolveDeleteVolumePreservation(activeNodeMeta?.capabilities);
// One-shot boot milestone: the app shell has mounted. Developer mode gates the
// hydration-timing overlay for the active node; it follows node switches.
@@ -1087,6 +1100,7 @@ export default function EditorLayout() {
composeReapply={composeReapply}
canSaveAndReapply={canSaveAndReapply}
canOfferVolumeRemoval={canOfferVolumeRemoval}
deleteVolumePreservation={deleteVolumePreservation}
onOpenFleetNodeUpdates={() => {
if (isMobile) {
navigateMobileAware('fleet');
@@ -316,9 +316,17 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
});
if (!saveResponse.ok) {
// Roll back the empty stack we just created so we don't leave an orphan.
await apiFetch(`/stacks/${encodeURIComponent(stackName)}`, { method: 'DELETE' }).catch((cleanupError) => {
// pruneVolumes=true: nothing has been deployed yet, so there is no volume
// to preserve, and passing it avoids nodes that require an explicit
// acknowledgement to delete without a guaranteed-preserving capability.
try {
const cleanupRes = await apiFetch(`/stacks/${encodeURIComponent(stackName)}?pruneVolumes=true`, { method: 'DELETE' });
if (!cleanupRes.ok) {
console.error('Failed to roll back orphan stack after save failure:', await cleanupRes.text());
}
} catch (cleanupError) {
console.error('Failed to roll back orphan stack after save failure:', cleanupError);
});
}
createdStack = false;
throw new Error('Could not save the converted YAML. Please try again.');
}
@@ -2,10 +2,21 @@ import { useState } from 'react';
import { ConfirmModal } from '../ui/modal';
import { Checkbox } from '../ui/checkbox';
/**
* Whether the active node is confirmed (via its fetched capabilities) to preserve volumes
* on delete unless the operator opts in to removing them. 'unknown' covers both "not
* fetched yet" and "fetch failed": a node we simply have not confirmed must not be assumed
* incapable, so it requests preservation like a 'supported' node (a genuinely stale remote
* rejects that request outright instead of silently destroying data). Only 'unsupported'
* forces removal.
*/
export type VolumePreservationOnDelete = 'supported' | 'unsupported' | 'unknown';
export interface DeleteStackDialogProps {
open: boolean;
onOpenChange: (open: boolean) => void;
stackName: string | null;
volumePreservation?: VolumePreservationOnDelete;
onConfirm: (pruneVolumes: boolean) => void | Promise<void>;
/** True while the stack delete request owns the flow (from stackActionMap). */
confirming?: boolean;
@@ -15,10 +26,20 @@ export function DeleteStackDialog({
open,
onOpenChange,
stackName,
volumePreservation = 'unknown',
onConfirm,
confirming = false,
}: DeleteStackDialogProps) {
const [pruneVolumes, setPruneVolumes] = useState(false);
const showVolumeOption = volumePreservation === 'supported';
const confirmedUnsupported = volumePreservation === 'unsupported';
// Single source of truth for what confirming will do: the operator's choice when the
// node offers one, forced removal only on a node confirmed unable to preserve volumes.
const willRemoveVolumes = showVolumeOption ? pruneVolumes : confirmedUnsupported;
let volumeHint = 'VOLUMES KEPT';
if (confirmedUnsupported) volumeHint = 'VOLUMES WILL BE REMOVED';
else if (willRemoveVolumes) volumeHint = 'VOLUMES PRUNED';
const handleOpenChange = (next: boolean) => {
if (!next) setPruneVolumes(false);
@@ -48,24 +69,34 @@ export function DeleteStackDialog({
)
}
description={`Confirm deletion of ${stackName ?? 'stack'}.`}
hint={pruneVolumes ? 'VOLUMES PRUNED' : 'VOLUMES KEPT'}
hint={volumeHint}
confirmLabel="Delete"
busyConfirmLabel="Deleting..."
confirming={confirming}
onConfirm={() => onConfirm(pruneVolumes)}
onConfirm={() => {
setPruneVolumes(false);
onConfirm(willRemoveVolumes);
}}
>
<p className="text-sm text-muted-foreground">This action cannot be undone.</p>
<div className="flex items-center gap-2">
<Checkbox
id="prune-volumes"
checked={pruneVolumes}
disabled={confirming}
onCheckedChange={(v) => setPruneVolumes(v === true)}
/>
<label htmlFor="prune-volumes" className="text-sm text-muted-foreground cursor-pointer select-none">
Also remove associated volumes
</label>
</div>
{showVolumeOption && (
<div className="flex items-center gap-2">
<Checkbox
id="prune-volumes"
checked={pruneVolumes}
disabled={confirming}
onCheckedChange={(v) => setPruneVolumes(v === true)}
/>
<label htmlFor="prune-volumes" className="text-sm text-muted-foreground cursor-pointer select-none">
Also remove associated volumes
</label>
</div>
)}
{confirmedUnsupported && (
<p className="text-sm text-destructive">
This node can&apos;t preserve volumes on delete. Any volumes associated with this stack will be removed too.
</p>
)}
</ConfirmModal>
);
}
@@ -6,7 +6,7 @@ import { UpdateReadinessDialog } from '../stack/UpdateReadinessDialog';
import { SelfStackProtectedDialog } from '../stack/SelfStackProtectedDialog';
import { LocalUpdateConfirmDialog } from '../FleetView/LocalUpdateConfirmDialog';
import { ReconnectingOverlay } from '../FleetView/ReconnectingOverlay';
import { DeleteStackDialog } from './DeleteStackDialog';
import { DeleteStackDialog, type VolumePreservationOnDelete } from './DeleteStackDialog';
import { TakeDownStackDialog } from './TakeDownStackDialog';
import { UnsavedChangesDialog } from './UnsavedChangesDialog';
import { StackAlertSheet } from '../StackAlertSheet';
@@ -41,6 +41,7 @@ interface ShellOverlaysProps {
composeReapply: ReturnType<typeof useComposeReapplyAction>;
canSaveAndReapply: boolean;
canOfferVolumeRemoval: boolean;
deleteVolumePreservation: VolumePreservationOnDelete;
onOpenFleetNodeUpdates: () => void;
}
@@ -61,6 +62,7 @@ export function ShellOverlays({
composeReapply,
canSaveAndReapply,
canOfferVolumeRemoval,
deleteVolumePreservation,
onOpenFleetNodeUpdates,
}: ShellOverlaysProps) {
const {
@@ -93,6 +95,7 @@ export function ShellOverlays({
open={deleteDialogOpen}
onOpenChange={(open) => { if (!open) closeDeleteDialog(); }}
stackName={stackToDelete}
volumePreservation={deleteVolumePreservation}
onConfirm={stackActions.deleteStack}
confirming={isDeleteConfirming}
/>
@@ -1,10 +1,13 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { ComponentProps } from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { CreateStackDialog } from '../CreateStackDialog';
import { apiFetch } from '@/lib/api';
vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() }));
vi.mock('@/components/ui/toast-store', () => ({ toast: { error: vi.fn(), dismiss: vi.fn() } }));
vi.mock('@/components/ui/toast-store', () => ({
toast: { error: vi.fn(), success: vi.fn(), loading: vi.fn(), dismiss: vi.fn() },
}));
vi.mock('@/context/NodeContext', () => ({
useNodes: () => ({ activeNode: { id: 1, name: 'local' } }),
}));
@@ -54,4 +57,28 @@ describe('CreateStackDialog', () => {
renderOpen();
expect(screen.queryByRole('button', { name: /adopt existing files instead/i })).toBeNull();
});
it('rolls back an orphaned stack with pruneVolumes=true when saving converted YAML fails', async () => {
const fetchMock = vi.mocked(apiFetch);
fetchMock
.mockResolvedValueOnce(new Response(JSON.stringify({ yaml: 'services:\n app:\n image: nginx' }), { status: 200 }))
.mockResolvedValueOnce(new Response(null, { status: 200 })) // POST /stacks create
.mockResolvedValueOnce(new Response('save failed', { status: 500 })) // PUT save
.mockResolvedValueOnce(new Response(null, { status: 200 })); // DELETE rollback
renderOpen();
fireEvent.click(screen.getByRole('tab', { name: 'From Docker Run' }));
fireEvent.change(screen.getByLabelText('Paste your docker run command'), {
target: { value: 'docker run -d --name nginx -p 8080:80 nginx:latest' },
});
fireEvent.click(screen.getByRole('button', { name: /convert/i }));
await waitFor(() => expect(screen.getByText('compose.yaml preview')).toBeVisible());
fireEvent.change(screen.getByLabelText('Stack Name'), { target: { value: 'nginx' } });
fireEvent.click(screen.getByRole('button', { name: /create stack/i }));
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(4));
expect(fetchMock).toHaveBeenLastCalledWith('/stacks/nginx?pruneVolumes=true', { method: 'DELETE' });
});
});
@@ -1,5 +1,5 @@
import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import { render, screen, fireEvent } from '@testing-library/react';
import { DeleteStackDialog } from '../DeleteStackDialog';
const LONG_STACK_NAME = 'this-is-a-very-long-stack-name-that-should-not-push-actions-off-screen';
@@ -35,6 +35,7 @@ describe('DeleteStackDialog', () => {
open
onOpenChange={vi.fn()}
stackName="web"
volumePreservation="supported"
onConfirm={vi.fn()}
confirming
/>,
@@ -44,4 +45,75 @@ describe('DeleteStackDialog', () => {
expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled();
expect(screen.getByRole('checkbox')).toBeDisabled();
});
it('warns instead of offering a checkbox when the node is confirmed unable to preserve volumes', () => {
render(
<DeleteStackDialog
open
onOpenChange={vi.fn()}
stackName="web"
volumePreservation="unsupported"
onConfirm={vi.fn()}
/>,
);
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
expect(screen.getByText(/can't preserve volumes on delete/i)).toBeVisible();
expect(screen.getByText('VOLUMES WILL BE REMOVED')).toBeVisible();
});
it('confirms with pruneVolumes: true on a node confirmed unable to preserve volumes, without the operator opting in', () => {
const onConfirm = vi.fn();
render(
<DeleteStackDialog
open
onOpenChange={vi.fn()}
stackName="web"
volumePreservation="unsupported"
onConfirm={onConfirm}
/>,
);
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));
expect(onConfirm).toHaveBeenCalledWith(true);
});
it('confirms with pruneVolumes: false by default when the node can preserve volumes', () => {
const onConfirm = vi.fn();
render(
<DeleteStackDialog
open
onOpenChange={vi.fn()}
stackName="web"
volumePreservation="supported"
onConfirm={onConfirm}
/>,
);
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));
expect(onConfirm).toHaveBeenCalledWith(false);
});
it('requests preservation (pruneVolumes: false) and shows no destructive warning when node support is not yet confirmed', () => {
const onConfirm = vi.fn();
render(
<DeleteStackDialog
open
onOpenChange={vi.fn()}
stackName="web"
volumePreservation="unknown"
onConfirm={onConfirm}
/>,
);
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
expect(screen.queryByText(/can't preserve volumes on delete/i)).not.toBeInTheDocument();
expect(screen.getByText('VOLUMES KEPT')).toBeVisible();
fireEvent.click(screen.getByRole('button', { name: 'Delete' }));
expect(onConfirm).toHaveBeenCalledWith(false);
});
});
@@ -1729,7 +1729,7 @@ describe('useStackActions.deleteStack', () => {
await result.current.deleteStack(false);
});
expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml', { method: 'DELETE' });
expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml', { method: 'DELETE', nodeId: 1 });
expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null);
expect(navState.setActiveView).toHaveBeenCalledWith('dashboard');
expect(navState.setActiveView).toHaveBeenCalledTimes(1);
@@ -1738,6 +1738,21 @@ describe('useStackActions.deleteStack', () => {
expect(stackListState.refreshStacks).toHaveBeenCalled();
});
it('passes pruneVolumes=true through unconditionally, including on nodes without the capability', async () => {
vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 }));
const { result } = setup({
overlay: { stackToDelete: 'web.yml' },
stackList: { selectedFile: 'web.yml', files: ['web.yml'] },
navState: { activeView: 'editor' },
});
await act(async () => {
await result.current.deleteStack(true);
});
expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml?pruneVolumes=true', { method: 'DELETE', nodeId: 1 });
});
it('clears isFileLoading on delete-leave so the URL writer is not blocked', async () => {
vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 }));
const { result, editorState } = setup({
@@ -1766,7 +1781,7 @@ describe('useStackActions.deleteStack', () => {
await result.current.deleteStack(false);
});
expect(apiFetch).toHaveBeenCalledWith('/stacks/web', { method: 'DELETE' });
expect(apiFetch).toHaveBeenCalledWith('/stacks/web', { method: 'DELETE', nodeId: 1 });
expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null);
expect(navState.setActiveView).toHaveBeenCalledWith('dashboard');
expect(onDeletedOpenStack).toHaveBeenCalledTimes(1);
@@ -1832,6 +1847,30 @@ describe('useStackActions.deleteStack', () => {
expect(toast.error).toHaveBeenCalled();
});
it('surfaces the parsed error message, not the raw JSON body, on a non-OK delete response', async () => {
vi.mocked(apiFetch).mockResolvedValue(
new Response(
JSON.stringify({
error: 'This node cannot guarantee volumes are preserved on delete.',
code: 'capability_unavailable',
}),
{ status: 400 },
),
);
const { toast } = await import('@/components/ui/toast-store');
const { result } = setup({
overlay: { stackToDelete: 'web.yml' },
stackList: { selectedFile: 'web.yml', files: ['web.yml'] },
navState: { activeView: 'editor' },
});
await act(async () => {
await result.current.deleteStack(true);
});
expect(toast.error).toHaveBeenCalledWith('This node cannot guarantee volumes are preserved on delete.');
});
it('does not navigate on a self-stack-protected response', async () => {
vi.mocked(apiFetch).mockResolvedValue(
new Response(JSON.stringify({ code: 'self_stack_protected' }), { status: 409 }),
@@ -1952,12 +1952,13 @@ export function useStackActions(options: UseStackActionsOptions) {
const deleteKey = resolveStackFileKey(stackListState.files, stackToDelete);
const canonicalName = deleteKey.replace(/\.(yml|yaml)$/, '');
if (stackListState.isStackBusy(deleteKey)) return;
const opNodeId = activeNode?.id ?? null;
stackListState.setStackAction(deleteKey, 'delete');
try {
const url = pruneVolumes
? `/stacks/${stackToDelete}?pruneVolumes=true`
: `/stacks/${stackToDelete}`;
const response = await apiFetch(url, { method: 'DELETE' });
const response = await apiFetch(url, { method: 'DELETE', nodeId: opNodeId });
if (!response.ok) {
const errText = await response.text();
if (isSelfStackProtectedResponse(errText, response.status)) {
@@ -1965,7 +1966,7 @@ export function useStackActions(options: UseStackActionsOptions) {
overlayState.closeDeleteDialog();
return;
}
throw new Error(errText || 'Failed to delete stack');
throw parseStackActionError(errText, 'Failed to delete stack', response.status);
}
toast.success('Stack deleted successfully!');
overlayState.closeDeleteDialog();
+2
View File
@@ -37,6 +37,7 @@ export const CAPABILITIES = [
'compose-storage',
'cross-node-rbac',
'stack-down-remove-volumes',
'stack-delete-prune-volumes',
'guided-external-network-preflight',
'service-scoped-update',
'service-scoped-stack-alert',
@@ -52,6 +53,7 @@ export const HOST_CONSOLE_CAPABILITY = 'host-console' as const satisfies Capabil
export const HOST_CONSOLE_COMMUNITY_CAPABILITY = 'host-console-community' as const satisfies Capability;
export const STACK_DOWN_REMOVE_VOLUMES_CAPABILITY = 'stack-down-remove-volumes' as const satisfies Capability;
export const STACK_DELETE_PRUNE_VOLUMES_CAPABILITY = 'stack-delete-prune-volumes' as const satisfies Capability;
export const GUIDED_EXTERNAL_NETWORK_PREFLIGHT_CAPABILITY = 'guided-external-network-preflight' as const satisfies Capability;
export const SERVICE_SCOPED_UPDATE_CAPABILITY = 'service-scoped-update' as const satisfies Capability;
export const SERVICE_SCOPED_STACK_ALERT_CAPABILITY = 'service-scoped-stack-alert' as const satisfies Capability;