feat(catalog-backend): stop checking almost-similar names of entities
This commit is contained in:
@@ -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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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<any, any>,
|
||||
data: Entity,
|
||||
): Promise<void> {
|
||||
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,
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user