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
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 3ad4401..6c785f8 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -2296,17 +2296,6 @@ 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.
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java index 6c4454f..0e6bb29 100644 --- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -53,8 +53,6 @@ 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 @@ 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));
diff --git a/java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java b/java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java index 884325d..2ea1f82 100644 --- a/java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java +++ b/java/com/google/gerrit/pgm/util/ErrorLogJsonLayout.java
@@ -26,7 +26,6 @@ /** 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);
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java index 34f0eb5..3968fc0 100644 --- a/java/com/google/gerrit/server/IdentifiedUser.java +++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -36,7 +36,6 @@ 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 @@ private final Provider<String> canonicalUrl; private final AccountCache accountCache; private final GroupBackend groupBackend; - private final Boolean enableReverseDnsLookup; @Inject public GenericFactory( @@ -77,7 +75,6 @@ Realm realm, @AnonymousCowardName String anonymousCowardName, @CanonicalWebUrl Provider<String> canonicalUrl, - @EnableReverseDnsLookup Boolean enableReverseDnsLookup, AccountCache accountCache, GroupBackend groupBackend) { this.authConfig = authConfig; @@ -86,7 +83,6 @@ this.canonicalUrl = canonicalUrl; this.accountCache = accountCache; this.groupBackend = groupBackend; - this.enableReverseDnsLookup = enableReverseDnsLookup; } public IdentifiedUser create(AccountState state) { @@ -97,7 +93,6 @@ canonicalUrl, accountCache, groupBackend, - enableReverseDnsLookup, Providers.of(null), state, null); @@ -134,7 +129,6 @@ canonicalUrl, accountCache, groupBackend, - enableReverseDnsLookup, Providers.of(remotePeer), id, caller, @@ -156,7 +150,6 @@ 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 @@ @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 @@ this.canonicalUrl = canonicalUrl; this.accountCache = accountCache; this.groupBackend = groupBackend; - this.enableReverseDnsLookup = enableReverseDnsLookup; this.remotePeerProvider = remotePeerProvider; } @@ -191,7 +182,6 @@ canonicalUrl, accountCache, groupBackend, - enableReverseDnsLookup, remotePeerProvider, id, null, @@ -206,7 +196,6 @@ canonicalUrl, accountCache, groupBackend, - enableReverseDnsLookup, remotePeerProvider, id, caller, @@ -224,7 +213,6 @@ 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 @@ Provider<String> canonicalUrl, AccountCache accountCache, GroupBackend groupBackend, - Boolean enableReverseDnsLookup, @Nullable Provider<SocketAddress> remotePeerProvider, AccountState state, @Nullable CurrentUser realUser) { @@ -254,7 +241,6 @@ canonicalUrl, accountCache, groupBackend, - enableReverseDnsLookup, remotePeerProvider, state.account().id(), realUser, @@ -269,7 +255,6 @@ Provider<String> canonicalUrl, AccountCache accountCache, GroupBackend groupBackend, - Boolean enableReverseDnsLookup, @Nullable Provider<SocketAddress> remotePeerProvider, Account.Id id, @Nullable CurrentUser realUser, @@ -281,7 +266,6 @@ 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 @@ Providers.of(canonicalUrl.get()), accountCache, groupBackend, - enableReverseDnsLookup, remotePeer, state, realUser); @@ -541,18 +524,11 @@ 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(); - } }
diff --git a/java/com/google/gerrit/server/config/EnableReverseDnsLookup.java b/java/com/google/gerrit/server/config/EnableReverseDnsLookup.java deleted file mode 100644 index ec57338..0000000 --- a/java/com/google/gerrit/server/config/EnableReverseDnsLookup.java +++ /dev/null
@@ -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 {}
diff --git a/java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java b/java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java deleted file mode 100644 index 71086a9..0000000 --- a/java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java +++ /dev/null
@@ -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; - } -}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java index 3da3fd3..b2f6d17 100644 --- a/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -309,10 +309,6 @@ 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);
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java index aaeed02..d9cda26 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -44,7 +44,6 @@ 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 @@ 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 @@ 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()))); }
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java index 1672ce1..69bf3f3 100644 --- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java +++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -25,7 +25,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.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 @@ 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)
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java index 83c6542..d399ed4 100644 --- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -45,7 +45,6 @@ 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 @@ 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);