Add push option to skip code owner config validation on demand
Change-Id: Icfdf8770146b684fdd490d8554abc99b4d84195e
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 7ec5b1f..24be1ce 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -22,6 +22,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
@@ -29,6 +30,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.plugins.codeowners.backend.ChangedFiles;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
@@ -137,6 +139,7 @@
private final ChangeNotes.Factory changeNotesFactory;
private final PatchSetUtil patchSetUtil;
private final IdentifiedUser.GenericFactory userFactory;
+ private final SkipCodeOwnerConfigValidationPushOption skipCodeOwnerConfigValidationPushOption;
@Inject
CodeOwnerConfigValidator(
@@ -149,7 +152,8 @@
ProjectCache projectCache,
ChangeNotes.Factory changeNotesFactory,
PatchSetUtil patchSetUtil,
- IdentifiedUser.GenericFactory userFactory) {
+ IdentifiedUser.GenericFactory userFactory,
+ SkipCodeOwnerConfigValidationPushOption skipCodeOwnerConfigValidationPushOption) {
this.pluginName = pluginName;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.repoManager = repoManager;
@@ -160,6 +164,7 @@
this.changeNotesFactory = changeNotesFactory;
this.patchSetUtil = patchSetUtil;
this.userFactory = userFactory;
+ this.skipCodeOwnerConfigValidationPushOption = skipCodeOwnerConfigValidationPushOption;
}
@Override
@@ -197,7 +202,8 @@
receiveEvent.revWalk,
receiveEvent.commit,
receiveEvent.user,
- codeOwnerConfigValidationPolicy.isForced());
+ codeOwnerConfigValidationPolicy.isForced(),
+ receiveEvent.pushOptions);
} catch (RuntimeException e) {
if (!codeOwnerConfigValidationPolicy.isDryRun()) {
throw e;
@@ -272,7 +278,8 @@
revWalk,
commit,
patchSetUploader,
- codeOwnerConfigValidationPolicy.isForced());
+ codeOwnerConfigValidationPolicy.isForced(),
+ /* pushOptions= */ ImmutableListMultimap.of());
} catch (RuntimeException e) {
if (!codeOwnerConfigValidationPolicy.isDryRun()) {
throw e;
@@ -314,7 +321,8 @@
RevWalk revWalk,
RevCommit revCommit,
IdentifiedUser user,
- boolean force) {
+ boolean force,
+ ImmutableListMultimap<String, String> pushOptions) {
CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(branchNameKey.project());
logger.atFine().log("force = %s", force);
@@ -326,6 +334,36 @@
new CommitValidationMessage(
"code-owners functionality is disabled", ValidationMessage.Type.HINT)));
}
+
+ try {
+ if (skipCodeOwnerConfigValidationPushOption.skipValidation(pushOptions)) {
+ logger.atFine().log("skip validation requested");
+ return Optional.of(
+ ValidationResult.create(
+ pluginName,
+ "skipping validation of code owner config files",
+ new CommitValidationMessage(
+ String.format(
+ "the validation is skipped due to the --%s~%s push option",
+ pluginName, SkipCodeOwnerConfigValidationPushOption.NAME),
+ ValidationMessage.Type.HINT)));
+ }
+ } catch (AuthException e) {
+ logger.atFine().withCause(e).log("Not allowed to skip code owner config validation");
+ return Optional.of(
+ ValidationResult.create(
+ pluginName,
+ "skipping code owner config validation not allowed",
+ new CommitValidationMessage(e.getMessage(), ValidationMessage.Type.ERROR)));
+ } catch (SkipCodeOwnerConfigValidationPushOption.InvalidValueException e) {
+ logger.atFine().log(e.getMessage());
+ return Optional.of(
+ ValidationResult.create(
+ pluginName,
+ "invalid push option",
+ new CommitValidationMessage(e.getMessage(), ValidationMessage.Type.ERROR)));
+ }
+
if (codeOwnersConfig.areCodeOwnerConfigsReadOnly()) {
return Optional.of(
ValidationResult.create(
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationCapability.java b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationCapability.java
new file mode 100644
index 0000000..1fcb717
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationCapability.java
@@ -0,0 +1,46 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.validation;
+
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/**
+ * Global capability that allows a user to skip the code owner config validation on push via the
+ * {@code code-owners~skip-validation} push option.
+ */
+@Singleton
+public class SkipCodeOwnerConfigValidationCapability extends CapabilityDefinition {
+ public static final String ID = "canSkipCodeOwnerConfigValidation";
+
+ private final String pluginName;
+
+ @Inject
+ SkipCodeOwnerConfigValidationCapability(@PluginName String pluginName) {
+ this.pluginName = pluginName;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Can Skip Code Owner Config Validation";
+ }
+
+ public PluginPermission getPermission() {
+ return new PluginPermission(pluginName, ID, /* fallBackToAdmin= */ true);
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java
new file mode 100644
index 0000000..bc7ac6b
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java
@@ -0,0 +1,127 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.validation;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException;
+import com.google.gerrit.server.git.receive.PluginPushOption;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/** Push option that allows to skip the code owner config validation. */
+@Singleton
+public class SkipCodeOwnerConfigValidationPushOption implements PluginPushOption {
+ public static final String NAME = "skip-validation";
+
+ private static final String DESCRIPTION = "skips the code owner config validation";
+
+ private final String pluginName;
+ private final PermissionBackend permissionBackend;
+ private final SkipCodeOwnerConfigValidationCapability skipCodeOwnerConfigValidationCapability;
+
+ @Inject
+ SkipCodeOwnerConfigValidationPushOption(
+ @PluginName String pluginName,
+ PermissionBackend permissionBackend,
+ SkipCodeOwnerConfigValidationCapability skipCodeOwnerConfigValidationCapability) {
+ this.pluginName = pluginName;
+ this.permissionBackend = permissionBackend;
+ this.skipCodeOwnerConfigValidationCapability = skipCodeOwnerConfigValidationCapability;
+ }
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return DESCRIPTION;
+ }
+
+ /**
+ * Whether the code owner config validation should be skipped.
+ *
+ * <p>Only returns {@code true} if the {@code --code-owners~skip-validation} push option was
+ * specified and the calling user is allowed to skip the code owner config validation (requires
+ * the {@link SkipCodeOwnerConfigValidationCapability}).
+ *
+ * @param pushOptions the push options that have been specified on the push
+ * @return {@code true} if the {@code --code-owners~skip-validation} push option was specified and
+ * the calling user is allowed to skip the code owner config validation
+ * @throws InvalidValueException if the {@code --code-owners~skip-validation} push option was
+ * specified with an invalid value or if the {@code --code-owners~skip-validation} push option
+ * was specified multiple times
+ * @throws AuthException thrown if the {@code --code-owners~skip-validation} push option was
+ * specified, but the calling user is not allowed to skip the code owner config validation
+ */
+ public boolean skipValidation(ImmutableListMultimap<String, String> pushOptions)
+ throws InvalidValueException, AuthException {
+ String qualifiedName = pluginName + "~" + NAME;
+ if (!pushOptions.containsKey(qualifiedName)) {
+ return false;
+ }
+ ImmutableList<String> values = pushOptions.get(qualifiedName);
+ if (values.size() != 1) {
+ throw new InvalidValueException(values);
+ }
+
+ String value = values.get(0);
+ if (Boolean.parseBoolean(value) || value.isEmpty()) {
+ canSkipCodeOwnerConfigValidation();
+ return true;
+ }
+
+ if (value.equalsIgnoreCase(Boolean.FALSE.toString())) {
+ return false;
+ }
+
+ // value was neither 'true', 'false' nor empty
+ throw new InvalidValueException(values);
+ }
+
+ private void canSkipCodeOwnerConfigValidation() throws AuthException {
+ try {
+ permissionBackend
+ .currentUser()
+ .check(skipCodeOwnerConfigValidationCapability.getPermission());
+ } catch (PermissionBackendException e) {
+ throw new CodeOwnersInternalServerErrorException(
+ String.format(
+ "Failed to check %s capability", SkipCodeOwnerConfigValidationCapability.ID),
+ e);
+ }
+ }
+
+ public class InvalidValueException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ InvalidValueException(ImmutableList<String> invalidValues) {
+ super(
+ invalidValues.size() == 1
+ ? String.format(
+ "Invalid value for --%s~%s push option: %s",
+ pluginName, NAME, invalidValues.get(0))
+ : String.format(
+ "--%s~%s push option can be specified only once, received multiple values: %s",
+ pluginName, NAME, invalidValues));
+ }
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/ValidationModule.java b/java/com/google/gerrit/plugins/codeowners/validation/ValidationModule.java
index a5d06ee..d28925c 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/ValidationModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/ValidationModule.java
@@ -14,7 +14,10 @@
package com.google.gerrit.plugins.codeowners.validation;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.git.receive.PluginPushOption;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener;
import com.google.inject.AbstractModule;
@@ -25,5 +28,11 @@
protected void configure() {
DynamicSet.bind(binder(), CommitValidationListener.class).to(CodeOwnerConfigValidator.class);
DynamicSet.bind(binder(), MergeValidationListener.class).to(CodeOwnerConfigValidator.class);
+
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named(SkipCodeOwnerConfigValidationCapability.ID))
+ .to(SkipCodeOwnerConfigValidationCapability.class);
+ DynamicSet.bind(binder(), PluginPushOption.class)
+ .to(SkipCodeOwnerConfigValidationPushOption.class);
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index c364abc..fda0093 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -62,12 +63,16 @@
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoCodeOwnerConfigParser;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerConfigValidationPolicy;
+import com.google.gerrit.plugins.codeowners.validation.SkipCodeOwnerConfigValidationCapability;
+import com.google.gerrit.plugins.codeowners.validation.SkipCodeOwnerConfigValidationPushOption;
import com.google.gerrit.server.IdentifiedUser;
import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.util.Providers;
import java.nio.file.Path;
import java.util.Optional;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -430,6 +435,146 @@
}
@Test
+ public void userCannotSkipCodeOwnerConfigValidationWithoutCapability() throws Exception {
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ user, String.format("code-owners~%s", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertErrorWithMessages(
+ r,
+ "skipping code owner config validation not allowed",
+ String.format(
+ "%s for plugin code-owners not permitted", SkipCodeOwnerConfigValidationCapability.ID));
+ }
+
+ @Test
+ public void adminCanSkipCodeOwnerConfigValidation() throws Exception {
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ admin, String.format("code-owners~%s", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertOkWithHints(
+ r,
+ "skipping validation of code owner config files",
+ String.format(
+ "the validation is skipped due to the --code-owners~%s push option",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+ }
+
+ @Test
+ public void canUploadNonParseableConfigWithSkipOption() throws Exception {
+ allowRegisteredUsersToSkipValidation();
+
+ // with --code-owners~skip-validation
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ user, String.format("code-owners~%s", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertOkWithHints(
+ r,
+ "skipping validation of code owner config files",
+ String.format(
+ "the validation is skipped due to the --code-owners~%s push option",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+
+ // with --code-owners~skip-validation=true
+ r =
+ uploadNonParseableConfigWithPushOption(
+ user,
+ String.format("code-owners~%s=true", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertOkWithHints(
+ r,
+ "skipping validation of code owner config files",
+ String.format(
+ "the validation is skipped due to the --code-owners~%s push option",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+
+ // with --code-owners~skip-validation=TRUE
+ r =
+ uploadNonParseableConfigWithPushOption(
+ user,
+ String.format("code-owners~%s=TRUE", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertOkWithHints(
+ r,
+ "skipping validation of code owner config files",
+ String.format(
+ "the validation is skipped due to the --code-owners~%s push option",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+ }
+
+ @Test
+ public void cannotUploadNonParseableConfigIfSkipOptionIsFalse() throws Exception {
+ allowRegisteredUsersToSkipValidation();
+
+ // with --code-owners~skip-validation=false
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ user,
+ String.format("code-owners~%s=false", SkipCodeOwnerConfigValidationPushOption.NAME));
+ String abbreviatedCommit = abbreviateName(r.getCommit());
+ r.assertErrorStatus(
+ String.format(
+ "commit %s: [code-owners] %s", abbreviatedCommit, "invalid code owner config files"));
+ }
+
+ @Test
+ public void cannotUploadNonParseableConfigWithInvalidSkipOption() throws Exception {
+ allowRegisteredUsersToSkipValidation();
+
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ user,
+ String.format("code-owners~%s=INVALID", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertErrorWithMessages(
+ r,
+ "invalid push option",
+ String.format(
+ "Invalid value for --code-owners~%s push option: INVALID",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+ }
+
+ @Test
+ public void cannotUploadNonParseableConfigIfSkipOptionIsSetMultipleTimes() throws Exception {
+ allowRegisteredUsersToSkipValidation();
+
+ PushOneCommit.Result r =
+ uploadNonParseableConfigWithPushOption(
+ user,
+ String.format("code-owners~%s", SkipCodeOwnerConfigValidationPushOption.NAME),
+ String.format("code-owners~%s=false", SkipCodeOwnerConfigValidationPushOption.NAME));
+ assertErrorWithMessages(
+ r,
+ "invalid push option",
+ String.format(
+ "--code-owners~%s push option can be specified only once, received multiple values: [, false]",
+ SkipCodeOwnerConfigValidationPushOption.NAME));
+ }
+
+ private PushOneCommit.Result uploadNonParseableConfigWithPushOption(
+ TestAccount testAccount, String... pushOptions) throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, testAccount);
+ PushOneCommit push =
+ pushFactory.create(
+ testAccount.newIdent(),
+ userRepo,
+ "Add code owners",
+ codeOwnerConfigOperations
+ .codeOwnerConfig(createCodeOwnerConfigKey("/"))
+ .getJGitFilePath(),
+ "INVALID");
+ push.setPushOptions(ImmutableList.copyOf(pushOptions));
+ return push.to("refs/for/master");
+ }
+
+ private void allowRegisteredUsersToSkipValidation() {
+ // grant the global capability that is required to use the
+ // --code-owners~skip-validation push option to registered users
+ projectOperations
+ .allProjectsForUpdate()
+ .add(
+ allowCapability("code-owners-" + SkipCodeOwnerConfigValidationCapability.ID)
+ .group(REGISTERED_USERS))
+ .update();
+ }
+
+ @Test
@GerritConfig(name = "plugin.code-owners.enableValidationOnCommitReceived", value = "forced")
public void
cannotUploadNonParseableConfigIfCodeOwnersFunctionalityIsDisabledButValidationIsEnforced()
diff --git a/resources/Documentation/validation.md b/resources/Documentation/validation.md
index 1e4687b..1c660a5 100644
--- a/resources/Documentation/validation.md
+++ b/resources/Documentation/validation.md
@@ -41,6 +41,8 @@
[enableValidationOnCommitReceived](config.html#codeOwnersEnableValidationOnCommitReceived)
or [enableValidationOnSubmit](config.html#codeOwnersEnableValidationOnSubmit)
config options
+* the [--code-owners~skip-validation](#skipCodeOwnerConfigValidationOnDemand)
+ push option was specified on push
In addition for [code owner config files](user-guide.html#codeOwnerConfigFiles)
no validation is done when:
@@ -53,6 +55,22 @@
blocking all uploads, to reduce the risk of breaking the plugin configuration
`code-owner.config` files are validated too)
+## <a id="skipCodeOwnerConfigValidationOnDemand">Skip code owner config validation on demand
+
+By setting the `--code-owners~skip-validation` push option it is possible to
+skip the code owner config validation on push.
+
+Using this push option requires the calling user to have to
+`Can Skip Code Owner Config Validation` global capability. Host administrators
+have this capability implicitly assigned via the `Administrate Server` global
+capability.
+
+**NOTE:** Using this option only makes sense if the [code owner config validation
+on submit](config.html#pluginCodeOwnersEnableValidationOnSubmit) is disabled, as
+otherwise it's not possible to submit the created change (using the push option
+only skips the validation for the push, but not for the submission of the
+change).
+
## <a id="howCodeOwnerConfigsCanGetIssuesAfterSubmit">
In addition it is possible that [code owner config
files](user-guide.hmtl#codeOwnerConfigFiles) get issues after they have been
