Skip to content

Commit

Permalink
feat(platform/github): flag to control whether PRs can be edited by m…
Browse files Browse the repository at this point in the history
…aintainers if `forkToken`is set (#19771)
  • Loading branch information
devversion authored Feb 22, 2023
1 parent 27eda56 commit 340a913
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 3 deletions.
16 changes: 16 additions & 0 deletions docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,22 @@ Renovate follows tags _strictly_, this can cause problems when a tagged stream i
For example: you're following the `next` tag, but later the stream you actually want is called `stable` instead.
If `next` is no longer getting updates, you must switch your `followTag` to `stable` to get updates again.

## forkModeDisallowMaintainerEdits

Use `forkModeDisallowMaintainerEdits` to disallow maintainers from editing Renovate's pull requests when in fork mode.

If GitHub pull requests are created from a [fork repository](https://docs.github.com/en/get-started/quickstart/fork-a-repo), the PR author can decide to allow upstream repository to modify the PR directly.

Allowing maintainers to edit pull requests directly is helpful when Renovate pull requests require additional changes.
The reviewer can simply push to the pull request without having to create a new PR. [More details here](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork).

You may decide to disallow edits to Renovate pull requests in order to workaround issues in Renovate where modified fork branches are not deleted properly: [See this issue](https://github.com/renovatebot/renovate/issues/16657).
If this option is enabled, reviewers will need to create a new PR if additional changes are needed.

<!-- prettier-ignore -->
!!! note
This option is only relevant if you set `forkToken`.

## gitAuthor

You can customize the Git author that's used whenever Renovate creates a commit.
Expand Down
8 changes: 8 additions & 0 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,14 @@ const options: RenovateOptions[] = [
default: false,
supportedPlatforms: ['gitlab'],
},
{
name: 'forkModeDisallowMaintainerEdits',
description:
'Disallow maintainers to push to Renovate pull requests when running in fork mode.',
type: 'boolean',
supportedPlatforms: ['github'],
default: false,
},
{
name: 'confidential',
description:
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export interface RenovateConfig
postUpdateOptions?: string[];
prConcurrentLimit?: number;
prHourlyLimit?: number;
forkModeDisallowMaintainerEdits?: boolean;

defaultRegistryUrls?: string[];
registryUrls?: string[] | null;
Expand Down
94 changes: 92 additions & 2 deletions lib/modules/platform/github/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ describe('modules/platform/github/index', () => {
repository: string,
forkExisted: boolean,
forkResult = 200,
forkDefaulBranch = 'master'
forkDefaultBranch = 'master'
): void {
scope
// repo info
Expand Down Expand Up @@ -307,7 +307,7 @@ describe('modules/platform/github/index', () => {
{
full_name: 'forked/repo',
owner: { login: 'forked' },
default_branch: forkDefaulBranch,
default_branch: forkDefaultBranch,
},
]
: []
Expand Down Expand Up @@ -2180,6 +2180,96 @@ describe('modules/platform/github/index', () => {
expect(pr).toMatchObject({ number: 123 });
});

describe('with forkToken', () => {
let scope: httpMock.Scope;

beforeEach(async () => {
scope = httpMock.scope(githubApiHost);
forkInitRepoMock(scope, 'some/repo', false);
scope.get('/user').reply(200, {
login: 'forked',
});
scope.post('/repos/some/repo/forks').reply(200, {
full_name: 'forked/repo',
default_branch: 'master',
});

await github.initRepo({
repository: 'some/repo',
forkToken: 'true',
});
});

it('should allow maintainer edits if explicitly enabled via options', async () => {
scope
.post(
'/repos/some/repo/pulls',
// Ensure the `maintainer_can_modify` option is set in the REST API request.
(body) => body.maintainer_can_modify === true
)
.reply(200, {
number: 123,
head: { repo: { full_name: 'some/repo' }, ref: 'some-branch' },
});
const pr = await github.createPr({
sourceBranch: 'some-branch',
targetBranch: 'main',
prTitle: 'PR title',
prBody: 'PR can be edited by maintainers.',
labels: null,
platformOptions: {
forkModeDisallowMaintainerEdits: false,
},
});
expect(pr).toMatchObject({ number: 123 });
});

it('should allow maintainer edits if not explicitly set', async () => {
scope
.post(
'/repos/some/repo/pulls',
// Ensure the `maintainer_can_modify` option is `false` in the REST API request.
(body) => body.maintainer_can_modify === true
)
.reply(200, {
number: 123,
head: { repo: { full_name: 'some/repo' }, ref: 'some-branch' },
});
const pr = await github.createPr({
sourceBranch: 'some-branch',
targetBranch: 'main',
prTitle: 'PR title',
prBody: 'PR *cannot* be edited by maintainers.',
labels: null,
});
expect(pr).toMatchObject({ number: 123 });
});

it('should disallow maintainer edits if explicitly disabled', async () => {
scope
.post(
'/repos/some/repo/pulls',
// Ensure the `maintainer_can_modify` option is `false` in the REST API request.
(body) => body.maintainer_can_modify === false
)
.reply(200, {
number: 123,
head: { repo: { full_name: 'some/repo' }, ref: 'some-branch' },
});
const pr = await github.createPr({
sourceBranch: 'some-branch',
targetBranch: 'main',
prTitle: 'PR title',
prBody: 'PR *cannot* be edited by maintainers.',
labels: null,
platformOptions: {
forkModeDisallowMaintainerEdits: true,
},
});
expect(pr).toMatchObject({ number: 123 });
});
});

describe('automerge', () => {
const createdPrResp = {
number: 123,
Expand Down
3 changes: 2 additions & 1 deletion lib/modules/platform/github/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1499,7 +1499,8 @@ export async function createPr({
// istanbul ignore if
if (config.forkToken) {
options.token = config.forkToken;
options.body.maintainer_can_modify = true;
options.body.maintainer_can_modify =
platformOptions?.forkModeDisallowMaintainerEdits !== true;
}
logger.debug({ title, head, base, draft: draftPR }, 'Creating PR');
const ghPr = (
Expand Down
1 change: 1 addition & 0 deletions lib/modules/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export type PlatformPrOptions = {
bbUseDefaultReviewers?: boolean;
gitLabIgnoreApprovals?: boolean;
usePlatformAutomerge?: boolean;
forkModeDisallowMaintainerEdits?: boolean;
};
export interface CreatePRConfig {
sourceBranch: string;
Expand Down
1 change: 1 addition & 0 deletions lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export function getPlatformPrOptions(
azureWorkItemId: config.azureWorkItemId,
bbUseDefaultReviewers: config.bbUseDefaultReviewers,
gitLabIgnoreApprovals: config.gitLabIgnoreApprovals,
forkModeDisallowMaintainerEdits: config.forkModeDisallowMaintainerEdits,
usePlatformAutomerge,
};
}
Expand Down

0 comments on commit 340a913

Please sign in to comment.