Skip to content

Commit 4234c16

Browse files
dcoricdcoric
committed
Revert "refactor(config): remove deprecated proxyUrl, sslKeyPemPath, sslCertPemPath fields"
This reverts commit c48d0bf. Removing these fields is a breaking change for existing 2.x configs. Keep them in the schema as deprecated and defer removal to the next major (3.0).
1 parent 2e974d6 commit 4234c16

8 files changed

Lines changed: 424 additions & 160 deletions

File tree

‎config.schema.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
"description": "Configuration for customizing git-proxy",
66
"type": "object",
77
"properties": {
8+
"proxyUrl": {
9+
"type": "string",
10+
"description": "Deprecated: Used in early versions of git proxy to configure the remote host that traffic is proxied to. In later versions, the repository URL is used to determine the domain proxied, allowing multiple hosts to be proxied by one instance.",
11+
"deprecated": true
12+
},
813
"cookieSecret": { "type": "string" },
914
"sessionMaxAgeHours": { "type": "number" },
1015
"api": {
@@ -328,6 +333,16 @@
328333
},
329334
"required": ["enabled", "key", "cert"]
330335
},
336+
"sslKeyPemPath": {
337+
"description": "Deprecated: Path to SSL private key file (use tls.key instead)",
338+
"type": "string",
339+
"deprecated": true
340+
},
341+
"sslCertPemPath": {
342+
"description": "Deprecated: Path to SSL certificate file (use tls.cert instead)",
343+
"type": "string",
344+
"deprecated": true
345+
},
331346
"configurationSources": {
332347
"enabled": { "type": "boolean" },
333348
"reloadIntervalSeconds": { "type": "number" },

‎src/config/generated/config.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ export interface GitProxyConfig {
6565
* commitConfig.diff.block.providers
6666
*/
6767
privateOrganizations?: any[];
68+
/**
69+
* Deprecated: Used in early versions of git proxy to configure the remote host that traffic
70+
* is proxied to. In later versions, the repository URL is used to determine the domain
71+
* proxied, allowing multiple hosts to be proxied by one instance.
72+
*/
73+
proxyUrl?: string;
6874
/**
6975
* API Rate limiting configuration.
7076
*/
@@ -75,6 +81,14 @@ export interface GitProxyConfig {
7581
* used.
7682
*/
7783
sink?: Database[];
84+
/**
85+
* Deprecated: Path to SSL certificate file (use tls.cert instead)
86+
*/
87+
sslCertPemPath?: string;
88+
/**
89+
* Deprecated: Path to SSL private key file (use tls.key instead)
90+
*/
91+
sslKeyPemPath?: string;
7892
/**
7993
* Toggle the generation of temporary password for git-proxy admin user
8094
*/
@@ -779,9 +793,12 @@ const typeMap: any = {
779793
{ json: 'domains', js: 'domains', typ: u(undefined, r('Domains')) },
780794
{ json: 'plugins', js: 'plugins', typ: u(undefined, a('')) },
781795
{ json: 'privateOrganizations', js: 'privateOrganizations', typ: u(undefined, a('any')) },
796+
{ json: 'proxyUrl', js: 'proxyUrl', typ: u(undefined, '') },
782797
{ json: 'rateLimit', js: 'rateLimit', typ: u(undefined, r('RateLimit')) },
783798
{ json: 'sessionMaxAgeHours', js: 'sessionMaxAgeHours', typ: u(undefined, 3.14) },
784799
{ json: 'sink', js: 'sink', typ: u(undefined, a(r('Database'))) },
800+
{ json: 'sslCertPemPath', js: 'sslCertPemPath', typ: u(undefined, '') },
801+
{ json: 'sslKeyPemPath', js: 'sslKeyPemPath', typ: u(undefined, '') },
785802
{ json: 'tempPassword', js: 'tempPassword', typ: u(undefined, r('TempPassword')) },
786803
{ json: 'tls', js: 'tls', typ: u(undefined, r('TLS')) },
787804
{ json: 'uiRouteAuth', js: 'uiRouteAuth', typ: u(undefined, r('UIRouteAuth')) },

‎src/config/index.ts‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ import { getConfigFile } from './file';
2525
import { validateConfig } from './validators';
2626
import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
2727

28-
export type FullGitProxyConfig = Required<GitProxyConfig>;
28+
// Deprecated compatibility fields are still optional because the defaults do not set them.
29+
type OptionalTopLevelConfigKey = 'proxyUrl' | 'sslCertPemPath' | 'sslKeyPemPath';
30+
type RequiredTopLevelConfigKey = Exclude<keyof GitProxyConfig, OptionalTopLevelConfigKey>;
31+
32+
export type FullGitProxyConfig = Required<Omit<GitProxyConfig, OptionalTopLevelConfigKey>> &
33+
Pick<GitProxyConfig, OptionalTopLevelConfigKey>;
2934

3035
const REQUIRED_TOP_LEVEL_CONFIG_KEYS = [
3136
'api',
@@ -49,10 +54,10 @@ const REQUIRED_TOP_LEVEL_CONFIG_KEYS = [
4954
'uiRouteAuth',
5055
'upstreamProxy',
5156
'urlShortener',
52-
] as const satisfies readonly (keyof GitProxyConfig)[];
57+
] as const satisfies readonly RequiredTopLevelConfigKey[];
5358

5459
type MissingRequiredTopLevelConfigKeys = Exclude<
55-
keyof GitProxyConfig,
60+
RequiredTopLevelConfigKey,
5661
(typeof REQUIRED_TOP_LEVEL_CONFIG_KEYS)[number]
5762
>;
5863
type AssertNever<T extends never> = T;
@@ -151,6 +156,19 @@ function mergeConfigurations(
151156
defaultConfig: GitProxyConfig,
152157
userSettings: Partial<GitProxyConfig>,
153158
): FullGitProxyConfig {
159+
// Special handling for TLS configuration when legacy fields are used
160+
let tlsConfig = userSettings.tls || defaultConfig.tls;
161+
162+
// If user doesn't specify tls but has legacy SSL fields, use only legacy fallback
163+
if (!userSettings.tls && (userSettings.sslKeyPemPath || userSettings.sslCertPemPath)) {
164+
tlsConfig = {
165+
enabled: defaultConfig.tls?.enabled || false,
166+
// Use empty strings so legacy fallback works
167+
key: '',
168+
cert: '',
169+
};
170+
}
171+
154172
const config = {
155173
...defaultConfig,
156174
...userSettings,
@@ -160,8 +178,11 @@ function mergeConfigurations(
160178
commitConfig: { ...defaultConfig.commitConfig, ...userSettings.commitConfig },
161179
attestationConfig: { ...defaultConfig.attestationConfig, ...userSettings.attestationConfig },
162180
rateLimit: userSettings.rateLimit || defaultConfig.rateLimit,
163-
tls: userSettings.tls || defaultConfig.tls,
181+
tls: tlsConfig,
164182
tempPassword: { ...defaultConfig.tempPassword, ...userSettings.tempPassword },
183+
// Preserve legacy SSL fields
184+
sslKeyPemPath: userSettings.sslKeyPemPath || defaultConfig.sslKeyPemPath,
185+
sslCertPemPath: userSettings.sslCertPemPath || defaultConfig.sslCertPemPath,
165186
// Environment variable overrides
166187
cookieSecret:
167188
serverConfig.GIT_PROXY_COOKIE_SECRET ||
@@ -173,6 +194,12 @@ function mergeConfigurations(
173194
return config;
174195
}
175196

197+
// Get configured proxy URL
198+
export const getProxyUrl = (): string | undefined => {
199+
const config = loadFullConfiguration();
200+
return config.proxyUrl;
201+
};
202+
176203
/**
177204
* Redacts the userinfo (credentials) from a proxy URL for safe logging.
178205
* e.g. http://user:pass@proxy.corp.local:8080 → http://<redacted>@proxy.corp.local:8080
@@ -330,12 +357,12 @@ export const getPlugins = () => {
330357

331358
export const getTLSKeyPemPath = (): string | undefined => {
332359
const config = loadFullConfiguration();
333-
return config.tls?.key || undefined;
360+
return config.tls?.key && config.tls.key !== '' ? config.tls.key : config.sslKeyPemPath;
334361
};
335362

336363
export const getTLSCertPemPath = (): string | undefined => {
337364
const config = loadFullConfiguration();
338-
return config.tls?.cert || undefined;
365+
return config.tls?.cert && config.tls.cert !== '' ? config.tls.cert : config.sslCertPemPath;
339366
};
340367

341368
export const getTLSEnabled = (): boolean => {

‎test/ConfigLoader.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe('ConfigLoader', () => {
6868
describe('loadFromFile', () => {
6969
it('should load configuration from file', async () => {
7070
const testConfig = {
71-
contactEmail: 'admin@test.com',
71+
proxyUrl: 'https://test.com',
7272
cookieSecret: 'test-secret',
7373
};
7474
fs.writeFileSync(tempConfigFile, JSON.stringify(testConfig));
@@ -81,15 +81,15 @@ describe('ConfigLoader', () => {
8181
});
8282

8383
expect(result).toBeTypeOf('object');
84-
expect(result.contactEmail).toBe('admin@test.com');
84+
expect(result.proxyUrl).toBe('https://test.com');
8585
expect(result.cookieSecret).toBe('test-secret');
8686
});
8787
});
8888

8989
describe('loadFromHttp', () => {
9090
it('should load configuration from HTTP endpoint', async () => {
9191
const testConfig = {
92-
contactEmail: 'admin@test.com',
92+
proxyUrl: 'https://test.com',
9393
cookieSecret: 'test-secret',
9494
};
9595

@@ -104,7 +104,7 @@ describe('ConfigLoader', () => {
104104
});
105105

106106
expect(result).toBeTypeOf('object');
107-
expect(result.contactEmail).toBe('admin@test.com');
107+
expect(result.proxyUrl).toBe('https://test.com');
108108
expect(result.cookieSecret).toBe('test-secret');
109109
});
110110

@@ -145,7 +145,7 @@ describe('ConfigLoader', () => {
145145
};
146146

147147
const newConfig = {
148-
contactEmail: 'admin@new-test.com',
148+
proxyUrl: 'https://new-test.com',
149149
};
150150

151151
fs.writeFileSync(tempConfigFile, JSON.stringify(newConfig));
@@ -162,7 +162,7 @@ describe('ConfigLoader', () => {
162162

163163
it('should not emit event if config has not changed', async () => {
164164
const testConfig = {
165-
contactEmail: 'admin@test.com',
165+
proxyUrl: 'https://test.com',
166166
};
167167

168168
const config: Configuration = {
@@ -211,7 +211,7 @@ describe('ConfigLoader', () => {
211211

212212
it('should skip reload and log error when configuration is invalid', async () => {
213213
const invalidConfig = {
214-
contactEmail: 'admin@test.com',
214+
proxyUrl: 'https://test.com',
215215
commitConfig: {
216216
author: {
217217
email: {
@@ -261,7 +261,7 @@ describe('ConfigLoader', () => {
261261

262262
it('should successfully reload when configuration is valid', async () => {
263263
const validConfig = {
264-
contactEmail: 'admin@test.com',
264+
proxyUrl: 'https://test.com',
265265
commitConfig: {
266266
author: {
267267
email: {

‎test/configValidators.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('validators', () => {
3737

3838
it('should return true for config without commitConfig', () => {
3939
const config: GitProxyConfig = {
40-
contactEmail: 'admin@test.com',
40+
proxyUrl: 'https://test.com',
4141
cookieSecret: 'secret',
4242
};
4343
expect(validateConfig(config)).toBe(true);

‎test/generated-config.test.ts‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ describe('Generated Config (QuickType)', () => {
2222
describe('Convert class', () => {
2323
it('should parse valid configuration JSON', () => {
2424
const validConfig = {
25-
contactEmail: 'admin@example.com',
25+
proxyUrl: 'https://proxy.example.com',
2626
cookieSecret: 'test-secret',
2727
authorisedList: [
2828
{
@@ -48,7 +48,7 @@ describe('Generated Config (QuickType)', () => {
4848
const result = Convert.toGitProxyConfig(JSON.stringify(validConfig));
4949

5050
assert.isObject(result);
51-
expect(result.contactEmail).toBe('admin@example.com');
51+
expect(result.proxyUrl).toBe('https://proxy.example.com');
5252
expect(result.cookieSecret).toBe('test-secret');
5353
assert.isArray(result.authorisedList);
5454
assert.isArray(result.authentication);
@@ -57,7 +57,7 @@ describe('Generated Config (QuickType)', () => {
5757

5858
it('should convert config object back to JSON', () => {
5959
const configObject = {
60-
contactEmail: 'admin@example.com',
60+
proxyUrl: 'https://proxy.example.com',
6161
cookieSecret: 'test-secret',
6262
authorisedList: [],
6363
authentication: [
@@ -72,7 +72,7 @@ describe('Generated Config (QuickType)', () => {
7272
const parsed = JSON.parse(jsonString);
7373

7474
assert.isObject(parsed);
75-
expect(parsed.contactEmail).toBe('admin@example.com');
75+
expect(parsed.proxyUrl).toBe('https://proxy.example.com');
7676
expect(parsed.cookieSecret).toBe('test-secret');
7777
});
7878

@@ -91,7 +91,7 @@ describe('Generated Config (QuickType)', () => {
9191

9292
it('should handle configuration with valid rate limit structure', () => {
9393
const validConfig = {
94-
contactEmail: 'admin@example.com',
94+
proxyUrl: 'https://proxy.example.com',
9595
cookieSecret: 'secret',
9696
sessionMaxAgeHours: 24,
9797
rateLimit: {
@@ -121,6 +121,7 @@ describe('Generated Config (QuickType)', () => {
121121
enabled: true,
122122
},
123123
],
124+
contactEmail: 'admin@example.com',
124125
csrfProtection: true,
125126
plugins: [],
126127
privateOrganizations: ['private-org'],
@@ -137,15 +138,15 @@ describe('Generated Config (QuickType)', () => {
137138
assert.isBoolean(result.csrfProtection);
138139
assert.isArray(result.plugins);
139140
assert.isArray(result.privateOrganizations);
140-
assert.isString(result.contactEmail);
141+
assert.isString(result.proxyUrl);
141142
assert.isObject(result.rateLimit);
142143
assert.isNumber(result.sessionMaxAgeHours);
143144
assert.isArray(result.sink);
144145
});
145146

146147
it('should handle malformed configuration gracefully', () => {
147148
const malformedConfig = {
148-
contactEmail: 123, // Wrong type
149+
proxyUrl: 123, // Wrong type
149150
authentication: 'not-an-array', // Wrong type
150151
};
151152

@@ -154,7 +155,7 @@ describe('Generated Config (QuickType)', () => {
154155

155156
it('should preserve array structures', () => {
156157
const configWithArrays = {
157-
contactEmail: 'admin@example.com',
158+
proxyUrl: 'https://proxy.example.com',
158159
cookieSecret: 'secret',
159160
authorisedList: [
160161
{ project: 'proj1', name: 'repo1', url: 'https://github.com/proj1/repo1.git' },
@@ -176,7 +177,7 @@ describe('Generated Config (QuickType)', () => {
176177

177178
it('should handle nested object structures', () => {
178179
const configWithNesting = {
179-
contactEmail: 'admin@example.com',
180+
proxyUrl: 'https://proxy.example.com',
180181
cookieSecret: 'secret',
181182
authentication: [{ type: 'local', enabled: true }],
182183
sink: [{ type: 'fs', enabled: true }],
@@ -206,7 +207,7 @@ describe('Generated Config (QuickType)', () => {
206207
it('should handle complex validation scenarios', () => {
207208
// Test configuration that will trigger more validation paths
208209
const complexConfig = {
209-
contactEmail: 'admin@example.com',
210+
proxyUrl: 'https://proxy.example.com',
210211
cookieSecret: 'secret',
211212
authentication: [{ type: 'local', enabled: true }],
212213
sink: [{ type: 'fs', enabled: true }],
@@ -245,7 +246,7 @@ describe('Generated Config (QuickType)', () => {
245246

246247
it('should handle array validation edge cases', () => {
247248
const configWithArrays = {
248-
contactEmail: 'admin@example.com',
249+
proxyUrl: 'https://proxy.example.com',
249250
cookieSecret: 'secret',
250251
authentication: [{ type: 'local', enabled: true }],
251252
sink: [{ type: 'fs', enabled: true }],
@@ -276,7 +277,7 @@ describe('Generated Config (QuickType)', () => {
276277

277278
it('should exercise transformation functions with edge cases', () => {
278279
const edgeCaseConfig = {
279-
contactEmail: 'admin@example.com',
280+
proxyUrl: 'https://proxy.example.com',
280281
cookieSecret: 'secret',
281282
authentication: [{ type: 'local', enabled: true }],
282283
sink: [{ type: 'fs', enabled: true }],
@@ -315,13 +316,14 @@ describe('Generated Config (QuickType)', () => {
315316

316317
it('should test validation error paths', () => {
317318
assert.throws(() =>
318-
Convert.toGitProxyConfig('{"contactEmail": 123, "authentication": "not-array"}'),
319+
Convert.toGitProxyConfig('{"proxyUrl": 123, "authentication": "not-array"}'),
319320
);
320321
});
321322

322323
it('should test date and null handling', () => {
323324
// Test that null values cause validation errors (covers error paths)
324325
const configWithNulls = {
326+
proxyUrl: 'https://proxy.example.com',
325327
cookieSecret: null,
326328
authentication: [{ type: 'local', enabled: true }],
327329
sink: [{ type: 'fs', enabled: true }],
@@ -336,7 +338,7 @@ describe('Generated Config (QuickType)', () => {
336338

337339
it('should test serialization back to JSON', () => {
338340
const testConfig = {
339-
contactEmail: 'admin@test.com',
341+
proxyUrl: 'https://test.com',
340342
cookieSecret: 'secret',
341343
authentication: [{ type: 'local', enabled: true }],
342344
sink: [{ type: 'fs', enabled: true }],
@@ -354,7 +356,7 @@ describe('Generated Config (QuickType)', () => {
354356
const serialized = Convert.gitProxyConfigToJson(parsed);
355357
const reparsed = JSON.parse(serialized);
356358

357-
expect(reparsed.contactEmail).toBe('admin@test.com');
359+
expect(reparsed.proxyUrl).toBe('https://test.com');
358360
assert.isObject(reparsed.rateLimit);
359361
});
360362

0 commit comments

Comments
 (0)