ReviewOS
Reload

also looking at this

stacks/ts-cloud

feat(fleet): refuse to destroy a server still holding a moved site's rollback

#183
Merged glennmichael123 wants to merge feat/destroy-drained-guard into main
4 files +284 -2

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/ts-cloud/bin/commands/compute-lifecycle.tsmodified+68-2
@@ -1,9 +1,65 @@
11import type { CLI } from '@stacksjs/clapp'
2import { resolveCloudProvider } from '@ts-cloud/core'
2import type { CloudDriver, EnvironmentType } from '@ts-cloud/core'
3import { resolveCloudProvider, resolveProjectStackName } from '@ts-cloud/core'
34import * as cli from '../../src/utils/cli'
45import { createCloudDriver } from '../../src/drivers'
6import { buildDrainedSiteScanScript, formatDrainedSiteRefusal, parseDrainedSites } from '../../src/operations/drained-sites'
57import { loadValidatedConfig } from './shared'
68
9/** The flag that authorizes discarding a drained site's files. */
10const DISCARD_FLAG = '--discard-drained-sites'
11
12/**
13 * Refuse a teardown that would take a moved site's rollback with it.
14 *
15 * Returns true when the destroy may proceed. Deliberately permissive about its
16 * own failure: a box that cannot be reached, or a driver that cannot run remote
17 * commands, produces a note rather than a block — this exists to stop a specific
18 * silent loss, not to stand between an operator and an unreachable server they
19 * are trying to get rid of.
20 */
21async function drainedSitesAllowTeardown(
22 driver: CloudDriver,
23 config: Awaited<ReturnType<typeof loadValidatedConfig>>,
24 environment: EnvironmentType,
25 discard: boolean,
26): Promise<boolean> {
27 if (discard) return true
28
29 const slug = config.project.slug
30 let output: string | undefined
31 try {
32 const targets = await driver.findComputeTargets({
33 slug,
34 environment,
35 role: 'app',
36 stackName: resolveProjectStackName(config, environment),
37 })
38 if (targets.length === 0) return true
39
40 const result = await driver.runRemoteDeploy({
41 targets,
42 commands: buildDrainedSiteScanScript(slug),
43 comment: `ts-cloud scan drained sites ${slug}`,
44 tags: { Project: slug, Environment: environment, Role: 'app' },
45 })
46 if (!result.success) {
47 cli.warn(`Could not check the server for moved-off site files: ${result.error || 'unknown error'}`)
48 return true
49 }
50 output = result.perInstance.map((instance) => instance.output ?? '').join('\n')
51 } catch (error) {
52 cli.warn(`Could not check the server for moved-off site files: ${error instanceof Error ? error.message : String(error)}`)
53 return true
54 }
55
56 const drained = parseDrainedSites(output)
57 if (drained.length === 0) return true
58
59 cli.error(formatDrainedSiteRefusal(drained, slug, DISCARD_FLAG))
60 return false
61}
62
763/**
864 * Lifecycle commands for the lightweight single-server (Forge-style) compute
965 * provisioned by `cloud deploy` when `compute.mode: 'server'`.
@@ -13,7 +69,8 @@ export function registerComputeLifecycleCommands(app: CLI): void {
1369 .command('destroy', 'Destroy the single-server compute (instance + firewall)')
1470 .option('--env <env>', 'Environment', { default: 'production' })
1571 .option('--force', 'Skip the confirmation prompt')
16 .action(async (options?: { env?: string; force?: boolean }) => {
72 .option('--discard-drained-sites', 'Destroy even though a moved site left its rollback files here')
73 .action(async (options?: { env?: string; force?: boolean; discardDrainedSites?: boolean }) => {
1774 cli.header('Destroy Compute')
1875 const config = await loadValidatedConfig()
1976 const environment = (options?.env || 'production') as 'production' | 'staging' | 'development'
@@ -28,6 +85,15 @@ export function registerComputeLifecycleCommands(app: CLI): void {
2885 cli.warn(
2986 `This terminates the ${provider} server for ${config.project.slug}/${environment} and deletes its firewall.`,
3087 )
88
89 // Checked BEFORE the prompt: an operator answering "yes" to a generic
90 // irreversibility warning has not been told that a moved site's rollback
91 // is sitting on this disk.
92 if (!(await drainedSitesAllowTeardown(driver, config, environment, !!options?.discardDrainedSites))) {
93 process.exitCode = 1
94 return
95 }
96
3197 if (!options?.force) {
3298 const ok = await cli.confirm('This is irreversible. Continue?', false)
3399 if (!ok) {
packages/ts-cloud/src/operations/drained-sites.test.tsadded+83-0
@@ -0,0 +1,83 @@
1import { describe, expect, it } from 'bun:test'
2import { buildDrainedSiteScanScript, formatDrainedSiteRefusal, parseDrainedSites } from './drained-sites'
3
4describe('buildDrainedSiteScanScript', () => {
5 it('scans this project\'s trees only', () => {
6 const script = buildDrainedSiteScanScript('hq').join('\n')
7 expect(script).toContain('/var/www/hq-*')
8 expect(script).toContain('^hq-$TS_CLOUD_SITE[-@.]')
9 })
10
11 /** A stray folder under /var/www must not block a teardown. */
12 it('counts only directories that hold a release tree', () => {
13 expect(buildDrainedSiteScanScript('hq').join('\n')).toContain('[ -d "$TS_CLOUD_DIR/releases" ] || continue')
14 })
15
16 /**
17 * A scan that could change the box would be a poor thing to run immediately
18 * before deciding whether to keep it.
19 */
20 it('only reads', () => {
21 const script = buildDrainedSiteScanScript('hq').join('\n')
22 for (const mutation of ['rm ', 'systemctl stop', 'systemctl start', 'systemctl disable', 'systemctl enable', 'mv ', 'tar ']) {
23 expect(script.includes(mutation)).toBe(false)
24 }
25 // Every redirect to a FILE goes to /dev/null (`>&1` duplicates a descriptor
26 // rather than writing anywhere), so nothing lands on disk.
27 for (const redirect of script.match(/>(?!&)\s*\S+/g) ?? []) {
28 expect(redirect.replace(/^>\s*/, '')).toBe('/dev/null')
29 }
30 })
31
32 it('escapes a slug with regex metacharacters', () => {
33 expect(buildDrainedSiteScanScript('my.app').join('\n')).toContain('^my\\.app-')
34 })
35
36 it('always exits 0, so a scan is never mistaken for a failure', () => {
37 expect(buildDrainedSiteScanScript('hq').at(-1)).toBe('exit 0')
38 })
39})
40
41describe('parseDrainedSites', () => {
42 it('reports the trees with nothing running for them', () => {
43 const drained = parseDrainedSites('site:bughq:no:1.2G\nsite:loghq:yes:800M')
44 expect(drained).toEqual([{ name: 'bughq', size: '1.2G' }])
45 })
46
47 /**
48 * A site with something active is a site on a box being torn down — ordinary,
49 * and already covered by the teardown's own confirmation.
50 */
51 it('ignores a site that is still running', () => {
52 expect(parseDrainedSites('site:bughq:yes:1.2G')).toEqual([])
53 })
54
55 it('is empty for a box with no trees, or no output at all', () => {
56 expect(parseDrainedSites('')).toEqual([])
57 expect(parseDrainedSites(undefined)).toEqual([])
58 expect(parseDrainedSites('some unrelated chatter')).toEqual([])
59 })
60
61 it('survives a missing size', () => {
62 expect(parseDrainedSites('site:bughq:no:')).toEqual([{ name: 'bughq', size: '?' }])
63 })
64})
65
66describe('formatDrainedSiteRefusal', () => {
67 it('names the sites, what the files are for, and the way forward', () => {
68 const message = formatDrainedSiteRefusal([{ name: 'bughq', size: '1.2G' }], 'hq', '--discard-drained-sites')
69 expect(message).toContain('hq-bughq (1.2G)')
70 expect(message).toContain('they are the rollback')
71 expect(message).toContain('--discard-drained-sites')
72 })
73
74 it('reads correctly for several', () => {
75 const message = formatDrainedSiteRefusal(
76 [{ name: 'a', size: '1G' }, { name: 'b', size: '2G' }],
77 'hq',
78 '--discard-drained-sites',
79 )
80 expect(message).toContain('2 site trees')
81 expect(message).toContain('for them')
82 })
83})
packages/ts-cloud/src/operations/drained-sites.tsadded+110-0
@@ -0,0 +1,110 @@
1/**
2 * Detect sites whose files are still on a box that no longer serves them.
3 *
4 * `site:move` deliberately never deletes anything on the source: it stops the
5 * units, drops the gateway route, and leaves the whole tree in place. That
6 * leftover tree IS the rollback — the one thing that makes a bad cutover
7 * recoverable, and the reason the operation can promise reversibility "until the
8 * source server is destroyed".
9 *
10 * Which makes destroying that server the moment the promise expires, and nothing
11 * about the box says so. It has no running units for the site, so it looks idle;
12 * the config has moved on; the operator is tidying up. Terminating it is exactly
13 * the right thing to do once the move is verified, and exactly the wrong thing
14 * to do before — and those two look identical from outside.
15 *
16 * So: before a teardown, ask the box whether it is holding anyone's rollback.
17 *
18 * @see https://github.com/stacksjs/ts-cloud/issues/167
19 */
20
21/** Escape a value for safe use inside a POSIX ERE. */
22function reEscape(value: string): string {
23 return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
24}
25
26/** Single-quote a value for safe embedding in the generated shell. */
27function sh(value: string): string {
28 return `'${value.split('\'').join('\'\\\'\'')}'`
29}
30
31/**
32 * Report every one of this project's site trees on the box and whether anything
33 * is still running for it.
34 *
35 * Read-only by construction — it lists and queries, and that is all. A scan that
36 * could change the box would be a poor thing to run immediately before deciding
37 * whether to keep it.
38 *
39 * A directory is only counted when it holds `releases/`, so a stray folder under
40 * `/var/www` is not mistaken for a deployed site and does not block a teardown
41 * the operator genuinely wants.
42 */
43export function buildDrainedSiteScanScript(slug: string, wwwRoot = '/var/www'): string[] {
44 const prefix = `${wwwRoot}/${slug}-`
45 return [
46 'set -u',
47 `for TS_CLOUD_DIR in ${prefix}*; do`,
48 ' [ -d "$TS_CLOUD_DIR" ] || continue',
49 // Only a real release tree counts as a deployed site.
50 ' [ -d "$TS_CLOUD_DIR/releases" ] || continue',
51 ` TS_CLOUD_SITE="\${TS_CLOUD_DIR#${prefix}}"`,
52 ' TS_CLOUD_ACTIVE=no',
53 ` for TS_CLOUD_UNIT in $(ls /etc/systemd/system/ 2>/dev/null | grep -E "^${reEscape(slug)}-$TS_CLOUD_SITE[-@.]" || true); do`,
54 ' systemctl is-active "$TS_CLOUD_UNIT" >/dev/null 2>&1 && TS_CLOUD_ACTIVE=yes',
55 ' done',
56 // Size is reported so the refusal can say how much is at stake.
57 ' TS_CLOUD_SIZE="$(du -sh "$TS_CLOUD_DIR" 2>/dev/null | cut -f1)"',
58 ' echo "site:$TS_CLOUD_SITE:$TS_CLOUD_ACTIVE:${TS_CLOUD_SIZE:-?}"',
59 'done',
60 'exit 0',
61 ]
62}
63
64export interface DrainedSite {
65 name: string
66 /** Human-readable size of the tree left behind, e.g. `1.2G`. */
67 size: string
68}
69
70/**
71 * Sites the box still holds but no longer runs.
72 *
73 * A site with something active is NOT drained — it is simply a site on a box
74 * being torn down, which is ordinary and already covered by the teardown's own
75 * confirmation. The interesting case is files with nothing running: either a
76 * completed `site:move` whose rollback this is, or a site that has been stopped
77 * and forgotten. Both are worth a sentence before the disk goes away.
78 */
79export function parseDrainedSites(output: string | undefined): DrainedSite[] {
80 if (!output) return []
81 const drained: DrainedSite[] = []
82 for (const line of output.split('\n')) {
83 const trimmed = line.trim()
84 if (!trimmed.startsWith('site:')) continue
85 const [, name, active, size] = trimmed.split(':')
86 if (!name || active !== 'no') continue
87 drained.push({ name, size: size || '?' })
88 }
89 return drained
90}
91
92/**
93 * What the operator sees instead of a teardown.
94 *
95 * Names the sites, says what the files are FOR, and gives the two ways forward.
96 * The flag is deliberately its own thing rather than `--force`: `--force` exists
97 * so a teardown can run unattended, and a CI job that skips a prompt must not
98 * also silently discard the only copy of a rollback.
99 */
100export function formatDrainedSiteRefusal(sites: readonly DrainedSite[], slug: string, flag: string): string {
101 const list = sites.map(site => ` ${slug}-${site.name} (${site.size})`).join('\n')
102 const them = sites.length === 1 ? 'it' : 'them'
103 return (
104 `This server still holds ${sites.length} site tree${sites.length === 1 ? '' : 's'} with nothing running for `
105 + `${them}:\n${list}\n`
106 + 'A site moved off this box keeps its files here on purpose — they are the rollback, and they are what makes '
107 + 'the move reversible until this server is destroyed. Destroying it now discards that.\n'
108 + `Verify the site is serving from its new home first, then re-run with ${flag} to destroy it anyway.`
109 )
110}
packages/ts-cloud/test/integration/site-move-e2e.test.tsmodified+23-0
@@ -24,6 +24,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
2424import { mkdtemp, rm, writeFile } from 'node:fs/promises'
2525import { tmpdir } from 'node:os'
2626import { join } from 'node:path'
27import { buildDrainedSiteScanScript, parseDrainedSites } from '../../src/operations/drained-sites'
2728import {
2829 buildCertificatePackScript,
2930 buildCertificateStateScript,
@@ -318,10 +319,32 @@ describe.skipIf(!canRun)('site:move between two boxes (docker)', () => {
318319 expect((await exec(SOURCE, `test -f /etc/systemd/system/${SLUG}-${SITE}.service`)).code).toBe(0)
319320 })
320321
322 /**
323 * The drained tree is the rollback, and a teardown would take it. The scan has
324 * to see that on a real box: the site's files present, nothing running for it.
325 */
326 it('is reported as a drained site, so a teardown can refuse', async () => {
327 const scan = await run(SOURCE, buildDrainedSiteScanScript(SLUG).join('\n'))
328 const drained = parseDrainedSites(scan)
329 expect(drained.map(site => site.name)).toContain(SITE)
330 expect(drained[0].size).not.toBe('?')
331 })
332
333 it('is not reported as drained on the target, which is serving it', async () => {
334 const scan = await run(TARGET, buildDrainedSiteScanScript(SLUG).join('\n'))
335 expect(parseDrainedSites(scan)).toEqual([])
336 })
337
321338 it('comes back on the source by restarting it, with its data intact', async () => {
322339 await run(SOURCE, `systemctl start ${SLUG}-${SITE}.service`)
323340 const health = await exec(SOURCE, buildHealthGateScript(PORT, '/'))
324341 expect(health.code).toBe(0)
325342 expect((await run(SOURCE, `cat ${APP_BASE}/shared/database/app.sqlite`)).trim()).toBe(SHARED_DB_CONTENT)
326343 })
344
345 /** Once the source is serving again, it is no longer holding anyone's rollback. */
346 it('stops being reported as drained once it serves again', async () => {
347 const scan = await run(SOURCE, buildDrainedSiteScanScript(SLUG).join('\n'))
348 expect(parseDrainedSites(scan)).toEqual([])
349 })
327350})