Skip to content

Commit

Permalink
fix(owl-bot): OwlBot to report a skip in pull request checks (#5643)
Browse files Browse the repository at this point in the history
- **fix: OwlBot postprocessor to report a check when skipping a PR**

b/385183332. After
#5587 and before
this change, the post processor checks were silently skipped and those
repositories that mark it as required cannot merge pull requests from
external contributors.
  • Loading branch information
suztomo authored Jan 6, 2025
1 parent 508bcc9 commit c9fc622
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 10 deletions.
3 changes: 3 additions & 0 deletions packages/owl-bot/cloudbuild-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,8 @@ steps:
- "-t"
- "gcr.io/$PROJECT_ID/owl-bot"
- "."
logsBucket: 'gs://owl-bot-deploy-logs'
options:
logging: GCS_ONLY

# TODO: run container test and functional/integration test
2 changes: 1 addition & 1 deletion packages/owl-bot/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface CheckArgs {
pr: number;
repo: string;
summary: string;
conclusion: 'success' | 'failure';
conclusion: 'success' | 'failure' | 'skipped';
detailsURL: string;
text: string;
title: string;
Expand Down
24 changes: 16 additions & 8 deletions packages/owl-bot/src/owl-bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -615,14 +615,6 @@ const runPostProcessor = async (
logger: GCFLogger,
breakLoop = true
) => {
// If the pull request is from a fork and not from allowlist, skip.
const forkOwner = opts.head.split('/')[0];
if (opts.head !== opts.base && !ALLOWED_FORK_OWNERS.includes(forkOwner)) {
logger.info(
`head ${opts.head} does not match base ${opts.base}, skipping PR from fork`
);
return;
}
// Fetch the .Owlbot.lock.yaml from head of PR:
let lock: OwlBotLock | undefined = undefined;
async function createCheck(
Expand Down Expand Up @@ -658,6 +650,22 @@ const runPostProcessor = async (
logger.info(`Created ${logLine}`);
}

// If the pull request is from a fork and not from allowlist, skip.
const forkOwner = opts.head.split('/')[0];
if (opts.head !== opts.base && !ALLOWED_FORK_OWNERS.includes(forkOwner)) {
logger.info(
`head ${opts.head} does not match base ${opts.base}, skipping PR from fork`
);
await createCheck({
text: 'Ignored by Owl Bot because the pull request was created from a fork. See go/owlbot-skip-forks.',
summary:
'Ignored by Owl Bot because the pull request was created from a fork',
conclusion: 'skipped',
title: '🦉 OwlBot - skipped this pull request from a fork',
});
return;
}

// Attempt to load the OwlBot lock file for a repository, if the lock
// file is corrupt an error will be thrown and we should show a failing
// status:
Expand Down
12 changes: 11 additions & 1 deletion packages/owl-bot/test/owl-bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,17 @@ describe('OwlBot', () => {
sandbox.match(/.*skipping PR from fork*/)
);
sandbox.assert.notCalled(triggerBuildStub);
sandbox.assert.notCalled(createCheckStub);
sandbox.assert.calledWith(
createCheckStub,
sinon.match.has('conclusion', 'skipped')
);
sandbox.assert.calledWith(
createCheckStub,
sinon.match.has(
'text',
'Ignored by Owl Bot because the pull request was created from a fork. See go/owlbot-skip-forks.'
)
);
sandbox.assert.notCalled(hasOwlBotLoopStub);
sandbox.assert.notCalled(updatePullRequestStub);
githubMock.done();
Expand Down

0 comments on commit c9fc622

Please sign in to comment.