Rename bypass-review to skip-validation and document it

This push option is misnamed; It does not control whether the user is
allowed to bypass review. By definition any push directly to a branch
(rather than refs/for/) is bypassing review, which is the wording that
is used in user-upload.txt and elsewhere in the documentation.

Rename the option to describe what it actually does: it skips the commit
validation step. Describe this behavior in the push documentation.

Since this is now properly documented, make the constant public as well.

Change-Id: I012e1ea42a9a92a07c09d32beb9471a55e84508c
This commit is contained in:
Dave Borowitz 2017-08-31 11:15:04 -04:00
parent 1ea05bdc99
commit bf46140d77
6 changed files with 42 additions and 6 deletions

View File

@ -224,6 +224,7 @@ maximum Git object size limit that is actually used on the project.
The defined maximum Git object size limit is inherited by any child The defined maximum Git object size limit is inherited by any child
project. project.
[[require-signed-off-by]]
=== Require Signed-off-by === Require Signed-off-by
The `Require Signed-off-by in commit message` option defines whether a The `Require Signed-off-by in commit message` option defines whether a

View File

@ -517,6 +517,39 @@ grant nothing at all. This ensures that accidental pushes don't
make undesired changes to the public repository. make undesired changes to the public repository.
[[skip_validation]]
=== Skip Validation
Even when a user has permission to push directly to a branch
link:#bypass_review[bypassing review], by default Gerrit will still validate any
new commits, for example to check author/committer identities, and run
link:config-validation.html#new-commit-validation[validation plugins]. This
behavior can be bypassed with a push option:
----
git push -o skip-validation HEAD:master
----
Using the `skip-validation` option requires the user to have a specific set
of permissions, *in addition* to those permissions already required to bypass
review:
* link:access-control.html#category_forge_author[Forge Author]
* link:access-control.html#category_forge_committer[Forge Committer]
* link:access-control.html#category_forge_server[Forge Server]
* link:access-control.html#category_push_merge[Push Merge Commits]
Plus these additional requirements on the project:
* Project must not link:project-configuration.html#require-signed-off-by[require
Signed-off-by].
* Project must not have `refs/meta/reject-commits`.
This option only applies when pushing directly to a branch bypassing review.
Validation also occurs when pushing new changes for review, and that type of
validation cannot be skipped.
[[auto_merge]] [[auto_merge]]
=== Auto-Merge during Push === Auto-Merge during Push

View File

@ -23,6 +23,7 @@ import static com.google.gerrit.server.change.HashtagsUtil.cleanupHashtag;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.git.receive.ReceiveConstants.COMMAND_REJECTION_MESSAGE_FOOTER; import static com.google.gerrit.server.git.receive.ReceiveConstants.COMMAND_REJECTION_MESSAGE_FOOTER;
import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_OWNER_CAN_MODIFY_WIP; import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_OWNER_CAN_MODIFY_WIP;
import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
import static com.google.gerrit.server.git.receive.ReceiveConstants.SAME_CHANGE_ID_IN_MULTIPLE_CHANGES; import static com.google.gerrit.server.git.receive.ReceiveConstants.SAME_CHANGE_ID_IN_MULTIPLE_CHANGES;
import static com.google.gerrit.server.git.validators.CommitValidators.NEW_PATCHSET_PATTERN; import static com.google.gerrit.server.git.validators.CommitValidators.NEW_PATCHSET_PATTERN;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
@ -198,7 +199,6 @@ import org.slf4j.LoggerFactory;
/** Receives change upload using the Git receive-pack protocol. */ /** Receives change upload using the Git receive-pack protocol. */
class ReceiveCommits { class ReceiveCommits {
private static final Logger log = LoggerFactory.getLogger(ReceiveCommits.class); private static final Logger log = LoggerFactory.getLogger(ReceiveCommits.class);
private static final String BYPASS_REVIEW = "bypass-review";
private enum ReceiveError { private enum ReceiveError {
CONFIG_UPDATE( CONFIG_UPDATE(
@ -2685,11 +2685,11 @@ class ReceiveCommits {
if (!RefNames.REFS_CONFIG.equals(cmd.getRefName()) if (!RefNames.REFS_CONFIG.equals(cmd.getRefName())
&& !(MagicBranch.isMagicBranch(cmd.getRefName()) && !(MagicBranch.isMagicBranch(cmd.getRefName())
|| NEW_PATCHSET_PATTERN.matcher(cmd.getRefName()).matches()) || NEW_PATCHSET_PATTERN.matcher(cmd.getRefName()).matches())
&& pushOptions.containsKey(BYPASS_REVIEW)) { && pushOptions.containsKey(PUSH_OPTION_SKIP_VALIDATION)) {
try { try {
perm.check(RefPermission.BYPASS_REVIEW); perm.check(RefPermission.SKIP_VALIDATION);
if (!Iterables.isEmpty(rejectCommits)) { if (!Iterables.isEmpty(rejectCommits)) {
throw new AuthException("reject-commits prevents " + BYPASS_REVIEW); throw new AuthException("reject-commits prevents " + PUSH_OPTION_SKIP_VALIDATION);
} }
logDebug("Short-circuiting new commit validation"); logDebug("Short-circuiting new commit validation");
} catch (AuthException denied) { } catch (AuthException denied) {

View File

@ -17,6 +17,8 @@ package com.google.gerrit.server.git.receive;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
public final class ReceiveConstants { public final class ReceiveConstants {
public static final String PUSH_OPTION_SKIP_VALIDATION = "skip-validation";
@VisibleForTesting @VisibleForTesting
public static final String ONLY_OWNER_CAN_MODIFY_WIP = public static final String ONLY_OWNER_CAN_MODIFY_WIP =
"only change owner can modify Work-in-Progress"; "only change owner can modify Work-in-Progress";

View File

@ -29,7 +29,7 @@ public enum RefPermission {
FORGE_COMMITTER(Permission.FORGE_COMMITTER), FORGE_COMMITTER(Permission.FORGE_COMMITTER),
FORGE_SERVER(Permission.FORGE_SERVER), FORGE_SERVER(Permission.FORGE_SERVER),
MERGE, MERGE,
BYPASS_REVIEW, SKIP_VALIDATION,
/** Create a change to code review a commit. */ /** Create a change to code review a commit. */
CREATE_CHANGE, CREATE_CHANGE,

View File

@ -609,7 +609,7 @@ public class RefControl {
case UPDATE_BY_SUBMIT: case UPDATE_BY_SUBMIT:
return projectControl.controlForRef("refs/for/" + getRefName()).canSubmit(true); return projectControl.controlForRef("refs/for/" + getRefName()).canSubmit(true);
case BYPASS_REVIEW: case SKIP_VALIDATION:
return canForgeAuthor() return canForgeAuthor()
&& canForgeCommitter() && canForgeCommitter()
&& canForgeGerritServerIdentity() && canForgeGerritServerIdentity()