diff --git a/packages/catalog-model/src/entity/Entity.ts b/packages/catalog-model/src/entity/Entity.ts index 6f57626749..80c5765ee1 100644 --- a/packages/catalog-model/src/entity/Entity.ts +++ b/packages/catalog-model/src/entity/Entity.ts @@ -32,9 +32,9 @@ export type Entity = { kind: string; /** - * Optional metadata related to the entity. + * Metadata related to the entity. */ - metadata?: EntityMeta; + metadata: EntityMeta; /** * The specification data describing the entity itself. @@ -86,9 +86,9 @@ export type EntityMeta = { * 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. + * given namespace + kind pair. */ - name?: string; + name: string; /** * The namespace that the entity belongs to. diff --git a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts index 697e45b371..4ccd8ec711 100644 --- a/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts @@ -65,23 +65,20 @@ export class FieldFormatEntityPolicy implements EntityPolicy { require('apiVersion', entity.apiVersion, this.validators.isValidApiVersion); require('kind', entity.kind, this.validators.isValidKind); - optional( - 'metadata.name', - entity.metadata?.name, - this.validators.isValidEntityName, - ); + require('metadata.name', entity.metadata.name, this.validators + .isValidEntityName); optional( 'metadata.namespace', - entity.metadata?.namespace, + entity.metadata.namespace, this.validators.isValidNamespace, ); - for (const [k, v] of Object.entries(entity.metadata?.labels ?? [])) { + for (const [k, v] of Object.entries(entity.metadata.labels ?? [])) { require(`labels.${k}`, k, this.validators.isValidLabelKey); require(`labels.${k}`, v, this.validators.isValidLabelValue); } - for (const [k, v] of Object.entries(entity.metadata?.annotations ?? [])) { + for (const [k, v] of Object.entries(entity.metadata.annotations ?? [])) { require(`annotations.${k}`, k, this.validators.isValidAnnotationKey); require(`annotations.${k}`, v, this.validators.isValidAnnotationValue); } diff --git a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts index d97469eecc..57029ffa24 100644 --- a/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts @@ -50,12 +50,12 @@ export class ReservedFieldsEntityPolicy implements EntityPolicy { `The spec may not contain the field ${field}, because it has reserved meaning`, ); } - if (entity.metadata?.labels?.hasOwnProperty(field)) { + if (entity.metadata.labels?.hasOwnProperty(field)) { throw new Error( `A label may not have the field ${field}, because it has reserved meaning`, ); } - if (entity.metadata?.annotations?.hasOwnProperty(field)) { + if (entity.metadata.annotations?.hasOwnProperty(field)) { throw new Error( `An annotation may not have the field ${field}, 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 0837b75759..c53fbf3e46 100644 --- a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts +++ b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts @@ -80,9 +80,9 @@ describe('SchemaValidEntityPolicy', () => { // metadata // - it('accepts missing metadata', async () => { + it('rejects missing metadata', async () => { delete data.metadata; - await expect(policy.enforce(data)).resolves.toBe(data); + await expect(policy.enforce(data)).rejects.toThrow(/metadata/); }); it('rejects bad metadata type', async () => { @@ -120,9 +120,9 @@ describe('SchemaValidEntityPolicy', () => { await expect(policy.enforce(data)).rejects.toThrow(/generation/); }); - it('accepts missing name', async () => { + it('rejects missing name', async () => { delete data.metadata.name; - await expect(policy.enforce(data)).resolves.toBe(data); + await expect(policy.enforce(data)).rejects.toThrow(/name/); }); it('rejects bad name type', async () => { diff --git a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts index 3ae20a5094..d367022ff1 100644 --- a/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts +++ b/packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts @@ -47,12 +47,12 @@ const DEFAULT_ENTITY_SCHEMA = yup.object({ 'The generation must be an integer greater than zero', value => value === undefined || (value === (value | 0) && value > 0), ), - name: yup.string().notRequired(), + name: yup.string().required(), namespace: yup.string().notRequired(), labels: yup.object>().notRequired(), annotations: yup.object>().notRequired(), }) - .notRequired(), + .required(), spec: yup.object({}).notRequired(), }); diff --git a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts index 1de606d44d..64ac5f57d5 100644 --- a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts @@ -31,7 +31,7 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { } async entityByUid(uid: string): Promise { - const item = this._entities.find(e => uid === e.metadata?.uid); + const item = this._entities.find(e => uid === e.metadata.uid); if (!item) { throw new NotFoundError('Entity cannot be found'); } @@ -46,8 +46,8 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { const item = this._entities.find( e => kind === e.kind && - name === e.metadata?.name && - namespace === e.metadata?.namespace, + name === e.metadata.name && + namespace === e.metadata.namespace, ); if (!item) { throw new NotFoundError('Entity cannot be found'); diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/Database.test.ts index 7a38adf02f..bb1a66ac7c 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/Database.test.ts @@ -128,7 +128,7 @@ describe('Database', () => { catalog.addEntity(tx, entityRequest), ); expect(added).toStrictEqual(entityResponse); - expect(added.entity.metadata!.generation).toBe(1); + expect(added.entity.metadata.generation).toBe(1); }); it('rejects adding the same-named entity twice', async () => { @@ -141,9 +141,9 @@ describe('Database', () => { it('accepts adding the same-named entity twice if on different namespaces', async () => { const catalog = new Database(database, getVoidLogger()); - entityRequest.entity.metadata!.namespace = 'namespace1'; + entityRequest.entity.metadata.namespace = 'namespace1'; await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); - entityRequest.entity.metadata!.namespace = 'namespace2'; + entityRequest.entity.metadata.namespace = 'namespace2'; await expect( catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), ).resolves.toBeDefined(); @@ -161,17 +161,15 @@ describe('Database', () => { ); 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.etag).not.toEqual( + added.entity.metadata.etag, ); - expect(updated.entity.metadata!.generation).toEqual( - added.entity.metadata!.generation, + 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, + expect(updated.entity.metadata.name).toEqual(added.entity.metadata.name); + expect(updated.entity.metadata.namespace).toEqual( + added.entity.metadata.namespace, ); }); @@ -180,11 +178,11 @@ describe('Database', () => { const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); - added.entity.metadata!.name! = 'new!'; + added.entity.metadata.name! = 'new!'; const updated = await catalog.transaction(tx => catalog.updateEntity(tx, { entity: added.entity }), ); - expect(updated.entity.metadata!.name).toEqual('new!'); + expect(updated.entity.metadata.name).toEqual('new!'); }); it('can update fields if kind, name, and namespace match', async () => { @@ -193,8 +191,8 @@ describe('Database', () => { catalog.addEntity(tx, entityRequest), ); added.entity.apiVersion = 'something.new'; - delete added.entity.metadata!.uid; - delete added.entity.metadata!.generation; + delete added.entity.metadata.uid; + delete added.entity.metadata.generation; const updated = await catalog.transaction(tx => catalog.updateEntity(tx, { entity: added.entity }), ); @@ -207,9 +205,9 @@ describe('Database', () => { 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'; + 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 }), @@ -222,7 +220,7 @@ describe('Database', () => { const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); - added.entity.metadata!.etag = 'garbage'; + added.entity.metadata.etag = 'garbage'; await expect( catalog.transaction(tx => catalog.updateEntity(tx, { entity: added.entity }), @@ -235,7 +233,7 @@ describe('Database', () => { const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); - added.entity.metadata!.generation! += 100; + added.entity.metadata.generation! += 100; await expect( catalog.transaction(tx => catalog.updateEntity(tx, { entity: added.entity }), @@ -247,10 +245,15 @@ describe('Database', () => { describe('entities', () => { it('can get all entities with empty filters list', async () => { const catalog = new Database(database, getVoidLogger()); - const e1: Entity = { apiVersion: 'a', kind: 'b' }; - const e2: Entity = { + const e1: Entity = { apiVersion: 'a', - kind: 'b', + kind: 'k1', + metadata: { name: 'n' }, + }; + const e2: Entity = { + apiVersion: 'c', + kind: 'k2', + metadata: { name: 'n' }, spec: { c: null }, }; await catalog.transaction(async tx => { @@ -263,8 +266,14 @@ describe('Database', () => { expect(result.length).toEqual(2); expect(result).toEqual( expect.arrayContaining([ - { locationId: undefined, entity: expect.objectContaining(e1) }, - { locationId: undefined, entity: expect.objectContaining(e2) }, + { + locationId: undefined, + entity: expect.objectContaining({ kind: 'k1' }), + }, + { + locationId: undefined, + entity: expect.objectContaining({ kind: 'k2' }), + }, ]), ); }); @@ -272,15 +281,17 @@ describe('Database', () => { it('can get all specific entities for matching filters (naive case)', async () => { const catalog = new Database(database, getVoidLogger()); const entities: Entity[] = [ - { apiVersion: 'a', kind: 'b' }, + { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { apiVersion: 'a', - kind: 'b', + kind: 'k2', + metadata: { name: 'n' }, spec: { c: 'some' }, }, { apiVersion: 'a', - kind: 'b', + kind: 'k3', + metadata: { name: 'n' }, spec: { c: null }, }, ]; @@ -294,27 +305,32 @@ describe('Database', () => { await expect( catalog.transaction(async tx => catalog.entities(tx, [ - { key: 'kind', values: ['b'] }, + { key: 'kind', values: ['k2'] }, { key: 'spec.c', values: ['some'] }, ]), ), ).resolves.toEqual([ - { locationId: undefined, entity: expect.objectContaining(entities[1]) }, + { + locationId: undefined, + entity: expect.objectContaining({ kind: 'k2' }), + }, ]); }); it('can get all specific entities for matching filters with nulls (both missing and literal null value)', async () => { const catalog = new Database(database, getVoidLogger()); const entities: Entity[] = [ - { apiVersion: 'a', kind: 'b' }, + { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { apiVersion: 'a', - kind: 'b', + kind: 'k2', + metadata: { name: 'n' }, spec: { c: 'some' }, }, { apiVersion: 'a', - kind: 'b', + kind: 'k3', + metadata: { name: 'n' }, spec: { c: null }, }, ]; @@ -327,7 +343,7 @@ describe('Database', () => { const rows = await catalog.transaction(async tx => catalog.entities(tx, [ - { key: 'kind', values: ['b'] }, + { key: 'apiVersion', values: ['a'] }, { key: 'spec.c', values: [null, 'some'] }, ]), ); @@ -337,15 +353,15 @@ describe('Database', () => { expect.arrayContaining([ { locationId: undefined, - entity: expect.objectContaining(entities[0]), + entity: expect.objectContaining({ kind: 'k1' }), }, { locationId: undefined, - entity: expect.objectContaining(entities[1]), + entity: expect.objectContaining({ kind: 'k2' }), }, { locationId: undefined, - entity: expect.objectContaining(entities[2]), + entity: expect.objectContaining({ kind: 'k3' }), }, ]), ); diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 1f5cc7562f..26deb3362e 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -46,11 +46,7 @@ function getStrippedMetadata(metadata: EntityMeta): EntityMeta { return output; } -function serializeMetadata(metadata: EntityMeta | undefined): string | null { - if (!metadata) { - return null; - } - +function serializeMetadata(metadata: EntityMeta): string { return JSON.stringify(getStrippedMetadata(metadata)); } @@ -67,14 +63,14 @@ function toEntityRow( entity: Entity, ): DbEntitiesRow { return { - id: entity.metadata!.uid!, + id: entity.metadata.uid!, location_id: locationId || null, - etag: entity.metadata!.etag!, - generation: entity.metadata!.generation!, + 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, + name: entity.metadata.name || null, + namespace: entity.metadata.namespace || null, metadata: serializeMetadata(entity.metadata), spec: serializeSpec(entity.spec), }; @@ -85,17 +81,13 @@ function toEntityResponse(row: DbEntitiesRow): DbEntityResponse { apiVersion: row.api_version, kind: row.kind, metadata: { + ...(JSON.parse(row.metadata) as Entity['metadata']), uid: row.id, etag: row.etag, generation: Number(row.generation), // cast because of sqlite }, }; - if (row.metadata) { - const metadata = JSON.parse(row.metadata) as Entity['metadata']; - entity.metadata = { ...entity.metadata, ...metadata }; - } - if (row.spec) { const spec = JSON.parse(row.spec); entity.spec = spec; @@ -125,9 +117,7 @@ function generateUid(): string { } function generateEtag(): string { - return Buffer.from(uuidv4(), 'utf8') - .toString('base64') - .replace(/[^\w]/g, ''); + return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, ''); } /** @@ -178,11 +168,11 @@ export class Database { tx: Knex.Transaction, request: DbEntityRequest, ): Promise { - if (request.entity.metadata?.uid !== undefined) { + if (request.entity.metadata.uid !== undefined) { throw new InputError('May not specify uid for new entities'); - } else if (request.entity.metadata?.etag !== undefined) { + } else if (request.entity.metadata.etag !== undefined) { throw new InputError('May not specify etag for new entities'); - } else if (request.entity.metadata?.generation !== undefined) { + } else if (request.entity.metadata.generation !== undefined) { throw new InputError('May not specify generation for new entities'); } @@ -289,9 +279,9 @@ export class Database { if (oldRow.metadata) { const oldMetadata = JSON.parse(oldRow.metadata) as EntityMeta; if (oldMetadata.annotations) { - newEntity.metadata!.annotations = { + newEntity.metadata.annotations = { ...oldMetadata.annotations, - ...newEntity.metadata!.annotations, + ...newEntity.metadata.annotations, }; } } @@ -374,9 +364,7 @@ export class Database { target, }); - return (await tx('locations') - .where({ id }) - .select())![0]; + return (await tx('locations').where({ id }).select())![0]; }); } diff --git a/plugins/catalog-backend/src/database/DatabaseManager.ts b/plugins/catalog-backend/src/database/DatabaseManager.ts index c6e7a2e684..05b6eaef8d 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.ts @@ -96,14 +96,14 @@ export class DatabaseManager { await DatabaseManager.logUpdateSuccess( database, location.id, - entity.metadata!.name, + entity.metadata.name, ); } catch (error) { await DatabaseManager.logUpdateFailure( database, location.id, error, - readerItem.data.metadata?.name, + readerItem.data.metadata.name, ); } } diff --git a/plugins/catalog-backend/src/database/migrations/20200511113813_init.ts b/plugins/catalog-backend/src/database/migrations/20200511113813_init.ts index f029871a70..2ff06c4046 100644 --- a/plugins/catalog-backend/src/database/migrations/20200511113813_init.ts +++ b/plugins/catalog-backend/src/database/migrations/20200511113813_init.ts @@ -76,7 +76,7 @@ export async function up(knex: Knex): Promise { .comment('The metadata.namespace field of the entity'); table .string('metadata') - .nullable() + .notNullable() .comment('The entire metadata JSON blob of the entity'); table .string('spec') diff --git a/plugins/catalog-backend/src/database/search.test.ts b/plugins/catalog-backend/src/database/search.test.ts index 7ad06aee14..8f011fb250 100644 --- a/plugins/catalog-backend/src/database/search.test.ts +++ b/plugins/catalog-backend/src/database/search.test.ts @@ -102,14 +102,15 @@ describe('search', () => { const input: Entity = { apiVersion: 'a', kind: 'b', + metadata: { name: 'n' }, }; expect(buildEntitySearch('eid', input)).toEqual([ - { entity_id: 'eid', key: 'metadata.name', value: null }, + { entity_id: 'eid', key: 'metadata.name', value: 'n' }, { entity_id: 'eid', key: 'metadata.namespace', value: null }, { entity_id: 'eid', key: 'metadata.uid', value: null }, { entity_id: 'eid', key: 'apiVersion', value: 'a' }, { entity_id: 'eid', key: 'kind', value: 'b' }, - { entity_id: 'eid', key: 'name', value: null }, + { entity_id: 'eid', key: 'name', value: 'n' }, { entity_id: 'eid', key: 'namespace', value: null }, { entity_id: 'eid', key: 'uid', value: null }, ]); diff --git a/plugins/catalog-backend/src/database/search.ts b/plugins/catalog-backend/src/database/search.ts index fcacf1a9f2..87fc59185d 100644 --- a/plugins/catalog-backend/src/database/search.ts +++ b/plugins/catalog-backend/src/database/search.ts @@ -127,17 +127,17 @@ export function buildEntitySearch( { entity_id: entityId, key: 'metadata.name', - value: toValue(entity.metadata?.name), + value: toValue(entity.metadata.name), }, { entity_id: entityId, key: 'metadata.namespace', - value: toValue(entity.metadata?.namespace), + value: toValue(entity.metadata.namespace), }, { entity_id: entityId, key: 'metadata.uid', - value: toValue(entity.metadata?.uid), + value: toValue(entity.metadata.uid), }, ]; diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index ca1cbbdfd4..9b5a91d249 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -26,7 +26,7 @@ export type DbEntitiesRow = { namespace: string | null; etag: string; generation: number; - metadata: string | null; + metadata: string; spec: string | null; }; diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 36e8b55770..f5675e6516 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -37,7 +37,9 @@ class MockLocationsCatalog implements LocationsCatalog { describe('createRouter', () => { describe('entities', () => { it('happy path: lists entities', async () => { - const entities: Entity[] = [{ apiVersion: 'a', kind: 'b' }]; + const entities: Entity[] = [ + { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } }, + ]; const catalog = new MockEntitiesCatalog(); catalog.entities.mockResolvedValueOnce(entities); @@ -190,9 +192,7 @@ describe('createRouter', () => { }); const app = express().use(router); - const response = await request(app) - .post('/locations') - .send(location); + const response = await request(app).post('/locations').send(location); expect(response.status).toEqual(400); });