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

Address review comments

Browse Source
This commit is contained in:
Fredrik Adelöw
2020-05-20 08:51:43 +02:00
parent 55b94dbd54
commit ba75fe50a7
6 changed files with 79 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.test.ts
+35 -2
View File
@@ -15,7 +15,6 @@
*/
import yaml from 'yaml';
import { unindent } from '../../util';
import { makeValidator } from '../../validation';
import { DescriptorEnvelopeParser } from './DescriptorEnvelopeParser';
@@ -24,7 +23,7 @@ describe('DescriptorEnvelopeParser', () => {
let parser: DescriptorEnvelopeParser;
beforeEach(() => {
data = yaml.parse(unindent`
data = yaml.parse(`
apiVersion: backstage.io/v1beta1
kind: Component
metadata:
@@ -44,11 +43,25 @@ describe('DescriptorEnvelopeParser', () => {
await expect(parser.parse(data)).resolves.toBe(data);
});
it('rejects missing apiVersion', async () => {
delete data.apiVersion;
await expect(parser.parse(data)).rejects.toThrow(/apiVersion/);
});
it('rejects wrong root type', async () => {
await expect(parser.parse(7)).rejects.toThrow(/object/);
});
it('rejects bad apiVersion', async () => {
data.apiVersion = 'a#b';
await expect(parser.parse(data)).rejects.toThrow(/apiVersion/);
});
it('rejects missing kind', async () => {
delete data.kind;
await expect(parser.parse(data)).rejects.toThrow(/kind/);
});
it('rejects bad kind', async () => {
data.kind = 'a#b';
await expect(parser.parse(data)).rejects.toThrow(/kind/);
@@ -103,4 +116,24 @@ describe('DescriptorEnvelopeParser', () => {
data.metadata.annotations.a = [];
await expect(parser.parse(data)).rejects.toThrow(/annotation.*value/i);
});
it('rejects unknown root keys', async () => {
data.spec2 = {};
await expect(parser.parse(data)).rejects.toThrow(/spec2/i);
});
it('rejects reserved keys in the spec root', async () => {
data.spec.apiVersion = 'a/b';
await expect(parser.parse(data)).rejects.toThrow(/spec.*apiVersion/i);
});
it('rejects reserved keys in labels', async () => {
data.metadata.labels.apiVersion = 'a';
await expect(parser.parse(data)).rejects.toThrow(/label.*apiVersion/i);
});
it('rejects reserved keys in annotations', async () => {
data.metadata.annotations.apiVersion = 'a';
await expect(parser.parse(data)).rejects.toThrow(/annotation.*apiVersion/i);
});
});
plugins/catalog-backend/src/ingestion/descriptors/DescriptorEnvelopeParser.ts
+42 -5
View File
@@ -17,7 +17,9 @@
import * as yup from 'yup';
import { Validators } from '../../validation';
// The envelope that's common to all versions/kinds
/**
* The format envelope that's common to all versions/kinds
*/
export type DescriptorEnvelope = {
apiVersion: string;
kind: string;
@@ -30,7 +32,9 @@ export type DescriptorEnvelope = {
spec?: object;
};
// Parses some raw structured data as a descriptor envelope
/**
* Parses some raw structured data as a descriptor envelope
*/
export class DescriptorEnvelopeParser {
private schema: yup.Schema<DescriptorEnvelope>;
@@ -59,7 +63,7 @@ export class DescriptorEnvelopeParser {
.test(
'metadata.name',
'The name is not formatted according to schema',
(value) => value === undefined || validators.isValidEntityName(value),
value => value === undefined || validators.isValidEntityName(value),
);
const namespaceSchema = yup
@@ -68,7 +72,7 @@ export class DescriptorEnvelopeParser {
.test(
'metadata.namespace',
'The namespace is malformed',
(value) => value === undefined || validators.isValidEntityName(value),
value => value === undefined || validators.isValidEntityName(value),
);
const labelsSchema = yup
@@ -141,10 +145,43 @@ export class DescriptorEnvelopeParser {
}
async parse(data: any): Promise<DescriptorEnvelope> {
let result: DescriptorEnvelope;
try {
return await this.schema.validate(data, { strict: true });
result = await this.schema.validate(data, { strict: true });
} catch (e) {
throw new Error(`Malformed envelope, ${e}`);
}
// These are keys with specific semantic meaning in a document, that we do
// not want to appear in the root of the spec, or as labels or as
// annotations, because they will lead to confusion.
const reservedKeys = [
'apiVersion',
'kind',
'name',
'namespace',
'labels',
'annotations',
'spec',
];
for (const key of reservedKeys) {
if (result.spec?.hasOwnProperty(key)) {
throw new Error(
`The spec may not contain the key ${key}, because it has reserved meaning`,
);
}
if (result.metadata?.labels?.hasOwnProperty(key)) {
throw new Error(
`A label may not have the key ${key}, because it has reserved meaning`,
);
}
if (result.metadata?.annotations?.hasOwnProperty(key)) {
throw new Error(
`An annotation may not have the key ${key}, because it has reserved meaning`,
);
}
}
return result;
}
}
plugins/catalog-backend/src/util/index.ts
-1
View File
@@ -15,4 +15,3 @@
*/
export * from './runPeriodically';
export * from './unindent';
plugins/catalog-backend/src/util/unindent.test.ts
-27
View File
@@ -1,27 +0,0 @@
/*
* 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 { unindent } from './unindent';
describe('unindent', () => {
it('handles the happy path', () => {
const data = unindent`
line1
line2
line3`;
expect(data).toBe('line1\n line2\nline3');
});
});
plugins/catalog-backend/src/util/unindent.ts
-57
View File
@@ -1,57 +0,0 @@
/*
* 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.
*/
/**
* For use in tagged templates, to remove initial indentation on all lines.
*
* This is convenient for specifying e.g. YAML data.
*/
export function unindent(
strings: TemplateStringsArray,
...values: string[]
): string {
// Interweave the strings with the substitution vars first
let output = '';
for (let i = 0; i < values.length; i++) {
output += strings[i] + values[i];
}
output += strings[values.length];
// Split on newlines
const lines = output.split(/(?:\r\n|\n|\r)/);
if (lines.length < 2) {
throw new Error('Expected at least one non-empty line');
} else if (lines[0]) {
throw new Error('The first line must be empty');
}
// Second line decides indentation level
const match = lines[1].match(/^(\s+)/);
if (!match) {
// There was no indentation
return output;
}
const whitespace = match[1];
return lines
.map((line) => {
return line.startsWith(whitespace)
? line.substr(whitespace.length)
: line;
})
.join('\n')
.trim();
}
plugins/catalog-backend/src/validation/CommonValidatorFunctions.ts
+2 -2
View File
@@ -58,7 +58,7 @@ export class CommonValidatorFunctions {
static isJsonSafe(value: any): boolean {
try {
return lodash.isEqual(value, JSON.parse(JSON.stringify(value)));
} catch (e) {
} catch {
return false;
}
}
@@ -101,7 +101,7 @@ export class CommonValidatorFunctions {
static normalizeToLowercaseAlphanum(value: string): string {
return value
.split('')
.filter((x) => /[a-zA-Z0-9]/.test(x))
.filter(x => /[a-zA-Z0-9]/.test(x))
.join('')
.toLowerCase();
}