Merge branch 'stable-3.3' * stable-3.3: ContentTypeUtil: Add singleton scope Upgrade bazlets to latest stable-3.1 to build with 3.1.12 API Upgrade bazlets to latest stable-3.0 to build with 3.0.15 API Upgrade bazlets to latest stable-2.16 to build with 2.16.26 API Upgrade bazlets to latest stable-3.2 to build with 3.2.6 API Upgrade bazlets to latest stable-3.1 to build with 3.1.11 API Upgrade bazlets to latest stable-2.16 to build with 2.16.23 API Change-Id: Ifed03352924be4a62978f2672d3c50d5ad9e0505
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java index 288afcf..1e805e9 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java
@@ -21,6 +21,7 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.flogger.FluentLogger; @@ -30,6 +31,7 @@ import com.google.gerrit.extensions.annotations.Exports; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; +import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.validators.CommentForValidation; import com.google.gerrit.extensions.validators.CommentValidationContext; @@ -43,6 +45,10 @@ import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidationListener; import com.google.gerrit.server.git.validators.CommitValidationMessage; +import com.google.gerrit.server.patch.PatchList; +import com.google.gerrit.server.patch.PatchListCache; +import com.google.gerrit.server.patch.PatchListKey; +import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.inject.AbstractModule; import com.google.inject.Inject; @@ -65,6 +71,7 @@ import java.util.concurrent.ExecutionException; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; @@ -115,6 +122,7 @@ private final GitRepositoryManager repoManager; private final LoadingCache<String, Pattern> patternCache; private final ContentTypeUtil contentTypeUtil; + private final PatchListCache patchListCache; private final ValidatorConfig validatorConfig; @Inject @@ -124,12 +132,14 @@ @Named(CACHE_NAME) LoadingCache<String, Pattern> patternCache, PluginConfigFactory cfgFactory, GitRepositoryManager repoManager, + PatchListCache patchListCache, ValidatorConfig validatorConfig) { this.pluginName = pluginName; this.patternCache = patternCache; this.cfgFactory = cfgFactory; this.repoManager = repoManager; this.contentTypeUtil = contentTypeUtil; + this.patchListCache = patchListCache; this.validatorConfig = validatorConfig; } @@ -149,13 +159,15 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_CHECK_BLOCKED_KEYWORD)) { + KEY_CHECK_BLOCKED_KEYWORD, + receiveEvent.pushOptions)) { ImmutableMap<String, Pattern> blockedKeywordPatterns = patternCache.getAll( Arrays.asList(cfg.getStringList(KEY_CHECK_BLOCKED_KEYWORD_PATTERN))); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation( + receiveEvent.project.getNameKey(), repo, receiveEvent.commit, receiveEvent.revWalk, @@ -167,7 +179,10 @@ } } } - } catch (NoSuchProjectException | IOException | ExecutionException e) { + } catch (NoSuchProjectException + | IOException + | ExecutionException + | PatchListNotAvailableException e) { throw new CommitValidationException("failed to check on blocked keywords", e); } return Collections.emptyList(); @@ -181,7 +196,11 @@ PluginConfig cfg = cfgFactory.getFromProjectConfigWithInheritance(projectNameKey, pluginName); if (isActive(cfg) && validatorConfig.isEnabled( - null, projectNameKey, "", KEY_CHECK_COMMENT_BLOCKED_KEYWORD)) { + null, + projectNameKey, + "", + KEY_CHECK_COMMENT_BLOCKED_KEYWORD, + ImmutableListMultimap.of())) { ImmutableMap<String, Pattern> blockedKeywordPatterns = patternCache.getAll( Arrays.asList(cfg.getStringList(KEY_CHECK_BLOCKED_KEYWORD_PATTERN))); @@ -199,15 +218,21 @@ @VisibleForTesting List<CommitValidationMessage> performValidation( + Project.NameKey project, Repository repo, RevCommit c, RevWalk revWalk, ImmutableCollection<Pattern> blockedKeywordPatterns, PluginConfig cfg) - throws IOException, ExecutionException { + throws IOException, ExecutionException, PatchListNotAvailableException { List<CommitValidationMessage> messages = new LinkedList<>(); checkCommitMessageForBlockedKeywords(blockedKeywordPatterns, messages, c.getFullMessage()); Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c, revWalk); + PatchList patchList = + patchListCache.get( + PatchListKey.againstDefaultBase(c, DiffPreferencesInfo.Whitespace.IGNORE_NONE), + project); + for (String path : content.keySet()) { ObjectLoader ol = revWalk.getObjectReader().open(content.get(path)); try (InputStream in = ol.openStream()) { @@ -215,21 +240,25 @@ continue; } } - checkFileForBlockedKeywords(blockedKeywordPatterns, messages, path, ol); + checkLineDiffForBlockedKeywords( + patchList.get(path).getEdits(), blockedKeywordPatterns, messages, path, ol); } return messages; } private static Optional<CommentValidationFailure> validateComment( - ImmutableMap<String, Pattern> blockedKeywordPatterns, CommentForValidation comment) { + ImmutableMap<String, Pattern> blockedKeywordPatterns, CommentForValidation comment) { // Uses HashSet data structure for de-duping found blocked keywords. - Set<String> findings = new LinkedHashSet<String>( - findBlockedKeywordsInString(blockedKeywordPatterns.values(), comment.getText())); + Set<String> findings = + new LinkedHashSet<String>( + findBlockedKeywordsInString(blockedKeywordPatterns.values(), comment.getText())); if (findings.isEmpty()) { return Optional.empty(); } - return Optional.of(comment.failValidation( - String.format("banned words found in your comment (%s)", Iterables.toString(findings)))); + return Optional.of( + comment.failValidation( + String.format( + "banned words found in your comment (%s)", Iterables.toString(findings)))); } private static void checkCommitMessageForBlockedKeywords( @@ -243,18 +272,23 @@ } } - private static void checkFileForBlockedKeywords( + private static void checkLineDiffForBlockedKeywords( + List<Edit> edits, ImmutableCollection<Pattern> blockedKeywordPatterns, List<CommitValidationMessage> messages, String path, ObjectLoader ol) throws IOException { + List<String> lines = new ArrayList<>(); try (BufferedReader br = new BufferedReader(new InputStreamReader(ol.openStream(), StandardCharsets.UTF_8))) { - int line = 0; for (String l = br.readLine(); l != null; l = br.readLine()) { - line++; - checkLineForBlockedKeywords(blockedKeywordPatterns, messages, path, line, l); + lines.add(l); + } + } + for (Edit edit : edits) { + for (int i = edit.getBeginB(); i < edit.getEndB(); i++) { + checkLineForBlockedKeywords(blockedKeywordPatterns, messages, path, i + 1, lines.get(i)); } } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java index 04525ec..b872620 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java
@@ -49,7 +49,8 @@ ProjectConfigEntryType.ARRAY, null, false, - "Commits with author email not matching one of these pattterns will be rejected.")); + "Commits with author email not matching one of these pattterns will be" + + " rejected.")); bind(ProjectConfigEntry.class) .annotatedWith(Exports.named(KEY_ALLOWED_COMMITTER_EMAIL_PATTERN)) .toInstance( @@ -59,7 +60,8 @@ ProjectConfigEntryType.ARRAY, null, false, - "Commits with committer email not matching one of these patterns will be rejected.")); + "Commits with committer email not matching one of these patterns will be" + + " rejected.")); } }; } @@ -112,7 +114,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_ALLOWED_AUTHOR_EMAIL_PATTERN)) { + KEY_ALLOWED_AUTHOR_EMAIL_PATTERN, + receiveEvent.pushOptions)) { if (!performValidation( receiveEvent.commit.getAuthorIdent().getEmailAddress(), getAllowedAuthorEmailPatterns(cfg))) { @@ -127,7 +130,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_ALLOWED_COMMITTER_EMAIL_PATTERN)) { + KEY_ALLOWED_COMMITTER_EMAIL_PATTERN, + receiveEvent.pushOptions)) { if (!performValidation( receiveEvent.commit.getCommitterIdent().getEmailAddress(), getAllowedCommitterEmailPatterns(cfg))) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java index df1b633..3044ccd 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java
@@ -127,7 +127,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_BLOCKED_CONTENT_TYPE)) { + KEY_BLOCKED_CONTENT_TYPE, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java index 8a9ea2b..d8dd451 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java
@@ -185,7 +185,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_REJECT_DUPLICATE_PATHNAMES)) { + KEY_REJECT_DUPLICATE_PATHNAMES, + receiveEvent.pushOptions)) { locale = getLocale(cfg); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages =
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java index 4b29b1f..5fc1657 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java
@@ -104,7 +104,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_BLOCKED_FILE_EXTENSION)) { + KEY_BLOCKED_FILE_EXTENSION, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FooterValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FooterValidator.java index b5b92c2..67d49a6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FooterValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FooterValidator.java
@@ -89,7 +89,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_REQUIRED_FOOTER)) { + KEY_REQUIRED_FOOTER, + receiveEvent.pushOptions)) { List<CommitValidationMessage> messages = new LinkedList<>(); Set<String> footers = FluentIterable.from(receiveEvent.commit.getFooterLines())
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java index c6371df..f624830 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java
@@ -99,7 +99,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_INVALID_FILENAME)) { + KEY_INVALID_FILENAME, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java index aa72ee0..4bffce6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java
@@ -109,7 +109,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_CHECK_REJECT_WINDOWS_LINE_ENDINGS)) { + KEY_CHECK_REJECT_WINDOWS_LINE_ENDINGS, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit, receiveEvent.revWalk, cfg);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java index a3db8f4..6183317 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java
@@ -93,7 +93,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_MAX_PATH_LENGTH)) { + KEY_MAX_PATH_LENGTH, + receiveEvent.pushOptions)) { int maxPathLength = cfg.getInt(KEY_MAX_PATH_LENGTH, 0); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages =
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java index e15ecb2..98261ea 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java
@@ -14,6 +14,8 @@ package com.googlesource.gerrit.plugins.uploadvalidator; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.server.git.receive.PluginPushOption; import com.google.inject.AbstractModule; import com.google.inject.Scopes; @@ -38,5 +40,7 @@ install(ValidatorConfig.module()); bind(ConfigFactory.class).to(PluginConfigWithInheritanceFactory.class).in(Scopes.SINGLETON); + + DynamicSet.bind(binder(), PluginPushOption.class).to(SkipValidationPushOption.class); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationPushOption.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationPushOption.java new file mode 100644 index 0000000..b690bd4 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationPushOption.java
@@ -0,0 +1,30 @@ +// 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.googlesource.gerrit.plugins.uploadvalidator; + +import com.google.gerrit.server.git.receive.PluginPushOption; + +/** Push option that allows to skip the uploadvalidator plugin validation. */ +public final class SkipValidationPushOption implements PluginPushOption { + public static final String NAME = "skip"; + + public String getName() { + return NAME; + } + + public String getDescription() { + return "skip uploadvalidator validation"; + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java index f96d325..99e2641 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java
@@ -96,7 +96,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_CHECK_SUBMODULE)) { + KEY_CHECK_SUBMODULE, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit, receiveEvent.revWalk);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java index 85a8151..31faa75 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java
@@ -97,7 +97,8 @@ receiveEvent.user, receiveEvent.getProjectNameKey(), receiveEvent.getRefName(), - KEY_CHECK_SYMLINK)) { + KEY_CHECK_SYMLINK, + receiveEvent.pushOptions)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit, receiveEvent.revWalk);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java index d1c5f7e..d7cca55 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
@@ -17,18 +17,20 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.AccessSection; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.AccountGroup.UUID; +import com.google.gerrit.entities.InternalGroup; import com.google.gerrit.entities.Project; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.extensions.annotations.Exports; +import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; -import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.project.RefPatternMatcher; import com.google.gerrit.server.query.group.InternalGroupQuery; import com.google.inject.AbstractModule; @@ -44,6 +46,7 @@ private static final Logger log = LoggerFactory.getLogger(ValidatorConfig.class); private static final String KEY_PROJECT = "project"; private static final String KEY_REF = "ref"; + private final String pluginName; private final ConfigFactory configFactory; private final GroupByNameFinder groupByNameFinder; @@ -77,7 +80,11 @@ } @Inject - public ValidatorConfig(ConfigFactory configFactory, GroupByNameFinder groupByNameFinder) { + public ValidatorConfig( + @PluginName String pluginName, + ConfigFactory configFactory, + GroupByNameFinder groupByNameFinder) { + this.pluginName = pluginName; this.configFactory = configFactory; this.groupByNameFinder = groupByNameFinder; } @@ -96,15 +103,18 @@ @Nullable IdentifiedUser user, Project.NameKey projectName, String refName, - String validatorOp) { + String validatorOp, + ImmutableListMultimap<String, String> pushOptions) { PluginConfig conf = configFactory.get(projectName); return conf != null && isValidConfig(conf, projectName) - && (activeForRef(conf, refName)) + && !isDisabledByPushOption(conf, pushOptions) + && activeForRef(conf, refName) && (user == null || activeForEmail(conf, user.getAccount().preferredEmail())) - && (activeForProject(conf, projectName.get())) - && (!isDisabledValidatorOp(conf, validatorOp)) + && activeForGroup(conf, user) + && activeForProject(conf, projectName.get()) + && !isDisabledValidatorOp(conf, validatorOp) && (!hasCriteria(conf, "skipGroup") || !canSkipValidation(conf, validatorOp) || !canSkipRef(conf, refName) @@ -141,6 +151,15 @@ return Arrays.asList(c).contains(validatorOp); } + private boolean isDisabledByPushOption( + PluginConfig config, ImmutableListMultimap<String, String> pushOptions) { + String qualifiedName = pluginName + "~" + SkipValidationPushOption.NAME; + if (!config.getBoolean("skipViaPushOption", false)) { + return false; + } + return pushOptions.containsKey(qualifiedName); + } + private boolean activeForProject(PluginConfig config, String project) { return matchCriteria(config, "project", project, true, false); } @@ -153,6 +172,22 @@ return matchCriteria(config, "email", email, true, false); } + private boolean activeForGroup(PluginConfig config, @Nullable IdentifiedUser user) { + if (user == null) { + return true; + } + + ImmutableList<UUID> groups = + Arrays.stream(config.getStringList("group")) + .map(this::groupUUID) + .collect(toImmutableList()); + if (groups.isEmpty()) { + return true; + } + + return user.getEffectiveGroups().containsAnyOf(groups); + } + private boolean canSkipValidation(PluginConfig config, String validatorOp) { return matchCriteria(config, "skipValidation", validatorOp, false, false); } @@ -188,7 +223,7 @@ } private boolean canSkipGroup(PluginConfig conf, @Nullable IdentifiedUser user) { - if (user == null || !user.isIdentifiedUser()) { + if (user == null) { return false; } @@ -196,7 +231,7 @@ Arrays.stream(conf.getStringList("skipGroup")) .map(this::groupUUID) .collect(toImmutableList()); - return user.asIdentifiedUser().getEffectiveGroups().containsAnyOf(skipGroups); + return user.getEffectiveGroups().containsAnyOf(skipGroups); } private AccountGroup.UUID groupUUID(String groupNameOrUUID) {
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 878b272..aa7b0cf 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -143,7 +143,7 @@ : Patterns for blocked keywords. This check looks for blocked keywords in files. If the check finds an - blocked keyword the push will be rejected. + blocked keyword in the diff between the pushed commit and it's parent. To find a keyword it is possible to pass a regular expressions by blockedKeywordPattern. @@ -290,12 +290,33 @@ project = ^platform/.* ``` +Group-specific validations +--------------------------- + +By default, the validation will be enabled for all users. However, it can be +limited to particular user group by setting `plugin.@PLUGIN@.group`. The group +may be configured using a specific group name or UUID. Multiple groups may +be specified. + +NOTE: For [system groups](../../../Documentation/access-control.html#system_groups) +and external groups (e.g. +[LDAP groups](../../../Documentation/access-control.html#ldap_groups)) the use +of UUIDs is required. This is because group names are resolved through the +group index and the group index only contains Gerrit internal groups. + +E.g. to limit the validation to all users that are part of group `foo` the +following could be configured: + +``` + [plugin "@PLUGIN@"] + group = foo +``` + Permission to skip the rules ---------------------------- -Some users may be allowed to skip some of the rules on a per project and -per repository basis by configuring the appropriate "skip" settings in the -project.config. +Some users may be allowed to skip some of the rules by configuring the +appropriate "skip" settings in the project.config. Skip of the rules is controlled by: @@ -309,6 +330,9 @@ NOTE: When skipGroup isn't defined, all the other skip settings are ignored. +NOTE: If skipGroup is the same as group, all users are able to skip validations +based on other skip rules. + NOTE: For [system groups](../../../Documentation/access-control.html#system_groups) and external groups (e.g. [LDAP groups](../../../Documentation/access-control.html#ldap_groups)) the use @@ -394,3 +418,12 @@ skipGroup = ldap/GerritAdmins skipRef = refs/heads/master ``` + +plugin.@PLUGIN@.skipViaPushOption +: Allow all users to skip validation using push options + + This check allows all users to bypass all validation rules if they set push + option "@PLUGIN@~skip" (e.g. git push -o "@PLUGIN@~skip"), regardless of + other skip rules. + + Default: false
diff --git a/src/main/resources/Documentation/toc.md b/src/main/resources/Documentation/toc.md new file mode 100644 index 0000000..2490085 --- /dev/null +++ b/src/main/resources/Documentation/toc.md
@@ -0,0 +1,6 @@ +### Admin Guides +* [Configuration](config.html) + +### Contributor Guides +* [Build](build.html) +
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java index c709622..245f2dc 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java
@@ -17,11 +17,19 @@ import static com.google.common.truth.Truth.assertThat; import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.EMPTY_PLUGIN_CONFIG; import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.PATTERN_CACHE; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Patch; +import com.google.gerrit.entities.Project; import com.google.gerrit.server.git.validators.CommitValidationMessage; +import com.google.gerrit.server.patch.PatchList; +import com.google.gerrit.server.patch.PatchListCache; +import com.google.gerrit.server.patch.PatchListEntry; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -31,11 +39,26 @@ import java.util.Set; import java.util.regex.Pattern; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class BlockedKeywordValidatorTest extends ValidatorTestCase { + /** Maps file names to content. */ + private static final Map<String, String> FILE_CONTENTS = + ImmutableMap.of( + "bar.txt", + "$Id$\n" + + "$Header$\n" + + "$Author$\n" + + "processXFile($File::Find::name, $Config{$type});\n" + + "$Id: foo bar$\n", + "foo.txt", + "http://foo.bar.tld/?pw=myp4ssw0rdTefoobarstline2\n", + "foobar.txt", + "Testline1\n" + "Testline2\n" + "Testline3\n" + "Testline4"); + private static ImmutableMap<String, Pattern> getPatterns() { return ImmutableMap.<String, Pattern>builder() .put("myp4ssw0rd", Pattern.compile("myp4ssw0rd")) @@ -46,39 +69,42 @@ private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); - // invalid files - String content = "http://foo.bar.tld/?pw=myp4ssw0rdTefoobarstline2\n"; - files.put( - new File(repo.getDirectory().getParent(), "foo.txt"), - content.getBytes(StandardCharsets.UTF_8)); - - content = - "$Id$\n" - + "$Header$\n" - + "$Author$\n" - + "processXFile($File::Find::name, $Config{$type});\n" - + "$Id: foo bar$\n"; - files.put( - new File(repo.getDirectory().getParent(), "bar.txt"), - content.getBytes(StandardCharsets.UTF_8)); - - // valid file - content = "Testline1\n" + "Testline2\n" + "Testline3\n" + "Testline4"; - files.put( - new File(repo.getDirectory().getParent(), "foobar.txt"), - content.getBytes(StandardCharsets.UTF_8)); + for (Map.Entry<String, String> fileContents : FILE_CONTENTS.entrySet()) { + files.put( + new File(repo.getDirectory().getParent(), fileContents.getKey()), + fileContents.getValue().getBytes(StandardCharsets.UTF_8)); + } return TestUtils.makeCommit(rw, repo, "Commit foobar with test files.", files); } @Test - public void testKeywords() throws Exception { + public void keywords() throws Exception { + // Mock the PatchListCache to return a diff for each file in our new commit + PatchListCache patchListCacheMock = mock(PatchListCache.class); + PatchList mockPatchList = mock(PatchList.class); + when(patchListCacheMock.get(any(), any(Project.NameKey.class))).thenReturn(mockPatchList); + for (Map.Entry<String, String> fileContent : FILE_CONTENTS.entrySet()) { + PatchListEntry file = mock(PatchListEntry.class); + when(file.getEdits()) + .thenReturn( + ImmutableList.of(new Edit(0, 0, 0, numberOfLinesInString(fileContent.getValue())))); + when(mockPatchList.get(fileContent.getKey())).thenReturn(file); + } + try (RevWalk rw = new RevWalk(repo)) { RevCommit c = makeCommit(rw); BlockedKeywordValidator validator = new BlockedKeywordValidator( - null, new ContentTypeUtil(PATTERN_CACHE), PATTERN_CACHE, null, null, null); + null, + new ContentTypeUtil(PATTERN_CACHE), + PATTERN_CACHE, + null, + null, + patchListCacheMock, + null); List<CommitValidationMessage> m = - validator.performValidation(repo, c, rw, getPatterns().values(), EMPTY_PLUGIN_CONFIG); + validator.performValidation( + Project.nameKey("project"), repo, c, rw, getPatterns().values(), EMPTY_PLUGIN_CONFIG); Set<String> expected = ImmutableSet.of( "ERROR: blocked keyword(s) found in: foo.txt (Line: 1)" @@ -95,4 +121,8 @@ public void validatorInactiveWhenConfigEmpty() { assertThat(BlockedKeywordValidator.isActive(EMPTY_PLUGIN_CONFIG)).isFalse(); } + + public static int numberOfLinesInString(String str) { + return str.length() - str.replace("\n", "").length(); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java index 8c00744..88ce317 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.entities.Project; import com.google.gerrit.server.IdentifiedUser; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -30,7 +31,9 @@ ValidatorConfig config = getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar"); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); } @@ -40,7 +43,13 @@ ValidatorConfig config = getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar"); - assertThat(config.isEnabled(missingEmail, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + missingEmail, + projectName, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); } @@ -56,7 +65,11 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); } @@ -70,7 +83,11 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } @@ -85,11 +102,19 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); assertThat( config.isEnabled( - exampleOrgUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + exampleOrgUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); } @@ -104,7 +129,11 @@ assertThat( config.isEnabled( - missingEmail, projectName, "refs/heads/anyref", "blockedFileExtension")) + missingEmail, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } @@ -121,19 +150,34 @@ assertThat( config.isEnabled( - exampleOrgUser, projectName, "refs/heads/anyref", "blockedFileExtension")) - .isTrue(); - assertThat( - config.isEnabled(xUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + exampleOrgUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + xUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) + .isTrue(); + assertThat( + config.isEnabled( + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException { return new ValidatorConfig( - new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder()); + "uploadvalidator", + new FakeConfigFactory(projectName, defaultConfig), + new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java index 2d3fed6..b028f5d 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java
@@ -16,7 +16,7 @@ import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.AccountGroup; -import com.google.gerrit.server.group.InternalGroup; +import com.google.gerrit.entities.InternalGroup; import java.sql.Timestamp; import java.util.Objects; import java.util.Optional;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/GroupAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/GroupAwareValidatorConfigTest.java new file mode 100644 index 0000000..c78b64b --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/GroupAwareValidatorConfigTest.java
@@ -0,0 +1,115 @@ +// 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.googlesource.gerrit.plugins.uploadvalidator; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableListMultimap; +import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.entities.Project; +import com.google.gerrit.server.util.time.TimeUtil; +import org.junit.Test; + +public class GroupAwareValidatorConfigTest { + private Project.NameKey projectName = Project.nameKey("testProject"); + private static final String pluginName = "uploadvalidator"; + + @Test + public void isEnabledForNoGroupsByDefault() throws Exception { + String config = "[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar\n"; + + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + + assertThat( + validatorConfig.isEnabled( + new FakeUserProvider().get(), + projectName, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) + .isTrue(); + } + + @Test + public void isEnabledWhenUserBelongsToOneGroup() throws Exception { + String config = + "[plugin \"uploadvalidator\"]\n" + + "blockedFileExtension = jar\n" + + "group=fooGroup\n" + + "group=barGroup\n"; + + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + + assertThat( + validatorConfig.isEnabled( + new FakeUserProvider("fooGroup", "bazGroup").get(), + projectName, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) + .isTrue(); + } + + @Test + public void isEnabledWhenUserInGroupUUID() throws Exception { + String config = + "[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar\n" + "group=testGroupName\n"; + + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, + new FakeConfigFactory(projectName, config), + new FakeGroupByNameFinder( + AccountGroup.nameKey("testGroupName"), + AccountGroup.id(1), + AccountGroup.uuid("testGroupId"), + TimeUtil.nowTs())); + + assertThat( + validatorConfig.isEnabled( + new FakeUserProvider("testGroupId").get(), + projectName, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) + .isTrue(); + } + + @Test + public void isDisabledWhenUserNotInGroup() throws Exception { + String config = + "[plugin \"uploadvalidator\"]\n" + + "blockedFileExtension = jar\n" + + "group=fooGroup\n" + + "group=barGroup\n"; + + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + + assertThat( + validatorConfig.isEnabled( + new FakeUserProvider("bazGroup").get(), + projectName, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) + .isFalse(); + } +}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java index c968ec5..6c3a059 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.entities.Project; import com.google.gerrit.server.IdentifiedUser; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -30,7 +31,9 @@ ValidatorConfig config = getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar", projectName); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); } @@ -43,7 +46,9 @@ + " blockedFileExtension = jar", projectName); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); } @@ -56,7 +61,9 @@ + " blockedFileExtension = jar", projectName); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isFalse(); } @@ -70,9 +77,17 @@ ValidatorConfig config = getConfig(configString, projectName); ValidatorConfig config2 = getConfig(configString, otherNameKey); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); - assertThat(config2.isEnabled(anyUser, otherNameKey, "anyRef", "blockedFileExtension")) + assertThat( + config2.isEnabled( + anyUser, + otherNameKey, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } @@ -89,17 +104,33 @@ ValidatorConfig config2 = getConfig(configString, anotherNameKey); ValidatorConfig config3 = getConfig(configString, someOtherNameKey); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); - assertThat(config2.isEnabled(anyUser, anotherNameKey, "anyRef", "blockedFileExtension")) + assertThat( + config2.isEnabled( + anyUser, + anotherNameKey, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); - assertThat(config3.isEnabled(anyUser, someOtherNameKey, "anyRef", "blockedFileExtension")) + assertThat( + config3.isEnabled( + anyUser, + someOtherNameKey, + "anyRef", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } private ValidatorConfig getConfig(String defaultConfig, Project.NameKey projName) throws ConfigInvalidException { return new ValidatorConfig( - new FakeConfigFactory(projName, defaultConfig), new FakeGroupByNameFinder()); + "uploadvalidator", + new FakeConfigFactory(projName, defaultConfig), + new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java index f74d346..97ae747 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.entities.Project; import com.google.gerrit.server.IdentifiedUser; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -30,7 +31,9 @@ ValidatorConfig config = getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar"); - assertThat(config.isEnabled(anyUser, projectName, "anyRef", "blockedFileExtension")) + assertThat( + config.isEnabled( + anyUser, projectName, "anyRef", "blockedFileExtension", ImmutableListMultimap.of())) .isTrue(); } @@ -44,7 +47,11 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); } @@ -58,7 +65,11 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anyref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anyref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } @@ -72,11 +83,19 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/anotherref", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/anotherref", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/mybranch123", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/mybranch123", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); } @@ -91,20 +110,34 @@ assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/branch1", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/branch1", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/branch2", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/branch2", + "blockedFileExtension", + ImmutableListMultimap.of())) .isTrue(); assertThat( config.isEnabled( - anyUser, projectName, "refs/heads/branch3", "blockedFileExtension")) + anyUser, + projectName, + "refs/heads/branch3", + "blockedFileExtension", + ImmutableListMultimap.of())) .isFalse(); } private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException { return new ValidatorConfig( - new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder()); + "uploadvalidator", + new FakeConfigFactory(projectName, defaultConfig), + new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java index b46ac86..81a4ddc 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableListMultimap; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.Project; import com.google.gerrit.server.IdentifiedUser; @@ -25,13 +26,18 @@ public class SkipValidationTest { private final Project.NameKey projectName = Project.nameKey("testProject"); private final IdentifiedUser anyUser = new FakeUserProvider().get(); + private static final String pluginName = "uploadvalidator"; @Test public void dontSkipByDefault() throws Exception { ValidatorConfig validatorConfig = - new ValidatorConfig(new FakeConfigFactory(projectName, ""), new FakeGroupByNameFinder()); + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, ""), new FakeGroupByNameFinder()); - assertThat(validatorConfig.isEnabled(anyUser, projectName, "anyRef", "anyOp")).isTrue(); + assertThat( + validatorConfig.isEnabled( + anyUser, projectName, "anyRef", "anyOp", ImmutableListMultimap.of())) + .isTrue(); } @Test @@ -44,14 +50,15 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabled( new FakeUserProvider("testGroup", "yetAnotherGroup").get(), projectName, "anyRef", - "testOp")) + "testOp", + ImmutableListMultimap.of())) .isFalse(); } @@ -62,6 +69,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder( AccountGroup.nameKey("testGroupName"), @@ -71,7 +79,11 @@ assertThat( validatorConfig.isEnabled( - new FakeUserProvider("testGroupId").get(), projectName, "anyRef", "testOp")) + new FakeUserProvider("testGroupId").get(), + projectName, + "anyRef", + "testOp", + ImmutableListMultimap.of())) .isFalse(); } @@ -85,11 +97,15 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabled( - new FakeUserProvider("yetAnotherGroup").get(), projectName, "anyRef", "testOp")) + new FakeUserProvider("yetAnotherGroup").get(), + projectName, + "anyRef", + "testOp", + ImmutableListMultimap.of())) .isTrue(); } @@ -103,9 +119,11 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); - assertThat(validatorConfig.isEnabled(anyUser, projectName, "anyRef", "anotherOp")) + assertThat( + validatorConfig.isEnabled( + anyUser, projectName, "anyRef", "anotherOp", ImmutableListMultimap.of())) .isTrue(); } @@ -119,11 +137,15 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabled( - new FakeUserProvider("testGroup").get(), projectName, "refs/heads/myref", "testOp")) + new FakeUserProvider("testGroup").get(), + projectName, + "refs/heads/myref", + "testOp", + ImmutableListMultimap.of())) .isFalse(); } @@ -137,11 +159,48 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabled( - anyUser, projectName, "refs/heads/anotherRef", "testOp")) + anyUser, + projectName, + "refs/heads/anotherRef", + "testOp", + ImmutableListMultimap.of())) .isTrue(); } + + @Test + public void dontSkipOnPushOptionIfNotEnabled() throws Exception { + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, ""), new FakeGroupByNameFinder()); + + assertThat( + validatorConfig.isEnabled( + anyUser, + projectName, + "anyRef", + "anyOp", + ImmutableListMultimap.of("uploadvalidator~skip", ""))) + .isTrue(); + } + + @Test + public void skipOnPushOptionEnabled() throws Exception { + String config = "[plugin \"uploadvalidator\"]\n" + "skipViaPushOption=true"; + ValidatorConfig validatorConfig = + new ValidatorConfig( + pluginName, new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); + + assertThat( + validatorConfig.isEnabled( + anyUser, + projectName, + "anyRef", + "anyOp", + ImmutableListMultimap.of("uploadvalidator~skip", ""))) + .isFalse(); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/UploadValidatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/UploadValidatorIT.java index a558336..0e9bf85 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/UploadValidatorIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/UploadValidatorIT.java
@@ -19,6 +19,7 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; @@ -30,11 +31,16 @@ import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; +import com.google.gerrit.extensions.client.ChangeStatus; +import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.extensions.common.MergeInput; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.server.group.SystemGroupBackend; import com.google.inject.Inject; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; import org.junit.Test; @@ -48,30 +54,17 @@ TestRepository<InMemoryRepository> clone; void pushConfig(String config) throws Exception { - TestRepository<InMemoryRepository> allProjectRepo = cloneProject(allProjects, admin); - GitUtil.fetch(allProjectRepo, RefNames.REFS_CONFIG + ":config"); - allProjectRepo.reset("config"); + TestRepository<InMemoryRepository> repo = cloneProject(project, admin); + GitUtil.fetch(repo, RefNames.REFS_CONFIG + ":config"); + repo.reset("config"); PushOneCommit push = - pushFactory.create(admin.newIdent(), allProjectRepo, "Subject", "project.config", config); + pushFactory.create(admin.newIdent(), repo, "Subject", "project.config", config); PushOneCommit.Result res = push.to(RefNames.REFS_CONFIG); res.assertOkStatus(); } @Before public void setup() throws Exception { - - pushConfig( - Joiner.on("\n") - .join( - "[plugin \"uploadvalidator\"]", - " blockedFileExtension = jar", - " blockedFileExtension = .zip", - " blockedKeywordPattern = secr3t", - " invalidFilenamePattern = [%:@]", - " rejectWindowsLineEndings = true", - " maxPathLength = 20", - " rejectDuplicatePathnames = true")); - projectOperations .project(allProjects) .forUpdate() @@ -84,20 +77,37 @@ } @Test - public void testFileExtension() throws Exception { + public void fileExtension() throws Exception { + pushConfig( + Joiner.on("\n") + .join( + "[plugin \"uploadvalidator\"]", + " blockedFileExtension = jar", + " blockedFileExtension = .zip")); + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); pushFactory .create(admin.newIdent(), clone, "Subject", "file.jar", "content") .to("refs/heads/master") .assertErrorStatus("blocked file extensions"); + clone.reset(head); pushFactory .create(admin.newIdent(), clone, "Subject", "file.zip", "content") .to("refs/heads/master") .assertErrorStatus("blocked file extensions"); + + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "content") + .to("refs/heads/master") + .assertOkStatus(); } @Test - public void testKeywordInComment() throws Exception { + public void keywordInComment() throws Exception { + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + PushOneCommit.Result r1 = createChange("Subject", "file.txt", "content"); DraftInput in = new DraftInput(); in.message = "the password is secr3t ! "; @@ -114,7 +124,10 @@ } @Test - public void testKeywordInFile() throws Exception { + public void keywordInNewFile() throws Exception { + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory .create(admin.newIdent(), clone, "Subject", "file.txt", "blah secr3t blah") .to("refs/heads/master") @@ -122,23 +135,47 @@ } @Test - public void testFilenamePattern() throws Exception { + public void filenamePattern() throws Exception { + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " invalidFilenamePattern = [%:@]")); + + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); pushFactory .create(admin.newIdent(), clone, "Subject", "f:le.txt", "content") .to("refs/heads/master") .assertErrorStatus("invalid filename"); + + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "content") + .to("refs/heads/master") + .assertOkStatus(); } @Test - public void testWindowsLineEndings() throws Exception { + public void windowsLineEndings() throws Exception { + pushConfig( + Joiner.on("\n") + .join("[plugin \"uploadvalidator\"]", " rejectWindowsLineEndings = true")); + + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); pushFactory .create(admin.newIdent(), clone, "Subject", "win.ini", "content\r\nline2\r\n") .to("refs/heads/master") .assertErrorStatus("Windows line ending"); + + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "content\nline2\n") + .to("refs/heads/master") + .assertOkStatus(); } @Test - public void testPathLength() throws Exception { + public void pathLength() throws Exception { + pushConfig(Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " maxPathLength = 20")); + + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); pushFactory .create( admin.newIdent(), @@ -148,10 +185,20 @@ "content\nline2\n") .to("refs/heads/master") .assertErrorStatus("too long paths"); + + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "content\nline2\n") + .to("refs/heads/master") + .assertOkStatus(); } @Test - public void testUniqueName() throws Exception { + public void uniqueName() throws Exception { + pushConfig( + Joiner.on("\n") + .join("[plugin \"uploadvalidator\"]", " rejectDuplicatePathnames = true")); + pushFactory .create( admin.newIdent(), @@ -161,4 +208,244 @@ .to("refs/heads/master") .assertErrorStatus("duplicate pathnames"); } + + @Test + public void rulesNotEnforcedForNonGroupMembers() throws Exception { + pushConfig( + Joiner.on("\n") + .join( + "[plugin \"uploadvalidator\"]", + " group = " + adminGroupUuid(), + " rejectDuplicatePathnames = true")); + + TestRepository<InMemoryRepository> userClone = + GitUtil.cloneProject(project, registerRepoConnection(project, user)); + pushFactory + .create( + user.newIdent(), + userClone, + "Subject", + ImmutableMap.of("a.txt", "content\nline2\n", "A.TXT", "content")) + .to("refs/heads/master") + .assertOkStatus(); + } + + @Test + public void rulesNotEnforcedForSkipPushOption() throws Exception { + pushConfig( + Joiner.on("\n") + .join( + "[plugin \"uploadvalidator\"]", + " skipViaPushOption = true", + " rejectDuplicatePathnames = true")); + + PushOneCommit push = + pushFactory.create( + admin.newIdent(), + clone, + "Subject", + ImmutableMap.of("a.txt", "content\nline2\n", "A.TXT", "content")); + push.setPushOptions(ImmutableList.of("uploadvalidator~skip")); + push.to("refs/heads/master").assertOkStatus(); + } + + @Test + public void keywordExistsInFileButNotInDiff() throws Exception { + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "blah" + "") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "foo" + "") + .to("refs/heads/master") + .assertOkStatus(); + } + + @Test + public void keywordExistsInNewFile() throws Exception { + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "foo\n" + "secr3t") + .to("refs/heads/master") + .assertErrorStatus("blocked keywords"); + } + + @Test + public void keywordExistsInFileAndInDiff() throws Exception { + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "blah" + "") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "blah\n" + "secr3t") + .to("refs/heads/master") + .assertErrorStatus("blocked keywords"); + } + + @Test + public void keywordExistsInFileAndIsRemoved() throws Exception { + pushFactory + .create( + admin.newIdent(), clone, "Subject", "file.txt", "" + "blah \n" + "secr3t\n" + "blah") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "" + "blah \n" + "blah\n") + .to("refs/heads/master") + .assertOkStatus(); + } + + @Test + public void keywordExistsInFileAndSameLineIsModified() throws Exception { + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "blah" + "") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + // This could be further improved using intra-line diffs + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t foobar\n" + "blah" + "") + .to("refs/heads/master") + .assertErrorStatus("blocked keywords"); + } + + @Test + public void keywordExistsInOldAndFileIsDeleted() throws Exception { + pushFactory + .create( + admin.newIdent(), + clone, + "Subject", + "file.txt", + "" + "blah \n" + "secr3t\n" + "blah" + "") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create(admin.newIdent(), clone, "Subject", "foo.txt", "blah") + .rmFile("file.txt") + .to("refs/heads/master") + .assertOkStatus(); + } + + @Test + public void createChangeSucceedsWhenKeywordDoesNotExistInFileAndDiff() throws Exception { + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "" + "blah \n") + .to("refs/heads/master") + .assertOkStatus(); + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "foo.txt", "" + "blah \n") + .to("refs/heads/stable") + .assertOkStatus(); + ChangeInput changeInput = new ChangeInput(); + changeInput.project = project.get(); + changeInput.branch = "master"; + changeInput.subject = "A change"; + changeInput.status = ChangeStatus.NEW; + MergeInput mergeInput = new MergeInput(); + mergeInput.source = gApi.projects().name(project.get()).branch("stable").get().revision; + changeInput.merge = mergeInput; + gApi.changes().create(changeInput); + } + + @Test + public void createChangeSucceedsWhenKeywordExistsInFileButNotInDiff() throws Exception { + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "" + "secr3t \n") + .to("refs/heads/master") + .assertOkStatus(); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "foo.txt", "" + "blah \n") + .to("refs/heads/stable") + .assertOkStatus(); + ChangeInput changeInput = new ChangeInput(); + changeInput.project = project.get(); + changeInput.branch = "master"; + changeInput.subject = "A change"; + changeInput.status = ChangeStatus.NEW; + MergeInput mergeInput = new MergeInput(); + mergeInput.source = gApi.projects().name(project.get()).branch("stable").get().revision; + changeInput.merge = mergeInput; + gApi.changes().create(changeInput); + } + + @Test + public void createChangeWithKeywordInMessageFails() throws Exception { + RevCommit head = getHead(testRepo.getRepository(), "HEAD"); + pushConfig( + Joiner.on("\n").join("[plugin \"uploadvalidator\"]", " blockedKeywordPattern = secr3t")); + pushFactory + .create(admin.newIdent(), clone, "Subject", "file.txt", "" + "blah \n") + .to("refs/heads/master") + .assertOkStatus(); + clone.reset(head); + pushFactory + .create(admin.newIdent(), clone, "Subject", "foo.txt", "" + "blah \n") + .to("refs/heads/stable") + .assertOkStatus(); + ChangeInput changeInput = new ChangeInput(); + changeInput.project = project.get(); + changeInput.branch = "master"; + changeInput.subject = "A secr3t change"; + changeInput.status = ChangeStatus.NEW; + MergeInput mergeInput = new MergeInput(); + mergeInput.source = gApi.projects().name(project.get()).branch("stable").get().revision; + changeInput.merge = mergeInput; + ResourceConflictException e = + assertThrows(ResourceConflictException.class, () -> gApi.changes().create(changeInput)); + assertThat(e).hasMessageThat().contains("blocked keyword(s) found"); + } }