From d81eec8680ba75d558b616b2408cfc5bb628b9a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Tue, 13 Dec 2022 14:33:27 +0100 Subject: [PATCH 1/3] Fetch only the latest checks --- dist/check-group/core/index.js | 11 +++++++++-- src/check-group/core/index.ts | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/dist/check-group/core/index.js b/dist/check-group/core/index.js index 23e43212..c1c494b2 100644 --- a/dist/check-group/core/index.js +++ b/dist/check-group/core/index.js @@ -78,6 +78,7 @@ var CheckGroup = /** @class */ (function () { this.intervalTimer = setTimeout(function () { return ''; }, 0); this.timeoutTimer = setTimeout(function () { return ''; }, 0); this.inputs = {}; + this.canComment = true; this.pullRequestNumber = pullRequestNumber; this.config = config; this.context = context; @@ -180,7 +181,11 @@ var CheckGroup = /** @class */ (function () { e_1 = _a.sent(); if (e_1 instanceof request_error_1.RequestError && e_1.status === 403) { // Forbidden: Resource not accessible by integration - core.info("Failed to comment on the PR: ".concat(JSON.stringify(e_1))); + if (this.canComment) { + core.info("Failed to comment on the PR: ".concat(JSON.stringify(e_1))); + } + // Use this boolean to only print the info message once + this.canComment = false; } else { throw e_1; @@ -223,7 +228,9 @@ var getPostedChecks = function (context, sha) { return __awaiter(void 0, void 0, var checkRuns, checkNames; return __generator(this, function (_a) { switch (_a.label) { - case 0: return [4 /*yield*/, context.octokit.paginate(context.octokit.checks.listForRef, context.repo({ ref: sha }), function (response) { return response.data; })]; + case 0: return [4 /*yield*/, context.octokit.paginate(context.octokit.checks.listForRef, + // only the latest runs, in case it was run multiple times + context.repo({ ref: sha, filter: "latest" }), function (response) { return response.data; })]; case 1: checkRuns = _a.sent(); core.debug("checkRuns: ".concat(JSON.stringify(checkRuns))); diff --git a/src/check-group/core/index.ts b/src/check-group/core/index.ts index fa87b820..300adf92 100644 --- a/src/check-group/core/index.ts +++ b/src/check-group/core/index.ts @@ -27,6 +27,8 @@ export class CheckGroup { timeoutTimer: ReturnType = setTimeout(() => '', 0); inputs: Record = {}; + canComment: boolean = true; + constructor( pullRequestNumber: number, config: CheckGroupConfig, @@ -119,7 +121,11 @@ export class CheckGroup { } catch (e) { if (e instanceof RequestError && e.status === 403) { // Forbidden: Resource not accessible by integration - core.info(`Failed to comment on the PR: ${JSON.stringify(e)}`) + if (this.canComment) { + core.info(`Failed to comment on the PR: ${JSON.stringify(e)}`) + } + // Use this boolean to only print the info message once + this.canComment = false } else { throw e } @@ -154,7 +160,8 @@ export {fetchConfig}; const getPostedChecks = async (context: Context, sha: string): Promise> => { const checkRuns = await context.octokit.paginate( context.octokit.checks.listForRef, - context.repo({ref: sha}), + // only the latest runs, in case it was run multiple times + context.repo({ref: sha, filter: "latest"}), (response) => response.data, ); core.debug(`checkRuns: ${JSON.stringify(checkRuns)}`) From df2344816dfb8d38c61578219b833df61297d0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Fri, 16 Dec 2022 15:32:53 +0100 Subject: [PATCH 2/3] Fetch the config from the target branch on forks --- dist/check-group/core/config_getter.js | 28 ++++++++++++++++++-------- src/check-group/core/config_getter.ts | 18 ++++++++++------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/dist/check-group/core/config_getter.js b/dist/check-group/core/config_getter.js index 2935e64b..ee437f86 100644 --- a/dist/check-group/core/config_getter.js +++ b/dist/check-group/core/config_getter.js @@ -80,12 +80,11 @@ var core = __importStar(require("@actions/core")); * @returns The configuration or default configuration if non exists. */ var fetchConfig = function (context) { return __awaiter(void 0, void 0, void 0, function () { - var configData, filename, payload, repoFullName, githubRepository, prBranch, params, config; + var configData, payload, repoFullName, githubRepository, prBranch, baseBranch; return __generator(this, function (_a) { switch (_a.label) { case 0: configData = undefined; - filename = "checkgroup.yml"; payload = context.payload; repoFullName = payload.pull_request.head.repo.full_name; githubRepository = payload.pull_request.base.repo.full_name; @@ -93,15 +92,15 @@ var fetchConfig = function (context) { return __awaiter(void 0, void 0, void 0, if (!(repoFullName == githubRepository)) return [3 /*break*/, 2]; prBranch = payload.pull_request.head.ref; core.info("The PR is from a branch in the repository. Reading the config in ".concat(prBranch)); - params = context.repo({ path: ".github/".concat(filename) }); - return [4 /*yield*/, context.octokit.config.get(__assign(__assign({}, params), { branch: prBranch }))]; + return [4 /*yield*/, readConfig(context, prBranch)]; case 1: - config = (_a.sent()).config; - configData = config; + configData = _a.sent(); return [3 /*break*/, 4]; - case 2: return [4 /*yield*/, context.config(filename)]; + case 2: + baseBranch = payload.pull_request.base.ref; + core.info("The PR is from a fork (".concat(repoFullName, "). For security, reading the config in ").concat(baseBranch)); + return [4 /*yield*/, readConfig(context, baseBranch)]; case 3: - // this will pull the config from master configData = _a.sent(); _a.label = 4; case 4: @@ -111,3 +110,16 @@ var fetchConfig = function (context) { return __awaiter(void 0, void 0, void 0, }); }); }; exports.fetchConfig = fetchConfig; +var readConfig = function (context, branch) { return __awaiter(void 0, void 0, void 0, function () { + var params, config; + return __generator(this, function (_a) { + switch (_a.label) { + case 0: + params = context.repo({ path: '.github/checkgroup.yml' }); + return [4 /*yield*/, context.octokit.config.get(__assign(__assign({}, params), { branch: branch }))]; + case 1: + config = (_a.sent()).config; + return [2 /*return*/, config]; + } + }); +}); }; diff --git a/src/check-group/core/config_getter.ts b/src/check-group/core/config_getter.ts index c1668bc0..153c6976 100644 --- a/src/check-group/core/config_getter.ts +++ b/src/check-group/core/config_getter.ts @@ -12,7 +12,6 @@ import * as core from '@actions/core' */ export const fetchConfig = async (context: Context): Promise => { let configData: Record = undefined - const filename = "checkgroup.yml" const payload = context.payload as PullRequestEvent; const repoFullName = payload.pull_request.head.repo.full_name const githubRepository = payload.pull_request.base.repo.full_name @@ -20,14 +19,19 @@ export const fetchConfig = async (context: Context): Promise = if (repoFullName == githubRepository) { const prBranch = payload.pull_request.head.ref; core.info(`The PR is from a branch in the repository. Reading the config in ${prBranch}`) - const params = context.repo({path: `.github/${filename}`}) - // https://github.com/probot/octokit-plugin-config - const { config } = await context.octokit.config.get({...params, branch: prBranch}) - configData = config + configData = await readConfig(context, prBranch) } else { - // this will pull the config from master - configData = await context.config(filename); + const baseBranch = payload.pull_request.base.ref; + core.info(`The PR is from a fork (${repoFullName}). For security, reading the config in ${baseBranch}`) + configData = await readConfig(context, baseBranch) } core.debug(`configData: ${JSON.stringify(configData)}`) return parseUserConfig(configData); }; + +const readConfig = async (context: Context, branch: string): Promise> => { + const params = context.repo({path: '.github/checkgroup.yml'}) + // https://github.com/probot/octokit-plugin-config + const { config } = await context.octokit.config.get({...params, branch: branch}) + return config +} \ No newline at end of file From 2ed6221d7c53cd59d0f584a40a84a93d8b5a18f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Fri, 16 Dec 2022 15:37:28 +0100 Subject: [PATCH 3/3] Formatting --- src/check-group/core/config_getter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/check-group/core/config_getter.ts b/src/check-group/core/config_getter.ts index 153c6976..2f4742fb 100644 --- a/src/check-group/core/config_getter.ts +++ b/src/check-group/core/config_getter.ts @@ -18,11 +18,11 @@ export const fetchConfig = async (context: Context): Promise = core.debug(`fetchConfig ${repoFullName} ${githubRepository}`) if (repoFullName == githubRepository) { const prBranch = payload.pull_request.head.ref; - core.info(`The PR is from a branch in the repository. Reading the config in ${prBranch}`) + core.info(`The PR is from a branch in the repository. Reading the config in '${prBranch}'`) configData = await readConfig(context, prBranch) } else { const baseBranch = payload.pull_request.base.ref; - core.info(`The PR is from a fork (${repoFullName}). For security, reading the config in ${baseBranch}`) + core.info(`The PR is from a fork: '${repoFullName}'. For security, reading the config in '${baseBranch}'`) configData = await readConfig(context, baseBranch) } core.debug(`configData: ${JSON.stringify(configData)}`)