forked from backstage/backstage
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(catalog-backend): use scm integration for codeowners
Signed-off-by: Andrew Thauer <[email protected]>
- Loading branch information
1 parent
1cc27f7
commit 4bc98a5
Showing
10 changed files
with
415 additions
and
327 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@backstage/plugin-catalog-backend': patch | ||
| --- | ||
|
|
||
| Refactor CodeOwnersProcessor to use ScmIntegrations |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,59 +16,23 @@ | |
|
|
||
| import { getVoidLogger } from '@backstage/backend-common'; | ||
| import { LocationSpec } from '@backstage/catalog-model'; | ||
| import { CodeOwnersEntry } from 'codeowners-utils'; | ||
| import { | ||
| buildCodeOwnerUrl, | ||
| buildUrl, | ||
| CodeOwnersProcessor, | ||
| findPrimaryCodeOwner, | ||
| findRawCodeOwners, | ||
| normalizeCodeOwner, | ||
| parseCodeOwners, | ||
| resolveCodeOwner, | ||
| } from './CodeOwnersProcessor'; | ||
| import { ConfigReader } from '@backstage/config'; | ||
| import { CodeOwnersProcessor } from './CodeOwnersProcessor'; | ||
|
|
||
| const logger = getVoidLogger(); | ||
| const mockCodeOwnersText = () => ` | ||
| * @acme/team-foo @acme/team-bar | ||
| /docs @acme/team-bar | ||
| `; | ||
|
|
||
| describe('CodeOwnersProcessor', () => { | ||
| const mockUrl = ({ basePath = '' } = {}): string => | ||
| `https://github.com/backstage/backstage/blob/master/${basePath}catalog-info.yaml`; | ||
| const mockLocation = ({ | ||
| basePath = '', | ||
| type = 'github', | ||
| } = {}): LocationSpec => ({ | ||
| type, | ||
| target: mockUrl({ basePath }), | ||
| target: `https://github.com/backstage/backstage/blob/master/${basePath}catalog-info.yaml`, | ||
| }); | ||
|
|
||
| const mockReadUrl = (basePath = '') => | ||
| `https://github.com/backstage/backstage/blob/master/${basePath}CODEOWNERS`; | ||
|
|
||
| const mockGitUri = (codeOwnersPath: string = '') => { | ||
| return { | ||
| source: 'github.com', | ||
| owner: 'backstage', | ||
| 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, | ||
|
|
@@ -82,156 +46,19 @@ describe('CodeOwnersProcessor', () => { | |
| 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/backstage/backstage/blob/master/.github/CODEOWNERS', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('buildCodeOwnerUrl', () => { | ||
| it('should build a location spec to the codeowners', () => { | ||
| expect(buildCodeOwnerUrl(mockUrl(), '/docs/CODEOWNERS')).toEqual( | ||
| 'https://github.com/backstage/backstage/blob/master/docs/CODEOWNERS', | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle nested paths from original location spec', () => { | ||
| expect( | ||
| buildCodeOwnerUrl( | ||
| mockUrl({ basePath: 'packages/foo/' }), | ||
| '/CODEOWNERS', | ||
| ), | ||
| ).toEqual( | ||
| 'https://github.com/backstage/backstage/blob/master/CODEOWNERS', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('parseCodeOwners', () => { | ||
| it('should parse the codeowners file', () => { | ||
| expect(parseCodeOwners(mockCodeOwnersText())).toEqual(mockCodeOwners()); | ||
| }); | ||
| }); | ||
|
|
||
| describe('normalizeCodeOwner', () => { | ||
| it('should remove the @ symbol', () => { | ||
| expect(normalizeCodeOwner('@yoda')).toBe('yoda'); | ||
| }); | ||
|
|
||
| it('should remove org from org/team format', () => { | ||
| expect(normalizeCodeOwner('@acme/foo')).toBe('foo'); | ||
| }); | ||
|
|
||
| it('should return username from email format', () => { | ||
| expect(normalizeCodeOwner('[email protected]')).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 reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
| const result = await findRawCodeOwners(mockLocation(), { | ||
| reader, | ||
| logger, | ||
| }); | ||
| expect(result).toEqual(ownersText); | ||
| }); | ||
|
|
||
| it('should return undefined when no codeowner', async () => { | ||
| const read = jest.fn().mockRejectedValue(mockReadResult()); | ||
| const reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
|
|
||
| await expect( | ||
| findRawCodeOwners(mockLocation(), { reader, logger }), | ||
| ).resolves.toBeUndefined(); | ||
| }); | ||
|
|
||
| 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 reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
|
|
||
| const result = await findRawCodeOwners(mockLocation(), { | ||
| reader, | ||
| logger, | ||
| }); | ||
|
|
||
| expect(read.mock.calls.length).toBe(5); | ||
| expect(read.mock.calls[0]).toEqual([mockReadUrl('')]); | ||
| expect(read.mock.calls[1]).toEqual([mockReadUrl('docs/')]); | ||
| expect(read.mock.calls[2]).toEqual([mockReadUrl('.bitbucket/')]); | ||
| expect(read.mock.calls[3]).toEqual([mockReadUrl('.github/')]); | ||
| expect(read.mock.calls[4]).toEqual([mockReadUrl('.gitlab/')]); | ||
| expect(result).toEqual(ownersText); | ||
| }); | ||
| }); | ||
|
|
||
| describe('resolveCodeOwner', () => { | ||
| it('should return found codeowner', async () => { | ||
| const read = jest | ||
| .fn() | ||
| .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); | ||
| const reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
|
|
||
| const owner = await resolveCodeOwner(mockLocation(), { reader, logger }); | ||
| expect(owner).toBe('backstage-core'); | ||
| }); | ||
|
|
||
| it('should return undefined when no codeowner', async () => { | ||
| const read = jest | ||
| .fn() | ||
| .mockImplementation(() => mockReadResult({ error: 'error: foo' })); | ||
| const reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
|
|
||
| await expect( | ||
| resolveCodeOwner(mockLocation(), { reader, logger }), | ||
| ).resolves.toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('CodeOwnersProcessor', () => { | ||
| describe('preProcessEntity', () => { | ||
| const setupTest = ({ kind = 'Component', spec = {} } = {}) => { | ||
| const entity = { kind, spec }; | ||
| const read = jest | ||
| .fn() | ||
| .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() })); | ||
|
|
||
| const config = new ConfigReader({}); | ||
| const reader = { read, readTree: jest.fn(), search: jest.fn() }; | ||
| const processor = new CodeOwnersProcessor({ reader, logger }); | ||
| const processor = CodeOwnersProcessor.fromConfig(config, { | ||
| logger: getVoidLogger(), | ||
| reader, | ||
| }); | ||
|
|
||
| return { entity, processor, read }; | ||
| }; | ||
|
|
@@ -249,18 +76,15 @@ describe('CodeOwnersProcessor', () => { | |
| expect(result).toEqual(entity); | ||
| }); | ||
|
|
||
| it('should handle url locations', async () => { | ||
| it('should ingore invalid locations type', async () => { | ||
| const { entity, processor } = setupTest(); | ||
|
|
||
| const result = await processor.preProcessEntity( | ||
| entity as any, | ||
| mockLocation({ type: 'url' }), | ||
| mockLocation({ type: 'github-org' }), | ||
| ); | ||
|
|
||
| expect(result).toEqual({ | ||
| ...entity, | ||
| spec: { owner: 'backstage-core' }, | ||
| }); | ||
| expect(result).toEqual(entity); | ||
| }); | ||
|
|
||
| it('should ignore invalid kinds', async () => { | ||
|
|
@@ -284,7 +108,7 @@ describe('CodeOwnersProcessor', () => { | |
|
|
||
| expect(result).toEqual({ | ||
| ...entity, | ||
| spec: { owner: 'backstage-core' }, | ||
| spec: { owner: 'team-foo' }, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
Oops, something went wrong.