Merge branch 'stable-2.15'

* stable-2.15:
  ElasticReindexIT: tag broken test as flaky -for CI
  Add documentation for the change report formatter interface
  CommitValidators: Fix repeated "Change-Id" in error message
  ChangeReportFormatter: Convert Input to use AutoValue.Builder
  Introduce Change Report formatter extension point
  Update elasticsearch-rest-client to 6.3.1
  user-review-ui: Fix typo
  Add new "Delete Changes" permission
  ChangeIT: Refactor tests for deletion of new changes
  ElasticTestUtils: Set index.maxLimit as integer rather than string
  ElasticConfigurationTest: Add tests for elasticsearch.maxRetryTimeout
  ElasticConfigurationTest: Add tests for elasticsearch.prefix
  ElasticConfigurationTest: Add tests for elasticsearch.{username,password}
  ElasticConfiguration: Extract constants to statics
  Elasticsearch: Simplify configuration of servers
  ElasticReindexIT: Add tests for v5 and v6
  Revert "ElasticContainer: Limit heap usage for test containers"
  config-gerrit: Mention that Elasticsearch must be reachable during init
  ElasticContainer: Limit heap usage for test containers
  config-gerrit: Move elasticsearch security settings to separate section
  Elasticsearch: Allow to omit the elasticsearch.username
  Test coverage for elasticsearch.username and elasticsearch.password
  Relax access to ssh set-account command
  Add --generate-http-password to ssh set-account
  Fix creation of plugin log file when log4j.configuration is set

Change-Id: I0b3075efd346f7b00db154a677ddba313ad79415

Documentation/access-control.txt

@@ -841,6 +841,17 @@ right assigned).
 This category permits users to delete their own changes if they are not merged
 yet. This means only own changes that are open or abandoned can be deleted.
 
+[[category_delete_changes]]
+=== Delete Changes
+
+This category permits users to delete other users' changes if they are not merged
+yet. This means only changes that are open or abandoned can be deleted.
+
+Having this permission implies having the link:#category_delete_own_changes[
+Delete Own Changes] permission.
+
+Administrators may always delete changes without having this permission.
+
 [[category_edit_topic_name]]
 === Edit Topic Name
 

Documentation/cmd-set-account.txt

@@ -12,6 +12,7 @@ _ssh_ -p <port> <host> _gerrit set-account_
   [--preferred-email <EMAIL>]
   [--add-ssh-key - | <KEY>]
   [--delete-ssh-key - | <KEY> | ALL]
+  [--generate-http-password]
   [--http-password <PASSWORD>]
   [--clear-http-password] <USER>
 --
@@ -25,8 +26,9 @@ It also allows managing email addresses, which bypasses the
 verification step we force within the UI.
 
 == ACCESS
-Caller must be a member of the privileged 'Administrators' group,
-or have been granted
+Users can call this to update their own accounts. To update a different
+account, a caller must be a member of the privileged 'Administrators'
+group, or have been granted
 link:access-control.html#capability_modifyAccount[the 'Modify Account' global capability].
 For security reasons only the members of the privileged 'Administrators'
 group can add or delete SSH keys for a user.
@@ -93,6 +95,11 @@ This most likely requires double quoting the value, for example
     May be supplied more than once to delete multiple SSH
     keys in a single command execution.
 
+--generate-http-password::
+    Generate a new random HTTP password for the user account
+    similar to the web ui. The password will be output to the
+    user on success with a line: `New password: <PASSWORD>`.
+
 --http-password::
     Set the HTTP password for the user account.
 

Documentation/config-gerrit.txt

@@ -2989,6 +2989,9 @@ respectively. When using version 6.2 or later, the open and closed changes are
 merged into the default `_doc` type. The latter is also used for the accounts and
 groups indices starting with Elasticsearch 6.2.
 
+Note that when Gerrit is configured to use Elasticsearch, the Elasticsearch
+server(s) must be reachable during the site initialization.
+
 [[elasticsearch.prefix]]elasticsearch.prefix::
 +
 This setting can be used to prefix index names to allow multiple Gerrit
@@ -2997,17 +3000,17 @@ change index named `gerrit1_changes_0001`.
 +
 Not set by default.
 
-[[elasticsearch.username]]elasticsearch.username::
+[[elasticsearch.server]]elasticsearch.server::
 +
-Username used to connect to Elasticsearch.
+Elasticsearch server URI in the form `http[s]://hostname:port`. The `port` is
+optional and defaults to `9200` if not specified.
 +
-Not set by default.
-
-[[elasticsearch.password]]elasticsearch.password::
+At least one server must be specified. May be specified multiple times to
+configure multiple Elasticsearch servers.
 +
-Password used to connect to Elasticsearch.
-+
-Not set by default.
+Note that the site initialization program only allows to configure a single
+server. To configure multiple servers the `gerrit.config` file must be edited
+manually.
 
 [[elasticsearch.maxRetryTimeout]]elasticsearch.maxRetryTimeout::
 +
@@ -3017,28 +3020,29 @@ The value is in the usual time-unit format like `1 m`, `5 m`.
 +
 Defaults to `30000 ms`.
 
-==== Elasticsearch server(s) configuration
+==== Elasticsearch Security
 
-Each section corresponds to one Elasticsearch server.
+When security is enabled in Elasticsearch, the username and password must be provided.
+Note that the same username and password are used for all servers.
 
-[[elasticsearch.name.protocol]]elasticsearch.name.protocol::
-+
-Elasticsearch server protocol. Can be `http` or `https`.
-+
-Defaults to `http`.
+For further information about Elasticsearch security, please refer to the documentation:
 
-[[elasticsearch.name.hostname]]elasticsearch.name.hostname::
-+
-Elasticsearch server hostname.
-+
-Defaults to `localhost`.
+* link:https://www.elastic.co/guide/en/elasticsearch/plugins/2.4/security.html[Elasticsearch 2.4]
+* link:https://www.elastic.co/guide/en/x-pack/5.6/security-getting-started.html[Elasticsearch 5.6]
+* link:https://www.elastic.co/guide/en/x-pack/6.2/security-getting-started.html[Elasticsearch 6.2]
+* link:https://www.elastic.co/guide/en/elastic-stack-overview/6.3/security-getting-started.html[Elasticsearch 6.3]
 
-[[elasticsearch.name.port]]elasticsearch.name.port::
+[[elasticsearch.username]]elasticsearch.username::
 +
-Elasticsearch server port.
+Username used to connect to Elasticsearch.
 +
-Defaults to `9200`.
+If a password is set, defaults to `elastic`, otherwise not set by default.
 
+[[elasticsearch.password]]elasticsearch.password::
++
+Password used to connect to Elasticsearch.
++
+Not set by default.
 
 [[ldap]]
 === Section ldap

Documentation/dev-plugins.txt

@@ -2313,6 +2313,16 @@ by implementing `com.google.gerrit.extensions.config.ExternalIncludedIn`,
 e.g. a plugin can provide a list of servers on which the change was
 deployed.
 
+[[change-report-formatting]]
+== Change Report Formatting
+
+When a change is pushed for review from the command line, Gerrit reports
+the change(s) received with their URL and subject.
+
+By implementing the
+`com.google.gerrit.server.git.ChangeReportFormatter` interface, a plugin
+may change the formatting of the report.
+
 [[links-to-external-tools]]
 == Links To External Tools
 

Documentation/user-review-ui.txt

@@ -258,7 +258,9 @@ Deletes the change.
 For open or abandoned changes, the `Delete Change` button will be available
 and if the user is the change owner and is granted the
 link:access-control.html#category_delete_own_changes[Delete Own Changes]
-permission or if they are an administrator.
+permission, if they are granted the
+link:access-control.html#category_delete_changes[Delete Changes] permission,
+or if they are an administrator.
 
 ** [[plugin-actions]]Further actions may be available if plugins are installed.
 

WORKSPACE

@@ -906,8 +906,8 @@ maven_jar(
 
 maven_jar(
     name = "elasticsearch-rest-client",
-    artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.0",
-    sha1 = "a95ef38262ef499aa07cdb736f4a47cb19162654",
+    artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.1",
+    sha1 = "99de036a2cd99dbecec1cc84f5d0e19032e74fa7",
 )
 
 JACKSON_VERSION = "2.8.9"

java/com/google/gerrit/common/data/Permission.java

@@ -27,6 +27,7 @@ public class Permission implements Comparable<Permission> {
   public static final String CREATE_SIGNED_TAG = "createSignedTag";
   public static final String CREATE_TAG = "createTag";
   public static final String DELETE = "delete";
+  public static final String DELETE_CHANGES = "deleteChanges";
   public static final String DELETE_OWN_CHANGES = "deleteOwnChanges";
   public static final String EDIT_ASSIGNEE = "editAssignee";
   public static final String EDIT_HASHTAGS = "editHashtags";
@@ -58,6 +59,7 @@ public class Permission implements Comparable<Permission> {
     NAMES_LC.add(CREATE_SIGNED_TAG.toLowerCase());
     NAMES_LC.add(CREATE_TAG.toLowerCase());
     NAMES_LC.add(DELETE.toLowerCase());
+    NAMES_LC.add(DELETE_CHANGES.toLowerCase());
     NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase());
     NAMES_LC.add(EDIT_ASSIGNEE.toLowerCase());
     NAMES_LC.add(EDIT_HASHTAGS.toLowerCase());

java/com/google/gerrit/elasticsearch/ElasticConfiguration.java

@@ -14,16 +14,18 @@
 
 package com.google.gerrit.elasticsearch;
 
-import com.google.common.base.MoreObjects;
+import static com.google.common.base.MoreObjects.firstNonNull;
+
 import com.google.common.base.Strings;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
+import com.google.inject.ProvisionException;
 import com.google.inject.Singleton;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import org.apache.http.HttpHost;
 import org.eclipse.jgit.lib.Config;
@@ -32,9 +34,16 @@ import org.eclipse.jgit.lib.Config;
 class ElasticConfiguration {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  private static final String DEFAULT_HOST = "localhost";
-  private static final String DEFAULT_PORT = "9200";
-  private static final String DEFAULT_PROTOCOL = "http";
+  static final String SECTION_ELASTICSEARCH = "elasticsearch";
+  static final String KEY_PASSWORD = "password";
+  static final String KEY_USERNAME = "username";
+  static final String KEY_MAX_RETRY_TIMEOUT = "maxRetryTimeout";
+  static final String KEY_PREFIX = "prefix";
+  static final String KEY_SERVER = "server";
+  static final String DEFAULT_PORT = "9200";
+  static final String DEFAULT_USERNAME = "elastic";
+  static final int DEFAULT_MAX_RETRY_TIMEOUT_MS = 30000;
+  static final TimeUnit MAX_RETRY_TIMEOUT_UNIT = TimeUnit.MILLISECONDS;
 
   private final Config cfg;
   private final List<HttpHost> hosts;
@@ -47,31 +56,40 @@ class ElasticConfiguration {
   @Inject
   ElasticConfiguration(@GerritServerConfig Config cfg) {
     this.cfg = cfg;
-    this.username = cfg.getString("elasticsearch", null, "username");
-    this.password = cfg.getString("elasticsearch", null, "password");
+    this.password = cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD);
+    this.username =
+        password == null
+            ? null
+            : firstNonNull(
+                cfg.getString(SECTION_ELASTICSEARCH, null, KEY_USERNAME), DEFAULT_USERNAME);
     this.maxRetryTimeout =
         (int)
-            cfg.getTimeUnit("elasticsearch", null, "maxRetryTimeout", 30000, TimeUnit.MILLISECONDS);
-    this.prefix = Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix"));
-
-    Set<String> subsections = cfg.getSubsections("elasticsearch");
-    if (subsections.isEmpty()) {
-      HttpHost httpHost =
-          new HttpHost(DEFAULT_HOST, Integer.valueOf(DEFAULT_PORT), DEFAULT_PROTOCOL);
-      this.hosts = Collections.singletonList(httpHost);
-    } else {
-      this.hosts = new ArrayList<>(subsections.size());
-      for (String subsection : subsections) {
-        String port = getString(cfg, subsection, "port", DEFAULT_PORT);
-        String host = getString(cfg, subsection, "hostname", DEFAULT_HOST);
-        String protocol = getString(cfg, subsection, "protocol", DEFAULT_PROTOCOL);
-
-        HttpHost httpHost = new HttpHost(host, Integer.valueOf(port), protocol);
+            cfg.getTimeUnit(
+                SECTION_ELASTICSEARCH,
+                null,
+                KEY_MAX_RETRY_TIMEOUT,
+                DEFAULT_MAX_RETRY_TIMEOUT_MS,
+                MAX_RETRY_TIMEOUT_UNIT);
+    this.prefix = Strings.nullToEmpty(cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PREFIX));
+    this.hosts = new ArrayList<>();
+    for (String server : cfg.getStringList(SECTION_ELASTICSEARCH, null, KEY_SERVER)) {
+      try {
+        URI uri = new URI(server);
+        int port = uri.getPort();
+        HttpHost httpHost =
+            new HttpHost(
+                uri.getHost(), port == -1 ? Integer.valueOf(DEFAULT_PORT) : port, uri.getScheme());
         this.hosts.add(httpHost);
+      } catch (URISyntaxException | IllegalArgumentException e) {
+        logger.atSevere().log("Invalid server URI %s: %s", server, e.getMessage());
       }
     }
 
-    logger.atInfo().log("Elasticsearch hosts: %s", hosts);
+    if (hosts.isEmpty()) {
+      throw new ProvisionException("No valid Elasticsearch servers configured");
+    }
+
+    logger.atInfo().log("Elasticsearch servers: %s", hosts);
   }
 
   Config getConfig() {
@@ -85,8 +103,4 @@ class ElasticConfiguration {
   String getIndexName(String name, int schemaVersion) {
     return String.format("%s%s_%04d", prefix, name, schemaVersion);
   }
-
-  private String getString(Config cfg, String subsection, String name, String defaultValue) {
-    return MoreObjects.firstNonNull(cfg.getString("elasticsearch", subsection, name), defaultValue);
-  }
 }

java/com/google/gerrit/pgm/init/InitIndex.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.pgm.init;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 import com.google.gerrit.index.SchemaDefinitions;
 import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.init.api.InitFlags;
@@ -63,13 +62,7 @@ class InitIndex implements InitStep {
     if (type == IndexType.ELASTICSEARCH) {
       Section elasticsearch = sections.get("elasticsearch", null);
       elasticsearch.string("Index Prefix", "prefix", "gerrit_");
-      String name = ui.readString("default", "Server Name");
-
-      Section defaultServer = sections.get("elasticsearch", name);
-      defaultServer.select(
-          "Transport protocol", "protocol", "http", Sets.newHashSet("http", "https"));
-      defaultServer.string("Hostname", "hostname", "localhost");
-      defaultServer.string("Port", "port", "9200");
+      elasticsearch.string("Server", "server", "http://localhost:9200");
       index.string("Result window size", "maxLimit", "10000");
     }
 

java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -342,7 +342,7 @@ public class CommitValidators {
           sb.append('\n');
           sb.append("Hint: A potential ");
           sb.append(FooterConstants.CHANGE_ID.getName());
-          sb.append("Change-Id was found, but it was not in the ");
+          sb.append(" was found, but it was not in the ");
           sb.append("footer (last paragraph) of the commit message.");
         }
       }

java/com/google/gerrit/server/permissions/ChangeControl.java

@@ -327,8 +327,7 @@ class ChangeControl {
           case ABANDON:
             return canAbandon();
           case DELETE:
-            return (getProjectControl().isAdmin()
-                || (isOwner() && refControl.canDeleteOwnChanges(isOwner())));
+            return (getProjectControl().isAdmin() || (refControl.canDeleteChanges(isOwner())));
           case ADD_PATCH_SET:
             return canAddPatchSet();
           case EDIT_ASSIGNEE:

java/com/google/gerrit/server/permissions/RefControl.java

@@ -133,9 +133,10 @@ class RefControl {
     return canPerform(Permission.EDIT_TOPIC_NAME, false, true);
   }
 
-  /** @return true if this user can delete their own changes. */
-  boolean canDeleteOwnChanges(boolean isChangeOwner) {
-    return canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner, false);
+  /** @return true if this user can delete changes. */
+  boolean canDeleteChanges(boolean isChangeOwner) {
+    return canPerform(Permission.DELETE_CHANGES)
+        || (isChangeOwner && canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner, false));
   }
 
   /** The range of permitted values associated with a label permission. */

java/com/google/gerrit/server/util/PluginLogFile.java

@@ -38,7 +38,7 @@ public abstract class PluginLogFile implements LifecycleListener {
 
   @Override
   public void start() {
-    AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout);
+    AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true);
     Logger logger = LogManager.getLogger(logName);
     logger.removeAppender(logName);
     logger.addAppender(asyncAppender);

java/com/google/gerrit/server/util/SystemLog.java

@@ -77,13 +77,18 @@ public class SystemLog {
   }
 
   private AsyncAppender createAsyncAppender(String name, Layout layout, boolean rotate) {
+    return createAsyncAppender(name, layout, rotate, false);
+  }
+
+  public AsyncAppender createAsyncAppender(
+      String name, Layout layout, boolean rotate, boolean forPlugin) {
     AsyncAppender async = new AsyncAppender();
     async.setName(name);
     async.setBlocking(true);
     async.setBufferSize(asyncLoggingBufferSize);
     async.setLocationInfo(false);
 
-    if (shouldConfigure()) {
+    if (forPlugin || shouldConfigure()) {
       async.addAppender(createAppender(site.logs_dir, name, layout, rotate));
     } else {
       Appender appender = LogManager.getLogger(name).getAppender(name);

java/com/google/gerrit/sshd/commands/SetAccountCommand.java

@@ -18,9 +18,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.common.RawInputUtil;
-import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.errors.EmailException;
-import com.google.gerrit.extensions.annotations.RequiresCapability;
 import com.google.gerrit.extensions.api.accounts.EmailInput;
 import com.google.gerrit.extensions.api.accounts.SshKeyInput;
 import com.google.gerrit.extensions.common.EmailInfo;
@@ -31,11 +29,15 @@ import com.google.gerrit.extensions.common.SshKeyInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
 import com.google.gerrit.server.account.AccountSshKey;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.restapi.account.AddSshKey;
 import com.google.gerrit.server.restapi.account.CreateEmail;
@@ -52,6 +54,7 @@ import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -66,7 +69,6 @@ import org.kohsuke.args4j.Option;
 
 /** Set a user's account settings. * */
 @CommandMetaData(name = "set-account", description = "Change an account's settings")
-@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
 final class SetAccountCommand extends SshCommand {
 
   @Argument(
@@ -118,6 +120,9 @@ final class SetAccountCommand extends SshCommand {
   @Option(name = "--clear-http-password", usage = "clear HTTP password for the account")
   private boolean clearHttpPassword;
 
+  @Option(name = "--generate-http-password", usage = "generate a new HTTP password for the account")
+  private boolean generateHttpPassword;
+
   @Inject private IdentifiedUser.GenericFactory genericUserFactory;
 
   @Inject private CreateEmail createEmail;
@@ -142,20 +147,54 @@ final class SetAccountCommand extends SshCommand {
 
   @Inject private DeleteSshKey deleteSshKey;
 
+  @Inject private PermissionBackend permissionBackend;
+
+  @Inject private Provider<CurrentUser> userProvider;
+
   private AccountResource rsrc;
 
   @Override
   public void run() throws Exception {
+    user = genericUserFactory.create(id);
+
     validate();
     setAccount();
   }
 
   private void validate() throws UnloggedFailure {
-    if (active && inactive) {
-      throw die("--active and --inactive options are mutually exclusive.");
+    PermissionBackend.WithUser userPermission = permissionBackend.user(userProvider.get());
+
+    boolean isAdmin = userPermission.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
+    boolean canModifyAccount =
+        isAdmin || userPermission.testOrFalse(GlobalPermission.MODIFY_ACCOUNT);
+
+    if (!user.hasSameAccountId(userProvider.get()) && !canModifyAccount) {
+      throw die(
+          "Setting another user's account information requries 'modify account' or 'administrate server' capabilities.");
     }
-    if (clearHttpPassword && !Strings.isNullOrEmpty(httpPassword)) {
-      throw die("--http-password and --clear-http-password options are mutually exclusive.");
+    if (active || inactive) {
+      if (!canModifyAccount) {
+        throw die(
+            "--active and --inactive require 'modify account' or 'administrate server' capabilities.");
+      }
+      if (active && inactive) {
+        throw die("--active and --inactive options are mutually exclusive.");
+      }
+    }
+
+    if (generateHttpPassword && clearHttpPassword) {
+      throw die("--generate-http-password and --clear-http-password are mutually exclusive.");
+    }
+    if (!Strings.isNullOrEmpty(httpPassword)) { // gave --http-password
+      if (!isAdmin) {
+        throw die("--http-password requires 'administrate server' capabilities.");
+      }
+      if (generateHttpPassword) {
+        throw die("--http-password and --generate-http-password options are mutually exclusive.");
+      }
+      if (clearHttpPassword) {
+        throw die("--http-password and --clear-http-password options are mutually exclusive.");
+      }
     }
     if (addSshKeys.contains("-") && deleteSshKeys.contains("-")) {
       throw die("Only one option may use the stdin");
@@ -197,10 +236,16 @@ final class SetAccountCommand extends SshCommand {
         putName.apply(rsrc, in);
       }
 
-      if (httpPassword != null || clearHttpPassword) {
+      if (httpPassword != null || clearHttpPassword || generateHttpPassword) {
         HttpPasswordInput in = new HttpPasswordInput();
         in.httpPassword = httpPassword;
-        putHttpPassword.apply(rsrc, in);
+        if (generateHttpPassword) {
+          in.generate = true;
+        }
+        Response<String> resp = putHttpPassword.apply(rsrc, in);
+        if (generateHttpPassword) {
+          stdout.print("New password: " + resp.value() + "\n");
+        }
       }
 
       if (active) {

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -44,6 +44,7 @@ import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
 import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
+import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.testing.Util.category;
 import static com.google.gerrit.server.project.testing.Util.value;
@@ -91,6 +92,8 @@ import com.google.gerrit.extensions.api.groups.GroupApi;
 import com.google.gerrit.extensions.api.projects.BranchApi;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.api.projects.ProjectApi;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
 import com.google.gerrit.extensions.client.ChangeKind;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.Comment.Range;
@@ -999,12 +1002,7 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void deleteNewChangeAsAdmin() throws Exception {
-    PushOneCommit.Result changeResult = createChange();
-    String changeId = changeResult.getChangeId();
-
-    gApi.changes().id(changeId).delete();
-
-    assertThat(query(changeId)).isEmpty();
+    deleteChangeAsUser(admin, admin);
   }
 
   @Test
@@ -1021,51 +1019,86 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
-  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
-    allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
-    deleteChangeAsUser();
+  public void deleteNewChangeAsUserWithDeleteChangesPermissionForGroup() throws Exception {
+    allow("refs/*", Permission.DELETE_CHANGES, REGISTERED_USERS);
+    deleteChangeAsUser(admin, user);
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
-  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
-    allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
-    deleteChangeAsUser();
+  public void deleteNewChangeAsUserWithDeleteChangesPermissionForProjectOwners() throws Exception {
+    GroupApi groupApi = gApi.groups().create(name("delete-change"));
+    groupApi.addMembers("user");
+
+    ProjectInput in = new ProjectInput();
+    in.name = name("delete-change");
+    in.owners = Lists.newArrayListWithCapacity(1);
+    in.owners.add(groupApi.name());
+    in.createEmptyCommit = true;
+    ProjectApi api = gApi.projects().create(in);
+
+    Project.NameKey nameKey = new Project.NameKey(api.get().name);
+
+    try (ProjectConfigUpdate u = updateProject(nameKey)) {
+      Util.allow(u.getConfig(), Permission.DELETE_CHANGES, PROJECT_OWNERS, "refs/*");
+      u.save();
+    }
+
+    deleteChangeAsUser(nameKey, admin, user);
   }
 
-  private void deleteChangeAsUser() throws Exception {
-    try {
-      PushOneCommit.Result changeResult =
-          pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
-      String changeId = changeResult.getChangeId();
-      int id = changeResult.getChange().getId().id;
-      RevCommit commit = changeResult.getCommit();
+  @Test
+  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
+    allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
+    deleteChangeAsUser(user, user);
+  }
 
-      setApiUser(user);
+  @Test
+  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
+    allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
+    deleteChangeAsUser(user, user);
+  }
+
+  private void deleteChangeAsUser(
+      com.google.gerrit.acceptance.TestAccount owner,
+      com.google.gerrit.acceptance.TestAccount deleteAs)
+      throws Exception {
+    deleteChangeAsUser(project, owner, deleteAs);
+  }
+
+  private void deleteChangeAsUser(
+      Project.NameKey projectName,
+      com.google.gerrit.acceptance.TestAccount owner,
+      com.google.gerrit.acceptance.TestAccount deleteAs)
+      throws Exception {
+    try {
+      setApiUser(owner);
+      ChangeInput in = new ChangeInput();
+      in.project = projectName.get();
+      in.branch = "refs/heads/master";
+      in.subject = "test";
+      ChangeInfo changeInfo = gApi.changes().create(in).get();
+      String changeId = changeInfo.changeId;
+      int id = changeInfo._number;
+      String commit = changeInfo.currentRevision;
+
+      assertThat(gApi.changes().id(changeId).info().owner._accountId).isEqualTo(owner.id.get());
+
+      setApiUser(deleteAs);
       gApi.changes().id(changeId).delete();
 
       assertThat(query(changeId)).isEmpty();
 
       String ref = new Change.Id(id).toRefPrefix() + "1";
-      eventRecorder.assertRefUpdatedEvents(project.get(), ref, null, commit, commit, null);
+      eventRecorder.assertRefUpdatedEvents(projectName.get(), ref, null, commit, commit, null);
     } finally {
       removePermission(project, "refs/*", Permission.DELETE_OWN_CHANGES);
+      removePermission(project, "refs/*", Permission.DELETE_CHANGES);
     }
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
   public void deleteNewChangeOfAnotherUserAsAdmin() throws Exception {
-    PushOneCommit.Result changeResult =
-        pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
-    changeResult.assertOkStatus();
-    String changeId = changeResult.getChangeId();
-
-    setApiUser(admin);
-    gApi.changes().id(changeId).delete();
-
-    assertThat(query(changeId)).isEmpty();
+    deleteChangeAsUser(user, admin);
   }
 
   @Test

javatests/com/google/gerrit/acceptance/pgm/BUILD

@@ -22,6 +22,7 @@ acceptance_tests(
     group = "elastic",
     labels = [
         "elastic",
+        "flaky",
         "pgm",
         "no_windows",
     ],

javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java

@@ -32,7 +32,8 @@ public class ElasticIndexIT extends AbstractIndexTests {
     elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
     String indicesPrefix = UUID.randomUUID().toString();
     Config cfg = new Config();
-    ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix);
+    String password = version == ElasticVersion.V5_6 ? "changeme" : null;
+    ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix, password);
     return cfg;
   }
 

javatests/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java

@@ -0,0 +1,153 @@
+// Copyright (C) 2018 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.elasticsearch;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.DEFAULT_MAX_RETRY_TIMEOUT_MS;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.DEFAULT_USERNAME;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_MAX_RETRY_TIMEOUT;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_PASSWORD;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_PREFIX;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_SERVER;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_USERNAME;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.MAX_RETRY_TIMEOUT_UNIT;
+import static com.google.gerrit.elasticsearch.ElasticConfiguration.SECTION_ELASTICSEARCH;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.ProvisionException;
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class ElasticConfigurationTest {
+  @Rule public ExpectedException exception = ExpectedException.none();
+
+  @Test
+  public void singleServerNoOtherConfig() throws Exception {
+    Config cfg = newConfig();
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertHosts(esCfg, "http://elastic:1234");
+    assertThat(esCfg.username).isNull();
+    assertThat(esCfg.password).isNull();
+    assertThat(esCfg.prefix).isEmpty();
+    assertThat(esCfg.maxRetryTimeout).isEqualTo(DEFAULT_MAX_RETRY_TIMEOUT_MS);
+  }
+
+  @Test
+  public void serverWithoutPortSpecified() throws Exception {
+    Config cfg = new Config();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertHosts(esCfg, "http://elastic:9200");
+  }
+
+  @Test
+  public void prefix() throws Exception {
+    Config cfg = newConfig();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PREFIX, "myprefix");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertThat(esCfg.prefix).isEqualTo("myprefix");
+  }
+
+  @Test
+  public void maxRetryTimeoutInDefaultUnit() {
+    Config cfg = newConfig();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_MAX_RETRY_TIMEOUT, "45000");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertThat(esCfg.maxRetryTimeout).isEqualTo(45000);
+  }
+
+  @Test
+  public void maxRetryTimeoutInOtherUnit() {
+    Config cfg = newConfig();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_MAX_RETRY_TIMEOUT, "45 s");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertThat(esCfg.maxRetryTimeout)
+        .isEqualTo(MAX_RETRY_TIMEOUT_UNIT.convert(45, TimeUnit.SECONDS));
+  }
+
+  @Test
+  public void withAuthentication() throws Exception {
+    Config cfg = newConfig();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_USERNAME, "myself");
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertThat(esCfg.username).isEqualTo("myself");
+    assertThat(esCfg.password).isEqualTo("s3kr3t");
+  }
+
+  @Test
+  public void withAuthenticationPasswordOnlyUsesDefaultUsername() throws Exception {
+    Config cfg = newConfig();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD, "s3kr3t");
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertThat(esCfg.username).isEqualTo(DEFAULT_USERNAME);
+    assertThat(esCfg.password).isEqualTo("s3kr3t");
+  }
+
+  @Test
+  public void multipleServers() throws Exception {
+    Config cfg = new Config();
+    cfg.setStringList(
+        SECTION_ELASTICSEARCH,
+        null,
+        KEY_SERVER,
+        ImmutableList.of("http://elastic1:1234", "http://elastic2:1234"));
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234");
+  }
+
+  @Test
+  public void noServers() throws Exception {
+    assertProvisionException(new Config());
+  }
+
+  @Test
+  public void singleServerInvalid() throws Exception {
+    Config cfg = new Config();
+    cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "foo");
+    assertProvisionException(cfg);
+  }
+
+  @Test
+  public void multipleServersIncludingInvalid() throws Exception {
+    Config cfg = new Config();
+    cfg.setStringList(
+        SECTION_ELASTICSEARCH, null, KEY_SERVER, ImmutableList.of("http://elastic1:1234", "foo"));
+    ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
+    assertHosts(esCfg, "http://elastic1:1234");
+  }
+
+  private static Config newConfig() {
+    Config config = new Config();
+    config.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic:1234");
+    return config;
+  }
+
+  private void assertHosts(ElasticConfiguration cfg, Object... hostURIs) throws Exception {
+    assertThat(Arrays.asList(cfg.getHosts()).stream().map(h -> h.toURI()).collect(toList()))
+        .containsExactly(hostURIs);
+  }
+
+  private void assertProvisionException(Config cfg) throws Exception {
+    exception.expect(ProvisionException.class);
+    exception.expectMessage("No valid Elasticsearch servers configured");
+    new ElasticConfiguration(cfg);
+  }
+}

javatests/com/google/gerrit/elasticsearch/ElasticContainer.java

@@ -45,11 +45,11 @@ public class ElasticContainer<SELF extends ElasticContainer<SELF>> extends Gener
       case V2_4:
         return "elasticsearch:2.4.6-alpine";
       case V5_6:
-        return "elasticsearch:5.6.10-alpine";
+        return "docker.elastic.co/elasticsearch/elasticsearch:5.6.10";
       case V6_2:
         return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.2.4";
       case V6_3:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.0";
+        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.1";
     }
     throw new IllegalStateException("No tests for version: " + version.name());
   }

javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java

@@ -32,13 +32,18 @@ public final class ElasticTestUtils {
     }
   }
 
-  public static void configure(Config config, int port, String prefix) {
+  public static void configure(Config config, int port, String prefix, String password) {
     config.setEnum("index", null, "type", IndexType.ELASTICSEARCH);
-    config.setString("elasticsearch", "test", "protocol", "http");
-    config.setString("elasticsearch", "test", "hostname", "localhost");
-    config.setInt("elasticsearch", "test", "port", port);
+    config.setString("elasticsearch", null, "server", "http://localhost:" + port);
     config.setString("elasticsearch", null, "prefix", prefix);
-    config.setString("index", null, "maxLimit", "10000");
+    config.setInt("index", null, "maxLimit", 10000);
+    if (password != null) {
+      config.setString("elasticsearch", null, "password", password);
+    }
+  }
+
+  public static void configure(Config config, int port, String prefix) {
+    configure(config, port, prefix, null);
   }
 
   public static void createAllIndexes(Injector injector) throws IOException {

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java

@@ -67,7 +67,7 @@ public class ElasticV5QueryAccountsTest extends AbstractQueryAccountsTest {
     Config elasticsearchConfig = new Config(config);
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = testName();
-    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
     return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java

@@ -67,7 +67,7 @@ public class ElasticV5QueryChangesTest extends AbstractQueryChangesTest {
     Config elasticsearchConfig = new Config(config);
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = testName();
-    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
     return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java

@@ -67,7 +67,7 @@ public class ElasticV5QueryGroupsTest extends AbstractQueryGroupsTest {
     Config elasticsearchConfig = new Config(config);
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = testName();
-    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
+    ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix, "changeme");
     return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
   }
 }