diff --git a/.changeset/light-trainers-allow.md b/.changeset/light-trainers-allow.md new file mode 100644 index 0000000000..0ea13cd19e --- /dev/null +++ b/.changeset/light-trainers-allow.md @@ -0,0 +1,22 @@ +--- +'@backstage/backend-common': patch +'@backstage/config': patch +'@backstage/config-loader': patch +'@backstage/plugin-app-backend': patch +--- + +Loading of app configurations now reference the `@deprecated` construct from +JSDoc to determine if a property in-use has been deprecated. Users are notified +of deprecated keys in the format: + +```txt +The configuration key 'catalog.processors.githubOrg' of app-config.yaml is deprecated and may be removed soon. Configure a GitHub integration instead. +``` + +When the `withDeprecatedKeys` option is set to `true` in the `process` method +of `loadConfigSchema`, the user will be notified that deprecated keys have been +identified in their app configuration. + +The `backend-common` and `plugin-app-backend` packages have been updated to set +`withDeprecatedKeys` to true so that users are notified of deprecated settings +by default. diff --git a/.changeset/long-clouds-rest.md b/.changeset/long-clouds-rest.md new file mode 100644 index 0000000000..701be91536 --- /dev/null +++ b/.changeset/long-clouds-rest.md @@ -0,0 +1,12 @@ +--- +'@backstage/cli': patch +--- + +Introduce `--deprecated` option to `config:check` to log all deprecated app configuration properties + +```sh +$ yarn backstage-cli config:check --lax --deprecated +config:check --lax --deprecated +Loaded config from app-config.yaml +The configuration key 'catalog.processors.githubOrg' of app-config.yaml is deprecated and may be removed soon. Configure a GitHub integration instead. +``` diff --git a/docs/local-dev/cli-commands.md b/docs/local-dev/cli-commands.md index caec60ed70..21b129e0c2 100644 --- a/docs/local-dev/cli-commands.md +++ b/docs/local-dev/cli-commands.md @@ -556,6 +556,7 @@ Options: --package <name> Only load config schema that applies to the given package --lax Do not require environment variables to be set --frontend Only validate the frontend configuration + --deprecated List all deprecated configuration settings --config <path> Config files to load instead of app-config.yaml (default: []) -h, --help display help for command ``` diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index 12cc2e3a9d..9081b99284 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -39,7 +39,10 @@ const updateRedactionList = ( configs: AppConfig[], logger: Logger, ) => { - const secretAppConfigs = schema.process(configs, { visibility: ['secret'] }); + const secretAppConfigs = schema.process(configs, { + visibility: ['secret'], + withDeprecatedKeys: true, + }); const secretConfig = ConfigReader.fromConfigs(secretAppConfigs); const values = new Set(); const data = secretConfig.get(); diff --git a/packages/cli/src/commands/config/validate.ts b/packages/cli/src/commands/config/validate.ts index 2d3ce60366..8d528b8300 100644 --- a/packages/cli/src/commands/config/validate.ts +++ b/packages/cli/src/commands/config/validate.ts @@ -23,5 +23,6 @@ export default async (cmd: Command) => { fromPackage: cmd.package, mockEnv: cmd.lax, fullVisibility: !cmd.frontend, + withDeprecatedKeys: cmd.deprecated, }); }; diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index fa00bf4073..dfe427b27b 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -193,6 +193,7 @@ export function registerCommands(program: CommanderStatic) { ) .option('--lax', 'Do not require environment variables to be set') .option('--frontend', 'Only validate the frontend configuration') + .option('--deprecated', 'Output deprecated configuration settings') .option(...configOption) .description( 'Validate that the given configuration loads and matches schema', diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index b43e4798b4..e7ceeadf08 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -30,6 +30,7 @@ type Options = { fromPackage?: string; mockEnv?: boolean; withFilteredKeys?: boolean; + withDeprecatedKeys?: boolean; fullVisibility?: boolean; }; @@ -91,6 +92,7 @@ export async function loadCliConfig(options: Options) { ? ['frontend', 'backend', 'secret'] : ['frontend'], withFilteredKeys: options.withFilteredKeys, + withDeprecatedKeys: options.withDeprecatedKeys, }); const frontendConfig = ConfigReader.fromConfigs(frontendAppConfigs); diff --git a/packages/config-loader/api-report.md b/packages/config-loader/api-report.md index 106c744d7b..06868e0f47 100644 --- a/packages/config-loader/api-report.md +++ b/packages/config-loader/api-report.md @@ -21,6 +21,7 @@ export type ConfigSchemaProcessingOptions = { visibility?: ConfigVisibility[]; valueTransform?: TransformFunc; withFilteredKeys?: boolean; + withDeprecatedKeys?: boolean; }; // @public (undocumented) diff --git a/packages/config-loader/src/lib/schema/collect.ts b/packages/config-loader/src/lib/schema/collect.ts index 8e7f582e44..a05796139b 100644 --- a/packages/config-loader/src/lib/schema/collect.ts +++ b/packages/config-loader/src/lib/schema/collect.ts @@ -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 in doc comments { required: true, - validationKeywords: ['visibility'], + validationKeywords: ['visibility', 'deprecated'], }, [path.split(sep).join('/')], // Unix paths are expected for all OSes here ) as JsonObject | null; diff --git a/packages/config-loader/src/lib/schema/compile.test.ts b/packages/config-loader/src/lib/schema/compile.test.ts index 1e63f3cbf8..7b7db87c8f 100644 --- a/packages/config-loader/src/lib/schema/compile.test.ts +++ b/packages/config-loader/src/lib/schema/compile.test.ts @@ -40,6 +40,7 @@ describe('compileConfigSchemas', () => { ], visibilityByDataPath: new Map(), visibilityBySchemaPath: new Map(), + deprecationByDataPath: new Map(), }); expect(validate([{ data: { b: 'b' }, context: 'test' }])).toEqual({ errors: [ @@ -53,6 +54,7 @@ describe('compileConfigSchemas', () => { ], visibilityByDataPath: new Map(), visibilityBySchemaPath: new Map(), + deprecationByDataPath: new Map(), }); }); @@ -112,6 +114,7 @@ describe('compileConfigSchemas', () => { '/properties/d/items': 'frontend', }), ), + deprecationByDataPath: 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({ + deprecationByDataPath: new Map( + Object.entries({ + '/a': 'deprecation reason for a', + '/b': 'deprecation reason for b', + }), + ), + visibilityByDataPath: new Map(), + visibilityBySchemaPath: new Map(), + }); + }); }); diff --git a/packages/config-loader/src/lib/schema/compile.ts b/packages/config-loader/src/lib/schema/compile.ts index 1cbf5a404d..b7f705751e 100644 --- a/packages/config-loader/src/lib/schema/compile.ts +++ b/packages/config-loader/src/lib/schema/compile.ts @@ -40,6 +40,7 @@ export function compileConfigSchemas( // output during validation. We work around this by having this extra piece // of state that we reset before each validation. const visibilityByDataPath = new Map(); + const deprecationByDataPath = new Map(); const ajv = new Ajv({ allErrors: true, @@ -47,28 +48,48 @@ export function compileConfigSchemas( schemas: { 'https://backstage.io/schema/config-v1': true, }, - }).addKeyword({ - keyword: 'visibility', - metaSchema: { - type: 'string', - enum: CONFIG_VISIBILITIES, - }, - compile(visibility: ConfigVisibility) { - return (_data, context) => { - if (context?.dataPath === undefined) { - return false; - } - if (visibility && visibility !== 'backend') { + }) + .addKeyword({ + keyword: 'visibility', + metaSchema: { + type: 'string', + enum: CONFIG_VISIBILITIES, + }, + compile(visibility: ConfigVisibility) { + return (_data, context) => { + if (context?.dataPath === undefined) { + return false; + } + if (visibility && visibility !== 'backend') { + const normalizedPath = context.dataPath.replace( + /\['?(.*?)'?\]/g, + (_, segment) => `/${segment}`, + ); + visibilityByDataPath.set(normalizedPath, visibility); + } + return true; + }; + }, + }) + .removeKeyword('deprecated') // remove `deprecated` keyword so that we can implement our own compiler + .addKeyword({ + keyword: 'deprecated', + metaSchema: { type: 'string' }, + compile(deprecationDescription: string) { + return (_data, context) => { + if (context?.dataPath === undefined) { + return false; + } const normalizedPath = context.dataPath.replace( /\['?(.*?)'?\]/g, (_, segment) => `/${segment}`, ); - visibilityByDataPath.set(normalizedPath, visibility); - } - return true; - }; - }, - }); + // create mapping of deprecation description and data path of property + deprecationByDataPath.set(normalizedPath, deprecationDescription); + return true; + }; + }, + }); for (const schema of schemas) { try { @@ -79,6 +100,7 @@ export function compileConfigSchemas( } const merged = mergeConfigSchemas(schemas.map(_ => _.value)); + const validate = ajv.compile(merged); const visibilityBySchemaPath = new Map(); @@ -94,17 +116,20 @@ export function compileConfigSchemas( visibilityByDataPath.clear(); const valid = validate(config); + if (!valid) { return { errors: validate.errors ?? [], visibilityByDataPath: new Map(visibilityByDataPath), visibilityBySchemaPath, + deprecationByDataPath, }; } return { visibilityByDataPath: new Map(visibilityByDataPath), visibilityBySchemaPath, + deprecationByDataPath, }; }; } diff --git a/packages/config-loader/src/lib/schema/filtering.test.ts b/packages/config-loader/src/lib/schema/filtering.test.ts index be07b915ed..df06975c9f 100644 --- a/packages/config-loader/src/lib/schema/filtering.test.ts +++ b/packages/config-loader/src/lib/schema/filtering.test.ts @@ -64,6 +64,13 @@ const visibility = new Map( }), ); +const deprecations = new Map( + Object.entries({ + '/arr': 'deprecated array', + '/objB/never': 'deprecated nested property', + }), +); + describe('filterByVisibility', () => { test.each<[ConfigVisibility[], JsonObject]>([ [ @@ -185,9 +192,34 @@ describe('filterByVisibility', () => { [['frontend', 'backend', 'secret'], { data, filteredKeys: [] }], ])('should filter correctly with %p', (filter, expected) => { expect( - filterByVisibility(data, filter, visibility, undefined, true), + filterByVisibility( + data, + filter, + visibility, + deprecations, + undefined, + true, + false, + ), ).toEqual(expected); }); + + it('should include deprecated keys regardless of visibility', () => { + expect( + filterByVisibility( + data, + [], + visibility, + deprecations, + undefined, + true, + true, + ).deprecatedKeys, + ).toEqual([ + { key: 'arr', description: 'deprecated array' }, + { key: 'objB.never', description: 'deprecated nested property' }, + ]); + }); }); describe('filterErrorsByVisibility', () => { diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts index b2f39d3cc4..2855979b1f 100644 --- a/packages/config-loader/src/lib/schema/filtering.ts +++ b/packages/config-loader/src/lib/schema/filtering.ts @@ -30,10 +30,17 @@ export function filterByVisibility( data: JsonObject, includeVisibilities: ConfigVisibility[], visibilityByDataPath: Map, + deprecationByDataPath: Map, transformFunc?: TransformFunc, withFilteredKeys?: boolean, -): { data: JsonObject; filteredKeys?: string[] } { + withDeprecatedKeys?: boolean, +): { + data: JsonObject; + filteredKeys?: string[]; + deprecatedKeys?: { key: string; description: string }[]; +} { const filteredKeys = new Array(); + const deprecatedKeys = new Array<{ key: string; description: string }>(); function transform( jsonVal: JsonValue, @@ -44,6 +51,12 @@ export function filterByVisibility( visibilityByDataPath.get(visibilityPath) ?? DEFAULT_CONFIG_VISIBILITY; const isVisible = includeVisibilities.includes(visibility); + // deprecated keys are added regardless of visibility indicator + const deprecation = deprecationByDataPath.get(visibilityPath); + if (deprecation) { + deprecatedKeys.push({ key: filterPath, description: deprecation }); + } + if (typeof jsonVal !== 'object') { if (isVisible) { if (transformFunc) { @@ -109,6 +122,7 @@ export function filterByVisibility( return { filteredKeys: withFilteredKeys ? filteredKeys : undefined, + deprecatedKeys: withDeprecatedKeys ? deprecatedKeys : undefined, data: (transform(data, '', '') as JsonObject) ?? {}, }; } diff --git a/packages/config-loader/src/lib/schema/load.test.ts b/packages/config-loader/src/lib/schema/load.test.ts index e2ae3cc2c7..8510ccb1d8 100644 --- a/packages/config-loader/src/lib/schema/load.test.ts +++ b/packages/config-loader/src/lib/schema/load.test.ts @@ -62,7 +62,10 @@ 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', + }, ]); expect( schema.process(configs, { @@ -71,7 +74,11 @@ describe('loadConfigSchema', () => { withFilteredKeys: true, }), ).toEqual([ - { data: { key1: 'X' }, context: 'test', filteredKeys: ['key2'] }, + { + data: { key1: 'X' }, + context: 'test', + filteredKeys: ['key2'], + }, ]); expect( schema.process(configs, { @@ -79,14 +86,21 @@ describe('loadConfigSchema', () => { withFilteredKeys: true, }), ).toEqual([ - { data: { key1: 'X', key2: 'X' }, context: 'test', filteredKeys: [] }, + { + data: { key1: 'X', key2: 'X' }, + context: 'test', + filteredKeys: [], + }, ]); 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', + }, ]); expect(() => schema2.process([...configs, { data: { key1: 3 }, context: 'test2' }]), @@ -131,7 +145,10 @@ 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', + }, ]); expect(() => schema.process(configs, { visibility: ['secret'] })).toThrow( 'Config validation failed, Config should be number { type=number } at /key2', @@ -179,7 +196,12 @@ describe('loadConfigSchema', () => { ]; expect( schema.process(mkConfig({ x: 1 }), { visibility: ['frontend'] }), - ).toEqual([{ data: { nested: [{}] }, context: 'test' }]); + ).toEqual([ + { + data: { nested: [{}] }, + context: 'test', + }, + ]); expect(() => schema.process(mkConfig({ y: 1 }))).toThrow( 'Config validation failed, Config should be string { type=string } at /nested/0/y', ); @@ -190,10 +212,20 @@ describe('loadConfigSchema', () => { ); expect( schema.process(mkConfig({ x: 'a' }), { visibility: ['frontend'] }), - ).toEqual([{ data: { nested: [{}] }, context: 'test' }]); + ).toEqual([ + { + data: { nested: [{}] }, + context: 'test', + }, + ]); expect( schema.process(mkConfig({ y: 'aaa' }), { visibility: ['frontend'] }), - ).toEqual([{ data: { nested: [{ y: 'aaa' }] }, context: 'test' }]); + ).toEqual([ + { + data: { nested: [{ y: 'aaa' }] }, + context: 'test', + }, + ]); expect(() => schema.process(mkConfig({ y: 'aaaa' }), { visibility: ['frontend'] }), ).toThrow( diff --git a/packages/config-loader/src/lib/schema/load.ts b/packages/config-loader/src/lib/schema/load.ts index 2d78785304..aa7ed06305 100644 --- a/packages/config-loader/src/lib/schema/load.ts +++ b/packages/config-loader/src/lib/schema/load.ts @@ -82,7 +82,7 @@ export async function loadConfigSchema( return { process( configs: AppConfig[], - { visibility, valueTransform, withFilteredKeys } = {}, + { visibility, valueTransform, withFilteredKeys, withDeprecatedKeys } = {}, ): AppConfig[] { const result = validate(configs); @@ -105,8 +105,10 @@ export async function loadConfigSchema( data, visibility, result.visibilityByDataPath, + result.deprecationByDataPath, valueTransform, withFilteredKeys, + withDeprecatedKeys, ), })); } else if (valueTransform) { @@ -116,8 +118,10 @@ export async function loadConfigSchema( data, Array.from(CONFIG_VISIBILITIES), result.visibilityByDataPath, + result.deprecationByDataPath, valueTransform, withFilteredKeys, + withDeprecatedKeys, ), })); } diff --git a/packages/config-loader/src/lib/schema/types.ts b/packages/config-loader/src/lib/schema/types.ts index 53f4dec88e..75993b3c78 100644 --- a/packages/config-loader/src/lib/schema/types.ts +++ b/packages/config-loader/src/lib/schema/types.ts @@ -81,6 +81,13 @@ type ValidationResult = { * The path in the key uses the form `/properties//items/additionalProperties/` */ visibilityBySchemaPath: Map; + + /** + * The deprecated options that were discovered during validation. + * + * The path in the key uses the form `////` + */ + deprecationByDataPath: Map; }; /** @@ -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; }; /** diff --git a/packages/config/api-report.md b/packages/config/api-report.md index efa6b85559..df9baeb4cd 100644 --- a/packages/config/api-report.md +++ b/packages/config/api-report.md @@ -13,6 +13,10 @@ export type AppConfig = { context: string; data: JsonObject_2; filteredKeys?: string[]; + deprecatedKeys?: { + key: string; + description: string; + }[]; }; // @public diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 129a703784..1ce54590b5 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -83,9 +83,21 @@ 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( - (previousReader, { data, context, filteredKeys }) => { + (previousReader, { data, context, filteredKeys, deprecatedKeys }) => { const reader = new ConfigReader(data, context, previousReader); reader.filteredKeys = filteredKeys; + + if (deprecatedKeys) { + for (const { key, description } of deprecatedKeys) { + // eslint-disable-next-line no-console + console.warn( + `The configuration key '${key}' of ${context} is deprecated and may be removed soon. ${ + description || '' + }`, + ); + } + } + return reader; }, undefined!, diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts index a543233277..37c9595565 100644 --- a/packages/config/src/types.ts +++ b/packages/config/src/types.ts @@ -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 }[]; }; /** diff --git a/plugins/app-backend/src/lib/config.ts b/plugins/app-backend/src/lib/config.ts index c37dc34dcc..6959253336 100644 --- a/plugins/app-backend/src/lib/config.ts +++ b/plugins/app-backend/src/lib/config.ts @@ -91,7 +91,7 @@ export async function readConfigs(options: ReadOptions): Promise { const frontendConfigs = await schema.process( [{ data: config.get() as JsonObject, context: 'app' }], - { visibility: ['frontend'] }, + { visibility: ['frontend'], withDeprecatedKeys: true }, ); appConfigs.push(...frontendConfigs); } catch (error) {