From 7eb4ec9919f62b12f5dc6fb8fdead693d1a790b1 Mon Sep 17 00:00:00 2001 From: Eric Nilsson Date: Wed, 14 Oct 2020 10:24:31 +0200 Subject: [PATCH 1/4] TechDocsPageHeader: Changed repo location icon --- plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx index c59cce2b11..a7db342544 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import GitHubIcon from '@material-ui/icons/GitHub'; +import CodeIcon from '@material-ui/icons/Code'; import { Header, HeaderLabel, Link } from '@backstage/core'; import { CircularProgress } from '@material-ui/core'; import { ParsedEntityId } from '../../types'; @@ -73,7 +73,7 @@ export const TechDocsPageHeader = ({ target="_blank" rel="noopener noreferrer" > - + } /> From dab96d0c689f563af25e068b20e04bbcac4a3b9f Mon Sep 17 00:00:00 2001 From: Eric Nilsson Date: Thu, 22 Oct 2020 12:18:25 +0200 Subject: [PATCH 2/4] Made validation messages more descriptive --- .../policies/FieldFormatEntityPolicy.ts | 32 ++++++++++++++++++- .../src/ingestion/HigherOrderOperations.ts | 5 +-- .../src/ingestion/LocationReaders.ts | 4 +-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts index 20a3724079..724de8271d 100644 --- a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts @@ -50,7 +50,37 @@ export class FieldFormatEntityPolicy implements EntityPolicy { } if (!isValid) { - throw new Error(`${field} "${value}" is not valid`); + let expectation; + switch (validator.name) { + case 'isValidLabelValue': + expectation = + 'a string that is sequences of [a-zA-Z0-9] separated by any of [-_.], at most 63 characters in total'; + break; + case 'isValidLabelKey': + case 'isValidApiVersion': + case 'isValidAnnotationKey': + expectation = 'a valid prefix and/or suffix'; + break; + case 'isValidAnnotationValue': + expectation = 'a string'; + break; + case 'isValidNamespace': + expectation = + 'a string that is sequences of [a-zA-Z0-9] seperated by [-], at most 63 characters in total'; + break; + default: + expectation = undefined; + break; + } + + // ensure that if there are other/future validators, the error message defaults to a general "is not valid, visit link" + const message = expectation + ? ` expected ${expectation} but found "${value}".` + : ''; + + throw new Error( + `"${field}" is not valid;${message} To learn more about catalog file format, visit: https://github.com/spotify/backstage/blob/master/docs/architecture-decisions/adr002-default-catalog-file-format.md`, + ); } } diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 6df6d9b4b7..a644dd48aa 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { InputError } from '@backstage/backend-common'; import { Entity, Location, LocationSpec } from '@backstage/catalog-model'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; @@ -81,9 +80,7 @@ export class HigherOrderOperations implements HigherOrderOperation { const readerOutput = await this.locationReader.read(spec); if (readerOutput.errors.length) { const item = readerOutput.errors[0]; - throw new InputError( - `Failed to read location ${item.location.type}:${item.location.target}, ${item.error}`, - ); + throw item.error; } // TODO(freben): At this point, we could detect orphaned entities, by way diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index e23a4524b6..8fec1541df 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -189,7 +189,7 @@ export class LocationReaders implements LocationReader { ); } catch (e) { const message = `Processor ${processor.constructor.name} threw an error while preprocessing entity ${kind}:${namespace}/${name} at ${item.location.type} ${item.location.target}, ${e}`; - emit(result.generalError(item.location, message)); + emit(result.generalError(item.location, e.message)); logger.warn(message); return undefined; } @@ -207,7 +207,7 @@ export class LocationReaders implements LocationReader { current = next; } catch (e) { const message = `Policy check failed while analyzing entity ${kind}:${namespace}/${name} at ${item.location.type} ${item.location.target}, ${e}`; - emit(result.inputError(item.location, message)); + emit(result.inputError(item.location, e.message)); logger.warn(message); return undefined; } From 7dfc8d3eb26fc4fa7c1f2be6e9ced1d88c1165aa Mon Sep 17 00:00:00 2001 From: Eric Nilsson Date: Thu, 22 Oct 2020 13:38:31 +0200 Subject: [PATCH 3/4] prettified a null-check --- .../src/entity/policies/FieldFormatEntityPolicy.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts index 724de8271d..896d5b8999 100644 --- a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts @@ -53,6 +53,7 @@ export class FieldFormatEntityPolicy implements EntityPolicy { let expectation; switch (validator.name) { case 'isValidLabelValue': + case 'isValidObjectName': expectation = 'a string that is sequences of [a-zA-Z0-9] separated by any of [-_.], at most 63 characters in total'; break; @@ -61,12 +62,17 @@ export class FieldFormatEntityPolicy implements EntityPolicy { case 'isValidAnnotationKey': expectation = 'a valid prefix and/or suffix'; break; + case 'isValidNamespace': + case 'isValidDnsLabel': + expectation = + 'a string that is sequences of [a-zA-Z0-9] seperated by [-], at most 63 characters in total'; + break; case 'isValidAnnotationValue': expectation = 'a string'; break; - case 'isValidNamespace': + case 'isValidKind': expectation = - 'a string that is sequences of [a-zA-Z0-9] seperated by [-], at most 63 characters in total'; + 'a string that is a sequence of [a-zA-Z][a-z0-9A-Z], at most 63 characters in total'; break; default: expectation = undefined; From e63631e7329008a9b4d31a84f995145c2fe1dc49 Mon Sep 17 00:00:00 2001 From: Eric Nilsson Date: Thu, 22 Oct 2020 21:59:07 +0200 Subject: [PATCH 4/4] spelling mistake and type safety --- .../entity/policies/FieldFormatEntityPolicy.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts index 896d5b8999..7193c1a89b 100644 --- a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts @@ -15,7 +15,12 @@ */ import { EntityPolicy } from '../../types'; -import { makeValidator, Validators } from '../../validation'; +import { + CommonValidatorFunctions, + KubernetesValidatorFunctions, + makeValidator, + Validators, +} from '../../validation'; import { Entity } from '../Entity'; /** @@ -51,7 +56,11 @@ export class FieldFormatEntityPolicy implements EntityPolicy { if (!isValid) { let expectation; - switch (validator.name) { + switch ( + validator.name as + | keyof typeof KubernetesValidatorFunctions + | keyof typeof CommonValidatorFunctions + ) { case 'isValidLabelValue': case 'isValidObjectName': expectation = @@ -65,7 +74,7 @@ export class FieldFormatEntityPolicy implements EntityPolicy { case 'isValidNamespace': case 'isValidDnsLabel': expectation = - 'a string that is sequences of [a-zA-Z0-9] seperated by [-], at most 63 characters in total'; + 'a string that is sequences of [a-zA-Z0-9] separated by [-], at most 63 characters in total'; break; case 'isValidAnnotationValue': expectation = 'a string';