From 84fcbc49d6d444bcfb347886ed8ca33cb139954b Mon Sep 17 00:00:00 2001 From: David Festal Date: Tue, 6 Feb 2024 16:43:27 +0100 Subject: [PATCH] Fix review comment: don't use `ConfigSchemaPackageEntry` nor make it public. Signed-off-by: David Festal --- packages/backend-app-api/api-report.md | 5 ++-- packages/backend-app-api/src/config/config.ts | 4 +-- .../rootLoggerServiceFactory.test.ts | 28 ++++++++----------- packages/backend-common/api-report.md | 6 ++-- packages/backend-common/src/config.test.ts | 19 ++++++------- packages/backend-common/src/config.ts | 8 ++---- .../src/scanner/plugin-scanner.ts | 6 ++-- .../src/scanner/schemas.ts | 11 +++----- .../backend-plugin-api/api-report-alpha.md | 6 ++-- packages/backend-plugin-api/src/alpha.ts | 8 ++++-- packages/config-loader/api-report.md | 10 ++----- packages/config-loader/src/index.ts | 1 - .../config-loader/src/schema/load.test.ts | 15 ++++------ packages/config-loader/src/schema/load.ts | 13 +++++++-- packages/config-loader/src/schema/types.ts | 4 +-- plugins/app-backend/api-report.md | 6 ++-- plugins/app-backend/src/lib/config.ts | 3 +- plugins/app-backend/src/service/router.ts | 6 ++-- 18 files changed, 76 insertions(+), 83 deletions(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 1143beec15..cff8c3c0cd 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -9,7 +9,6 @@ import type { AppConfig } from '@backstage/config'; import { BackendFeature } from '@backstage/backend-plugin-api'; import { CacheClient } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; import { CorsOptions } from 'cors'; import { DiscoveryService } from '@backstage/backend-plugin-api'; import { ErrorRequestHandler } from 'express'; @@ -65,7 +64,9 @@ export const cacheServiceFactory: () => ServiceFactory; export function createConfigSecretEnumerator(options: { logger: LoggerService; dir?: string; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { + [context: string]: JsonObject; + }; }): Promise<(config: Config) => Iterable>; // @public diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index d4eb5fccb9..abca385e62 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -23,19 +23,19 @@ import { loadConfig, ConfigTarget, LoadConfigOptionsRemote, - ConfigSchemaPackageEntry, } from '@backstage/config-loader'; import { ConfigReader } from '@backstage/config'; import type { Config, AppConfig } from '@backstage/config'; import { getPackages } from '@manypkg/get-packages'; import { ObservableConfigProxy } from './ObservableConfigProxy'; import { isValidUrl } from '../lib/urls'; +import { JsonObject } from '@backstage/types'; /** @public */ export async function createConfigSecretEnumerator(options: { logger: LoggerService; dir?: string; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { [context: string]: JsonObject }; }): Promise<(config: Config) => Iterable> { const { logger, dir = process.cwd() } = options; const { packages } = await getPackages(dir); diff --git a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerServiceFactory.test.ts b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerServiceFactory.test.ts index ac7b9e8cf2..b840822c6f 100644 --- a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerServiceFactory.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerServiceFactory.test.ts @@ -22,15 +22,12 @@ import { createServiceFactory, } from '@backstage/backend-plugin-api'; import { schemaDiscoveryServiceRef } from '@backstage/backend-plugin-api/alpha'; -import { - ConfigSchemaPackageEntry, - ConfigSources, - StaticConfigSource, -} from '@backstage/config-loader'; +import { ConfigSources, StaticConfigSource } from '@backstage/config-loader'; import { transports } from 'winston'; import { rootLifecycleServiceFactory } from '../rootLifecycle'; import { lifecycleServiceFactory } from '../lifecycle'; import { loggerServiceFactory } from '../logger'; +import { JsonObject } from '@backstage/types'; describe('rootLogger', () => { describe('rootLoggerServiceFactory', () => { @@ -46,20 +43,17 @@ describe('rootLogger', () => { }, }; - const additionalSchemas = [ - { - path: 'test', - value: { - type: 'object', - properties: { - secretValue: { - type: 'string', - visibility: 'secret', - }, + const additionalSchemas = { + test: { + type: 'object', + properties: { + secretValue: { + type: 'string', + visibility: 'secret', }, }, }, - ]; + }; const logs: string[] = []; jest @@ -97,7 +91,7 @@ describe('rootLogger', () => { }, factory: async () => ({ getAdditionalSchemas: async (): Promise<{ - schemas: Array; + schemas: { [context: string]: JsonObject }; }> => ({ schemas: additionalSchemas, }), diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 7a3fc72a5b..607eaa010e 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -20,7 +20,6 @@ import { CacheService as CacheClient } from '@backstage/backend-plugin-api'; import { CacheServiceOptions as CacheClientOptions } from '@backstage/backend-plugin-api'; import { CacheServiceSetOptions as CacheClientSetOptions } from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; import cors from 'cors'; import Docker from 'dockerode'; import { ErrorRequestHandler } from 'express'; @@ -33,6 +32,7 @@ import { GitLabIntegration } from '@backstage/integration'; import { HostDiscovery as HostDiscovery_2 } from '@backstage/backend-app-api'; import { IdentityService } from '@backstage/backend-plugin-api'; import { isChildPath } from '@backstage/cli-common'; +import { JsonObject } from '@backstage/types'; import { Knex } from 'knex'; import knexFactory from 'knex'; import { KubeConfig } from '@kubernetes/client-node'; @@ -561,7 +561,9 @@ export function loadBackendConfig(options: { logger: LoggerService; remote?: LoadConfigOptionsRemote; additionalConfigs?: AppConfig[]; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { + [context: string]: JsonObject; + }; argv: string[]; watch?: boolean; }): Promise; diff --git a/packages/backend-common/src/config.test.ts b/packages/backend-common/src/config.test.ts index 931c0b5c84..05d1cbc3ec 100644 --- a/packages/backend-common/src/config.test.ts +++ b/packages/backend-common/src/config.test.ts @@ -36,20 +36,17 @@ describe('config', () => { }, ]; - const additionalSchemas = [ - { - path: 'test', - value: { - type: 'object', - properties: { - secretValue: { - type: 'string', - visibility: 'secret', - }, + const additionalSchemas = { + test: { + type: 'object', + properties: { + secretValue: { + type: 'string', + visibility: 'secret', }, }, }, - ]; + }; const logs: string[] = []; jest diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index b5d1d2b896..ac89f3f4ec 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -20,11 +20,9 @@ import { } from '@backstage/backend-app-api'; import { LoggerService } from '@backstage/backend-plugin-api'; import { AppConfig, Config } from '@backstage/config'; -import { - ConfigSchemaPackageEntry, - LoadConfigOptionsRemote, -} from '@backstage/config-loader'; +import { LoadConfigOptionsRemote } from '@backstage/config-loader'; import { setRootLoggerRedactionList } from './logging/createRootLogger'; +import { JsonObject } from '@backstage/types'; /** * Load configuration for a Backend. @@ -38,7 +36,7 @@ export async function loadBackendConfig(options: { // process.argv or any other overrides remote?: LoadConfigOptionsRemote; additionalConfigs?: AppConfig[]; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { [context: string]: JsonObject }; argv: string[]; watch?: boolean; }): Promise { diff --git a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts index 0c6ad90e52..a9e11ca60b 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts +++ b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts @@ -28,9 +28,9 @@ import { createServiceFactory, } from '@backstage/backend-plugin-api'; import { schemaDiscoveryServiceRef } from '@backstage/backend-plugin-api/alpha'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; import { findPaths } from '@backstage/cli-common'; import { gatherDynamicPluginsSchemas } from './schemas'; +import { JsonObject } from '@backstage/types'; export interface DynamicPluginScannerOptions { config: Config; @@ -350,11 +350,11 @@ export const schemaDiscoveryServiceFactory = createServiceFactory( config: coreServices.rootConfig, }, factory({ config }) { - let schemas: ConfigSchemaPackageEntry[] | undefined; + let schemas: { [context: string]: JsonObject } | undefined; return { async getAdditionalSchemas(): Promise<{ - schemas: Array; + schemas: { [context: string]: JsonObject }; }> { if (schemas) { return { diff --git a/packages/backend-dynamic-feature-service/src/scanner/schemas.ts b/packages/backend-dynamic-feature-service/src/scanner/schemas.ts index ebb456e63b..f00828859a 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/schemas.ts +++ b/packages/backend-dynamic-feature-service/src/scanner/schemas.ts @@ -15,20 +15,20 @@ */ import { ScannedPluginPackage } from '@backstage/backend-dynamic-feature-service'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; import fs from 'fs-extra'; import * as path from 'path'; import * as url from 'url'; import { isEmpty } from 'lodash'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { JsonObject } from '@backstage/types'; export async function gatherDynamicPluginsSchemas( packages: ScannedPluginPackage[], logger: LoggerService, schemaLocator: (pluginPackage: ScannedPluginPackage) => string = () => path.join('dist', 'configSchema.json'), -): Promise { - const allSchemas: { value: any; path: string }[] = []; +): Promise<{ [context: string]: JsonObject }> { + const allSchemas: { [context: string]: JsonObject } = {}; for (const pluginPackage of packages) { let schemaLocation = schemaLocator(pluginPackage); @@ -61,10 +61,7 @@ export async function gatherDynamicPluginsSchemas( continue; } - allSchemas.push({ - path: schemaLocation, - value: serialized, - }); + allSchemas[schemaLocation] = serialized; } return allSchemas; diff --git a/packages/backend-plugin-api/api-report-alpha.md b/packages/backend-plugin-api/api-report-alpha.md index 33b5e748e7..0d9eaf4a0e 100644 --- a/packages/backend-plugin-api/api-report-alpha.md +++ b/packages/backend-plugin-api/api-report-alpha.md @@ -4,7 +4,7 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; +import { JsonObject } from '@backstage/types'; import { ServiceRef } from '@backstage/backend-plugin-api'; // @alpha (undocumented) @@ -25,7 +25,9 @@ export const featureDiscoveryServiceRef: ServiceRef< export interface SchemaDiscoveryService { // (undocumented) getAdditionalSchemas(): Promise<{ - schemas: Array; + schemas: { + [context: string]: JsonObject; + }; }>; } diff --git a/packages/backend-plugin-api/src/alpha.ts b/packages/backend-plugin-api/src/alpha.ts index e5eac99e4a..7d7e3177c2 100644 --- a/packages/backend-plugin-api/src/alpha.ts +++ b/packages/backend-plugin-api/src/alpha.ts @@ -20,7 +20,7 @@ import { createServiceFactory, createServiceRef, } from '@backstage/backend-plugin-api'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; +import { JsonObject } from '@backstage/types'; /** @alpha */ export interface FeatureDiscoveryService { @@ -39,7 +39,9 @@ export const featureDiscoveryServiceRef = /** @alpha */ export interface SchemaDiscoveryService { - getAdditionalSchemas(): Promise<{ schemas: Array }>; + getAdditionalSchemas(): Promise<{ + schemas: { [context: string]: JsonObject }; + }>; } /** @@ -59,7 +61,7 @@ export const schemaDiscoveryServiceRef = factory() { return { async getAdditionalSchemas() { - return { schemas: [] }; + return { schemas: {} }; }, }; }, diff --git a/packages/config-loader/api-report.md b/packages/config-loader/api-report.md index 04f1b6a77b..c3fc9f522e 100644 --- a/packages/config-loader/api-report.md +++ b/packages/config-loader/api-report.md @@ -45,12 +45,6 @@ export type ConfigSchema = { serialize(): JsonObject; }; -// @public -export type ConfigSchemaPackageEntry = { - value: JsonObject; - path: string; -}; - // @public export type ConfigSchemaProcessingOptions = { visibility?: ConfigVisibility[]; @@ -199,7 +193,9 @@ export type LoadConfigSchemaOptions = ( } ) & { noUndeclaredProperties?: boolean; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { + [context: string]: JsonObject; + }; }; // @public diff --git a/packages/config-loader/src/index.ts b/packages/config-loader/src/index.ts index 80435d6000..e20e105c7f 100644 --- a/packages/config-loader/src/index.ts +++ b/packages/config-loader/src/index.ts @@ -27,7 +27,6 @@ export type { ConfigVisibility, LoadConfigSchemaOptions, TransformFunc, - ConfigSchemaPackageEntry, } from './schema'; export { loadConfig } from './loader'; export type { diff --git a/packages/config-loader/src/schema/load.test.ts b/packages/config-loader/src/schema/load.test.ts index 107f8d1f3b..f9299340c3 100644 --- a/packages/config-loader/src/schema/load.test.ts +++ b/packages/config-loader/src/schema/load.test.ts @@ -142,17 +142,14 @@ describe('loadConfigSchema', () => { }, ], }, - additionalSchemas: [ - { - path: 'additionalSchema', - value: { - type: 'object', - properties: { - additionalKey: { type: 'string', visibility: 'frontend' }, - }, + additionalSchemas: { + additionalSchema: { + type: 'object', + properties: { + additionalKey: { type: 'string', visibility: 'frontend' }, }, }, - ], + }, }); expect(schema.serialize()).toEqual({ diff --git a/packages/config-loader/src/schema/load.ts b/packages/config-loader/src/schema/load.ts index 5c535d1ff6..03b85b2756 100644 --- a/packages/config-loader/src/schema/load.ts +++ b/packages/config-loader/src/schema/load.ts @@ -43,7 +43,7 @@ export type LoadConfigSchemaOptions = } ) & { noUndeclaredProperties?: boolean; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { [context: string]: JsonObject }; }; function errorsToError(errors: ValidationError[]): Error { @@ -84,7 +84,16 @@ export async function loadConfigSchema( } schemas = serialized.schemas as ConfigSchemaPackageEntry[]; } - schemas.push(...(options.additionalSchemas || [])); + if (options.additionalSchemas) { + schemas.push( + ...Object.keys(options.additionalSchemas).map(context => { + return { + path: context, + value: options.additionalSchemas![context], + }; + }), + ); + } const validate = compileConfigSchemas(schemas, { noUndeclaredProperties: options.noUndeclaredProperties, diff --git a/packages/config-loader/src/schema/types.ts b/packages/config-loader/src/schema/types.ts index 66e77018ee..8f679c93fc 100644 --- a/packages/config-loader/src/schema/types.ts +++ b/packages/config-loader/src/schema/types.ts @@ -18,9 +18,7 @@ import { AppConfig } from '@backstage/config'; import { JsonObject } from '@backstage/types'; /** - * A sub-set of configuration schema for a given package. - * - * @public + * An sub-set of configuration schema. */ export type ConfigSchemaPackageEntry = { /** diff --git a/plugins/app-backend/api-report.md b/plugins/app-backend/api-report.md index 9ea42d2fbd..fd85b68a87 100644 --- a/plugins/app-backend/api-report.md +++ b/plugins/app-backend/api-report.md @@ -4,8 +4,8 @@ ```ts import { Config } from '@backstage/config'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; import express from 'express'; +import { JsonObject } from '@backstage/types'; import { Logger } from 'winston'; import { PluginDatabaseManager } from '@backstage/backend-common'; @@ -14,7 +14,9 @@ export function createRouter(options: RouterOptions): Promise; // @public (undocumented) export interface RouterOptions { - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { + [context: string]: JsonObject; + }; appPackageName: string; // (undocumented) config: Config; diff --git a/plugins/app-backend/src/lib/config.ts b/plugins/app-backend/src/lib/config.ts index 0e223c3650..dc041a2593 100644 --- a/plugins/app-backend/src/lib/config.ts +++ b/plugins/app-backend/src/lib/config.ts @@ -20,7 +20,6 @@ import { Logger } from 'winston'; import { AppConfig, Config } from '@backstage/config'; import { JsonObject } from '@backstage/types'; import { loadConfigSchema, readEnvConfig } from '@backstage/config-loader'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; type InjectOptions = { appConfigs: AppConfig[]; @@ -75,7 +74,7 @@ type ReadOptions = { env: { [name: string]: string | undefined }; appDistDir: string; config: Config; - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { [context: string]: JsonObject }; }; /** diff --git a/plugins/app-backend/src/service/router.ts b/plugins/app-backend/src/service/router.ts index 8e4750dcef..204d5df26a 100644 --- a/plugins/app-backend/src/service/router.ts +++ b/plugins/app-backend/src/service/router.ts @@ -37,7 +37,7 @@ import { CACHE_CONTROL_NO_CACHE, CACHE_CONTROL_REVALIDATE_CACHE, } from '../lib/headers'; -import { ConfigSchemaPackageEntry } from '@backstage/config-loader'; +import { JsonObject } from '@backstage/types'; // express uses mime v1 while we only have types for mime v2 type Mime = { lookup(arg0: string): string }; @@ -89,14 +89,14 @@ export interface RouterOptions { /** * - * Provides a list of additional config schemas, in addition to the serialized schemas + * Provides a map of additional config schemas, in addition to the serialized schemas * generated during the application build. * This is useful when additional plugins are dynamically loaded in the application at start, * which were not part of the application build. This option allows feeding the corresponding * JSON schemas. * */ - additionalSchemas?: ConfigSchemaPackageEntry[]; + additionalSchemas?: { [context: string]: JsonObject }; } /** @public */