Merge "Allow to disable the validation of code owner configs on commit received"
diff --git a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
index ed479e0..1c6f591 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/CodeOwnersPluginConfiguration.java
@@ -109,6 +109,18 @@
}
/**
+ * Whether code owner configs should be validated when a commit is received.
+ *
+ * @param project the project for it should be checked whether code owner configs should be
+ * validated when a commit is received
+ * @return whether code owner configs should be validated when a commit is received
+ */
+ public boolean validateCodeOwnerConfigsOnCommitReceived(Project.NameKey project) {
+ requireNonNull(project, "project");
+ return generalConfig.enableValidationOnCommitReceived(getPluginConfig(project));
+ }
+
+ /**
* Gets the merge commit strategy for the given project.
*
* @param project the project for which the merge commit strategy should be retrieved
diff --git a/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java b/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
index 3065aba..f764e71 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/GeneralConfig.java
@@ -58,6 +58,11 @@
@VisibleForTesting public static final String KEY_ALLOWED_EMAIL_DOMAIN = "allowedEmailDomain";
@VisibleForTesting public static final String KEY_FILE_EXTENSION = "fileExtension";
@VisibleForTesting public static final String KEY_READ_ONLY = "readOnly";
+
+ @VisibleForTesting
+ public static final String KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED =
+ "enableValidationOnCommitReceived";
+
@VisibleForTesting public static final String KEY_MERGE_COMMIT_STRATEGY = "mergeCommitStrategy";
@VisibleForTesting public static final String KEY_GLOBAL_CODE_OWNER = "globalCodeOwner";
@@ -166,6 +171,26 @@
}
/**
+ * Gets the enable validation on commit received configuration from the given plugin config with
+ * fallback to {@code gerrit.config} and default to {@code true}.
+ *
+ * <p>The enable validation on commit received controls whether code owner config files should be
+ * validated when a commit is received.
+ *
+ * @param pluginConfig the plugin config from which the read-only configuration should be read.
+ * @return whether code owner config files should be validated when a commit is received
+ */
+ boolean enableValidationOnCommitReceived(Config pluginConfig) {
+ requireNonNull(pluginConfig, "pluginConfig");
+
+ return pluginConfig.getBoolean(
+ SECTION_CODE_OWNERS,
+ null,
+ KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED,
+ pluginConfigFromGerritConfig.getBoolean(KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED, true));
+ }
+
+ /**
* Gets the merge commit strategy from the given plugin config with fallback to {@code
* gerrit.config}.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 46efd6e..4e883c5 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -167,13 +167,24 @@
.branchName(receiveEvent.refName)
.username(receiveEvent.user.getLoggableName())
.build())) {
- Optional<ValidationResult> validationResult =
- validateCodeOwnerConfig(
- receiveEvent.getBranchNameKey(),
- receiveEvent.repoConfig,
- receiveEvent.revWalk,
- receiveEvent.commit,
- receiveEvent.user);
+ Optional<ValidationResult> validationResult;
+ if (!codeOwnersPluginConfiguration.validateCodeOwnerConfigsOnCommitReceived(
+ receiveEvent.getProjectNameKey())) {
+ validationResult =
+ Optional.of(
+ ValidationResult.create(
+ "skipping validation of code owner config files",
+ new CommitValidationMessage(
+ "code owners config validation is disabled", ValidationMessage.Type.HINT)));
+ } else {
+ validationResult =
+ validateCodeOwnerConfig(
+ receiveEvent.getBranchNameKey(),
+ receiveEvent.repoConfig,
+ receiveEvent.revWalk,
+ receiveEvent.commit,
+ receiveEvent.user);
+ }
if (!validationResult.isPresent()) {
return ImmutableList.of();
}
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 fc2fcf6..6f3374b 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -293,6 +293,46 @@
}
@Test
+ @GerritConfig(name = "plugin.code-owners.enableValidationOnCommitReceived", value = "false")
+ public void onReceiveCommitValidationDisabled() throws Exception {
+ // upload a change with a code owner config that has issues (non-resolvable code owners)
+ String unknownEmail = "non-existing-email@example.com";
+ CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ JgitPath.of(getCodeOwnerConfigFilePath(codeOwnerConfigKey)).get(),
+ format(
+ CodeOwnerConfig.builder(codeOwnerConfigKey, TEST_REVISION)
+ .addCodeOwnerSet(CodeOwnerSet.createWithoutPathExpressions(unknownEmail))
+ .build()));
+ assertOkWithHints(
+ r,
+ "skipping validation of code owner config files",
+ "code owners config validation is disabled");
+
+ // approve the change
+ approve(r.getChangeId());
+
+ // try to submit the change, we expect that this fails since the validation on submit is enabled
+ ResourceConflictException exception =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().submit());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Failed to submit 1 change due to the following problems:\n"
+ + "Change %d: invalid code owner config files:\n"
+ + " ERROR: code owner email '%s' in '%s' cannot be resolved for %s",
+ r.getChange().getId().get(),
+ unknownEmail,
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey),
+ identifiedUserFactory.create(admin.id()).getLoggableName()));
+ }
+
+ @Test
public void noValidationOnDeletionOfConfig() throws Exception {
// Disable the code owners functionality so that we can upload an invalid config that we can
// delete afterwards.
diff --git a/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
index 2ef39b9..3037cd4 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/config/GeneralConfigTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_ENABLE_IMPLICIT_APPROVALS;
+import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_FILE_EXTENSION;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_GLOBAL_CODE_OWNER;
import static com.google.gerrit.plugins.codeowners.config.GeneralConfig.KEY_MERGE_COMMIT_STRATEGY;
@@ -122,6 +123,37 @@
}
@Test
+ public void cannotGetEnableValidationOnCommitReceivedForNullPluginConfig() throws Exception {
+ NullPointerException npe =
+ assertThrows(
+ NullPointerException.class, () -> generalConfig.enableValidationOnCommitReceived(null));
+ assertThat(npe).hasMessageThat().isEqualTo("pluginConfig");
+ }
+
+ @Test
+ public void noEnableValidationOnCommitReceivedConfiguration() throws Exception {
+ assertThat(generalConfig.enableValidationOnCommitReceived(new Config())).isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableValidationOnCommitReceived", value = "false")
+ public void
+ enableValidationOnCommitReceivedConfigurationIsRetrievedFromGerritConfigIfNotSpecifiedOnProjectLevel()
+ throws Exception {
+ assertThat(generalConfig.enableValidationOnCommitReceived(new Config())).isFalse();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableValidationOnCommitReceived", value = "false")
+ public void
+ enableValidationOnCommitReceivedConfigurationInPluginConfigOverridesEnableValidationOnCommitReceivedConfigurationInGerritConfig()
+ throws Exception {
+ Config cfg = new Config();
+ cfg.setString(SECTION_CODE_OWNERS, null, KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED, "true");
+ assertThat(generalConfig.enableValidationOnCommitReceived(cfg)).isTrue();
+ }
+
+ @Test
public void cannotGetMergeCommitStrategyForNullPluginConfig() throws Exception {
NullPointerException npe =
assertThrows(
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index a5f6d91..6b5db3b 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -96,6 +96,18 @@
`@PLUGIN@.config`.\
By default `false`.
+<a id="pluginCodeOwnersEnableValidationOnCommitReceived">plugin.@PLUGIN@.enableValidationOnCommitReceived</a>
+: Whether code owner config files are validated when a commit is
+ received.\
+ If enabled, the upload of commits that contain invalid code owner config
+ files is rejected.\
+ Should only be disabled if there is bot that validates the code owner
+ config files in open changes as part of a pre-submit validation.\
+ Can be overridden per project by setting
+ [codeOwners.enableValidationOnCommitReceived](#codeOwnersEnableValidationOnCommitReceived)
+ in `@PLUGIN@.config`.\
+ By default `true`.
+
<a id="pluginCodeOwnersAllowedEmailDomain">plugin.@PLUGIN@.allowedEmailDomain</a>
: Email domain that allows to assign code ownerships to emails with this
domain.\
@@ -293,6 +305,20 @@
[plugin.@PLUGIN@.readOnly](#pluginCodeOwnersReadOnly) in
`gerrit.config` is used.
+<a id="codeOwnersEnableValidationOnCommitReceived">codeOwners.enableValidationOnCommitReceived</a>
+: Whether code owner config files are validated when a commit is
+ received.\
+ If enabled, the upload of commits that contain invalid code owner config
+ files is rejected.\
+ Should only be disabled if there is bot that validates the code owner
+ config files in open changes as part of a pre-submit validation.\
+ Overrides the global setting
+ [plugin.@PLUGIN@.enableValidationOnCommitReceived](#pluginCodeOwnersEnableValidationOnCommitReceived)
+ in `gerrit.config`.\
+ If not set, the global setting
+ [plugin.@PLUGIN@.enableValidationOnCommitReceived](#pluginCodeOwnersEnableValidationOnCommitReceived)
+ in `gerrit.config` is used.
+
<a id="codeOwnersRequiredApproval">codeOwners.requiredApproval</a>
: Approval that is required from code owners to approve the files in a
change.\
diff --git a/resources/Documentation/user-guide.md b/resources/Documentation/user-guide.md
index 9541795..43badb0 100644
--- a/resources/Documentation/user-guide.md
+++ b/resources/Documentation/user-guide.md
@@ -33,7 +33,9 @@
in Gerrit. This the same as creating/editing any other source files.
On push, Gerrit [validates](validation.html) any code owner config file that is
-touched by the new commits. Commits that make code owner config files invalid
+touched by the new commits, unless [the validation for received commits is
+disabled](config.html#codeOwnersEnableValidationOnCommitReceived).
+If the validation is enabled, commits that make code owner config files invalid
are rejected.
**NOTE:** There is no dedicated editor for code owner config files in the Gerrit
diff --git a/resources/Documentation/validation.md b/resources/Documentation/validation.md
index 9966f09..d06f5c8 100644
--- a/resources/Documentation/validation.md
+++ b/resources/Documentation/validation.md
@@ -32,6 +32,8 @@
plugin gets installed/enabled, it is possible that invalid configuration files
already exist in the repository)
* updates happen behind Gerrit's back (e.g. pushes that bypass Gerrit)
+* the validation is disabled in the
+ [plugin configuration](config.html#codeOwnersEnableValidationOnCommitReceived).
In addition for [code owner config files](user-guide.html#codeOwnerConfigFiles)
no validation is done when:
