PUSH_MERGE should evaluate against refs/heads/*

Any "PUSH_MERGE" on "refs/for/*" will be automatially migrated
when the config is loaded.

Change-Id: I0069d7f73f2e13532831adc43aabf92ebd43d9e8
This commit is contained in:
Gustaf Lundh
2018-04-20 11:24:56 +02:00
parent 66c52dae95
commit 8392e982c0
5 changed files with 27 additions and 12 deletions

View File

@@ -628,13 +628,6 @@ push to happen. Some projects wish to restrict merges to being created
by Gerrit. By granting `Push` without `Push Merge Commit`, the only
merges that enter the system will be those created by Gerrit.
The reference name connected to a `Push Merge Commit` entry must always
be prefixed with `refs/for/`, for example `refs/for/refs/heads/BRANCH`.
This applies even for an entry that complements a `Push` entry for
`refs/heads/BRANCH` that allows direct pushes of non-merge commits, and
the intention of the `Push Merge Commit` entry is to allow direct pushes
of merge commits.
[[category_push_annotated]]
[[category_create_annotated]]
@@ -915,7 +908,7 @@ Suggested access rights to grant:
* xref:category_read[`Read`] on 'refs/heads/\*' and 'refs/tags/*'
* xref:category_push[`Create Review`] to 'refs/heads/*'
* xref:category_push_merge[`Push merge commit`] to 'refs/for/refs/heads/*'
* xref:category_push_merge[`Push merge commit`] to 'refs/heads/*'
* xref:category_forge_author[`Forge Author Identity`] to 'refs/heads/*'
* link:config-labels.html#label_Code-Review[`Label: Code-Review`] with range '-2' to '+2' for 'refs/heads/*'
* link:config-labels.html#label_Verified[`Label: Verified`] with range '-1' to '+1' for 'refs/heads/*'

View File

@@ -150,7 +150,7 @@ class RefControl {
/** @return true if this user can submit merge patch sets to this ref */
private boolean canUploadMerges() {
return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH_MERGE);
return canPerform(Permission.PUSH_MERGE);
}
/** @return true if the user can update the reference as a fast-forward. */

View File

@@ -779,6 +779,12 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
migratedAs.addPermission(perm);
return true;
}
if (isRefsForExclusively(refName) && perm.getName().equals(Permission.PUSH_MERGE)) {
// No need to migrate PUSH_MERGE on refs/*
AccessSection migratedAs = getAccessSection(unRefsFor(refName), true);
migratedAs.addPermission(perm);
return true;
}
return false;
}

View File

@@ -162,7 +162,6 @@ public class AllProjectsCreator {
AccessSection heads = config.getAccessSection(AccessSection.HEADS, true);
AccessSection tags = config.getAccessSection("refs/tags/*", true);
AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true);
AccessSection magic = config.getAccessSection("refs/for/" + AccessSection.ALL, true);
grant(config, cap, GlobalCapability.ADMINISTRATE_SERVER, admin);
grant(config, all, Permission.READ, admin, anonymous);
@@ -185,6 +184,7 @@ public class AllProjectsCreator {
grant(config, heads, cr, -2, 2, admin, owners);
grant(config, heads, Permission.CREATE, admin, owners);
grant(config, heads, Permission.PUSH, admin, owners);
grant(config, all, Permission.PUSH_MERGE, registered);
grant(config, heads, Permission.SUBMIT, admin, owners);
grant(config, heads, Permission.FORGE_AUTHOR, registered);
grant(config, heads, Permission.FORGE_COMMITTER, admin, owners);
@@ -194,8 +194,6 @@ public class AllProjectsCreator {
grant(config, tags, Permission.CREATE_TAG, admin, owners);
grant(config, tags, Permission.CREATE_SIGNED_TAG, admin, owners);
grant(config, magic, Permission.PUSH_MERGE, registered);
meta.getPermission(Permission.READ, true).setExclusiveGroup(true);
grant(config, meta, Permission.READ, admin, owners);
grant(config, meta, cr, -2, 2, admin, owners);

View File

@@ -692,6 +692,24 @@ public class ProjectConfigTest extends GerritBaseTests {
.isEqualTo(Lists.newArrayList(rule));
}
@Test
public void readConfigAddPushMergeRefsForStarIsMigrated() throws Exception {
RevCommit rev =
tr.commit()
.add("groups", group(developers))
.add("project.config", "[access \"refs/for/*\"]\n" + " pushMerge = group Developers\n")
.create();
ProjectConfig cfg = read(rev);
AccessSection as = cfg.getAccessSection("refs/for/*");
assertThat(as).isNull();
as = cfg.getAccessSection("refs/*");
assertThat(as).isNotNull();
PermissionRule rule = new PermissionRule(developers);
assertThat(as.getPermission(Permission.PUSH_MERGE, false).getRules())
.isEqualTo(Lists.newArrayList(rule));
}
@Test
public void readCommentLinkMatchButNoHtmlOrLink() throws Exception {
RevCommit rev =