Skip to content

Commit

Permalink
Merge branch 'stable-3.5' into stable-3.6
Browse files Browse the repository at this point in the history
* stable-3.5:
  Set version to 3.5.6-SNAPSHOT
  Set version to 3.5.5
  GroupBackend: Provide a default isOrContainsExternalGroup
  Fix LabelPredicate group matching for included external groups
  ChangeQueryBuilder: De-dup parseGroup()
  ChangeQueryBuilder: containsExternalSubGroups fixup
  Fix ownerin/uploaderin for internal groups that include external groups
  Fix rendering String input values in plugin project options
  Update JGit to a1901305b
  Fix negated label: queries with external groups
  Fix documentation for 'secure-store-lib'
  Bump JGit to 801a56b48
  Bump sshd version to 2.9.2
  Bump SSHD version to 2.9.1
  Documentation: fixup ExternalId case insensitivity

Change-Id: I7412b04ea7a59f6fdaa9a661aee76460aee66919
Release-Notes: skip
  • Loading branch information
Nasser Grainawi committed Feb 21, 2023
2 parents 0dee511 + f978e3f commit faa31d5
Show file tree
Hide file tree
Showing 23 changed files with 632 additions and 355 deletions.
35 changes: 20 additions & 15 deletions Documentation/externalid-case-insensitivity.txt
Original file line number Diff line number Diff line change
@@ -1,38 +1,41 @@
:linkattrs:
= Gerrit Code Review - ExternalId case insensitivity

Gerrit usernames are case insensitive by default: e.g. johndoe and JohnDoe
represents the same account. However, for installations older than v3.5.x,
the usernames were case sensitive, e.g. johndoe and JohnDoe can both exist
Gerrit usernames are case insensitive by default: e.g. `johndoe` and `JohnDoe`
represent the same account. However, for installations older than v3.5.x,
the usernames were case sensitive, e.g. `johndoe` and `JohnDoe` can both exist
as separate accounts. This could lead to issues when migrating an account
from LDAP to an internal account, if ldap.localUsernameToLowerCase was set.
Such usernames can also be rather confusing for users, if they try to identify
authors of comments or changes.
from LDAP to an internal account, if
xref:config-gerrit.txt#ldap.localUsernameToLowerCase[ldap.localUsernameToLowerCase]
was set. Such usernames can also be rather confusing for users, if they try to
identify authors of comments or changes.

When Gerrit handles case insensitive usernames (external IDs using the
`gerrit:` or `username:` scheme, their external IDs SHA-1 is always computed
`gerrit:` or `username:` scheme), their external IDs SHA-1 is always computed
using the lowercase external ID, hence there cannot be any account differing
only in the capitalization of their usernames.

Gerrit installations older than v3.5.x that are switching to the case-insensitive
username need to migrating all their existing accounts SHA-1s.
username need to migrate all their existing accounts SHA-1s.

[[migration]]
== Migration

Migrating external ID notes can take several minutes for large sites(for example
migration ~45000 accounts can take up to five minutes), so administrators choose
whether to do the migration offline or online, depending on their available
resources and tolerance for downtime.
migration ++~++45000 accounts can take up to five minutes), so administrators
choose whether to do the migration offline or online, depending on their
available resources and tolerance for downtime.

NOTE: Migration is required only on Gerrit primary instances.

[[offline-migration]]
=== Offline

To run the offline migration execute following steps:

* Stop all Gerrit primary instances
* Set the `auth.userNameCaseInsensitive` to false

----
[auth]
userNameCaseInsensitive = false
Expand All @@ -46,7 +49,7 @@ _java_ -jar gerrit.war _ChangeExternalIdCaseSensitivity_
[--batch]
--

See: link:pgm-ChangeExternalIdCaseSensitivity.html
See: link:pgm-ChangeExternalIdCaseSensitivity.html[]

* During the migration `auth.userNameCaseInsensitive` will be set to true
on a node which is executing the migration. When the migration is finished,
Expand All @@ -69,13 +72,14 @@ restart Gerrit:
$ ssh -p <port> <host> gerrit migrate-externalids-to-insensitive
----

See: link:cmd-migrate-externalids-to-insensitive.html
See: link:cmd-migrate-externalids-to-insensitive.html[]

[online-ha-migration]
== Online migration for high-availability setup

To start the online migration with a setup containing multiple primary
instances execute following steps:

* On all Gerrit primary instances set `auth.userNameCaseInsensitive` and
`auth.userNameCaseInsensitiveMigrationMode` and perform a rolling restart
----
Expand All @@ -88,7 +92,7 @@ instances execute following steps:
$ ssh -p <port> <host> gerrit migrate-externalids-to-insensitive
----

See: link:cmd-migrate-externalids-to-insensitive.html
See: link:cmd-migrate-externalids-to-insensitive.html[]

* When the migration is finished, on all other primary nodes set
`auth.userNameCaseInsensitiveMigrationMode` to false and perform a
Expand All @@ -105,6 +109,7 @@ The offline migration tool allows to calculate external ID notes named with the
from the case sensitive external ID.

To rollback external ID notes migration execute following steps:

* Stop all Gerrit primary instances
* Set the `auth.userNameCaseInsensitive` to true
----
Expand All @@ -120,7 +125,7 @@ _java_ -jar gerrit.war _ChangeExternalIdCaseSensitivity_
[--batch]
--

See: link:pgm-ChangeExternalIdCaseSensitivity.html
See: link:pgm-ChangeExternalIdCaseSensitivity.html[]

* During the migration `auth.userNameCaseInsensitive` will be set to false
on a node which is executing the migration. When the migration is finished,
Expand Down
2 changes: 1 addition & 1 deletion Documentation/pgm-SwitchSecureStore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SwitchSecureStore - Changes the currently used SecureStore implementation
[verse]
--
_java_ -jar gerrit.war _SwitchSecureStore_
[--new-secure-store-lib]
[--new-secure-store-lib=<PATH_TO_JAR>]
--

== DESCRIPTION
Expand Down
2 changes: 1 addition & 1 deletion Documentation/pgm-init.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ _java_ -jar gerrit.war _init_
[--list-plugins]
[--install-plugin=<PLUGIN_NAME>]
[--install-all-plugins]
[--secure-store-lib]
[--secure-store-lib=<PATH_TO_JAR>]
[--dev]
[--skip-all-downloads]
[--skip-download=<LIBRARY_NAME>]
Expand Down
1 change: 1 addition & 0 deletions java/com/google/gerrit/acceptance/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ TEST_DEPS = [
"//java/com/google/gerrit/pgm/util",
"//java/com/google/gerrit/truth",
"//java/com/google/gerrit/acceptance/config",
"//java/com/google/gerrit/acceptance/testsuite/group",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/server/fixes/testing",
"//java/com/google/gerrit/server/data",
Expand Down
25 changes: 25 additions & 0 deletions java/com/google/gerrit/acceptance/testsuite/group/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_testonly = 1)

java_library(
name = "group",
srcs = glob(["*.java"]),
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/acceptance:function",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/exceptions",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
"//lib:guava",
"//lib:jgit",
"//lib:jgit-junit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/commons:lang3",
"//lib/guice",
],
)
24 changes: 24 additions & 0 deletions java/com/google/gerrit/server/account/GroupBackend.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,28 @@ public interface GroupBackend {

/** Returns {@code true} if the group with the given UUID is visible to all registered users. */
boolean isVisibleToAll(AccountGroup.UUID uuid);

default boolean isOrContainsExternalGroup(AccountGroup.UUID groupId) {
if (groupId != null) {
GroupDescription.Basic groupDescription = get(groupId);
if (!(groupDescription instanceof GroupDescription.Internal)
|| containsExternalSubGroups((GroupDescription.Internal) groupDescription)) {
return true;
}
}
return false;
}

private boolean containsExternalSubGroups(GroupDescription.Internal internalGroup) {
for (AccountGroup.UUID subGroupUuid : internalGroup.getSubgroups()) {
GroupDescription.Basic subGroupDescription = get(subGroupUuid);
if (!(subGroupDescription instanceof GroupDescription.Internal)) {
return true;
}
if (containsExternalSubGroups((GroupDescription.Internal) subGroupDescription)) {
return true;
}
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public void setMembershipsOf(Account.Id user, GroupMembership membership) {
memberships.put(user, membership);
}

/** Remove the memberships of the given user. No-op if the user does not have any memberships. */
public void removeMembershipsOf(Account.Id user) {
memberships.remove(user);
}

@Override
public boolean handles(AccountGroup.UUID uuid) {
if (uuid != null) {
Expand Down
4 changes: 4 additions & 0 deletions java/com/google/gerrit/server/index/change/ChangeField.java
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,10 @@ public static String formatLabel(
+ (count != null ? ",count=" + count : "");
}

public static String formatLabel(String label, String value, @Nullable Integer count) {
return formatLabel(label, value, /* accountId= */ null, count);
}

public static String formatLabel(
String label, String value, @Nullable Account.Id accountId, @Nullable Integer count) {
return label.toLowerCase()
Expand Down
24 changes: 5 additions & 19 deletions java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
Expand Down Expand Up @@ -1295,14 +1294,9 @@ private Predicate<ChangeData> assignee(Set<Account.Id> who) {

@Operator
public Predicate<ChangeData> ownerin(String group) throws QueryParseException, IOException {
GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
if (g == null) {
throw error("Group " + group + " not found");
}

GroupReference g = parseGroup(group);
AccountGroup.UUID groupId = g.getUUID();
GroupDescription.Basic groupDescription = args.groupBackend.get(groupId);
if (!(groupDescription instanceof GroupDescription.Internal)) {
if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new OwnerinPredicate(args.userFactory, groupId);
}

Expand All @@ -1318,14 +1312,9 @@ public Predicate<ChangeData> ownerin(String group) throws QueryParseException, I
public Predicate<ChangeData> uploaderin(String group) throws QueryParseException, IOException {
checkFieldAvailable(ChangeField.UPLOADER, "uploaderin");

GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
if (g == null) {
throw error("Group " + group + " not found");
}

GroupReference g = parseGroup(group);
AccountGroup.UUID groupId = g.getUUID();
GroupDescription.Basic groupDescription = args.groupBackend.get(groupId);
if (!(groupDescription instanceof GroupDescription.Internal)) {
if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new UploaderinPredicate(args.userFactory, groupId);
}

Expand Down Expand Up @@ -1372,10 +1361,7 @@ public Predicate<ChangeData> cc(String who)

@Operator
public Predicate<ChangeData> reviewerin(String group) throws QueryParseException {
GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
if (g == null) {
throw error("Group " + group + " not found");
}
GroupReference g = parseGroup(group);
return new ReviewerinPredicate(args.userFactory, g.getUUID());
}

Expand Down

0 comments on commit faa31d5

Please sign in to comment.