config,config-loader: warn when trying to access invisible config values
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
@@ -14,6 +14,7 @@
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { withLogCollector } from '../../test-utils-core/src';
|
||||
import { ConfigReader } from './reader';
|
||||
|
||||
const DATA = {
|
||||
@@ -185,6 +186,61 @@ describe('ConfigReader', () => {
|
||||
const config = new ConfigReader(DATA, CTX);
|
||||
expectInvalidValues(config);
|
||||
});
|
||||
|
||||
it('should warn when accessing filtered keys in development mode', () => {
|
||||
const oldEnv = process.env.NODE_ENV;
|
||||
(process.env as any).NODE_ENV = 'development';
|
||||
|
||||
const config = ConfigReader.fromConfigs([
|
||||
{
|
||||
data: DATA,
|
||||
context: CTX,
|
||||
filteredKeys: ['a', 'b[0]'],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(withLogCollector(() => config.getOptional('a'))).toMatchObject({
|
||||
warn: [
|
||||
"Failed to read configuration value at 'a' as it is not visible. See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.",
|
||||
],
|
||||
});
|
||||
expect(withLogCollector(() => config.getOptionalString('a'))).toMatchObject(
|
||||
{
|
||||
warn: [
|
||||
"Failed to read configuration value at 'a' as it is not visible. See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.",
|
||||
],
|
||||
},
|
||||
);
|
||||
expect(
|
||||
withLogCollector(() => config.getOptionalConfigArray('b')),
|
||||
).toMatchObject({
|
||||
warn: [
|
||||
"Failed to read configuration array at 'b' as it does not have any visible elements. See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.",
|
||||
],
|
||||
});
|
||||
|
||||
(process.env as any).NODE_ENV = oldEnv;
|
||||
});
|
||||
|
||||
it('should not warn when accessing filtered keys outside of development mode', () => {
|
||||
const config = ConfigReader.fromConfigs([
|
||||
{
|
||||
data: DATA,
|
||||
context: CTX,
|
||||
filteredKeys: ['a', 'b[0]'],
|
||||
},
|
||||
]);
|
||||
|
||||
expect(withLogCollector(() => config.getOptional('a'))).toMatchObject({
|
||||
warn: [],
|
||||
});
|
||||
expect(
|
||||
withLogCollector(() => config.getOptionalString('a')),
|
||||
).toMatchObject({ warn: [] });
|
||||
expect(
|
||||
withLogCollector(() => config.getOptionalConfigArray('b')),
|
||||
).toMatchObject({ warn: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe('ConfigReader with fallback', () => {
|
||||
|
||||
@@ -55,6 +55,15 @@ const errors = {
|
||||
};
|
||||
|
||||
export class ConfigReader implements Config {
|
||||
/**
|
||||
* A set of key paths that where removed from the config due to not being visible.
|
||||
*
|
||||
* This was added as a mutable private member to avoid changes to the public API.
|
||||
* Its only purpose of this is to warn users of missing visibility when running
|
||||
* the frontend in development mode.
|
||||
*/
|
||||
private filteredKeys?: string[];
|
||||
|
||||
static fromConfigs(configs: AppConfig[]): ConfigReader {
|
||||
if (configs.length === 0) {
|
||||
return new ConfigReader(undefined);
|
||||
@@ -62,9 +71,14 @@ export class ConfigReader implements Config {
|
||||
|
||||
// Merge together all configs into a single config with recursive fallback
|
||||
// readers, giving the first config object in the array the lowest priority.
|
||||
return configs.reduce<ConfigReader>((previousReader, { data, context }) => {
|
||||
return new ConfigReader(data, context, previousReader);
|
||||
}, undefined!);
|
||||
return configs.reduce<ConfigReader>(
|
||||
(previousReader, { data, context, filteredKeys }) => {
|
||||
const reader = new ConfigReader(data, context, previousReader);
|
||||
reader.filteredKeys = filteredKeys;
|
||||
return reader;
|
||||
},
|
||||
undefined!,
|
||||
);
|
||||
}
|
||||
|
||||
constructor(
|
||||
@@ -101,6 +115,18 @@ export class ConfigReader implements Config {
|
||||
const fallbackValue = this.fallback?.getOptional<T>(key);
|
||||
|
||||
if (value === undefined) {
|
||||
if (process.env.NODE_ENV === 'development') {
|
||||
if (fallbackValue === undefined && key) {
|
||||
const fullKey = this.fullKey(key);
|
||||
if (this.filteredKeys?.includes(fullKey)) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`Failed to read configuration value at '${fullKey}' as it is not visible. ` +
|
||||
'See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
return fallbackValue;
|
||||
} else if (fallbackValue === undefined) {
|
||||
return value as T;
|
||||
@@ -127,10 +153,9 @@ export class ConfigReader implements Config {
|
||||
getOptionalConfig(key: string): ConfigReader | undefined {
|
||||
const value = this.readValue(key);
|
||||
const fallbackConfig = this.fallback?.getOptionalConfig(key);
|
||||
const prefix = this.fullKey(key);
|
||||
|
||||
if (isObject(value)) {
|
||||
return new ConfigReader(value, this.context, fallbackConfig, prefix);
|
||||
return this.copy(value, key, fallbackConfig);
|
||||
}
|
||||
if (value !== undefined) {
|
||||
throw new TypeError(
|
||||
@@ -163,18 +188,20 @@ export class ConfigReader implements Config {
|
||||
});
|
||||
|
||||
if (!configs) {
|
||||
if (process.env.NODE_ENV === 'development') {
|
||||
const fullKey = this.fullKey(key);
|
||||
if (this.filteredKeys?.some(k => k.startsWith(fullKey))) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`Failed to read configuration array at '${key}' as it does not have any visible elements. ` +
|
||||
'See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.',
|
||||
);
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return configs.map(
|
||||
(obj, index) =>
|
||||
new ConfigReader(
|
||||
obj,
|
||||
this.context,
|
||||
undefined,
|
||||
this.fullKey(`${key}[${index}]`),
|
||||
),
|
||||
);
|
||||
return configs.map((obj, index) => this.copy(obj, `${key}[${index}]`));
|
||||
}
|
||||
|
||||
getNumber(key: string): number {
|
||||
@@ -261,6 +288,17 @@ export class ConfigReader implements Config {
|
||||
return `${this.prefix}${this.prefix ? '.' : ''}${key}`;
|
||||
}
|
||||
|
||||
private copy(data: JsonObject, key: string, fallback?: ConfigReader) {
|
||||
const reader = new ConfigReader(
|
||||
data,
|
||||
this.context,
|
||||
fallback,
|
||||
this.fullKey(key),
|
||||
);
|
||||
reader.filteredKeys = this.filteredKeys;
|
||||
return reader;
|
||||
}
|
||||
|
||||
private readConfigValue<T extends JsonValue>(
|
||||
key: string,
|
||||
validate: (
|
||||
@@ -270,6 +308,17 @@ export class ConfigReader implements Config {
|
||||
const value = this.readValue(key);
|
||||
|
||||
if (value === undefined) {
|
||||
if (process.env.NODE_ENV === 'development') {
|
||||
const fullKey = this.fullKey(key);
|
||||
if (this.filteredKeys?.includes(fullKey)) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`Failed to read configuration value at '${fullKey}' as it is not visible. ` +
|
||||
'See https://backstage.io/docs/conf/defining#visibility for instructions on how to make it visible.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return this.fallback?.readConfigValue(key, validate);
|
||||
}
|
||||
const result = validate(value);
|
||||
|
||||
@@ -22,6 +22,7 @@ export type JsonValue = JsonObject | JsonArray | JsonPrimitive;
|
||||
export type AppConfig = {
|
||||
context: string;
|
||||
data: JsonObject;
|
||||
filteredKeys?: string[];
|
||||
};
|
||||
|
||||
export type Config = {
|
||||
|
||||
Reference in New Issue
Block a user