mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 18:56:53 +00:00
fix: harden deploy/update concurrency and node-targeting safety (#1390)
* fix: harden deploy/update concurrency and node-targeting safety Release stabilization for deploy/update operational safety. Per-stack operation locking is now global. Background lifecycle paths (scheduler auto stop/down/start/backup/update, webhook execute, Git source auto-deploy, image auto-update, label bulk actions, fleet snapshot redeploy, and mesh redeploy) acquire the per-node, per-stack lock through a new StackOpLockService.runExclusive helper and skip rather than race a manual deploy/update/rollback/backup on the same stack and node. Skips surface honestly (a failed scheduled run, a recorded webhook failure, a per-stack batch result, or a thrown error) instead of a silent no-op. Update readiness and policy-bypass now run against the node captured when the dialog opened, not the live active node, so switching nodes while a dialog is open cannot retarget the update or the bypass retry. Rollback readiness no longer presents a moving-tag or unpinned image as a ready image revert. Restoring files does not revert a moving tag, so those stacks read as partial, and the rollback success message states that the compose and env files were restored. * fix: lock blueprint reconcile against manual ops and correct rollback wording Follow-up to the deploy/update safety hardening, closing two more gaps from a verification pass. BlueprintService.deployLocal and withdrawLocal called ComposeService directly, so blueprint reconciliation could race a manual deploy/update/rollback/backup on an owned stack. Both now run their compose lifecycle call through StackOpLockService.runExclusive and skip (recorded as a failed reconcile, retried on the next cycle) on conflict. The withdraw holds the lock across both the compose down and the directory delete so neither races a manual operation. The runtime rollback messages overstated recovery: a rollback restores the compose and env files and recreates containers, but does not revert an image behind a moving tag. The auto-rollback deploy-progress output, the recovery panel and chip, the failure toasts, and the manual rollback route message now state that the compose and env files were restored, with the matching OpenAPI example and atomic-deployments doc updated. * fix: acquire stack lock before blueprint deploy mutates compose and marker files Local blueprint deploy wrote the compose and marker files and ran the policy assert before acquiring the per-stack lock; the lock only wrapped the deploy itself. A reconcile could therefore rewrite an owned stack's files while a manual deploy/update/rollback/backup was running. The lock now wraps the whole critical section (create, write compose, write marker, policy assert, deploy), so on conflict nothing is written and the reconcile records a failed outcome. Adds a test asserting a deploy under a held lock records failed, writes no marker file, and leaves the manual lock untouched. * fix: make remote blueprint apply atomic under the receiving node's stack lock Remote blueprint deploy wrote the compose and marker files to the target node via separate HTTP calls and only locked on the final deploy, so the file writes could race a manual operation on that node. A node's operation lock is process-local and cannot be held by the hub across HTTP calls, so the locked create/write/deploy now runs on the receiving node. The locked critical section is extracted into BlueprintService.applyLocalUnderLock and exposed via POST /api/blueprints/apply-local. The hub posts the blueprint to that endpoint in one call; the receiving node runs create + write compose+marker + deploy under its own per-stack lock. Older nodes without the route answer 404 and fall back to the legacy multi-call flow. The endpoint is gated by paid tier and the same per-stack stack:edit and stack:deploy permissions as the PUT-compose + deploy it bundles, validates the stack name, compose size, and marker structure, and returns 409 on a lock conflict without writing anything. Adds tests for the atomic single-call path, the 404 legacy fallback, the 409 lock-conflict mapping, the route validation and permission paths, and the write-compose-then-marker-then-deploy ordering of the shared locked apply. * fix(deps): bump undici to 7.28.0 to clear high-severity advisory The frontend CI npm audit gate (--audit-level=high) failed on a transitive undici 7.25.0 (a dev-only dependency via jsdom): TLS certificate validation bypass (GHSA-vmh5-mc38-953g) and cross-user cache information disclosure (GHSA-pr7r-676h-xcf6). Bumping undici within jsdom's existing ^7.25.0 range to 7.28.0 clears the high-severity advisory and unblocks the frontend job. Lockfile only; no direct dependency or source change.
This commit is contained in:
@@ -62,7 +62,7 @@ export function RecoveryChip({
|
||||
</p>
|
||||
<p className="mt-0.5 break-words text-xs text-muted-foreground">
|
||||
{result.errorMessage ?? 'The operation did not complete.'}
|
||||
{result.rolledBack && ' · rolled back to previous version'}
|
||||
{result.rolledBack && ' · restored previous compose and env files'}
|
||||
</p>
|
||||
<div className="mt-1.5">
|
||||
<RecoveryClassification result={result} />
|
||||
|
||||
@@ -63,7 +63,7 @@ export function RecoveryPanel({
|
||||
</p>
|
||||
<p className="mt-0.5 truncate text-xs text-muted-foreground" title={result.errorMessage}>
|
||||
{result.errorMessage ?? 'The operation did not complete.'}
|
||||
{result.rolledBack && ' · rolled back to previous version'}
|
||||
{result.rolledBack && ' · restored previous compose and env files'}
|
||||
</p>
|
||||
<div className="mt-1.5">
|
||||
<RecoveryClassification result={result} />
|
||||
|
||||
@@ -96,6 +96,7 @@ export function ShellOverlays({
|
||||
<UpdateReadinessDialog
|
||||
open={updateReadiness !== null}
|
||||
stackName={updateReadiness?.stackName ?? ''}
|
||||
nodeId={updateReadiness?.nodeId ?? null}
|
||||
onCancel={() => setUpdateReadiness(null)}
|
||||
onProceed={() => updateReadiness?.proceed()}
|
||||
/>
|
||||
|
||||
@@ -18,6 +18,10 @@ type PolicyBlock = {
|
||||
stackFile: string;
|
||||
action: PolicyBlockableAction;
|
||||
payload: PolicyBlockPayload;
|
||||
// Node captured when the block was raised, so a bypass retry (deploy, update,
|
||||
// or rollback) targets that node even if the active node changes while the
|
||||
// dialog is open.
|
||||
nodeId: number | null;
|
||||
};
|
||||
type Container = { id: string; name: string };
|
||||
|
||||
@@ -90,10 +94,13 @@ export function useOverlayState() {
|
||||
const [policyBypassing, setPolicyBypassing] = useState(false);
|
||||
|
||||
// Pre-update readiness dialog. `proceed` runs the actual update when the
|
||||
// user confirms; opened by useStackActions.requestStackUpdate.
|
||||
// user confirms; opened by useStackActions.requestStackUpdate. `nodeId` is
|
||||
// captured at open time so both the readiness fetch and the update run against
|
||||
// the same node even if the active node changes while the dialog is open.
|
||||
const [updateReadiness, setUpdateReadiness] = useState<{
|
||||
stackName: string;
|
||||
stackFile: string;
|
||||
nodeId: number | null;
|
||||
proceed: () => void;
|
||||
} | null>(null);
|
||||
|
||||
|
||||
@@ -513,6 +513,19 @@ describe('useStackActions.bypassPolicyAndRetry', () => {
|
||||
expect(urls).toContain('/stacks/web.yml/rollback?ignorePolicy=true');
|
||||
});
|
||||
|
||||
it('retries on the node captured in the policy block, not the live active node', async () => {
|
||||
vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // update OK
|
||||
vi.mocked(apiFetch).mockResolvedValueOnce(new Response('[]', { status: 200 })); // containers refresh
|
||||
const { result } = setup({
|
||||
activeNode: { id: 1, type: 'local' } as never, // active node has since moved to 1
|
||||
overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload, nodeId: 9 } as never },
|
||||
});
|
||||
await result.current.bypassPolicyAndRetry();
|
||||
const updateCall = vi.mocked(apiFetch).mock.calls.find(c => String(c[0]).includes('/update?ignorePolicy=true'));
|
||||
expect(updateCall).toBeDefined();
|
||||
expect((updateCall![1] as { nodeId?: number | null }).nodeId).toBe(9);
|
||||
});
|
||||
|
||||
it('does nothing when no policy block is stored', async () => {
|
||||
const { result } = setup({ overlay: { policyBlock: null as never } });
|
||||
await result.current.bypassPolicyAndRetry();
|
||||
|
||||
@@ -660,14 +660,16 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
};
|
||||
|
||||
// Parse a 409 body for a scan-policy block. When it is one, record it (with
|
||||
// the originating action and file so the bypass retries the right endpoint)
|
||||
// so PolicyBlockDialog can open, and return the policy name. Returns null
|
||||
// when the body is not a policy block (e.g. a stack-op-in-progress 409).
|
||||
// the originating action, file, and the node the operation targeted so the
|
||||
// bypass retries the right endpoint on the right node) so PolicyBlockDialog
|
||||
// can open, and return the policy name. Returns null when the body is not a
|
||||
// policy block (e.g. a stack-op-in-progress 409).
|
||||
const tryOpenPolicyBlock = (
|
||||
rawBody: string,
|
||||
stackName: string,
|
||||
stackFile: string,
|
||||
action: PolicyBlockableAction,
|
||||
opNodeId: number | null,
|
||||
): string | null => {
|
||||
let parsed: PolicyBlockPayload | null = null;
|
||||
try {
|
||||
@@ -676,7 +678,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
/* not JSON */
|
||||
}
|
||||
if (parsed && parsed.policy && Array.isArray(parsed.violations)) {
|
||||
overlayState.setPolicyBlock({ stackName, stackFile, action, payload: parsed });
|
||||
overlayState.setPolicyBlock({ stackName, stackFile, action, payload: parsed, nodeId: opNodeId });
|
||||
return parsed.policy.name;
|
||||
}
|
||||
return null;
|
||||
@@ -712,7 +714,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
toast.error(message);
|
||||
return { ok: false, errorMessage: message };
|
||||
}
|
||||
const blockedBy = tryOpenPolicyBlock(rawBody, stackName, stackFile, 'deploy');
|
||||
const blockedBy = tryOpenPolicyBlock(rawBody, stackName, stackFile, 'deploy', opNodeId ?? null);
|
||||
if (blockedBy) {
|
||||
const message = `Deploy blocked by policy "${blockedBy}"`;
|
||||
toast.error(message);
|
||||
@@ -747,7 +749,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const errorMessage = deployError.message || 'Failed to deploy stack';
|
||||
toast.error(
|
||||
deployError.rolledBack === true
|
||||
? `${errorMessage} - automatically rolled back to previous version.`
|
||||
? `${errorMessage} - automatically restored the previous compose and env files.`
|
||||
: errorMessage,
|
||||
);
|
||||
recordActionFailureFor(stackFile, stackName, 'deploy', startedAt, errorMessage, deployError.rolledBack === true, deployError.failure);
|
||||
@@ -831,19 +833,20 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const bypassPolicyAndRetry = async () => {
|
||||
const policyBlock = overlayState.policyBlock;
|
||||
if (!policyBlock) return;
|
||||
const { stackName, stackFile, action } = policyBlock;
|
||||
// Retry on the node the block was raised against, not the live active node,
|
||||
// which may have changed while the dialog was open.
|
||||
const { stackName, stackFile, action, nodeId: opNodeId } = policyBlock;
|
||||
const existingFile = stackListState.files.includes(stackFile)
|
||||
? stackFile
|
||||
: (stackListState.files.find(f => f.replace(/\.(yml|yaml)$/, '') === stackName) ?? stackFile);
|
||||
overlayState.setPolicyBypassing(true);
|
||||
try {
|
||||
if (action === 'update') {
|
||||
await runStackAction(existingFile, 'update', 'update', 'running', 'Stack updated successfully!', true);
|
||||
await runStackAction(existingFile, 'update', 'update', 'running', 'Stack updated successfully!', true, opNodeId);
|
||||
} else if (action === 'rollback') {
|
||||
await rollbackStack(true);
|
||||
await rollbackStack(true, opNodeId);
|
||||
} else {
|
||||
stackListState.setStackAction(existingFile, 'deploy');
|
||||
const opNodeId = activeNode?.id ?? null;
|
||||
try {
|
||||
await runWithLog({ stackName, action: 'deploy', nodeId: opNodeId }, (started, ds) =>
|
||||
runDeploy(stackName, existingFile, true, started, ds, opNodeId),
|
||||
@@ -858,7 +861,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
}
|
||||
};
|
||||
|
||||
const rollbackStack = async (ignorePolicy = false) => {
|
||||
const rollbackStack = async (ignorePolicy = false, opNodeId: number | null = activeNode?.id ?? null) => {
|
||||
if (!stackListState.selectedFile || stackListState.isStackBusy(stackListState.selectedFile))
|
||||
return;
|
||||
const stackFile = stackListState.selectedFile;
|
||||
@@ -870,7 +873,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const path = ignorePolicy
|
||||
? `/stacks/${stackFile}/rollback?ignorePolicy=true`
|
||||
: `/stacks/${stackFile}/rollback`;
|
||||
const res = await apiFetch(path, { method: 'POST' });
|
||||
const res = await apiFetch(path, { method: 'POST', nodeId: opNodeId });
|
||||
if (!res.ok) {
|
||||
const rawBody = await res.text();
|
||||
if (res.status === 409) {
|
||||
@@ -880,7 +883,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
toast.error(message);
|
||||
return;
|
||||
}
|
||||
const blockedBy = tryOpenPolicyBlock(rawBody, stackName, stackFile, 'rollback');
|
||||
const blockedBy = tryOpenPolicyBlock(rawBody, stackName, stackFile, 'rollback', opNodeId);
|
||||
if (blockedBy) {
|
||||
toast.error(`Rollback blocked by policy "${blockedBy}"`);
|
||||
return;
|
||||
@@ -889,7 +892,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
throw parseStackActionError(rawBody, 'Rollback failed', res.status);
|
||||
}
|
||||
overlayState.setPolicyBlock(null);
|
||||
toast.success('Stack rolled back successfully.');
|
||||
toast.success('Stack rolled back: compose and env files restored.');
|
||||
stackListState.recordActionSuccess(stackFile);
|
||||
// The rollback already succeeded; a failure of the cosmetic refetches below
|
||||
// (containers redeployed by the rollback, restored compose content, backup
|
||||
@@ -968,6 +971,11 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
optimisticStatus: 'running' | 'exited',
|
||||
successMessage: string,
|
||||
ignorePolicy = false,
|
||||
// Node the operation targets. Defaults to the live active node for direct
|
||||
// callers (toolbar stop/restart); the update path passes the node captured
|
||||
// when its readiness dialog opened so a mid-dialog node switch cannot
|
||||
// retarget the update.
|
||||
opNodeId: number | null = activeNode?.id ?? null,
|
||||
): Promise<void> => {
|
||||
if (stackListState.isStackBusy(stackFile)) return;
|
||||
const stackName = stackFile.replace(/\.(yml|yaml)$/, '');
|
||||
@@ -975,9 +983,6 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const startedAt = Date.now();
|
||||
stackListState.setStackAction(stackFile, action);
|
||||
stackListState.setOptimisticStatus(stackFile, optimisticStatus);
|
||||
// Snapshot the node once so stop/restart/update stays bound to it even if
|
||||
// the active node changes while the operation is in flight.
|
||||
const opNodeId = activeNode?.id ?? null;
|
||||
try {
|
||||
await runWithLog({ stackName, action, nodeId: opNodeId }, async (started, ds) => {
|
||||
await started;
|
||||
@@ -996,7 +1001,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
return { ok: false as const, errorMessage: message };
|
||||
}
|
||||
if (action === 'update') {
|
||||
const blockedBy = tryOpenPolicyBlock(errText, stackName, stackFile, 'update');
|
||||
const blockedBy = tryOpenPolicyBlock(errText, stackName, stackFile, 'update', opNodeId);
|
||||
if (blockedBy) {
|
||||
const message = `Update blocked by policy "${blockedBy}"`;
|
||||
toast.error(message);
|
||||
@@ -1091,11 +1096,15 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const requestStackUpdate = async (stackFile: string): Promise<void> => {
|
||||
if (stackListState.isStackBusy(stackFile)) return;
|
||||
const stackName = stackFile.replace(/\.(yml|yaml)$/, '');
|
||||
const run = () => runStackAction(stackFile, 'update', 'update', 'running', 'Stack updated successfully!');
|
||||
// Capture the node now so the readiness fetch and the update both target it
|
||||
// even if the active node changes while the readiness dialog is open.
|
||||
const opNodeId = activeNode?.id ?? null;
|
||||
const run = () => runStackAction(stackFile, 'update', 'update', 'running', 'Stack updated successfully!', false, opNodeId);
|
||||
if (hasUpdateGuard) {
|
||||
overlayState.setUpdateReadiness({
|
||||
stackName,
|
||||
stackFile,
|
||||
nodeId: opNodeId,
|
||||
proceed: () => {
|
||||
overlayState.setUpdateReadiness(null);
|
||||
void run();
|
||||
@@ -1208,6 +1217,9 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
}
|
||||
const stackName = stackFile.replace(/\.(yml|yaml)$/, '');
|
||||
const startedAt = Date.now();
|
||||
// Bind this sidebar action to the active node now so a policy-block bypass
|
||||
// retries on the same node even if the active node changes meanwhile.
|
||||
const opNodeId = activeNode?.id ?? null;
|
||||
stackListState.setStackAction(stackFile, action);
|
||||
|
||||
if (action === 'stop') {
|
||||
@@ -1217,7 +1229,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
}
|
||||
|
||||
try {
|
||||
const response = await apiFetch(`/stacks/${stackName}/${endpoint}`, { method: 'POST' });
|
||||
const response = await apiFetch(`/stacks/${stackName}/${endpoint}`, { method: 'POST', nodeId: opNodeId });
|
||||
if (!response.ok) {
|
||||
const errText = await response.text();
|
||||
if (response.status === 409) {
|
||||
@@ -1227,7 +1239,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
return;
|
||||
}
|
||||
if (action === 'deploy') {
|
||||
const blockedBy = tryOpenPolicyBlock(errText, stackName, stackFile, action);
|
||||
const blockedBy = tryOpenPolicyBlock(errText, stackName, stackFile, action, opNodeId);
|
||||
if (blockedBy) {
|
||||
toast.error(`Deploy blocked by policy "${blockedBy}"`);
|
||||
return;
|
||||
@@ -1253,7 +1265,7 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
const msg = actionError.message || `Failed to ${action} stack`;
|
||||
toast.error(
|
||||
action === 'deploy' && actionError.rolledBack === true
|
||||
? `${msg} - automatically rolled back to previous version.`
|
||||
? `${msg} - automatically restored the previous compose and env files.`
|
||||
: msg,
|
||||
);
|
||||
recordActionFailureFor(stackFile, stackName, action, startedAt, msg, actionError.rolledBack === true, actionError.failure);
|
||||
|
||||
@@ -27,7 +27,7 @@ export function buildDiagnostics(
|
||||
`Stack: ${stackName}`,
|
||||
`Node: ${activeNode?.name ?? 'local'}${activeNode?.id != null ? ` (id ${activeNode.id})` : ''}`,
|
||||
`Action: ${result.action}`,
|
||||
`Outcome: failed${result.rolledBack ? ' (rolled back to previous version)' : ''}`,
|
||||
`Outcome: failed${result.rolledBack ? ' (restored previous compose and env files)' : ''}`,
|
||||
`Elapsed: ${formatElapsed(result.endedAt - result.startedAt)}`,
|
||||
`Error: ${result.errorMessage ?? 'unknown'}`,
|
||||
`Backup: ${backupInfo.exists
|
||||
|
||||
Reference in New Issue
Block a user