diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index 616cbaaadc..f729316223 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -139,7 +139,7 @@ describe('AzureUrlReader', () => { describe('readTree', () => { const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.zip'), + path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'), ); beforeEach(() => { @@ -169,8 +169,8 @@ describe('AzureUrlReader', () => { const files = await response.files(); expect(files.length).toBe(2); - const mkDocsFile = await files[1].content(); - const indexMarkdownFile = await files[0].content(); + const mkDocsFile = await files[0].content(); + const indexMarkdownFile = await files[1].content(); expect(mkDocsFile.toString()).toBe('site_name: Test\n'); expect(indexMarkdownFile.toString()).toBe('# Test\n'); diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 6fdab9dfa1..24a49cf81c 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,7 +21,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; -import { NotModifiedError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { GithubUrlReader } from './GithubUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -168,6 +168,13 @@ describe('GithubUrlReader', () => { ), ); + worker.use( + rest.get( + 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', + (_, res, ctx) => res(ctx.status(404)), + ), + ); + // For a GHE host worker.use( rest.get( @@ -321,5 +328,14 @@ describe('GithubUrlReader', () => { ); expect((await response.files()).length).toBe(2); }); + + it('should throw error on missing branch', async () => { + const fnGithub = async () => { + await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/branchDoesNotExist', + ); + }; + await expect(fnGithub).rejects.toThrow(NotFoundError); + }); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 2acca10d4f..8e0410642b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -119,7 +119,7 @@ export class GithubUrlReader implements UrlReader { }, ); if (!repoGitHubResponse.ok) { - const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; + const message = `Failed to read tree (repository) from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; if (repoGitHubResponse.status === 404) { throw new NotFoundError(message); } @@ -142,6 +142,13 @@ export class GithubUrlReader implements UrlReader { headers, }, ); + if (!branchGitHubResponse.ok) { + const message = `Failed to read tree (branch) from ${url}, ${branchGitHubResponse.status} ${branchGitHubResponse.statusText}`; + if (branchGitHubResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } const commitSha = (await branchGitHubResponse.json()).commit.sha; if (options?.sha && options.sha === commitSha) { @@ -162,6 +169,13 @@ export class GithubUrlReader implements UrlReader { headers, }, ); + if (!archive.ok) { + const message = `Failed to read tree (archive) from ${url}, ${archive.status} ${archive.statusText}`; + if (archive.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } const path = `${repoName}-${branch}/${filepath}`; diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 6cc5e30dbd..a2dda48b16 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -23,6 +23,7 @@ import path from 'path'; import { getVoidLogger } from '../logging'; import { GitlabUrlReader } from './GitlabUrlReader'; import { ReadTreeResponseFactory } from './tree'; +import { NotModifiedError, NotFoundError } from '../errors'; const logger = getVoidLogger(); @@ -30,6 +31,22 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); +const gitlabProcessor = new GitlabUrlReader( + { + host: 'gitlab.com', + apiBaseUrl: 'https://gitlab.com/api/v4', + }, + { treeResponseFactory }, +); + +const hostedGitlabProcessor = new GitlabUrlReader( + { + host: 'gitlab.mycompany.com', + apiBaseUrl: 'https://gitlab.mycompany.com/api/v4', + }, + { treeResponseFactory }, +); + describe('GitlabUrlReader', () => { const worker = setupServer(); msw.setupDefaultHandlers(worker); @@ -136,39 +153,112 @@ describe('GitlabUrlReader', () => { }); describe('readTree', () => { - const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.zip'), + const archiveBuffer = fs.readFileSync( + path.resolve('src', 'reading', '__fixtures__', 'gitlab-archive.zip'), ); + const projectGitlabApiResponse = { + id: 11111111, + default_branch: 'main', + }; + + const branchGitlabApiResponse = { + commit: { + id: 'sha123abc', + }, + }; + beforeEach(() => { worker.use( rest.get( - 'https://gitlab.com/backstage/mock/-/archive/repo/mock-repo.zip', + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', (_, res, ctx) => res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), + ctx.body(archiveBuffer), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(projectGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/branchDoesNotExist', + (_, res, ctx) => res(ctx.status(404)), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(projectGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchGitlabApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/zip'), + ctx.body(archiveBuffer), ), ), ); }); it('returns the wanted files from an archive', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://gitlab.com/backstage/mock/tree/repo', + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main', ); const files = await response.files(); expect(files.length).toBe(2); - const indexMarkdownFile = await files[0].content(); - const mkDocsFile = await files[1].content(); + const mkDocsFile = await files[0].content(); + const indexMarkdownFile = await files[1].content(); expect(mkDocsFile.toString()).toBe('site_name: Test\n'); expect(indexMarkdownFile.toString()).toBe('# Test\n'); @@ -177,23 +267,18 @@ describe('GitlabUrlReader', () => { it('returns the wanted files from hosted gitlab', async () => { worker.use( rest.get( - 'https://git.mycompany.com/backstage/mock/-/archive/repo/mock-repo.zip', + 'https://gitlab.mycompany.com/backstage/mock/-/archive/main.zip', (_, res, ctx) => res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), + ctx.body(archiveBuffer), ), ), ); - const processor = new GitlabUrlReader( - { host: 'git.mycompany.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://git.mycompany.com/backstage/mock/tree/repo/docs', + const response = await hostedGitlabProcessor.readTree( + 'https://gitlab.mycompany.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -204,27 +289,9 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws an error when branch is not specified', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - await expect( - processor.readTree('https://gitlab.com/backstage/mock'), - ).rejects.toThrow( - 'GitLab URL must contain a branch to be able to fetch its tree', - ); - }); - it('returns the wanted files from an archive with a subpath', async () => { - const processor = new GitlabUrlReader( - { host: 'gitlab.com' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( - 'https://gitlab.com/backstage/mock/tree/repo/docs', + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -234,5 +301,51 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnGitlab = async () => { + await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', { + sha: 'sha123abc', + }); + }; + + const fnHostedGitlab = async () => { + await hostedGitlabProcessor.readTree( + 'https://gitlab.mycompany.com/backstage/mock', + { + sha: 'sha123abc', + }, + ); + }; + + await expect(fnGitlab).rejects.toThrow(NotModifiedError); + await expect(fnHostedGitlab).rejects.toThrow(NotModifiedError); + }); + + it('should not throw error when given an outdated sha in options', async () => { + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/main', + { + sha: 'outdatedSha123abc', + }, + ); + expect((await response.files()).length).toBe(2); + }); + + it('should detect the default branch', async () => { + const response = await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock', + ); + expect((await response.files()).length).toBe(2); + }); + + it('should throw error on missing branch', async () => { + const fnGithub = async () => { + await gitlabProcessor.readTree( + 'https://gitlab.com/backstage/mock/tree/branchDoesNotExist', + ); + }; + await expect(fnGithub).rejects.toThrow(NotFoundError); + }); }); }); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 76cfea7c38..0c0ce4f565 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -21,7 +21,7 @@ import { readGitLabIntegrationConfigs, } from '@backstage/integration'; import fetch from 'cross-fetch'; -import { InputError, NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -39,26 +39,26 @@ export class GitlabUrlReader implements UrlReader { const configs = readGitLabIntegrationConfigs( config.getOptionalConfigArray('integrations.gitlab') ?? [], ); - return configs.map(options => { - const reader = new GitlabUrlReader(options, { treeResponseFactory }); - const predicate = (url: URL) => url.host === options.host; + return configs.map(provider => { + const reader = new GitlabUrlReader(provider, { treeResponseFactory }); + const predicate = (url: URL) => url.host === provider.host; return { reader, predicate }; }); }; constructor( - private readonly options: GitLabIntegrationConfig, + private readonly config: GitLabIntegrationConfig, deps: { treeResponseFactory: ReadTreeResponseFactory }, ) { this.treeResponseFactory = deps.treeResponseFactory; } async read(url: string): Promise { - const builtUrl = await getGitLabFileFetchUrl(url, this.options); + const builtUrl = await getGitLabFileFetchUrl(url, this.config); let response: Response; try { - response = await fetch(builtUrl, getGitLabRequestOptions(this.options)); + response = await fetch(builtUrl, getGitLabRequestOptions(this.config)); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); } @@ -78,35 +78,71 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const { - name: repoName, - ref, - protocol, - resource, - full_name, - filepath, - } = parseGitUrl(url); + const { name: repoName, ref, full_name, filepath } = parseGitUrl(url); - if (!ref) { - throw new InputError( - 'GitLab URL must contain a branch to be able to fetch its tree', - ); - } - - const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`; - const archiveGitLabResponse = await fetch( - archive, - getGitLabRequestOptions(this.options), + // Use GitLab API to get the default branch + // encodeURIComponent is required for GitLab API + // https://docs.gitlab.com/ee/api/README.html#namespaced-path-encoding + const projectGitlabResponse = await fetch( + new URL( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent(full_name)}`, + ).toString(), + getGitLabRequestOptions(this.config), ); - if (!archiveGitLabResponse.ok) { - const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; - if (archiveGitLabResponse.status === 404) { + if (!projectGitlabResponse.ok) { + const msg = `Failed to read tree from ${url}, ${projectGitlabResponse.status} ${projectGitlabResponse.statusText}`; + if (projectGitlabResponse.status === 404) { throw new NotFoundError(msg); } throw new Error(msg); } + const projectGitlabResponseJson = await projectGitlabResponse.json(); - const path = filepath ? `${repoName}-${ref}/${filepath}/` : ''; + // ref is an empty string if no branch is set in provided url to readTree. + const branch = ref === '' ? projectGitlabResponseJson.default_branch : ref; + + // Fetch the latest commit in the provided or default branch to compare against + // the provided sha. + const branchGitlabResponse = await fetch( + new URL( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent( + full_name, + )}/repository/branches/${branch}`, + ).toString(), + getGitLabRequestOptions(this.config), + ); + if (!branchGitlabResponse.ok) { + const message = `Failed to read tree (branch) from ${url}, ${branchGitlabResponse.status} ${branchGitlabResponse.statusText}`; + if (branchGitlabResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } + + const commitSha = (await branchGitlabResponse.json()).commit.id; + + if (options?.sha && options.sha === commitSha) { + throw new NotModifiedError(); + } + + // https://docs.gitlab.com/ee/api/repositories.html#get-file-archive + const archiveGitLabResponse = await fetch( + `${this.config.apiBaseUrl}/projects/${encodeURIComponent( + full_name, + )}/repository/archive.zip?sha=${branch}`, + getGitLabRequestOptions(this.config), + ); + if (!archiveGitLabResponse.ok) { + const message = `Failed to read tree (archive) from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; + if (archiveGitLabResponse.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); + } + + const path = filepath + ? `${repoName}-${branch}-${commitSha}/${filepath}/` + : ''; const archiveResponse = await this.treeResponseFactory.fromZipArchive({ stream: (archiveGitLabResponse.body as unknown) as Readable, @@ -115,13 +151,12 @@ export class GitlabUrlReader implements UrlReader { }); const response = archiveResponse as ReadTreeResponse; - // TODO: Just a placeholder for now. - response.sha = ''; + response.sha = commitSha; return response; } toString() { - const { host, token } = this.options; + const { host, token } = this.config; return `gitlab{host=${host},authed=${Boolean(token)}}`; } } diff --git a/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip b/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip new file mode 100644 index 0000000000..884ec20004 Binary files /dev/null and b/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip differ diff --git a/packages/backend-common/src/reading/__fixtures__/mock-main.zip b/packages/backend-common/src/reading/__fixtures__/mock-main.zip new file mode 100644 index 0000000000..beee59d3a0 Binary files /dev/null and b/packages/backend-common/src/reading/__fixtures__/mock-main.zip differ diff --git a/packages/backend-common/src/reading/__fixtures__/repo.zip b/packages/backend-common/src/reading/__fixtures__/repo.zip deleted file mode 100644 index f66bf2d612..0000000000 Binary files a/packages/backend-common/src/reading/__fixtures__/repo.zip and /dev/null differ diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 6c2592ffce..345ec7a783 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path'; import { ZipArchiveResponse } from './ZipArchiveResponse'; const archiveData = fs.readFileSync( - resolvePath(__filename, '../../__fixtures__/repo.zip'), + resolvePath(__filename, '../../__fixtures__/mock-main.zip'), ); describe('ZipArchiveResponse', () => { @@ -38,30 +38,30 @@ describe('ZipArchiveResponse', () => { it('should read files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); const files = await res.files(); expect(files).toEqual([ { - path: 'docs/index.md', + path: 'mkdocs.yml', content: expect.any(Function), }, { - path: 'mkdocs.yml', + path: 'docs/index.md', content: expect.any(Function), }, ]); const contents = await Promise.all(files.map(f => f.content())); expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - '# Test', 'site_name: Test', + '# Test', ]); }); it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,7 +79,7 @@ describe('ZipArchiveResponse', () => { it('should read as archive and files', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( @@ -91,18 +91,18 @@ describe('ZipArchiveResponse', () => { expect(files).toEqual([ { - path: 'docs/index.md', + path: 'mkdocs.yml', content: expect.any(Function), }, { - path: 'mkdocs.yml', + path: 'docs/index.md', content: expect.any(Function), }, ]); const contents = await Promise.all(files.map(f => f.content())); expect(contents.map(c => c.toString('utf8').trim())).toEqual([ - '# Test', 'site_name: Test', + '# Test', ]); }); @@ -113,17 +113,17 @@ describe('ZipArchiveResponse', () => { const dir = await res.dir(); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/mkdocs.yml'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'), ).resolves.toBe('site_name: Test\n'); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/docs/index.md'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/docs/index.md'), 'utf8'), ).resolves.toBe('# Test\n'); }); it('should extract archive into directory with a subpath', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/docs/', '/tmp'); + const res = new ZipArchiveResponse(stream, 'mock-main/docs/', '/tmp'); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,7 +135,7 @@ describe('ZipArchiveResponse', () => { it('should extract archive into directory with a subpath and filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index c727ba078b..d5c24ed1db 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -83,6 +83,10 @@ export type ReadTreeResponseDirOptions = { export type ReadTreeArchiveResponse = { files(): Promise; archive(): Promise; + + /** + * dir() extracts the tree response into a directory and returns the path of the directory. + */ dir(options?: ReadTreeResponseDirOptions): Promise; };