Redis authentication support #244
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 { | ||
|
Contributor
There was a problem hiding this comment. It will be useful to also add another testcase showing failure to connect to redisServer (with Auth) and client (no auth).
Contributor
Author
There was a problem hiding this comment. I had to drop from the
|
||
|
|
||
| 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<Jedis> hostConnectionPool = | ||
| new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); | ||
| Connection<Jedis> 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<Jedis> hostConnectionPool = | ||
| new HostConnectionPoolImpl<>(authHost, conFactory, cpConfig, poolMonitor); | ||
| Connection<Jedis> 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<Jedis> hostConnectionPool = | ||
| new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); | ||
| Connection<Jedis> 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<Jedis> hostConnectionPool = | ||
| new HostConnectionPoolImpl<>(noAuthHost, conFactory, cpConfig, poolMonitor); | ||
| Connection<Jedis> 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<Jedis> hostConnectionPool = new HostConnectionPoolImpl<>(authHost, | ||
| jedisConnectionFactory, connectionPoolConfiguration, new CountingConnectionPoolMonitor()); | ||
| Connection<Jedis> 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<HostToken> getTokens(Set<Host> activeHosts) { | ||
| return Collections.singletonList(localHostToken); | ||
| } | ||
|
|
||
| @Override | ||
| public HostToken getTokenForHost(Host host, Set<Host> activeHosts) { | ||
| return localHostToken; | ||
| } | ||
|
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If embedded-redis doesn't work on Windows, use conditional ignore (https://junit.org/junit4/javadoc/4.12/org/junit/Assume.html) to ignore such tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I am curious to know why embedded-redis doesn't work on windows
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a change introduced on the Spinnaker fork of kstyrc/embedded-redis (which smelled abandoned to me). I saw it noted on the README of the Spinnaker fork.
The reason is the library actually embeds OS-specific executables that are executed as separate processes.
I will follow your recommendation to conditionally ignore this test and make sure things are green on Windows.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is reflected here. Checked the tests are skipped on Windows