Merge pull request #8719 from cmpadden/warn-app-config-deprecations

Warn users of deprecated keys in app configurations

.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.

.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.
+```

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
 ```

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<string>();
   const data = secretConfig.get();

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,
   });
 };

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',

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);
 

packages/config-loader/api-report.md

@@ -21,6 +21,7 @@ export type ConfigSchemaProcessingOptions = {
   visibility?: ConfigVisibility[];
   valueTransform?: TransformFunc<any>;
   withFilteredKeys?: boolean;
+  withDeprecatedKeys?: boolean;
 };
 
 // @public (undocumented)

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;

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(),
+    });
+  });
 });

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<string, ConfigVisibility>();
+  const deprecationByDataPath = new Map<string, string>();
 
   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<string, ConfigVisibility>();
@@ -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,
     };
   };
 }

packages/config-loader/src/lib/schema/filtering.test.ts

@@ -64,6 +64,13 @@ const visibility = new Map<string, ConfigVisibility>(
   }),
 );
 
+const deprecations = new Map<string, string>(
+  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', () => {

packages/config-loader/src/lib/schema/filtering.ts

@@ -30,10 +30,17 @@ export function filterByVisibility(
   data: JsonObject,
   includeVisibilities: ConfigVisibility[],
   visibilityByDataPath: Map<string, ConfigVisibility>,
+  deprecationByDataPath: Map<string, string>,
   transformFunc?: TransformFunc<number | string | boolean>,
   withFilteredKeys?: boolean,
-): { data: JsonObject; filteredKeys?: string[] } {
+  withDeprecatedKeys?: boolean,
+): {
+  data: JsonObject;
+  filteredKeys?: string[];
+  deprecatedKeys?: { key: string; description: string }[];
+} {
   const filteredKeys = new Array<string>();
+  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) ?? {},
   };
 }

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(

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,
           ),
         }));
       }

packages/config-loader/src/lib/schema/types.ts

@@ -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 `/<key>/<sub-key>/<array-index>/<leaf-key>`
+   */
+  deprecationByDataPath: 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

@@ -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

@@ -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<ConfigReader>(
-      (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!,

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 }[];
 };
 
 /**

plugins/app-backend/src/lib/config.ts

@@ -91,7 +91,7 @@ export async function readConfigs(options: ReadOptions): Promise<AppConfig[]> {
 
       const frontendConfigs = await schema.process(
         [{ data: config.get() as JsonObject, context: 'app' }],
-        { visibility: ['frontend'] },
+        { visibility: ['frontend'], withDeprecatedKeys: true },
       );
       appConfigs.push(...frontendConfigs);
     } catch (error) {