Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(platform/github): flag to control whether PRs can be edited by maintainers if forkTokenis set #19771

Merged
merged 9 commits into from
Feb 22, 2023
14 changes: 14 additions & 0 deletions docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,20 @@ 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
viceice marked this conversation as resolved.
Show resolved Hide resolved

Use `forkModeDisallowMaintainerEdits` to disallow maintainers from editing Renovate's pull requests when in fork mode.
viceice marked this conversation as resolved.
Show resolved Hide resolved

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).
devversion marked this conversation as resolved.
Show resolved Hide resolved

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.
devversion marked this conversation as resolved.
Show resolved Hide resolved

<!-- prettier-ignore -->
!!! note
This option is only relevant if you set `forkToken`.
devversion marked this conversation as resolved.
Show resolved Hide resolved

## 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',
viceice marked this conversation as resolved.
Show resolved Hide resolved
description:
'Disallow maintainers to push to Renovate pull requests when running in fork mode.',
viceice marked this conversation as resolved.
Show resolved Hide resolved
type: 'boolean',
supportedPlatforms: ['github'],
default: true,
devversion marked this conversation as resolved.
Show resolved Hide resolved
},
{
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;
viceice marked this conversation as resolved.
Show resolved Hide resolved

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: {
githubForkModeDisallowMaintainerEdits: 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: {
githubForkModeDisallowMaintainerEdits: 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;
devversion marked this conversation as resolved.
Show resolved Hide resolved
options.body.maintainer_can_modify =
platformOptions?.githubForkModeDisallowMaintainerEdits !== 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;
githubForkModeDisallowMaintainerEdits?: boolean;
devversion marked this conversation as resolved.
Show resolved Hide resolved
};
export interface CreatePRConfig {
sourceBranch: string;
Expand Down
2 changes: 2 additions & 0 deletions lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export function getPlatformPrOptions(
azureWorkItemId: config.azureWorkItemId,
bbUseDefaultReviewers: config.bbUseDefaultReviewers,
gitLabIgnoreApprovals: config.gitLabIgnoreApprovals,
githubForkModeDisallowMaintainerEdits:
config.forkModeDisallowMaintainerEdits,
usePlatformAutomerge,
};
}
Expand Down