From 218c91d7d8075e5d8d341b9794bec486ad63b45a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 5 Oct 2020 13:32:27 +0200 Subject: [PATCH] feat(catalog-backend): stop checking almost-similar names of entities --- .../CommonValidatorFunctions.test.ts | 14 ----- .../validation/CommonValidatorFunctions.ts | 13 ----- .../src/validation/makeValidator.ts | 1 - .../catalog-model/src/validation/types.ts | 1 - .../src/database/CommonDatabase.test.ts | 18 ------- .../src/database/CommonDatabase.ts | 52 ------------------- .../src/database/DatabaseManager.ts | 11 ++-- 7 files changed, 4 insertions(+), 106 deletions(-) diff --git a/packages/catalog-model/src/validation/CommonValidatorFunctions.test.ts b/packages/catalog-model/src/validation/CommonValidatorFunctions.test.ts index 200e90b406..457ef64611 100644 --- a/packages/catalog-model/src/validation/CommonValidatorFunctions.test.ts +++ b/packages/catalog-model/src/validation/CommonValidatorFunctions.test.ts @@ -161,18 +161,4 @@ describe('CommonValidatorFunctions', () => { ])(`isValidDnsLabel %p ? %p`, (value, result) => { expect(CommonValidatorFunctions.isValidDnsLabel(value)).toBe(result); }); - - it.each([ - ['', ''], - ['a', 'a'], - ['a-b', 'ab'], - ['-a-b', 'ab'], - ['a_b', 'ab'], - [`${'a'.repeat(6000)}`, `${'a'.repeat(6000)}`], - ['_:;>!"#€', ''], - ])(`normalizeToLowercaseAlphanum %p ? %p`, (value, result) => { - expect(CommonValidatorFunctions.normalizeToLowercaseAlphanum(value)).toBe( - result, - ); - }); }); diff --git a/packages/catalog-model/src/validation/CommonValidatorFunctions.ts b/packages/catalog-model/src/validation/CommonValidatorFunctions.ts index 96a91aca06..349d6d7f72 100644 --- a/packages/catalog-model/src/validation/CommonValidatorFunctions.ts +++ b/packages/catalog-model/src/validation/CommonValidatorFunctions.ts @@ -92,17 +92,4 @@ export class CommonValidatorFunctions { /^[a-z0-9]+(\-[a-z0-9]+)*$/.test(value) ); } - - /** - * Normalizes by keeping only a-z, A-Z, and 0-9; and converts to lowercase. - * - * @param value The value to normalize - */ - static normalizeToLowercaseAlphanum(value: string): string { - return value - .split('') - .filter(x => /[a-zA-Z0-9]/.test(x)) - .join('') - .toLowerCase(); - } } diff --git a/packages/catalog-model/src/validation/makeValidator.ts b/packages/catalog-model/src/validation/makeValidator.ts index 3602c01a63..63341f5c33 100644 --- a/packages/catalog-model/src/validation/makeValidator.ts +++ b/packages/catalog-model/src/validation/makeValidator.ts @@ -23,7 +23,6 @@ const defaultValidators: Validators = { isValidKind: KubernetesValidatorFunctions.isValidKind, isValidEntityName: KubernetesValidatorFunctions.isValidObjectName, isValidNamespace: KubernetesValidatorFunctions.isValidNamespace, - normalizeEntityName: CommonValidatorFunctions.normalizeToLowercaseAlphanum, isValidLabelKey: KubernetesValidatorFunctions.isValidLabelKey, isValidLabelValue: KubernetesValidatorFunctions.isValidLabelValue, isValidAnnotationKey: KubernetesValidatorFunctions.isValidAnnotationKey, diff --git a/packages/catalog-model/src/validation/types.ts b/packages/catalog-model/src/validation/types.ts index ff00036991..14706485ca 100644 --- a/packages/catalog-model/src/validation/types.ts +++ b/packages/catalog-model/src/validation/types.ts @@ -19,7 +19,6 @@ export type Validators = { isValidKind(value: any): boolean; isValidEntityName(value: any): boolean; isValidNamespace(value: any): boolean; - normalizeEntityName(value: string): string; isValidLabelKey(value: any): boolean; isValidLabelValue(value: any): boolean; isValidAnnotationKey(value: any): boolean; diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index 8b8067f641..8a42b39cc5 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -149,24 +149,6 @@ describe('CommonDatabase', () => { ).rejects.toThrow(ConflictError); }); - it('rejects adding the almost-same-kind entity twice', async () => { - entityRequest.entity.kind = 'some-kind'; - await db.transaction(tx => db.addEntity(tx, entityRequest)); - entityRequest.entity.kind = 'SomeKind'; - await expect( - db.transaction(tx => db.addEntity(tx, entityRequest)), - ).rejects.toThrow(ConflictError); - }); - - it('rejects adding the almost-same-named entity twice', async () => { - entityRequest.entity.metadata.name = 'some-name'; - await db.transaction(tx => db.addEntity(tx, entityRequest)); - entityRequest.entity.metadata.name = 'SomeName'; - await expect( - db.transaction(tx => db.addEntity(tx, entityRequest)), - ).rejects.toThrow(ConflictError); - }); - it('rejects adding the almost-same-namespace entity twice', async () => { entityRequest.entity.metadata.namespace = undefined; await db.transaction(tx => db.addEntity(tx, entityRequest)); diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts index 9ac1fff951..c40cefb625 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -27,7 +27,6 @@ import { ENTITY_META_GENERATED_FIELDS, generateEntityEtag, generateEntityUid, - getEntityName, Location, } from '@backstage/catalog-model'; import Knex from 'knex'; @@ -53,7 +52,6 @@ import type { export class CommonDatabase implements Database { constructor( private readonly database: Knex, - private readonly normalize: (value: string) => string, private readonly logger: Logger, ) {} @@ -88,8 +86,6 @@ export class CommonDatabase implements Database { throw new InputError('May not specify generation for new entities'); } - await this.ensureNoSimilarNames(tx, request.entity); - const newEntity = lodash.cloneDeep(request.entity); newEntity.metadata = { ...newEntity.metadata, @@ -147,8 +143,6 @@ export class CommonDatabase implements Database { } } - await this.ensureNoSimilarNames(tx, request.entity); - // Store the updated entity; select on the old etag to ensure that we do // not lose to another writer const newRow = this.toEntityRow(request.locationId, request.entity); @@ -385,52 +379,6 @@ export class CommonDatabase implements Database { } } - private async ensureNoSimilarNames( - tx: Knex.Transaction, - data: Entity, - ): Promise { - const { - kind: newKind, - namespace: newNamespace, - name: newName, - } = getEntityName(data); - const newKindNorm = this.normalize(newKind); - const newNamespaceNorm = this.normalize(newNamespace); - const newNameNorm = this.normalize(newName); - - for (const item of await this.entities(tx)) { - if (data.metadata.uid === item.entity.metadata.uid) { - continue; - } - - const { - kind: oldKind, - namespace: oldNamespace, - name: oldName, - } = getEntityName(item.entity); - const oldKindNorm = this.normalize(oldKind); - const oldNamespaceNorm = this.normalize(oldNamespace); - const oldNameNorm = this.normalize(oldName); - - if ( - oldKindNorm === newKindNorm && - oldNamespaceNorm === newNamespaceNorm && - oldNameNorm === newNameNorm - ) { - // Only throw if things were actually different - for completely equal - // things, we let the database handle the conflict - if ( - oldKind !== newKind || - oldNamespace !== newNamespace || - oldName !== newName - ) { - const message = `Kind, namespace, name are too similar to an existing entity`; - throw new ConflictError(message); - } - } - } - } - private toEntityRow( locationId: string | undefined, entity: Entity, diff --git a/plugins/catalog-backend/src/database/DatabaseManager.ts b/plugins/catalog-backend/src/database/DatabaseManager.ts index ce91cc737f..afff254097 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.ts @@ -15,7 +15,6 @@ */ import { getVoidLogger, resolvePackagePath } from '@backstage/backend-common'; -import { makeValidator } from '@backstage/catalog-model'; import Knex from 'knex'; import { Logger } from 'winston'; import { CommonDatabase } from './CommonDatabase'; @@ -28,12 +27,10 @@ const migrationsDir = resolvePackagePath( export type CreateDatabaseOptions = { logger: Logger; - fieldNormalizer: (value: string) => string; }; const defaultOptions: CreateDatabaseOptions = { logger: getVoidLogger(), - fieldNormalizer: makeValidator().normalizeEntityName, }; export class DatabaseManager { @@ -44,8 +41,8 @@ export class DatabaseManager { await knex.migrate.latest({ directory: migrationsDir, }); - const { logger, fieldNormalizer } = { ...defaultOptions, ...options }; - return new CommonDatabase(knex, fieldNormalizer, logger); + const { logger } = { ...defaultOptions, ...options }; + return new CommonDatabase(knex, logger); } public static async createInMemoryDatabase( @@ -74,7 +71,7 @@ export class DatabaseManager { await knex.migrate.latest({ directory: migrationsDir, }); - const { logger, fieldNormalizer } = defaultOptions; - return new CommonDatabase(knex, fieldNormalizer, logger); + const { logger } = defaultOptions; + return new CommonDatabase(knex, logger); } }