paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

config,config-loader: warn when trying to access invisible config values

Browse Source 
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
Patrik Oldsberg
2021-07-30 16:31:44 +02:00
parent 8c6054a587
commit e9d3983eee
16 changed files with 283 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/curvy-poets-dance.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/cli': patch
---
Keep track of filtered configuration values when running frontend in development mode.
.changeset/large-flies-bow.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/config': patch
---
Add warning when trying to access configuration values that have been filtered out by visibility.
.changeset/sharp-seals-explode.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/config-loader': patch
---
Add option to populate the `filteredKeys` property when processing configuration with a schema.
packages/cli/src/commands/app/serve.ts
+1
View File
@@ -59,6 +59,7 @@ export default async (cmd: Command) => {
...(await loadCliConfig({
args: cmd.config,
fromPackage: name,
withFilteredKeys: true,
})),
});
packages/cli/src/commands/plugin/serve.ts
+1
View File
@@ -28,6 +28,7 @@ export default async (cmd: Command) => {
...(await loadCliConfig({
args: cmd.config,
fromPackage: name,
withFilteredKeys: true,
})),
});
packages/cli/src/lib/config.ts
+2
View File
@@ -22,6 +22,7 @@ type Options = {
args: string[];
fromPackage?: string;
mockEnv?: boolean;
withFilteredKeys?: boolean;
};
export async function loadCliConfig(options: Options) {
@@ -57,6 +58,7 @@ export async function loadCliConfig(options: Options) {
try {
const frontendAppConfigs = schema.process(appConfigs, {
visibility: ['frontend'],
withFilteredKeys: options.withFilteredKeys,
});
const frontendConfig = ConfigReader.fromConfigs(frontendAppConfigs);
packages/config-loader/api-report.md
+1 -1
View File
@@ -59,7 +59,7 @@ export function readEnvConfig(env: {
// Warnings were encountered during analysis:
//
// src/lib/schema/types.d.ts:77:5 - (ae-forgotten-export) The symbol "ConfigProcessingOptions" needs to be exported by the entry point index.d.ts
// src/lib/schema/types.d.ts:83:5 - (ae-forgotten-export) The symbol "ConfigProcessingOptions" needs to be exported by the entry point index.d.ts
// src/loader.d.ts:13:5 - (ae-forgotten-export) The symbol "EnvFunc" needs to be exported by the entry point index.d.ts
// (No @packageDocumentation comment for this package)
packages/config-loader/src/lib/schema/filtering.test.ts
+94 -22
View File
@@ -63,43 +63,115 @@ const visibility = new Map<string, ConfigVisibility>(
describe('filterByVisibility', () => {
test.each<[ConfigVisibility[], JsonObject]>([
[[], {}],
[
[],
{
data: {},
filteredKeys: [
'arr[0]',
'arr[1]',
'arr[2]',
'objArr[0].f',
'objArr[0].b',
'objArr[0].s',
'objArr[1].f',
'objArr[1].b',
'objArr[1].s',
'obj.f',
'obj.b.s',
'arrF[0].never',
'arrB[0].never',
'arrS[0].never',
'objF.never',
'objB.never',
'objS.never',
],
},
],
[
['frontend'],
{
arr: ['f'],
objArr: [{ f: 1 }, { f: 4 }],
obj: { f: 'a' },
arrF: [],
objF: {},
data: {
arr: ['f'],
objArr: [{ f: 1 }, { f: 4 }],
obj: { f: 'a' },
arrF: [],
objF: {},
},
filteredKeys: [
'arr[1]',
'arr[2]',
'objArr[0].b',
'objArr[0].s',
'objArr[1].b',
'objArr[1].s',
'obj.b.s',
'arrF[0].never',
'arrB[0].never',
'arrS[0].never',
'objF.never',
'objB.never',
'objS.never',
],
},
],
[
['backend'],
{
arr: ['b'],
objArr: [{ b: 2 }, { b: 5 }],
obj: { b: {} },
arrF: [{ never: 'here' }],
arrB: [{ never: 'here' }],
arrS: [{ never: 'here' }],
objF: { never: 'here' },
objB: { never: 'here' },
objS: { never: 'here' },
data: {
arr: ['b'],
objArr: [{ b: 2 }, { b: 5 }],
obj: { b: {} },
arrF: [{ never: 'here' }],
arrB: [{ never: 'here' }],
arrS: [{ never: 'here' }],
objF: { never: 'here' },
objB: { never: 'here' },
objS: { never: 'here' },
},
filteredKeys: [
'arr[0]',
'arr[2]',
'objArr[0].f',
'objArr[0].s',
'objArr[1].f',
'objArr[1].s',
'obj.f',
'obj.b.s',
],
},
],
[
['secret'],
{
arr: ['s'],
objArr: [{ s: 3 }, { s: 6 }],
obj: { b: { s: true } },
arrS: [],
objS: {},
data: {
arr: ['s'],
objArr: [{ s: 3 }, { s: 6 }],
obj: { b: { s: true } },
arrS: [],
objS: {},
},
filteredKeys: [
'arr[0]',
'arr[1]',
'objArr[0].f',
'objArr[0].b',
'objArr[1].f',
'objArr[1].b',
'obj.f',
'arrF[0].never',
'arrB[0].never',
'arrS[0].never',
'objF.never',
'objB.never',
'objS.never',
],
},
],
[['frontend', 'backend', 'secret'], data],
[['frontend', 'backend', 'secret'], { data, filteredKeys: [] }],
])('should filter correctly with %p', (filter, expected) => {
expect(filterByVisibility(data, filter, visibility)).toEqual(expected);
expect(
filterByVisibility(data, filter, visibility, undefined, true),
).toEqual(expected);
});
});
packages/config-loader/src/lib/schema/filtering.ts
+28 -6
View File
@@ -30,9 +30,17 @@ export function filterByVisibility(
includeVisibilities: ConfigVisibility[],
visibilityByPath: Map<string, ConfigVisibility>,
transformFunc?: TransformFunc<number | string | boolean>,
): JsonObject {
function transform(jsonVal: JsonValue, path: string): JsonValue | undefined {
const visibility = visibilityByPath.get(path) ?? DEFAULT_CONFIG_VISIBILITY;
withFilteredKeys?: boolean,
): { data: JsonObject; filteredKeys?: string[] } {
const filteredKeys = new Array<string>();
function transform(
jsonVal: JsonValue,
visibilityPath: string, // Matches the format we get from ajv
filterPath: string, // Matches the format of the ConfigReader
): JsonValue | undefined {
const visibility =
visibilityByPath.get(visibilityPath) ?? DEFAULT_CONFIG_VISIBILITY;
const isVisible = includeVisibilities.includes(visibility);
if (typeof jsonVal !== 'object') {
@@ -42,6 +50,9 @@ export function filterByVisibility(
}
return jsonVal;
}
if (withFilteredKeys) {
filteredKeys.push(filterPath);
}
return undefined;
} else if (jsonVal === null) {
return undefined;
@@ -49,7 +60,11 @@ export function filterByVisibility(
const arr = new Array<JsonValue>();
for (const [index, value] of jsonVal.entries()) {
const out = transform(value, `${path}/${index}`);
const out = transform(
value,
`${visibilityPath}/${index}`,
`${filterPath}[${index}]`,
);
if (out !== undefined) {
arr.push(out);
}
@@ -68,7 +83,11 @@ export function filterByVisibility(
if (value === undefined) {
continue;
}
const out = transform(value, `${path}/${key}`);
const out = transform(
value,
`${visibilityPath}/${key}`,
filterPath ? `${filterPath}.${key}` : key,
);
if (out !== undefined) {
outObj[key] = out;
hasOutput = true;
@@ -81,5 +100,8 @@ export function filterByVisibility(
return undefined;
}
return (transform(data, '') as JsonObject) ?? {};
return {
filteredKeys: withFilteredKeys ? filteredKeys : undefined,
data: (transform(data, '', '') as JsonObject) ?? {},
};
}
packages/config-loader/src/lib/schema/load.test.ts
+8 -2
View File
@@ -68,13 +68,19 @@ describe('loadConfigSchema', () => {
schema.process(configs, {
visibility: ['frontend'],
valueTransform: () => 'X',
withFilteredKeys: true,
}),
).toEqual([{ data: { key1: 'X' }, context: 'test' }]);
).toEqual([
{ data: { key1: 'X' }, context: 'test', filteredKeys: ['key2'] },
]);
expect(
schema.process(configs, {
valueTransform: () => 'X',
withFilteredKeys: true,
}),
).toEqual([{ data: { key1: 'X', key2: 'X' }, context: 'test' }]);
).toEqual([
{ data: { key1: 'X', key2: 'X' }, context: 'test', filteredKeys: [] },
]);
const serialized = schema.serialize();
packages/config-loader/src/lib/schema/load.ts
+5 -3
View File
@@ -57,7 +57,7 @@ export async function loadConfigSchema(
return {
process(
configs: AppConfig[],
{ visibility, valueTransform } = {},
{ visibility, valueTransform, withFilteredKeys } = {},
): AppConfig[] {
const result = validate(configs);
if (result.errors) {
@@ -73,21 +73,23 @@ export async function loadConfigSchema(
if (visibility) {
processedConfigs = processedConfigs.map(({ data, context }) => ({
context,
data: filterByVisibility(
...filterByVisibility(
data,
visibility,
result.visibilityByPath,
valueTransform,
withFilteredKeys,
),
}));
} else if (valueTransform) {
processedConfigs = processedConfigs.map(({ data, context }) => ({
context,
data: filterByVisibility(
...filterByVisibility(
data,
Array.from(CONFIG_VISIBILITIES),
result.visibilityByPath,
valueTransform,
withFilteredKeys,
),
}));
}
packages/config-loader/src/lib/schema/types.ts
+7
View File
@@ -96,6 +96,13 @@ type ConfigProcessingOptions = {
* will be omitted.
*/
valueTransform?: TransformFunc<any>;
/**
* Whether or not to include the `filteredKeys` property in the output `AppConfig`s.
*
* Default: `false`.
*/
withFilteredKeys?: boolean;
};
/**
packages/config/api-report.md
+1
View File
@@ -9,6 +9,7 @@
export type AppConfig = {
context: string;
data: JsonObject;
filteredKeys?: string[];
};
// Warning: (ae-missing-release-tag) "Config" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
packages/config/src/reader.test.ts
+56
View File
@@ -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', () => {
packages/config/src/reader.ts
+63 -14
View File
@@ -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);
packages/config/src/types.ts
+1
View File
@@ -22,6 +22,7 @@ export type JsonValue = JsonObject | JsonArray | JsonPrimitive;
export type AppConfig = {
context: string;
data: JsonObject;
filteredKeys?: string[];
};
export type Config = {