Skip to content

Commit

Permalink
Client connector config improvements.
Browse files Browse the repository at this point in the history
* Move HTTP config from neo4j-server to neo4j-dbms.
* Better defaults for dbms.connector.
* bolt, http and https connectors all enabled by default.
* Add tests cover old behaviour, and show that new behaviour is
  unsurprising.
* Remove unnecessary lines from neo4j.conf.
  • Loading branch information
apcj committed Oct 3, 2016
1 parent 0ec84b4 commit 75b29e3
Show file tree
Hide file tree
Showing 36 changed files with 620 additions and 219 deletions.
Expand Up @@ -45,8 +45,10 @@ public class BoltConfigIT
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(),
settings -> {
settings.put( boltConnector("0").type.name(), "BOLT" );
settings.put( boltConnector("0").enabled.name(), "true" );
settings.put( boltConnector("0").address.name(), "localhost:7888" );
settings.put( boltConnector("1").type.name(), "BOLT" );
settings.put( boltConnector("1").enabled.name(), "true" );
settings.put( boltConnector("1").address.name(), "localhost:7687" );
settings.put( boltConnector("1").encryption_level.name(), REQUIRED.name() );
Expand Down
Expand Up @@ -135,6 +135,7 @@ public void ensureDatabase( Consumer<Map<String,String>> overrideSettingsFunctio
private Map<String,String> configure( Consumer<Map<String,String>> overrideSettingsFunction ) throws IOException
{
Map<String,String> settings = new HashMap<>();
settings.put( boltConnector( "0" ).type.name(), "BOLT" );
settings.put( boltConnector( "0" ).enabled.name(), "true" );
settings.put( boltConnector( "0" ).encryption_level.name(), OPTIONAL.name() );
settings.put( BoltKernelExtension.Settings.tls_key_file.name(), tempPath( "key.key" ) );
Expand Down
Expand Up @@ -45,7 +45,10 @@ public class RejectTransportEncryptionIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(),
settings -> settings.put( boltConnector( "0" ).encryption_level.name(), DISABLED.name() ) );
settings -> {
settings.put( boltConnector( "0" ).type.name(), "BOLT" );
settings.put( boltConnector( "0" ).encryption_level.name(), DISABLED.name() );
} );
@Rule
public ExpectedException exception = ExpectedException.none();

Expand Down
@@ -0,0 +1,105 @@
package org.neo4j.server.configuration;

import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.factory.Description;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.AdvertisedSocketAddress;
import org.neo4j.helpers.ListenSocketAddress;
import org.neo4j.kernel.configuration.Config;

import static java.util.Collections.singletonList;

import static org.neo4j.graphdb.factory.GraphDatabaseSettings.Connector.ConnectorType.HTTP;
import static org.neo4j.kernel.configuration.GroupSettingSupport.enumerate;
import static org.neo4j.kernel.configuration.Settings.NO_DEFAULT;
import static org.neo4j.kernel.configuration.Settings.advertisedAddress;
import static org.neo4j.kernel.configuration.Settings.legacyFallback;
import static org.neo4j.kernel.configuration.Settings.listenAddress;
import static org.neo4j.kernel.configuration.Settings.options;
import static org.neo4j.kernel.configuration.Settings.setting;

public class ClientConnectorSettings
{
public static HttpConnector httpConnector( String key )
{
return new HttpConnector( key, HttpConnector.Encryption.NONE );
}

public static Optional<HttpConnector> httpConnector( Config config, HttpConnector.Encryption encryption )
{
List<HttpConnector> httpConnectors = config
.view( enumerate( GraphDatabaseSettings.Connector.class ) )
.map( ( key ) -> new HttpConnector( key, encryption ) )
.filter( ( connConfig ) -> connConfig.group.groupKey.equals( encryption.uriScheme ) ||
(config.get( connConfig.type ) == HTTP && config.get( connConfig.encryption ) == encryption) )
.collect( Collectors.toList() );
if ( httpConnectors.isEmpty() )
{
httpConnectors = singletonList( new HttpConnector( encryption ) );
}

return httpConnectors.stream()
.filter( ( connConfig ) -> config.get( connConfig.enabled ) )
.findFirst();
}

@Description("Configuration options for HTTP connectors. " +
"\"(http-connector-key)\" is a placeholder for a unique name for the connector, for instance " +
"\"http-public\" or some other name that describes what the connector is for.")
public static class HttpConnector extends GraphDatabaseSettings.Connector
{
@Description("Enable TLS for this connector")
public final Setting<Encryption> encryption;

@Description( "Address the connector should bind to. " +
"This setting is deprecated and will be replaced by `+listen_address+`" )
public final Setting<ListenSocketAddress> address;

@Description( "Address the connector should bind to" )
public final Setting<ListenSocketAddress> listen_address;

@Description( "Advertised address for this connector" )
public final Setting<AdvertisedSocketAddress> advertised_address;

public HttpConnector()
{
this( Encryption.NONE );
}

public HttpConnector( Encryption encryptionLevel )
{
this( "(http-connector-key)", encryptionLevel );
}

public HttpConnector( String key, Encryption encryptionLevel )
{
super( key, null );
encryption = group.scope( setting( "encryption", options( HttpConnector.Encryption.class ), NO_DEFAULT ) );
Setting<ListenSocketAddress> legacyAddressSetting = listenAddress( "address", encryptionLevel.defaultPort );
Setting<ListenSocketAddress> listenAddressSetting = legacyFallback( legacyAddressSetting,
listenAddress( "listen_address", encryptionLevel.defaultPort ) );

this.address = group.scope( legacyAddressSetting );
this.listen_address = group.scope( listenAddressSetting );
this.advertised_address = group.scope( advertisedAddress( "advertised_address", listenAddressSetting ) );
}

public enum Encryption
{
NONE( "http", 7474 ), TLS( "https", 7473 );

final String uriScheme;
final int defaultPort;

Encryption( String uriScheme, int defaultPort )
{
this.uriScheme = uriScheme;
this.defaultPort = defaultPort;
}
}
}
}
@@ -0,0 +1,165 @@
/*
* Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.server.configuration;

import org.junit.Test;

import org.neo4j.helpers.ListenSocketAddress;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.server.configuration.ClientConnectorSettings.HttpConnector.Encryption;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;

import static org.neo4j.helpers.collection.MapUtil.stringMap;
import static org.neo4j.server.configuration.ClientConnectorSettings.httpConnector;

@SuppressWarnings("OptionalGetWithoutIsPresent")
public class ClientConnectorSettingsTest
{
@Test
public void shouldHaveHttpAndHttpsEnabledByDefault() throws Exception
{
// given
Config config = Config.defaults();

// when
ClientConnectorSettings.HttpConnector httpConnector = httpConnector( config, Encryption.NONE ).get();
ClientConnectorSettings.HttpConnector httpsConnector = httpConnector( config, Encryption.TLS ).get();

// then
assertEquals( new ListenSocketAddress( "localhost", 7474 ), config.get( httpConnector.listen_address ) );
assertEquals( new ListenSocketAddress( "localhost", 7473 ), config.get( httpsConnector.listen_address ) );
}

@Test
public void shouldBeAbleToDisableHttpConnectorWithJustOneParameter() throws Exception
{
// given
Config disableHttpConfig = Config.defaults();
disableHttpConfig.augment( stringMap( "dbms.connector.http.enabled", "false" ) );

// then
assertFalse( httpConnector( disableHttpConfig, Encryption.NONE ).isPresent() );
}

@Test
public void shouldBeAbleToDisableHttpsConnectorWithJustOneParameter() throws Exception
{
// given
Config disableHttpsConfig = Config.defaults();
disableHttpsConfig.augment( stringMap( "dbms.connector.https.enabled", "false" ) );

// then
assertFalse( httpConnector( disableHttpsConfig, Encryption.TLS ).isPresent() );
}

@Test
public void shouldBeAbleToOverrideHttpListenAddressWithJustOneParameter() throws Exception
{
// given
Config config = Config.defaults();
config.augment( stringMap( "dbms.connector.http.listen_address", ":8000" ) );

ClientConnectorSettings.HttpConnector httpConnector = httpConnector( config, Encryption.NONE ).get();

// then
assertEquals( new ListenSocketAddress( "localhost", 8000 ), config.get( httpConnector.listen_address ) );
}

@Test
public void shouldBeAbleToOverrideHttpsListenAddressWithJustOneParameter() throws Exception
{
// given
Config config = Config.defaults();
config.augment( stringMap( "dbms.connector.https.listen_address", ":9000" ) );

ClientConnectorSettings.HttpConnector httpsConnector = httpConnector( config, Encryption.TLS ).get();

// then
assertEquals( new ListenSocketAddress( "localhost", 9000 ), config.get( httpsConnector.listen_address ) );
}

@Test
public void shouldDeriveListenAddressFromDefaultListenAddress() throws Exception
{
// given
Config config = Config.defaults();
config.augment( stringMap( "dbms.connectors.default_listen_address", "0.0.0.0" ) );

// when
ClientConnectorSettings.HttpConnector httpConnector = httpConnector( config, Encryption.NONE ).get();
ClientConnectorSettings.HttpConnector httpsConnector = httpConnector( config, Encryption.TLS ).get();

// then
assertEquals( new ListenSocketAddress( "0.0.0.0", 7474 ), config.get( httpConnector.listen_address ) );
assertEquals( new ListenSocketAddress( "0.0.0.0", 7473 ), config.get( httpsConnector.listen_address ) );
}

@Test
public void shouldDeriveListenAddressFromDefaultListenAddressAndSpecifiedPorts() throws Exception
{
// given
Config config = Config.defaults();
config.augment( stringMap( "dbms.connectors.default_listen_address", "0.0.0.0" ) );
config.augment( stringMap( "dbms.connector.http.listen_address", ":8000" ) );
config.augment( stringMap( "dbms.connector.https.listen_address", ":9000" ) );

// when
ClientConnectorSettings.HttpConnector httpConnector = httpConnector( config, Encryption.NONE ).get();
ClientConnectorSettings.HttpConnector httpsConnector = httpConnector( config, Encryption.TLS ).get();

// then
assertEquals( new ListenSocketAddress( "0.0.0.0", 8000 ), config.get( httpConnector.listen_address ) );
assertEquals( new ListenSocketAddress( "0.0.0.0", 9000 ), config.get( httpsConnector.listen_address ) );
}

@Test
public void shouldStillSupportCustomNameForHttpConnector() throws Exception
{
Config config = Config.defaults();
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.type", "HTTP" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.encryption", "NONE" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.enabled", "true" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.listen_address", ":8000" ) );

// when
ClientConnectorSettings.HttpConnector httpConnector = httpConnector( config, Encryption.NONE ).get();

// then
assertEquals( new ListenSocketAddress( "localhost", 8000 ), config.get( httpConnector.listen_address ) );
}

@Test
public void shouldStillSupportCustomNameForHttpsConnector() throws Exception
{
Config config = Config.defaults();
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.type", "HTTP" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.encryption", "TLS" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.enabled", "true" ) );
config.augment( stringMap( "dbms.connector.random_name_that_will_be_unsupported.listen_address", ":9000" ) );

// when
ClientConnectorSettings.HttpConnector httpsConnector = httpConnector( config, Encryption.TLS ).get();

// then
assertEquals( new ListenSocketAddress( "localhost", 9000 ), config.get( httpsConnector.listen_address ) );
}
}
Expand Up @@ -40,6 +40,8 @@
import org.neo4j.kernel.impl.util.OsBeanUtil;
import org.neo4j.logging.Level;

import static java.util.Collections.singletonList;

import static org.neo4j.graphdb.factory.GraphDatabaseSettings.BoltConnector.EncryptionLevel.OPTIONAL;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.Connector.ConnectorType.BOLT;
import static org.neo4j.kernel.configuration.GroupSettingSupport.enumerate;
Expand Down Expand Up @@ -547,15 +549,16 @@ public static class Connector
// consider future options like non-tcp transports, making `address` a bad choice
// as a setting that applies to every connector, for instance.

protected final GroupSettingSupport group;
public final GroupSettingSupport group;

// For sub-classes, we provide this protected constructor that allows overriding
// the default 'type' setting value.
protected Connector( String key, String typeDefault )
// Note: We no longer use the typeDefault parameter because it made for confusing behaviour;
// connectors with unspecified would override settings of other, unrelated connectors.
// However, we cannot remove the parameter at this
protected Connector( String key, @SuppressWarnings("UnusedParameters") String typeDefault )
{
group = new GroupSettingSupport( Connector.class, key );
enabled = group.scope( setting( "enabled", BOOLEAN, "false" ) );
type = group.scope( setting( "type", options( ConnectorType.class ), typeDefault ) );
enabled = group.scope( setting( "enabled", BOOLEAN, "true" ) );
type = group.scope( setting( "type", options( ConnectorType.class ), NO_DEFAULT ) );
}

public enum ConnectorType
Expand Down Expand Up @@ -590,7 +593,7 @@ public BoltConnector()

public BoltConnector(String key)
{
super(key, ConnectorType.BOLT.name() );
super(key, null );
encryption_level = group.scope(
setting( "tls_level", options( EncryptionLevel.class ), OPTIONAL.name() ));
Setting<ListenSocketAddress> legacyAddressSetting = listenAddress( "address", 7687 );
Expand Down Expand Up @@ -628,12 +631,19 @@ public static BoltConnector boltConnector( String key )

public static List<BoltConnector> boltConnectors( Config config )
{
return config
List<BoltConnector> boltConnectors = config
.view( enumerate( Connector.class ) )
.map( BoltConnector::new )
.filter( ( connConfig ) ->
config.get( connConfig.type ) == BOLT &&
config.get( connConfig.enabled ) )
.filter( connConfig -> connConfig.group.groupKey.equals( "bolt" ) ||
config.get( connConfig.type ) == BOLT )
.collect( Collectors.toList() );

if ( boltConnectors.isEmpty() )
{
boltConnectors = singletonList( new BoltConnector() );
}
return boltConnectors.stream()
.filter( connConfig -> config.get( connConfig.enabled ) )
.collect( Collectors.toList() );
}
}
Expand Up @@ -34,6 +34,7 @@
public class GroupSettingSupport
{
private final String groupName;
public final String groupKey;

/**
* List all keys for a given group type, this is a way to enumerate all instances of a group
Expand Down Expand Up @@ -61,7 +62,7 @@ private static String groupPrefix( Class<?> groupClass )
return groupClass.getAnnotation( Group.class ).value();
}

public GroupSettingSupport( Class<?> groupClass, Object groupKey )
public GroupSettingSupport( Class<?> groupClass, String groupKey )
{
this( groupPrefix( groupClass ), groupKey );
}
Expand All @@ -71,8 +72,9 @@ public GroupSettingSupport( Class<?> groupClass, Object groupKey )
* @param groupKey the unique key for this particular group instance, eg. '0' or 'group1',
* this gets combined with the groupPrefix to eg. `dbms.mygroup.0`
*/
private GroupSettingSupport( String groupPrefix, Object groupKey )
private GroupSettingSupport( String groupPrefix, String groupKey )
{
this.groupKey = groupKey;
this.groupName = String.format( "%s.%s", groupPrefix, groupKey );
}

Expand Down

0 comments on commit 75b29e3

Please sign in to comment.