fix(dev-infra): use API pagination for retrieving project branches (#42666)

We rely on a Github API `/branches` request to determine the active
release trains. Currently this logic is broken if more than 100
protected branches exist within a repository. This issue surfaced
recently where the `items_per_page` setting was set to `30`, causing
the merge tooling and release tooling to not detect the proper "latest"
release train.

This commit uses Github pagination for retrieving branches to determine
the active release trains, and makes the logic more long-term proof.

PR Close #42666
This commit is contained in:
Paul Gschwendtner 2021-06-26 00:48:24 +02:00 committed by Jessica Janiuk
parent 279e63f65f
commit f29fe5ced0
5 changed files with 69 additions and 9 deletions

View File

@ -899,9 +899,7 @@ function getVersionForVersionBranch(branchName) {
*/ */
function getBranchesForMajorVersions(repo, majorVersions) { function getBranchesForMajorVersions(repo, majorVersions) {
return tslib.__awaiter(this, void 0, void 0, function* () { return tslib.__awaiter(this, void 0, void 0, function* () {
// TODO(alxhub): actually paginate this, since eventually the number of branches we have will run const branchData = yield repo.api.paginate(repo.api.repos.listBranches, { owner: repo.owner, repo: repo.name, protected: true });
// off the end of the first page of data returned by `listBranches`.
const { data: branchData } = yield repo.api.repos.listBranches({ owner: repo.owner, repo: repo.name, protected: true, per_page: 100 });
const branches = []; const branches = [];
for (const { name } of branchData) { for (const { name } of branchData) {
if (!isVersionBranch(name)) { if (!isVersionBranch(name)) {

View File

@ -38,6 +38,7 @@ ts_library(
"//dev-infra/release/config", "//dev-infra/release/config",
"//dev-infra/release/versioning", "//dev-infra/release/versioning",
"//dev-infra/utils", "//dev-infra/utils",
"//dev-infra/utils/testing",
"@npm//@types/jasmine", "@npm//@types/jasmine",
"@npm//@types/node", "@npm//@types/node",
"@npm//@types/node-fetch", "@npm//@types/node-fetch",

View File

@ -7,12 +7,14 @@
*/ */
import * as nock from 'nock'; import * as nock from 'nock';
import {ParsedUrlQuery} from 'querystring';
import {ReleaseConfig} from '../../../release/config/index'; import {ReleaseConfig} from '../../../release/config/index';
import {_npmPackageInfoCache, NpmPackageInfo} from '../../../release/versioning/npm-registry'; import {_npmPackageInfoCache, NpmPackageInfo} from '../../../release/versioning/npm-registry';
import {GithubConfig} from '../../../utils/config'; import {GithubConfig} from '../../../utils/config';
import * as console from '../../../utils/console'; import * as console from '../../../utils/console';
import {GithubClient} from '../../../utils/git/github'; import {GithubClient} from '../../../utils/git/github';
import {buildGithubPaginationResponseHeader} from '../../../utils/testing/github-pagination-header';
import {TargetLabel} from '../config'; import {TargetLabel} from '../config';
import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest} from '../target-label'; import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest} from '../target-label';
@ -79,7 +81,35 @@ describe('default target labels', () => {
nock(getRepoApiRequestUrl()) nock(getRepoApiRequestUrl())
.get('/branches') .get('/branches')
.query(true) .query(true)
.reply(200, branches.map(name => ({name}))); .reply(200, branches.slice(0, 29).map(name => ({name})));
}
/**
* Mocks a repository branch list API request with pagination.
* https://docs.github.com/en/rest/guides/traversing-with-pagination.
* https://docs.github.com/en/rest/reference/repos#list-branches.
*/
function interceptBranchesListRequestWithPagination(branches: string[]) {
const apiUrl = getRepoApiRequestUrl();
// For each branch, create its own API page so that pagination is required
// to resolve all given branches.
for (let index = 0; index < branches.length; index++) {
// Pages start with `1` as per the Github API specification.
const pageNum = index + 1;
const name = branches[index];
const linkHeader =
buildGithubPaginationResponseHeader(branches.length, pageNum, `${apiUrl}/branches`);
// For the first page, either `?page=1` needs to be set, or no `page` should be specified.
const queryMatch = pageNum === 1 ?
(params: ParsedUrlQuery) => params.page === '1' || params.page === undefined :
{page: pageNum};
nock(getRepoApiRequestUrl()).get('/branches').query(queryMatch).reply(200, [{name}], {
link: linkHeader,
});
}
} }
async function getBranchesForLabel( async function getBranchesForLabel(
@ -99,7 +129,12 @@ describe('default target labels', () => {
it('should detect "master" as branch for target: minor', async () => { it('should detect "master" as branch for target: minor', async () => {
interceptBranchVersionRequest('master', '11.0.0-next.0'); interceptBranchVersionRequest('master', '11.0.0-next.0');
interceptBranchVersionRequest('10.2.x', '10.2.4'); interceptBranchVersionRequest('10.2.x', '10.2.4');
interceptBranchesListRequest(['10.2.x']);
// Note: We add a few more branches here to ensure that branches API requests are
// paginated properly. In Angular projects, there are usually many branches so that
// pagination is ultimately needed to detect the active release trains.
// See: https://github.com/angular/angular/commit/261b060fa168754db00248d1c5c9574bb19a72b4.
interceptBranchesListRequestWithPagination(['9.8.x', '10.1.x', '10.2.x']);
expect(await getBranchesForLabel('target: minor')).toEqual(['master']); expect(await getBranchesForLabel('target: minor')).toEqual(['master']);
}); });

View File

@ -72,10 +72,8 @@ export function getVersionForVersionBranch(branchName: string): semver.SemVer|nu
*/ */
export async function getBranchesForMajorVersions( export async function getBranchesForMajorVersions(
repo: GithubRepoWithApi, majorVersions: number[]): Promise<VersionBranch[]> { repo: GithubRepoWithApi, majorVersions: number[]): Promise<VersionBranch[]> {
// TODO(alxhub): actually paginate this, since eventually the number of branches we have will run const branchData = await repo.api.paginate(
// off the end of the first page of data returned by `listBranches`. repo.api.repos.listBranches, {owner: repo.owner, repo: repo.name, protected: true});
const {data: branchData} = await repo.api.repos.listBranches(
{owner: repo.owner, repo: repo.name, protected: true, per_page: 100});
const branches: VersionBranch[] = []; const branches: VersionBranch[] = [];
for (const {name} of branchData) { for (const {name} of branchData) {

View File

@ -0,0 +1,28 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
/**
* Builds the `Link` response header to indicate an API response that is suitable
* for pagination. This follows the specification as outlined within:
* https://docs.github.com/en/rest/guides/traversing-with-pagination
*/
export function buildGithubPaginationResponseHeader(
totalPages: number, currentPage: number, baseUrl: string) {
const links = [`<${baseUrl}?page=1>; rel="first"`, `<${baseUrl}?page=${totalPages}>; rel="last"`];
if (currentPage < totalPages) {
links.push(`<${baseUrl}?page=${currentPage + 1}>; rel="next"`);
}
// Pages start with `1` as per the Github API specification.
if (currentPage > 1) {
links.push(`<${baseUrl}?page=${currentPage - 1}>; rel="prev"`);
}
return links.join(',');
}