Merge pull request #34108 from backstage/rugvip/forward-embedded-pg-config

cli-module-build: forward user config to embedded Postgres

packages/cli-module-build/src/lib/runner/runBackend.test.ts

@@ -49,16 +49,6 @@ jest.mock('ctrlc-windows', () => ({
   ctrlc: jest.fn(),
 }));
 
-const mockToConfig = jest.fn();
-const mockConfigSourcesDefault = jest.fn().mockReturnValue({});
-
-jest.mock('@backstage/config-loader', () => ({
-  ConfigSources: {
-    default: (...args: any[]) => mockConfigSourcesDefault(...args),
-    toConfig: (...args: any[]) => mockToConfig(...args),
-  },
-}));
-
 const mockStartEmbeddedDb = jest.fn();
 
 jest.mock('./startEmbeddedDb', () => ({
@@ -85,11 +75,8 @@ describe('runBackend', () => {
     // Mock process.once to prevent actual signal handling
     jest.spyOn(process, 'once').mockReturnValue(process);
 
-    mockToConfig.mockResolvedValue({
-      close: jest.fn(),
-      getOptionalString: () => undefined,
-    });
     mockStartEmbeddedDb.mockReset();
+    mockStartEmbeddedDb.mockResolvedValue(undefined);
   });
 
   afterEach(() => {
@@ -180,18 +167,16 @@ describe('runBackend', () => {
   });
 
   describe('embedded-postgres support', () => {
-    it('should start embedded DB and inject config when database client is embedded-postgres', async () => {
-      mockToConfig.mockResolvedValue({
-        close: jest.fn(),
-        getOptionalString: (key: string) =>
-          key === 'backend.database.client' ? 'embedded-postgres' : undefined,
-      });
+    it('should inject config override from startEmbeddedDb when it returns a result', async () => {
       mockStartEmbeddedDb.mockResolvedValue({
-        connection: {
-          host: 'localhost',
-          user: 'postgres',
-          password: 'password',
-          port: 5555,
+        configOverride: {
+          client: 'pg',
+          connection: {
+            host: 'localhost',
+            user: 'postgres',
+            password: 'password',
+            port: 5555,
+          },
         },
         close: jest.fn(),
       });
@@ -199,8 +184,10 @@ describe('runBackend', () => {
       runBackend({ entry: 'src/index' });
       await jest.advanceTimersByTimeAsync(100);
 
-      expect(mockStartEmbeddedDb).toHaveBeenCalled();
-      expect(mockSpawn).toHaveBeenCalled();
+      expect(mockStartEmbeddedDb).toHaveBeenCalledWith({
+        configPaths: undefined,
+        targetDir: undefined,
+      });
       const spawnEnv = mockSpawn.mock.calls[0][2]?.env as Record<
         string,
         string
@@ -217,12 +204,7 @@ describe('runBackend', () => {
       });
     });
 
-    it('should resolve config paths relative to targetDir', async () => {
-      mockToConfig.mockResolvedValue({
-        close: jest.fn(),
-        getOptionalString: () => undefined,
-      });
-
+    it('should forward configPaths and targetDir to startEmbeddedDb', async () => {
       runBackend({
         entry: 'src/index',
         targetDir: '/root/packages/backend',
@@ -230,25 +212,18 @@ describe('runBackend', () => {
       });
       await jest.advanceTimersByTimeAsync(100);
 
-      expect(mockConfigSourcesDefault).toHaveBeenCalledWith(
-        expect.objectContaining({
-          argv: ['--config', '/root/config/local.yaml'],
-        }),
-      );
+      expect(mockStartEmbeddedDb).toHaveBeenCalledWith({
+        configPaths: ['../../config/local.yaml'],
+        targetDir: '/root/packages/backend',
+      });
     });
 
-    it('should not start embedded DB for other database clients', async () => {
-      mockToConfig.mockResolvedValue({
-        close: jest.fn(),
-        getOptionalString: (key: string) =>
-          key === 'backend.database.client' ? 'better-sqlite3' : undefined,
-      });
+    it('should not inject config override when startEmbeddedDb returns undefined', async () => {
+      mockStartEmbeddedDb.mockResolvedValue(undefined);
 
       runBackend({ entry: 'src/index' });
       await jest.advanceTimersByTimeAsync(100);
 
-      expect(mockStartEmbeddedDb).not.toHaveBeenCalled();
-      expect(mockSpawn).toHaveBeenCalled();
       const spawnEnv = mockSpawn.mock.calls[0][2]?.env as Record<
         string,
         string

packages/cli-module-build/src/lib/runner/runBackend.ts

@@ -20,12 +20,8 @@ import { ctrlc } from 'ctrlc-windows';
 import { IpcServer, ServerDataStore } from '../ipc';
 import debounce from 'lodash/debounce';
 import { fileURLToPath } from 'node:url';
-import {
-  isAbsolute as isAbsolutePath,
-  resolve as resolvePath,
-} from 'node:path';
+import { isAbsolute as isAbsolutePath } from 'node:path';
 import { targetPaths } from '@backstage/cli-common';
-import { ConfigSources } from '@backstage/config-loader';
 
 import spawn from 'cross-spawn';
 import { startEmbeddedDb } from './startEmbeddedDb';
@@ -66,18 +62,14 @@ export async function runBackend(options: RunBackendOptions) {
 
   const extraEnv: Record<string, string> = {};
 
-  let embeddedDb: Awaited<ReturnType<typeof startEmbeddedDb>> | undefined;
-
-  const dbClient = await readDatabaseClient(
-    options.configPaths,
-    options.targetDir,
-  );
-  if (dbClient === 'embedded-postgres') {
-    embeddedDb = await startEmbeddedDb();
-    extraEnv.APP_CONFIG_backend_database = JSON.stringify({
-      client: 'pg',
-      connection: embeddedDb.connection,
-    });
+  const embeddedDb = await startEmbeddedDb({
+    configPaths: options.configPaths,
+    targetDir: options.targetDir,
+  });
+  if (embeddedDb) {
+    extraEnv.APP_CONFIG_backend_database = JSON.stringify(
+      embeddedDb.configOverride,
+    );
   }
 
   let exiting = false;
@@ -220,26 +212,3 @@ export async function runBackend(options: RunBackendOptions) {
 
   return () => exitPromise;
 }
-
-async function readDatabaseClient(
-  configPaths?: string[],
-  targetDir?: string,
-): Promise<string | undefined> {
-  const rootDir = targetPaths.rootDir;
-  const configBaseDir = targetDir ?? rootDir;
-  const source = ConfigSources.default({
-    rootDir,
-    allowMissingDefaultConfig: true,
-    argv: (configPaths ?? []).flatMap(p => [
-      '--config',
-      isAbsolutePath(p) ? p : resolvePath(configBaseDir, p),
-    ]),
-  });
-
-  const config = await ConfigSources.toConfig(source);
-  try {
-    return config.getOptionalString('backend.database.client');
-  } finally {
-    config.close();
-  }
-}

packages/cli-module-build/src/lib/runner/startEmbeddedDb.ts

@@ -16,9 +16,14 @@
 
 import os from 'node:os';
 import fs from 'fs-extra';
-import { resolve as resolvePath } from 'node:path';
+import {
+  isAbsolute as isAbsolutePath,
+  resolve as resolvePath,
+} from 'node:path';
 import { getPortPromise } from 'portfinder';
 import { ForwardedError } from '@backstage/errors';
+import { ConfigSources } from '@backstage/config-loader';
+import { targetPaths } from '@backstage/cli-common';
 import chalk from 'chalk';
 
 const TEMP_DIR_PREFIX = 'backstage-dev-db-';
@@ -52,7 +57,45 @@ async function cleanStaleDatabases() {
   );
 }
 
-export async function startEmbeddedDb() {
+export interface EmbeddedDbConnectionConfig {
+  host?: string;
+  port?: number;
+  user?: string;
+  password?: string;
+}
+
+export interface StartEmbeddedDbOptions {
+  configPaths?: string[];
+  targetDir?: string;
+}
+
+/**
+ * Reads the database configuration from the app config and, if the client is
+ * `embedded-postgres`, starts an embedded Postgres instance. Returns the config
+ * override to inject into the child process, or `undefined` if the database
+ * client is not `embedded-postgres`.
+ */
+export async function startEmbeddedDb(options: StartEmbeddedDbOptions): Promise<
+  | {
+      configOverride: Record<string, unknown>;
+      close(): Promise<void>;
+    }
+  | undefined
+> {
+  const dbConfig = await readDatabaseConfig(
+    options.configPaths,
+    options.targetDir,
+  );
+  if (dbConfig?.client !== 'embedded-postgres') {
+    return undefined;
+  }
+
+  return startEmbeddedDbInternal(dbConfig.connection);
+}
+
+async function startEmbeddedDbInternal(
+  userConfig?: EmbeddedDbConnectionConfig,
+) {
   console.warn(
     chalk.yellow(
       'WARNING: Using embedded-postgres for local development is experimental and subject to change',
@@ -72,10 +115,10 @@ export async function startEmbeddedDb() {
 
   await cleanStaleDatabases();
 
-  const host = 'localhost';
-  const user = 'postgres';
-  const password = 'password';
-  const port = await getPortPromise();
+  const host = userConfig?.host ?? 'localhost';
+  const user = userConfig?.user ?? 'postgres';
+  const password = userConfig?.password ?? 'postgres';
+  const port = userConfig?.port ?? (await getPortPromise());
   const tmpDir = await fs.mkdtemp(resolvePath(os.tmpdir(), TEMP_DIR_PREFIX));
 
   const pg = new EmbeddedPostgres({
@@ -100,16 +143,87 @@ export async function startEmbeddedDb() {
     throw error;
   }
 
+  const configOverride: Record<string, unknown> = {
+    client: 'pg',
+  };
+  const defaultedConnection: Record<string, string | number> = {};
+  if (!userConfig?.host) {
+    defaultedConnection.host = host;
+  }
+  if (!userConfig?.user) {
+    defaultedConnection.user = user;
+  }
+  if (!userConfig?.password) {
+    defaultedConnection.password = password;
+  }
+  if (!userConfig?.port) {
+    defaultedConnection.port = port;
+  }
+  if (Object.keys(defaultedConnection).length > 0) {
+    configOverride.connection = defaultedConnection;
+  }
+
   return {
-    connection: {
-      host,
-      user,
-      password,
-      port,
-    },
+    configOverride,
     async close() {
       await pg.stop();
       await fs.remove(tmpDir);
     },
   };
 }
+
+async function readDatabaseConfig(
+  configPaths?: string[],
+  targetDir?: string,
+): Promise<
+  | {
+      client: string;
+      connection?: EmbeddedDbConnectionConfig;
+    }
+  | undefined
+> {
+  const rootDir = targetPaths.rootDir;
+  const configBaseDir = targetDir ?? rootDir;
+  const source = ConfigSources.default({
+    rootDir,
+    allowMissingDefaultConfig: true,
+    argv: (configPaths ?? []).flatMap(p => [
+      '--config',
+      isAbsolutePath(p) ? p : resolvePath(configBaseDir, p),
+    ]),
+  });
+
+  const config = await ConfigSources.toConfig(source);
+  try {
+    const client = config.getOptionalString('backend.database.client');
+    if (!client) {
+      return undefined;
+    }
+
+    // Only read structured connection config if the value is an object;
+    // it can also be a plain string (e.g. ':memory:' for better-sqlite3).
+    const rawConnection = config.getOptional('backend.database.connection');
+    if (typeof rawConnection === 'string' || rawConnection === undefined) {
+      return { client };
+    }
+
+    const host = config.getOptionalString('backend.database.connection.host');
+    const port = config.getOptionalNumber('backend.database.connection.port');
+    const user = config.getOptionalString('backend.database.connection.user');
+    const password = config.getOptionalString(
+      'backend.database.connection.password',
+    );
+
+    const connection =
+      host !== undefined ||
+      port !== undefined ||
+      user !== undefined ||
+      password !== undefined
+        ? { host, port, user, password }
+        : undefined;
+
+    return { client, connection };
+  } finally {
+    config.close();
+  }
+}