From 8e8d1afe9cfa49d03b0c0b4309bd48cc7642a6ca Mon Sep 17 00:00:00 2001 From: Bret Hubbard Date: Thu, 10 Feb 2022 22:55:10 -0700 Subject: [PATCH 1/4] catalog-backend: add validate-entity endpoint Signed-off-by: Bret Hubbard --- .../src/service/CatalogBuilder.ts | 1 + .../src/service/createRouter.test.ts | 87 ++++++++++++++++++- .../src/service/createRouter.ts | 25 +++++- plugins/catalog-backend/src/service/util.ts | 21 +++++ 4 files changed, 130 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 99d7f0e41a..075fcd6a5d 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -438,6 +438,7 @@ export class CatalogBuilder { entitiesCatalog, locationAnalyzer, locationService, + orchestrator, refreshService, logger, config, diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 6def04a11c..06a5ba74e0 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -18,7 +18,11 @@ import { getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { NotFoundError } from '@backstage/errors'; import type { Location } from '@backstage/catalog-client'; -import type { Entity } from '@backstage/catalog-model'; +import { + ANNOTATION_ORIGIN_LOCATION, + ANNOTATION_SOURCE_LOCATION, + Entity, +} from '@backstage/catalog-model'; import express from 'express'; import request from 'supertest'; import { EntitiesCatalog } from '../catalog/types'; @@ -31,10 +35,12 @@ import { createPermissionRule, } from '@backstage/plugin-permission-node'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; +import { CatalogProcessingOrchestrator } from '../processing/types'; describe('createRouter readonly disabled', () => { let entitiesCatalog: jest.Mocked; let locationService: jest.Mocked; + let orchestrator: jest.Mocked; let app: express.Express; let refreshService: RefreshService; @@ -52,9 +58,11 @@ describe('createRouter readonly disabled', () => { deleteLocation: jest.fn(), }; refreshService = { refresh: jest.fn() }; + orchestrator = { process: jest.fn() }; const router = await createRouter({ entitiesCatalog, locationService, + orchestrator, logger: getVoidLogger(), refreshService, config: new ConfigReader(undefined), @@ -386,6 +394,83 @@ describe('createRouter readonly disabled', () => { expect(response.status).toEqual(204); }); }); + + describe('POST /validate-entity', () => { + describe('valid entity', () => { + it('returns 200', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + }; + + orchestrator.process.mockResolvedValueOnce({ + ok: true, + state: {}, + completedEntity: entity, + deferredEntities: [], + relations: [], + errors: [], + }); + + const response = await request(app) + .post('/validate-entity') + .send(entity); + + expect(response.status).toEqual(200); + expect(orchestrator.process).toHaveBeenCalledTimes(1); + expect(orchestrator.process).toHaveBeenCalledWith({ + entity: { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'n', + annotations: { + [ANNOTATION_SOURCE_LOCATION]: 'validate:entity', + [ANNOTATION_ORIGIN_LOCATION]: 'validate:entity', + }, + }, + }, + }); + }); + }); + + describe('invalid entity', () => { + it('returns 422', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'invalid*name' }, + }; + + orchestrator.process.mockResolvedValueOnce({ + ok: false, + errors: [new Error('Invalid entity name')], + }); + + const response = await request(app) + .post('/validate-entity') + .send(entity); + + expect(response.status).toEqual(422); + expect(response.body).toEqual(['Invalid entity name']); + expect(orchestrator.process).toHaveBeenCalledTimes(1); + expect(orchestrator.process).toHaveBeenCalledWith({ + entity: { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'invalid*name', + annotations: { + [ANNOTATION_SOURCE_LOCATION]: 'validate:entity', + [ANNOTATION_ORIGIN_LOCATION]: 'validate:entity', + }, + }, + }, + }); + }); + }); + }); }); describe('createRouter readonly enabled', () => { diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index a35309035f..221b5413c2 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -15,7 +15,7 @@ */ import { errorHandler } from '@backstage/backend-common'; -import { stringifyEntityRef } from '@backstage/catalog-model'; +import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { NotFoundError } from '@backstage/errors'; import express from 'express'; @@ -34,10 +34,13 @@ import { disallowReadonlyMode, locationInput, validateRequestBody, + requireRequestBody, + setRequiredEntityLocationAnnotations, } from './util'; -import { RefreshOptions, LocationService, RefreshService } from './types'; import { z } from 'zod'; import { parseEntityFacetParams } from './request/parseEntityFacetParams'; +import { RefreshOptions, LocationService, RefreshService } from './types'; +import { CatalogProcessingOrchestrator } from '../processing/types'; /** * Options used by {@link createRouter}. @@ -48,6 +51,7 @@ export interface RouterOptions { entitiesCatalog?: EntitiesCatalog; locationAnalyzer?: LocationAnalyzer; locationService: LocationService; + orchestrator?: CatalogProcessingOrchestrator; refreshService?: RefreshService; logger: Logger; config: Config; @@ -66,12 +70,12 @@ export async function createRouter( entitiesCatalog, locationAnalyzer, locationService, + orchestrator, refreshService, config, logger, permissionIntegrationRouter, } = options; - const router = Router(); router.use(express.json()); @@ -228,6 +232,21 @@ export async function createRouter( }); } + if (orchestrator) { + router.post('/validate-entity', async (req, res) => { + const input = (await requireRequestBody(req)) as Entity; + const entity = setRequiredEntityLocationAnnotations(input); + const processingResult = await orchestrator.process({ + entity, + }); + if (!processingResult.ok) + return res + .status(422) + .json(processingResult.errors.map(e => e.message)); + return res.status(200).send(); + }); + } + router.use(errorHandler()); return router; } diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 9a914aadcb..15ffdaa013 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -14,6 +14,11 @@ * limitations under the License. */ +import { + Entity, + ANNOTATION_SOURCE_LOCATION, + ANNOTATION_ORIGIN_LOCATION, +} from '@backstage/catalog-model'; import { InputError, NotAllowedError } from '@backstage/errors'; import { Request } from 'express'; import lodash from 'lodash'; @@ -48,6 +53,22 @@ export const locationInput = z }) .strict(); // no unknown keys; +export function setRequiredEntityLocationAnnotations(entity: Entity): Entity { + const clonedEntity = lodash.cloneDeep(entity); + lodash.set( + clonedEntity, + `metadata.annotations.['${ANNOTATION_SOURCE_LOCATION}']`, + 'validate:entity', + ); + lodash.set( + clonedEntity, + `metadata.annotations.['${ANNOTATION_ORIGIN_LOCATION}']`, + 'validate:entity', + ); + + return clonedEntity; +} + export async function validateRequestBody( req: Request, schema: z.Schema, From bf82edf4c953eef8ddb6f3a0e01bfb38d6ae26c9 Mon Sep 17 00:00:00 2001 From: Bret Hubbard Date: Thu, 10 Feb 2022 22:59:17 -0700 Subject: [PATCH 2/4] catalog-backend: add changeset Signed-off-by: Bret Hubbard --- .changeset/swift-trees-rest.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/swift-trees-rest.md diff --git a/.changeset/swift-trees-rest.md b/.changeset/swift-trees-rest.md new file mode 100644 index 0000000000..2e76195997 --- /dev/null +++ b/.changeset/swift-trees-rest.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Added `/validate-entity` endpoint From 0d5f9fe6e9359d85649aac772b3977a608daf4ff Mon Sep 17 00:00:00 2001 From: Bret Hubbard Date: Mon, 21 Mar 2022 17:43:52 -0600 Subject: [PATCH 3/4] catalog-backend: add location for validate-entity endpoint Signed-off-by: Bret Hubbard --- .../src/service/createRouter.test.ts | 52 +++++++++++++++++-- .../src/service/createRouter.ts | 20 ++++--- plugins/catalog-backend/src/service/util.ts | 9 ++-- 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 06a5ba74e0..29c551db06 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -415,7 +415,7 @@ describe('createRouter readonly disabled', () => { const response = await request(app) .post('/validate-entity') - .send(entity); + .send({ entity, location: 'validate:entity' }); expect(response.status).toEqual(200); expect(orchestrator.process).toHaveBeenCalledTimes(1); @@ -436,7 +436,7 @@ describe('createRouter readonly disabled', () => { }); describe('invalid entity', () => { - it('returns 422', async () => { + it('returns 400', async () => { const entity: Entity = { apiVersion: 'a', kind: 'b', @@ -450,10 +450,10 @@ describe('createRouter readonly disabled', () => { const response = await request(app) .post('/validate-entity') - .send(entity); + .send({ entity, location: 'validate:entity' }); - expect(response.status).toEqual(422); - expect(response.body).toEqual(['Invalid entity name']); + expect(response.status).toEqual(400); + expect(response.body.error.message).toContain('Invalid entity name'); expect(orchestrator.process).toHaveBeenCalledTimes(1); expect(orchestrator.process).toHaveBeenCalledWith({ entity: { @@ -470,6 +470,48 @@ describe('createRouter readonly disabled', () => { }); }); }); + + describe('no location', () => { + it('returns 400', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + }; + + const response = await request(app) + .post('/validate-entity') + .send({ entity, location: null }); + + expect(response.status).toEqual(400); + expect(response.body.error.message).toContain('Malformed request:'); + expect(orchestrator.process).toHaveBeenCalledTimes(0); + }); + }); + + describe('no entity', () => { + it('returns 400', async () => { + orchestrator.process.mockResolvedValueOnce({ + ok: false, + errors: [ + new Error( + 'Entity envelope failed validation before processing; caused by TypeError: must be object - type: object', + ), + ], + }); + + const response = await request(app) + .post('/validate-entity') + .send({ entity: null, location: 'validate:entity' }); + + expect(response.status).toEqual(400); + expect(response.body.error.message).toContain( + 'Entity envelope failed validation', + ); + // We expect this to be called because of an open issue with zod where unknown values are optional https://github.com/colinhacks/zod/issues/493 + expect(orchestrator.process).toHaveBeenCalledTimes(1); + }); + }); }); }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 221b5413c2..0bcdd6e6d6 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -17,7 +17,7 @@ import { errorHandler } from '@backstage/backend-common'; import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { NotFoundError } from '@backstage/errors'; +import { InputError, NotFoundError } from '@backstage/errors'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; @@ -34,7 +34,6 @@ import { disallowReadonlyMode, locationInput, validateRequestBody, - requireRequestBody, setRequiredEntityLocationAnnotations, } from './util'; import { z } from 'zod'; @@ -234,15 +233,22 @@ export async function createRouter( if (orchestrator) { router.post('/validate-entity', async (req, res) => { - const input = (await requireRequestBody(req)) as Entity; - const entity = setRequiredEntityLocationAnnotations(input); + const bodySchema = z.object({ + entity: z.unknown(), + location: z.string(), + }); + + const body = await validateRequestBody(req, bodySchema); + + const entity = setRequiredEntityLocationAnnotations( + body.entity as Entity, + body.location, + ); const processingResult = await orchestrator.process({ entity, }); if (!processingResult.ok) - return res - .status(422) - .json(processingResult.errors.map(e => e.message)); + throw new InputError(processingResult.errors[0].message); return res.status(200).send(); }); } diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 15ffdaa013..7bf6cd8619 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -53,17 +53,20 @@ export const locationInput = z }) .strict(); // no unknown keys; -export function setRequiredEntityLocationAnnotations(entity: Entity): Entity { +export function setRequiredEntityLocationAnnotations( + entity: Entity, + location: string, +): Entity { const clonedEntity = lodash.cloneDeep(entity); lodash.set( clonedEntity, `metadata.annotations.['${ANNOTATION_SOURCE_LOCATION}']`, - 'validate:entity', + location, ); lodash.set( clonedEntity, `metadata.annotations.['${ANNOTATION_ORIGIN_LOCATION}']`, - 'validate:entity', + location, ); return clonedEntity; From ac9fe6357e222f8869342d2e8dec98e10b8ff94a Mon Sep 17 00:00:00 2001 From: Bret Hubbard Date: Mon, 28 Mar 2022 13:48:13 -0600 Subject: [PATCH 4/4] catalog-backend: add pre-validation and consistent errors, refactor for simplicity Signed-off-by: Bret Hubbard --- .../src/service/createRouter.test.ts | 38 ++++++-------- .../src/service/createRouter.ts | 52 +++++++++++++++---- plugins/catalog-backend/src/service/util.ts | 24 --------- 3 files changed, 56 insertions(+), 58 deletions(-) diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 29c551db06..8eafd8bfa3 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -19,8 +19,8 @@ import { ConfigReader } from '@backstage/config'; import { NotFoundError } from '@backstage/errors'; import type { Location } from '@backstage/catalog-client'; import { + ANNOTATION_LOCATION, ANNOTATION_ORIGIN_LOCATION, - ANNOTATION_SOURCE_LOCATION, Entity, } from '@backstage/catalog-model'; import express from 'express'; @@ -415,7 +415,7 @@ describe('createRouter readonly disabled', () => { const response = await request(app) .post('/validate-entity') - .send({ entity, location: 'validate:entity' }); + .send({ entity, location: 'url:validate-entity' }); expect(response.status).toEqual(200); expect(orchestrator.process).toHaveBeenCalledTimes(1); @@ -426,8 +426,8 @@ describe('createRouter readonly disabled', () => { metadata: { name: 'n', annotations: { - [ANNOTATION_SOURCE_LOCATION]: 'validate:entity', - [ANNOTATION_ORIGIN_LOCATION]: 'validate:entity', + [ANNOTATION_LOCATION]: 'url:validate-entity', + [ANNOTATION_ORIGIN_LOCATION]: 'url:validate-entity', }, }, }, @@ -450,10 +450,11 @@ describe('createRouter readonly disabled', () => { const response = await request(app) .post('/validate-entity') - .send({ entity, location: 'validate:entity' }); + .send({ entity, location: 'url:validate-entity' }); expect(response.status).toEqual(400); - expect(response.body.error.message).toContain('Invalid entity name'); + expect(response.body.errors.length).toEqual(1); + expect(response.body.errors[0].message).toEqual('Invalid entity name'); expect(orchestrator.process).toHaveBeenCalledTimes(1); expect(orchestrator.process).toHaveBeenCalledWith({ entity: { @@ -462,8 +463,8 @@ describe('createRouter readonly disabled', () => { metadata: { name: 'invalid*name', annotations: { - [ANNOTATION_SOURCE_LOCATION]: 'validate:entity', - [ANNOTATION_ORIGIN_LOCATION]: 'validate:entity', + [ANNOTATION_LOCATION]: 'url:validate-entity', + [ANNOTATION_ORIGIN_LOCATION]: 'url:validate-entity', }, }, }, @@ -484,32 +485,23 @@ describe('createRouter readonly disabled', () => { .send({ entity, location: null }); expect(response.status).toEqual(400); - expect(response.body.error.message).toContain('Malformed request:'); + expect(response.body.errors.length).toEqual(1); + expect(response.body.errors[0].message).toContain('Malformed request:'); expect(orchestrator.process).toHaveBeenCalledTimes(0); }); }); describe('no entity', () => { it('returns 400', async () => { - orchestrator.process.mockResolvedValueOnce({ - ok: false, - errors: [ - new Error( - 'Entity envelope failed validation before processing; caused by TypeError: must be object - type: object', - ), - ], - }); - const response = await request(app) .post('/validate-entity') - .send({ entity: null, location: 'validate:entity' }); + .send({ entity: null, location: 'url:entity' }); expect(response.status).toEqual(400); - expect(response.body.error.message).toContain( - 'Entity envelope failed validation', + expect(response.body.errors.length).toEqual(1); + expect(response.body.errors[0].message).toContain( + ' must be object - type: object', ); - // We expect this to be called because of an open issue with zod where unknown values are optional https://github.com/colinhacks/zod/issues/493 - expect(orchestrator.process).toHaveBeenCalledTimes(1); }); }); }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 0bcdd6e6d6..639893dcc1 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -15,9 +15,15 @@ */ import { errorHandler } from '@backstage/backend-common'; -import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; +import { + ANNOTATION_LOCATION, + ANNOTATION_ORIGIN_LOCATION, + Entity, + parseLocationRef, + stringifyEntityRef, +} from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { InputError, NotFoundError } from '@backstage/errors'; +import { NotFoundError, serializeError } from '@backstage/errors'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; @@ -34,12 +40,12 @@ import { disallowReadonlyMode, locationInput, validateRequestBody, - setRequiredEntityLocationAnnotations, } from './util'; import { z } from 'zod'; import { parseEntityFacetParams } from './request/parseEntityFacetParams'; import { RefreshOptions, LocationService, RefreshService } from './types'; import { CatalogProcessingOrchestrator } from '../processing/types'; +import { validateEntityEnvelope } from '../processing/util'; /** * Options used by {@link createRouter}. @@ -238,18 +244,42 @@ export async function createRouter( location: z.string(), }); - const body = await validateRequestBody(req, bodySchema); + let body: z.infer; + let entity: Entity; + let location: { type: string; target: string }; + try { + body = await validateRequestBody(req, bodySchema); + entity = validateEntityEnvelope(body.entity); + location = parseLocationRef(body.location); + if (location.type !== 'url') + throw new TypeError( + `Invalid location ref ${body.location}, only 'url:' is supported, e.g. url:https://host/path`, + ); + } catch (err) { + return res.status(400).json({ + errors: [serializeError(err)], + }); + } - const entity = setRequiredEntityLocationAnnotations( - body.entity as Entity, - body.location, - ); const processingResult = await orchestrator.process({ - entity, + entity: { + ...entity, + metadata: { + ...entity.metadata, + annotations: { + [ANNOTATION_LOCATION]: body.location, + [ANNOTATION_ORIGIN_LOCATION]: body.location, + ...entity.metadata.annotations, + }, + }, + }, }); + if (!processingResult.ok) - throw new InputError(processingResult.errors[0].message); - return res.status(200).send(); + res.status(400).json({ + errors: processingResult.errors.map(e => serializeError(e)), + }); + return res.status(200).end(); }); } diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 7bf6cd8619..9a914aadcb 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -14,11 +14,6 @@ * limitations under the License. */ -import { - Entity, - ANNOTATION_SOURCE_LOCATION, - ANNOTATION_ORIGIN_LOCATION, -} from '@backstage/catalog-model'; import { InputError, NotAllowedError } from '@backstage/errors'; import { Request } from 'express'; import lodash from 'lodash'; @@ -53,25 +48,6 @@ export const locationInput = z }) .strict(); // no unknown keys; -export function setRequiredEntityLocationAnnotations( - entity: Entity, - location: string, -): Entity { - const clonedEntity = lodash.cloneDeep(entity); - lodash.set( - clonedEntity, - `metadata.annotations.['${ANNOTATION_SOURCE_LOCATION}']`, - location, - ); - lodash.set( - clonedEntity, - `metadata.annotations.['${ANNOTATION_ORIGIN_LOCATION}']`, - location, - ); - - return clonedEntity; -} - export async function validateRequestBody( req: Request, schema: z.Schema,