From f0038dbe4acfa32faf23db0c883039c107d90a28 Mon Sep 17 00:00:00 2001 From: Amit Vijapur Date: Mon, 31 Aug 2026 21:17:43 +0200 Subject: [PATCH 1/2] refactor(config): drive deprecation warnings from config.schema.json `getDeprecatedConfigWarnings` hardcoded a check per deprecated field while `config.schema.json` already marked the same three fields `"deprecated": true`, so the two had to be kept in step by hand. The warnings are now generated from the schema. Adding `"deprecated": true` to a property is enough to produce a warning; `x-deprecated-replacement` names the field that supersedes it, and its absence produces the "deprecated and ignored" wording instead. Warning text is unchanged, and quicktype ignores the custom keyword, so `src/config/generated/config.ts` regenerates byte for byte. Closes #1680 --- config.schema.json | 6 ++++-- src/config/deprecatedFields.ts | 37 ++++++++++++++++++++++++---------- test/deprecatedFields.test.ts | 16 +++++++++++++++ 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/config.schema.json b/config.schema.json index a54d828d4..3638cf485 100644 --- a/config.schema.json +++ b/config.schema.json @@ -372,12 +372,14 @@ "sslKeyPemPath": { "description": "Deprecated: Path to SSL private key file (use tls.key instead)", "type": "string", - "deprecated": true + "deprecated": true, + "x-deprecated-replacement": "tls.key" }, "sslCertPemPath": { "description": "Deprecated: Path to SSL certificate file (use tls.cert instead)", "type": "string", - "deprecated": true + "deprecated": true, + "x-deprecated-replacement": "tls.cert" }, "configurationSources": { "enabled": { "type": "boolean" }, diff --git a/src/config/deprecatedFields.ts b/src/config/deprecatedFields.ts index 972550166..ae9158dcb 100644 --- a/src/config/deprecatedFields.ts +++ b/src/config/deprecatedFields.ts @@ -14,27 +14,42 @@ * limitations under the License. */ +import _ from 'lodash'; +import schema from '../../config.schema.json'; import { GitProxyConfig } from './generated/config'; +interface SchemaProperty { + deprecated?: boolean; + 'x-deprecated-replacement'?: string; +} + +const schemaProperties = (schema as { properties: Record }).properties; + +function isSet(value: unknown): boolean { + return typeof value === 'string' && value.trim() !== ''; +} + /** * Returns deprecation warnings for legacy top-level config keys in user overrides. * PR 3.0 (#1545) will replace warnings with startup failure for legacy-only configs. */ export function getDeprecatedConfigWarnings(userSettings: Partial): string[] { + const settings = userSettings as Record; const warnings: string[] = []; - if (userSettings.sslKeyPemPath?.trim() && !userSettings.tls?.key?.trim()) { - warnings.push('"sslKeyPemPath" is deprecated; use "tls.key" instead (removal in GitProxy 3.0)'); - } - - if (userSettings.sslCertPemPath?.trim() && !userSettings.tls?.cert?.trim()) { - warnings.push( - '"sslCertPemPath" is deprecated; use "tls.cert" instead (removal in GitProxy 3.0)', - ); - } + for (const [key, property] of Object.entries(schemaProperties)) { + if (!property.deprecated || !isSet(settings[key])) { + continue; + } - if (typeof userSettings.proxyUrl === 'string' && userSettings.proxyUrl.trim() !== '') { - warnings.push('"proxyUrl" is deprecated and ignored; remove it before GitProxy 3.0'); + const replacement = property['x-deprecated-replacement']; + if (!replacement) { + warnings.push(`"${key}" is deprecated and ignored; remove it before GitProxy 3.0`); + } else if (!isSet(_.get(settings, replacement))) { + warnings.push( + `"${key}" is deprecated; use "${replacement}" instead (removal in GitProxy 3.0)`, + ); + } } return warnings; diff --git a/test/deprecatedFields.test.ts b/test/deprecatedFields.test.ts index aee06fcef..16089e352 100644 --- a/test/deprecatedFields.test.ts +++ b/test/deprecatedFields.test.ts @@ -33,6 +33,22 @@ describe('getDeprecatedConfigWarnings', () => { ); }); + it('warns for a deprecated field that has no replacement', () => { + expect(getDeprecatedConfigWarnings({ proxyUrl: 'https://github.com' })).toEqual([ + '"proxyUrl" is deprecated and ignored; remove it before GitProxy 3.0', + ]); + }); + + it('stays quiet when the replacement is already set', () => { + expect( + getDeprecatedConfigWarnings({ + sslKeyPemPath: 'key.pem', + sslCertPemPath: 'cert.pem', + tls: { enabled: true, key: 'k.pem', cert: 'c.pem' }, + }), + ).toEqual([]); + }); + it('returns no warnings for non-deprecated overrides', () => { expect(getDeprecatedConfigWarnings({ uiPort: 9000 })).toEqual([]); expect( From 14c727021c656dd31a15ec9f5b4fa5b1bbdc9ad4 Mon Sep 17 00:00:00 2001 From: Amit Vijapur Date: Thu, 3 Sep 2026 20:48:53 +0100 Subject: [PATCH 2/2] fix: treat any non-empty value as set in deprecation checks Applies review feedback from @jescalada. The previous guard only counted strings as set, so a non-string legacy value (or a non-string replacement) was silently ignored instead of warned about. Signed-off-by: Amit Vijapur --- src/config/deprecatedFields.ts | 4 +++- test/deprecatedFields.test.ts | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/config/deprecatedFields.ts b/src/config/deprecatedFields.ts index ae9158dcb..296e2663d 100644 --- a/src/config/deprecatedFields.ts +++ b/src/config/deprecatedFields.ts @@ -26,7 +26,9 @@ interface SchemaProperty { const schemaProperties = (schema as { properties: Record }).properties; function isSet(value: unknown): boolean { - return typeof value === 'string' && value.trim() !== ''; + return ( + value !== undefined && value !== null && (typeof value !== 'string' || value.trim() !== '') + ); } /** diff --git a/test/deprecatedFields.test.ts b/test/deprecatedFields.test.ts index 16089e352..7848f16bf 100644 --- a/test/deprecatedFields.test.ts +++ b/test/deprecatedFields.test.ts @@ -39,6 +39,11 @@ describe('getDeprecatedConfigWarnings', () => { ]); }); + it('treats non-string values as set', () => { + expect(getDeprecatedConfigWarnings({ proxyUrl: 0 } as never)).toHaveLength(1); + expect(getDeprecatedConfigWarnings({ proxyUrl: false } as never)).toHaveLength(1); + }); + it('stays quiet when the replacement is already set', () => { expect( getDeprecatedConfigWarnings({