Merge pull request #3013 from SDA-SE/dry-run-experiment

Allow to dry run the register component operation
This commit is contained in:
Oliver Sand
2020-10-30 16:50:08 +01:00
committed by GitHub
29 changed files with 637 additions and 254 deletions
+9
View File
@@ -0,0 +1,9 @@
---
'@backstage/plugin-catalog-backend': minor
---
Add ability to dry run adding a new location to the catalog API.
The location is now added in a transaction and afterwards rolled back.
This allows users to dry run this operation to see if there entity has issues.
This is probably done by automated tools in the CI/CD pipeline.
+8
View File
@@ -0,0 +1,8 @@
---
'@backstage/plugin-catalog': minor
'@backstage/plugin-register-component': minor
---
Add a validate button to the register-component page
This allows the user to validate his location before adding it.
@@ -122,6 +122,7 @@ export default async function createPlugin({
entitiesCatalog,
locationsCatalog,
locationReader,
db,
logger,
);
+1 -1
View File
@@ -43,7 +43,7 @@
"express-promise-router": "^3.0.3",
"git-url-parse": "^11.4.0",
"helmet": "^4.0.0",
"knex": "^0.21.1",
"knex": "^0.21.6",
"lodash": "^4.17.15",
"logform": "^2.1.1",
"minimist": "^1.2.5",
+1 -1
View File
@@ -38,7 +38,7 @@
"example-app": "^0.1.1-alpha.26",
"express": "^4.17.1",
"express-promise-router": "^3.0.3",
"knex": "^0.21.1",
"knex": "^0.21.6",
"pg": "^8.3.0",
"pg-connection-string": "^2.3.0",
"sqlite3": "^5.0.0",
@@ -31,7 +31,7 @@
"dockerode": "^3.2.0",
"express": "^4.17.1",
"express-promise-router": "^3.0.3",
"knex": "^0.21.1",
"knex": "^0.21.6",
{{#if dbTypePG}}
"pg": "^8.3.0",
{{/if}}
+1 -1
View File
@@ -35,7 +35,7 @@
"helmet": "^4.0.0",
"jose": "^1.27.1",
"jwt-decode": "2.2.0",
"knex": "^0.21.1",
"knex": "^0.21.6",
"moment": "^2.26.0",
"morgan": "^1.10.0",
"passport": "^0.4.1",
+1 -1
View File
@@ -32,7 +32,7 @@
"express-promise-router": "^3.0.3",
"fs-extra": "^9.0.0",
"git-url-parse": "^11.4.0",
"knex": "^0.21.1",
"knex": "^0.21.6",
"ldapjs": "^2.2.0",
"lodash": "^4.17.15",
"morgan": "^1.10.0",
@@ -16,12 +16,13 @@
import { getVoidLogger } from '@backstage/backend-common';
import type { Entity } from '@backstage/catalog-model';
import { Database, DatabaseManager } from '../database';
import { Database, DatabaseManager, Transaction } from '../database';
import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog';
import { EntityUpsertRequest } from './types';
describe('DatabaseEntitiesCatalog', () => {
let db: jest.Mocked<Database>;
let transaction: jest.Mocked<Transaction>;
beforeAll(() => {
db = {
@@ -40,11 +41,14 @@ describe('DatabaseEntitiesCatalog', () => {
locationHistory: jest.fn(),
addLocationUpdateLogEvent: jest.fn(),
};
transaction = {
rollback: jest.fn(),
};
});
beforeEach(() => {
jest.resetAllMocks();
db.transaction.mockImplementation(async f => f('tx'));
db.transaction.mockImplementation(async f => f(transaction));
});
describe('batchAddOrUpdateEntities', () => {
@@ -82,6 +86,84 @@ describe('DatabaseEntitiesCatalog', () => {
expect(result).toEqual([{ entityId: 'u' }]);
});
it('dry run of add operation', async () => {
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: {
name: 'c',
namespace: 'd',
},
};
db.entities.mockResolvedValue([]);
db.addEntities.mockResolvedValue([
{ entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } },
]);
const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger());
const result = await catalog.batchAddOrUpdateEntities(
[{ entity, relations: [] }],
{ dryRun: true },
);
expect(db.entities).toHaveBeenCalledTimes(1);
expect(db.entities).toHaveBeenCalledWith(expect.anything(), [
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': ['c'],
},
]);
expect(db.setRelations).toHaveBeenCalledTimes(1);
expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []);
expect(db.addEntities).toHaveBeenCalledTimes(1);
expect(transaction.rollback).toBeCalledTimes(1);
expect(result).toEqual([{ entityId: 'u' }]);
});
it('output modified entities', async () => {
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: {
name: 'c',
namespace: 'd',
},
};
const dbEntity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: {
name: 'c',
namespace: 'd',
uid: 'u',
},
};
db.entities.mockResolvedValue([
{
entity: dbEntity,
},
]);
db.addEntities.mockResolvedValue([
{ entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } },
]);
const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger());
const result = await catalog.batchAddOrUpdateEntities(
[{ entity, relations: [] }],
{ outputEntities: true },
);
expect(db.entities).toHaveBeenCalledTimes(2);
expect(db.setRelations).toHaveBeenCalledTimes(1);
expect(result).toEqual([
{
entityId: 'u',
entity: dbEntity,
},
]);
});
it('updates when given uid', async () => {
const entity: Entity = {
apiVersion: 'a',
@@ -131,10 +213,10 @@ describe('DatabaseEntitiesCatalog', () => {
]);
expect(db.entityByName).not.toHaveBeenCalled();
expect(db.entityByUid).toHaveBeenCalledTimes(1);
expect(db.entityByUid).toHaveBeenCalledWith(expect.anything(), 'u');
expect(db.entityByUid).toHaveBeenCalledWith(transaction, 'u');
expect(db.updateEntity).toHaveBeenCalledTimes(1);
expect(db.updateEntity).toHaveBeenCalledWith(
expect.anything(),
transaction,
{
entity: {
apiVersion: 'a',
@@ -206,14 +288,14 @@ describe('DatabaseEntitiesCatalog', () => {
},
]);
expect(db.entityByName).toHaveBeenCalledTimes(1);
expect(db.entityByName).toHaveBeenCalledWith(expect.anything(), {
expect(db.entityByName).toHaveBeenCalledWith(transaction, {
kind: 'b',
namespace: 'd',
name: 'c',
});
expect(db.updateEntity).toHaveBeenCalledTimes(1);
expect(db.updateEntity).toHaveBeenCalledWith(
expect.anything(),
transaction,
{
entity: {
apiVersion: 'a',
@@ -27,7 +27,12 @@ import {
import { chunk, groupBy } from 'lodash';
import limiterFactory from 'p-limit';
import { Logger } from 'winston';
import type { Database, DbEntityResponse, EntityFilters } from '../database';
import type {
Database,
DbEntityResponse,
EntityFilters,
Transaction,
} from '../database';
import { durationText } from '../util/timing';
import type {
EntitiesCatalog,
@@ -69,35 +74,34 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async addOrUpdateEntity(
entity: Entity,
tx: Transaction,
locationId?: string,
): Promise<Entity> {
return await this.database.transaction(async tx => {
// Find a matching (by uid, or by compound name, depending on the given
// entity) existing entity, to know whether to update or add
const existing = entity.metadata.uid
? await this.database.entityByUid(tx, entity.metadata.uid)
: await this.database.entityByName(tx, getEntityName(entity));
// Find a matching (by uid, or by compound name, depending on the given
// entity) existing entity, to know whether to update or add
const existing = entity.metadata.uid
? await this.database.entityByUid(tx, entity.metadata.uid)
: await this.database.entityByName(tx, getEntityName(entity));
// If it's an update, run the algorithm for annotation merging, updating
// etag/generation, etc.
let response: DbEntityResponse;
if (existing) {
const updated = generateUpdatedEntity(existing.entity, entity);
response = await this.database.updateEntity(
tx,
{ locationId, entity: updated },
existing.entity.metadata.etag,
existing.entity.metadata.generation,
);
} else {
const added = await this.database.addEntities(tx, [
{ locationId, entity },
]);
response = added[0];
}
// If it's an update, run the algorithm for annotation merging, updating
// etag/generation, etc.
let response: DbEntityResponse;
if (existing) {
const updated = generateUpdatedEntity(existing.entity, entity);
response = await this.database.updateEntity(
tx,
{ locationId, entity: updated },
existing.entity.metadata.etag,
existing.entity.metadata.generation,
);
} else {
const added = await this.database.addEntities(tx, [
{ locationId, entity },
]);
response = added[0];
}
return response.entity;
});
return response.entity;
}
async removeEntityByUid(uid: string): Promise<void> {
@@ -131,92 +135,126 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
* Writes a number of entities efficiently to storage.
*
* @param entities Some entities
* @param locationId The location that they all belong to
* @param options.locationId The location that they all belong to
* @param options.tx A database transaction to execute the queries in
*/
async batchAddOrUpdateEntities(
requests: EntityUpsertRequest[],
locationId?: string,
options?: {
locationId?: string;
dryRun?: boolean;
outputEntities?: boolean;
},
): Promise<EntityUpsertResponse[]> {
// Group the entities by unique kind+namespace combinations
const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => {
const name = getEntityName(entity);
return `${name.kind}:${name.namespace}`.toLowerCase();
});
const locationId = options?.locationId;
const limiter = limiterFactory(BATCH_CONCURRENCY);
const tasks: Promise<EntityUpsertResponse[]>[] = [];
return await this.database.transaction(async tx => {
// Group the entities by unique kind+namespace combinations
const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => {
const name = getEntityName(entity);
return `${name.kind}:${name.namespace}`.toLowerCase();
});
for (const groupRequests of Object.values(entitiesByKindAndNamespace)) {
const { kind, namespace } = getEntityName(groupRequests[0].entity);
const limiter = limiterFactory(BATCH_CONCURRENCY);
const tasks: Promise<EntityUpsertResponse[]>[] = [];
// Go through the new entities in reasonable chunk sizes (sometimes,
// sources produce tens of thousands of entities, and those are too large
// batch sizes to reasonably send to the database)
for (const batch of chunk(groupRequests, BATCH_SIZE)) {
tasks.push(
limiter(async () => {
const first = serializeEntityRef(batch[0].entity);
const last = serializeEntityRef(batch[batch.length - 1].entity);
const modifiedEntityIds: EntityUpsertResponse[] = [];
this.logger.debug(
`Considering batch ${first}-${last} (${batch.length} entries)`,
);
for (const groupRequests of Object.values(entitiesByKindAndNamespace)) {
const { kind, namespace } = getEntityName(groupRequests[0].entity);
// Retry the batch write a few times to deal with contention
const context = { kind, namespace, locationId };
for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) {
try {
const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch(
batch,
context,
);
if (toAdd.length) {
modifiedEntityIds.push(
...(await this.batchAdd(toAdd, context)),
// Go through the new entities in reasonable chunk sizes (sometimes,
// sources produce tens of thousands of entities, and those are too large
// batch sizes to reasonably send to the database)
for (const batch of chunk(groupRequests, BATCH_SIZE)) {
tasks.push(
limiter(async () => {
const first = serializeEntityRef(batch[0].entity);
const last = serializeEntityRef(batch[batch.length - 1].entity);
let modifiedEntityIds: EntityUpsertResponse[] = [];
this.logger.debug(
`Considering batch ${first}-${last} (${batch.length} entries)`,
);
// Retry the batch write a few times to deal with contention
const context = {
kind,
namespace,
locationId,
};
for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) {
try {
const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch(
batch,
context,
tx,
);
}
if (toUpdate.length) {
modifiedEntityIds.push(
...(await this.batchUpdate(toUpdate, context)),
);
}
// TODO(Rugvip): We currently always update relations, but we
// likely want to figure out a way to avoid that
for (const { entity, relations } of toIgnore) {
const entityId = entity.metadata.uid!;
await this.setRelations(entityId, relations);
modifiedEntityIds.push({ entityId });
}
if (toAdd.length) {
modifiedEntityIds.push(
...(await this.batchAdd(toAdd, context, tx)),
);
}
if (toUpdate.length) {
modifiedEntityIds.push(
...(await this.batchUpdate(toUpdate, context, tx)),
);
}
// TODO(Rugvip): We currently always update relations, but we
// likely want to figure out a way to avoid that
for (const { entity, relations } of toIgnore) {
const entityId = entity.metadata.uid!;
await this.setRelations(entityId, relations, tx);
modifiedEntityIds.push({ entityId });
}
break;
} catch (e) {
if (e instanceof ConflictError && attempt < BATCH_ATTEMPTS) {
this.logger.warn(
`Failed to write batch at attempt ${attempt}/${BATCH_ATTEMPTS}, ${e}`,
);
} else {
throw e;
break;
} catch (e) {
if (e instanceof ConflictError && attempt < BATCH_ATTEMPTS) {
this.logger.warn(
`Failed to write batch at attempt ${attempt}/${BATCH_ATTEMPTS}, ${e}`,
);
} else {
throw e;
}
}
}
}
return modifiedEntityIds;
}),
);
if (options?.outputEntities) {
const writtenEntities = await this.database.entities(tx, [
{ 'metadata.uid': modifiedEntityIds.map(e => e.entityId) },
]);
modifiedEntityIds = writtenEntities.map(e => ({
entityId: e.entity.metadata.uid!,
entity: e.entity,
}));
}
return modifiedEntityIds;
}),
);
}
}
}
return (await Promise.all(tasks)).flat();
const entityUpserts = (await Promise.all(tasks)).flat();
if (options?.dryRun) {
// If this is only a dry run, cancel the database transaction even if it was successful.
await tx.rollback();
this.logger.debug(`Perfomed successful dry run of adding entities`);
}
return entityUpserts;
});
}
// Set the relations originating from an entity using the DB layer
private async setRelations(
originatingEntityId: string,
relations: EntityRelationSpec[],
tx: Transaction,
): Promise<void> {
return await this.database.transaction(tx =>
this.database.setRelations(tx, originatingEntityId, relations),
);
await this.database.setRelations(tx, originatingEntityId, relations);
}
// Given a batch of entities that were just read from a location, take them
@@ -226,6 +264,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async analyzeBatch(
requests: EntityUpsertRequest[],
{ kind, namespace }: BatchContext,
tx: Transaction,
): Promise<{
toAdd: EntityUpsertRequest[];
toUpdate: EntityUpsertRequest[];
@@ -234,7 +273,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
const markTimestamp = process.hrtime();
const names = requests.map(({ entity }) => entity.metadata.name);
const oldEntities = await this.entities([
const oldEntities = await this.database.entities(tx, [
{
kind: kind,
'metadata.namespace': namespace,
@@ -243,7 +282,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
]);
const oldEntitiesByName = new Map(
oldEntities.map(e => [e.metadata.name, e]),
oldEntities.map(e => [e.entity.metadata.name, e.entity]),
);
const toAdd: EntityUpsertRequest[] = [];
@@ -279,15 +318,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async batchAdd(
requests: EntityUpsertRequest[],
{ locationId }: BatchContext,
tx: Transaction,
): Promise<EntityUpsertResponse[]> {
const markTimestamp = process.hrtime();
const res = await this.database.transaction(
async tx =>
await this.database.addEntities(
tx,
requests.map(({ entity }) => ({ locationId, entity })),
),
const res = await this.database.addEntities(
tx,
requests.map(({ entity }) => ({ locationId, entity })),
);
const entityIds = res.map(({ entity }) => ({
@@ -295,7 +332,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
}));
for (const [index, { entityId }] of entityIds.entries()) {
await this.setRelations(entityId, requests[index].relations);
await this.setRelations(entityId, requests[index].relations, tx);
}
this.logger.debug(
@@ -310,15 +347,16 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private async batchUpdate(
requests: EntityUpsertRequest[],
{ locationId }: BatchContext,
tx: Transaction,
): Promise<EntityUpsertResponse[]> {
const markTimestamp = process.hrtime();
const responseIds: EntityUpsertResponse[] = [];
// TODO(freben): Still not batched
for (const entity of requests) {
const res = await this.addOrUpdateEntity(entity.entity, locationId);
const res = await this.addOrUpdateEntity(entity.entity, tx, locationId);
const entityId = res.metadata.uid!;
responseIds.push({ entityId });
await this.setRelations(entityId, entity.relations);
await this.setRelations(entityId, entity.relations, tx);
}
this.logger.debug(
@@ -26,8 +26,9 @@ export class DatabaseLocationsCatalog implements LocationsCatalog {
constructor(private readonly database: Database) {}
async addLocation(location: Location): Promise<Location> {
const added = await this.database.addLocation(location);
return added;
return await this.database.transaction(
async tx => await this.database.addLocation(tx, location),
);
}
async removeLocation(id: string): Promise<void> {
+6 -1
View File
@@ -28,6 +28,7 @@ export type EntityUpsertRequest = {
export type EntityUpsertResponse = {
entityId: string;
entity?: Entity;
};
export type EntitiesCatalog = {
@@ -42,7 +43,11 @@ export type EntitiesCatalog = {
*/
batchAddOrUpdateEntities(
entities: EntityUpsertRequest[],
locationId?: string,
options?: {
locationId?: string;
dryRun?: boolean;
outputEntities?: boolean;
},
): Promise<EntityUpsertResponse[]>;
};
@@ -91,7 +91,7 @@ describe('CommonDatabase', () => {
timestamp: null,
};
await db.addLocation(input);
await db.transaction(async tx => await db.addLocation(tx, input));
const locations = await db.locations();
expect(locations).toEqual(
@@ -238,7 +238,8 @@ describe('CommonDatabase', () => {
type: 'a',
target: 'b',
};
await db.addLocation(location);
await db.transaction(async tx => await db.addLocation(tx, location));
await db.addLocationUpdateLogEvent(
'dd12620d-0436-422f-93bd-929aa0788123',
@@ -46,6 +46,7 @@ import {
DbLocationsRow,
DbLocationsRowWithStatus,
EntityFilters,
Transaction,
} from './types';
// The number of items that are sent per batch to the database layer, when
@@ -63,9 +64,23 @@ export class CommonDatabase implements Database {
private readonly logger: Logger,
) {}
async transaction<T>(fn: (tx: unknown) => Promise<T>): Promise<T> {
async transaction<T>(fn: (tx: Transaction) => Promise<T>): Promise<T> {
try {
return await this.database.transaction<T>(fn);
let result: T | undefined = undefined;
await this.database.transaction(
async tx => {
// We can't return here, as knex swallows the return type in case the transaction is rolled back:
// https://github.com/knex/knex/blob/e37aeaa31c8ef9c1b07d2e4d3ec6607e557d800d/lib/transaction.js#L136
result = await fn(tx);
},
{
// If we explicitly trigger a rollback, don't fail.
doNotRejectOnRollback: true,
},
);
return result!;
} catch (e) {
this.logger.debug(`Error during transaction, ${e}`);
@@ -81,7 +96,7 @@ export class CommonDatabase implements Database {
}
async addEntity(
txOpaque: unknown,
txOpaque: Transaction,
request: DbEntityRequest,
): Promise<DbEntityResponse> {
const tx = txOpaque as Knex.Transaction<any, any>;
@@ -110,7 +125,7 @@ export class CommonDatabase implements Database {
}
async addEntities(
txOpaque: unknown,
txOpaque: Transaction,
request: DbEntityRequest[],
): Promise<DbEntityResponse[]> {
const tx = txOpaque as Knex.Transaction<any, any>;
@@ -158,7 +173,7 @@ export class CommonDatabase implements Database {
}
async updateEntity(
txOpaque: unknown,
txOpaque: Transaction,
request: DbEntityRequest,
matchingEtag?: string,
matchingGeneration?: number,
@@ -217,7 +232,7 @@ export class CommonDatabase implements Database {
}
async entities(
txOpaque: unknown,
txOpaque: Transaction,
filters?: EntityFilters[],
): Promise<DbEntityResponse[]> {
const tx = txOpaque as Knex.Transaction<any, any>;
@@ -295,7 +310,7 @@ export class CommonDatabase implements Database {
}
async entityByName(
txOpaque: unknown,
txOpaque: Transaction,
name: EntityName,
): Promise<DbEntityResponse | undefined> {
const tx = txOpaque as Knex.Transaction<any, any>;
@@ -314,7 +329,7 @@ export class CommonDatabase implements Database {
}
async entityByUid(
txOpaque: unknown,
txOpaque: Transaction,
uid: string,
): Promise<DbEntityResponse | undefined> {
const tx = txOpaque as Knex.Transaction<any, any>;
@@ -330,7 +345,7 @@ export class CommonDatabase implements Database {
return this.toEntityResponse(tx, rows[0]);
}
async removeEntityByUid(txOpaque: unknown, uid: string): Promise<void> {
async removeEntityByUid(txOpaque: Transaction, uid: string): Promise<void> {
const tx = txOpaque as Knex.Transaction<any, any>;
const result = await tx<DbEntitiesRow>('entities').where({ id: uid }).del();
@@ -341,7 +356,7 @@ export class CommonDatabase implements Database {
}
async setRelations(
txOpaque: unknown,
txOpaque: Transaction,
originatingEntityId: string,
relations: EntityRelationSpec[],
): Promise<void> {
@@ -368,19 +383,22 @@ export class CommonDatabase implements Database {
await tx.batchInsert('entities_relations', relationsRows, BATCH_SIZE);
}
async addLocation(location: Location): Promise<DbLocationsRow> {
return await this.database.transaction<DbLocationsRow>(async tx => {
const row: DbLocationsRow = {
id: location.id,
type: location.type,
target: location.target,
};
await tx<DbLocationsRow>('locations').insert(row);
return row;
});
async addLocation(
txOpaque: Transaction,
location: Location,
): Promise<DbLocationsRow> {
const tx = txOpaque as Knex.Transaction<any, any>;
const row: DbLocationsRow = {
id: location.id,
type: location.type,
target: location.target,
};
await tx<DbLocationsRow>('locations').insert(row);
return row;
}
async removeLocation(txOpaque: unknown, id: string): Promise<void> {
async removeLocation(txOpaque: Transaction, id: string): Promise<void> {
const tx = txOpaque as Knex.Transaction<any, any>;
await tx<DbEntitiesRow>('entities')
@@ -22,4 +22,5 @@ export type {
DbEntityResponse,
EntityFilter,
EntityFilters,
Transaction,
} from './types';
+23 -10
View File
@@ -101,6 +101,13 @@ export type EntityFilter = null | string | (null | string)[];
*/
export type EntityFilters = Record<string, EntityFilter>;
/**
* An abstraction for transactions of the underlying database technology.
*/
export type Transaction = {
rollback(): Promise<void>;
};
/**
* An abstraction on top of the underlying database, wrapping the basic CRUD
* needs.
@@ -114,7 +121,7 @@ export type Database = {
*
* @param fn The callback that implements the transaction
*/
transaction<T>(fn: (tx: unknown) => Promise<T>): Promise<T>;
transaction<T>(fn: (tx: Transaction) => Promise<T>): Promise<T>;
/**
* Adds a set of new entities to the catalog.
@@ -123,7 +130,7 @@ export type Database = {
* @param request The entities being added
*/
addEntities(
tx: unknown,
tx: Transaction,
request: DbEntityRequest[],
): Promise<DbEntityResponse[]>;
@@ -147,22 +154,28 @@ export type Database = {
* @returns The updated entity
*/
updateEntity(
tx: unknown,
tx: Transaction,
request: DbEntityRequest,
matchingEtag?: string,
matchingGeneration?: number,
): Promise<DbEntityResponse>;
entities(tx: unknown, filters?: EntityFilters[]): Promise<DbEntityResponse[]>;
entities(
tx: Transaction,
filters?: EntityFilters[],
): Promise<DbEntityResponse[]>;
entityByName(
tx: unknown,
tx: Transaction,
name: EntityName,
): Promise<DbEntityResponse | undefined>;
entityByUid(tx: unknown, uid: string): Promise<DbEntityResponse | undefined>;
entityByUid(
tx: Transaction,
uid: string,
): Promise<DbEntityResponse | undefined>;
removeEntityByUid(tx: unknown, uid: string): Promise<void>;
removeEntityByUid(tx: Transaction, uid: string): Promise<void>;
/**
* Remove current relations for the entity and replace them with the new relations array
@@ -171,14 +184,14 @@ export type Database = {
* @param relations the relationships to be set
*/
setRelations(
tx: unknown,
tx: Transaction,
entityUid: string,
relations: EntityRelationSpec[],
): Promise<void>;
addLocation(location: Location): Promise<DbLocationsRow>;
addLocation(tx: Transaction, location: Location): Promise<DbLocationsRow>;
removeLocation(tx: unknown, id: string): Promise<void>;
removeLocation(tx: Transaction, id: string): Promise<void>;
location(id: string): Promise<DbLocationsRowWithStatus>;
@@ -93,6 +93,68 @@ describe('HigherOrderOperations', () => {
);
});
it('insert the location and its entities', async () => {
const spec = {
type: 'a',
target: 'b',
};
const location: LocationSpec = { type: '', target: '' };
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: { name: 'n' },
};
locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x));
locationsCatalog.locations.mockResolvedValue([]);
locationsCatalog.locations.mockResolvedValue([]);
entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([
{
entityId: 'id',
entity,
},
]);
locationReader.read.mockResolvedValue({
entities: [
{
location,
entity,
relations: [],
},
],
errors: [],
});
const result = await higherOrderOperation.addLocation(spec);
expect(result.location).toEqual(
expect.objectContaining({
id: expect.anything(),
...spec,
}),
);
expect(result.entities).toEqual([entity]);
expect(locationsCatalog.locations).toBeCalledTimes(1);
expect(locationsCatalog.addLocation).toBeCalledTimes(1);
expect(locationsCatalog.addLocation).toBeCalledWith(
expect.objectContaining({
id: expect.anything(),
...spec,
}),
);
expect(locationReader.read).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' });
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1);
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledWith(
expect.anything(),
expect.objectContaining({
locationId: expect.anything(),
dryRun: false,
outputEntities: true,
}),
);
});
it('reuses the location if a match already existed', async () => {
const spec = {
type: 'a',
@@ -150,6 +212,60 @@ describe('HigherOrderOperations', () => {
expect(entitiesCatalog.batchAddOrUpdateEntities).not.toBeCalled();
expect(locationsCatalog.addLocation).not.toBeCalled();
});
it('rollback everything after a dry run', async () => {
const spec = {
type: 'a',
target: 'b',
};
const location: LocationSpec = { type: '', target: '' };
const entity: Entity = {
apiVersion: 'a',
kind: 'b',
metadata: { name: 'n' },
};
locationsCatalog.locations.mockResolvedValue([]);
locationsCatalog.locations.mockResolvedValue([]);
entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([
{
entityId: 'id',
entity,
},
]);
locationReader.read.mockResolvedValue({
entities: [
{
location,
entity,
relations: [],
},
],
errors: [],
});
const result = await higherOrderOperation.addLocation(spec, {
dryRun: true,
});
expect(result.location).toEqual(
expect.objectContaining({
id: expect.anything(),
...spec,
}),
);
expect(result.entities).toEqual([entity]);
expect(locationsCatalog.locations).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledTimes(1);
expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' });
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1);
expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledWith(
expect.anything(),
expect.objectContaining({
dryRun: true,
outputEntities: true,
}),
);
});
});
describe('refreshLocations', () => {
@@ -213,7 +329,9 @@ describe('HigherOrderOperations', () => {
relations: [],
}),
],
'123',
{
locationId: '123',
},
);
});
@@ -33,22 +33,12 @@ import {
* database more directly.
*/
export class HigherOrderOperations implements HigherOrderOperation {
private readonly entitiesCatalog: EntitiesCatalog;
private readonly locationsCatalog: LocationsCatalog;
private readonly locationReader: LocationReader;
private readonly logger: Logger;
constructor(
entitiesCatalog: EntitiesCatalog,
locationsCatalog: LocationsCatalog,
locationReader: LocationReader,
logger: Logger,
) {
this.entitiesCatalog = entitiesCatalog;
this.locationsCatalog = locationsCatalog;
this.locationReader = locationReader;
this.logger = logger;
}
private readonly entitiesCatalog: EntitiesCatalog,
private readonly locationsCatalog: LocationsCatalog,
private readonly locationReader: LocationReader,
private readonly logger: Logger,
) {}
/**
* Adds a single location to the catalog.
@@ -62,7 +52,12 @@ export class HigherOrderOperations implements HigherOrderOperation {
*
* @param spec The location to add
*/
async addLocation(spec: LocationSpec): Promise<AddLocationResult> {
async addLocation(
spec: LocationSpec,
options?: { dryRun?: boolean },
): Promise<AddLocationResult> {
const dryRun = options?.dryRun || false;
// Attempt to find a previous location matching the spec
const previousLocations = await this.locationsCatalog.locations();
const previousLocation = previousLocations.find(
@@ -88,7 +83,9 @@ export class HigherOrderOperations implements HigherOrderOperation {
// in the entities list. But we aren't sure what to do about those yet.
// Write
if (!previousLocation) {
if (!previousLocation && !dryRun) {
// TODO: We do not include location operations in the dryRun. We might perform
// this operation as a seperate dry run.
await this.locationsCatalog.addLocation(location);
}
if (readerOutput.entities.length === 0) {
@@ -97,12 +94,14 @@ export class HigherOrderOperations implements HigherOrderOperation {
const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities(
readerOutput.entities,
location.id,
{
locationId: dryRun ? undefined : location.id,
dryRun,
outputEntities: true,
},
);
const entities = await this.entitiesCatalog.entities([
{ 'metadata.uid': writtenEntities.map(e => e.entityId) },
]);
const entities = writtenEntities.map(e => e.entity!);
return { location, entities };
}
@@ -168,7 +167,7 @@ export class HigherOrderOperations implements HigherOrderOperation {
try {
await this.entitiesCatalog.batchAddOrUpdateEntities(
readerOutput.entities,
location.id,
{ locationId: location.id },
);
} catch (e) {
for (const entity of readerOutput.entities) {
@@ -26,7 +26,10 @@ import type {
//
export type HigherOrderOperation = {
addLocation(spec: LocationSpec): Promise<AddLocationResult>;
addLocation(
spec: LocationSpec,
options?: { dryRun?: boolean },
): Promise<AddLocationResult>;
refreshAllLocations(): Promise<void>;
};
@@ -285,7 +285,36 @@ describe('createRouter', () => {
const response = await request(app).post('/locations').send(spec);
expect(higherOrderOperation.addLocation).toHaveBeenCalledTimes(1);
expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec);
expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec, {
dryRun: false,
});
expect(response.status).toEqual(201);
expect(response.body).toEqual(
expect.objectContaining({
location: { id: 'a', ...spec },
}),
);
});
it('supports dry run', async () => {
const spec: LocationSpec = {
type: 'b',
target: 'c',
};
higherOrderOperation.addLocation.mockResolvedValue({
location: { id: 'a', ...spec },
entities: [],
});
const response = await request(app)
.post('/locations?dryRun=true')
.send(spec);
expect(higherOrderOperation.addLocation).toHaveBeenCalledTimes(1);
expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec, {
dryRun: true,
});
expect(response.status).toEqual(201);
expect(response.body).toEqual(
expect.objectContaining({
@@ -20,6 +20,7 @@ import { locationSpecSchema } from '@backstage/catalog-model';
import express from 'express';
import Router from 'express-promise-router';
import { Logger } from 'winston';
import yn from 'yn';
import { EntitiesCatalog, LocationsCatalog } from '../catalog';
import { HigherOrderOperation } from '../ingestion/types';
import {
@@ -101,7 +102,8 @@ export async function createRouter(
if (higherOrderOperation) {
router.post('/locations', async (req, res) => {
const input = await validateRequestBody(req, locationSpecSchema);
const output = await higherOrderOperation.addLocation(input);
const dryRun = yn(req.query.dryRun, { default: false });
const output = await higherOrderOperation.addLocation(input, { dryRun });
res.status(201).send(output);
});
}
+4 -1
View File
@@ -95,9 +95,12 @@ export class CatalogClient implements CatalogApi {
async addLocation({
type = 'url',
target,
dryRun,
}: AddLocationRequest): Promise<AddLocationResponse> {
const response = await fetch(
`${await this.discoveryApi.getBaseUrl('catalog')}/locations`,
`${await this.discoveryApi.getBaseUrl('catalog')}/locations${
dryRun ? '?dryRun=true' : ''
}`,
{
headers: {
'Content-Type': 'application/json',
+1
View File
@@ -43,6 +43,7 @@ export interface CatalogApi {
export type AddLocationRequest = {
type?: string;
target: string;
dryRun?: boolean;
};
export type AddLocationResponse = {
@@ -20,17 +20,18 @@ import React from 'react';
import { RegisterComponentForm } from './RegisterComponentForm';
describe('RegisterComponentForm', () => {
it('should initially render a disabled button', async () => {
it('should initially render disabled buttons', async () => {
render(<RegisterComponentForm onSubmit={jest.fn()} />);
expect(
await screen.findByText(/Enter the full path to the catalog-info.yaml/),
).toBeInTheDocument();
expect(screen.getByText('Submit').closest('button')).toBeDisabled();
expect(screen.getByText('Validate').closest('button')).toBeDisabled();
expect(screen.getByText('Register').closest('button')).toBeDisabled();
});
it('should enable a submit button when the target url is set', async () => {
it('should enable the submit buttons when the target url is set', async () => {
render(<RegisterComponentForm onSubmit={jest.fn()} />);
await act(async () => {
@@ -40,7 +41,8 @@ describe('RegisterComponentForm', () => {
);
});
expect(screen.getByText('Submit').closest('button')).not.toBeDisabled();
expect(screen.getByText('Validate').closest('button')).not.toBeDisabled();
expect(screen.getByText('Register').closest('button')).not.toBeDisabled();
});
it('should show spinner while submitting', async () => {
@@ -33,8 +33,11 @@ const useStyles = makeStyles<BackstageTheme>(theme => ({
display: 'flex',
flexFlow: 'column nowrap',
},
submit: {
marginTop: theme.spacing(1),
buttonSpacing: {
marginLeft: theme.spacing(1),
},
buttons: {
marginTop: theme.spacing(2),
},
select: {
minWidth: 120,
@@ -54,12 +57,21 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
const hasErrors = !!errors.entityLocation;
const dirty = formState?.isDirty;
const onSubmitValidate = handleSubmit(data => {
data.mode = 'validate';
onSubmit(data);
});
const onSubmitRegister = handleSubmit(data => {
data.mode = 'register';
onSubmit(data);
});
return submitting ? (
<LinearProgress data-testid="loading-progress" />
) : (
<form
autoComplete="off"
onSubmit={handleSubmit(onSubmit)}
className={classes.form}
data-testid="register-form"
>
@@ -87,15 +99,27 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
)}
</FormControl>
<Button
variant="contained"
color="primary"
type="submit"
disabled={!dirty || hasErrors}
className={classes.submit}
>
Submit
</Button>
<div className={classes.buttons}>
<Button
variant="outlined"
color="primary"
type="submit"
disabled={!dirty || hasErrors}
onClick={onSubmitValidate}
>
Validate
</Button>
<Button
variant="contained"
color="primary"
type="submit"
className={classes.buttonSpacing}
disabled={!dirty || hasErrors}
onClick={onSubmitRegister}
>
Register
</Button>
</div>
</form>
);
};
@@ -75,27 +75,30 @@ export const RegisterComponentPage = ({
location: Location;
} | null;
error: null | Error;
dryRun: boolean;
}>({
data: null,
error: null,
dryRun: false,
});
const handleSubmit = async (formData: Record<string, string>) => {
setFormState(FormStates.Submitting);
const { entityLocation: target } = formData;
const { entityLocation: target, mode } = formData;
const dryRun = mode === 'validate';
try {
const data = await catalogApi.addLocation({ target });
const data = await catalogApi.addLocation({ target, dryRun });
if (!isMounted()) return;
setResult({ error: null, data });
setResult({ error: null, data, dryRun });
setFormState(FormStates.Success);
} catch (e) {
errorApi.post(e);
if (!isMounted()) return;
setResult({ error: e, data: null });
setResult({ error: e, data: null, dryRun });
setFormState(FormStates.Idle);
}
};
@@ -124,6 +127,7 @@ export const RegisterComponentPage = ({
{formState === FormStates.Success && (
<RegisterComponentResultDialog
entities={result.data!.entities}
dryRun={result.dryRun}
onClose={() => setFormState(FormStates.Idle)}
classes={{ paper: classes.dialogPaper }}
catalogRouteRef={catalogRouteRef}
@@ -29,7 +29,7 @@ import {
List,
ListItem,
} from '@material-ui/core';
import React from 'react';
import React, { useState } from 'react';
import { generatePath, resolvePath } from 'react-router';
import { Link as RouterLink } from 'react-router-dom';
@@ -37,6 +37,7 @@ type Props = {
onClose: () => void;
classes?: Record<string, string>;
entities: Entity[];
dryRun?: boolean;
catalogRouteRef: RouteRef;
};
@@ -64,49 +65,71 @@ export const RegisterComponentResultDialog = ({
onClose,
classes,
entities,
dryRun,
catalogRouteRef,
}: Props) => (
<Dialog open onClose={onClose} classes={classes}>
<DialogTitle>Registration Result</DialogTitle>
<DialogContent>
<DialogContentText>
The following entities have been successfully created:
</DialogContentText>
<List>
{entities.map((entity: any, index: number) => {
const entityPath = getEntityCatalogPath({ entity, catalogRouteRef });
return (
<React.Fragment
key={`${entity.metadata.namespace}-${entity.metadata.name}`}
>
<ListItem>
<StructuredMetadataTable
dense
metadata={{
name: entity.metadata.name,
type: entity.spec.type,
link: (
<Link component={RouterLink} to={entityPath}>
{entityPath}
</Link>
),
}}
/>
</ListItem>
{index < entities.length - 1 && <Divider component="li" />}
</React.Fragment>
);
})}
</List>
</DialogContent>
<DialogActions>
<Button
component={RouterLink}
to={`/${catalogRouteRef.path}`}
color="default"
>
To Catalog
</Button>
</DialogActions>
</Dialog>
);
}: Props) => {
const [open, setOpen] = useState(true);
const handleClose = () => {
setOpen(false);
};
return (
<Dialog open={open} onClose={onClose} classes={classes}>
<DialogTitle>
{dryRun ? 'Validation Result' : 'Registration Result'}
</DialogTitle>
<DialogContent>
<DialogContentText>
{dryRun
? 'The following entities would be created:'
: 'The following entities have been successfully created:'}
</DialogContentText>
<List>
{entities.map((entity: any, index: number) => {
const entityPath = getEntityCatalogPath({
entity,
catalogRouteRef,
});
return (
<React.Fragment
key={`${entity.metadata.namespace}-${entity.metadata.name}`}
>
<ListItem>
<StructuredMetadataTable
dense
metadata={{
name: entity.metadata.name,
type: entity.spec.type,
link: (
<Link component={RouterLink} to={entityPath}>
{entityPath}
</Link>
),
}}
/>
</ListItem>
{index < entities.length - 1 && <Divider component="li" />}
</React.Fragment>
);
})}
</List>
</DialogContent>
<DialogActions>
{dryRun && (
<Button onClick={handleClose} color="default">
Close
</Button>
)}
{!dryRun && (
<Button
component={RouterLink}
to={`/${catalogRouteRef.path}`}
color="default"
>
To Catalog
</Button>
)}
</DialogActions>
</Dialog>
);
};
+1 -1
View File
@@ -32,7 +32,7 @@
"express-promise-router": "^3.0.3",
"fs-extra": "^9.0.1",
"git-url-parse": "^11.4.0",
"knex": "^0.21.1",
"knex": "^0.21.6",
"nodegit": "^0.27.0",
"winston": "^3.2.1"
},
+10 -12
View File
@@ -13482,7 +13482,7 @@ inflight@^1.0.4:
once "^1.3.0"
wrappy "1"
inherits@2, inherits@2.0.4, inherits@^2.0.0, inherits@^2.0.1, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.0, inherits@~2.0.1, inherits@~2.0.3, inherits@~2.0.4:
inherits@2, inherits@2.0.4, inherits@^2.0.0, inherits@^2.0.1, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.0, inherits@~2.0.1, inherits@~2.0.3:
version "2.0.4"
resolved "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz#0fa2c64f932917c3433a0ded55363aae37416b7c"
integrity sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==
@@ -15217,23 +15217,21 @@ kleur@^3.0.3:
resolved "https://registry.npmjs.org/kleur/-/kleur-3.0.3.tgz#a79c9ecc86ee1ce3fa6206d1216c501f147fc07e"
integrity sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==
knex@^0.21.1:
version "0.21.5"
resolved "https://registry.npmjs.org/knex/-/knex-0.21.5.tgz#c4be1958488f348aed3510aa4b7115639ee1bd01"
integrity sha512-cQj7F2D/fu03eTr6ZzYCYKdB9w7fPYlvTiU/f2OeXay52Pq5PwD+NAkcf40WDnppt/4/4KukROwlMOaE7WArcA==
knex@^0.21.6:
version "0.21.8"
resolved "https://registry.npmjs.org/knex/-/knex-0.21.8.tgz#e5c07af61ee6aa006d3468e10e3a69351deb0c26"
integrity sha512-ziUu4vAlIGA8j2l0S4xcD1d3XdpJA4HYGhwHEhgAgefGCmB1OLSjUGCs/ebkJal42fSvAkyZaB0tcOtTXKgS5g==
dependencies:
colorette "1.2.1"
commander "^5.1.0"
debug "4.1.1"
esm "^3.2.25"
getopts "2.2.5"
inherits "~2.0.4"
interpret "^2.2.0"
liftoff "3.1.0"
lodash "^4.17.20"
mkdirp "^1.0.4"
pg-connection-string "2.3.0"
tarn "^3.0.0"
tarn "^3.0.1"
tildify "2.0.0"
uuid "^7.0.3"
v8flags "^3.2.0"
@@ -22445,10 +22443,10 @@ tar@^6.0.1, tar@^6.0.2:
mkdirp "^1.0.3"
yallist "^4.0.0"
tarn@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/tarn/-/tarn-3.0.0.tgz#a4082405216c0cce182b8b4cb2639c52c1e870d4"
integrity sha512-PKUnlDFODZueoA8owLehl8vLcgtA8u4dRuVbZc92tspDYZixjJL6TqYOmryf/PfP/EBX+2rgNcrj96NO+RPkdQ==
tarn@^3.0.1:
version "3.0.1"
resolved "https://registry.npmjs.org/tarn/-/tarn-3.0.1.tgz#ebac2c6dbc6977d34d4526e0a7814200386a8aec"
integrity sha512-6usSlV9KyHsspvwu2duKH+FMUhqJnAh6J5J/4MITl8s94iSUQTLkJggdiewKv4RyARQccnigV48Z+khiuVZDJw==
tdigest@^0.1.1:
version "0.1.1"