From 2f185958599854e64f7591e0c1d3bb4471e1bb2e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 15:53:27 +0200 Subject: [PATCH 1/6] config-loader: throw error rather than silently ignore invalid config Signed-off-by: Patrik Oldsberg --- .changeset/dirty-chefs-listen.md | 7 +++++ packages/config-loader/src/schema/collect.ts | 29 ++++++++++++++++---- 2 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 .changeset/dirty-chefs-listen.md diff --git a/.changeset/dirty-chefs-listen.md b/.changeset/dirty-chefs-listen.md new file mode 100644 index 0000000000..84b684b977 --- /dev/null +++ b/.changeset/dirty-chefs-listen.md @@ -0,0 +1,7 @@ +--- +'@backstage/config-loader': minor +--- + +Loading invalid TypeScript configuration schemas will now throw an error rather than silently being ignored. + +In particular this includes defining any additional types other than `Config` in the schema file, or use of unsupported types such as `Record` or `Partial`. diff --git a/packages/config-loader/src/schema/collect.ts b/packages/config-loader/src/schema/collect.ts index ac3e4af341..6c9737f7b7 100644 --- a/packages/config-loader/src/schema/collect.ts +++ b/packages/config-loader/src/schema/collect.ts @@ -15,6 +15,7 @@ */ import fs from 'fs-extra'; +import { EOL } from 'os'; import { resolve as resolvePath, relative as relativePath, @@ -163,7 +164,7 @@ async function compileTsSchemas(paths: string[]) { // Lazy loaded, because this brings up all of TypeScript and we don't // want that eagerly loaded in tests - const { getProgramFromFiles, generateSchema } = await import( + const { getProgramFromFiles, buildGenerator } = await import( 'typescript-json-schema' ); @@ -183,17 +184,35 @@ async function compileTsSchemas(paths: string[]) { const tsSchemas = paths.map(path => { let value; try { - value = generateSchema( + const generator = buildGenerator( program, - // All schemas should export a `Config` symbol - 'Config', // This enables the use of these tags in TSDoc comments { required: true, validationKeywords: ['visibility', 'deepVisibility', 'deprecated'], }, [path.split(sep).join('/')], // Unix paths are expected for all OSes here - ) as JsonObject | null; + ); + + const userSymbols = new Set(generator?.getUserSymbols()); + userSymbols.delete('Config'); + if (userSymbols.size !== 0) { + const names = Array.from(userSymbols).join("', '"); + throw new Error( + `Invalid configuration schema in ${path}, additional symbol definitions are not allowed, found '${names}'`, + ); + } + + // All schemas should export a `Config` symbol + value = generator?.getSchemaForSymbol('Config') as JsonObject | null; + + const reffedDefs = Object.keys(generator?.ReffedDefinitions ?? {}); + if (reffedDefs.length !== 0) { + const lines = reffedDefs.join(`${EOL} `); + throw new Error( + `Invalid configuration schema in ${path}, the following definitions are not supported:${EOL}${EOL} ${lines}`, + ); + } } catch (error) { assertError(error); if (error.message !== 'type Config not found') { From 7245b3e17e6de62177ecd17f7931f5dd950c5e07 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 16:05:56 +0200 Subject: [PATCH 2/6] removed usage of Record and top-level types from configuration schemas Signed-off-by: Patrik Oldsberg --- .../catalog-backend-module-aws/config.d.ts | 9 ++- .../catalog-backend-module-azure/config.d.ts | 60 +++++++++---------- .../config.d.ts | 9 ++- .../config.d.ts | 9 ++- .../catalog-backend-module-gerrit/config.d.ts | 9 ++- .../catalog-backend-module-github/config.d.ts | 9 ++- .../catalog-backend-module-gitlab/config.d.ts | 9 ++- .../config.d.ts | 9 ++- .../config.d.ts | 9 ++- .../events-backend-module-aws-sqs/config.d.ts | 9 ++- 10 files changed, 66 insertions(+), 75 deletions(-) diff --git a/plugins/catalog-backend-module-aws/config.d.ts b/plugins/catalog-backend-module-aws/config.d.ts index b3610ccfc5..f3f80fc7db 100644 --- a/plugins/catalog-backend-module-aws/config.d.ts +++ b/plugins/catalog-backend-module-aws/config.d.ts @@ -59,9 +59,8 @@ export interface Config { */ schedule?: TaskScheduleDefinitionConfig; } - | Record< - string, - { + | { + [name: string]: { /** * (Required) AWS S3 Bucket Name */ @@ -81,8 +80,8 @@ export interface Config { * (Optional) TaskScheduleDefinition for the refresh. */ schedule?: TaskScheduleDefinitionConfig; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-azure/config.d.ts b/plugins/catalog-backend-module-azure/config.d.ts index 940695c8ea..757ed2bfd3 100644 --- a/plugins/catalog-backend-module-azure/config.d.ts +++ b/plugins/catalog-backend-module-azure/config.d.ts @@ -16,35 +16,6 @@ import { TaskScheduleDefinitionConfig } from '@backstage/backend-tasks'; -interface AzureDevOpsConfig { - /** - * (Optional) The DevOps host; leave empty for `dev.azure.com`, otherwise set to your self-hosted instance host. - */ - host: string; - /** - * (Required) Your organization slug. - */ - organization: string; - /** - * (Required) Your project slug. - */ - project: string; - /** - * (Optional) The repository name. Wildcards are supported as show on the examples above. - * If not set, all repositories will be searched. - */ - repository?: string; - /** - * (Optional) Where to find catalog-info.yaml files. Wildcards are supported. - * If not set, defaults to /catalog-info.yaml. - */ - path?: string; - /** - * (Optional) TaskScheduleDefinition for the refresh. - */ - schedule?: TaskScheduleDefinitionConfig; -} - export interface Config { catalog?: { /** @@ -54,7 +25,36 @@ export interface Config { /** * AzureDevopsEntityProvider configuration */ - azureDevOps?: Record; + azureDevOps?: { + [name: string]: { + /** + * (Optional) The DevOps host; leave empty for `dev.azure.com`, otherwise set to your self-hosted instance host. + */ + host: string; + /** + * (Required) Your organization slug. + */ + organization: string; + /** + * (Required) Your project slug. + */ + project: string; + /** + * (Optional) The repository name. Wildcards are supported as show on the examples above. + * If not set, all repositories will be searched. + */ + repository?: string; + /** + * (Optional) Where to find catalog-info.yaml files. Wildcards are supported. + * If not set, defaults to /catalog-info.yaml. + */ + path?: string; + /** + * (Optional) TaskScheduleDefinition for the refresh. + */ + schedule?: TaskScheduleDefinitionConfig; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts b/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts index 8fa58413c0..9e97597724 100644 --- a/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts +++ b/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts @@ -60,9 +60,8 @@ export interface Config { */ schedule?: TaskScheduleDefinitionConfig; } - | Record< - string, - { + | { + [name: string]: { /** * (Optional) Path to the catalog file. Default to "/catalog-info.yaml". * @visibility frontend @@ -93,8 +92,8 @@ export interface Config { * (Optional) TaskScheduleDefinition for the discovery. */ schedule?: TaskScheduleDefinitionConfig; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-bitbucket-server/config.d.ts b/plugins/catalog-backend-module-bitbucket-server/config.d.ts index 67f694d419..0554d62df3 100644 --- a/plugins/catalog-backend-module-bitbucket-server/config.d.ts +++ b/plugins/catalog-backend-module-bitbucket-server/config.d.ts @@ -52,9 +52,8 @@ export interface Config { */ schedule?: TaskScheduleDefinitionConfig; } - | Record< - string, - { + | { + [name: string]: { /** * (Optional) Path to the catalog file. Default to "/catalog-info.yaml". * @visibility frontend @@ -80,8 +79,8 @@ export interface Config { * (Optional) TaskScheduleDefinition for the refresh. */ schedule?: TaskScheduleDefinitionConfig; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-gerrit/config.d.ts b/plugins/catalog-backend-module-gerrit/config.d.ts index d8b48d2967..a246662b0a 100644 --- a/plugins/catalog-backend-module-gerrit/config.d.ts +++ b/plugins/catalog-backend-module-gerrit/config.d.ts @@ -25,9 +25,8 @@ export interface Config { * * Maps provider id with configuration. */ - gerrit?: Record< - string, - { + gerrit?: { + [name: string]: { /** * (Required) The host of the Gerrit integration to use. */ @@ -42,8 +41,8 @@ export interface Config { * The branch where the provider will try to find entities. Defaults to "master". */ branch?: string; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-github/config.d.ts b/plugins/catalog-backend-module-github/config.d.ts index 5a699e745f..9755504ee2 100644 --- a/plugins/catalog-backend-module-github/config.d.ts +++ b/plugins/catalog-backend-module-github/config.d.ts @@ -118,9 +118,8 @@ export interface Config { */ schedule?: TaskScheduleDefinitionConfig; } - | Record< - string, - { + | { + [name: string]: { /** * (Optional) The hostname of your GitHub Enterprise instance. * Default: `github.com`. @@ -182,8 +181,8 @@ export interface Config { * (Optional) TaskScheduleDefinition for the refresh. */ schedule?: TaskScheduleDefinitionConfig; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-gitlab/config.d.ts b/plugins/catalog-backend-module-gitlab/config.d.ts index f5afc71093..e9732c0642 100644 --- a/plugins/catalog-backend-module-gitlab/config.d.ts +++ b/plugins/catalog-backend-module-gitlab/config.d.ts @@ -22,9 +22,8 @@ export interface Config { /** * GitlabDiscoveryEntityProvider configuration */ - gitlab?: Record< - string, - { + gitlab?: { + [name: string]: { /** * (Required) Gitlab's host name. */ @@ -64,8 +63,8 @@ export interface Config { * (Optional) Skip forked repository */ skipForkedRepos?: boolean; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-msgraph/config.d.ts b/plugins/catalog-backend-module-msgraph/config.d.ts index 904ce1f646..9d341215cd 100644 --- a/plugins/catalog-backend-module-msgraph/config.d.ts +++ b/plugins/catalog-backend-module-msgraph/config.d.ts @@ -209,9 +209,8 @@ export interface Config { */ schedule?: TaskScheduleDefinitionConfig; } - | Record< - string, - { + | { + [name: string]: { /** * The prefix of the target that this matches on, e.g. * "https://graph.microsoft.com/v1.0", with no trailing slash. @@ -296,8 +295,8 @@ export interface Config { * (Optional) TaskScheduleDefinition for the refresh. */ schedule?: TaskScheduleDefinitionConfig; - } - >; + }; + }; }; }; } diff --git a/plugins/catalog-backend-module-puppetdb/config.d.ts b/plugins/catalog-backend-module-puppetdb/config.d.ts index edd7ab3889..8aab239047 100644 --- a/plugins/catalog-backend-module-puppetdb/config.d.ts +++ b/plugins/catalog-backend-module-puppetdb/config.d.ts @@ -46,9 +46,8 @@ export interface Config { */ schedule?: TaskScheduleDefinition; } - | Record< - string, - { + | { + [name: string]: { /** * (Required) The base URL of PuppetDB API instance. */ @@ -61,8 +60,8 @@ export interface Config { * (Optional) Task schedule definition for the refresh. */ schedule?: TaskScheduleDefinition; - } - >; + }; + }; }; }; } diff --git a/plugins/events-backend-module-aws-sqs/config.d.ts b/plugins/events-backend-module-aws-sqs/config.d.ts index dfef35bf22..c2edfeae8e 100644 --- a/plugins/events-backend-module-aws-sqs/config.d.ts +++ b/plugins/events-backend-module-aws-sqs/config.d.ts @@ -31,9 +31,8 @@ export interface Config { * Contains a record per topic for which an AWS SQS queue * should be used as source of events. */ - topics: Record< - string, - { + topics: { + [name: string]: { /** * (Required) Queue-related configuration. */ @@ -69,8 +68,8 @@ export interface Config { * Default: 1 minute. */ waitTimeAfterEmptyReceive: HumanDuration; - } - >; + }; + }; }; }; }; From 8f7b618a490c3f8c067192336e4346c4572c1418 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 16:06:18 +0200 Subject: [PATCH 3/6] removed usage of Partial in configuration schemas Signed-off-by: Patrik Oldsberg --- packages/backend-common/config.d.ts | 6 +++--- plugins/jenkins-backend/config.d.ts | 10 +++++----- plugins/proxy-backend/config.d.ts | 14 +++++++------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index 3fd886874e..e5b193c543 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -77,17 +77,17 @@ export interface Config { */ connection: | string - | Partial<{ + | { /** * Password that belongs to the client User * @visibility secret */ - password: string; + password?: string; /** * Other connection settings */ [key: string]: unknown; - }>; + }; /** Database name prefix override */ prefix?: string; /** diff --git a/plugins/jenkins-backend/config.d.ts b/plugins/jenkins-backend/config.d.ts index 5b725dd801..1773ea7e75 100644 --- a/plugins/jenkins-backend/config.d.ts +++ b/plugins/jenkins-backend/config.d.ts @@ -42,13 +42,13 @@ export interface Config { username: string; /** @visibility secret */ apiKey: string; - extraRequestHeaders?: Partial<{ + extraRequestHeaders?: { /** @visibility secret */ - Authorization: string; + Authorization?: string; /** @visibility secret */ - authorization: string; - [key: string]: string; - }>; + authorization?: string; + [key: string]: string | undefined; + }; }[]; }; } diff --git a/plugins/proxy-backend/config.d.ts b/plugins/proxy-backend/config.d.ts index 410ff86a2e..bc9cf7765a 100644 --- a/plugins/proxy-backend/config.d.ts +++ b/plugins/proxy-backend/config.d.ts @@ -31,17 +31,17 @@ export interface Config { /** * Object with extra headers to be added to target requests. */ - headers?: Partial<{ + headers?: { /** @visibility secret */ - Authorization: string; + Authorization?: string; /** @visibility secret */ - authorization: string; + authorization?: string; /** @visibility secret */ - 'X-Api-Key': string; + 'X-Api-Key'?: string; /** @visibility secret */ - 'x-api-key': string; - [key: string]: string; - }>; + 'x-api-key'?: string; + [key: string]: string | undefined; + }; /** * Changes the origin of the host header to the target URL. Default: true. */ From 787e747d11fd95e1d2d975e6fe0371f863886d24 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 16:06:36 +0200 Subject: [PATCH 4/6] removed usage of RegExp in configuration schemas Signed-off-by: Patrik Oldsberg --- .../config.d.ts | 16 ++++++++-------- .../config.d.ts | 16 ++++++++-------- .../catalog-backend-module-gitlab/config.d.ts | 6 +++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts b/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts index 9e97597724..fa7c070042 100644 --- a/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts +++ b/plugins/catalog-backend-module-bitbucket-cloud/config.d.ts @@ -45,15 +45,15 @@ export interface Config { */ filters?: { /** - * (Optional) Filter for the repository slug. + * (Optional) Regular expression filter for the repository slug. * @visibility frontend */ - repoSlug?: RegExp; + repoSlug?: string; /** - * (Optional) Filter for the project key. + * (Optional) Regular expression filter for the project key. * @visibility frontend */ - projectKey?: RegExp; + projectKey?: string; }; /** * (Optional) TaskScheduleDefinition for the discovery. @@ -78,15 +78,15 @@ export interface Config { */ filters?: { /** - * (Optional) Filter for the repository slug. + * (Optional) Regular expression filter for the repository slug. * @visibility frontend */ - repoSlug?: RegExp; + repoSlug?: string; /** - * (Optional) Filter for the project key. + * (Optional) Regular expression filter for the project key. * @visibility frontend */ - projectKey?: RegExp; + projectKey?: string; }; /** * (Optional) TaskScheduleDefinition for the discovery. diff --git a/plugins/catalog-backend-module-bitbucket-server/config.d.ts b/plugins/catalog-backend-module-bitbucket-server/config.d.ts index 0554d62df3..57a94550e9 100644 --- a/plugins/catalog-backend-module-bitbucket-server/config.d.ts +++ b/plugins/catalog-backend-module-bitbucket-server/config.d.ts @@ -37,15 +37,15 @@ export interface Config { */ filters?: { /** - * (Optional) Filter for the repository slug. + * (Optional) Regular expression filter for the repository slug. * @visibility frontend */ - repoSlug?: RegExp; + repoSlug?: string; /** - * (Optional) Filter for the project key. + * (Optional) Regular expression filter for the project key. * @visibility frontend */ - projectKey?: RegExp; + projectKey?: string; }; /** * (Optional) TaskScheduleDefinition for the refresh. @@ -65,15 +65,15 @@ export interface Config { */ filters?: { /** - * (Optional) Filter for the repository slug. + * (Optional) Regular expression filter for the repository slug. * @visibility frontend */ - repoSlug?: RegExp; + repoSlug?: string; /** - * (Optional) Filter for the project key. + * (Optional) Regular expression filter for the project key. * @visibility frontend */ - projectKey?: RegExp; + projectKey?: string; }; /** * (Optional) TaskScheduleDefinition for the refresh. diff --git a/plugins/catalog-backend-module-gitlab/config.d.ts b/plugins/catalog-backend-module-gitlab/config.d.ts index e9732c0642..7d183b6e58 100644 --- a/plugins/catalog-backend-module-gitlab/config.d.ts +++ b/plugins/catalog-backend-module-gitlab/config.d.ts @@ -50,15 +50,15 @@ export interface Config { /** * (Optional) RegExp for the Project Name Pattern */ - projectPattern?: RegExp; + projectPattern?: string; /** * (Optional) RegExp for the User Name Pattern */ - userPattern?: RegExp; + userPattern?: string; /** * (Optional) RegExp for the Group Name Pattern */ - groupPattern?: RegExp; + groupPattern?: string; /** * (Optional) Skip forked repository */ From 4b82382ed8c24f7e7fed5c049ca140607fbdb10c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 16:08:04 +0200 Subject: [PATCH 5/6] added changeset for fixed configuration schemas Signed-off-by: Patrik Oldsberg --- .changeset/neat-coins-raise.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .changeset/neat-coins-raise.md diff --git a/.changeset/neat-coins-raise.md b/.changeset/neat-coins-raise.md new file mode 100644 index 0000000000..c3045634bf --- /dev/null +++ b/.changeset/neat-coins-raise.md @@ -0,0 +1,17 @@ +--- +'@backstage/plugin-catalog-backend-module-bitbucket-server': patch +'@backstage/plugin-catalog-backend-module-bitbucket-cloud': patch +'@backstage/plugin-catalog-backend-module-puppetdb': patch +'@backstage/plugin-catalog-backend-module-msgraph': patch +'@backstage/plugin-catalog-backend-module-gerrit': patch +'@backstage/plugin-catalog-backend-module-github': patch +'@backstage/plugin-catalog-backend-module-gitlab': patch +'@backstage/plugin-events-backend-module-aws-sqs': patch +'@backstage/plugin-catalog-backend-module-azure': patch +'@backstage/plugin-catalog-backend-module-aws': patch +'@backstage/backend-common': patch +'@backstage/plugin-jenkins-backend': patch +'@backstage/plugin-proxy-backend': patch +--- + +Fixed invalid configuration schema. The configuration schema may be more strict as a result. From 7fc676a876aca92ee9b5e082597db3294178a9dd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 28 Jul 2023 16:18:24 +0200 Subject: [PATCH 6/6] config-loader: bit more explanation around why we reject invalid schema Signed-off-by: Patrik Oldsberg --- packages/config-loader/src/schema/collect.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/config-loader/src/schema/collect.ts b/packages/config-loader/src/schema/collect.ts index 6c9737f7b7..7120c12d90 100644 --- a/packages/config-loader/src/schema/collect.ts +++ b/packages/config-loader/src/schema/collect.ts @@ -194,6 +194,12 @@ async function compileTsSchemas(paths: string[]) { [path.split(sep).join('/')], // Unix paths are expected for all OSes here ); + // All schemas should export a `Config` symbol + value = generator?.getSchemaForSymbol('Config') as JsonObject | null; + + // This makes sure that no additional symbols are defined in the schema. We don't allow + // this because they share a global namespace and will be merged together, leading to + // unpredictable behavior. const userSymbols = new Set(generator?.getUserSymbols()); userSymbols.delete('Config'); if (userSymbols.size !== 0) { @@ -203,9 +209,8 @@ async function compileTsSchemas(paths: string[]) { ); } - // All schemas should export a `Config` symbol - value = generator?.getSchemaForSymbol('Config') as JsonObject | null; - + // This makes sure that no unsupported types are used in the schema, for example `Record<,>`. + // The generator will extract these as a schema reference, which will in turn be broken for our usage. const reffedDefs = Object.keys(generator?.ReffedDefinitions ?? {}); if (reffedDefs.length !== 0) { const lines = reffedDefs.join(`${EOL} `);