From 645bee1b4c1a31a10e3fa3ec235fc269f84628cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 23 May 2020 19:29:21 +0200 Subject: [PATCH 1/3] Add code for building a search index over added/modified entities This is not actually being inserted yet, because doing so needs a little refactoring of the database code that happens in another PR. Adding separate PR for the index to make it easier to review the logic. --- plugins/catalog-backend/src/database/Database.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index c797a57f3c..2413fb217c 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -27,6 +27,7 @@ import { DbEntityResponse, DbLocationsRow, } from './types'; +import { buildEntitySearch } from './search'; function serializeMetadata( metadata: DescriptorEnvelope['metadata'], From 44844af0bdf5479f6f79ee1042994ca9a5fdd754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 20 May 2020 14:21:20 +0200 Subject: [PATCH 2/3] Store entities completely in the database --- packages/backend-common/src/logging/index.ts | 1 + .../backend-common/src/logging/voidLogger.ts | 21 +- packages/backend/src/plugins/catalog.ts | 2 +- .../fixtures/two_components.yaml | 2 +- .../src/catalog/DatabaseEntitiesCatalog.ts | 18 +- .../src/catalog/StaticEntitiesCatalog.ts | 20 +- plugins/catalog-backend/src/catalog/types.ts | 6 +- .../src/database/Database.test.ts | 191 +++++++++- .../catalog-backend/src/database/Database.ts | 327 ++++++++++++++---- .../src/database/DatabaseManager.test.ts | 68 ++-- .../src/database/DatabaseManager.ts | 97 +++++- ...0200520140700_location_update_log_table.ts | 2 +- .../src/database/search.test.ts | 2 +- .../src/ingestion/LocationReaders.ts | 3 +- .../ComponentDescriptorV1beta1Parser.ts | 3 +- .../descriptors/DescriptorEnvelopeParser.ts | 4 +- .../src/ingestion/descriptors/types.ts | 38 -- .../ingestion/sources/FileLocationSource.ts | 3 +- .../catalog-backend/src/ingestion/types.ts | 125 +++---- plugins/catalog-backend/src/service/router.ts | 14 +- 20 files changed, 690 insertions(+), 257 deletions(-) rename plugins/catalog-backend/src/ingestion/sources/types.ts => packages/backend-common/src/logging/voidLogger.ts (61%) delete mode 100644 plugins/catalog-backend/src/ingestion/descriptors/types.ts diff --git a/packages/backend-common/src/logging/index.ts b/packages/backend-common/src/logging/index.ts index 06ce76ac54..0ebf0371e9 100644 --- a/packages/backend-common/src/logging/index.ts +++ b/packages/backend-common/src/logging/index.ts @@ -15,3 +15,4 @@ */ export * from './rootLogger'; +export * from './voidLogger'; diff --git a/plugins/catalog-backend/src/ingestion/sources/types.ts b/packages/backend-common/src/logging/voidLogger.ts similarity index 61% rename from plugins/catalog-backend/src/ingestion/sources/types.ts rename to packages/backend-common/src/logging/voidLogger.ts index c078693a19..1837b0e412 100644 --- a/plugins/catalog-backend/src/ingestion/sources/types.ts +++ b/packages/backend-common/src/logging/voidLogger.ts @@ -14,15 +14,14 @@ * limitations under the License. */ -import { ReaderOutput } from '../types'; +import { PassThrough } from 'stream'; +import winston, { Logger } from 'winston'; -export type LocationSource = { - /** - * Reads the contents of a single location. - * - * @param target The location target to read - * @returns The parsed contents, as an array of unverified descriptors - * @throws An error if the location target could not be read - */ - read(target: string): Promise; -}; +/** + * A logger that just throws away all messages. + */ +export function getVoidLogger(): Logger { + return winston.createLogger({ + transports: [new winston.transports.Stream({ stream: new PassThrough() })], + }); +} diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index d48fda5681..e18d021395 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -29,7 +29,7 @@ export default async function ({ logger, database }: PluginEnvironment) { const reader = LocationReaders.create(); const parser = DescriptorParsers.create(); - const db = await DatabaseManager.createDatabase(database); + const db = await DatabaseManager.createDatabase(database, logger); runPeriodically( () => DatabaseManager.refreshLocations(db, reader, parser, logger), 10000, diff --git a/plugins/catalog-backend/fixtures/two_components.yaml b/plugins/catalog-backend/fixtures/two_components.yaml index d0e51ca79e..470106116d 100644 --- a/plugins/catalog-backend/fixtures/two_components.yaml +++ b/plugins/catalog-backend/fixtures/two_components.yaml @@ -11,4 +11,4 @@ kind: Component metadata: name: component2 spec: - type: service + type: website diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 5c2c79d427..697da0ea35 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { NotFoundError } from '@backstage/backend-common'; import { Database } from '../database'; import { DescriptorEnvelope } from '../ingestion/types'; import { EntitiesCatalog } from './types'; @@ -22,12 +23,23 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { constructor(private readonly database: Database) {} async entities(): Promise { - const items = await this.database.entities(); + const items = await this.database.transaction(tx => + this.database.entities(tx), + ); return items.map(i => i.entity); } - async entity(name: string): Promise { - const item = await this.database.entity(name); + async entity( + kind: string, + name: string, + namespace: string | undefined, + ): Promise { + const item = await this.database.transaction(tx => + this.database.entity(tx, kind, name, namespace), + ); + if (!item) { + throw new NotFoundError('Entity cannot be found'); + } return item.entity; } } diff --git a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts index d74487ab2c..17f7603fb7 100644 --- a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts @@ -15,6 +15,7 @@ */ import { NotFoundError } from '@backstage/backend-common'; +import lodash from 'lodash'; import { DescriptorEnvelope } from '../ingestion'; import { EntitiesCatalog } from './types'; @@ -26,14 +27,23 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { } async entities(): Promise { - return this._entities.slice(); + return lodash.cloneDeep(this._entities); } - async entity(name: string): Promise { - const item = this._entities.find(e => e.metadata?.name === name); + async entity( + kind: string, + name: string, + namespace: string | undefined, + ): Promise { + const item = this._entities.find( + e => + kind === e.kind && + name === e.metadata?.name && + namespace === e.metadata?.namespace, + ); if (!item) { - throw new NotFoundError(`Found no entity with name ${name}`); + throw new NotFoundError('Entity cannot be found'); } - return item; + return lodash.cloneDeep(item); } } diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index bb50a00783..92c059d25b 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -23,7 +23,11 @@ import { DescriptorEnvelope } from '../ingestion'; export type EntitiesCatalog = { entities(): Promise; - entity(id: string): Promise; + entity( + kind: string, + name: string, + namespace: string | undefined, + ): Promise; }; // diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/Database.test.ts index 459968412e..0eb64916e5 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/Database.test.ts @@ -14,30 +14,74 @@ * limitations under the License. */ -import knex from 'knex'; +import { + ConflictError, + getVoidLogger, + NotFoundError, +} from '@backstage/backend-common'; +import Knex from 'knex'; import path from 'path'; import { Database } from './Database'; -import { AddDatabaseLocation, DbLocationsRow } from './types'; +import { + AddDatabaseLocation, + DbEntityRequest, + DbEntityResponse, + DbLocationsRow, +} from './types'; describe('Database', () => { - const database = knex({ - client: 'sqlite3', - connection: ':memory:', - useNullAsDefault: true, - }); - database.client.pool.on('createSuccess', (_eventId: any, resource: any) => { - resource.run('PRAGMA foreign_keys = ON', () => {}); - }); + let database: Knex; + let entityRequest: DbEntityRequest; + let entityResponse: DbEntityResponse; beforeEach(async () => { + database = Knex({ + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }); + + await database.raw('PRAGMA foreign_keys = ON'); await database.migrate.latest({ directory: path.resolve(__dirname, 'migrations'), loadExtensions: ['.ts'], }); + + entityRequest = { + entity: { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + labels: { e: 'f' }, + annotations: { g: 'h' }, + }, + spec: { i: 'j' }, + }, + }; + + entityResponse = { + locationId: undefined, + entity: { + apiVersion: 'a', + kind: 'b', + metadata: { + uid: expect.anything(), + etag: expect.anything(), + generation: expect.anything(), + name: 'c', + namespace: 'd', + labels: { e: 'f' }, + annotations: { g: 'h' }, + }, + spec: { i: 'j' }, + }, + }; }); it('manages locations', async () => { - const db = new Database(database); + const db = new Database(database, getVoidLogger()); const input: AddDatabaseLocation = { type: 'a', target: 'b' }; const output: DbLocationsRow = { id: expect.anything(), @@ -62,7 +106,7 @@ describe('Database', () => { it('instead of adding second location with the same target, returns existing one', async () => { // Prepare - const catalog = new Database(database); + const catalog = new Database(database, getVoidLogger()); const input: AddDatabaseLocation = { type: 'a', target: 'b' }; const output1: DbLocationsRow = await catalog.addLocation(input); @@ -75,4 +119,127 @@ describe('Database', () => { // Locations contain only one record expect(locations).toEqual([output1]); }); + + describe('addEntity', () => { + it('happy path: adds entity to empty database', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + expect(added).toStrictEqual(entityResponse); + expect(added.entity.metadata!.generation).toBe(1); + }); + + it('rejects adding the same-named entity twice', async () => { + const catalog = new Database(database, getVoidLogger()); + await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); + await expect( + catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), + ).rejects.toThrow(ConflictError); + }); + + it('accepts adding the same-named entity twice if on different namespaces', async () => { + const catalog = new Database(database, getVoidLogger()); + entityRequest.entity.metadata!.namespace = 'namespace1'; + await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); + entityRequest.entity.metadata!.namespace = 'namespace2'; + await expect( + catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), + ).resolves.toBeDefined(); + }); + }); + + describe('updateEntity', () => { + it('can read and no-op-update an entity', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + const updated = await catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ); + expect(updated.entity.apiVersion).toEqual(added.entity.apiVersion); + expect(updated.entity.kind).toEqual(added.entity.kind); + expect(updated.entity.metadata!.etag).not.toEqual( + added.entity.metadata!.etag, + ); + expect(updated.entity.metadata!.generation).toEqual( + added.entity.metadata!.generation, + ); + expect(updated.entity.metadata!.name).toEqual( + added.entity.metadata!.name, + ); + expect(updated.entity.metadata!.namespace).toEqual( + added.entity.metadata!.namespace, + ); + }); + + it('can update name if uid matches', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + added.entity.metadata!.name! = 'new!'; + const updated = await catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ); + expect(updated.entity.metadata!.name).toEqual('new!'); + }); + + it('can update fields if kind, name, and namespace match', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + added.entity.apiVersion = 'something.new'; + delete added.entity.metadata!.uid; + delete added.entity.metadata!.generation; + const updated = await catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ); + expect(updated.entity.apiVersion).toEqual('something.new'); + }); + + it('rejects if kind, name, but not namespace match', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + added.entity.apiVersion = 'something.new'; + delete added.entity.metadata!.uid; + delete added.entity.metadata!.generation; + added.entity.metadata!.namespace = 'something.wrong'; + await expect( + catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ), + ).rejects.toThrow(NotFoundError); + }); + + it('fails to update an entity if etag does not match', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + added.entity.metadata!.etag = 'garbage'; + await expect( + catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ), + ).rejects.toThrow(ConflictError); + }); + + it('fails to update an entity if generation does not match', async () => { + const catalog = new Database(database, getVoidLogger()); + const added = await catalog.transaction(tx => + catalog.addEntity(tx, entityRequest), + ); + added.entity.metadata!.generation! += 100; + await expect( + catalog.transaction(tx => + catalog.updateEntity(tx, { entity: added.entity }), + ), + ).rejects.toThrow(ConflictError); + }); + }); }); diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 2413fb217c..cd4ca731a3 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -14,34 +14,43 @@ * limitations under the License. */ -import { InputError, NotFoundError } from '@backstage/backend-common'; +import { + ConflictError, + InputError, + NotFoundError, +} from '@backstage/backend-common'; import Knex from 'knex'; +import lodash from 'lodash'; import { v4 as uuidv4 } from 'uuid'; -import { DescriptorEnvelope } from '../ingestion'; +import { Logger } from 'winston'; +import { DescriptorEnvelope, EntityMeta } from '../ingestion'; +import { buildEntitySearch } from './search'; import { AddDatabaseLocation, DatabaseLocationUpdateLogEvent, DatabaseLocationUpdateLogStatus, DbEntitiesRow, + DbEntitiesSearchRow, DbEntityRequest, DbEntityResponse, DbLocationsRow, } from './types'; -import { buildEntitySearch } from './search'; -function serializeMetadata( - metadata: DescriptorEnvelope['metadata'], -): DbEntitiesRow['metadata'] { - if (!metadata) { - return null; - } - - const output = { ...metadata }; +function getStrippedMetadata(metadata: EntityMeta): EntityMeta { + const output = lodash.cloneDeep(metadata); delete output.uid; delete output.etag; delete output.generation; - return JSON.stringify(output); + return output; +} + +function serializeMetadata(metadata: EntityMeta | undefined): string | null { + if (!metadata) { + return null; + } + + return JSON.stringify(getStrippedMetadata(metadata)); } function serializeSpec( @@ -54,29 +63,32 @@ function serializeSpec( return JSON.stringify(spec); } -function entityRequestToDb(request: DbEntityRequest): DbEntitiesRow { +function toEntityRow( + locationId: string | undefined, + entity: DescriptorEnvelope, +): DbEntitiesRow { return { - id: '', - location_id: request.locationId || null, - etag: new Buffer(uuidv4()).toString('base64').replace(/[^\w]/g, ''), // TODO(freben): Atomicity isn't checked using these yet - generation: 1, // TODO(freben): These aren't updated yet - api_version: request.entity.apiVersion, - kind: request.entity.kind, - name: request.entity.metadata?.name || null, - namespace: request.entity.metadata?.namespace || null, - metadata: serializeMetadata(request.entity.metadata), - spec: serializeSpec(request.entity.spec), + id: entity.metadata!.uid!, + location_id: locationId || null, + etag: entity.metadata!.etag!, + generation: entity.metadata!.generation!, + api_version: entity.apiVersion, + kind: entity.kind, + name: entity.metadata!.name || null, + namespace: entity.metadata!.namespace || null, + metadata: serializeMetadata(entity.metadata), + spec: serializeSpec(entity.spec), }; } -function entityDbToResponse(row: DbEntitiesRow): DbEntityResponse { +function toEntityResponse(row: DbEntitiesRow): DbEntityResponse { const entity: DescriptorEnvelope = { apiVersion: row.api_version, kind: row.kind, metadata: { uid: row.id, etag: row.etag, - generation: row.generation, + generation: Number(row.generation), // cast because of sqlite }, }; @@ -96,49 +108,229 @@ function entityDbToResponse(row: DbEntitiesRow): DbEntityResponse { }; } -export class Database { - constructor(private readonly database: Knex) {} - - async addOrUpdateEntity(request: DbEntityRequest): Promise { - if (!request.entity.metadata?.name) { - throw new InputError(`Entities without names are not yet supported`); - } - - const newRow = entityRequestToDb(request); - - await this.database.transaction(async tx => { - // TODO(freben): Currently, several locations can compete for the same entity - // TODO(freben): If locationId is unset in the input, it won't be overwritten - should we instead replace with null? - const count = await tx('entities') - .where({ name: request.entity.metadata?.name }) - .update({ - ...newRow, - id: undefined, - }); - if (!count) { - await tx('entities').insert({ - ...newRow, - id: uuidv4(), - }); - } - }); +function specsAreEqual( + first: string | null, + second: object | undefined, +): boolean { + if (!first && !second) { + return true; + } else if (!first || !second) { + return false; } - async entities(): Promise { - const items = await this.database('entities') + return lodash.isEqual(JSON.parse(first), second); +} + +function generateUid(): string { + return uuidv4(); +} + +function generateEtag(): string { + return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, ''); +} + +/** + * An abstraction on top of the underlying database, wrapping the basic CRUD + * needs. + */ +export class Database { + constructor( + private readonly database: Knex, + private readonly logger: Logger, + ) {} + + /** + * Runs a transaction. + * + * The callback is expected to make calls back into this class. When it + * completes, the transaction is closed. + * + * @param fn The callback that implements the transaction + */ + async transaction( + fn: (tx: Knex.Transaction) => Promise, + ): Promise { + try { + return await this.database.transaction(fn); + } catch (e) { + this.logger.debug(`Error during transaction, ${e}`); + + if ( + /SQLITE_CONSTRAINT: UNIQUE/.test(e.message) || + /unique constraint/.test(e.message) + ) { + throw new ConflictError(`Rejected due to a conflicting entity`, e); + } + + throw e; + } + } + + /** + * Adds a new entity to the catalog. + * + * @param tx An ongoing transaction + * @param request The entity being added + * @returns The added entity, with uid, etag and generation set + */ + async addEntity( + tx: Knex.Transaction, + request: DbEntityRequest, + ): Promise { + if (request.entity.metadata?.uid !== undefined) { + throw new InputError('May not specify uid for new entities'); + } else if (request.entity.metadata?.etag !== undefined) { + throw new InputError('May not specify etag for new entities'); + } else if (request.entity.metadata?.generation !== undefined) { + throw new InputError('May not specify generation for new entities'); + } + + const newEntity = lodash.cloneDeep(request.entity); + newEntity.metadata = Object.assign({}, newEntity.metadata, { + uid: generateUid(), + etag: generateEtag(), + generation: 1, + }); + + const newRow = toEntityRow(request.locationId, newEntity); + await tx('entities').insert(newRow); + await this.updateEntitiesSearch(tx, newRow.id, newEntity); + + return { locationId: request.locationId, entity: newEntity }; + } + + /** + * Updates an existing entity in the catalog. + * + * The given entity must contain enough information to identify an already + * stored entity in the catalog - either by uid, or by kind + namespace + + * name. If no matching entity is found, the operation fails. + * + * If etag or generation are given, they are taken into account. Attempts to + * update a matching entity, but where the etag and/or generation are not + * equal to the passed values, will fail. + * + * @param tx An ongoing transaction + * @param request The entity being updated + * @returns The updated entity + */ + async updateEntity( + tx: Knex.Transaction, + request: DbEntityRequest, + ): Promise { + const { kind } = request.entity; + const { + uid, + etag: expectedOldEtag, + generation: expectedOldGeneration, + name, + namespace, + } = request.entity.metadata ?? {}; + + // Find existing entities that match the given metadata + let entitySelector: Partial; + if (uid) { + entitySelector = { id: uid }; + } else if (kind && name) { + entitySelector = { + kind, + name: name, + namespace: namespace || null, + }; + } else { + throw new InputError( + 'Must specify either uid, or kind + name + namespace to be able to identify an entity', + ); + } + const oldRows = await tx('entities') + .where(entitySelector) + .select(); + if (oldRows.length !== 1) { + throw new NotFoundError('No matching entity found'); + } + + // Validate the old entity + const oldRow = oldRows[0]; + // The Number cast is here because sqlite reads it as a string, no matter + // what the table actually says + oldRow.generation = Number(oldRow.generation); + if (expectedOldEtag) { + if (expectedOldEtag !== oldRow.etag) { + throw new ConflictError( + `Etag mismatch, expected="${expectedOldEtag}" found="${oldRow.etag}"`, + ); + } + } + if (expectedOldGeneration) { + if (expectedOldGeneration !== oldRow.generation) { + throw new ConflictError( + `Generation mismatch, expected="${expectedOldGeneration}" found="${oldRow.generation}"`, + ); + } + } + + // Build the new shape of the entity + const newEtag = generateEtag(); + const newGeneration = specsAreEqual(oldRow.spec, request.entity.spec) + ? oldRow.generation + : oldRow.generation + 1; + const newEntity = lodash.cloneDeep(request.entity); + newEntity.metadata = Object.assign({}, request.entity.metadata, { + uid: oldRow.id, + etag: newEtag, + generation: newGeneration, + }); + + // Preserve annotations that were set on the old version of the entity, + // unless the new version overwrites them + if (oldRow.metadata) { + const oldMetadata = JSON.parse(oldRow.metadata) as EntityMeta; + if (oldMetadata.annotations) { + newEntity.metadata!.annotations = { + ...oldMetadata.annotations, + ...newEntity.metadata!.annotations, + }; + } + } + + // Store the updated entity; select on the old etag to ensure that we do + // not lose to another writer + const newRow = toEntityRow(request.locationId, newEntity); + const updatedRows = await tx('entities') + .where({ id: oldRow.id, etag: oldRow.etag }) + .update(newRow); + + // If this happens, somebody else changed the entity just now + if (updatedRows !== 1) { + throw new ConflictError(`Failed to update entity`); + } + + await this.updateEntitiesSearch(tx, oldRow.id, newEntity); + return { locationId: request.locationId, entity: newEntity }; + } + + async entities(tx: Knex.Transaction): Promise { + const rows = await tx('entities') .orderBy('namespace', 'name') .select(); - return items.map(entityDbToResponse); + return rows.map(row => toEntityResponse(row)); } - async entity(name: string): Promise { - const items = await this.database('entities') - .where({ name }) + async entity( + tx: Knex.Transaction, + kind: string, + name: string, + namespace?: string, + ): Promise { + const rows = await tx('entities') + .where({ kind, name, namespace: namespace || null }) .select(); - if (!items.length) { - throw new NotFoundError(`Found no entity with name ${name}`); + + if (rows.length !== 1) { + return undefined; } - return entityDbToResponse(items[0]); + + return toEntityResponse(rows[0]); } async addLocation(location: AddDatabaseLocation): Promise { @@ -206,15 +398,20 @@ export class Database { }); } - /* private async updateEntitiesSearch( tx: Knex.Transaction, entityId: string, data: DescriptorEnvelope, ): Promise { - const entries = buildEntitySearch(entityId, data); - await tx('entities_search').where({ entity_id: entityId }).del(); - await tx('entities_search').insert(entries); + try { + const entries = buildEntitySearch(entityId, data); + await tx('entities_search') + .where({ entity_id: entityId }) + .del(); + await tx('entities_search').insert(entries); + } catch { + // ignore intentionally - if this happens, the entity was deleted before + // we got around to writing the entries + } } - */ } diff --git a/plugins/catalog-backend/src/database/DatabaseManager.test.ts b/plugins/catalog-backend/src/database/DatabaseManager.test.ts index 1043733206..2da3c33a00 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.test.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.test.ts @@ -14,8 +14,7 @@ * limitations under the License. */ -import { PassThrough } from 'stream'; -import winston from 'winston'; +import { getVoidLogger } from '@backstage/backend-common'; import { ComponentDescriptor, DescriptorParser, @@ -25,16 +24,12 @@ import { import { Database } from './Database'; import { DatabaseManager } from './DatabaseManager'; import { DatabaseLocationUpdateLogStatus, DbLocationsRow } from './types'; +import Knex from 'knex'; describe('DatabaseManager', () => { - const logger = winston.createLogger({ - transports: [new winston.transports.Stream({ stream: new PassThrough() })], - }); - describe('refreshLocations', () => { it('works with no locations added', async () => { const db = ({ - addOrUpdateEntity: jest.fn(), locations: jest.fn().mockResolvedValue([]), } as unknown) as Database; const reader: LocationReader = { @@ -45,33 +40,34 @@ describe('DatabaseManager', () => { }; await expect( - DatabaseManager.refreshLocations(db, reader, parser, logger), + DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()), ).resolves.toBeUndefined(); expect(reader.read).not.toHaveBeenCalled(); expect(parser.parse).not.toHaveBeenCalled(); }); it('can update a single location', async () => { - const db = ({ - addOrUpdateEntity: jest.fn(), - locations: jest.fn(() => - Promise.resolve([ - { - id: '123', - type: 'some', - target: 'thing', - } as DbLocationsRow, - ]), - ), - addLocationUpdateLogEvent: jest.fn(), - } as unknown) as Database; - + const location: DbLocationsRow = { + id: '123', + type: 'some', + target: 'thing', + }; const desc: ComponentDescriptor = { apiVersion: 'backstage.io/v1beta1', kind: 'Component', metadata: { name: 'c1' }, spec: { type: 'service' }, }; + const tx = (undefined as unknown) as Knex.Transaction; + + const db = ({ + transaction: jest.fn(f => f(tx)), + entity: jest.fn(() => Promise.resolve(undefined)), + addEntity: jest.fn(), + locations: jest.fn(() => Promise.resolve([location])), + addLocationUpdateLogEvent: jest.fn(), + } as Partial) as Database; + const reader: LocationReader = { read: jest.fn(() => Promise.resolve([{ type: 'data', data: desc }])), }; @@ -80,12 +76,12 @@ describe('DatabaseManager', () => { }; await expect( - DatabaseManager.refreshLocations(db, reader, parser, logger), + DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()), ).resolves.toBeUndefined(); expect(reader.read).toHaveBeenCalledTimes(1); expect(reader.read).toHaveBeenNthCalledWith(1, 'some', 'thing'); - expect(db.addOrUpdateEntity).toHaveBeenCalledTimes(1); - expect(db.addOrUpdateEntity).toHaveBeenNthCalledWith(1, { + expect(db.addEntity).toHaveBeenCalledTimes(1); + expect(db.addEntity).toHaveBeenNthCalledWith(1, undefined, { locationId: '123', entity: expect.objectContaining({ metadata: expect.objectContaining({ name: 'c1' }), @@ -94,8 +90,12 @@ describe('DatabaseManager', () => { }); it('logs successful updates', async () => { + const tx = (undefined as unknown) as Knex.Transaction; + const db = ({ - addOrUpdateEntity: jest.fn(), + transaction: jest.fn(f => f(tx)), + addEntity: jest.fn(), + entity: jest.fn(() => Promise.resolve(undefined)), locations: jest.fn(() => Promise.resolve([ { @@ -122,7 +122,7 @@ describe('DatabaseManager', () => { }; await expect( - DatabaseManager.refreshLocations(db, reader, parser, logger), + DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()), ).resolves.toBeUndefined(); expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith( @@ -141,8 +141,11 @@ describe('DatabaseManager', () => { }); it('logs unsuccessful updates when parser fails', async () => { + const tx = (undefined as unknown) as Knex.Transaction; + const db = ({ - addOrUpdateEntity: jest.fn(), + transaction: jest.fn(f => f(tx)), + addEntity: jest.fn(), locations: jest.fn(() => Promise.resolve([ { @@ -171,7 +174,7 @@ describe('DatabaseManager', () => { }; await expect( - DatabaseManager.refreshLocations(db, reader, parser, logger), + DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()), ).resolves.toBeUndefined(); expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith( @@ -191,8 +194,11 @@ describe('DatabaseManager', () => { }); it('logs unsuccessful updates when reader fails', async () => { + const tx = (undefined as unknown) as Knex.Transaction; + const db = ({ - addOrUpdateEntity: jest.fn(), + transaction: jest.fn(f => f(tx)), + addEntity: jest.fn(), locations: jest.fn(() => Promise.resolve([ { @@ -217,7 +223,7 @@ describe('DatabaseManager', () => { }; await expect( - DatabaseManager.refreshLocations(db, reader, parser, logger), + DatabaseManager.refreshLocations(db, reader, parser, getVoidLogger()), ).resolves.toBeUndefined(); expect(db.addLocationUpdateLogEvent).toHaveBeenNthCalledWith( diff --git a/plugins/catalog-backend/src/database/DatabaseManager.ts b/plugins/catalog-backend/src/database/DatabaseManager.ts index 29b0badb7a..7f646971db 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.ts @@ -15,19 +15,28 @@ */ import Knex from 'knex'; +import lodash from 'lodash'; import path from 'path'; import { Logger } from 'winston'; -import { DescriptorParser, LocationReader, ParserError } from '../ingestion'; +import { + DescriptorEnvelope, + DescriptorParser, + LocationReader, + ParserError, +} from '../ingestion'; import { Database } from './Database'; import { DatabaseLocationUpdateLogStatus, DbEntityRequest } from './types'; export class DatabaseManager { - public static async createDatabase(database: Knex): Promise { + public static async createDatabase( + database: Knex, + logger: Logger, + ): Promise { await database.migrate.latest({ directory: path.resolve(__dirname, 'migrations'), loadExtensions: ['.js'], }); - return new Database(database); + return new Database(database, logger); } private static async logUpdateSuccess( @@ -66,20 +75,25 @@ export class DatabaseManager { for (const location of locations) { try { logger.debug( - `Refreshing location ${location.id} type "${location.type}" target "${location.target}"`, + `Refreshing location id="${location.id}" type="${location.type}" target="${location.target}"`, ); const readerOutput = await reader.read(location.type, location.target); for (const readerItem of readerOutput) { if (readerItem.type === 'error') { - logger.debug(readerItem.error); + logger.info(readerItem.error); continue; } + try { const entity = await parser.parse(readerItem.data); - const dbc: DbEntityRequest = { locationId: location.id, entity }; - await database.addOrUpdateEntity(dbc); + await DatabaseManager.refreshSingleEntity( + database, + location.id, + entity, + logger, + ); await DatabaseManager.logUpdateSuccess( database, location.id, @@ -98,11 +112,76 @@ export class DatabaseManager { ); } } - await DatabaseManager.logUpdateSuccess(database, location.id); + await DatabaseManager.logUpdateSuccess( + database, + location.id, + undefined, + ); } catch (error) { - logger.debug(`Failed to refresh location ${location.id}, ${error}`); + logger.debug( + `Failed to refresh location id="${location.id}", ${error}`, + ); await DatabaseManager.logUpdateFailure(database, location.id, error); } } } + + private static async refreshSingleEntity( + database: Database, + locationId: string, + entity: DescriptorEnvelope, + logger: Logger, + ): Promise { + const { kind } = entity; + const { name, namespace } = entity.metadata || {}; + if (!name) { + throw new Error('Entities without names are not yet supported'); + } + + const request: DbEntityRequest = { + locationId: locationId, + entity: entity, + }; + + logger.debug( + `Read entity kind="${kind}" name="${name}" namespace="${namespace}"`, + ); + + await database.transaction(async tx => { + const previous = await database.entity(tx, kind, name, namespace); + if (!previous) { + logger.debug(`No such entity found, adding`); + await database.addEntity(tx, request); + } else if ( + !DatabaseManager.entitiesAreEqual(previous.entity, request.entity) + ) { + logger.debug(`Different from existing entity, updating`); + await database.updateEntity(tx, request); + } else { + logger.debug(`Equal to existing entity, skipping update`); + } + }); + } + + private static entitiesAreEqual( + first: DescriptorEnvelope, + second: DescriptorEnvelope, + ) { + const firstClone = lodash.cloneDeep(first); + const secondClone = lodash.cloneDeep(second); + + // Remove generated fields + if (firstClone.metadata) { + delete firstClone.metadata.uid; + delete firstClone.metadata.etag; + delete firstClone.metadata.generation; + } + if (secondClone.metadata) { + delete secondClone.metadata.uid; + delete secondClone.metadata.etag; + delete secondClone.metadata.generation; + } + + return lodash.isEqual(firstClone, secondClone); + } } diff --git a/plugins/catalog-backend/src/database/migrations/20200520140700_location_update_log_table.ts b/plugins/catalog-backend/src/database/migrations/20200520140700_location_update_log_table.ts index 594641ead7..b2e1dc0d32 100644 --- a/plugins/catalog-backend/src/database/migrations/20200520140700_location_update_log_table.ts +++ b/plugins/catalog-backend/src/database/migrations/20200520140700_location_update_log_table.ts @@ -27,7 +27,7 @@ export async function up(knex: Knex): Promise { .inTable('locations') .onUpdate('CASCADE') .onDelete('CASCADE'); - table.string('entity_name').notNullable(); + table.string('entity_name').nullable(); }); } diff --git a/plugins/catalog-backend/src/database/search.test.ts b/plugins/catalog-backend/src/database/search.test.ts index 7fe4928f51..26b20d1846 100644 --- a/plugins/catalog-backend/src/database/search.test.ts +++ b/plugins/catalog-backend/src/database/search.test.ts @@ -14,9 +14,9 @@ * limitations under the License. */ +import { DescriptorEnvelope } from '../ingestion'; import { buildEntitySearch, visitEntityPart } from './search'; import { DbEntitiesSearchRow } from './types'; -import { DescriptorEnvelope } from '../ingestion'; describe('search', () => { describe('visitEntityPart', () => { diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index 80f58aed6a..aa88d299e4 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -16,8 +16,7 @@ import { FileLocationSource } from './sources/FileLocationSource'; import { GitHubLocationSource } from './sources/GitHubLocationSource'; -import { LocationSource } from './sources/types'; -import { LocationReader, ReaderOutput } from './types'; +import { LocationReader, LocationSource, ReaderOutput } from './types'; export class LocationReaders implements LocationReader { static create(): LocationReader { diff --git a/plugins/catalog-backend/src/ingestion/descriptors/ComponentDescriptorV1beta1Parser.ts b/plugins/catalog-backend/src/ingestion/descriptors/ComponentDescriptorV1beta1Parser.ts index 8d756ac010..974b34aa8e 100644 --- a/plugins/catalog-backend/src/ingestion/descriptors/ComponentDescriptorV1beta1Parser.ts +++ b/plugins/catalog-backend/src/ingestion/descriptors/ComponentDescriptorV1beta1Parser.ts @@ -15,8 +15,7 @@ */ import * as yup from 'yup'; -import { DescriptorEnvelope, ParserError } from '../types'; -import { KindParser } from './types'; +import { DescriptorEnvelope, KindParser, ParserError } from '../types'; export interface ComponentDescriptorV1beta1 extends DescriptorEnvelope { spec: { diff --git a/plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts b/plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts index ccf5fc3051..1f21e2df9e 100644 --- a/plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts +++ b/plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts @@ -89,7 +89,7 @@ export class DescriptorEnvelopeParser { ); const labelsSchema = yup - .object() + .object>() .notRequired() .test({ name: 'metadata.labels.keys', @@ -113,7 +113,7 @@ export class DescriptorEnvelopeParser { }); const annotationsSchema = yup - .object() + .object>() .notRequired() .test({ name: 'metadata.annotations.keys', diff --git a/plugins/catalog-backend/src/ingestion/descriptors/types.ts b/plugins/catalog-backend/src/ingestion/descriptors/types.ts deleted file mode 100644 index 92b5fdab11..0000000000 --- a/plugins/catalog-backend/src/ingestion/descriptors/types.ts +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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. - */ - -import { DescriptorEnvelope } from '../types'; - -/** - * Parses and validates a single envelope into its materialized kind. - * - * These parsers may assume that the envelope is already validated and well - * formed. - */ -export type KindParser = { - /** - * Try to parse an envelope into a materialized kind. - * - * @param envelope A valid descriptor envelope - * @returns A materialized type, or undefined if the given version/kind is - * not meant to be handled by this parser - * @throws An Error if the type was handled and found to not be properly - * formatted - */ - tryParse( - envelope: DescriptorEnvelope, - ): Promise; -}; diff --git a/plugins/catalog-backend/src/ingestion/sources/FileLocationSource.ts b/plugins/catalog-backend/src/ingestion/sources/FileLocationSource.ts index 67d889e7e5..9d2794ec4f 100644 --- a/plugins/catalog-backend/src/ingestion/sources/FileLocationSource.ts +++ b/plugins/catalog-backend/src/ingestion/sources/FileLocationSource.ts @@ -15,8 +15,7 @@ */ import fs from 'fs-extra'; -import { ReaderOutput } from '../types'; -import { LocationSource } from './types'; +import { LocationSource, ReaderOutput } from '../types'; import { readDescriptorYaml } from './util'; export class FileLocationSource implements LocationSource { diff --git a/plugins/catalog-backend/src/ingestion/types.ts b/plugins/catalog-backend/src/ingestion/types.ts index 52817737ac..e5083a65f1 100644 --- a/plugins/catalog-backend/src/ingestion/types.ts +++ b/plugins/catalog-backend/src/ingestion/types.ts @@ -18,6 +18,70 @@ import { ComponentDescriptorV1beta1 } from './descriptors/ComponentDescriptorV1b export type ComponentDescriptor = ComponentDescriptorV1beta1; +/** + * Metadata fields common to all versions/kinds of entity. + * + * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta + */ +export type EntityMeta = { + /** + * A globally unique ID for the entity. + * + * This field can not be set by the user at creation time, and the server + * will reject an attempt to do so. The field will be populated in read + * operations. The field can (optionally) be specified when performing + * update or delete operations, but the server is free to reject requests + * that do so in such a way that it breaks semantics. + */ + uid?: string; + + /** + * An opaque string that changes for each update operation to any part of + * the entity, including metadata. + * + * This field can not be set by the user at creation time, and the server + * will reject an attempt to do so. The field will be populated in read + * operations. The field can (optionally) be specified when performing + * update or delete operations, and the server will then reject the + * operation if it does not match the current stored value. + */ + etag?: string; + + /** + * A positive nonzero number that indicates the current generation of data + * for this entity; the value is incremented each time the spec changes. + * + * This field can not be set by the user at creation time, and the server + * will reject an attempt to do so. The field will be populated in read + * operations. + */ + generation?: number; + + /** + * The name of the entity. + * + * Must be uniqe within the catalog at any given point in time, for any + * given namespace, for any given kind. + */ + name?: string; + + /** + * The namespace that the entity belongs to. + */ + namespace?: string; + + /** + * Key/value pairs of identifying information attached to the entity. + */ + labels?: Record; + + /** + * Key/value pairs of non-identifying auxiliary information attached to the + * entity. + */ + annotations?: Record; +}; + /** * The format envelope that's common to all versions/kinds. * @@ -37,67 +101,8 @@ export type DescriptorEnvelope = { /** * Optional metadata related to the entity. - * - * @see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#objectmeta-v1-meta */ - metadata?: { - /** - * A globally unique ID for the entity. - * - * This field can not be set by the user at creation time, and the server - * will reject an attempt to do so. The field will be populated in read - * operations. The field can (optionally) be specified when performing - * update or delete operations, but the server is free to reject requests - * that do so in such a way that it breaks semantics. - */ - uid?: string; - - /** - * An opaque string that changes for each update operation to any part of - * the entity, including metadata. - * - * This field can not be set by the user at creation time, and the server - * will reject an attempt to do so. The field will be populated in read - * operations. The field can (optionally) be specified when performing - * update or delete operations, and the server will then reject the - * operation if it does not match the current stored value. - */ - etag?: string; - - /** - * A positive nonzero number that indicates the current generation of data - * for this entity; the value is incremented each time the spec changes. - * - * This field can not be set by the user at creation time, and the server - * will reject an attempt to do so. The field will be populated in read - * operations. - */ - generation?: number; - - /** - * The name of the entity. - * - * Must be uniqe within the catalog at any given point in time, for any - * given namespace, for any given kind. - */ - name?: string; - - /** - * The namespace that the entity belongs to. - */ - namespace?: string; - - /** - * Key/value pairs of identifying information attached to the entity. - */ - labels?: object; - - /** - * Key/value pairs of non-identifying auxiliary information attached to the - * entity. - */ - annotations?: object; - }; + metadata?: EntityMeta; /** * The specification data describing the entity itself. diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 5bcf58b942..af714e4026 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -38,16 +38,10 @@ export async function createRouter( if (entitiesCatalog) { // Entities - router - .get('/entities', async (_req, res) => { - const entities = await entitiesCatalog.entities(); - res.status(200).send(entities); - }) - .get('/entities/:id', async (req, res) => { - const { id } = req.params; - const entity = await entitiesCatalog.entity(id); - res.status(200).send(entity); - }); + router.get('/entities', async (_req, res) => { + const entities = await entitiesCatalog.entities(); + res.status(200).send(entities); + }); } // Locations From 9a0885304eb54de144b613d58ad2ab9868038a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 25 May 2020 14:19:29 +0200 Subject: [PATCH 3/3] Unbreak build --- .../src/ingestion/sources/GitHubLocationSource.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/sources/GitHubLocationSource.ts b/plugins/catalog-backend/src/ingestion/sources/GitHubLocationSource.ts index ac0a2faa23..69ba4911a6 100644 --- a/plugins/catalog-backend/src/ingestion/sources/GitHubLocationSource.ts +++ b/plugins/catalog-backend/src/ingestion/sources/GitHubLocationSource.ts @@ -15,10 +15,9 @@ */ import fetch from 'node-fetch'; -import { ReaderOutput } from '../types'; -import { LocationSource } from './types'; -import { readDescriptorYaml } from './util'; import { URL } from 'url'; +import { LocationSource, ReaderOutput } from '../types'; +import { readDescriptorYaml } from './util'; // Pointing to raw.githubusercontent.com for now // to be changed in the future, after auth and tokens are done @@ -58,7 +57,7 @@ export class GitHubLocationSource implements LocationSource { let rawYaml; try { - rawYaml = await fetch(url.toString()).then((x) => { + rawYaml = await fetch(url.toString()).then(x => { return x.text(); }); } catch (e) {