diff --git a/packages/catalog-model/src/entity/Entity.ts b/packages/catalog-model/src/entity/Entity.ts index 80c5765ee1..70d2ba4d2b 100644 --- a/packages/catalog-model/src/entity/Entity.ts +++ b/packages/catalog-model/src/entity/Entity.ts @@ -95,6 +95,12 @@ export type EntityMeta = { */ namespace?: string; + /** + * A short (typically relatively few words, on one line) description of the + * entity. + */ + description?: string; + /** * Key/value pairs of identifying information attached to the entity. */ diff --git a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.test.ts b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.test.ts index 8db33955a7..5bc39f4e93 100644 --- a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.test.ts +++ b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.test.ts @@ -61,4 +61,9 @@ describe('ReservedFieldsEntityPolicy', () => { /annotation.*apiVersion/i, ); }); + + it('rejects core fields mistakenly placed in metadata', async () => { + data.metadata.owner = 'emma'; + await expect(policy.enforce(data)).rejects.toThrow(/owner/i); + }); }); diff --git a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts index 57029ffa24..a258e9a16a 100644 --- a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts @@ -17,47 +17,70 @@ import { EntityPolicy } from '../../types'; import { Entity } from '../Entity'; -const DEFAULT_RESERVED_ENTITY_FIELDS = [ - 'apiVersion', - 'kind', - 'uid', - 'etag', - 'generation', - 'name', - 'namespace', - 'labels', - 'annotations', - 'spec', -]; +type ExceptWhere = 'metadata' | 'spec' | 'labels' | 'annotations'; +type ReservedFields = Record; + +const DEFAULT_RESERVED_ENTITY_FIELDS: ReservedFields = { + apiVersion: [], + kind: [], + spec: [], + uid: ['metadata'], + etag: ['metadata'], + generation: ['metadata'], + name: ['metadata'], + namespace: ['metadata'], + description: ['metadata'], + labels: ['metadata'], + annotations: ['metadata'], + // The below items are known to appear in core kinds, and therefore should + // not be appearing in metadata (which would indicate that the user made a + // mistake in where to place them). + lifecycle: ['spec'], + owner: ['spec'], +}; /** * Ensures that fields are not given certain reserved names. */ export class ReservedFieldsEntityPolicy implements EntityPolicy { - private readonly reservedFields: string[]; + private readonly reservedFields: ReservedFields; - constructor(fields?: string[]) { - this.reservedFields = [ - ...(fields ?? []), + constructor(fields?: ReservedFields) { + this.reservedFields = { + ...(fields ?? {}), ...DEFAULT_RESERVED_ENTITY_FIELDS, - ]; + }; } async enforce(entity: Entity): Promise { - for (const field of this.reservedFields) { - if (entity.spec?.hasOwnProperty(field)) { + for (const [name, exceptWhere] of Object.entries(this.reservedFields)) { + if ( + !exceptWhere.includes('metadata') && + entity.metadata.hasOwnProperty(name) + ) { throw new Error( - `The spec may not contain the field ${field}, because it has reserved meaning`, + `The metadata may not contain the field ${name}, because it has reserved meaning`, ); } - if (entity.metadata.labels?.hasOwnProperty(field)) { + if (!exceptWhere.includes('spec') && entity.spec?.hasOwnProperty(name)) { throw new Error( - `A label may not have the field ${field}, because it has reserved meaning`, + `The spec may not contain the field ${name}, because it has reserved meaning`, ); } - if (entity.metadata.annotations?.hasOwnProperty(field)) { + if ( + !exceptWhere.includes('labels') && + entity.metadata.labels?.hasOwnProperty(name) + ) { throw new Error( - `An annotation may not have the field ${field}, because it has reserved meaning`, + `A label may not have the field ${name}, because it has reserved meaning`, + ); + } + if ( + !exceptWhere.includes('annotations') && + entity.metadata.annotations?.hasOwnProperty(name) + ) { + throw new Error( + `An annotation may not have the field ${name}, because it has reserved meaning`, ); } } diff --git a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts index c53fbf3e46..9a3c02d85a 100644 --- a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts +++ b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts @@ -100,6 +100,11 @@ describe('SchemaValidEntityPolicy', () => { await expect(policy.enforce(data)).rejects.toThrow(/uid/); }); + it('rejects empty uid', async () => { + data.metadata.uid = ''; + await expect(policy.enforce(data)).rejects.toThrow(/uid/); + }); + it('accepts missing etag', async () => { delete data.metadata.etag; await expect(policy.enforce(data)).resolves.toBe(data); @@ -110,6 +115,11 @@ describe('SchemaValidEntityPolicy', () => { await expect(policy.enforce(data)).rejects.toThrow(/etag/); }); + it('rejects empty etag', async () => { + data.metadata.etag = ''; + await expect(policy.enforce(data)).rejects.toThrow(/etag/); + }); + it('accepts missing generation', async () => { delete data.metadata.generation; await expect(policy.enforce(data)).resolves.toBe(data); @@ -120,6 +130,16 @@ describe('SchemaValidEntityPolicy', () => { await expect(policy.enforce(data)).rejects.toThrow(/generation/); }); + it('rejects zero generation', async () => { + data.metadata.generation = 0; + await expect(policy.enforce(data)).rejects.toThrow(/generation/); + }); + + it('rejects non-integer generation', async () => { + data.metadata.generation = 1.5; + await expect(policy.enforce(data)).rejects.toThrow(/generation/); + }); + it('rejects missing name', async () => { delete data.metadata.name; await expect(policy.enforce(data)).rejects.toThrow(/name/); @@ -140,6 +160,16 @@ describe('SchemaValidEntityPolicy', () => { await expect(policy.enforce(data)).rejects.toThrow(/namespace/); }); + it('accepts missing description', async () => { + delete data.metadata.description; + await expect(policy.enforce(data)).resolves.toBe(data); + }); + + it('rejects bad description type', async () => { + data.metadata.description = 7; + await expect(policy.enforce(data)).rejects.toThrow(/description/); + }); + it('accepts missing labels', async () => { delete data.metadata.labels; await expect(policy.enforce(data)).resolves.toBe(data); diff --git a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts index d367022ff1..e901e82d8b 100644 --- a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts @@ -23,32 +23,12 @@ const DEFAULT_ENTITY_SCHEMA = yup.object({ kind: yup.string().required(), metadata: yup .object({ - uid: yup - .string() - .notRequired() - .test( - 'metadata.uid', - 'The uid must not be empty', - value => value === undefined || value.length > 0, - ), - etag: yup - .string() - .notRequired() - .test( - 'metadata.etag', - 'The etag must not be empty', - value => value === undefined || value.length > 0, - ), - generation: yup - .number() - .notRequired() - .test( - 'metadata.generation', - 'The generation must be an integer greater than zero', - value => value === undefined || (value === (value | 0) && value > 0), - ), + uid: yup.string().notRequired().min(1), + etag: yup.string().notRequired().min(1), + generation: yup.number().notRequired().integer().min(1), name: yup.string().required(), namespace: yup.string().notRequired(), + description: yup.string().notRequired(), labels: yup.object>().notRequired(), annotations: yup.object>().notRequired(), }) diff --git a/packages/catalog-model/src/kinds/ComponentEntityV1beta1.test.ts b/packages/catalog-model/src/kinds/ComponentEntityV1beta1.test.ts new file mode 100644 index 0000000000..a7988047b7 --- /dev/null +++ b/packages/catalog-model/src/kinds/ComponentEntityV1beta1.test.ts @@ -0,0 +1,101 @@ +/* + * 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 { EntityPolicy } from '../types'; +import { + ComponentEntityV1beta1, + ComponentEntityV1beta1Policy, +} from './ComponentEntityV1beta1'; + +describe('ComponentV1beta1Policy', () => { + let entity: ComponentEntityV1beta1; + let policy: EntityPolicy; + + beforeEach(() => { + entity = { + apiVersion: 'backstage.io/v1beta1', + kind: 'Component', + metadata: { + name: 'test', + }, + spec: { + type: 'service', + lifecycle: 'production', + owner: 'me', + }, + }; + policy = new ComponentEntityV1beta1Policy(); + }); + + it('happy path: accepts valid data', async () => { + await expect(policy.enforce(entity)).resolves.toBe(entity); + }); + + it('rejects unknown apiVersion', async () => { + (entity as any).apiVersion = 'backstage.io/v1beta2'; + await expect(policy.enforce(entity)).rejects.toThrow(/apiVersion/); + }); + + it('rejects unknown kind', async () => { + (entity as any).kind = 'Wizard'; + await expect(policy.enforce(entity)).rejects.toThrow(/kind/); + }); + + it('rejects missing type', async () => { + delete (entity as any).spec.type; + await expect(policy.enforce(entity)).rejects.toThrow(/type/); + }); + + it('rejects wrong type', async () => { + (entity as any).spec.type = 7; + await expect(policy.enforce(entity)).rejects.toThrow(/type/); + }); + + it('rejects empty wrong type', async () => { + (entity as any).spec.type = ''; + await expect(policy.enforce(entity)).rejects.toThrow(/type/); + }); + + it('rejects missing lifecycle', async () => { + delete (entity as any).spec.lifecycle; + await expect(policy.enforce(entity)).rejects.toThrow(/lifecycle/); + }); + + it('rejects wrong lifecycle', async () => { + (entity as any).spec.lifecycle = 7; + await expect(policy.enforce(entity)).rejects.toThrow(/lifecycle/); + }); + + it('rejects wrong empty', async () => { + (entity as any).spec.lifecycle = ''; + await expect(policy.enforce(entity)).rejects.toThrow(/lifecycle/); + }); + + it('rejects missing owner', async () => { + delete (entity as any).spec.owner; + await expect(policy.enforce(entity)).rejects.toThrow(/owner/); + }); + + it('rejects wrong owner', async () => { + (entity as any).spec.owner = 7; + await expect(policy.enforce(entity)).rejects.toThrow(/owner/); + }); + + it('rejects empty owner', async () => { + (entity as any).spec.owner = ''; + await expect(policy.enforce(entity)).rejects.toThrow(/owner/); + }); +}); diff --git a/packages/catalog-model/src/kinds/ComponentEntityV1beta1.ts b/packages/catalog-model/src/kinds/ComponentEntityV1beta1.ts index 2f5e8e079c..02bcffbd28 100644 --- a/packages/catalog-model/src/kinds/ComponentEntityV1beta1.ts +++ b/packages/catalog-model/src/kinds/ComponentEntityV1beta1.ts @@ -26,6 +26,8 @@ export interface ComponentEntityV1beta1 extends Entity { kind: typeof KIND; spec: { type: string; + lifecycle: string; + owner: string; }; } @@ -36,7 +38,9 @@ export class ComponentEntityV1beta1Policy implements EntityPolicy { this.schema = yup.object>({ spec: yup .object({ - type: yup.string().required(), + type: yup.string().required().min(1), + lifecycle: yup.string().required().min(1), + owner: yup.string().required().min(1), }) .required(), });