From 38da156905151d49220413bc6221e61750094cfd Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 24 Jan 2023 15:02:17 +0000 Subject: [PATCH 1/8] feat: flag to control whether PRs can be edited by maintainers if `forkToken`is set Platforms like GitHub expose options to control whether a PR can be modified by maintainers. See e.g. 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 This commit introduces a way to control this in Renovate when Renovate operates in a fork. The option will default to `true` for backwards compatibility. Users may use this option to workaround issues as outlined in: #16657. --- docs/usage/configuration-options.md | 11 +++++++++++ lib/config/options/index.ts | 7 +++++++ lib/config/types.ts | 1 + lib/modules/platform/types.ts | 1 + lib/workers/repository/config-migration/pr/index.ts | 1 + lib/workers/repository/onboarding/pr/index.ts | 1 + lib/workers/repository/update/pr/index.ts | 1 + 7 files changed, 23 insertions(+) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 5f2bc37017576f..28db7ca5269556 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -2181,6 +2181,17 @@ Defaults to `update`, but can also be set to `branch`. This sets the level the postUpgradeTask runs on, if set to `update` the postUpgradeTask will be executed for every dependency on the branch. If set to `branch` the postUpgradeTask is executed for the whole branch. +## prAllowMaintainerEdits + +Use `prAllowMaintainerEdits` to control if maintainers can edit Renovate's pull requests. + +When Renovate runs in a fork, this allows project maintainers to make manual changes to the +Renovate PR branch, without needing to create another new PR. + + +!!! note + This option is only relevant if you set `forkToken`. + ## prBodyColumns Use this array to provide a list of column names you wish to include in the PR tables. diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 6a2aed7a40410b..39e27134fff0c2 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1555,6 +1555,13 @@ const options: RenovateOptions[] = [ allowedValues: ['strict', 'flexible', 'none'], default: 'strict', }, + { + name: 'prAllowMaintainerEdits', + description: 'Decides if maintainers can edit Renovate pull requests.', + type: 'boolean', + supportedPlatforms: ['github'], + default: true, + }, { name: 'prCreation', description: 'When to create the PR for a branch.', diff --git a/lib/config/types.ts b/lib/config/types.ts index 5e67b80df5683b..3e68c82f481d57 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -230,6 +230,7 @@ export interface RenovateConfig postUpdateOptions?: string[]; prConcurrentLimit?: number; prHourlyLimit?: number; + prAllowMaintainerEdits?: boolean; defaultRegistryUrls?: string[]; registryUrls?: string[] | null; diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 3dc626c3b09cb7..8bc5384768e0aa 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -103,6 +103,7 @@ export interface CreatePRConfig { labels?: string[] | null; platformOptions?: PlatformPrOptions; draftPR?: boolean; + allowMaintainerEdits?: boolean; } export interface UpdatePrConfig { number: number; diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts index dca8cf475f0f89..f2ecedaa574ff4 100644 --- a/lib/workers/repository/config-migration/pr/index.ts +++ b/lib/workers/repository/config-migration/pr/index.ts @@ -100,6 +100,7 @@ ${ sourceBranch: branchName, // TODO #7154 targetBranch: config.defaultBranch!, + allowMaintainerEdits: config.prAllowMaintainerEdits, prTitle, prBody, labels, diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index e562fe95ea5cf6..28a22508bc954f 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -174,6 +174,7 @@ If you need any further assistance then you can also [request help here](${ const pr = await platform.createPr({ sourceBranch: config.onboardingBranch!, targetBranch: config.defaultBranch!, + allowMaintainerEdits: config.prAllowMaintainerEdits, prTitle: config.onboardingPrTitle!, prBody, labels, diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 5091ef1abf259c..0ab2953466e965 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -353,6 +353,7 @@ export async function ensurePr( labels: prepareLabels(config), platformOptions: getPlatformPrOptions(config), draftPR: config.draftPR, + allowMaintainerEdits: config.prAllowMaintainerEdits, }); incLimitedValue('PullRequests'); From d97e5b8a559c65e63ebd3343c8b6fa721a6f83c3 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 24 Jan 2023 15:05:08 +0000 Subject: [PATCH 2/8] feat(github): respect new `prAllowMaintainerEdits` option when `forkToken` is set This commit ensures the GitHub platform logic respects the new `prallowMaintainerEdits` option when `forkToken` is set and Renovate operates in a fork. Whenever a PR is created with `forkToken` being set, depending on the option value, the GitHub API post request will have the respective option `maintainer_can_modify` option set. This option was always enabled in fork mode before this commit, and without fork mode enabled, the option is a noop because maintainers have access to upstream branches regardless. Users may want to leverage the option and disable it to avoid issues with modifications in Renovate fork accounts, as described in: #19771 Fixes #19771. --- lib/modules/platform/github/index.spec.ts | 90 ++++++++++++++++++++++- lib/modules/platform/github/index.ts | 3 +- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index d9a211a825ddc7..d5742afc0c708c 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -275,7 +275,7 @@ describe('modules/platform/github/index', () => { repository: string, forkExisted: boolean, forkResult = 200, - forkDefaulBranch = 'master' + forkDefaultBranch = 'master' ): void { scope // repo info @@ -308,7 +308,7 @@ describe('modules/platform/github/index', () => { { full_name: 'forked/repo', owner: { login: 'forked' }, - default_branch: forkDefaulBranch, + default_branch: forkDefaultBranch, }, ] : [] @@ -2181,6 +2181,92 @@ 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 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, + allowMaintainerEdits: true, + }); + expect(pr).toMatchObject({ number: 123 }); + }); + + it('should not 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 === 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, + }); + expect(pr).toMatchObject({ number: 123 }); + }); + + it('should not allow 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, + allowMaintainerEdits: false, + }); + expect(pr).toMatchObject({ number: 123 }); + }); + }); + describe('automerge', () => { const createdPrResp = { number: 123, diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index 1ce38129842978..2fcfd6c230acba 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -1479,6 +1479,7 @@ export async function createPr({ prBody: rawBody, labels, draftPR = false, + allowMaintainerEdits, platformOptions, }: CreatePRConfig): Promise { const body = sanitize(rawBody); @@ -1499,7 +1500,7 @@ 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 = !!allowMaintainerEdits; } logger.debug({ title, head, base, draft: draftPR }, 'Creating PR'); const ghPr = ( From 5840773007b845e586d5c4d7373a909ddaf2173a Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 8 Feb 2023 16:32:25 +0000 Subject: [PATCH 3/8] wrap earlier as per feedback --- docs/usage/configuration-options.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 28db7ca5269556..811ac2a7cbab66 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -2185,8 +2185,7 @@ If set to `branch` the postUpgradeTask is executed for the whole branch. Use `prAllowMaintainerEdits` to control if maintainers can edit Renovate's pull requests. -When Renovate runs in a fork, this allows project maintainers to make manual changes to the -Renovate PR branch, without needing to create another new PR. +When Renovate runs in a fork, this allows project maintainers to make manual changes to the Renovate PR branch, without needing to create another new PR. !!! note From a58825b9575b77d37a5e9bba45008696a85a9859 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 10 Feb 2023 16:14:50 +0000 Subject: [PATCH 4/8] address feedback, rename option to `forkModeAllowMaintainerEdits` --- docs/usage/configuration-options.md | 20 +++++++++---------- lib/config/options/index.ts | 15 +++++++------- lib/config/types.ts | 2 +- lib/modules/platform/github/index.spec.ts | 4 ++-- lib/modules/platform/github/index.ts | 4 ++-- .../repository/config-migration/pr/index.ts | 2 +- lib/workers/repository/onboarding/pr/index.ts | 2 +- lib/workers/repository/update/pr/index.ts | 2 +- 8 files changed, 26 insertions(+), 25 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 811ac2a7cbab66..6a737802304996 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -875,6 +875,16 @@ 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. +## forkModeAllowMaintainerEdits + +Use `forkModeAllowMaintainerEdits` to control if maintainers can edit Renovate's pull requests when in fork mode. + +This allows project maintainers to make manual changes to the Renovate PR branch, without needing to create another new PR. + + +!!! 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. @@ -2181,16 +2191,6 @@ Defaults to `update`, but can also be set to `branch`. This sets the level the postUpgradeTask runs on, if set to `update` the postUpgradeTask will be executed for every dependency on the branch. If set to `branch` the postUpgradeTask is executed for the whole branch. -## prAllowMaintainerEdits - -Use `prAllowMaintainerEdits` to control if maintainers can edit Renovate's pull requests. - -When Renovate runs in a fork, this allows project maintainers to make manual changes to the Renovate PR branch, without needing to create another new PR. - - -!!! note - This option is only relevant if you set `forkToken`. - ## prBodyColumns Use this array to provide a list of column names you wish to include in the PR tables. diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 39e27134fff0c2..0c254d8103a9b7 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1555,13 +1555,6 @@ const options: RenovateOptions[] = [ allowedValues: ['strict', 'flexible', 'none'], default: 'strict', }, - { - name: 'prAllowMaintainerEdits', - description: 'Decides if maintainers can edit Renovate pull requests.', - type: 'boolean', - supportedPlatforms: ['github'], - default: true, - }, { name: 'prCreation', description: 'When to create the PR for a branch.', @@ -1920,6 +1913,14 @@ const options: RenovateOptions[] = [ default: false, supportedPlatforms: ['gitlab'], }, + { + name: 'forkModeAllowMaintainerEdits', + description: + 'Decides if maintainers can edit Renovate pull requests when running in fork mode.', + type: 'boolean', + supportedPlatforms: ['github'], + default: true, + }, { name: 'confidential', description: diff --git a/lib/config/types.ts b/lib/config/types.ts index 3e68c82f481d57..bb4b174293d70b 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -230,7 +230,7 @@ export interface RenovateConfig postUpdateOptions?: string[]; prConcurrentLimit?: number; prHourlyLimit?: number; - prAllowMaintainerEdits?: boolean; + forkModeAllowMaintainerEdits?: boolean; defaultRegistryUrls?: string[]; registryUrls?: string[] | null; diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index d5742afc0c708c..5c23c00b1a3280 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -2218,7 +2218,7 @@ describe('modules/platform/github/index', () => { prTitle: 'PR title', prBody: 'PR can be edited by maintainers.', labels: null, - allowMaintainerEdits: true, + forkModeAllowMaintainerEdits: true, }); expect(pr).toMatchObject({ number: 123 }); }); @@ -2261,7 +2261,7 @@ describe('modules/platform/github/index', () => { prTitle: 'PR title', prBody: 'PR *cannot* be edited by maintainers.', labels: null, - allowMaintainerEdits: false, + forkModeAllowMaintainerEdits: false, }); expect(pr).toMatchObject({ number: 123 }); }); diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index 2fcfd6c230acba..931916070a1dfb 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -1479,7 +1479,7 @@ export async function createPr({ prBody: rawBody, labels, draftPR = false, - allowMaintainerEdits, + forkModeAllowMaintainerEdits, platformOptions, }: CreatePRConfig): Promise { const body = sanitize(rawBody); @@ -1500,7 +1500,7 @@ export async function createPr({ // istanbul ignore if if (config.forkToken) { options.token = config.forkToken; - options.body.maintainer_can_modify = !!allowMaintainerEdits; + options.body.maintainer_can_modify = !!forkModeAllowMaintainerEdits; } logger.debug({ title, head, base, draft: draftPR }, 'Creating PR'); const ghPr = ( diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts index f2ecedaa574ff4..dc28e380f63387 100644 --- a/lib/workers/repository/config-migration/pr/index.ts +++ b/lib/workers/repository/config-migration/pr/index.ts @@ -100,7 +100,7 @@ ${ sourceBranch: branchName, // TODO #7154 targetBranch: config.defaultBranch!, - allowMaintainerEdits: config.prAllowMaintainerEdits, + forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, prTitle, prBody, labels, diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 28a22508bc954f..f0fc007bd58862 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -174,7 +174,7 @@ If you need any further assistance then you can also [request help here](${ const pr = await platform.createPr({ sourceBranch: config.onboardingBranch!, targetBranch: config.defaultBranch!, - allowMaintainerEdits: config.prAllowMaintainerEdits, + forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, prTitle: config.onboardingPrTitle!, prBody, labels, diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 0ab2953466e965..7649de2107ee68 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -353,7 +353,7 @@ export async function ensurePr( labels: prepareLabels(config), platformOptions: getPlatformPrOptions(config), draftPR: config.draftPR, - allowMaintainerEdits: config.prAllowMaintainerEdits, + forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, }); incLimitedValue('PullRequests'); From 7bb8e1030b7f6ac37f1a3c6d40f65cb51d9d62bc Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 10 Feb 2023 16:18:17 +0000 Subject: [PATCH 5/8] follow-up unsaved edit --- lib/modules/platform/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 8bc5384768e0aa..75c2efd966f00f 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -103,7 +103,7 @@ export interface CreatePRConfig { labels?: string[] | null; platformOptions?: PlatformPrOptions; draftPR?: boolean; - allowMaintainerEdits?: boolean; + forkModeAllowMaintainerEdits?: boolean; } export interface UpdatePrConfig { number: number; From 017af7d654116004ab16f4752280a79a0b37c75b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 21 Feb 2023 17:10:05 +0000 Subject: [PATCH 6/8] Pass as platform option, update docs, and rename option to `forkModeDisallowMaintainerEdits` --- docs/usage/configuration-options.md | 12 +++++++++--- lib/config/options/index.ts | 4 ++-- lib/config/types.ts | 2 +- lib/modules/platform/github/index.spec.ts | 16 ++++++++++------ lib/modules/platform/github/index.ts | 4 ++-- lib/modules/platform/types.ts | 2 +- .../repository/config-migration/pr/index.ts | 1 - lib/workers/repository/onboarding/pr/index.ts | 1 - lib/workers/repository/update/pr/index.ts | 3 ++- 9 files changed, 27 insertions(+), 18 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 6a40031d876603..36a4e2960bd25d 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -894,11 +894,17 @@ 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. -## forkModeAllowMaintainerEdits +## forkModeDisallowMaintainerEdits -Use `forkModeAllowMaintainerEdits` to control if maintainers can edit Renovate's pull requests when in fork mode. +Use `forkModeDisallowMaintainerEdits` to disallow maintainers from editing Renovate's pull requests when in fork mode. -This allows project maintainers to make manual changes to the Renovate PR branch, without needing to create another new PR. +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. + +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: 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. !!! note diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 49503634122ca8..03df442fa82b76 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1930,9 +1930,9 @@ const options: RenovateOptions[] = [ supportedPlatforms: ['gitlab'], }, { - name: 'forkModeAllowMaintainerEdits', + name: 'forkModeDisallowMaintainerEdits', description: - 'Decides if maintainers can edit Renovate pull requests when running in fork mode.', + 'Disallow maintainers to push to Renovate pull requests when running in fork mode.', type: 'boolean', supportedPlatforms: ['github'], default: true, diff --git a/lib/config/types.ts b/lib/config/types.ts index ee07229f704a0f..d4572fafb49e74 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -230,7 +230,7 @@ export interface RenovateConfig postUpdateOptions?: string[]; prConcurrentLimit?: number; prHourlyLimit?: number; - forkModeAllowMaintainerEdits?: boolean; + forkModeDisallowMaintainerEdits?: boolean; defaultRegistryUrls?: string[]; registryUrls?: string[] | null; diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index bd58168b69eb0d..16fbbb669e5cfd 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -2200,7 +2200,7 @@ describe('modules/platform/github/index', () => { }); }); - it('should allow maintainer edits if enabled via options', async () => { + it('should allow maintainer edits if explicitly enabled via options', async () => { scope .post( '/repos/some/repo/pulls', @@ -2217,17 +2217,19 @@ describe('modules/platform/github/index', () => { prTitle: 'PR title', prBody: 'PR can be edited by maintainers.', labels: null, - forkModeAllowMaintainerEdits: true, + platformOptions: { + githubForkModeDisallowMaintainerEdits: false, + }, }); expect(pr).toMatchObject({ number: 123 }); }); - it('should not allow maintainer edits if not explicitly set', async () => { + 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 === false + (body) => body.maintainer_can_modify === true ) .reply(200, { number: 123, @@ -2243,7 +2245,7 @@ describe('modules/platform/github/index', () => { expect(pr).toMatchObject({ number: 123 }); }); - it('should not allow maintainer edits if explicitly disabled', async () => { + it('should disallow maintainer edits if explicitly disabled', async () => { scope .post( '/repos/some/repo/pulls', @@ -2260,7 +2262,9 @@ describe('modules/platform/github/index', () => { prTitle: 'PR title', prBody: 'PR *cannot* be edited by maintainers.', labels: null, - forkModeAllowMaintainerEdits: false, + platformOptions: { + githubForkModeDisallowMaintainerEdits: true, + }, }); expect(pr).toMatchObject({ number: 123 }); }); diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index 931916070a1dfb..ad75c35fc144f4 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -1479,7 +1479,6 @@ export async function createPr({ prBody: rawBody, labels, draftPR = false, - forkModeAllowMaintainerEdits, platformOptions, }: CreatePRConfig): Promise { const body = sanitize(rawBody); @@ -1500,7 +1499,8 @@ export async function createPr({ // istanbul ignore if if (config.forkToken) { options.token = config.forkToken; - options.body.maintainer_can_modify = !!forkModeAllowMaintainerEdits; + options.body.maintainer_can_modify = + platformOptions?.githubForkModeDisallowMaintainerEdits !== true; } logger.debug({ title, head, base, draft: draftPR }, 'Creating PR'); const ghPr = ( diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 810408ed9d90b2..eec1eaf91b6b7b 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -92,6 +92,7 @@ export type PlatformPrOptions = { bbUseDefaultReviewers?: boolean; gitLabIgnoreApprovals?: boolean; usePlatformAutomerge?: boolean; + githubForkModeDisallowMaintainerEdits?: boolean; }; export interface CreatePRConfig { sourceBranch: string; @@ -101,7 +102,6 @@ export interface CreatePRConfig { labels?: string[] | null; platformOptions?: PlatformPrOptions; draftPR?: boolean; - forkModeAllowMaintainerEdits?: boolean; } export interface UpdatePrConfig { number: number; diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts index dc28e380f63387..dca8cf475f0f89 100644 --- a/lib/workers/repository/config-migration/pr/index.ts +++ b/lib/workers/repository/config-migration/pr/index.ts @@ -100,7 +100,6 @@ ${ sourceBranch: branchName, // TODO #7154 targetBranch: config.defaultBranch!, - forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, prTitle, prBody, labels, diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 4a06f58681c4cd..5c765ad054bbc1 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -172,7 +172,6 @@ If you need any further assistance then you can also [request help here](${ const pr = await platform.createPr({ sourceBranch: config.onboardingBranch!, targetBranch: config.defaultBranch!, - forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, prTitle: config.onboardingPrTitle!, prBody, labels, diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 0ff112d5dc7b17..9cb322c7f31040 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -50,6 +50,8 @@ export function getPlatformPrOptions( azureWorkItemId: config.azureWorkItemId, bbUseDefaultReviewers: config.bbUseDefaultReviewers, gitLabIgnoreApprovals: config.gitLabIgnoreApprovals, + githubForkModeDisallowMaintainerEdits: + config.forkModeDisallowMaintainerEdits, usePlatformAutomerge, }; } @@ -385,7 +387,6 @@ export async function ensurePr( labels: prepareLabels(config), platformOptions: getPlatformPrOptions(config), draftPR: config.draftPR, - forkModeAllowMaintainerEdits: config.forkModeAllowMaintainerEdits, }); incLimitedValue('PullRequests'); From 2efbbfa55638e4c2b7227804dd596a0ef33719ef Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 21 Feb 2023 18:09:23 +0000 Subject: [PATCH 7/8] no bare links --- docs/usage/configuration-options.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 36a4e2960bd25d..05644fe567f6b1 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -900,11 +900,9 @@ Use `forkModeDisallowMaintainerEdits` to disallow maintainers from editing Renov 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. +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). -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: 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. +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. !!! note From 824c99f1764c519adfd3d05f045e90449f65bb15 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 22 Feb 2023 09:36:12 +0000 Subject: [PATCH 8/8] cleanup markdown and use same option name in platform options --- docs/usage/configuration-options.md | 6 ++++-- lib/config/options/index.ts | 2 +- lib/modules/platform/github/index.spec.ts | 4 ++-- lib/modules/platform/github/index.ts | 2 +- lib/modules/platform/types.ts | 2 +- lib/workers/repository/update/pr/index.ts | 3 +-- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 05644fe567f6b1..ba378faa185545 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -900,9 +900,11 @@ Use `forkModeDisallowMaintainerEdits` to disallow maintainers from editing Renov 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). +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. +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. !!! note diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 03df442fa82b76..a401297e2dafee 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1935,7 +1935,7 @@ const options: RenovateOptions[] = [ 'Disallow maintainers to push to Renovate pull requests when running in fork mode.', type: 'boolean', supportedPlatforms: ['github'], - default: true, + default: false, }, { name: 'confidential', diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index 16fbbb669e5cfd..acdf7859662641 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -2218,7 +2218,7 @@ describe('modules/platform/github/index', () => { prBody: 'PR can be edited by maintainers.', labels: null, platformOptions: { - githubForkModeDisallowMaintainerEdits: false, + forkModeDisallowMaintainerEdits: false, }, }); expect(pr).toMatchObject({ number: 123 }); @@ -2263,7 +2263,7 @@ describe('modules/platform/github/index', () => { prBody: 'PR *cannot* be edited by maintainers.', labels: null, platformOptions: { - githubForkModeDisallowMaintainerEdits: true, + forkModeDisallowMaintainerEdits: true, }, }); expect(pr).toMatchObject({ number: 123 }); diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index ad75c35fc144f4..7422d4a6ff93d0 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -1500,7 +1500,7 @@ export async function createPr({ if (config.forkToken) { options.token = config.forkToken; options.body.maintainer_can_modify = - platformOptions?.githubForkModeDisallowMaintainerEdits !== true; + platformOptions?.forkModeDisallowMaintainerEdits !== true; } logger.debug({ title, head, base, draft: draftPR }, 'Creating PR'); const ghPr = ( diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index eec1eaf91b6b7b..1d569aa44fc433 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -92,7 +92,7 @@ export type PlatformPrOptions = { bbUseDefaultReviewers?: boolean; gitLabIgnoreApprovals?: boolean; usePlatformAutomerge?: boolean; - githubForkModeDisallowMaintainerEdits?: boolean; + forkModeDisallowMaintainerEdits?: boolean; }; export interface CreatePRConfig { sourceBranch: string; diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 9cb322c7f31040..666dd0d81b5e79 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -50,8 +50,7 @@ export function getPlatformPrOptions( azureWorkItemId: config.azureWorkItemId, bbUseDefaultReviewers: config.bbUseDefaultReviewers, gitLabIgnoreApprovals: config.gitLabIgnoreApprovals, - githubForkModeDisallowMaintainerEdits: - config.forkModeDisallowMaintainerEdits, + forkModeDisallowMaintainerEdits: config.forkModeDisallowMaintainerEdits, usePlatformAutomerge, }; }