diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 3d3cd40b0a..d8ccce9214 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -688,7 +688,8 @@ requires the same permission as deleting a branch. To push an annotated tag on a new commit (commit not reachable from any branch/tag) grant `Push` permission on `+refs/tags/*+` too. The `Push` permission on `+refs/tags/*+` does *not* allow updating of annotated -tags, not even fast-forwarding of annotated tags. +tags, not even fast-forwarding of annotated tags. Update of annotated tags +is only allowed by granting `Push` with `force` option on `+refs/tags/*+`. [[category_push_signed]] diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java index 22c38bd4ac..ceab04fe11 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java @@ -179,16 +179,27 @@ public class GitUtil { public static PushResult pushHead(TestRepository testRepo, String ref, boolean pushTags, boolean force) throws GitAPIException { - return pushHead(testRepo, ref, pushTags, force, null); + return pushOne(testRepo, "HEAD", ref, pushTags, force, null); } public static PushResult pushHead(TestRepository testRepo, String ref, boolean pushTags, boolean force, List pushOptions) throws GitAPIException { + return pushOne(testRepo, "HEAD", ref, pushTags, force, pushOptions); + } + + public static PushResult deleteRef(TestRepository testRepo, String ref) + throws GitAPIException { + return pushOne(testRepo, "", ref, false, true, null); + } + + public static PushResult pushOne(TestRepository testRepo, String source, + String target, boolean pushTags, boolean force, List pushOptions) + throws GitAPIException { PushCommand pushCmd = testRepo.git().push(); pushCmd.setForce(force); pushCmd.setPushOptions(pushOptions); - pushCmd.setRefSpecs(new RefSpec("HEAD:" + ref)); + pushCmd.setRefSpecs(new RefSpec(source + ":" + target)); if (pushTags) { pushCmd.setPushTags(); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java index 65ab771494..7681436893 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.GitUtil.createAnnotatedTag; +import static com.google.gerrit.acceptance.GitUtil.deleteRef; import static com.google.gerrit.acceptance.GitUtil.pushHead; import static com.google.gerrit.acceptance.GitUtil.updateAnnotatedTag; import static com.google.gerrit.acceptance.rest.project.PushTagIT.TagType.ANNOTATED; @@ -30,6 +31,7 @@ import com.google.gerrit.common.data.Permission; import com.google.gerrit.reviewdb.client.RefNames; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.PushResult; import org.eclipse.jgit.transport.RemoteRefUpdate; import org.eclipse.jgit.transport.RemoteRefUpdate.Status; @@ -49,10 +51,14 @@ public class PushTagIT extends AbstractDaemonTest { } } + private RevCommit initialHead; + @Before public void setup() throws Exception { // clone with user to avoid inherited tag permissions of admin user testRepo = cloneProject(project, user); + + initialHead = getRemoteHead(); } @Test @@ -109,28 +115,83 @@ public class PushTagIT extends AbstractDaemonTest { } } + @Test + public void forceUpdate() throws Exception { + for (TagType tagType : TagType.values()) { + allowTagCreation(tagType); + String tagName = pushTagForExistingCommit(tagType, Status.OK); + + forceUpdateTagToExistingCommit(tagType, tagName, + Status.REJECTED_OTHER_REASON); + forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); + + allowPushOnRefsTags(); + forceUpdateTagToExistingCommit(tagType, tagName, + Status.REJECTED_OTHER_REASON); + forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); + + allowForcePushOnRefsTags(); + forceUpdateTagToExistingCommit(tagType, tagName, Status.OK); + forceUpdateTagToNewCommit(tagType, tagName, Status.OK); + + removePushFromRefsTags(); + } + } + + @Test + public void delete() throws Exception { + for (TagType tagType : TagType.values()) { + allowTagCreation(tagType); + String tagName = pushTagForExistingCommit(tagType, Status.OK); + + pushTagDeletion(tagType, tagName, Status.REJECTED_OTHER_REASON); + + allowPushOnRefsTags(); + pushTagDeletion(tagType, tagName, Status.REJECTED_OTHER_REASON); + } + + allowForcePushOnRefsTags(); + for (TagType tagType : TagType.values()) { + String tagName = pushTagForExistingCommit(tagType, Status.OK); + pushTagDeletion(tagType, tagName, Status.OK); + } + } + private String pushTagForExistingCommit(TagType tagType, Status expectedStatus) throws Exception { - return pushTag(tagType, null, false, expectedStatus); + return pushTag(tagType, null, false, false, expectedStatus); } private String pushTagForNewCommit(TagType tagType, Status expectedStatus) throws Exception { - return pushTag(tagType, null, true, expectedStatus); + return pushTag(tagType, null, true, false, expectedStatus); } private void fastForwardTagToExistingCommit(TagType tagType, String tagName, Status expectedStatus) throws Exception { - pushTag(tagType, tagName, false, expectedStatus); + pushTag(tagType, tagName, false, false, expectedStatus); } private void fastForwardTagToNewCommit(TagType tagType, String tagName, Status expectedStatus) throws Exception { - pushTag(tagType, tagName, true, expectedStatus); + pushTag(tagType, tagName, true, false, expectedStatus); + } + + private void forceUpdateTagToExistingCommit(TagType tagType, String tagName, + Status expectedStatus) throws Exception { + pushTag(tagType, tagName, false, true, expectedStatus); + } + + private void forceUpdateTagToNewCommit(TagType tagType, String tagName, + Status expectedStatus) throws Exception { + pushTag(tagType, tagName, true, true, expectedStatus); } private String pushTag(TagType tagType, String tagName, boolean newCommit, - Status expectedStatus) throws Exception { + boolean force, Status expectedStatus) throws Exception { + if (force) { + testRepo.reset(initialHead); + } commit(user.getIdent(), "subject"); boolean createTag = tagName == null; @@ -157,7 +218,7 @@ public class PushTagIT extends AbstractDaemonTest { String tagRef = tagRef(tagName); PushResult r = tagType == LIGHTWEIGHT - ? pushHead(testRepo, tagRef) + ? pushHead(testRepo, tagRef, false, force) : GitUtil.pushTag(testRepo, tagName, !createTag); RemoteRefUpdate refUpdate = r.getRemoteUpdate(tagRef); assertThat(refUpdate.getStatus()) @@ -166,6 +227,15 @@ public class PushTagIT extends AbstractDaemonTest { return tagName; } + private void pushTagDeletion(TagType tagType, String tagName, + Status expectedStatus) throws Exception { + String tagRef = tagRef(tagName); + PushResult r = deleteRef(testRepo, tagRef); + RemoteRefUpdate refUpdate = r.getRemoteUpdate(tagRef); + assertThat(refUpdate.getStatus()).named(tagType.name()) + .isEqualTo(expectedStatus); + } + private void allowTagCreation(TagType tagType) throws Exception { grant(tagType.createPermission, project, "refs/tags/*", false, REGISTERED_USERS); @@ -188,7 +258,7 @@ public class PushTagIT extends AbstractDaemonTest { private void commit(PersonIdent ident, String subject) throws Exception { commitBuilder() .ident(ident) - .message(subject) + .message(subject + " (" + System.nanoTime() + ")") .create(); }