From 1f21de7b4345ffdc84eed2bad73295cd9fc75f06 Mon Sep 17 00:00:00 2001 From: Andrew Thauer <6507159+andrewthauer@users.noreply.github.com> Date: Tue, 29 Sep 2020 07:35:05 -0400 Subject: [PATCH] feat(catalog): add codeowners processor --- packages/cli/config/tsconfig.json | 2 +- plugins/catalog-backend/package.json | 3 + .../src/ingestion/LocationReaders.ts | 2 + .../processors/AzureApiReaderProcessor.ts | 3 +- .../processors/BitbucketApiReaderProcessor.ts | 3 +- .../processors/CodeOwnersProcessor.test.ts | 279 ++++++++++++++++++ .../processors/CodeOwnersProcessor.ts | 162 ++++++++++ .../processors/GithubReaderProcessor.ts | 6 +- .../GitlabApiReaderProcessor.test.ts | 13 +- .../processors/GitlabApiReaderProcessor.ts | 4 - yarn.lock | 15 + 11 files changed, 467 insertions(+), 25 deletions(-) create mode 100644 plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts create mode 100644 plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts diff --git a/packages/cli/config/tsconfig.json b/packages/cli/config/tsconfig.json index ef1a8df353..62609aad5b 100644 --- a/packages/cli/config/tsconfig.json +++ b/packages/cli/config/tsconfig.json @@ -11,7 +11,7 @@ "incremental": true, "isolatedModules": true, "jsx": "react", - "lib": ["DOM", "DOM.Iterable", "ScriptHost", "ES2019"], + "lib": ["DOM", "DOM.Iterable", "ScriptHost", "ES2019", "ESNext.Promise"], "module": "ESNext", "moduleResolution": "node", "noEmit": false, diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 6e2aa096d3..13c300d4b9 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -24,6 +24,8 @@ "@backstage/catalog-model": "^0.1.1-alpha.23", "@backstage/config": "^0.1.1-alpha.23", "@types/express": "^4.17.6", + "codeowners-utils": "^1.0.2", + "core-js": "^3.6.5", "express": "^4.17.1", "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", @@ -41,6 +43,7 @@ }, "devDependencies": { "@backstage/cli": "^0.1.1-alpha.23", + "@types/core-js": "^2.5.4", "@types/git-url-parse": "^9.0.0", "@types/lodash": "^4.14.151", "@types/node-fetch": "^2.5.7", diff --git a/plugins/catalog-backend/src/ingestion/LocationReaders.ts b/plugins/catalog-backend/src/ingestion/LocationReaders.ts index dd3f9eacda..6200bd51c6 100644 --- a/plugins/catalog-backend/src/ingestion/LocationReaders.ts +++ b/plugins/catalog-backend/src/ingestion/LocationReaders.ts @@ -35,6 +35,7 @@ import { GitlabApiReaderProcessor } from './processors/GitlabApiReaderProcessor' import { GitlabReaderProcessor } from './processors/GitlabReaderProcessor'; import { LocationRefProcessor } from './processors/LocationEntityProcessor'; import { PlaceholderProcessor } from './processors/PlaceholderProcessor'; +import { CodeOwnersProcessor } from './processors/CodeOwnersProcessor'; import * as result from './processors/results'; import { StaticLocationProcessor } from './processors/StaticLocationProcessor'; import { @@ -88,6 +89,7 @@ export class LocationReaders implements LocationReader { new UrlReaderProcessor(), new YamlProcessor(), PlaceholderProcessor.default(), + new CodeOwnersProcessor(), new ApiDefinitionAtLocationProcessor(), new EntityPolicyProcessor(entityPolicy), new LocationRefProcessor(), diff --git a/plugins/catalog-backend/src/ingestion/processors/AzureApiReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/AzureApiReaderProcessor.ts index 03ff30ea69..70a84d72df 100644 --- a/plugins/catalog-backend/src/ingestion/processors/AzureApiReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/AzureApiReaderProcessor.ts @@ -107,8 +107,7 @@ export class AzureApiReaderProcessor implements LocationProcessor { srcKeyword !== '_git' || repoName === '' || path === '' || - ref === '' || - !path.match(/\.yaml$/) + ref === '' ) { throw new Error('Wrong Azure Devops URL or Invalid file path'); } diff --git a/plugins/catalog-backend/src/ingestion/processors/BitbucketApiReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/BitbucketApiReaderProcessor.ts index 97ecf0cd1f..c46fbd1737 100644 --- a/plugins/catalog-backend/src/ingestion/processors/BitbucketApiReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/BitbucketApiReaderProcessor.ts @@ -106,8 +106,7 @@ export class BitbucketApiReaderProcessor implements LocationProcessor { empty !== '' || userOrOrg === '' || repoName === '' || - srcKeyword !== 'src' || - !restOfPath.join('/').match(/\.yaml$/) + srcKeyword !== 'src' ) { throw new Error('Wrong Bitbucket URL or Invalid file path'); } diff --git a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts new file mode 100644 index 0000000000..29f94dd0d7 --- /dev/null +++ b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts @@ -0,0 +1,279 @@ +/* + * 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 { LocationSpec } from '@backstage/catalog-model'; +import { CodeOwnersEntry } from 'codeowners-utils'; +import { + buildCodeOwnerLocation, + buildUrl, + CodeOwnersProcessor, + findPrimaryCodeOwner, + findRawCodeOwners, + normalizeCodeOwner, + parseCodeOwners, + resolveCodeOwner, +} from './CodeOwnersProcessor'; + +describe(CodeOwnersProcessor, () => { + const mockLocation = ({ + basePath = '', + type = 'github', + } = {}): LocationSpec => ({ + type, + target: `https://github.com/spotify/backstage/blob/master/${basePath}catalog-info.yaml`, + }); + + const mockReadLocation = (basePath = '') => ({ + type: 'github', + target: `https://github.com/spotify/backstage/blob/master/${basePath}CODEOWNERS`, + }); + + const mockGitUri = (codeOwnersPath: string = '') => { + return { + source: 'github.com', + owner: 'spotify', + name: 'backstage', + codeOwnersPath, + }; + }; + + const mockCodeOwnersText = () => ` +# https://help.github.com/articles/about-codeowners/ +* @spotify/backstage-core @acme/team-foo +/plugins/techdocs @spotify/techdocs-core + `; + + const mockCodeOwners = (): CodeOwnersEntry[] => { + return [ + { + pattern: '/plugins/techdocs', + owners: ['@spotify/techdocs-core'], + }, + { pattern: '*', owners: ['@spotify/backstage-core', '@acme/team-foo'] }, + ]; + }; + + const mockReadResult = ({ + error = undefined, + data = undefined, + }: { + error?: string; + data?: string; + } = {}) => { + if (error) { + throw Error(error); + } + return data; + }; + + describe(buildUrl, () => { + it.each([['azure.com'], ['dev.azure.com']])( + 'should throw not implemented error', + source => { + expect(() => buildUrl({ ...mockGitUri(), source })).toThrow(); + }, + ); + + it('should build github.com url', () => { + expect( + buildUrl({ + ...mockGitUri(), + codeOwnersPath: '/.github/CODEOWNERS', + }), + ).toBe( + 'https://github.com/spotify/backstage/blob/master/.github/CODEOWNERS', + ); + }); + }); + + describe(buildCodeOwnerLocation, () => { + it('should builds a location spec to the codeowners', () => { + expect( + buildCodeOwnerLocation(mockLocation(), '/docs/CODEOWNERS'), + ).toEqual({ + type: 'github', + target: + 'https://github.com/spotify/backstage/blob/master/docs/CODEOWNERS', + }); + }); + + it('should handle nested paths from original location spec', () => { + expect( + buildCodeOwnerLocation( + mockLocation({ basePath: 'packages/foo/' }), + '/CODEOWNERS', + ), + ).toEqual({ + type: 'github', + target: 'https://github.com/spotify/backstage/blob/master/CODEOWNERS', + }); + }); + }); + + describe(parseCodeOwners, () => { + it('should parse the codeowners file', () => { + expect(parseCodeOwners(mockCodeOwnersText())).toEqual(mockCodeOwners()); + }); + }); + + describe(normalizeCodeOwner, () => { + it('should remove org from org/team format', () => { + expect(normalizeCodeOwner('@acme/foo')).toBe('foo'); + }); + + it('should return username from email format', () => { + expect(normalizeCodeOwner('foo@acme.com')).toBe('foo'); + }); + + it.each([['acme/foo'], ['dacme/foo']])( + 'should return string everything else', + owner => { + expect(normalizeCodeOwner(owner)).toBe(owner); + }, + ); + }); + + describe(findPrimaryCodeOwner, () => { + it('should return the primary owner', () => { + expect(findPrimaryCodeOwner(mockCodeOwners())).toBe('backstage-core'); + }); + }); + + describe(findRawCodeOwners, () => { + it('should return found codeowner', async () => { + const ownersText = mockCodeOwnersText(); + const read = jest + .fn() + .mockResolvedValue(mockReadResult({ data: ownersText })); + const result = await findRawCodeOwners(mockLocation(), read); + expect(result).toEqual(ownersText); + }); + + it('should raise error when no codeowner', async () => { + const read = jest.fn().mockRejectedValue(mockReadResult()); + + await expect( + findRawCodeOwners(mockLocation(), read), + ).rejects.toBeInstanceOf(Error); + }); + + it('should look at known codeowner locations', async () => { + const ownersText = mockCodeOwnersText(); + const read = jest + .fn() + .mockImplementationOnce(() => mockReadResult({ error: 'foo' })) + .mockImplementationOnce(() => mockReadResult({ error: 'bar' })) + .mockResolvedValue(mockReadResult({ data: ownersText })); + + const result = await findRawCodeOwners(mockLocation(), read); + + expect(read.mock.calls.length).toBe(3); + expect(read.mock.calls[0]).toEqual([mockReadLocation('.github/')]); + expect(read.mock.calls[1]).toEqual([mockReadLocation('')]); + expect(read.mock.calls[2]).toEqual([mockReadLocation('docs/')]); + expect(result).toEqual(ownersText); + }); + }); + + describe(resolveCodeOwner, () => { + it('should return found codeowner', async () => { + const read = jest + .fn() + .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); + const owner = await resolveCodeOwner(mockLocation(), read); + expect(owner).toBe('backstage-core'); + }); + + it('should raise an error when no codeowner', async () => { + const read = jest + .fn() + .mockImplementation(() => mockReadResult({ error: 'error: foo' })); + + await expect( + resolveCodeOwner(mockLocation(), read), + ).rejects.toBeInstanceOf(Error); + }); + }); + + describe(CodeOwnersProcessor, () => { + const setupTest = ({ kind = 'Component', spec = {} } = {}) => { + const entity = { kind, spec }; + const processor = new CodeOwnersProcessor(); + const read = jest + .fn() + .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); + + return { entity, processor, read }; + }; + + it('should not modify existing owner', async () => { + const { entity, processor } = setupTest({ + spec: { owner: '@acme/foo-team' }, + }); + + const result = await processor.processEntity( + entity as any, + mockLocation(), + null as any, + null as any, + ); + + expect(result).toEqual(entity); + }); + + it('should ignore url locations', async () => { + const { entity, processor } = setupTest(); + + const result = await processor.processEntity( + entity as any, + mockLocation({ type: 'url' }), + null as any, + null as any, + ); + + expect(result).toEqual(entity); + }); + + it('should ignore invalid kinds', async () => { + const { entity, processor } = setupTest({ kind: 'Group' }); + + const result = await processor.processEntity( + entity as any, + mockLocation(), + null as any, + null as any, + ); + + expect(result).toEqual(entity); + }); + + it('should set owner from codeowner', async () => { + const { entity, processor, read } = setupTest(); + + const result = await processor.processEntity( + entity as any, + mockLocation(), + null as any, + read, + ); + + expect(result).toEqual({ + ...entity, + spec: { owner: 'backstage-core' }, + }); + }); + }); +}); diff --git a/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts new file mode 100644 index 0000000000..c875bdc799 --- /dev/null +++ b/plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.ts @@ -0,0 +1,162 @@ +/* + * 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 { Entity, LocationSpec } from '@backstage/catalog-model'; +import { + LocationProcessor, + LocationProcessorEmit, + LocationProcessorRead, +} from './types'; +import * as codeowners from 'codeowners-utils'; +import { CodeOwnersEntry } from 'codeowners-utils'; +import parseGitUri from 'git-url-parse'; +import { filter, head, get, pipe, reverse } from 'lodash/fp'; + +// NOTE: This can be removed when ES2021 is implemented +import 'core-js/features/promise'; + +const ALLOWED_LOCATION_TYPES = [ + 'azure/api', + 'bitbucket/api', + 'github', + 'github/api', + 'gitlab', + 'gitlab/api', +]; + +export class CodeOwnersProcessor implements LocationProcessor { + async processEntity( + entity: Entity, + location: LocationSpec, + _emit: LocationProcessorEmit, + read: LocationProcessorRead, + ): Promise { + // Only continue if the owner is not set + if ( + !entity || + !['Component', 'API'].includes(entity.kind) || + !ALLOWED_LOCATION_TYPES.includes(location.type) || + (entity.spec && entity.spec.owner) + ) { + return entity; + } + + const owner = await resolveCodeOwner(location, read); + + return { + ...entity, + spec: { ...entity.spec, owner }, + }; + } +} + +export async function resolveCodeOwner( + location: LocationSpec, + read: LocationProcessorRead, +): Promise { + const ownersText = await findRawCodeOwners(location, read); + + if (!ownersText) { + throw Error(`Unable to find codeowners file for: ${location.target}`); + } + + const owners = parseCodeOwners(ownersText); + + return findPrimaryCodeOwner(owners); +} + +export async function findRawCodeOwners( + location: LocationSpec, + read: LocationProcessorRead, +): Promise { + const readOwnerLocation = async (basePath: string): Promise => { + const ownerLocation = buildCodeOwnerLocation( + location, + `${basePath}/CODEOWNERS`, + ); + + const data = await read(ownerLocation); + return data.toString(); + }; + + const gitProvider = location.type.split('/')[0]; + + return Promise.any([ + readOwnerLocation(`/.${gitProvider}`), + readOwnerLocation(''), + readOwnerLocation('/docs'), + ]); +} + +export function parseCodeOwners(ownersText: string) { + return codeowners.parse(ownersText); +} + +export function findPrimaryCodeOwner( + owners: CodeOwnersEntry[], +): string | undefined { + return pipe( + filter((e: CodeOwnersEntry) => e.pattern === '*'), + reverse, + head, + get('owners'), + head, + normalizeCodeOwner, + )(owners); +} + +export function normalizeCodeOwner(owner: string) { + if (owner.match(/^@.*\/.*/)) { + return owner.split('/')[1]; + } else if (owner.match(/^.*@.*\..*$/)) { + return owner.split('@')[0]; + } + + return owner; +} + +export function buildCodeOwnerLocation( + location: LocationSpec, + codeOwnersPath: string, +): LocationSpec { + const { type, target } = location; + + return { type, target: buildUrl({ ...parseGitUri(target), codeOwnersPath }) }; +} + +export function buildUrl({ + protocol = 'https', + source = 'github.com', + owner, + name, + ref = 'master', + codeOwnersPath = '/CODEOWNERS', +}: { + protocol?: string; + source?: string; + owner: string; + name: string; + ref?: string; + codeOwnersPath?: string; +}) { + switch (source) { + case 'dev.azure.com': + case 'azure.com': + throw Error('Azure codeowner url builder not implemented'); + default: + return `${protocol}://${source}/${owner}/${name}/blob/${ref}${codeOwnersPath}`; + } +} diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.ts index 4f0c66148f..cc50e53240 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubReaderProcessor.ts @@ -99,8 +99,7 @@ export function getApiUrl(target: string, provider: ProviderConfig): URL { !owner || !name || !ref || - (filepathtype !== 'blob' && filepathtype !== 'raw') || - !filepath?.match(/\.ya?ml$/) + (filepathtype !== 'blob' && filepathtype !== 'raw') ) { throw new Error('Wrong URL or invalid file path'); } @@ -125,8 +124,7 @@ export function getRawUrl(target: string, provider: ProviderConfig): URL { !owner || !name || !ref || - (filepathtype !== 'blob' && filepathtype !== 'raw') || - !filepath?.match(/\.ya?ml$/) + (filepathtype !== 'blob' && filepathtype !== 'raw') ) { throw new Error('Wrong URL or invalid file path'); } diff --git a/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.test.ts index 48ed270049..7e55fece1b 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.test.ts @@ -62,21 +62,10 @@ describe('GitlabApiReaderProcessor', () => { ), err: undefined, }, - { - target: - 'https://gitlab.com/groupA/teams/teamA/repoA/-/blob/branch/my/path/', - url: null, - err: - 'Incorrect url: https://gitlab.com/groupA/teams/teamA/repoA/-/blob/branch/my/path/, Error: GitLab url does not end in .ya?ml', - }, ]; for (const test of tests) { - if (test.err) { - expect(() => processor.buildRawUrl(test.target, 12345)).toThrowError( - test.err, - ); - } else if (test.url) { + if (test.url) { expect(processor.buildRawUrl(test.target, 12345).toString()).toEqual( test.url.toString(), ); diff --git a/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.ts index 067edba3d9..a51c306777 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GitlabApiReaderProcessor.ts @@ -83,10 +83,6 @@ export class GitlabApiReaderProcessor implements LocationProcessor { const branchAndfilePath = url.pathname.split('/-/blob/')[1]; - if (!branchAndfilePath.match(/\.ya?ml$/)) { - throw new Error('GitLab url does not end in .ya?ml'); - } - const [branch, ...filePath] = branchAndfilePath.split('/'); url.pathname = [ diff --git a/yarn.lock b/yarn.lock index 74761d70ba..af1528c202 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4766,6 +4766,11 @@ "@types/keygrip" "*" "@types/node" "*" +"@types/core-js@^2.5.4": + version "2.5.4" + resolved "https://registry.npmjs.org/@types/core-js/-/core-js-2.5.4.tgz#fc42ebde7d9cfa7c5f2668f117449b02348e41fd" + integrity sha512-Xwy8o12ak+iYgFr/KCVaVK5Sy+jFMiiPAID3+ObvMlBzy26XQJw5xu+a6rlHsrJENXj/AwJOGsJpVohUjAzSKQ== + "@types/cors@^2.8.4", "@types/cors@^2.8.6": version "2.8.6" resolved "https://registry.npmjs.org/@types/cors/-/cors-2.8.6.tgz#cfaab33c49c15b1ded32f235111ce9123009bd02" @@ -8328,6 +8333,16 @@ codemirror@^5.52.2: resolved "https://registry.npmjs.org/codemirror/-/codemirror-5.53.2.tgz#9799121cf8c50809cca487304e9de3a74d33f428" integrity sha512-wvSQKS4E+P8Fxn/AQ+tQtJnF1qH5UOlxtugFLpubEZ5jcdH2iXTVinb+Xc/4QjshuOxRm4fUsU2QPF1JJKiyXA== +codeowners-utils@^1.0.2: + version "1.0.2" + resolved "https://registry.npmjs.org/codeowners-utils/-/codeowners-utils-1.0.2.tgz#9d30148bf957c53d55f75df432cb1e3b4bc6ee28" + integrity sha512-4oLRCymV7azxGHMpM3F297D651VdwZa21hVfFCn/cOd8Fq8tFrpfpyRpSBQkaZCyFPkfOhEld9xceCF7btyiug== + dependencies: + cross-spawn "^7.0.2" + find-up "^4.1.0" + ignore "^5.1.4" + locate-path "^5.0.0" + collapse-white-space@^1.0.2: version "1.0.6" resolved "https://registry.npmjs.org/collapse-white-space/-/collapse-white-space-1.0.6.tgz#e63629c0016665792060dbbeb79c42239d2c5287"