paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

Merge pull request #1053 from spotify/freben/names

Browse Source 
Make metadata and name mandatory
This commit is contained in:
Fredrik Adelöw
2020-05-29 09:13:18 +02:00
committed by GitHub
parent 267ec74f2a 60e1ad28aa
commit e9ba121b8d
14 changed files with 102 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/catalog-model/src/entity/Entity.ts
+4 -4
View File
@@ -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.
packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts
+5 -8
View File
@@ -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);
}
packages/catalog-model/src/entity/policies/ReservedFieldsEntityPolicy.ts
+2 -2
View File
@@ -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`,
);
packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.test.ts
+4 -4
View File
@@ -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 () => {
packages/catalog-model/src/entity/policies/SchemaValidEntityPolicy.ts
+2 -2
View File
@@ -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<Record<string, string>>().notRequired(),
annotations: yup.object<Record<string, string>>().notRequired(),
})
.notRequired(),
.required(),
spec: yup.object({}).notRequired(),
});
plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts
+3 -3
View File
@@ -31,7 +31,7 @@ export class StaticEntitiesCatalog implements EntitiesCatalog {
}
async entityByUid(uid: string): Promise<Entity | undefined> {
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');
plugins/catalog-backend/src/database/Database.test.ts
+54 -38
View File
@@ -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' }),
},
]),
);
plugins/catalog-backend/src/database/Database.ts
+14 -26
View File
@@ -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<any, any>,
request: DbEntityRequest,
): Promise<DbEntityResponse> {
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<DbLocationsRow>('locations')
.where({ id })
.select())![0];
return (await tx<DbLocationsRow>('locations').where({ id }).select())![0];
});
}
plugins/catalog-backend/src/database/DatabaseManager.ts
+2 -2
View File
@@ -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,
);
}
}
plugins/catalog-backend/src/database/migrations/20200511113813_init.ts
+1 -1
View File
@@ -76,7 +76,7 @@ export async function up(knex: Knex): Promise<any> {
.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')
plugins/catalog-backend/src/database/search.test.ts
+3 -2
View File
@@ -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 },
]);
plugins/catalog-backend/src/database/search.ts
+3 -3
View File
@@ -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),
},
];
plugins/catalog-backend/src/database/types.ts
+1 -1
View File
@@ -26,7 +26,7 @@ export type DbEntitiesRow = {
namespace: string | null;
etag: string;
generation: number;
metadata: string | null;
metadata: string;
spec: string | null;
};
plugins/catalog-backend/src/service/router.test.ts
+4 -4
View File
@@ -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);
});