From b31daba201b555b2fa522043602c8ac691266a8a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 15 Oct 2021 13:58:54 +0200 Subject: [PATCH 1/4] config-loader: collect visibility paths both by data and schema path Signed-off-by: Patrik Oldsberg --- packages/config-loader/package.json | 1 + .../src/lib/schema/compile.test.ts | 16 ++++++++++++--- .../config-loader/src/lib/schema/compile.ts | 20 ++++++++++++++----- .../config-loader/src/lib/schema/filtering.ts | 4 ++-- packages/config-loader/src/lib/schema/load.ts | 4 ++-- .../config-loader/src/lib/schema/types.ts | 9 ++++++++- 6 files changed, 41 insertions(+), 13 deletions(-) diff --git a/packages/config-loader/package.json b/packages/config-loader/package.json index d423c915d0..a719c6389b 100644 --- a/packages/config-loader/package.json +++ b/packages/config-loader/package.json @@ -38,6 +38,7 @@ "fs-extra": "9.1.0", "json-schema": "^0.3.0", "json-schema-merge-allof": "^0.8.1", + "json-schema-traverse": "^1.0.0", "typescript-json-schema": "^0.50.1", "yaml": "^1.9.2", "yup": "^0.32.9" diff --git a/packages/config-loader/src/lib/schema/compile.test.ts b/packages/config-loader/src/lib/schema/compile.test.ts index c9330a0440..e4ffc4801a 100644 --- a/packages/config-loader/src/lib/schema/compile.test.ts +++ b/packages/config-loader/src/lib/schema/compile.test.ts @@ -30,11 +30,13 @@ describe('compileConfigSchemas', () => { ]); expect(validate([{ data: { a: 1 }, context: 'test' }])).toEqual({ errors: ['Config should be string { type=string } at /a'], - visibilityByPath: new Map(), + visibilityByDataPath: new Map(), + visibilityBySchemaPath: new Map(), }); expect(validate([{ data: { b: 'b' }, context: 'test' }])).toEqual({ errors: ['Config should be number { type=number } at /b'], - visibilityByPath: new Map(), + visibilityByDataPath: new Map(), + visibilityBySchemaPath: new Map(), }); }); @@ -78,7 +80,7 @@ describe('compileConfigSchemas', () => { { data: { a: 'a', b: 'b', c: 'c', d: ['d'] }, context: 'test' }, ]), ).toEqual({ - visibilityByPath: new Map( + visibilityByDataPath: new Map( Object.entries({ '/a': 'frontend', '/b': 'secret', @@ -86,6 +88,14 @@ describe('compileConfigSchemas', () => { '/d/0': 'frontend', }), ), + visibilityBySchemaPath: new Map( + Object.entries({ + '/properties/a': 'frontend', + '/properties/b': 'secret', + '/properties/d': 'secret', + '/properties/d/items': 'frontend', + }), + ), }); }); diff --git a/packages/config-loader/src/lib/schema/compile.ts b/packages/config-loader/src/lib/schema/compile.ts index a36d2eb9a2..746a3a7a25 100644 --- a/packages/config-loader/src/lib/schema/compile.ts +++ b/packages/config-loader/src/lib/schema/compile.ts @@ -17,6 +17,7 @@ import Ajv from 'ajv'; import { JSONSchema7 as JSONSchema } from 'json-schema'; import mergeAllOf, { Resolvers } from 'json-schema-merge-allof'; +import traverse from 'json-schema-traverse'; import { ConfigReader } from '@backstage/config'; import { ConfigSchemaPackageEntry, @@ -38,7 +39,7 @@ export function compileConfigSchemas( // The ajv instance below is stateful and doesn't really allow for additional // output during validation. We work around this by having this extra piece // of state that we reset before each validation. - const visibilityByPath = new Map(); + const visibilityByDataPath = new Map(); const ajv = new Ajv({ allErrors: true, @@ -62,7 +63,7 @@ export function compileConfigSchemas( /\['?(.*?)'?\]/g, (_, segment) => `/${segment}`, ); - visibilityByPath.set(normalizedPath, visibility); + visibilityByDataPath.set(normalizedPath, visibility); } return true; }; @@ -80,10 +81,17 @@ export function compileConfigSchemas( const merged = mergeConfigSchemas(schemas.map(_ => _.value)); const validate = ajv.compile(merged); + const visibilityBySchemaPath = new Map(); + traverse(merged, (schema, path) => { + if (schema.visibility && schema.visibility !== 'backend') { + visibilityBySchemaPath.set(path, schema.visibility); + } + }); + return configs => { const config = ConfigReader.fromConfigs(configs).get(); - visibilityByPath.clear(); + visibilityByDataPath.clear(); const valid = validate(config); if (!valid) { @@ -95,12 +103,14 @@ export function compileConfigSchemas( .join(' '); return `Config ${message || ''} { ${paramStr} } at ${dataPath}`; }), - visibilityByPath: new Map(), + visibilityByDataPath: new Map(), + visibilityBySchemaPath: new Map(), }; } return { - visibilityByPath: new Map(visibilityByPath), + visibilityByDataPath: new Map(visibilityByDataPath), + visibilityBySchemaPath, }; }; } diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts index 3533be01b7..dc7fac0293 100644 --- a/packages/config-loader/src/lib/schema/filtering.ts +++ b/packages/config-loader/src/lib/schema/filtering.ts @@ -28,7 +28,7 @@ import { export function filterByVisibility( data: JsonObject, includeVisibilities: ConfigVisibility[], - visibilityByPath: Map, + visibilityByDataPath: Map, transformFunc?: TransformFunc, withFilteredKeys?: boolean, ): { data: JsonObject; filteredKeys?: string[] } { @@ -40,7 +40,7 @@ export function filterByVisibility( filterPath: string, // Matches the format of the ConfigReader ): JsonValue | undefined { const visibility = - visibilityByPath.get(visibilityPath) ?? DEFAULT_CONFIG_VISIBILITY; + visibilityByDataPath.get(visibilityPath) ?? DEFAULT_CONFIG_VISIBILITY; const isVisible = includeVisibilities.includes(visibility); if (typeof jsonVal !== 'object') { diff --git a/packages/config-loader/src/lib/schema/load.ts b/packages/config-loader/src/lib/schema/load.ts index 2d4af3454c..60402d5237 100644 --- a/packages/config-loader/src/lib/schema/load.ts +++ b/packages/config-loader/src/lib/schema/load.ts @@ -83,7 +83,7 @@ export async function loadConfigSchema( ...filterByVisibility( data, visibility, - result.visibilityByPath, + result.visibilityByDataPath, valueTransform, withFilteredKeys, ), @@ -94,7 +94,7 @@ export async function loadConfigSchema( ...filterByVisibility( data, Array.from(CONFIG_VISIBILITIES), - result.visibilityByPath, + result.visibilityByDataPath, valueTransform, withFilteredKeys, ), diff --git a/packages/config-loader/src/lib/schema/types.ts b/packages/config-loader/src/lib/schema/types.ts index ef6a197f55..6763b3e1e1 100644 --- a/packages/config-loader/src/lib/schema/types.ts +++ b/packages/config-loader/src/lib/schema/types.ts @@ -65,7 +65,14 @@ type ValidationResult = { * * The path in the key uses the form `////` */ - visibilityByPath: Map; + visibilityByDataPath: Map; + + /** + * The configuration visibilities that were discovered during validation. + * + * The path in the key uses the form `/properties//items/additionalProperties/` + */ + visibilityBySchemaPath: Map; }; /** From 8f30ac386bffb7169688928737ddac2756a44bf1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 15 Oct 2021 13:59:42 +0200 Subject: [PATCH 2/4] config-loader: filter out schema errors that aren't visible Signed-off-by: Patrik Oldsberg --- .../src/lib/schema/compile.test.ts | 20 ++- .../config-loader/src/lib/schema/compile.ts | 12 +- .../config-loader/src/lib/schema/filtering.ts | 52 +++++++ .../config-loader/src/lib/schema/load.test.ts | 142 ++++++++++++++++++ packages/config-loader/src/lib/schema/load.ts | 30 +++- .../config-loader/src/lib/schema/types.ts | 9 +- 6 files changed, 246 insertions(+), 19 deletions(-) diff --git a/packages/config-loader/src/lib/schema/compile.test.ts b/packages/config-loader/src/lib/schema/compile.test.ts index e4ffc4801a..1e63f3cbf8 100644 --- a/packages/config-loader/src/lib/schema/compile.test.ts +++ b/packages/config-loader/src/lib/schema/compile.test.ts @@ -29,12 +29,28 @@ describe('compileConfigSchemas', () => { }, ]); expect(validate([{ data: { a: 1 }, context: 'test' }])).toEqual({ - errors: ['Config should be string { type=string } at /a'], + errors: [ + { + keyword: 'type', + dataPath: '/a', + schemaPath: '#/properties/a/type', + message: 'should be string', + params: { type: 'string' }, + }, + ], visibilityByDataPath: new Map(), visibilityBySchemaPath: new Map(), }); expect(validate([{ data: { b: 'b' }, context: 'test' }])).toEqual({ - errors: ['Config should be number { type=number } at /b'], + errors: [ + { + keyword: 'type', + dataPath: '/b', + schemaPath: '#/properties/b/type', + message: 'should be number', + params: { type: 'number' }, + }, + ], 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 746a3a7a25..1cbf5a404d 100644 --- a/packages/config-loader/src/lib/schema/compile.ts +++ b/packages/config-loader/src/lib/schema/compile.ts @@ -95,16 +95,10 @@ export function compileConfigSchemas( const valid = validate(config); if (!valid) { - const errors = validate.errors ?? []; return { - errors: errors.map(({ dataPath, message, params }) => { - const paramStr = Object.entries(params) - .map(([name, value]) => `${name}=${value}`) - .join(' '); - return `Config ${message || ''} { ${paramStr} } at ${dataPath}`; - }), - visibilityByDataPath: new Map(), - visibilityBySchemaPath: new Map(), + errors: validate.errors ?? [], + visibilityByDataPath: new Map(visibilityByDataPath), + visibilityBySchemaPath, }; } diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts index dc7fac0293..7cb122ee90 100644 --- a/packages/config-loader/src/lib/schema/filtering.ts +++ b/packages/config-loader/src/lib/schema/filtering.ts @@ -19,6 +19,7 @@ import { ConfigVisibility, DEFAULT_CONFIG_VISIBILITY, TransformFunc, + ValidationError, } from './types'; /** @@ -105,3 +106,54 @@ export function filterByVisibility( data: (transform(data, '', '') as JsonObject) ?? {}, }; } + +export function filterErrorsByVisibility( + errors: ValidationError[] | undefined, + includeVisibilities: ConfigVisibility[] | undefined, + visibilityByDataPath: Map, + visibilityBySchemaPath: Map, +): ValidationError[] { + if (!errors) { + return []; + } + if (!includeVisibilities) { + return errors; + } + + const visibleSchemaPaths = Array.from(visibilityBySchemaPath) + .filter(([, v]) => includeVisibilities.includes(v)) + .map(([k]) => k); + + // If we're filtering by visibility we only care about the errors that happened + // in a visible path. + return errors.filter(error => { + // We always include structural errors as we don't know whether there are + // any visible paths within the structures. + if ( + error.keyword === 'type' && + ['object', 'array'].includes(error.params.type) + ) { + return true; + } + + // For fields that were required we use the schema path to determine whether + // it was visible in addition to the data path. This is because the data path + // visibilities are only populated for values that we reached, which we won't + // if the value is missing. + // We don't use this method for all the errors as the data path is more robust + // and doesn't require us to properly trim the schema path. + if (error.keyword === 'required') { + const trimmedPath = error.schemaPath.slice(1, -'/required'.length); + if ( + visibleSchemaPaths.some(visiblePath => + visiblePath.startsWith(trimmedPath), + ) + ) { + return true; + } + } + + const vis = visibilityByDataPath.get(error.dataPath); + return vis && includeVisibilities.includes(vis); + }); +} diff --git a/packages/config-loader/src/lib/schema/load.test.ts b/packages/config-loader/src/lib/schema/load.test.ts index 73fa55b90c..e2ae3cc2c7 100644 --- a/packages/config-loader/src/lib/schema/load.test.ts +++ b/packages/config-loader/src/lib/schema/load.test.ts @@ -102,4 +102,146 @@ describe('loadConfigSchema', () => { 'Serialized configuration schema is invalid or has an invalid version number', ); }); + + describe('should consider schema', () => { + it('when filtering simple config', async () => { + mockFs({ + 'package.json': JSON.stringify({ + name: 'a', + configSchema: { + type: 'object', + properties: { + key1: { type: 'string', visibility: 'frontend' }, + key2: { type: 'number', visibility: 'secret' }, + }, + }, + }), + }); + + const schema = await loadConfigSchema({ + packagePaths: ['package.json'], + dependencies: [], + }); + + const configs = [ + { data: { key1: 'a', key2: 'not-a-number' }, context: 'test' }, + ]; + + expect(() => schema.process(configs)).toThrow( + 'Config validation failed, Config should be number { type=number } at /key2', + ); + expect(schema.process(configs, { visibility: ['frontend'] })).toEqual([ + { data: { key1: 'a' }, context: 'test' }, + ]); + expect(() => schema.process(configs, { visibility: ['secret'] })).toThrow( + 'Config validation failed, Config should be number { type=number } at /key2', + ); + }); + + it('when filtering nested config', async () => { + mockFs({ + 'package.json': JSON.stringify({ + name: 'a', + configSchema: { + type: 'object', + properties: { + nested: { + allOf: [ + { + type: 'array', + items: { + type: 'object', + visibility: 'frontend', + properties: { + x: { type: 'number' }, + }, + additionalProperties: { + visibility: 'frontend', + type: 'string', + pattern: '^...$', + }, + }, + }, + ], + }, + }, + }, + }), + }); + + const schema = await loadConfigSchema({ + packagePaths: ['package.json'], + dependencies: [], + }); + + const mkConfig = (nested: any) => [ + { data: { nested: [nested] }, context: 'test' }, + ]; + expect( + schema.process(mkConfig({ x: 1 }), { visibility: ['frontend'] }), + ).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', + ); + expect(() => + schema.process(mkConfig({ y: 1 }), { visibility: ['frontend'] }), + ).toThrow( + 'Config validation failed, Config should be string { type=string } at /nested/0/y', + ); + expect( + schema.process(mkConfig({ x: 'a' }), { visibility: ['frontend'] }), + ).toEqual([{ data: { nested: [{}] }, context: 'test' }]); + expect( + schema.process(mkConfig({ y: 'aaa' }), { visibility: ['frontend'] }), + ).toEqual([{ data: { nested: [{ y: 'aaa' }] }, context: 'test' }]); + expect(() => + schema.process(mkConfig({ y: 'aaaa' }), { visibility: ['frontend'] }), + ).toThrow( + 'Config validation failed, Config should match pattern "^...$" { pattern=^...$ } at /nested/0/y', + ); + + // This is a bit of an edge case where we have a structural error, these should always be reported + expect(() => + schema.process([{ data: { nested: {} }, context: 'test' }], { + visibility: ['frontend'], + }), + ).toThrow( + 'Config validation failed, Config should be array { type=array } at /nested', + ); + }); + }); + + it('when filtering config with required values', async () => { + mockFs({ + 'package.json': JSON.stringify({ + name: 'a', + configSchema: { + type: 'object', + properties: { + other: { + required: ['x a'], + type: 'object', + properties: { + 'x a': { type: 'number', visibility: 'frontend' }, + }, + }, + }, + }, + }), + }); + + const schema = await loadConfigSchema({ + packagePaths: ['package.json'], + dependencies: [], + }); + + // Errors about required values should also be filtered like the rest + expect(() => + schema.process([{ data: { other: {} }, context: 'test' }], { + visibility: ['frontend'], + }), + ).toThrow( + "Config should have required property 'x a' { missingProperty=x a } at /other", + ); + }); }); diff --git a/packages/config-loader/src/lib/schema/load.ts b/packages/config-loader/src/lib/schema/load.ts index 60402d5237..0e9ddabcb3 100644 --- a/packages/config-loader/src/lib/schema/load.ts +++ b/packages/config-loader/src/lib/schema/load.ts @@ -17,8 +17,9 @@ import { AppConfig, JsonObject } from '@backstage/config'; import { compileConfigSchemas } from './compile'; import { collectConfigSchemas } from './collect'; -import { filterByVisibility } from './filtering'; +import { filterByVisibility, filterErrorsByVisibility } from './filtering'; import { + ValidationError, ConfigSchema, ConfigSchemaPackageEntry, CONFIG_VISIBILITIES, @@ -34,6 +35,18 @@ export type LoadConfigSchemaOptions = serialized: JsonObject; }; +function errorsToError(errors: ValidationError[]): Error { + const messages = errors.map(({ dataPath, message, params }) => { + const paramStr = Object.entries(params) + .map(([name, value]) => `${name}=${value}`) + .join(' '); + return `Config ${message || ''} { ${paramStr} } at ${dataPath}`; + }); + const error = new Error(`Config validation failed, ${messages.join('; ')}`); + (error as any).messages = messages; + return error; +} + /** * Loads config schema for a Backstage instance. * @@ -67,12 +80,15 @@ export async function loadConfigSchema( { visibility, valueTransform, withFilteredKeys } = {}, ): AppConfig[] { const result = validate(configs); - if (result.errors) { - const error = new Error( - `Config validation failed, ${result.errors.join('; ')}`, - ); - (error as any).messages = result.errors; - throw error; + + const visibleErrors = filterErrorsByVisibility( + result.errors, + visibility, + result.visibilityByDataPath, + result.visibilityBySchemaPath, + ); + if (visibleErrors.length > 0) { + throw errorsToError(visibleErrors); } let processedConfigs = configs; diff --git a/packages/config-loader/src/lib/schema/types.ts b/packages/config-loader/src/lib/schema/types.ts index 6763b3e1e1..d1c07f4dac 100644 --- a/packages/config-loader/src/lib/schema/types.ts +++ b/packages/config-loader/src/lib/schema/types.ts @@ -50,7 +50,14 @@ export const DEFAULT_CONFIG_VISIBILITY: ConfigVisibility = 'backend'; /** * An explanation of a configuration validation error. */ -type ValidationError = string; +export type ValidationError = { + keyword: string; + dataPath: string; + schemaPath: string; + params: Record; + propertyName?: string; + message?: string; +}; /** * The result of validating configuration data using a schema. From 3e2b3e99e4893acf309db0d949e4b97a55225857 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 15 Oct 2021 14:20:12 +0200 Subject: [PATCH 3/4] config-loader: unit tests for error visibility filtering + fixes Signed-off-by: Patrik Oldsberg --- .../src/lib/schema/filtering.test.ts | 194 +++++++++++++++++- .../config-loader/src/lib/schema/filtering.ts | 8 +- 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/packages/config-loader/src/lib/schema/filtering.test.ts b/packages/config-loader/src/lib/schema/filtering.test.ts index d4b3cd55b6..3b9e548026 100644 --- a/packages/config-loader/src/lib/schema/filtering.test.ts +++ b/packages/config-loader/src/lib/schema/filtering.test.ts @@ -16,7 +16,7 @@ import { JsonObject } from '@backstage/config'; import { ConfigVisibility } from './types'; -import { filterByVisibility } from './filtering'; +import { filterByVisibility, filterErrorsByVisibility } from './filtering'; const data = { arr: ['f', 'b', 's'], @@ -175,3 +175,195 @@ describe('filterByVisibility', () => { ).toEqual(expected); }); }); + +describe('filterErrorsByVisibility', () => { + it('should allow empty input', () => { + expect( + filterErrorsByVisibility(undefined, ['frontend'], new Map(), new Map()), + ).toEqual([]); + expect( + filterErrorsByVisibility( + ['my-error' as any], + undefined, + new Map(), + new Map(), + ), + ).toEqual(['my-error']); + expect( + filterErrorsByVisibility([], ['frontend'], new Map(), new Map()), + ).toEqual([]); + }); + + it('should filter generic errors', () => { + const errors = [ + { + keyword: 'something', + dataPath: '/a', + schemaPath: '#/properties/a/something', + params: {}, + message: 'a', + }, + { + keyword: 'something', + dataPath: '/b', + schemaPath: '#/properties/b/something', + params: {}, + message: 'b', + }, + { + keyword: 'something', + dataPath: '/c', + schemaPath: '#/properties/c/something', + params: {}, + message: 'c', + }, + ]; + const visibilityByDataPath = new Map([ + ['/a', 'frontend'], + ['/c', 'secret'], + ]); + + expect( + filterErrorsByVisibility( + errors, + undefined, + visibilityByDataPath, + new Map(), + ), + ).toEqual([ + expect.objectContaining({ message: 'a' }), + expect.objectContaining({ message: 'b' }), + expect.objectContaining({ message: 'c' }), + ]); + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'a' })]); + expect( + filterErrorsByVisibility( + errors, + ['backend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'b' })]); + expect( + filterErrorsByVisibility( + errors, + ['secret'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([expect.objectContaining({ message: 'c' })]); + expect( + filterErrorsByVisibility(errors, [], visibilityByDataPath, new Map()), + ).toEqual([]); + }); + + it('should always forward structural type errors', () => { + const errors = [ + { + keyword: 'type', + dataPath: '/a', + schemaPath: '#/properties/a/type', + params: { type: 'number' }, + message: 'a', + }, + { + keyword: 'type', + dataPath: '/b', + schemaPath: '#/properties/b/type', + params: { type: 'string' }, + message: 'b', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'array' }, + message: 'c', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'object' }, + message: 'd', + }, + { + keyword: 'type', + dataPath: '/c', + schemaPath: '#/properties/c/type', + params: { type: 'null' }, + message: 'e', + }, + ]; + const visibilityByDataPath = new Map([ + ['/a', 'secret'], + ['/b', 'secret'], + ['/c', 'secret'], + ['/d', 'secret'], + ['/e', 'secret'], + ]); + + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + visibilityByDataPath, + new Map(), + ), + ).toEqual([ + expect.objectContaining({ message: 'c' }), + expect.objectContaining({ message: 'd' }), + ]); + }); + + it('should filter requirement errors based on schema path', () => { + const errors = [ + { + keyword: 'required', + dataPath: '/a', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'a' }, + message: 'a', + }, + { + keyword: 'required', + dataPath: '/b', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'b' }, + message: 'b', + }, + { + keyword: 'required', + dataPath: '/c', + schemaPath: '#/properties/o/required', + params: { missingProperty: 'c' }, + message: 'c', + }, + ]; + const visibilityBySchemaPath = new Map([ + ['/properties/o', 'secret'], + ['/properties/o/properties/a', 'frontend'], + ['/properties/o/properties/b', 'secret'], + ['/properties/o/properties/c/properties/x', 'frontend'], + ]); + + expect( + filterErrorsByVisibility( + errors, + ['frontend'], + new Map(), + visibilityBySchemaPath, + ), + ).toEqual([ + expect.objectContaining({ message: 'a' }), + expect.objectContaining({ message: 'c' }), + ]); + }); +}); diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts index 7cb122ee90..2b13f7ac3f 100644 --- a/packages/config-loader/src/lib/schema/filtering.ts +++ b/packages/config-loader/src/lib/schema/filtering.ts @@ -144,16 +144,16 @@ export function filterErrorsByVisibility( // and doesn't require us to properly trim the schema path. if (error.keyword === 'required') { const trimmedPath = error.schemaPath.slice(1, -'/required'.length); + const fullPath = `${trimmedPath}/properties/${error.params.missingProperty}`; if ( - visibleSchemaPaths.some(visiblePath => - visiblePath.startsWith(trimmedPath), - ) + visibleSchemaPaths.some(visiblePath => visiblePath.startsWith(fullPath)) ) { return true; } } - const vis = visibilityByDataPath.get(error.dataPath); + const vis = + visibilityByDataPath.get(error.dataPath) ?? DEFAULT_CONFIG_VISIBILITY; return vis && includeVisibilities.includes(vis); }); } From 223e8de6b4813c48ef13650caf9ea22343aaf459 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 15 Oct 2021 14:27:52 +0200 Subject: [PATCH 4/4] changesets: added changeset for schema error filtering Signed-off-by: Patrik Oldsberg --- .changeset/chilled-tools-worry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/chilled-tools-worry.md diff --git a/.changeset/chilled-tools-worry.md b/.changeset/chilled-tools-worry.md new file mode 100644 index 0000000000..58f89ef503 --- /dev/null +++ b/.changeset/chilled-tools-worry.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': patch +--- + +Configuration schema errors are now filtered using the provided visibility option. This means that schema errors due to missing backend configuration will no longer break frontend builds.