From 369dbd243d49145971d6aac109c107d007587436 Mon Sep 17 00:00:00 2001 From: preko06 Date: Thu, 4 Jun 2026 01:53:35 +0200 Subject: [PATCH] Fix env overrides for empty config values --- src/config/config.server.ts | 35 +---- src/config/env-config-overrides.spec.ts | 163 ++++++++++++++++++++++++ src/config/env-config-overrides.ts | 152 ++++++++++++++++++++++ 3 files changed, 320 insertions(+), 30 deletions(-) create mode 100644 src/config/env-config-overrides.spec.ts create mode 100644 src/config/env-config-overrides.ts diff --git a/src/config/config.server.ts b/src/config/config.server.ts index 7e153121f56..07dd5afbed9 100644 --- a/src/config/config.server.ts +++ b/src/config/config.server.ts @@ -26,6 +26,7 @@ import { import { Config } from './config.interface'; import { mergeConfig } from './config.util'; import { DefaultAppConfig } from './default-app-config'; +import { applyEnvironmentConfigOverrides } from './env-config-overrides'; import { ServerConfig } from './server-config.interface'; const CONFIG_PATH = join(process.cwd(), 'config'); @@ -135,35 +136,8 @@ const overrideWithConfig = (config: Config, pathToConfig: string) => { } }; -const overrideWithEnvironment = (config: Config, key: string = '') => { - // eslint-disable-next-line guard-for-in - for (const property in config) { - const variable = `${key}${isNotEmpty(key) ? '_' : ''}${property.toUpperCase()}`; - const innerConfig = config[property]; - if (isNotEmpty(innerConfig)) { - if (typeof innerConfig === 'object') { - overrideWithEnvironment(innerConfig, variable); - } else { - const value = ENV(variable, true); - if (isNotEmpty(value)) { - console.info(`Applying environment variable ${DSPACE(variable)} with value ${value}`); - switch (typeof innerConfig) { - case 'number': - config[property] = getNumberFromString(value); - break; - case 'boolean': - config[property] = getBooleanFromString(value); - break; - case 'string': - config[property] = value; - break; - default: - console.warn(`Unsupported environment variable type ${typeof innerConfig} ${DSPACE(variable)}`); - } - } - } - } - } +const overrideWithEnvironment = (config: Config) => { + applyEnvironmentConfigOverrides(config, ENV); }; @@ -237,7 +211,8 @@ export const buildAppConfig = (destConfigPath?: string, mapping?: ServerHashedFi } } - // override with environment variables + // Ordering invariant: environment overrides must run before buildBaseUrl() + // so DSPACE_REST_BASEURL can suppress computed rest.baseUrl. overrideWithEnvironment(appConfig); // apply existing non convention UI environment variables diff --git a/src/config/env-config-overrides.spec.ts b/src/config/env-config-overrides.spec.ts new file mode 100644 index 00000000000..2a76a2cfe23 --- /dev/null +++ b/src/config/env-config-overrides.spec.ts @@ -0,0 +1,163 @@ +import { + AppConfig, + toClientConfig, +} from './app-config.interface'; +import { DefaultAppConfig } from './default-app-config'; +import { + applyEnvironmentConfigOverrides, + getEnvConfigName, + SUPPLEMENTAL_ENV_CONFIG_OVERRIDES, +} from './env-config-overrides'; + +describe('Environment config overrides', () => { + const applyOverrides = (appConfig: AppConfig, env: { [key: string]: string }): void => { + applyEnvironmentConfigOverrides(appConfig, (envName: string) => env[envName]); + }; + + const getConfigValueByPath = (config: any, path: string[]): any => { + return path.reduce((currentConfig: any, pathSegment: string) => { + if (typeof currentConfig === 'object' && currentConfig !== null) { + return currentConfig[pathSegment]; + } + return undefined; + }, config); + }; + + const getEnvValueForSupplementalOverride = (type: string): string => { + switch (type) { + case 'number': + return '100'; + case 'boolean': + return 'true'; + default: + return 'supplemental-override'; + } + }; + + beforeEach(() => { + spyOn(console, 'info'); + spyOn(console, 'warn'); + }); + + it('should derive environment variable names from config paths', () => { + expect(getEnvConfigName(['matomo', 'trackerUrl'])).toEqual('DSPACE_MATOMO_TRACKERURL'); + expect(getEnvConfigName(['rest', 'ssrBaseUrl'])).toEqual('DSPACE_REST_SSRBASEURL'); + expect(getEnvConfigName(['cache', 'msToLive', 'default'])).toEqual('DSPACE_CACHE_MSTOLIVE_DEFAULT'); + expect(getEnvConfigName(['info', 'enableCookieConsentPopup'])).toEqual('DSPACE_INFO_ENABLECOOKIECONSENTPOPUP'); + }); + + it('should set absent optional scalar leaves from the supplemental map', () => { + const appConfig = new DefaultAppConfig(); + + applyOverrides(appConfig, { + DSPACE_MATOMO_TRACKERURL: 'https://analytics.example.org/matomo.php', + DSPACE_REST_BASEURL: 'https://api.example.org/server', + DSPACE_REST_SSRBASEURL: 'http://internal-rest:8080/server', + }); + + expect(appConfig.matomo.trackerUrl).toEqual('https://analytics.example.org/matomo.php'); + expect(appConfig.rest.baseUrl).toEqual('https://api.example.org/server'); + expect(appConfig.rest.ssrBaseUrl).toEqual('http://internal-rest:8080/server'); + }); + + it('should override empty optional scalar leaves from the supplemental map', () => { + const appConfig = new DefaultAppConfig(); + appConfig.matomo.trackerUrl = ''; + + applyOverrides(appConfig, { + DSPACE_MATOMO_TRACKERURL: 'https://analytics.example.org/matomo.php', + }); + + expect(appConfig.matomo.trackerUrl).toEqual('https://analytics.example.org/matomo.php'); + }); + + it('should preserve populated scalar conversion behavior', () => { + const appConfig = new DefaultAppConfig(); + + applyOverrides(appConfig, { + DSPACE_CACHE_MSTOLIVE_DEFAULT: '12345', + DSPACE_INFO_ENABLECOOKIECONSENTPOPUP: 'false', + DSPACE_MARKDOWN_ENABLED: '1', + DSPACE_FALLBACKLANGUAGE: 'de', + }); + + expect(appConfig.cache.msToLive.default).toEqual(12345); + expect(appConfig.info.enableCookieConsentPopup).toBeFalse(); + expect(appConfig.markdown.enabled).toBeTrue(); + expect(appConfig.fallbackLanguage).toEqual('de'); + }); + + it('should keep empty environment values as non-overrides', () => { + const appConfig = new DefaultAppConfig(); + + applyOverrides(appConfig, { + DSPACE_FALLBACKLANGUAGE: '', + DSPACE_MATOMO_TRACKERURL: '', + }); + + expect(appConfig.fallbackLanguage).not.toEqual(''); + expect(appConfig.matomo.trackerUrl).toBeUndefined(); + }); + + it('should not clobber existing non-object parents for supplemental override paths', () => { + const appConfig = new DefaultAppConfig(); + const supplementalOverride = { + path: ['fallbackLanguage', 'code'], + type: 'string' as const, + clientPartition: 'public' as const, + }; + + SUPPLEMENTAL_ENV_CONFIG_OVERRIDES.push(supplementalOverride); + + try { + applyOverrides(appConfig, { + DSPACE_FALLBACKLANGUAGE_CODE: 'de', + }); + + expect(appConfig.fallbackLanguage).toEqual(new DefaultAppConfig().fallbackLanguage); + expect((appConfig.fallbackLanguage as any).code).toBeUndefined(); + } finally { + SUPPLEMENTAL_ENV_CONFIG_OVERRIDES.splice(SUPPLEMENTAL_ENV_CONFIG_OVERRIDES.indexOf(supplementalOverride), 1); + } + }); + + it('should strip server-only values from direct TransferState and generated client config output', () => { + const appConfig = new DefaultAppConfig(); + const serverOnlySupplementalOverrides = SUPPLEMENTAL_ENV_CONFIG_OVERRIDES.filter((entry) => { + return entry.clientPartition === 'server-only'; + }); + + const serverOnlyEnv = serverOnlySupplementalOverrides.reduce((env, entry) => { + env[getEnvConfigName(entry.path)] = getEnvValueForSupplementalOverride(entry.type); + return env; + }, {}); + + applyOverrides(appConfig, { + ...serverOnlyEnv, + DSPACE_REST_SSRBASEURL: 'http://internal-rest:8080/server', + DSPACE_CACHE_SERVERSIDE_DEBUG: 'true', + DSPACE_UI_RATELIMITER_LIMIT: '100', + DSPACE_UI_USEPROXIES: 'false', + }); + appConfig.rest.hasSsrBaseUrl = true; + + const transferStateClientConfig = toClientConfig(appConfig) as AppConfig; + const generatedClientConfig = JSON.parse(JSON.stringify(toClientConfig(appConfig))) as AppConfig; + + expect(serverOnlySupplementalOverrides.length).toBeGreaterThan(0); + serverOnlySupplementalOverrides.forEach((entry) => { + expect(getConfigValueByPath(transferStateClientConfig, entry.path)).toBeUndefined(); + expect(getConfigValueByPath(generatedClientConfig, entry.path)).toBeUndefined(); + }); + expect(transferStateClientConfig.rest.ssrBaseUrl).toBeUndefined(); + expect(transferStateClientConfig.rest.hasSsrBaseUrl).toBeUndefined(); + expect(transferStateClientConfig.cache.serverSide).toBeUndefined(); + expect(transferStateClientConfig.ui.rateLimiter).toBeUndefined(); + expect(transferStateClientConfig.ui.useProxies).toBeUndefined(); + expect(generatedClientConfig.rest.ssrBaseUrl).toBeUndefined(); + expect(generatedClientConfig.rest.hasSsrBaseUrl).toBeUndefined(); + expect(generatedClientConfig.cache.serverSide).toBeUndefined(); + expect(generatedClientConfig.ui.rateLimiter).toBeUndefined(); + expect(generatedClientConfig.ui.useProxies).toBeUndefined(); + }); +}); diff --git a/src/config/env-config-overrides.ts b/src/config/env-config-overrides.ts new file mode 100644 index 00000000000..653fea8b861 --- /dev/null +++ b/src/config/env-config-overrides.ts @@ -0,0 +1,152 @@ +import { isNotEmpty } from '@dspace/shared/utils/empty.util'; + +import { Config } from './config.interface'; + +type EnvConfigValueType = 'number' | 'boolean' | 'string'; + +type EnvConfigClientPartition = 'public' | 'server-only'; + +/** + * Reads the string value for a named environment config override. + */ +export type EnvConfigValueGetter = (envName: string) => string; + +/** + * Describes an optional scalar config path that needs explicit environment override support. + */ +export interface SupplementalEnvConfigOverride { + path: string[]; + type: EnvConfigValueType; + clientPartition: EnvConfigClientPartition; +} + +/** + * Bounded supplemental map for optional scalar config leaves that cannot be discovered by the runtime walk when their + * value is absent or empty in the merged config object. Future absent/empty optional scalar environment overrides + * require an explicit entry here. + */ +export const SUPPLEMENTAL_ENV_CONFIG_OVERRIDES: SupplementalEnvConfigOverride[] = [ + // MatomoConfig.trackerUrl is optional, and DefaultAppConfig.matomo starts as an empty object. + { + path: ['matomo', 'trackerUrl'], + type: 'string', + clientPartition: 'public', + }, + // ServerConfig.baseUrl is optional; if supplied by env, it must be applied before buildBaseUrl(). + { + path: ['rest', 'baseUrl'], + type: 'string', + clientPartition: 'public', + }, + // ServerConfig.ssrBaseUrl is optional and server-only; toClientConfig() strips it from client config. + { + path: ['rest', 'ssrBaseUrl'], + type: 'string', + clientPartition: 'server-only', + }, +]; + +/** + * Builds the environment variable name for a config path. + */ +export const getEnvConfigName = (path: string[]): string => { + return `DSPACE_${path.map((segment: string) => segment.toUpperCase()).join('_')}`; +}; + +const getBooleanFromString = (variable: string): boolean => { + return variable === 'true' || variable === '1'; +}; + +const getNumberFromString = (variable: string): number => { + return Number(variable); +}; + +const applyEnvValueByPath = ( + config: Config, + path: string[], + type: string, + getEnvValue: EnvConfigValueGetter, +): void => { + const envName = getEnvConfigName(path); + const value = getEnvValue(envName); + + if (isNotEmpty(value)) { + const leaf = path[path.length - 1]; + let targetConfig = config; + + for (const pathSegment of path.slice(0, -1)) { + if (targetConfig[pathSegment] === undefined || targetConfig[pathSegment] === null) { + targetConfig[pathSegment] = {}; + } else if (typeof targetConfig[pathSegment] !== 'object') { + console.warn(`Skipping environment variable ${envName}; non-object config parent at ${pathSegment}`); + return; + } + targetConfig = targetConfig[pathSegment]; + } + + console.info(`Applying environment variable ${envName} with value ${value}`); + switch (type) { + case 'number': + targetConfig[leaf] = getNumberFromString(value); + break; + case 'boolean': + targetConfig[leaf] = getBooleanFromString(value); + break; + case 'string': + targetConfig[leaf] = value; + break; + default: + console.warn(`Unsupported environment variable type ${type} ${envName}`); + } + } +}; + +const applyRuntimeEnvironmentConfigOverrides = ( + rootConfig: Config, + config: Config, + getEnvValue: EnvConfigValueGetter, + path: string[] = [], +): void => { + Object.keys(config).forEach((property: string) => { + const innerConfig = config[property]; + if (isNotEmpty(innerConfig)) { + const innerPath = [...path, property]; + if (typeof innerConfig === 'object') { + applyRuntimeEnvironmentConfigOverrides(rootConfig, innerConfig, getEnvValue, innerPath); + } else { + applyEnvValueByPath(rootConfig, innerPath, typeof innerConfig, getEnvValue); + } + } + }); +}; + +const getConfigValueByPath = (config: Config, path: string[]): any => { + return path.reduce((currentConfig: any, pathSegment: string) => { + if (typeof currentConfig === 'object' && currentConfig !== null) { + return currentConfig[pathSegment]; + } + return undefined; + }, config); +}; + +const applySupplementalEnvironmentConfigOverrides = ( + config: Config, + getEnvValue: EnvConfigValueGetter, +): void => { + SUPPLEMENTAL_ENV_CONFIG_OVERRIDES.forEach((override: SupplementalEnvConfigOverride) => { + if (!isNotEmpty(getConfigValueByPath(config, override.path))) { + applyEnvValueByPath(config, override.path, override.type, getEnvValue); + } + }); +}; + +/** + * Applies runtime and supplemental environment overrides to the provided config object. + */ +export const applyEnvironmentConfigOverrides = ( + config: Config, + getEnvValue: EnvConfigValueGetter, +): void => { + applyRuntimeEnvironmentConfigOverrides(config, config, getEnvValue); + applySupplementalEnvironmentConfigOverrides(config, getEnvValue); +};