diff --git a/build.gradle b/build.gradle index 3673c6ac..8b4cb2b3 100644 --- a/build.gradle +++ b/build.gradle @@ -87,6 +87,7 @@ project(':dyno-jedis') { compile project(':dyno-core') compile project(':dyno-contrib') compile 'redis.clients:jedis:2.9.0' + testCompile 'com.netflix.spinnaker.embedded-redis:embedded-redis:0.8.0' } } diff --git a/dyno-core/src/main/java/com/netflix/dyno/connectionpool/Host.java b/dyno-core/src/main/java/com/netflix/dyno/connectionpool/Host.java index ded96411..11348feb 100644 --- a/dyno-core/src/main/java/com/netflix/dyno/connectionpool/Host.java +++ b/dyno-core/src/main/java/com/netflix/dyno/connectionpool/Host.java @@ -16,8 +16,10 @@ package com.netflix.dyno.connectionpool; import java.net.InetSocketAddress; +import java.util.Objects; import com.netflix.dyno.connectionpool.impl.utils.ConfigUtils; +import org.apache.commons.lang3.StringUtils; /** * Class encapsulating information about a host. @@ -45,6 +47,7 @@ public class Host implements Comparable { private final String datacenter; private String hashtag; private Status status = Status.Down; + private final String password; public enum Status { Up, Down; @@ -66,6 +69,10 @@ public Host(String hostname, int port, String rack, Status status, String hashta this(hostname, null, port, port, rack, ConfigUtils.getDataCenterFromRack(rack), status, hashtag); } + public Host(String hostname, int port, String rack, Status status, String hashtag, String password) { + this(hostname, null, port, port, rack, ConfigUtils.getDataCenterFromRack(rack), status, hashtag, password); + } + public Host(String hostname, String ipAddress, int port, String rack) { this(hostname, ipAddress, port, port, rack, ConfigUtils.getDataCenterFromRack(rack), Status.Down, null); } @@ -88,6 +95,10 @@ public Host(String name, String ipAddress, int port, String rack, String datacen } public Host(String name, String ipAddress, int port, int securePort, String rack, String datacenter, Status status, String hashtag) { + this(name, ipAddress, port, port, rack, datacenter, status, hashtag, null); + } + + public Host(String name, String ipAddress, int port, int securePort, String rack, String datacenter, Status status, String hashtag, String password) { this.hostname = name; this.ipAddress = ipAddress; this.port = port; @@ -96,6 +107,7 @@ public Host(String name, String ipAddress, int port, int securePort, String rack this.status = status; this.datacenter = datacenter; this.hashtag = hashtag; + this.password = StringUtils.isEmpty(password) ? null : password; // Used for the unit tests to prevent host name resolution if (port != -1) { @@ -153,6 +165,10 @@ public boolean isUp() { return status == Status.Up; } + public String getPassword() { + return password; + } + public InetSocketAddress getSocketAddress() { return socketAddress; } @@ -205,7 +221,7 @@ public int compareTo(Host o) { public String toString() { return "Host [hostname=" + hostname + ", ipAddress=" + ipAddress + ", port=" + port + ", rack: " - + rack + ", datacenter: " + datacenter + ", status: " + status.name() + ", hashtag=" + hashtag + "]"; - + + rack + ", datacenter: " + datacenter + ", status: " + status.name() + ", hashtag=" + + hashtag + ", password=" + (Objects.nonNull(password) ? "masked" : "null") + "]"; } } diff --git a/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/HostsUpdater.java b/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/HostsUpdater.java index e13d8bfd..dce7606b 100644 --- a/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/HostsUpdater.java +++ b/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/HostsUpdater.java @@ -109,14 +109,16 @@ public HostStatusTracker refreshHosts() { hostsUpFromHostSupplier.add(new Host(hostFromHostSupplier.getHostName(), hostFromHostSupplier.getIpAddress(), hostFromTokenMapSupplier.getPort(), hostFromTokenMapSupplier.getSecurePort(), hostFromTokenMapSupplier.getRack(), - hostFromTokenMapSupplier.getDatacenter(), Host.Status.Up, hostFromTokenMapSupplier.getHashtag())); + hostFromTokenMapSupplier.getDatacenter(), Host.Status.Up, hostFromTokenMapSupplier.getHashtag(), + hostFromTokenMapSupplier.getPassword())); allHostSetFromTokenMapSupplier.remove(hostFromTokenMapSupplier); } else { Host hostFromTokenMapSupplier = allHostSetFromTokenMapSupplier.get(hostFromHostSupplier); hostsDownFromHostSupplier.add(new Host(hostFromHostSupplier.getHostName(), hostFromHostSupplier.getIpAddress(), hostFromTokenMapSupplier.getPort(), hostFromTokenMapSupplier.getSecurePort(), hostFromTokenMapSupplier.getRack(), - hostFromTokenMapSupplier.getDatacenter(), Host.Status.Down, hostFromTokenMapSupplier.getHashtag())); + hostFromTokenMapSupplier.getDatacenter(), Host.Status.Down, hostFromTokenMapSupplier.getHashtag(), + hostFromTokenMapSupplier.getPassword())); allHostSetFromTokenMapSupplier.remove(hostFromTokenMapSupplier); } } diff --git a/dyno-jedis/src/main/java/com/netflix/dyno/jedis/JedisConnectionFactory.java b/dyno-jedis/src/main/java/com/netflix/dyno/jedis/JedisConnectionFactory.java index 6eca6fda..88db8fea 100644 --- a/dyno-jedis/src/main/java/com/netflix/dyno/jedis/JedisConnectionFactory.java +++ b/dyno-jedis/src/main/java/com/netflix/dyno/jedis/JedisConnectionFactory.java @@ -39,7 +39,9 @@ import com.netflix.dyno.connectionpool.impl.OperationResultImpl; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisShardInfo; import redis.clients.jedis.exceptions.JedisConnectionException; +import redis.clients.util.Sharded; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocketFactory; @@ -75,14 +77,19 @@ public JedisConnection(HostConnectionPool hostPool) { this.hostPool = hostPool; Host host = hostPool.getHost(); - if(sslSocketFactory == null) - { - jedisClient = new Jedis(host.getHostAddress(), host.getPort(), hostPool.getConnectionTimeout(), - hostPool.getSocketTimeout()); - } - else { - jedisClient = new Jedis(host.getHostAddress(), host.getSecurePort(), hostPool.getConnectionTimeout(), - hostPool.getSocketTimeout(), true, sslSocketFactory, new SSLParameters(), null); + if (sslSocketFactory == null) { + JedisShardInfo shardInfo = new JedisShardInfo(host.getHostAddress(), host.getPort(), + hostPool.getConnectionTimeout(), hostPool.getSocketTimeout(), Sharded.DEFAULT_WEIGHT); + shardInfo.setPassword(host.getPassword()); + + jedisClient = new Jedis(shardInfo); + } else { + JedisShardInfo shardInfo = new JedisShardInfo(host.getHostAddress(), host.getPort(), + hostPool.getConnectionTimeout(), hostPool.getSocketTimeout(), Sharded.DEFAULT_WEIGHT, + true, sslSocketFactory, new SSLParameters(), null); + shardInfo.setPassword(host.getPassword()); + + jedisClient = new Jedis(shardInfo); } } diff --git a/dyno-jedis/src/test/java/com/netflix/dyno/jedis/RedisAuthenticationIntegrationTest.java b/dyno-jedis/src/test/java/com/netflix/dyno/jedis/RedisAuthenticationIntegrationTest.java new file mode 100644 index 00000000..9d442dad --- /dev/null +++ b/dyno-jedis/src/test/java/com/netflix/dyno/jedis/RedisAuthenticationIntegrationTest.java @@ -0,0 +1,245 @@ +package com.netflix.dyno.jedis; + +import com.google.common.base.Throwables; +import com.netflix.dyno.connectionpool.Connection; +import com.netflix.dyno.connectionpool.ConnectionPoolConfiguration; +import com.netflix.dyno.connectionpool.Host; +import com.netflix.dyno.connectionpool.Host.Status; +import com.netflix.dyno.connectionpool.HostConnectionPool; +import com.netflix.dyno.connectionpool.HostSupplier; +import com.netflix.dyno.connectionpool.TokenMapSupplier; +import com.netflix.dyno.connectionpool.exception.DynoConnectException; +import com.netflix.dyno.connectionpool.impl.ConnectionPoolConfigurationImpl; +import com.netflix.dyno.connectionpool.impl.CountingConnectionPoolMonitor; +import com.netflix.dyno.connectionpool.impl.HostConnectionPoolImpl; +import com.netflix.dyno.connectionpool.impl.lb.HostToken; +import com.netflix.dyno.contrib.DynoOPMonitor; +import java.net.ConnectException; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import org.junit.After; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; +import redis.clients.jedis.Jedis; +import redis.clients.jedis.exceptions.JedisDataException; +import redis.embedded.RedisServer; +import redis.embedded.RedisServerBuilder; + +public class RedisAuthenticationIntegrationTest { + + private static final int REDIS_PORT = 8998; + private static final String REDIS_RACK = "rack-1c"; + private static final String REDIS_DATACENTER = "rack-1"; + + private RedisServer redisServer; + + @Before + public void setUp() throws Exception { + // skip tests on windows due to https://github.com/spinnaker/embedded-redis#redis-version + Assume.assumeFalse(System.getProperty("os.name").toLowerCase().startsWith("win")); + } + + @After + public void tearDown() throws Exception { + if (redisServer != null) { + redisServer.stop(); + } + } + + @Test + public void testDynoClient_noAuthSuccess() throws Exception { + redisServer = new RedisServer(REDIS_PORT); + redisServer.start(); + + Host noAuthHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Host.Status.Up); + TokenMapSupplierImpl tokenMapSupplier = new TokenMapSupplierImpl(noAuthHost); + DynoJedisClient dynoClient = constructJedisClient(tokenMapSupplier, + () -> Collections.singletonList(noAuthHost)); + + String statusCodeReply = dynoClient.set("some-key", "some-value"); + Assert.assertEquals("OK", statusCodeReply); + + String value = dynoClient.get("some-key"); + Assert.assertEquals("some-value", value); + } + + @Test + public void testDynoClient_authSuccess() throws Exception { + redisServer = new RedisServerBuilder() + .port(REDIS_PORT) + .setting("requirepass password") + .build(); + redisServer.start(); + + Host authHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Status.Up, null, "password"); + + TokenMapSupplierImpl tokenMapSupplier = new TokenMapSupplierImpl(authHost); + DynoJedisClient dynoClient = constructJedisClient(tokenMapSupplier, + () -> Collections.singletonList(authHost)); + + String statusCodeReply = dynoClient.set("some-key", "some-value"); + Assert.assertEquals("OK", statusCodeReply); + + String value = dynoClient.get("some-key"); + Assert.assertEquals("some-value", value); + } + + @Test + public void testJedisConnFactory_noAuthSuccess() throws Exception { + redisServer = new RedisServer(REDIS_PORT); + redisServer.start(); + + Host noAuthHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Host.Status.Up); + + JedisConnectionFactory conFactory = + new JedisConnectionFactory(new DynoOPMonitor("some-application-name"), null); + ConnectionPoolConfiguration cpConfig = new ConnectionPoolConfigurationImpl("some-name"); + CountingConnectionPoolMonitor poolMonitor = new CountingConnectionPoolMonitor(); + HostConnectionPool hostConnectionPool = + new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); + Connection connection = conFactory + .createConnection(hostConnectionPool, null); + + connection.execPing(); + } + + @Test + public void testJedisConnFactory_authSuccess() throws Exception { + redisServer = new RedisServerBuilder() + .port(REDIS_PORT) + .setting("requirepass password") + .build(); + redisServer.start(); + + Host authHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Status.Up, null, "password"); + + JedisConnectionFactory conFactory = + new JedisConnectionFactory(new DynoOPMonitor("some-application-name"), null); + ConnectionPoolConfiguration cpConfig = new ConnectionPoolConfigurationImpl("some-name"); + CountingConnectionPoolMonitor poolMonitor = new CountingConnectionPoolMonitor(); + HostConnectionPool hostConnectionPool = + new HostConnectionPoolImpl<>(authHost, conFactory, cpConfig, poolMonitor); + Connection connection = conFactory + .createConnection(hostConnectionPool, null); + + connection.execPing(); + } + + @Test + public void testJedisConnFactory_connectionFailed() throws Exception { + Host noAuthHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Host.Status.Up); + + JedisConnectionFactory conFactory = + new JedisConnectionFactory(new DynoOPMonitor("some-application-name"), null); + ConnectionPoolConfiguration cpConfig = new ConnectionPoolConfigurationImpl("some-name"); + CountingConnectionPoolMonitor poolMonitor = new CountingConnectionPoolMonitor(); + HostConnectionPool hostConnectionPool = + new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); + Connection connection = conFactory + .createConnection(hostConnectionPool, null); + + try { + connection.execPing(); + Assert.fail("expected to throw"); + } catch (DynoConnectException e) { + Assert.assertTrue("root cause should be connect exception", + Throwables.getRootCause(e) instanceof ConnectException); + } + } + + @Test + public void testJedisConnFactory_authenticationRequired() throws Exception { + redisServer = new RedisServerBuilder() + .port(REDIS_PORT) + .setting("requirepass password") + .build(); + redisServer.start(); + + Host noAuthHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Host.Status.Up); + + JedisConnectionFactory conFactory = + new JedisConnectionFactory(new DynoOPMonitor("some-application-name"), null); + ConnectionPoolConfiguration cpConfig = new ConnectionPoolConfigurationImpl("some-name"); + CountingConnectionPoolMonitor poolMonitor = new CountingConnectionPoolMonitor(); + HostConnectionPool hostConnectionPool = + new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); + Connection connection = conFactory + .createConnection(hostConnectionPool, null); + + try { + connection.execPing(); + Assert.fail("expected to throw"); + } catch (JedisDataException e) { + Assert.assertEquals("NOAUTH Authentication required.", e.getMessage()); + } + } + + @Test + public void testJedisConnFactory_invalidPassword() throws Exception { + redisServer = new RedisServerBuilder() + .port(REDIS_PORT) + .setting("requirepass password") + .build(); + redisServer.start(); + + Host authHost = new Host("localhost", REDIS_PORT, REDIS_RACK, Status.Up, null, + "invalid-password"); + + JedisConnectionFactory jedisConnectionFactory = + new JedisConnectionFactory(new DynoOPMonitor("some-application-name"), null); + + ConnectionPoolConfiguration connectionPoolConfiguration = new ConnectionPoolConfigurationImpl( + "some-name"); + HostConnectionPool hostConnectionPool = new HostConnectionPoolImpl<>(authHost, + jedisConnectionFactory, connectionPoolConfiguration, new CountingConnectionPoolMonitor()); + Connection connection = jedisConnectionFactory + .createConnection(hostConnectionPool, null); + + try { + connection.execPing(); + Assert.fail("expected to throw"); + } catch (JedisDataException e) { + Assert.assertEquals("ERR invalid password", e.getMessage()); + } + } + + private DynoJedisClient constructJedisClient(TokenMapSupplier tokenMapSupplier, + HostSupplier hostSupplier) { + + final ConnectionPoolConfigurationImpl connectionPoolConfiguration = + new ConnectionPoolConfigurationImpl(REDIS_RACK); + connectionPoolConfiguration.withTokenSupplier(tokenMapSupplier); + connectionPoolConfiguration.setLocalRack(REDIS_RACK); + connectionPoolConfiguration.setLocalDataCenter(REDIS_DATACENTER); + + return new DynoJedisClient.Builder() + .withApplicationName("some-application-name") + .withDynomiteClusterName(REDIS_RACK) + .withHostSupplier(hostSupplier) + .withCPConfig(connectionPoolConfiguration) + .build(); + } + + private static class TokenMapSupplierImpl implements TokenMapSupplier { + + private final HostToken localHostToken; + + private TokenMapSupplierImpl(Host host) { + this.localHostToken = new HostToken(100000L, host); + } + + @Override + public List getTokens(Set activeHosts) { + return Collections.singletonList(localHostToken); + } + + @Override + public HostToken getTokenForHost(Host host, Set activeHosts) { + return localHostToken; + } + + } +} diff --git a/dyno-jedis/src/test/resources/log4j.properties b/dyno-jedis/src/test/resources/log4j.properties new file mode 100644 index 00000000..732176f6 --- /dev/null +++ b/dyno-jedis/src/test/resources/log4j.properties @@ -0,0 +1,12 @@ +# logging conf solely for create_delete_tokens tool +log4j.rootLogger=DEBUG,stdout + +# stdout +log4j.appender.stdout=org.apache.log4j.ConsoleAppender +log4j.appender.stdout.layout=org.apache.log4j.PatternLayout +log4j.appender.stdout.layout.ConversionPattern=%-5p %d [%t] %c: %m%n + +log4j.logger.org.apache.http.wire=FATAL +log4j.logger.com.netflix.config=INFO +log4j.logger.com.netflix.discovery=INFO +log4j.logger.org.apache.http=INFO