diff --git a/.changeset/brave-forks-pull.md b/.changeset/brave-forks-pull.md new file mode 100644 index 0000000000..b7088f2721 --- /dev/null +++ b/.changeset/brave-forks-pull.md @@ -0,0 +1,9 @@ +--- +'@backstage/backend-common': patch +--- + +Support commit hashes at `GithubUrlReader.readTree/search` additionally to branch names. + +Additionally, this will reduce the number of API calls from 2 to 1 for retrieving the "repo details" +for all cases besides when the default branch has to be resolved and used +(e.g., repo URL without any branch or commit hash). diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 1ee25920f2..499f97d6ec 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -30,7 +30,7 @@ import path from 'path'; import { NotFoundError, NotModifiedError } from '@backstage/errors'; import { GhBlobResponse, - GhBranchResponse, + GhCombinedCommitStatusResponse, GhRepoResponse, GhTreeResponse, GithubUrlReader, @@ -309,12 +309,15 @@ describe('GithubUrlReader', () => { 'https://ghe.github.com/api/v3/repos/backstage/mock/{archive_format}{/ref}', } as Partial; - const branchesApiResponse = { - name: 'main', - commit: { - sha: 'etag123abc', - }, - } as Partial; + const commitStatusGithubResponse = { + sha: 'etag123abc', + repository: reposGithubApiResponse, + } as Partial; + + const commitStatusGheResponse = { + sha: 'etag123abc', + repository: reposGheApiResponse, + } as Partial; beforeEach(() => { worker.use( @@ -326,13 +329,18 @@ describe('GithubUrlReader', () => { ), ), rest.get( - 'https://api.github.com/repos/backstage/mock/branches/main', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/json'), - ctx.json(branchesApiResponse), - ), + 'https://api.github.com/repos/backstage/mock/commits/main/status', + (req, res, ctx) => { + if (req.url.searchParams.get('per_page') === '0') { + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(commitStatusGithubResponse), + ); + } + + return res(ctx.status(500)); + }, ), rest.get( 'https://api.github.com/repos/backstage/mock/tarball/etag123abc', @@ -348,7 +356,7 @@ describe('GithubUrlReader', () => { ), ), rest.get( - 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', + 'https://api.github.com/repos/backstage/mock/commits/branchDoesNotExist/status', (_, res, ctx) => res(ctx.status(404)), ), rest.get( @@ -374,13 +382,18 @@ describe('GithubUrlReader', () => { ), ), rest.get( - 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/json'), - ctx.json(branchesApiResponse), - ), + 'https://ghe.github.com/api/v3/repos/backstage/mock/commits/main/status', + (req, res, ctx) => { + if (req.url.searchParams.get('per_page') === '0') { + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(commitStatusGheResponse), + ); + } + + return res(ctx.status(500)); + }, ), ); }); @@ -684,36 +697,69 @@ describe('GithubUrlReader', () => { ); }); - // Branch details + // commit status for branch details beforeEach(() => { - const response = { - name: 'main', - commit: { - sha: 'etag123abc', + const githubResponse = { + sha: 'etag123abc', + repository: { + id: 123, + full_name: 'backstage/mock', + default_branch: 'main', + branches_url: + 'https://api.github.com/repos/backstage/mock/branches{/branch}', + archive_url: + 'https://api.github.com/repos/backstage/mock/{archive_format}{/ref}', + trees_url: + 'https://api.github.com/repos/backstage/mock/git/trees{/sha}', }, - } as Partial; + } as Partial; + + const gheResponse = { + sha: 'etag123abc', + repository: { + id: 123, + full_name: 'backstage/mock', + default_branch: 'main', + branches_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}', + archive_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/{archive_format}{/ref}', + trees_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/git/trees{/sha}', + }, + } as Partial; worker.use( rest.get( - 'https://api.github.com/repos/backstage/mock/branches/main', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/json'), - ctx.json(response), - ), + 'https://api.github.com/repos/backstage/mock/commits/main/status', + (req, res, ctx) => { + if (req.url.searchParams.get('per_page') === '0') { + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(githubResponse), + ); + } + + return res(ctx.status(500)); + }, ), rest.get( - 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/json'), - ctx.json(response), - ), + 'https://ghe.github.com/api/v3/repos/backstage/mock/commits/main/status', + (req, res, ctx) => { + if (req.url.searchParams.get('per_page') === '0') { + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(gheResponse), + ); + } + + return res(ctx.status(500)); + }, ), rest.get( - 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', + 'https://api.github.com/repos/backstage/mock/commits/branchDoesNotExist/status', (_, res, ctx) => res(ctx.status(404)), ), ); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index ee781daf0b..8300127c5b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -20,6 +20,7 @@ import { GithubCredentialsProvider, GithubIntegration, ScmIntegrations, + GithubCredentials, } from '@backstage/integration'; import { RestEndpointMethodTypes } from '@octokit/rest'; import fetch, { RequestInit, Response } from 'node-fetch'; @@ -44,8 +45,8 @@ import { parseLastModified } from './util'; export type GhRepoResponse = RestEndpointMethodTypes['repos']['get']['response']['data']; -export type GhBranchResponse = - RestEndpointMethodTypes['repos']['getBranch']['response']['data']; +export type GhCombinedCommitStatusResponse = + RestEndpointMethodTypes['repos']['getCombinedStatusForRef']['response']['data']; export type GhTreeResponse = RestEndpointMethodTypes['git']['getTree']['response']['data']; export type GhBlobResponse = @@ -163,7 +164,7 @@ export class GithubUrlReader implements UrlReader { options?: ReadTreeOptions, ): Promise { const repoDetails = await this.getRepoDetails(url); - const commitSha = repoDetails.branch.commit.sha!; + const commitSha = repoDetails.commitSha; if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); @@ -191,7 +192,7 @@ export class GithubUrlReader implements UrlReader { async search(url: string, options?: SearchOptions): Promise { const repoDetails = await this.getRepoDetails(url); - const commitSha = repoDetails.branch.commit.sha!; + const commitSha = repoDetails.commitSha; if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); @@ -302,32 +303,43 @@ export class GithubUrlReader implements UrlReader { } private async getRepoDetails(url: string): Promise<{ - repo: GhRepoResponse; - branch: GhBranchResponse; + commitSha: string; + repo: { + archive_url: string; + trees_url: string; + }; }> { const parsed = parseGitUrl(url); const { ref, full_name } = parsed; - // Caveat: The ref will totally be incorrect if the branch name includes a - // slash. Thus, some operations can not work on URLs containing branch - // names that have a slash in them. - - const { headers } = await this.deps.credentialsProvider.getCredentials({ + const credentials = await this.deps.credentialsProvider.getCredentials({ url, }); + const { headers } = credentials; + const commitStatus: GhCombinedCommitStatusResponse = await this.fetchJson( + `${this.integration.config.apiBaseUrl}/repos/${full_name}/commits/${ + ref || (await this.getDefaultBranch(full_name, credentials)) + }/status?per_page=0`, + { headers }, + ); + + return { + commitSha: commitStatus.sha, + repo: commitStatus.repository, + }; + } + + private async getDefaultBranch( + repoFullName: string, + credentials: GithubCredentials, + ): Promise { const repo: GhRepoResponse = await this.fetchJson( - `${this.integration.config.apiBaseUrl}/repos/${full_name}`, - { headers }, + `${this.integration.config.apiBaseUrl}/repos/${repoFullName}`, + { headers: credentials.headers }, ); - // branches_url looks like "https://api.github.com/repos/owner/repo/branches{/branch}" - const branch: GhBranchResponse = await this.fetchJson( - repo.branches_url.replace('{/branch}', `/${ref || repo.default_branch}`), - { headers }, - ); - - return { repo, branch }; + return repo.default_branch; } private async fetchResponse(