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

wip: warn users of deprecated keys in app configurations

Browse Source 
Signed-off-by: Colton Padden <colton.padden@fastmail.com>
This commit is contained in:
Colton Padden
2022-01-02 11:39:33 -05:00
parent 1fa18f2ab6
commit 1136ed83e3
13 changed files with 210 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/config-loader/api-report.md
+1
View File
@@ -21,6 +21,7 @@ export type ConfigSchemaProcessingOptions = {
visibility?: ConfigVisibility[];
valueTransform?: TransformFunc<any>;
withFilteredKeys?: boolean;
withDeprecatedKeys?: boolean;
};
// Warning: (ae-missing-release-tag) "ConfigTarget" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
packages/config-loader/package.json
+1
View File
@@ -41,6 +41,7 @@
"json-schema": "^0.4.0",
"json-schema-merge-allof": "^0.8.1",
"json-schema-traverse": "^1.0.0",
"lodash": "^4.17.21",
"node-fetch": "^2.6.1",
"typescript-json-schema": "^0.52.0",
"yaml": "^1.9.2",
packages/config-loader/src/lib/schema/collect.ts
+2 -2
View File
@@ -182,10 +182,10 @@ function compileTsSchemas(paths: string[]) {
program,
// All schemas should export a `Config` symbol
'Config',
// This enables usage of @visibility is doc comments
// This enables usage of @visibility and @deprecated is doc comments
{
required: true,
validationKeywords: ['visibility'],
validationKeywords: ['visibility', 'deprecated'],
},
[path.split(sep).join('/')], // Unix paths are expected for all OSes here
) as JsonObject | null;
packages/config-loader/src/lib/schema/compile.test.ts
+33
View File
@@ -40,6 +40,7 @@ describe('compileConfigSchemas', () => {
],
visibilityByDataPath: new Map(),
visibilityBySchemaPath: new Map(),
deprecationBySchemaPath: new Map(),
});
expect(validate([{ data: { b: 'b' }, context: 'test' }])).toEqual({
errors: [
@@ -53,6 +54,7 @@ describe('compileConfigSchemas', () => {
],
visibilityByDataPath: new Map(),
visibilityBySchemaPath: new Map(),
deprecationBySchemaPath: new Map(),
});
});
@@ -112,6 +114,7 @@ describe('compileConfigSchemas', () => {
'/properties/d/items': 'frontend',
}),
),
deprecationBySchemaPath: new Map(),
});
});
@@ -137,4 +140,34 @@ describe('compileConfigSchemas', () => {
"Config schema visibility is both 'frontend' and 'secret' for properties/a/visibility",
);
});
it('should discover deprecations', () => {
const validate = compileConfigSchemas([
{
path: 'a1',
value: {
type: 'object',
properties: {
a: { type: 'string', deprecated: 'deprecation reason for a' },
b: { type: 'string', deprecated: 'deprecation reason for b' },
c: { type: 'string' },
},
},
},
]);
expect(
validate([
{ data: { a: 'a', b: 'b', c: 'c', d: ['d'] }, context: 'test' },
]),
).toEqual({
deprecationBySchemaPath: new Map(
Object.entries({
'/properties/a': 'deprecation reason for a',
'/properties/b': 'deprecation reason for b',
}),
),
visibilityByDataPath: new Map(),
visibilityBySchemaPath: new Map(),
});
});
});
packages/config-loader/src/lib/schema/compile.ts
+9
View File
@@ -81,6 +81,13 @@ export function compileConfigSchemas(
const merged = mergeConfigSchemas(schemas.map(_ => _.value));
const validate = ajv.compile(merged);
const deprecationBySchemaPath = new Map<string, string>();
traverse(merged, (schema, path) => {
if ('deprecated' in schema) {
deprecationBySchemaPath.set(path, schema.deprecated);
}
});
const visibilityBySchemaPath = new Map<string, ConfigVisibility>();
traverse(merged, (schema, path) => {
if (schema.visibility && schema.visibility !== 'backend') {
@@ -99,12 +106,14 @@ export function compileConfigSchemas(
errors: validate.errors ?? [],
visibilityByDataPath: new Map(visibilityByDataPath),
visibilityBySchemaPath,
deprecationBySchemaPath,
};
}
return {
visibilityByDataPath: new Map(visibilityByDataPath),
visibilityBySchemaPath,
deprecationBySchemaPath,
};
};
}
packages/config-loader/src/lib/schema/filtering.test.ts
+26 -1
View File
@@ -16,7 +16,11 @@
import { JsonObject } from '@backstage/types';
import { ConfigVisibility } from './types';
import { filterByVisibility, filterErrorsByVisibility } from './filtering';
import {
filterByVisibility,
filterErrorsByVisibility,
filterByDeprecated,
} from './filtering';
const data = {
arr: ['f', 'b', 's'],
@@ -64,6 +68,13 @@ const visibility = new Map<string, ConfigVisibility>(
}),
);
const deprecations = new Map<string, string>(
Object.entries({
'/properties/arr': 'deprecated array',
'/properties/objB/properties/never': 'deprecated nested property',
}),
);
describe('filterByVisibility', () => {
test.each<[ConfigVisibility[], JsonObject]>([
[
@@ -381,3 +392,17 @@ describe('filterErrorsByVisibility', () => {
]);
});
});
describe('filterByDeprecated', () => {
it('should allow empty list of deprecations', () => {
expect(filterByDeprecated(data, new Map())).toEqual({ deprecatedKeys: [] });
});
it('should return a list of deprecated properties', () => {
expect(filterByDeprecated(data, deprecations)).toEqual({
deprecatedKeys: [
{ key: 'arr', description: 'deprecated array' },
{ key: 'objB.never', description: 'deprecated nested property' },
],
});
});
});
packages/config-loader/src/lib/schema/filtering.ts
+31
View File
@@ -15,6 +15,7 @@
*/
import { JsonObject, JsonValue } from '@backstage/types';
import { has } from 'lodash';
import {
ConfigVisibility,
DEFAULT_CONFIG_VISIBILITY,
@@ -22,6 +23,36 @@ import {
ValidationError,
} from './types';
/**
* This filters data by deprecation status to only include those which have been deprecated
*
* @param data - configuration data
* @param deprecationBySchemaPath - mapping of schema path to deprecation description
* @returns deprecated configuration keys with their deprecation description
*/
export function filterByDeprecated(
data: JsonObject,
deprecationBySchemaPath: Map<string, string>,
withDeprecatedKeys: boolean = true,
): { deprecatedKeys: { key: string; description: string }[] } {
const deprecatedKeys = new Array<{ key: string; description: string }>();
if (!withDeprecatedKeys) {
return { deprecatedKeys };
}
deprecationBySchemaPath.forEach((desc, schemaPath) => {
// convert schema path to object path (/properties/techdocs/properties/storageUrl -> techdocs.storageUrl)
const propertyPath = schemaPath
.replace(/^\/properties\//, '') // remove leading `/properties/` entirely
.replace(/\/properties\//g, '.'); // replace all other `/properties/` with a `.`
if (has(data, propertyPath)) {
deprecatedKeys.push({ key: propertyPath, description: desc });
}
});
return { deprecatedKeys };
}
/**
* This filters data by visibility by discovering the visibility of each
* value, and then only keeping the ones that are specified in `includeVisibilities`.
packages/config-loader/src/lib/schema/load.test.ts
+48 -8
View File
@@ -62,7 +62,11 @@ describe('loadConfigSchema', () => {
expect(schema.process(configs)).toEqual(configs);
expect(schema.process(configs, { visibility: ['frontend'] })).toEqual([
{ data: { key1: 'a' }, context: 'test' },
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(
schema.process(configs, {
@@ -71,7 +75,12 @@ describe('loadConfigSchema', () => {
withFilteredKeys: true,
}),
).toEqual([
{ data: { key1: 'X' }, context: 'test', filteredKeys: ['key2'] },
{
data: { key1: 'X' },
context: 'test',
filteredKeys: ['key2'],
deprecatedKeys: [],
},
]);
expect(
schema.process(configs, {
@@ -79,14 +88,23 @@ describe('loadConfigSchema', () => {
withFilteredKeys: true,
}),
).toEqual([
{ data: { key1: 'X', key2: 'X' }, context: 'test', filteredKeys: [] },
{
data: { key1: 'X', key2: 'X' },
context: 'test',
filteredKeys: [],
deprecatedKeys: [],
},
]);
const serialized = schema.serialize();
const schema2 = await loadConfigSchema({ serialized });
expect(schema2.process(configs, { visibility: ['frontend'] })).toEqual([
{ data: { key1: 'a' }, context: 'test' },
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() =>
schema2.process([...configs, { data: { key1: 3 }, context: 'test2' }]),
@@ -131,7 +149,11 @@ describe('loadConfigSchema', () => {
'Config validation failed, Config should be number { type=number } at /key2',
);
expect(schema.process(configs, { visibility: ['frontend'] })).toEqual([
{ data: { key1: 'a' }, context: 'test' },
{
data: { key1: 'a' },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() => schema.process(configs, { visibility: ['secret'] })).toThrow(
'Config validation failed, Config should be number { type=number } at /key2',
@@ -179,7 +201,13 @@ describe('loadConfigSchema', () => {
];
expect(
schema.process(mkConfig({ x: 1 }), { visibility: ['frontend'] }),
).toEqual([{ data: { nested: [{}] }, context: 'test' }]);
).toEqual([
{
data: { nested: [{}] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() => schema.process(mkConfig({ y: 1 }))).toThrow(
'Config validation failed, Config should be string { type=string } at /nested/0/y',
);
@@ -190,10 +218,22 @@ describe('loadConfigSchema', () => {
);
expect(
schema.process(mkConfig({ x: 'a' }), { visibility: ['frontend'] }),
).toEqual([{ data: { nested: [{}] }, context: 'test' }]);
).toEqual([
{
data: { nested: [{}] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(
schema.process(mkConfig({ y: 'aaa' }), { visibility: ['frontend'] }),
).toEqual([{ data: { nested: [{ y: 'aaa' }] }, context: 'test' }]);
).toEqual([
{
data: { nested: [{ y: 'aaa' }] },
context: 'test',
deprecatedKeys: [],
},
]);
expect(() =>
schema.process(mkConfig({ y: 'aaaa' }), { visibility: ['frontend'] }),
).toThrow(
packages/config-loader/src/lib/schema/load.ts
+16 -2
View File
@@ -18,7 +18,11 @@ import { AppConfig } from '@backstage/config';
import { JsonObject } from '@backstage/types';
import { compileConfigSchemas } from './compile';
import { collectConfigSchemas } from './collect';
import { filterByVisibility, filterErrorsByVisibility } from './filtering';
import {
filterByVisibility,
filterErrorsByVisibility,
filterByDeprecated,
} from './filtering';
import {
ValidationError,
ConfigSchema,
@@ -82,7 +86,7 @@ export async function loadConfigSchema(
return {
process(
configs: AppConfig[],
{ visibility, valueTransform, withFilteredKeys } = {},
{ visibility, valueTransform, withFilteredKeys, withDeprecatedKeys } = {},
): AppConfig[] {
const result = validate(configs);
@@ -108,6 +112,11 @@ export async function loadConfigSchema(
valueTransform,
withFilteredKeys,
),
...filterByDeprecated(
data,
result.deprecationBySchemaPath,
withDeprecatedKeys,
),
}));
} else if (valueTransform) {
processedConfigs = processedConfigs.map(({ data, context }) => ({
@@ -119,6 +128,11 @@ export async function loadConfigSchema(
valueTransform,
withFilteredKeys,
),
...filterByDeprecated(
data,
result.deprecationBySchemaPath,
withDeprecatedKeys,
),
}));
}
packages/config-loader/src/lib/schema/types.ts
+14
View File
@@ -81,6 +81,13 @@ type ValidationResult = {
* The path in the key uses the form `/properties/<key>/items/additionalProperties/<leaf-key>`
*/
visibilityBySchemaPath: Map<string, ConfigVisibility>;
/**
* The deprecated options that were discovered during validation.
*
* The path in the key uses the form `/properties/<key>/items/additionalProperties/<leaf-key>`
*/
deprecationBySchemaPath: Map<string, string>;
};
/**
@@ -124,6 +131,13 @@ export type ConfigSchemaProcessingOptions = {
* Default: `false`.
*/
withFilteredKeys?: boolean;
/**
* Whether or not to include the `deprecatedKeys` property in the output `AppConfig`s.
*
* Default: `true`.
*/
withDeprecatedKeys?: boolean;
};
/**
packages/config/api-report.md
+4
View File
@@ -13,6 +13,10 @@ export type AppConfig = {
context: string;
data: JsonObject_2;
filteredKeys?: string[];
deprecatedKeys?: {
key: string;
description: string;
}[];
};
// @public
packages/config/src/reader.ts
+19 -1
View File
@@ -83,9 +83,27 @@ 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, filteredKeys }) => {
(previousReader, { data, context, filteredKeys, deprecatedKeys }) => {
const reader = new ConfigReader(data, context, previousReader);
reader.filteredKeys = filteredKeys;
// TODO(cmpadden) introduce `deprecatedKeys` and `notifiedDeprecatedKeys` & use logger
if (deprecatedKeys) {
for (const { key, description } of deprecatedKeys) {
if (description) {
// eslint-disable-next-line no-console
console.warn(
`The configuration key '${key}' is deprecated and may be removed soon. (reason: ${description})`,
);
} else {
// eslint-disable-next-line no-console
console.warn(
`The configuration key '${key}' is deprecated and may be removed soon.`,
);
}
}
}
return reader;
},
undefined!,
packages/config/src/types.ts
+6
View File
@@ -36,6 +36,12 @@ export type AppConfig = {
* This can be used to warn the user if they try to read any of these keys.
*/
filteredKeys?: string[];
/**
* A list of deprecated keys that were found in the configuration when it was loaded.
*
* This can be used to warn the user if they are using deprecated properties.
*/
deprecatedKeys?: { key: string; description: string }[];
};
/**