fix: defer reading of database configuration until needed to fix race

index.ts

@@ -4,10 +4,10 @@ import path from 'path';
 import yargs from 'yargs';
 import { hideBin } from 'yargs/helpers';
 import * as fs from 'fs';
-import { configFile, setConfigFile, validate } from './src/config/file';
+import { getConfigFile, setConfigFile, validate } from './src/config/file';
 import { initUserConfig } from './src/config';
-import Proxy from './src/proxy';
-import service from './src/service';
+import { Proxy } from './src/proxy';
+import { Service } from './src/service';
 
 const argv = yargs(hideBin(process.argv))
   .usage('Usage: $0 [options]')
@@ -30,9 +30,11 @@ const argv = yargs(hideBin(process.argv))
   .strict()
   .parseSync();
 
+console.log('Setting config file to: ' + (argv.c as string) || '');
 setConfigFile((argv.c as string) || '');
 initUserConfig();
 
+const configFile = getConfigFile();
 if (argv.v) {
   if (!fs.existsSync(configFile)) {
     console.error(
@@ -46,10 +48,14 @@ if (argv.v) {
   process.exit(0);
 }
 
+console.log('validating config');
 validate();
 
+console.log('Setting up the proxy and Service');
+
+// The deferred imports should cause these to be loaded on first access
 const proxy = new Proxy();
 proxy.start();
-service.start(proxy);
+Service.start(proxy);
 
-export { proxy, service };
+export { proxy, Service };

src/config/file.ts

@@ -2,7 +2,7 @@ import { readFileSync } from 'fs';
 import { join } from 'path';
 import { Convert } from './generated/config';
 
-export let configFile: string = join(__dirname, '../../proxy.config.json');
+let configFile: string = join(__dirname, '../../proxy.config.json');
 
 /**
  * Sets the path to the configuration file.
@@ -14,6 +14,15 @@ export function setConfigFile(file: string) {
   configFile = file;
 }
 
+/**
+ * Gets the path to the current configuration file.
+ *
+ * @return {string} file - The path to the configuration file.
+ */
+export function getConfigFile() {
+  return configFile;
+}
+
 export function validate(filePath: string = configFile): boolean {
   // Use QuickType to validate the configuration
   const configContent = readFileSync(filePath, 'utf-8');

src/config/index.ts

@@ -5,7 +5,7 @@ import { GitProxyConfig, Convert } from './generated/config';
 import { ConfigLoader } from './ConfigLoader';
 import { Configuration } from './types';
 import { serverConfig } from './env';
-import { configFile } from './file';
+import { getConfigFile } from './file';
 
 // Cache for current configuration
 let _currentConfig: GitProxyConfig | null = null;
@@ -52,7 +52,7 @@ function loadFullConfiguration(): GitProxyConfig {
   const defaultConfig = cleanUndefinedValues(rawDefaultConfig);
 
   let userSettings: Partial<GitProxyConfig> = {};
-  const userConfigFile = process.env.CONFIG_FILE || configFile;
+  const userConfigFile = process.env.CONFIG_FILE || getConfigFile();
 
   if (existsSync(userConfigFile)) {
     try {

src/db/file/helper.ts

@@ -1 +1,6 @@
+import { existsSync, mkdirSync } from 'fs';
+
 export const getSessionStore = (): undefined => undefined;
+export const initializeFolders = () => {
+  if (!existsSync('./.data/db')) mkdirSync('./.data/db', { recursive: true });
+};

src/db/file/pushes.ts

@@ -1,4 +1,3 @@
-import fs from 'fs';
 import _ from 'lodash';
 import Datastore from '@seald-io/nedb';
 import { Action } from '../../proxy/actions/Action';
@@ -7,12 +6,6 @@ import { PushQuery } from '../types';
 
 const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
 
-// these don't get coverage in tests as they have already been run once before the test
-/* istanbul ignore if */
-if (!fs.existsSync('./.data')) fs.mkdirSync('./.data');
-/* istanbul ignore if */
-if (!fs.existsSync('./.data/db')) fs.mkdirSync('./.data/db');
-
 // export for testing purposes
 export let db: Datastore;
 if (process.env.NODE_ENV === 'test') {

src/db/index.ts

@@ -6,13 +6,31 @@ import * as mongo from './mongo';
 import * as neDb from './file';
 import { Action } from '../proxy/actions/Action';
 import MongoDBStore from 'connect-mongo';
-
-let sink: Sink;
-if (config.getDatabase().type === 'mongo') {
-  sink = mongo;
-} else if (config.getDatabase().type === 'fs') {
-  sink = neDb;
-}
+import { processGitUrl } from '../proxy/routes/helper';
+import { initializeFolders } from './file/helper';
+
+let _sink: Sink | null = null;
+
+/** The start function is before any attempt to use the DB adaptor and causes the configuration
+ * to be read. This allows the read of the config to be deferred, otherwise it will occur on
+ * import.
+ */
+const start = () => {
+  if (!_sink) {
+    if (config.getDatabase().type === 'mongo') {
+      console.log('Loading MongoDB database adaptor');
+      _sink = mongo;
+    } else if (config.getDatabase().type === 'fs') {
+      console.log('Loading neDB database adaptor');
+      initializeFolders();
+      _sink = neDb;
+    } else {
+      console.error(`Unsupported database type: ${config.getDatabase().type}`);
+      process.exit(1);
+    }
+  }
+  return _sink;
+};
 
 const isBlank = (str: string) => {
   return !str || /^\s*$/.test(str);
@@ -57,6 +75,7 @@ export const createUser = async (
     const errorMessage = `email cannot be empty`;
     throw new Error(errorMessage);
   }
+  const sink = start();
   const existingUser = await sink.findUser(username);
   if (existingUser) {
     const errorMessage = `user ${username} already exists`;
@@ -95,7 +114,7 @@ export const createRepo = async (repo: AuthorisedRepo) => {
     throw new Error('URL cannot be empty');
   }
 
-  return sink.createRepo(toCreate) as Promise<Required<Repo>>;
+  return start().createRepo(toCreate) as Promise<Required<Repo>>;
 };
 
 export const isUserPushAllowed = async (url: string, user: string) => {
@@ -114,7 +133,7 @@ export const canUserApproveRejectPush = async (id: string, user: string) => {
     return false;
   }
 
-  const theRepo = await sink.getRepoByUrl(action.url);
+  const theRepo = await start().getRepoByUrl(action.url);
 
   if (theRepo?.users?.canAuthorise?.includes(user)) {
     console.log(`user ${user} can approve/reject for repo ${action.url}`);
@@ -140,35 +159,55 @@ export const canUserCancelPush = async (id: string, user: string) => {
   }
 };
 
-export const getSessionStore = (): MongoDBStore | undefined =>
-  sink.getSessionStore ? sink.getSessionStore() : undefined;
-export const getPushes = (query: Partial<PushQuery>): Promise<Action[]> => sink.getPushes(query);
-export const writeAudit = (action: Action): Promise<void> => sink.writeAudit(action);
-export const getPush = (id: string): Promise<Action | null> => sink.getPush(id);
-export const deletePush = (id: string): Promise<void> => sink.deletePush(id);
+export const getSessionStore = (): MongoDBStore | undefined => start().getSessionStore();
+export const getPushes = (query: Partial<PushQuery>): Promise<Action[]> => start().getPushes(query);
+export const writeAudit = (action: Action): Promise<void> => start().writeAudit(action);
+export const getPush = (id: string): Promise<Action | null> => start().getPush(id);
+export const deletePush = (id: string): Promise<void> => start().deletePush(id);
 export const authorise = (id: string, attestation: any): Promise<{ message: string }> =>
-  sink.authorise(id, attestation);
-export const cancel = (id: string): Promise<{ message: string }> => sink.cancel(id);
+  start().authorise(id, attestation);
+export const cancel = (id: string): Promise<{ message: string }> => start().cancel(id);
 export const reject = (id: string, attestation: any): Promise<{ message: string }> =>
-  sink.reject(id, attestation);
-export const getRepos = (query?: Partial<RepoQuery>): Promise<Repo[]> => sink.getRepos(query);
-export const getRepo = (name: string): Promise<Repo | null> => sink.getRepo(name);
-export const getRepoByUrl = (url: string): Promise<Repo | null> => sink.getRepoByUrl(url);
-export const getRepoById = (_id: string): Promise<Repo | null> => sink.getRepoById(_id);
+  start().reject(id, attestation);
+export const getRepos = (query?: Partial<RepoQuery>): Promise<Repo[]> => start().getRepos(query);
+export const getRepo = (name: string): Promise<Repo | null> => start().getRepo(name);
+export const getRepoByUrl = (url: string): Promise<Repo | null> => start().getRepoByUrl(url);
+export const getRepoById = (_id: string): Promise<Repo | null> => start().getRepoById(_id);
 export const addUserCanPush = (_id: string, user: string): Promise<void> =>
-  sink.addUserCanPush(_id, user);
+  start().addUserCanPush(_id, user);
 export const addUserCanAuthorise = (_id: string, user: string): Promise<void> =>
-  sink.addUserCanAuthorise(_id, user);
+  start().addUserCanAuthorise(_id, user);
 export const removeUserCanPush = (_id: string, user: string): Promise<void> =>
-  sink.removeUserCanPush(_id, user);
+  start().removeUserCanPush(_id, user);
 export const removeUserCanAuthorise = (_id: string, user: string): Promise<void> =>
-  sink.removeUserCanAuthorise(_id, user);
-export const deleteRepo = (_id: string): Promise<void> => sink.deleteRepo(_id);
-export const findUser = (username: string): Promise<User | null> => sink.findUser(username);
-export const findUserByEmail = (email: string): Promise<User | null> => sink.findUserByEmail(email);
-export const findUserByOIDC = (oidcId: string): Promise<User | null> => sink.findUserByOIDC(oidcId);
-export const getUsers = (query?: Partial<UserQuery>): Promise<User[]> => sink.getUsers(query);
-export const deleteUser = (username: string): Promise<void> => sink.deleteUser(username);
-
-export const updateUser = (user: Partial<User>): Promise<void> => sink.updateUser(user);
+  start().removeUserCanAuthorise(_id, user);
+export const deleteRepo = (_id: string): Promise<void> => start().deleteRepo(_id);
+export const findUser = (username: string): Promise<User | null> => start().findUser(username);
+export const findUserByEmail = (email: string): Promise<User | null> =>
+  start().findUserByEmail(email);
+export const findUserByOIDC = (oidcId: string): Promise<User | null> =>
+  start().findUserByOIDC(oidcId);
+export const getUsers = (query?: Partial<UserQuery>): Promise<User[]> => start().getUsers(query);
+export const deleteUser = (username: string): Promise<void> => start().deleteUser(username);
+
+export const updateUser = (user: Partial<User>): Promise<void> => start().updateUser(user);
+/**
+ * Collect the Set of all host (host and port if specified) that we
+ * will be proxying requests for, to be used to initialize the proxy.
+ *
+ * @return {string[]} an array of origins
+ */
+
+export const getAllProxiedHosts = async (): Promise<string[]> => {
+  const repos = await getRepos();
+  const origins = new Set<string>();
+  repos.forEach((repo) => {
+    const parsedUrl = processGitUrl(repo.url);
+    if (parsedUrl) {
+      origins.add(parsedUrl.host);
+    } // failures are logged by parsing util fn
+  });
+  return Array.from(origins);
+};
+
 export type { PushQuery, Repo, Sink, User } from './types';

src/db/mongo/helper.ts

@@ -2,13 +2,14 @@ import { MongoClient, Db, Collection, Filter, Document, FindOptions } from 'mong
 import { getDatabase } from '../../config';
 import MongoDBStore from 'connect-mongo';
 
-const dbConfig = getDatabase();
-const connectionString = dbConfig.connectionString;
-const options = dbConfig.options;
-
 let _db: Db | null = null;
 
 export const connect = async (collectionName: string): Promise<Collection> => {
+  //retrieve config at point of use (rather than import)
+  const dbConfig = getDatabase();
+  const connectionString = dbConfig.connectionString;
+  const options = dbConfig.options;
+
   if (!_db) {
     if (!connectionString) {
       throw new Error('MongoDB connection string is not provided');
@@ -41,6 +42,10 @@ export const findOneDocument = async <T>(
 };
 
 export const getSessionStore = () => {
+  //retrieve config at point of use (rather than import)
+  const dbConfig = getDatabase();
+  const connectionString = dbConfig.connectionString;
+  const options = dbConfig.options;
   return new MongoDBStore({
     mongoUrl: connectionString,
     collectionName: 'user_session',

src/proxy/index.ts

@@ -35,7 +35,7 @@ const getServerOptions = (): ServerOptions => ({
   cert: getTLSEnabled() && getTLSCertPemPath() ? fs.readFileSync(getTLSCertPemPath()!) : undefined,
 });
 
-export default class Proxy {
+export class Proxy {
   private httpServer: http.Server | null = null;
   private httpsServer: https.Server | null = null;
   private expressApp: Express | null = null;

src/proxy/routes/helper.ts

@@ -1,5 +1,3 @@
-import * as db from '../../db';
-
 /** Regex used to analyze un-proxied Git URLs */
 const GIT_URL_REGEX = /(.+:\/\/)([^/]+)(\/.+\.git)(\/.+)*/;
 
@@ -174,21 +172,3 @@ export const validGitRequest = (gitPath: string, headers: any): boolean => {
   }
   return false;
 };
-
-/**
- * Collect the Set of all host (host and port if specified) that we
- * will be proxying requests for, to be used to initialize the proxy.
- *
- * @return {string[]} an array of origins
- */
-export const getAllProxiedHosts = async (): Promise<string[]> => {
-  const repos = await db.getRepos();
-  const origins = new Set<string>();
-  repos.forEach((repo) => {
-    const parsedUrl = processGitUrl(repo.url);
-    if (parsedUrl) {
-      origins.add(parsedUrl.host);
-    } // failures are logged by parsing util fn
-  });
-  return Array.from(origins);
-};

src/proxy/routes/index.ts

@@ -3,7 +3,8 @@ import proxy from 'express-http-proxy';
 import { PassThrough } from 'stream';
 import getRawBody from 'raw-body';
 import { executeChain } from '../chain';
-import { processUrlPath, validGitRequest, getAllProxiedHosts } from './helper';
+import { processUrlPath, validGitRequest } from './helper';
+import { getAllProxiedHosts } from '../../db';
 import { ProxyOptions } from 'express-http-proxy';
 
 enum ActionType {

src/service/index.ts

@@ -9,7 +9,7 @@ import lusca from 'lusca';
 import * as config from '../config';
 import * as db from '../db';
 import { serverConfig } from '../config/env';
-import Proxy from '../proxy';
+import { Proxy } from '../proxy';
 import routes from './routes';
 import { configure } from './passport';
 
@@ -109,7 +109,7 @@ async function stop() {
   _httpServer.close();
 }
 
-export default {
+export const Service = {
   start,
   stop,
   httpServer: _httpServer,

src/service/routes/repo.ts

@@ -2,7 +2,7 @@ import express, { Request, Response } from 'express';
 
 import * as db from '../../db';
 import { getProxyURL } from '../urls';
-import { getAllProxiedHosts } from '../../proxy/routes/helper';
+import { getAllProxiedHosts } from '../../db';
 import { RepoQuery } from '../../db/types';
 import { isAdminUser } from './utils';
 

test/1.test.ts

@@ -10,9 +10,9 @@
 
 import { describe, it, beforeAll, afterAll, beforeEach, afterEach, expect, vi } from 'vitest';
 import request from 'supertest';
-import service from '../src/service';
+import { Service } from '../src/service';
 import * as db from '../src/db';
-import Proxy from '../src/proxy';
+import { Proxy } from '../src/proxy';
 
 // Create constants for values used in multiple tests
 const TEST_REPO = {
@@ -29,7 +29,7 @@ describe('init', () => {
   beforeAll(async function () {
     // Starts the service and returns the express app
     const proxy = new Proxy();
-    app = await service.start(proxy);
+    app = await Service.start(proxy);
   });
 
   // Runs before each test
@@ -52,7 +52,7 @@ describe('init', () => {
   // Runs after all tests
   afterAll(function () {
     // Must close the server to avoid EADDRINUSE errors when running tests in parallel
-    service.httpServer.close();
+    Service.httpServer.close();
   });
 
   // Example test: check server is running