Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(setup,compose): bundle Redis, fix socket reconnect, and harden the setup wizard#5964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
dcd5733
feat(compose,setup): bundle Redis, always configure it, fix lifecycle…
TheodoreSpeaks 47fcf93
fix(setup): don't start managed Postgres with a password the volume w…
TheodoreSpeaks 2243cc5
improvement(setup): default to Docker Compose and sharpen the run-mod…
TheodoreSpeaks 6a093c3
fix(compose): point the browser socket at :3002 so it stops reconnecting
TheodoreSpeaks 6eec210
feat(setup): pass SIM_AGENT_API_URL through, and warn on a half-set m…
TheodoreSpeaks 54aadbf
fix(setup): survive a vanished port owner, and stop flagging our own …
TheodoreSpeaks a28f49f
fix(csp,setup): permit the socket origin the client actually uses; en…
TheodoreSpeaks ccac475
improvement(setup): make k8s mode end somewhere usable, and show inst…
TheodoreSpeaks bb2fd2a
fix(setup): identify Sim compose projects by content, not filename
TheodoreSpeaks 1c5443e
fix(setup): scope the compose port skip to published ports; print bot…
TheodoreSpeaks 02e93d8
fix(setup): one source for the k8s forwards, and surface a dead realt…
TheodoreSpeaks 4550411
fix(setup): pin the compose project on every lifecycle op
TheodoreSpeaks 84d9fe9
fix(setup): warn on both halves of a mothership mismatch
TheodoreSpeaks 910f367
fix(setup): warn about a half-set mothership before minting the key
TheodoreSpeaks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| * | ||
| * The bundled docker-compose stack runs NODE_ENV=production, serves the app from | ||
| * localhost, and leaves NEXT_PUBLIC_SOCKET_URL unset. getSocketUrl() falls back | ||
| * to localhost:3002 for a localhost page regardless of NODE_ENV, so the CSP has | ||
| * to permit that origin or the browser blocks the handshake and Socket.IO | ||
| * retries forever. Its own file because vi.mock is hoisted per-module and the | ||
| * sibling suite needs NEXT_PUBLIC_SOCKET_URL set. | ||
| */ | ||
| import { createEnvMock } from '@sim/testing' | ||
| import { describe, expect, it, vi } from 'vitest' | ||
| vi.mock('@/lib/core/config/env', () => | ||
| createEnvMock({ | ||
| NEXT_PUBLIC_APP_URL: 'http://localhost:3000', | ||
| NEXT_PUBLIC_SOCKET_URL: undefined, | ||
| }) | ||
| ) | ||
| vi.mock('@/lib/core/config/env-flags', () => ({ | ||
| isDev: false, | ||
| isHosted: false, | ||
| isReactGrabEnabled: false, | ||
| })) | ||
| import { generateRuntimeCSP } from './csp' | ||
| describe('generateRuntimeCSP — socket fallback on a localhost origin', () => { | ||
| it('permits the default socket origin when NEXT_PUBLIC_SOCKET_URL is unset', () => { | ||
| const csp = generateRuntimeCSP() | ||
| expect(csp).toContain('http://localhost:3002') | ||
| expect(csp).toContain('ws://localhost:3002') | ||
| }) | ||
| it('keeps the socket sources inside connect-src', () => { | ||
| const connectSrc = generateRuntimeCSP() | ||
| .split(';') | ||
| .map((directive) => directive.trim()) | ||
| .find((directive) => directive.startsWith('connect-src')) | ||
| expect(connectSrc).toBeDefined() | ||
| expect(connectSrc).toContain('ws://localhost:3002') | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -9,6 +9,18 @@ import { glyph, theme } from './theme.ts' | ||
| const DEFAULT_DSN = 'postgresql://postgres:postgres@localhost:5432/simstudio' | ||
| /** Postgres' wire message when the password is wrong — a live server, not a dead one. */ | ||
| const AUTH_FAILURE = /password authentication failed/i | ||
| /** | ||
| * Percent-encodes the password so characters that are structural in a URL | ||
| * (`@`, `:`, `/`, `#`, `?`) can't re-parse the DSN into a different host — which | ||
| * would fail a password that is actually correct. | ||
| */ | ||
| function buildDsn(password: string, hostPort: string | number): string { | ||
| return `postgresql://postgres:${encodeURIComponent(password)}@localhost:${hostPort}/simstudio` | ||
| } | ||
| export function docker(args: string[]): void { | ||
| const result = spawnSync('docker', args, { encoding: 'utf8' }) | ||
| if (result.status !== 0) { | ||
| @@ -63,7 +75,9 @@ function inspectManagedContainer(): ManagedContainer | null { | ||
| return { | ||
| running: running === 'true', | ||
| dsn: `postgresql://postgres:${password}@localhost:${hostPort}/simstudio`, | ||
| // Read back from the container env verbatim, so it may be a password the | ||
| // user supplied for an existing volume — encode it like any other. | ||
| dsn: buildDsn(password, hostPort), | ||
| } | ||
| } | ||
| @@ -122,6 +136,81 @@ async function promptExternalDsn(): Promise<string> { | ||
| } | ||
| } | ||
| const DB_VOLUME = 'sim-postgres-data' | ||
| /** True once initdb has run in the volume — PG_VERSION only exists after bootstrap. */ | ||
| function volumeInitialized(): boolean { | ||
| if (spawnSync('docker', ['volume', 'inspect', DB_VOLUME], { stdio: 'ignore' }).status !== 0) { | ||
| return false | ||
| } | ||
| // Read the marker from inside the volume; the image is already local, so this | ||
| // costs nothing extra and beats assuming "volume exists" means "bootstrapped" | ||
| // (a failed first run leaves an empty volume behind). | ||
| return ( | ||
| spawnSync( | ||
| 'docker', | ||
| [ | ||
| 'run', | ||
| '--rm', | ||
| '-v', | ||
| `${DB_VOLUME}:/pgdata`, | ||
| '--entrypoint', | ||
| 'test', | ||
| 'pgvector/pgvector:pg17', | ||
| '-f', | ||
| '/pgdata/PG_VERSION', | ||
| ], | ||
| { stdio: 'ignore' } | ||
| ).status === 0 | ||
| ) | ||
| } | ||
| /** | ||
| * The volume already holds a cluster whose password we cannot read back. Either | ||
| * the user supplies it, or the data goes — silently generating a new password | ||
| * would produce a container that never authenticates. | ||
| */ | ||
| async function resolveExistingVolume(): Promise<string> { | ||
| p.log.warn( | ||
| `The ${DB_VOLUME} volume already contains a database, but its password is not recoverable — Postgres ignores POSTGRES_PASSWORD on an existing data directory.` | ||
| ) | ||
| const choice = await p.select({ | ||
| message: 'How should the wizard proceed?', | ||
| options: [ | ||
| { | ||
| value: 'password', | ||
| label: 'Keep the data — I have its password', | ||
| hint: 'from a previous .env, or your notes', | ||
| }, | ||
| { | ||
| value: 'wipe', | ||
| label: 'Delete the old data and start fresh', | ||
| hint: `removes the ${DB_VOLUME} volume — this cannot be undone`, | ||
| }, | ||
| ], | ||
| initialValue: 'password', | ||
| }) | ||
| if (choice === 'password') { | ||
| return p.password({ | ||
| message: `Password for the existing ${DB_VOLUME} database`, | ||
| validate: (value) => (value ? undefined : 'required'), | ||
| }) | ||
| } | ||
| const sure = await p.confirm({ | ||
| message: theme.error(`Permanently delete the ${DB_VOLUME} volume and all its data?`), | ||
| initialValue: false, | ||
| }) | ||
| if (!sure) { | ||
| throw new SetupError('kept the existing database volume, so setup cannot continue.', [ | ||
| 're-run and supply the password, or remove it yourself:', | ||
| theme.command(`docker volume rm ${DB_VOLUME}`), | ||
| ]) | ||
| } | ||
| docker(['volume', 'rm', DB_VOLUME]) | ||
| p.log.step(`Removed ${DB_VOLUME}`) | ||
| return generateSecret().slice(0, 24) | ||
| } | ||
| /** | ||
| * Provisions the managed container, reconciling with one that already exists | ||
| * rather than colliding on the name. Recreating is always an explicit choice — | ||
| @@ -142,15 +231,27 @@ async function startManagedContainer(detection: Detection): Promise<string> { | ||
| throw new SetupError(`the existing ${DB_CONTAINER} container is not usable.`, [ | ||
| `inspect: ${theme.command(`docker logs ${DB_CONTAINER}`)}`, | ||
| `remove it: ${theme.command(`docker rm -f ${DB_CONTAINER}`)}`, | ||
| `start clean: ${theme.command('docker volume rm sim-postgres-data')} drops its data too`, | ||
| `start clean: ${theme.command(`docker volume rm ${DB_VOLUME}`)} drops its data too`, | ||
| ]) | ||
| } | ||
| docker(['rm', '-f', DB_CONTAINER]) | ||
| } | ||
| const password = generateSecret().slice(0, 24) | ||
| const hostPort = detection.postgresPortOpen ? 5433 : 5432 | ||
| const dsn = `postgresql://postgres:${password}@localhost:${hostPort}/simstudio` | ||
| // POSTGRES_PASSWORD only applies when initdb runs on an empty data directory. | ||
| // The volume outlives the container (sim down keeps it, so does `docker rm`), | ||
| // so once the container is gone the password it was created with is | ||
| // unrecoverable — inspectManagedContainer reads it from the container, not the | ||
| // volume. Running with a freshly generated password against an initialized | ||
| // volume starts a healthy Postgres that rejects every connection with | ||
| // "password authentication failed", which surfaces as a bogus "container did | ||
| // not become healthy". Ask instead of guessing. | ||
| const password = volumeInitialized() | ||
| ? await resolveExistingVolume() | ||
| : generateSecret().slice(0, 24) | ||
| // A user-supplied password can contain @ : / # — raw interpolation would | ||
| // re-parse the DSN into a different host and fail a password that is correct. | ||
| const dsn = buildDsn(password, hostPort) | ||
| docker([ | ||
| 'run', | ||
| '-d', | ||
| @@ -159,7 +260,7 @@ async function startManagedContainer(detection: Detection): Promise<string> { | ||
| '--label', | ||
| 'managed-by=sim-setup', | ||
| '-v', | ||
| 'sim-postgres-data:/var/lib/postgresql/data', | ||
| `${DB_VOLUME}:/var/lib/postgresql/data`, | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| '-e', | ||
| `POSTGRES_PASSWORD=${password}`, | ||
| '-e', | ||
| @@ -170,9 +271,31 @@ async function startManagedContainer(detection: Detection): Promise<string> { | ||
| ]) | ||
| const spin = p.spinner() | ||
| spin.start(`Starting ${DB_CONTAINER} container on :${hostPort}…`) | ||
| const healthy = await waitFor(async () => (await pgProbe(dsn)).ok, 45_000, 1500) | ||
| let lastError = '' | ||
| const healthy = await waitFor( | ||
| async () => { | ||
| const probe = await pgProbe(dsn) | ||
| if (!probe.ok) lastError = probe.error ?? '' | ||
| return probe.ok | ||
| }, | ||
| 45_000, | ||
| 1500 | ||
| ) | ||
| if (!healthy) { | ||
| spin.stop(`${glyph.fail} container did not become healthy`) | ||
| // Postgres running and refusing the password is a different failure from | ||
| // Postgres never starting, and it is the likely one on the keep-the-volume | ||
| // path. Reporting it as "did not become healthy" is the exact confusion | ||
| // this whole change set exists to remove. | ||
| if (AUTH_FAILURE.test(lastError)) { | ||
| throw new SetupError( | ||
| `Postgres started, but rejected that password for the existing ${DB_VOLUME} volume.`, | ||
| [ | ||
| 're-run and enter the password the volume was created with', | ||
| `or discard the old data: ${theme.command(`docker rm -f ${DB_CONTAINER} && docker volume rm ${DB_VOLUME}`)}`, | ||
| ] | ||
| ) | ||
| } | ||
| const logs = spawnSync('docker', ['logs', '--tail', '20', DB_CONTAINER], { encoding: 'utf8' }) | ||
| throw new SetupError( | ||
| `the Postgres container failed to start. Last logs:\n${logs.stdout}${logs.stderr}`, | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.