From f25427f665f7e355a0167d691bc279df4cf8cfcf Mon Sep 17 00:00:00 2001 From: Aramis Date: Sun, 21 May 2023 19:22:12 -0400 Subject: [PATCH] feat(config-loader): Fix handling of keys with backslashes. Signed-off-by: Aramis --- .changeset/khaki-radios-teach.md | 5 ++ .../config-loader/src/schema/compile.test.ts | 60 +++++++++++++++++++ packages/config-loader/src/schema/compile.ts | 13 ++-- .../config-loader/src/schema/filtering.ts | 9 ++- packages/config-loader/src/schema/load.ts | 5 +- packages/config-loader/src/schema/utils.ts | 29 +++++++++ 6 files changed, 109 insertions(+), 12 deletions(-) create mode 100644 .changeset/khaki-radios-teach.md create mode 100644 packages/config-loader/src/schema/utils.ts diff --git a/.changeset/khaki-radios-teach.md b/.changeset/khaki-radios-teach.md new file mode 100644 index 0000000000..05ede0593c --- /dev/null +++ b/.changeset/khaki-radios-teach.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': patch +--- + +Fix a bug where config items with `/` in the key were incorrectly handled. diff --git a/packages/config-loader/src/schema/compile.test.ts b/packages/config-loader/src/schema/compile.test.ts index c2541751e7..df81923684 100644 --- a/packages/config-loader/src/schema/compile.test.ts +++ b/packages/config-loader/src/schema/compile.test.ts @@ -170,4 +170,64 @@ describe('compileConfigSchemas', () => { visibilityBySchemaPath: new Map(), }); }); + + it('should handle slashes correctly', () => { + const validate = compileConfigSchemas([ + { + path: 'a1', + value: { + type: 'object', + properties: { + '/circleci/api': { + type: 'object', + properties: { + target: { + type: 'string', + }, + headers: { + type: 'object', + properties: { + 'Circle-Token': { + type: 'string', + visibility: 'secret', + }, + }, + }, + }, + }, + '/gocd': { type: 'string', visibility: 'backend' }, + }, + }, + }, + ]); + expect( + validate([ + { + data: { + '/circleci/api': { + target: 'test', + headers: { + 'Circle-Token': 'my-token', + }, + }, + '/gocd': 'test', + }, + context: 'test', + }, + ]), + ).toEqual({ + visibilityByDataPath: new Map( + Object.entries({ + '//circleci/api/headers/Circle-Token': 'secret', + }), + ), + visibilityBySchemaPath: new Map( + Object.entries({ + '/properties//circleci/api/properties/headers/properties/Circle-Token': + 'secret', + }), + ), + deprecationByDataPath: new Map(), + }); + }); }); diff --git a/packages/config-loader/src/schema/compile.ts b/packages/config-loader/src/schema/compile.ts index f9e1a3bf76..63a8e4b8a6 100644 --- a/packages/config-loader/src/schema/compile.ts +++ b/packages/config-loader/src/schema/compile.ts @@ -26,6 +26,7 @@ import { ConfigVisibility, } from './types'; import { SchemaObject } from 'json-schema-traverse'; +import { normalizeAjvPath } from './utils'; /** * This takes a collection of Backstage configuration schemas from various @@ -66,10 +67,7 @@ export function compileConfigSchemas( return false; } if (visibility && visibility !== 'backend') { - const normalizedPath = context.instancePath.replace( - /\['?(.*?)'?\]/g, - (_, segment) => `/${segment}`, - ); + const normalizedPath = normalizeAjvPath(context.instancePath); visibilityByDataPath.set(normalizedPath, visibility); } return true; @@ -85,10 +83,7 @@ export function compileConfigSchemas( if (context?.instancePath === undefined) { return false; } - const normalizedPath = context.instancePath.replace( - /\['?(.*?)'?\]/g, - (_, segment) => `/${segment}`, - ); + const normalizedPath = normalizeAjvPath(context.instancePath); // create mapping of deprecation description and data path of property deprecationByDataPath.set(normalizedPath, deprecationDescription); return true; @@ -123,7 +118,7 @@ export function compileConfigSchemas( const visibilityBySchemaPath = new Map(); traverse(merged, (schema, path) => { if (schema.visibility && schema.visibility !== 'backend') { - visibilityBySchemaPath.set(path, schema.visibility); + visibilityBySchemaPath.set(normalizeAjvPath(path), schema.visibility); } }); diff --git a/packages/config-loader/src/schema/filtering.ts b/packages/config-loader/src/schema/filtering.ts index e9a0327e06..1c2d0c122a 100644 --- a/packages/config-loader/src/schema/filtering.ts +++ b/packages/config-loader/src/schema/filtering.ts @@ -21,6 +21,7 @@ import { TransformFunc, ValidationError, } from './types'; +import { normalizeAjvPath } from './utils'; /** * This filters data by visibility by discovering the visibility of each @@ -163,7 +164,10 @@ export function filterErrorsByVisibility( // 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); + const trimmedPath = normalizeAjvPath(error.schemaPath).slice( + 1, + -'/required'.length, + ); const fullPath = `${trimmedPath}/properties/${error.params.missingProperty}`; if ( visibleSchemaPaths.some(visiblePath => visiblePath.startsWith(fullPath)) @@ -173,7 +177,8 @@ export function filterErrorsByVisibility( } const vis = - visibilityByDataPath.get(error.instancePath) ?? DEFAULT_CONFIG_VISIBILITY; + visibilityByDataPath.get(normalizeAjvPath(error.instancePath)) ?? + DEFAULT_CONFIG_VISIBILITY; return vis && includeVisibilities.includes(vis); }); } diff --git a/packages/config-loader/src/schema/load.ts b/packages/config-loader/src/schema/load.ts index 1a96305ae0..8ef971184f 100644 --- a/packages/config-loader/src/schema/load.ts +++ b/packages/config-loader/src/schema/load.ts @@ -25,6 +25,7 @@ import { ConfigSchemaPackageEntry, CONFIG_VISIBILITIES, } from './types'; +import { normalizeAjvPath } from './utils'; /** * Options that control the loading of configuration schema files in the backend. @@ -49,7 +50,9 @@ function errorsToError(errors: ValidationError[]): Error { const paramStr = Object.entries(params) .map(([name, value]) => `${name}=${value}`) .join(' '); - return `Config ${message || ''} { ${paramStr} } at ${instancePath}`; + return `Config ${message || ''} { ${paramStr} } at ${normalizeAjvPath( + instancePath, + )}`; }); const error = new Error(`Config validation failed, ${messages.join('; ')}`); (error as any).messages = messages; diff --git a/packages/config-loader/src/schema/utils.ts b/packages/config-loader/src/schema/utils.ts new file mode 100644 index 0000000000..25525a8c50 --- /dev/null +++ b/packages/config-loader/src/schema/utils.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * AJV does some encoding to schema and instance paths. Revert those encodings to + * easier to read and parse values. + * See https://github.com/ajv-validator/ajv/blob/master/lib/compile/util.ts#L69. + * + * @param path Path from AJV. + * @returns Updated path with correct characters. + */ +export function normalizeAjvPath(path: string) { + return path + .replace(/~1/g, '/') + .replace(/\['?(.*?)'?\]/g, (_, segment) => `/${segment}`); +}