fix: defer import of proxy and service until config file has been set to fix race

[kriswest]

resolves #1313

By deferring the import of the Proxy and Service we can avoid them initialising the database clients before config is loaded, which was preventing configuration to use the mongo adaptor

[netlify]

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name	Link
🔨 Latest commit	32f1585
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6937ebb8de47fc00081147d1

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.85%. Comparing base (7e625cd) to head (32f1585).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1314   +/-   ##
=======================================
  Coverage   80.85%   80.85%           
=======================================
  Files          66       66           
  Lines        4514     4514           
  Branches      778      778           
=======================================
  Hits         3650     3650           
  Misses        849      849           
  Partials       15       15           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kriswest]

This works in ts-node, but when you build the imports are hoisted resulting in the issue recurring. Will rethink.

[coopernetes]

@kriswest does this patch below work for you? This removes accessing the sink variable directly and wraps it in a simple get function. Then, we can initialize the database (also included a small optimization for the file initialization) before starting the actual Express apps.

Details

diff --git a/index.ts b/index.ts
index cc3cdea8..55dce72a 100755
--- a/index.ts
+++ b/index.ts
@@ -6,6 +6,7 @@ import { hideBin } from 'yargs/helpers';
 import * as fs from 'fs';
 import { configFile, setConfigFile, validate } from './src/config/file';
 import { initUserConfig } from './src/config';
+import { initializeDatabase } from './src/db';
 import Proxy from './src/proxy';
 import service from './src/service';
 
@@ -48,6 +49,8 @@ if (argv.v) {
 
 validate();
 
+initializeDatabase();
+
 const proxy = new Proxy();
 proxy.start();
 service.start(proxy);
diff --git a/proxy.config.json b/proxy.config.json
index a57d51da..a2353248 100644
--- a/proxy.config.json
+++ b/proxy.config.json
@@ -22,7 +22,7 @@
       "params": {
         "filepath": "./."
       },
-      "enabled": true
+      "enabled": false
     },
     {
       "type": "mongo",
@@ -33,7 +33,7 @@
         "tlsAllowInvalidCertificates": false,
         "ssl": true
       },
-      "enabled": false
+      "enabled": true
     }
   ],
   "authentication": [
diff --git a/src/db/file/index.ts b/src/db/file/index.ts
index 3f746dcf..fd541791 100644
--- a/src/db/file/index.ts
+++ b/src/db/file/index.ts
@@ -2,6 +2,11 @@ import * as users from './users';
 import * as repo from './repo';
 import * as pushes from './pushes';
 import * as helper from './helper';
+import { existsSync, mkdirSync } from 'fs';
+
+export const initializeFolders = () => {
+  if (!existsSync('./.data/db')) mkdirSync('./.data/db', { recursive: true });
+};
 
 export const { getSessionStore } = helper;
 
diff --git a/src/db/file/pushes.ts b/src/db/file/pushes.ts
index 5bdcd4df..aae31902 100644
--- a/src/db/file/pushes.ts
+++ b/src/db/file/pushes.ts
@@ -7,12 +7,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') {
diff --git a/src/db/file/repo.ts b/src/db/file/repo.ts
index 48214122..fed99157 100644
--- a/src/db/file/repo.ts
+++ b/src/db/file/repo.ts
@@ -1,4 +1,3 @@
-import fs from 'fs';
 import Datastore from '@seald-io/nedb';
 import _ from 'lodash';
 
@@ -7,12 +6,6 @@ import { toClass } from '../helper';
 
 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') {
diff --git a/src/db/file/users.ts b/src/db/file/users.ts
index a39b5b17..4025cefe 100644
--- a/src/db/file/users.ts
+++ b/src/db/file/users.ts
@@ -5,12 +5,6 @@ import { User, UserQuery } 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') {
diff --git a/src/db/index.ts b/src/db/index.ts
index d44b79f3..21277dd9 100644
--- a/src/db/index.ts
+++ b/src/db/index.ts
@@ -8,11 +8,24 @@ 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;
-}
+
+const getSink = () => {
+  if (!sink) {
+    console.error('Database sink not initialized!');
+    process.exit(1);
+  }
+  return sink;
+};
+
+export const initializeDatabase = () => {
+  console.log(`initializing database of type ${config.getDatabase().type}`);
+  if (config.getDatabase().type === 'mongo') {
+    sink = mongo;
+  } else if (config.getDatabase().type === 'fs') {
+    neDb.initializeFolders();
+    sink = neDb;
+  }
+};
 
 const isBlank = (str: string) => {
   return !str || /^\s*$/.test(str);
@@ -57,19 +70,19 @@ export const createUser = async (
     const errorMessage = `email cannot be empty`;
     throw new Error(errorMessage);
   }
-  const existingUser = await sink.findUser(username);
+  const existingUser = await getSink().findUser(username);
   if (existingUser) {
     const errorMessage = `user ${username} already exists`;
     throw new Error(errorMessage);
   }
 
-  const existingUserWithEmail = await sink.findUserByEmail(email);
+  const existingUserWithEmail = await getSink().findUserByEmail(email);
   if (existingUserWithEmail) {
     const errorMessage = `A user with email ${email} already exists`;
     throw new Error(errorMessage);
   }
 
-  await sink.createUser(data);
+  await getSink().createUser(data);
 };
 
 export const createRepo = async (repo: AuthorisedRepo) => {
@@ -95,7 +108,7 @@ export const createRepo = async (repo: AuthorisedRepo) => {
     throw new Error('URL cannot be empty');
   }
 
-  return sink.createRepo(toCreate) as Promise<Required<Repo>>;
+  return getSink().createRepo(toCreate) as Promise<Required<Repo>>;
 };
 
 export const isUserPushAllowed = async (url: string, user: string) => {
@@ -114,7 +127,7 @@ export const canUserApproveRejectPush = async (id: string, user: string) => {
     return false;
   }
 
-  const theRepo = await sink.getRepoByUrl(action.url);
+  const theRepo = await getSink().getRepoByUrl(action.url);
 
   if (theRepo?.users?.canAuthorise?.includes(user)) {
     console.log(`user ${user} can approve/reject for repo ${action.url}`);
@@ -141,34 +154,37 @@ 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);
+  getSink().getSessionStore ? getSink().getSessionStore() : undefined;
+export const getPushes = (query: Partial<PushQuery>): Promise<Action[]> =>
+  getSink().getPushes(query);
+export const writeAudit = (action: Action): Promise<void> => getSink().writeAudit(action);
+export const getPush = (id: string): Promise<Action | null> => getSink().getPush(id);
+export const deletePush = (id: string): Promise<void> => getSink().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);
+  getSink().authorise(id, attestation);
+export const cancel = (id: string): Promise<{ message: string }> => getSink().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);
+  getSink().reject(id, attestation);
+export const getRepos = (query?: Partial<RepoQuery>): Promise<Repo[]> => getSink().getRepos(query);
+export const getRepo = (name: string): Promise<Repo | null> => getSink().getRepo(name);
+export const getRepoByUrl = (url: string): Promise<Repo | null> => getSink().getRepoByUrl(url);
+export const getRepoById = (_id: string): Promise<Repo | null> => getSink().getRepoById(_id);
 export const addUserCanPush = (_id: string, user: string): Promise<void> =>
-  sink.addUserCanPush(_id, user);
+  getSink().addUserCanPush(_id, user);
 export const addUserCanAuthorise = (_id: string, user: string): Promise<void> =>
-  sink.addUserCanAuthorise(_id, user);
+  getSink().addUserCanAuthorise(_id, user);
 export const removeUserCanPush = (_id: string, user: string): Promise<void> =>
-  sink.removeUserCanPush(_id, user);
+  getSink().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);
+  getSink().removeUserCanAuthorise(_id, user);
+export const deleteRepo = (_id: string): Promise<void> => getSink().deleteRepo(_id);
+export const findUser = (username: string): Promise<User | null> => getSink().findUser(username);
+export const findUserByEmail = (email: string): Promise<User | null> =>
+  getSink().findUserByEmail(email);
+export const findUserByOIDC = (oidcId: string): Promise<User | null> =>
+  getSink().findUserByOIDC(oidcId);
+export const getUsers = (query?: Partial<UserQuery>): Promise<User[]> => getSink().getUsers(query);
+export const deleteUser = (username: string): Promise<void> => getSink().deleteUser(username);
+
+export const updateUser = (user: Partial<User>): Promise<void> => getSink().updateUser(user);
 export type { PushQuery, Repo, Sink, User } from './types';

[kriswest]

Great minds think alike @coopernetes, I have almost exactly the same PR awaiting review in Git Proxy + testing for our local deployment team. It works for me, I just need to check its still works from a bundle.

I'll copy in you initializeFolders change - I have couple of similar clean-ups in my PR.

[kriswest]

Closing to reopen a new PR (because of #1291)