Remove enableReverseDnsLookup configuration option

In [1] reflog record was extended to include peer's hostname.
In I13247799e2a configuration option was added disableReverseDnsLookup
to disable reverse DNS lookup resolution because of performance
implications.
In I4126828eeac disableReverseDnsLookup configuration option was
replaced with enableReverseDnsLookup.
In I68af8e545fc enableReverseDnsLookup configuration option was disabled
per default.

This change removes this configuration option. This is a preparation
change for making peer's IP address in reflog entry optional and
potentially removing it entirely in future gerrit releases.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/10997

Change-Id: If33c6e1901b70ca64d9b32b0ebb310f3a9daf017

Documentation/config-gerrit.txt

@@ -2296,17 +2296,6 @@ URL to direct users to when they need to report a bug.
 By default unset, meaning no bug report URL will be displayed. Administrators
 should set this to the URL of their issue tracker, if necessary.
 
-[[gerrit.enableReverseDnsLookup]]gerrit.enableReverseDnsLookup::
-+
-Enable reverse DNS lookup during computing ref log entry for identified user,
-to record the actual hostname of the user's host in the ref log.
-+
-Enabling reverse DNS lookup can cause performance issues on git push when
-the reverse DNS lookup is slow.
-+
-Defaults to false, reverse DNS lookup is disabled. The user's IP address
-will be recorded in the ref log rather than their hostname.
-
 [[gerrit.secureStoreClass]]gerrit.secureStoreClass::
 +
 Use the secure store implementation from a specified class.

java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -53,8 +53,6 @@ import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.CanonicalWebUrlProvider;
 import com.google.gerrit.server.config.DefaultPreferencesCacheImpl;
 import com.google.gerrit.server.config.DefaultUrlFormatter;
-import com.google.gerrit.server.config.EnableReverseDnsLookup;
-import com.google.gerrit.server.config.EnableReverseDnsLookupProvider;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
 import com.google.gerrit.server.config.SysExecutorModule;
@@ -135,10 +133,6 @@ public class BatchProgramModule extends FactoryModule {
     bind(String.class)
         .annotatedWith(CanonicalWebUrl.class)
         .toProvider(CanonicalWebUrlProvider.class);
-    bind(Boolean.class)
-        .annotatedWith(EnableReverseDnsLookup.class)
-        .toProvider(EnableReverseDnsLookupProvider.class)
-        .in(SINGLETON);
     bind(Realm.class).to(FakeRealm.class);
     bind(IdentifiedUser.class).toProvider(Providers.of(null));
     bind(ReplacePatchSetSender.Factory.class).toProvider(Providers.of(null));

java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java

@@ -26,7 +26,6 @@ import org.apache.log4j.spi.ThrowableInformation;
 
 /** Layout for formatting error log events in the JSON format. */
 public class ErrorLogJsonLayout extends JsonLayout {
-
   @Override
   public JsonLogEntry toJsonLogEntry(LoggingEvent event) {
     return new ErrorJsonLogEntry(event);

java/com/google/gerrit/server/IdentifiedUser.java

@@ -36,7 +36,6 @@ import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.EnableReverseDnsLookup;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.inject.Inject;
 import com.google.inject.OutOfScopeException;
@@ -69,7 +68,6 @@ public class IdentifiedUser extends CurrentUser {
     private final Provider<String> canonicalUrl;
     private final AccountCache accountCache;
     private final GroupBackend groupBackend;
-    private final Boolean enableReverseDnsLookup;
 
     @Inject
     public GenericFactory(
@@ -77,7 +75,6 @@ public class IdentifiedUser extends CurrentUser {
         Realm realm,
         @AnonymousCowardName String anonymousCowardName,
         @CanonicalWebUrl Provider<String> canonicalUrl,
-        @EnableReverseDnsLookup Boolean enableReverseDnsLookup,
         AccountCache accountCache,
         GroupBackend groupBackend) {
       this.authConfig = authConfig;
@@ -86,7 +83,6 @@ public class IdentifiedUser extends CurrentUser {
       this.canonicalUrl = canonicalUrl;
       this.accountCache = accountCache;
       this.groupBackend = groupBackend;
-      this.enableReverseDnsLookup = enableReverseDnsLookup;
     }
 
     public IdentifiedUser create(AccountState state) {
@@ -97,7 +93,6 @@ public class IdentifiedUser extends CurrentUser {
           canonicalUrl,
           accountCache,
           groupBackend,
-          enableReverseDnsLookup,
           Providers.of(null),
           state,
           null);
@@ -134,7 +129,6 @@ public class IdentifiedUser extends CurrentUser {
           canonicalUrl,
           accountCache,
           groupBackend,
-          enableReverseDnsLookup,
           Providers.of(remotePeer),
           id,
           caller,
@@ -156,7 +150,6 @@ public class IdentifiedUser extends CurrentUser {
     private final Provider<String> canonicalUrl;
     private final AccountCache accountCache;
     private final GroupBackend groupBackend;
-    private final Boolean enableReverseDnsLookup;
     private final Provider<SocketAddress> remotePeerProvider;
 
     @Inject
@@ -167,7 +160,6 @@ public class IdentifiedUser extends CurrentUser {
         @CanonicalWebUrl Provider<String> canonicalUrl,
         AccountCache accountCache,
         GroupBackend groupBackend,
-        @EnableReverseDnsLookup Boolean enableReverseDnsLookup,
         @RemotePeer Provider<SocketAddress> remotePeerProvider) {
       this.authConfig = authConfig;
       this.realm = realm;
@@ -175,7 +167,6 @@ public class IdentifiedUser extends CurrentUser {
       this.canonicalUrl = canonicalUrl;
       this.accountCache = accountCache;
       this.groupBackend = groupBackend;
-      this.enableReverseDnsLookup = enableReverseDnsLookup;
       this.remotePeerProvider = remotePeerProvider;
     }
 
@@ -191,7 +182,6 @@ public class IdentifiedUser extends CurrentUser {
           canonicalUrl,
           accountCache,
           groupBackend,
-          enableReverseDnsLookup,
           remotePeerProvider,
           id,
           null,
@@ -206,7 +196,6 @@ public class IdentifiedUser extends CurrentUser {
           canonicalUrl,
           accountCache,
           groupBackend,
-          enableReverseDnsLookup,
           remotePeerProvider,
           id,
           caller,
@@ -224,7 +213,6 @@ public class IdentifiedUser extends CurrentUser {
   private final Realm realm;
   private final GroupBackend groupBackend;
   private final String anonymousCowardName;
-  private final Boolean enableReverseDnsLookup;
   private final Set<String> validEmails = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
   private final CurrentUser realUser; // Must be final since cached properties depend on it.
 
@@ -243,7 +231,6 @@ public class IdentifiedUser extends CurrentUser {
       Provider<String> canonicalUrl,
       AccountCache accountCache,
       GroupBackend groupBackend,
-      Boolean enableReverseDnsLookup,
       @Nullable Provider<SocketAddress> remotePeerProvider,
       AccountState state,
       @Nullable CurrentUser realUser) {
@@ -254,7 +241,6 @@ public class IdentifiedUser extends CurrentUser {
         canonicalUrl,
         accountCache,
         groupBackend,
-        enableReverseDnsLookup,
         remotePeerProvider,
         state.account().id(),
         realUser,
@@ -269,7 +255,6 @@ public class IdentifiedUser extends CurrentUser {
       Provider<String> canonicalUrl,
       AccountCache accountCache,
       GroupBackend groupBackend,
-      Boolean enableReverseDnsLookup,
       @Nullable Provider<SocketAddress> remotePeerProvider,
       Account.Id id,
       @Nullable CurrentUser realUser,
@@ -281,7 +266,6 @@ public class IdentifiedUser extends CurrentUser {
     this.authConfig = authConfig;
     this.realm = realm;
     this.anonymousCowardName = anonymousCowardName;
-    this.enableReverseDnsLookup = enableReverseDnsLookup;
     this.remotePeerProvider = remotePeerProvider;
     this.accountId = id;
     this.realUser = realUser != null ? realUser : this;
@@ -519,7 +503,6 @@ public class IdentifiedUser extends CurrentUser {
         Providers.of(canonicalUrl.get()),
         accountCache,
         groupBackend,
-        enableReverseDnsLookup,
         remotePeer,
         state,
         realUser);
@@ -541,18 +524,11 @@ public class IdentifiedUser extends CurrentUser {
     if (remotePeer instanceof InetSocketAddress) {
       InetSocketAddress sa = (InetSocketAddress) remotePeer;
       InetAddress in = sa.getAddress();
-      host = in != null ? getHost(in) : sa.getHostName();
+      host = in != null ? in.getHostAddress() : sa.getHostName();
     }
     if (Strings.isNullOrEmpty(host)) {
       return "unknown";
     }
     return host;
   }
-
-  private String getHost(InetAddress in) {
-    if (Boolean.TRUE.equals(enableReverseDnsLookup)) {
-      return in.getCanonicalHostName();
-    }
-    return in.getHostAddress();
-  }
 }

java/com/google/gerrit/server/config/EnableReverseDnsLookup.java

@@ -1,24 +0,0 @@
-// Copyright (C) 2014 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.server.config;
-
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import com.google.inject.BindingAnnotation;
-import java.lang.annotation.Retention;
-
-@Retention(RUNTIME)
-@BindingAnnotation
-public @interface EnableReverseDnsLookup {}

java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java

@@ -1,33 +0,0 @@
-// Copyright (C) 2014 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.server.config;
-
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import org.eclipse.jgit.lib.Config;
-
-public class EnableReverseDnsLookupProvider implements Provider<Boolean> {
-  private final Boolean enableReverseDnsLookup;
-
-  @Inject
-  EnableReverseDnsLookupProvider(@GerritServerConfig Config config) {
-    enableReverseDnsLookup = config.getBoolean("gerrit", null, "enableReverseDnsLookup", false);
-  }
-
-  @Override
-  public Boolean get() {
-    return enableReverseDnsLookup;
-  }
-}

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -309,10 +309,6 @@ public class GerritGlobalModule extends FactoryModule {
 
     bind(SoySauce.class).annotatedWith(MailTemplates.class).toProvider(MailSoySauceProvider.class);
     bind(FromAddressGenerator.class).toProvider(FromAddressGeneratorProvider.class).in(SINGLETON);
-    bind(Boolean.class)
-        .annotatedWith(EnableReverseDnsLookup.class)
-        .toProvider(EnableReverseDnsLookupProvider.class)
-        .in(SINGLETON);
 
     bind(PatchSetInfoFactory.class);
     bind(IdentifiedUser.GenericFactory.class).in(SINGLETON);

javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java

@@ -44,7 +44,6 @@ import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.EnableReverseDnsLookup;
 import com.google.gson.reflect.TypeToken;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -56,7 +55,6 @@ import org.junit.Test;
 public class EmailIT extends AbstractDaemonTest {
   @Inject private @AnonymousCowardName String anonymousCowardName;
   @Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
-  @Inject private @EnableReverseDnsLookup boolean enableReverseDnsLookup;
   @Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
   @Inject private AuthConfig authConfig;
   @Inject private EmailExpander emailExpander;
@@ -276,13 +274,7 @@ public class EmailIT extends AbstractDaemonTest {
   private Context createContextWithCustomRealm(Realm realm) {
     IdentifiedUser.GenericFactory userFactory =
         new IdentifiedUser.GenericFactory(
-            authConfig,
-            realm,
-            anonymousCowardName,
-            canonicalUrl,
-            enableReverseDnsLookup,
-            accountCache,
-            groupBackend);
+            authConfig, realm, anonymousCowardName, canonicalUrl, accountCache, groupBackend);
     return atrScope.set(atrScope.newContext(null, userFactory.create(admin.id())));
   }
 

javatests/com/google/gerrit/server/IdentifiedUserTest.java

@@ -25,7 +25,6 @@ import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.AnonymousCowardNameProvider;
 import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.EnableReverseDnsLookup;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.util.time.TimeUtil;
@@ -79,9 +78,6 @@ public class IdentifiedUserTest {
         new AbstractModule() {
           @Override
           protected void configure() {
-            bind(Boolean.class)
-                .annotatedWith(EnableReverseDnsLookup.class)
-                .toInstance(Boolean.TRUE);
             bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
             bind(String.class)
                 .annotatedWith(AnonymousCowardName.class)

javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -45,7 +45,6 @@ import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.AnonymousCowardNameProvider;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.DefaultUrlFormatter;
-import com.google.gerrit.server.config.EnableReverseDnsLookup;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerId;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -152,9 +151,6 @@ public abstract class AbstractChangeNotesTest {
                 bind(String.class)
                     .annotatedWith(CanonicalWebUrl.class)
                     .toInstance("http://localhost:8080/");
-                bind(Boolean.class)
-                    .annotatedWith(EnableReverseDnsLookup.class)
-                    .toInstance(Boolean.TRUE);
                 bind(Realm.class).to(FakeRealm.class);
                 bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
                 bind(AccountCache.class).toInstance(accountCache);