opendev/gerrit
Code Issues Proposed changes

Merge changes from topic "reverse-dns-lookup"

Browse Source 
* changes:
  Disable gerrit.enableReverseDnsLookup by default
  Replace gerrit.disableReverseDnsLookup with gerrit.enableReverseDnsLookup
This commit is contained in:
David Pursehouse 2019-02-25 08:36:58 +00:00 committed by Gerrit Code Review
commit b4be95a3da
9 changed files with 47 additions and 42 deletions

11
Documentation/config-gerrit.txt

@ -2015,11 +2015,16 @@ Only used when `gerrit.reportBugUrl` is set.
+
Defaults to "Report Bug".
[[gerrit.disableReverseDnsLookup]]gerrit.disableReverseDnsLookup::
[[gerrit.enableReverseDnsLookup]]gerrit.enableReverseDnsLookup::
+
Disables reverse DNS lookup during computing ref log entry for identified user.
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.
+
Defaults to false.
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::
+

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

@ -48,8 +48,8 @@ import com.google.gerrit.server.config.AdministrateServerGroups;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultUrlFormatter;
import com.google.gerrit.server.config.DisableReverseDnsLookup;
import com.google.gerrit.server.config.DisableReverseDnsLookupProvider;
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;
@ -120,8 +120,8 @@ public class BatchProgramModule extends FactoryModule {
.annotatedWith(CanonicalWebUrl.class)
.toProvider(CanonicalWebUrlProvider.class);
bind(Boolean.class)
.annotatedWith(DisableReverseDnsLookup.class)
.toProvider(DisableReverseDnsLookupProvider.class)
.annotatedWith(EnableReverseDnsLookup.class)
.toProvider(EnableReverseDnsLookupProvider.class)
.in(SINGLETON);
bind(Realm.class).to(FakeRealm.class);
bind(IdentifiedUser.class).toProvider(Providers.of(null));

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

@ -32,7 +32,7 @@ import com.google.gerrit.server.account.Realm;
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.DisableReverseDnsLookup;
import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
import com.google.inject.OutOfScopeException;
@ -67,7 +67,7 @@ public class IdentifiedUser extends CurrentUser {
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
private final Boolean disableReverseDnsLookup;
private final Boolean enableReverseDnsLookup;
@Inject
public GenericFactory(
@ -75,7 +75,7 @@ public class IdentifiedUser extends CurrentUser {
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl Provider<String> canonicalUrl,
@DisableReverseDnsLookup Boolean disableReverseDnsLookup,
@EnableReverseDnsLookup Boolean enableReverseDnsLookup,
AccountCache accountCache,
GroupBackend groupBackend) {
this.authConfig = authConfig;
@ -84,7 +84,7 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.enableReverseDnsLookup = enableReverseDnsLookup;
}
public IdentifiedUser create(AccountState state) {
@ -95,7 +95,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
Providers.of(null),
state,
null);
@ -118,7 +118,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
Providers.of(remotePeer),
id,
caller);
@ -139,7 +139,7 @@ public class IdentifiedUser extends CurrentUser {
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
private final Boolean disableReverseDnsLookup;
private final Boolean enableReverseDnsLookup;
private final Provider<SocketAddress> remotePeerProvider;
@Inject
@ -150,7 +150,7 @@ public class IdentifiedUser extends CurrentUser {
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@DisableReverseDnsLookup Boolean disableReverseDnsLookup,
@EnableReverseDnsLookup Boolean enableReverseDnsLookup,
@RemotePeer Provider<SocketAddress> remotePeerProvider) {
this.authConfig = authConfig;
this.realm = realm;
@ -158,7 +158,7 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.enableReverseDnsLookup = enableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
}
@ -170,7 +170,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
id,
null);
@ -184,7 +184,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
id,
caller);
@ -201,7 +201,7 @@ public class IdentifiedUser extends CurrentUser {
private final Realm realm;
private final GroupBackend groupBackend;
private final String anonymousCowardName;
private final Boolean disableReverseDnsLookup;
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.
@ -221,7 +221,7 @@ public class IdentifiedUser extends CurrentUser {
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
Boolean disableReverseDnsLookup,
Boolean enableReverseDnsLookup,
@Nullable Provider<SocketAddress> remotePeerProvider,
AccountState state,
@Nullable CurrentUser realUser) {
@ -232,7 +232,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
state.getAccount().getId(),
realUser);
@ -246,7 +246,7 @@ public class IdentifiedUser extends CurrentUser {
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
Boolean disableReverseDnsLookup,
Boolean enableReverseDnsLookup,
@Nullable Provider<SocketAddress> remotePeerProvider,
Account.Id id,
@Nullable CurrentUser realUser) {
@ -256,7 +256,7 @@ public class IdentifiedUser extends CurrentUser {
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.enableReverseDnsLookup = enableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
this.accountId = id;
this.realUser = realUser != null ? realUser : this;
@ -523,7 +523,7 @@ public class IdentifiedUser extends CurrentUser {
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeer,
state,
realUser);
@ -554,7 +554,7 @@ public class IdentifiedUser extends CurrentUser {
}
private String getHost(InetAddress in) {
if (Boolean.FALSE.equals(disableReverseDnsLookup)) {
if (Boolean.TRUE.equals(enableReverseDnsLookup)) {
return in.getCanonicalHostName();
}
return in.getHostAddress();

2
java/com/google/gerrit/server/config/DisableReverseDnsLookup.java → java/com/google/gerrit/server/config/EnableReverseDnsLookup.java

@ -21,4 +21,4 @@ import java.lang.annotation.Retention;
@Retention(RUNTIME)
@BindingAnnotation
public @interface DisableReverseDnsLookup {}
public @interface EnableReverseDnsLookup {}

10
java/com/google/gerrit/server/config/DisableReverseDnsLookupProvider.java → java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java

@ -18,16 +18,16 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.Config;
public class DisableReverseDnsLookupProvider implements Provider<Boolean> {
private final boolean disableReverseDnsLookup;
public class EnableReverseDnsLookupProvider implements Provider<Boolean> {
private final Boolean enableReverseDnsLookup;
@Inject
DisableReverseDnsLookupProvider(@GerritServerConfig Config config) {
disableReverseDnsLookup = config.getBoolean("gerrit", null, "disableReverseDnsLookup", false);
EnableReverseDnsLookupProvider(@GerritServerConfig Config config) {
enableReverseDnsLookup = config.getBoolean("gerrit", null, "enableReverseDnsLookup", false);
}
@Override
public Boolean get() {
return disableReverseDnsLookup;
return enableReverseDnsLookup;
}
}

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

@ -295,8 +295,8 @@ public class GerritGlobalModule extends FactoryModule {
bind(SoyTofu.class).annotatedWith(MailTemplates.class).toProvider(MailSoyTofuProvider.class);
bind(FromAddressGenerator.class).toProvider(FromAddressGeneratorProvider.class).in(SINGLETON);
bind(Boolean.class)
.annotatedWith(DisableReverseDnsLookup.class)
.toProvider(DisableReverseDnsLookupProvider.class)
.annotatedWith(EnableReverseDnsLookup.class)
.toProvider(EnableReverseDnsLookupProvider.class)
.in(SINGLETON);
bind(PatchSetInfoFactory.class);

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

@ -43,7 +43,7 @@ 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.DisableReverseDnsLookup;
import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -55,7 +55,7 @@ import org.junit.Test;
public class EmailIT extends AbstractDaemonTest {
@Inject private @AnonymousCowardName String anonymousCowardName;
@Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
@Inject private @DisableReverseDnsLookup Boolean disableReverseDnsLookup;
@Inject private @EnableReverseDnsLookup boolean enableReverseDnsLookup;
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@Inject private AuthConfig authConfig;
@Inject private EmailExpander emailExpander;
@ -275,7 +275,7 @@ public class EmailIT extends AbstractDaemonTest {
realm,
anonymousCowardName,
canonicalUrl,
disableReverseDnsLookup,
enableReverseDnsLookup,
accountCache,
groupBackend);
return atrScope.set(atrScope.newContext(null, userFactory.create(admin.id)));

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

@ -25,7 +25,7 @@ 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.DisableReverseDnsLookup;
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;
@ -81,8 +81,8 @@ public class IdentifiedUserTest extends GerritBaseTests {
@Override
protected void configure() {
bind(Boolean.class)
.annotatedWith(DisableReverseDnsLookup.class)
.toInstance(Boolean.FALSE);
.annotatedWith(EnableReverseDnsLookup.class)
.toInstance(Boolean.TRUE);
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
bind(String.class)
.annotatedWith(AnonymousCowardName.class)

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

@ -42,7 +42,7 @@ 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.DisableReverseDnsLookup;
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;
@ -147,8 +147,8 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
.annotatedWith(CanonicalWebUrl.class)
.toInstance("http://localhost:8080/");
bind(Boolean.class)
.annotatedWith(DisableReverseDnsLookup.class)
.toInstance(Boolean.FALSE);
.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);