Repository: finos/git-proxy

Issue:
user settings no longer respected
Regression via 33c27a1

Given a user `proxy.config.json`, values specified in this file are _not_ set by the configuration loading steps. This is related to the use of a module variable, which is initialized when the `src/config` module is imported but is then is not set in time for configuration to be loaded into the app. See minimal repro below:

```bash
$ git clone https://github.com/finos/git-proxy && cd git-proxy
Cloning into 'git-proxy'...
remote: Enumerating objects: 14377, done.
remote: Counting objects: 100% (254/254), done.
remote: Compressing objects: 100% (135/135), done.
remote: Total 14377 (delta 204), reused 120 (delta 119), pack-reused 14123 (from 3)
Receiving objects: 100% (14377/14377), 15.82 MiB | 4.03 MiB/s, done.
Resolving deltas: 100% (8680/8680), done.
$ npm install @finos/git-proxy-plugin-samples

added 1 package, and changed 1 package in 9s
$ jq -n '{plugins:["finos/git-proxy-plugin-samples"]}' > user.config.json
$ npm run server -- --config user.config.json

> @finos/git-proxy@1.19.1 server
> tsx index.ts --config user.config.json

Configuration sources are disabled
No plugins configured
Found 0 plugin modules <--- No plugins loaded
Service Listening on 8080
HTTP Proxy Listening on 8000
$ npm run server -- --config user.config.json -v

> @finos/git-proxy@1.19.1 server
> tsx index.ts --config user.config.json -v

Configuration sources are disabled
user.config.json is valid
```

Wrapping the setting of `_userSettings` in a function + calling that during app startup should restore the prior behaviour.

```diff
diff --git a/index.ts b/index.ts
index 880ccfe..767f887 100755
--- a/index.ts
+++ b/index.ts
@@ -4,6 +4,7 @@ import yargs from 'yargs';
 import { hideBin } from 'yargs/helpers';
 import * as fs from 'fs';
 import { configFile, setConfigFile, validate } from './src/config/file';
+import { initUserConfig } from './src/config';
 import proxy from './src/proxy';
 import service from './src/service';
 
@@ -29,6 +30,7 @@ const argv = yargs(hideBin(process.argv))
   .parseSync();
 
 setConfigFile(argv.c as string || "");
+initUserConfig();
 
 if (argv.v) {
   if (!fs.existsSync(configFile)) {
diff --git a/src/config/index.ts b/src/config/index.ts
index a13cfef..2d6406d 100644
--- a/src/config/index.ts
+++ b/src/config/index.ts
@@ -14,9 +14,15 @@ import {
 } from './types';
 
 let _userSettings: UserSettings | null = null;
-if (existsSync(configFile)) {
-  _userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
-}
+console.log(`_userSettings during import: ${_userSettings}`); // for debugging only
+
+export const initUserConfig = () => {
+  console.log(`Initializing user configuration from ${configFile}`); // for debugging only
+  if (existsSync(configFile)) {
+    _userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
+  }
+};
+
 let _authorisedList: AuthorisedRepo[] = defaultSettings.authorisedList;
 let _database: Database[] = defaultSettings.sink;
 let _authentication: Authentication[] = defaultSettings.authentication;
```

```bash
$ npm run server -- --config user.config.json

> @finos/git-proxy@1.19.1 server
> tsx index.ts --config user.config.json

_userSettings during import: null
Configuration sources are disabled
Initializing user configuration from user.config.json
Service Listening on 8080
Configuration sources are disabled
Found 1 plugin modules
found pull plugin RunOnPullPlugin
Loaded plugin: RunOnPullPlugin
HTTP Proxy Listening on 8000
```

FYI @jescalada 

Discussion:
@jescalada: Hi @coopernetes, sorry for the delay! I was able to reproduce this issue and fix it with the diff you provided. Would you mind opening a PR for this - the full credit goes to you 😃 @coopernetes: @jescalada Will do, I'll raise a PR on this one. thanks! 

Solution patch:
diff --git a/index.ts b/index.ts
index cc72b6e0..29da39b4 100755
--- a/index.ts
+++ b/index.ts
@@ -5,6 +5,7 @@ import yargs from 'yargs';
 import { hideBin } from 'yargs/helpers';
 import * as fs from 'fs';
 import { configFile, setConfigFile, validate } from './src/config/file';
+import { initUserConfig } from './src/config';
 import proxy from './src/proxy';
 import service from './src/service';
 
@@ -30,6 +31,7 @@ const argv = yargs(hideBin(process.argv))
   .parseSync();
 
 setConfigFile(argv.c as string || "");
+initUserConfig();
 
 if (argv.v) {
   if (!fs.existsSync(configFile)) {
diff --git a/src/config/index.ts b/src/config/index.ts
index f853ef2e..7ffb6a96 100644
--- a/src/config/index.ts
+++ b/src/config/index.ts
@@ -14,9 +14,15 @@ import {
 } from './types';
 
 let _userSettings: UserSettings | null = null;
-if (existsSync(configFile)) {
-  _userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
-}
+console.log(`_userSettings during import: ${_userSettings}`); // for debugging only
+
+export const initUserConfig = () => {
+  console.log(`Initializing user configuration from ${configFile}`); // for debugging only
+  if (existsSync(configFile)) {
+    _userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
+  }
+};
+
 let _authorisedList: AuthorisedRepo[] = defaultSettings.authorisedList;
 let _database: Database[] = defaultSettings.sink;
 let _authentication: Authentication[] = defaultSettings.authentication;



Test patch:
diff --git a/test/testActiveDirectoryAuth.test.js b/test/testActiveDirectoryAuth.test.js
index 4d50dbb4..32d29174 100644
--- a/test/testActiveDirectoryAuth.test.js
+++ b/test/testActiveDirectoryAuth.test.js
@@ -50,6 +50,9 @@ describe('ActiveDirectory auth method', () => {
       fs: fsStub,
     });
 
+    // Initialize the user config after proxyquiring to load the stubbed config
+    config.initUserConfig();
+
     const { configure } = proxyquire('../src/service/passport/activeDirectory', {
       './ldaphelper': ldapStub,
       '../../db': dbStub,
diff --git a/test/testAuthMethods.test.js b/test/testAuthMethods.test.js
index 013c79d8..c6c5b9c0 100644
--- a/test/testAuthMethods.test.js
+++ b/test/testAuthMethods.test.js
@@ -30,6 +30,9 @@ describe('auth methods', async () => {
     const config = proxyquire('../src/config', {
       fs: fsStub,
     });
+
+    // Initialize the user config after proxyquiring to load the stubbed config
+    config.initUserConfig();
   
     expect(() => config.getAuthMethods()).to.throw(Error, 'No authentication method enabled');
   });
@@ -52,6 +55,9 @@ describe('auth methods', async () => {
       fs: fsStub,
     });
 
+    // Initialize the user config after proxyquiring to load the stubbed config
+    config.initUserConfig();
+
     const authMethods = config.getAuthMethods();
     expect(authMethods).to.have.lengthOf(3);
     expect(authMethods[0].type).to.equal('local');
diff --git a/test/testConfig.test.js b/test/testConfig.test.js
index e6ea0f75..2d34d91d 100644
--- a/test/testConfig.test.js
+++ b/test/testConfig.test.js
@@ -11,7 +11,7 @@ describe('default configuration', function () {
   it('should use default values if no user-settings.json file exists', function () {
     const config = require('../src/config');
     config.logConfiguration();
-    const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
+    const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
 
     expect(config.getAuthMethods()).to.deep.equal(enabledMethods);
     expect(config.getDatabase()).to.be.eql(defaultSettings.sink[0]);
@@ -41,26 +41,22 @@ describe('user configuration', function () {
 
   beforeEach(function () {
     delete require.cache[require.resolve('../src/config/env')];
+    delete require.cache[require.resolve('../src/config')];
     oldEnv = { ...process.env };
     tempDir = fs.mkdtempSync('gitproxy-test');
     tempUserFile = path.join(tempDir, 'test-settings.json');
-    require('../src/config/file').configFile = tempUserFile;
+    require('../src/config/file').setConfigFile(tempUserFile);
   });
 
   it('should override default settings for authorisedList', function () {
     const user = {
-      authorisedList: [
-        {
-          project: 'foo',
-          name: 'bar',
-          url: 'https://github.com/foo/bar.git',
-        },
-      ],
+      authorisedList: [{ project: 'foo', name: 'bar', url: 'https://github.com/foo/bar.git' }],
     };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
-    const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
+    config.initUserConfig();
+    const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
 
     expect(config.getAuthorisedList()).to.be.eql(user.authorisedList);
     expect(config.getAuthMethods()).to.deep.equal(enabledMethods);
@@ -69,19 +65,13 @@ describe('user configuration', function () {
   });
 
   it('should override default settings for authentication', function () {
-    const user = {
-      authentication: [
-        {
-          type: 'google',
-          enabled: true,
-        },
-      ],
-    };
+    const user = { authentication: [{ type: 'google', enabled: true }] };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
     const authMethods = config.getAuthMethods();
-    const googleAuth = authMethods.find(method => method.type === 'google');
+    const googleAuth = authMethods.find((method) => method.type === 'google');
 
     expect(googleAuth).to.not.be.undefined;
     expect(googleAuth.enabled).to.be.true;
@@ -92,18 +82,12 @@ describe('user configuration', function () {
   });
 
   it('should override default settings for database', function () {
-    const user = {
-      sink: [
-        {
-          type: 'postgres',
-          enabled: true,
-        },
-      ],
-    };
+    const user = { sink: [{ type: 'postgres', enabled: true }] };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
-    const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
+    config.initUserConfig();
+    const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
 
     expect(config.getDatabase()).to.be.eql(user.sink[0]);
     expect(config.getDatabase()).to.not.be.eql(defaultSettings.sink[0]);
@@ -112,30 +96,22 @@ describe('user configuration', function () {
   });
 
   it('should override default settings for SSL certificate', function () {
-    const user = {
-      tls: {
-        key: 'my-key.pem',
-        cert: 'my-cert.pem',
-      },
-    };
+    const user = { tls: { key: 'my-key.pem', cert: 'my-cert.pem' } };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
     expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
   });
 
   it('should override default settings for rate limiting', function () {
-    const limitConfig = {
-      rateLimit: {
-        windowMs: 60000,
-        limit: 1500,
-      },
-    };
+    const limitConfig = { rateLimit: { windowMs: 60000, limit: 1500 } };
     fs.writeFileSync(tempUserFile, JSON.stringify(limitConfig));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getRateLimit().windowMs).to.be.eql(limitConfig.rateLimit.windowMs);
     expect(config.getRateLimit().limit).to.be.eql(limitConfig.rateLimit.limit);
@@ -145,68 +121,55 @@ describe('user configuration', function () {
     const user = {
       attestationConfig: {
         questions: [
-          {
-            label: 'Testing Label Change',
-            tooltip: {
-              text: 'Testing Tooltip Change',
-              links: [],
-            },
-          },
+          { label: 'Testing Label Change', tooltip: { text: 'Testing Tooltip Change', links: [] } },
         ],
       },
     };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getAttestationConfig()).to.be.eql(user.attestationConfig);
   });
 
   it('should override default settings for url shortener', function () {
-    const user = {
-      urlShortener: 'https://url-shortener.com',
-    };
+    const user = { urlShortener: 'https://url-shortener.com' };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getURLShortener()).to.be.eql(user.urlShortener);
   });
 
   it('should override default settings for contact email', function () {
-    const user = {
-      contactEmail: 'test@example.com',
-    };
+    const user = { contactEmail: 'test@example.com' };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getContactEmail()).to.be.eql(user.contactEmail);
   });
 
   it('should override default settings for plugins', function () {
-    const user = {
-      plugins: ['plugin1', 'plugin2'],
-    };
+    const user = { plugins: ['plugin1', 'plugin2'] };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getPlugins()).to.be.eql(user.plugins);
   });
 
   it('should override default settings for sslCertPemPath', function () {
-    const user = {
-      tls: {
-        enabled: true,
-        key: 'my-key.pem',
-        cert: 'my-cert.pem',
-      }
-    };
+    const user = { tls: { enabled: true, key: 'my-key.pem', cert: 'my-cert.pem' } };
 
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
     expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
@@ -215,17 +178,14 @@ describe('user configuration', function () {
 
   it('should prioritize tls.key and tls.cert over sslKeyPemPath and sslCertPemPath', function () {
     const user = {
-      tls: {
-        enabled: true,
-        key: 'good-key.pem',
-        cert: 'good-cert.pem',
-      },
+      tls: { enabled: true, key: 'good-key.pem', cert: 'good-cert.pem' },
       sslKeyPemPath: 'bad-key.pem',
       sslCertPemPath: 'bad-cert.pem',
     };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
-    
+
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
     expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
@@ -233,13 +193,11 @@ describe('user configuration', function () {
   });
 
   it('should use sslKeyPemPath and sslCertPemPath if tls.key and tls.cert are not present', function () {
-    const user = {
-      sslKeyPemPath: 'good-key.pem',
-      sslCertPemPath: 'good-cert.pem',
-    };
+    const user = { sslKeyPemPath: 'good-key.pem', sslCertPemPath: 'good-cert.pem' };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
 
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getTLSCertPemPath()).to.be.eql(user.sslCertPemPath);
     expect(config.getTLSKeyPemPath()).to.be.eql(user.sslKeyPemPath);
@@ -247,41 +205,31 @@ describe('user configuration', function () {
   });
 
   it('should override default settings for api', function () {
-    const user = {
-      api: {
-        gitlab: {
-          baseUrl: 'https://gitlab.com',
-        },
-      },
-    };
+    const user = { api: { gitlab: { baseUrl: 'https://gitlab.com' } } };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
-    
+
     const config = require('../src/config');
+    config.initUserConfig();
 
     expect(config.getAPIs()).to.be.eql(user.api);
   });
 
   it('should override default settings for cookieSecret if env var is used', function () {
     fs.writeFileSync(tempUserFile, '{}');
-    process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret'
+    process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret';
 
     const config = require('../src/config');
+    config.initUserConfig();
     expect(config.getCookieSecret()).to.equal('test-cookie-secret');
   });
 
   it('should override default settings for mongo connection string if env var is used', function () {
-    const user = {
-      sink: [
-        {
-          type: 'mongo',
-          enabled: true,
-        }
-      ]
-    };
+    const user = { sink: [{ type: 'mongo', enabled: true }] };
     fs.writeFileSync(tempUserFile, JSON.stringify(user));
     process.env.GIT_PROXY_MONGO_CONNECTION_STRING = 'mongodb://example.com:27017/test';
 
     const config = require('../src/config');
+    config.initUserConfig();
     expect(config.getDatabase().connectionString).to.equal('mongodb://example.com:27017/test');
   });