diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 65bcb8a037..3d3cd40b0a 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -468,7 +468,8 @@ To push lightweight (non-annotated) tags, grant `Create Reference` for reference name `+refs/tags/*+`, as lightweight tags are implemented just like branches in Git. To push a lightweight tag on a new commit (commit not reachable from any branch/tag) grant -`Push` permission on `+refs/tags/*+` too. +`Push` permission on `+refs/tags/*+` too. The `Push` permission on +`+refs/tags/*+` also allows fast-forwarding of lightweight tags. For example, to grant the possibility to create new branches under the namespace `foo`, you have to grant this permission on @@ -686,6 +687,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. [[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 ce3934d740..22c38bd4ac 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 @@ -150,6 +150,16 @@ public class GitUtil { return cmd.call(); } + public static Ref updateAnnotatedTag(TestRepository testRepo, String name, + PersonIdent tagger) throws GitAPIException { + TagCommand tc = testRepo.git().tag().setName(name); + return tc.setAnnotated(true) + .setMessage(name) + .setTagger(tagger) + .setForceUpdate(true) + .call(); + } + public static void fetch(TestRepository testRepo, String spec) throws GitAPIException { FetchCommand fetch = testRepo.git().fetch(); @@ -205,7 +215,7 @@ public class GitUtil { return pushTag(testRepo, tag, false); } - private static PushResult pushTag(TestRepository testRepo, String tag, + public static PushResult pushTag(TestRepository testRepo, String tag, boolean force) throws GitAPIException { PushCommand pushCmd = testRepo.git().push(); pushCmd.setForce(force); 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 2a7728f94a..65ab771494 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 @@ -17,13 +17,17 @@ 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.pushHead; +import static com.google.gerrit.acceptance.GitUtil.updateAnnotatedTag; +import static com.google.gerrit.acceptance.rest.project.PushTagIT.TagType.ANNOTATED; import static com.google.gerrit.acceptance.rest.project.PushTagIT.TagType.LIGHTWEIGHT; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import com.google.common.base.MoreObjects; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.common.data.Permission; +import com.google.gerrit.reviewdb.client.RefNames; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.transport.PushResult; @@ -81,26 +85,65 @@ public class PushTagIT extends AbstractDaemonTest { } } - private void pushTagForExistingCommit(TagType tagType, - Status expectedStatus) throws Exception { - pushTag(tagType, false, expectedStatus); + @Test + public void fastForward() throws Exception { + for (TagType tagType : TagType.values()) { + allowTagCreation(tagType); + String tagName = pushTagForExistingCommit(tagType, Status.OK); + + fastForwardTagToExistingCommit(tagType, tagName, + Status.REJECTED_OTHER_REASON); + fastForwardTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); + + allowPushOnRefsTags(); + Status expectedStatus = + tagType == ANNOTATED ? Status.REJECTED_OTHER_REASON : Status.OK; + fastForwardTagToExistingCommit(tagType, tagName, expectedStatus); + fastForwardTagToNewCommit(tagType, tagName, expectedStatus); + + allowForcePushOnRefsTags(); + fastForwardTagToExistingCommit(tagType, tagName, Status.OK); + fastForwardTagToNewCommit(tagType, tagName, Status.OK); + + removePushFromRefsTags(); + } } - private void pushTagForNewCommit(TagType tagType, + private String pushTagForExistingCommit(TagType tagType, Status expectedStatus) throws Exception { - pushTag(tagType, true, expectedStatus); + return pushTag(tagType, null, false, expectedStatus); } - private void pushTag(TagType tagType, boolean newCommit, + private String pushTagForNewCommit(TagType tagType, + Status expectedStatus) throws Exception { + return pushTag(tagType, null, true, expectedStatus); + } + + private void fastForwardTagToExistingCommit(TagType tagType, String tagName, + Status expectedStatus) throws Exception { + pushTag(tagType, tagName, false, expectedStatus); + } + + private void fastForwardTagToNewCommit(TagType tagType, String tagName, + Status expectedStatus) throws Exception { + pushTag(tagType, tagName, true, expectedStatus); + } + + private String pushTag(TagType tagType, String tagName, boolean newCommit, Status expectedStatus) throws Exception { commit(user.getIdent(), "subject"); - String tagName = "v1" + "_" + System.nanoTime(); + boolean createTag = tagName == null; + tagName = MoreObjects.firstNonNull(tagName, "v1" + "_" + System.nanoTime()); switch (tagType) { case LIGHTWEIGHT: break; case ANNOTATED: - createAnnotatedTag(testRepo, tagName, user.getIdent()); + if (createTag) { + createAnnotatedTag(testRepo, tagName, user.getIdent()); + } else { + updateAnnotatedTag(testRepo, tagName, user.getIdent()); + } break; default: throw new IllegalStateException("unexpected tag type: " + tagType); @@ -112,13 +155,15 @@ public class PushTagIT extends AbstractDaemonTest { pushHead(testRepo, "refs/for/master%submit"); } + String tagRef = tagRef(tagName); PushResult r = tagType == LIGHTWEIGHT - ? pushHead(testRepo, "refs/tags/" + tagName) - : GitUtil.pushTag(testRepo, tagName); - RemoteRefUpdate refUpdate = r.getRemoteUpdate("refs/tags/" + tagName); + ? pushHead(testRepo, tagRef) + : GitUtil.pushTag(testRepo, tagName, !createTag); + RemoteRefUpdate refUpdate = r.getRemoteUpdate(tagRef); assertThat(refUpdate.getStatus()) .named(tagType.name()) .isEqualTo(expectedStatus); + return tagName; } private void allowTagCreation(TagType tagType) throws Exception { @@ -127,9 +172,15 @@ public class PushTagIT extends AbstractDaemonTest { } private void allowPushOnRefsTags() throws Exception { + removePushFromRefsTags(); grant(Permission.PUSH, project, "refs/tags/*", false, REGISTERED_USERS); } + private void allowForcePushOnRefsTags() throws Exception { + removePushFromRefsTags(); + grant(Permission.PUSH, project, "refs/tags/*", true, REGISTERED_USERS); + } + private void removePushFromRefsTags() throws Exception { removePermission(Permission.PUSH, project, "refs/tags/*"); } @@ -140,4 +191,8 @@ public class PushTagIT extends AbstractDaemonTest { .message(subject) .create(); } + + private static String tagRef(String tagName) { + return RefNames.REFS_TAGS + tagName; + } }