diff --git a/.changeset/clever-donkeys-fry.md b/.changeset/clever-donkeys-fry.md new file mode 100644 index 0000000000..8098bba682 --- /dev/null +++ b/.changeset/clever-donkeys-fry.md @@ -0,0 +1,5 @@ +--- +'@backstage/config': minor +--- + +The `ConfigReader` now treats `null` values as present but explicitly undefined, meaning it will not fall back to the next level of configuration. diff --git a/.changeset/gorgeous-apples-reply.md b/.changeset/gorgeous-apples-reply.md new file mode 100644 index 0000000000..a6bf04d0fe --- /dev/null +++ b/.changeset/gorgeous-apples-reply.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': minor +--- + +Forward `null` values read from configuration files in configuration data, rather than treating them as an absence of config. diff --git a/docs/conf/writing.md b/docs/conf/writing.md index e0eb72f828..48104699c7 100644 --- a/docs/conf/writing.md +++ b/docs/conf/writing.md @@ -101,6 +101,9 @@ All loaded configuration files are merged together using the following rules: contents. - Objects are merged together deeply, meaning that if any of the included configs contain a value for a given path, it will be found. +- A `null` value in a config file will be treated as an explicit absence of + configuration. This means that the reading will not fall back to a lower priority + config, but it will still be treated as if the configuration was not present. The priority of the configurations is determined by the following rules, in order: diff --git a/packages/config-loader/src/sources/transform/apply.test.ts b/packages/config-loader/src/sources/transform/apply.test.ts index ea87734036..9f1dfbc672 100644 --- a/packages/config-loader/src/sources/transform/apply.test.ts +++ b/packages/config-loader/src/sources/transform/apply.test.ts @@ -17,7 +17,7 @@ import { applyConfigTransforms } from './apply'; describe('applyConfigTransforms', () => { - it('should apply not transforms to input', async () => { + it('should apply no transforms to input', async () => { const data = applyConfigTransforms( { app: { @@ -35,7 +35,8 @@ describe('applyConfigTransforms', () => { app: { title: 'Test', x: 1, - y: [true], + y: [null, true], + z: null, }, }); }); @@ -77,7 +78,8 @@ describe('applyConfigTransforms', () => { app: { title: ['T', 'e', 's', 't'], x: 2, - y: [true], + y: [null, true], + z: null, }, }); }); diff --git a/packages/config-loader/src/sources/transform/apply.ts b/packages/config-loader/src/sources/transform/apply.ts index d0d8650fe7..48d9c5a1e8 100644 --- a/packages/config-loader/src/sources/transform/apply.ts +++ b/packages/config-loader/src/sources/transform/apply.ts @@ -58,7 +58,7 @@ export async function applyConfigTransforms( if (typeof obj !== 'object') { return obj; } else if (obj === null) { - return undefined; + return null; } else if (Array.isArray(obj)) { const arr = new Array(); diff --git a/packages/config/package.json b/packages/config/package.json index ddfe2da633..d8a2df01c5 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -37,8 +37,7 @@ }, "dependencies": { "@backstage/errors": "workspace:^", - "@backstage/types": "workspace:^", - "lodash": "^4.17.21" + "@backstage/types": "workspace:^" }, "devDependencies": { "@backstage/cli": "workspace:^", diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 90b9b28b41..97982ac265 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -48,15 +48,15 @@ const DATA = { }; function expectValidValues(config: ConfigReader) { - expect(config.keys()).toEqual(Object.keys(DATA)); + expect(config.keys()).toEqual(Object.keys(DATA).filter(k => k !== 'null')); expect(config.get('zero')).toBe(0); expect(config.has('zero')).toBe(true); expect(config.has('false')).toBe(true); - expect(config.has('null')).toBe(true); + expect(config.has('null')).toBe(false); expect(config.has('missing')).toBe(false); expect(config.has('nested.one')).toBe(true); expect(config.has('nested.missing')).toBe(false); - expect(config.has('nested.null')).toBe(true); + expect(config.has('nested.null')).toBe(false); expect(config.getNumber('zero')).toBe(0); expect(config.getNumber('one')).toBe(1); expect(config.getNumber('zeroString')).toBe(0); @@ -81,7 +81,7 @@ function expectValidValues(config: ConfigReader) { expect(config.getConfig('nested').getNumber('one')).toBe(1); expect(config.get('nested')).toEqual({ one: 1, - null: null, + null: undefined, string: 'string', strings: ['string1', 'string2'], }); @@ -90,6 +90,8 @@ function expectValidValues(config: ConfigReader) { ['string1', 'string2'], ); expect(config.getOptional('missing')).toBe(undefined); + expect(config.getOptionalConfig('null')).toBe(undefined); + expect(config.getOptionalConfig('null.nested')).toBe(undefined); expect(config.getOptionalConfig('missing')).toBe(undefined); expect(config.getOptionalConfigArray('missing')).toBe(undefined); expect(config.getNumber('zero')).toBe(0); @@ -120,7 +122,7 @@ function expectInvalidValues(config: ConfigReader) { "Invalid type in config for key 'true' in 'ctx', got boolean, wanted number", ); expect(() => config.getStringArray('null')).toThrow( - "Invalid type in config for key 'null' in 'ctx', got null, wanted string-array", + "Missing required config value at 'null' in 'ctx'", ); expect(() => config.getString('emptyString')).toThrow( "Invalid type in config for key 'emptyString' in 'ctx', got empty-string, wanted string", @@ -137,6 +139,9 @@ function expectInvalidValues(config: ConfigReader) { expect(() => config.getConfig('one')).toThrow( "Invalid type in config for key 'one' in 'ctx', got number, wanted object", ); + expect(() => config.getConfig('null')).toThrow( + "Missing required config value at 'null'", + ); expect(() => config.getConfigArray('one')).toThrow( "Invalid type in config for key 'one' in 'ctx', got number, wanted object-array", ); @@ -742,4 +747,42 @@ describe('ConfigReader.get()', () => { expect(data1.foo.baz).toEqual({}); expect(data2.x.y.z).toEqual({}); }); + + it('should treat null as explicitly undefined', () => { + const reader = ConfigReader.fromConfigs([ + { + data: { obj: { a: 2, b: 2, c: 2 }, objB: { a: 2, b: null } }, + context: 'fallback', + }, + { + data: { obj: { a: 1, b: null }, objA: { a: 1, b: null } }, + context: 'primary', + }, + ]); + + expect(reader.getOptionalNumber('obj.a')).toBe(1); + expect(reader.getOptionalNumber('obj.b')).toBe(undefined); + expect(reader.getOptionalNumber('obj.c')).toBe(2); + + expect(reader.getConfig('obj').getOptionalNumber('a')).toBe(1); + expect(reader.getConfig('obj').getOptionalNumber('b')).toBe(undefined); + expect(reader.getConfig('obj').getOptionalNumber('c')).toBe(2); + + expect(reader.getConfig('obj').get('a')).toBe(1); + expect(() => reader.getConfig('obj').get('b')).toThrow( + "Missing required config value at 'obj.b' in 'primary'", + ); + expect(reader.getConfig('obj').get('c')).toBe(2); + + expect(reader.getConfig('obj').getOptional('a')).toBe(1); + expect(reader.getConfig('obj').getOptional('b')).toBe(undefined); + expect(reader.getConfig('obj').getOptional('c')).toBe(2); + + expect(reader.get('obj')).toEqual({ a: 1, c: 2 }); + expect(reader.getConfig('obj').get()).toEqual({ a: 1, c: 2 }); + expect(reader.getConfig('obj').keys()).toEqual(['a', 'c']); + + expect(reader.get('objA')).toEqual({ a: 1 }); + expect(reader.get('objB')).toEqual({ a: 2 }); + }); }); diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 4a570b6977..52c33f8618 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -16,8 +16,6 @@ import { JsonValue, JsonObject } from '@backstage/types'; import { AppConfig, Config } from './types'; -import cloneDeep from 'lodash/cloneDeep'; -import mergeWith from 'lodash/mergeWith'; // Update the same pattern in config-loader package if this is changed const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i; @@ -26,6 +24,43 @@ function isObject(value: JsonValue | undefined): value is JsonObject { return typeof value === 'object' && value !== null && !Array.isArray(value); } +function cloneDeep(value: JsonValue | null | undefined): JsonValue | undefined { + if (typeof value !== 'object' || value === null) { + return value; + } + if (Array.isArray(value)) { + return value.map(cloneDeep) as JsonValue; + } + return Object.fromEntries( + Object.entries(value).map(([k, v]) => [k, cloneDeep(v)]), + ); +} + +function merge( + into: JsonValue | undefined, + from?: JsonValue | undefined, +): JsonValue | undefined { + if (into === null) { + return undefined; + } + if (into === undefined) { + return from === undefined ? undefined : merge(from); + } + if (typeof into !== 'object' || Array.isArray(into)) { + return into; + } + const fromObj = isObject(from) ? from : {}; + + const out: JsonObject = {}; + for (const key of new Set([...Object.keys(into), ...Object.keys(fromObj)])) { + const val = merge(into[key], fromObj[key]); + if (val !== undefined) { + out[key] = val; + } + } + return out; +} + function typeOf(value: JsonValue | undefined): string { if (value === null) { return 'null'; @@ -47,8 +82,8 @@ const errors = { type(key: string, context: string, typeName: string, expected: string) { return `Invalid type in config for key '${key}' in '${context}', got ${typeName}, wanted ${expected}`; }, - missing(key: string) { - return `Missing required config value at '${key}'`; + missing(key: string, context: string) { + return `Missing required config value at '${key}' in '${context}'`; }, convert(key: string, context: string, expected: string) { return `Unable to convert config value for key '${key}' in '${context}' to a ${expected}`; @@ -114,6 +149,9 @@ export class ConfigReader implements Config { /** {@inheritdoc Config.has} */ has(key: string): boolean { const value = this.readValue(key); + if (value === null) { + return false; + } if (value !== undefined) { return true; } @@ -124,14 +162,16 @@ export class ConfigReader implements Config { keys(): string[] { const localKeys = this.data ? Object.keys(this.data) : []; const fallbackKeys = this.fallback?.keys() ?? []; - return [...new Set([...localKeys, ...fallbackKeys])]; + return [...new Set([...localKeys, ...fallbackKeys])].filter( + k => this.data?.[k] !== null, + ); } /** {@inheritdoc Config.get} */ get(key?: string): T { const value = this.getOptional(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key ?? ''))); + throw new Error(errors.missing(this.fullKey(key ?? ''), this.context)); } return value as T; } @@ -139,8 +179,11 @@ export class ConfigReader implements Config { /** {@inheritdoc Config.getOptional} */ getOptional(key?: string): T | undefined { const value = cloneDeep(this.readValue(key)); - const fallbackValue = this.fallback?.getOptional(key); + const fallbackValue = this.fallback?.getOptional(key); + if (value === null) { + return undefined; + } if (value === undefined) { if (process.env.NODE_ENV === 'development') { if (fallbackValue === undefined && key) { @@ -158,23 +201,19 @@ export class ConfigReader implements Config { } } } - return fallbackValue; + return merge(fallbackValue) as T; } else if (fallbackValue === undefined) { - return value as T; + return merge(value) as T; } - // Avoid merging arrays and primitive values, since that's how merging works for other - // methods for reading config. - return mergeWith({}, { value: fallbackValue }, { value }, (into, from) => - !isObject(from) || !isObject(into) ? from : undefined, - ).value as T; + return merge(value, fallbackValue) as T; } /** {@inheritdoc Config.getConfig} */ getConfig(key: string): ConfigReader { const value = this.getOptionalConfig(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -187,6 +226,9 @@ export class ConfigReader implements Config { if (isObject(value)) { return this.copy(value, key, fallbackConfig); } + if (value === null) { + return undefined; + } if (value !== undefined) { throw new TypeError( errors.type(this.fullKey(key), this.context, typeOf(value), 'object'), @@ -199,7 +241,7 @@ export class ConfigReader implements Config { getConfigArray(key: string): ConfigReader[] { const value = this.getOptionalConfigArray(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -244,7 +286,7 @@ export class ConfigReader implements Config { getNumber(key: string): number { const value = this.getOptionalNumber(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -273,7 +315,7 @@ export class ConfigReader implements Config { getBoolean(key: string): boolean { const value = this.getOptionalBoolean(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -305,7 +347,7 @@ export class ConfigReader implements Config { getString(key: string): string { const value = this.getOptionalString(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -323,7 +365,7 @@ export class ConfigReader implements Config { getStringArray(key: string): string[] { const value = this.getOptionalStringArray(key); if (value === undefined) { - throw new Error(errors.missing(this.fullKey(key))); + throw new Error(errors.missing(this.fullKey(key), this.context)); } return value; } @@ -384,6 +426,9 @@ export class ConfigReader implements Config { return this.fallback?.readConfigValue(key, validate); } + if (value === null) { + return undefined; + } const result = validate(value); if (result !== true) { const { key: keyName = key, value: theValue = value, expected } = result; @@ -416,7 +461,7 @@ export class ConfigReader implements Config { for (const [index, part] of parts.entries()) { if (isObject(value)) { value = value[part]; - } else if (value !== undefined) { + } else if (value !== undefined && value !== null) { const badKey = this.fullKey(parts.slice(0, index).join('.')); throw new TypeError( errors.type(badKey, this.context, typeOf(value), 'object'), diff --git a/yarn.lock b/yarn.lock index 1693b3c283..fafc485f01 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3803,7 +3803,6 @@ __metadata: "@backstage/errors": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" - lodash: ^4.17.21 languageName: unknown linkType: soft