DATAREDIS-1060 - Redis password should not automatically be applied to Sentinel.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>2.3.0.BUILD-SNAPSHOT</version>
+	<version>2.3.0.DATAREDIS-1060-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 

src/main/asciidoc/reference/redis.adoc

@@ -174,13 +174,14 @@ public RedisConnectionFactory lettuceConnectionFactory() {
 .Configuration Properties
 * `spring.redis.sentinel.master`: name of the master node.
 * `spring.redis.sentinel.nodes`: Comma delimited list of host:port pairs.
+* `spring.redis.sentinel.password`: The password to apply when authenticating with Redis Sentinel
 ====
 
 Sometimes, direct interaction with one of the Sentinels is required. Using `RedisConnectionFactory.getSentinelConnection()` or `RedisConnection.getSentinelCommands()` gives you access to the first active Sentinel configured.
 
 [NOTE]
 ====
-Configuration options like password, timeouts, SSL... also get applied to Redis Sentinel when using https://lettuce.io/[Lettuce].
+Sentinel authentication is only available using https://lettuce.io/[Lettuce].
 ====
 
 [[redis:template]]

src/main/java/org/springframework/data/redis/connection/RedisConfiguration.java

@@ -337,6 +337,56 @@ default void setMaster(final String name) {
 		 * @return {@link Set} of sentinels. Never {@literal null}.
 		 */
 		Set<RedisNode> getSentinels();
+
+		/**
+		 * Get the {@link RedisPassword} used when authenticating with a Redis Server..
+		 *
+		 * @return never {@literal null}.
+		 * @since 2.2.2
+		 */
+		default RedisPassword getDataNodePassword() {
+			return getPassword();
+		}
+
+		/**
+		 * Create and set a {@link RedisPassword} to be used when authenticating with Redis Sentinel from the given
+		 * {@link String}.
+		 *
+		 * @param password can be {@literal null}.
+		 * @since 2.2.2
+		 */
+		default void setSentinelPassword(@Nullable String password) {
+			setSentinelPassword(RedisPassword.of(password));
+		}
+
+		/**
+		 * Create and set a {@link RedisPassword} to be used when authenticating with Redis Sentinel from the given
+		 * {@link Character} sequence.
+		 *
+		 * @param password can be {@literal null}.
+		 * @since 2.2.2
+		 */
+		default void setSentinelPassword(@Nullable char[] password) {
+			setSentinelPassword(RedisPassword.of(password));
+		}
+
+		/**
+		 * Set a {@link RedisPassword} to be used when authenticating with Redis Sentinel.
+		 *
+		 * @param password must not be {@literal null} use {@link RedisPassword#none()} instead.
+		 * @since 2.2.2
+		 */
+		void setSentinelPassword(RedisPassword password);
+
+		/**
+		 * Returns the {@link RedisPassword} to use when connecting to a Redis Sentinel. <br />
+		 * Can be set via {@link #setSentinelPassword(RedisPassword)} or {@link RedisPassword#none()} if no password has
+		 * been set.
+		 *
+		 * @return the {@link RedisPassword} for authenticating with Redis Sentinel.
+		 * @since 2.2.2
+		 */
+		RedisPassword getSentinelPassword();
 	}
 
 	/**

src/main/java/org/springframework/data/redis/connection/RedisSentinelConfiguration.java

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.redis.connection;
 
-import static org.springframework.util.Assert.*;
-import static org.springframework.util.Assert.hasText;
 import static org.springframework.util.StringUtils.*;
 
 import java.util.Collections;
@@ -46,11 +44,14 @@ public class RedisSentinelConfiguration implements RedisConfiguration, SentinelC
 
 	private static final String REDIS_SENTINEL_MASTER_CONFIG_PROPERTY = "spring.redis.sentinel.master";
 	private static final String REDIS_SENTINEL_NODES_CONFIG_PROPERTY = "spring.redis.sentinel.nodes";
+	private static final String REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY = "spring.redis.sentinel.password";
 
 	private @Nullable NamedNode master;
 	private Set<RedisNode> sentinels;
 	private int database;
-	private RedisPassword password = RedisPassword.none();
+
+	private RedisPassword dataNodePassword = RedisPassword.none();
+	private RedisPassword sentinelPassword = RedisPassword.none();
 
 	/**
 	 * Creates new {@link RedisSentinelConfiguration}.
@@ -89,7 +90,7 @@ public RedisSentinelConfiguration(String master, Set<String> sentinelHostAndPort
 	 */
 	public RedisSentinelConfiguration(PropertySource<?> propertySource) {
 
-		notNull(propertySource, "PropertySource must not be null!");
+		Assert.notNull(propertySource, "PropertySource must not be null!");
 
 		this.sentinels = new LinkedHashSet<>();
 
@@ -101,6 +102,10 @@ public RedisSentinelConfiguration(PropertySource<?> propertySource) {
 			appendSentinels(
 					commaDelimitedListToSet(propertySource.getProperty(REDIS_SENTINEL_NODES_CONFIG_PROPERTY).toString()));
 		}
+
+		if (propertySource.containsProperty(REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY)) {
+			this.setSentinelPassword(propertySource.getProperty(REDIS_SENTINEL_PASSWORD_CONFIG_PROPERTY).toString());
+		}
 	}
 
 	/**
@@ -110,7 +115,7 @@ public RedisSentinelConfiguration(PropertySource<?> propertySource) {
 	 */
 	public void setSentinels(Iterable<RedisNode> sentinels) {
 
-		notNull(sentinels, "Cannot set sentinels to 'null'.");
+		Assert.notNull(sentinels, "Cannot set sentinels to 'null'.");
 
 		this.sentinels.clear();
 
@@ -134,7 +139,7 @@ public Set<RedisNode> getSentinels() {
 	 */
 	public void addSentinel(RedisNode sentinel) {
 
-		notNull(sentinel, "Sentinel must not be 'null'.");
+		Assert.notNull(sentinel, "Sentinel must not be 'null'.");
 		this.sentinels.add(sentinel);
 	}
 
@@ -144,7 +149,7 @@ public void addSentinel(RedisNode sentinel) {
 	 */
 	public void setMaster(NamedNode master) {
 
-		notNull(master, "Sentinel master node must not be 'null'.");
+		Assert.notNull(master, "Sentinel master node must not be 'null'.");
 		this.master = master;
 	}
 
@@ -230,7 +235,7 @@ public void setDatabase(int index) {
 	 */
 	@Override
 	public RedisPassword getPassword() {
-		return password;
+		return dataNodePassword;
 	}
 
 	/*
@@ -242,15 +247,34 @@ public void setPassword(RedisPassword password) {
 
 		Assert.notNull(password, "RedisPassword must not be null!");
 
-		this.password = password;
+		this.dataNodePassword = password;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.SentinelConfiguration#setSentinelPassword(org.springframework.data.redis.connection.RedisPassword)
+	 */
+	public void setSentinelPassword(RedisPassword sentinelPassword) {
+
+		Assert.notNull(sentinelPassword, "SentinelPassword must not be null!");
+		this.sentinelPassword = sentinelPassword;
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.redis.connection.RedisConfiguration.SentinelConfiguration#setSentinelPassword()
+	 */
+	@Override
+	public RedisPassword getSentinelPassword() {
+		return sentinelPassword;
 	}
 
 	private RedisNode readHostAndPortFromString(String hostAndPort) {
 
 		String[] args = split(hostAndPort, ":");
 
-		notNull(args, "HostAndPort need to be seperated by  ':'.");
-		isTrue(args.length == 2, "Host and Port String needs to specified as host:port");
+		Assert.notNull(args, "HostAndPort need to be seperated by  ':'.");
+		Assert.isTrue(args.length == 2, "Host and Port String needs to specified as host:port");
 		return new RedisNode(args[0], Integer.valueOf(args[1]).intValue());
 	}
 
@@ -261,8 +285,8 @@ private RedisNode readHostAndPortFromString(String hostAndPort) {
 	 */
 	private static Map<String, Object> asMap(String master, Set<String> sentinelHostAndPorts) {
 
-		hasText(master, "Master address must not be null or empty!");
-		notNull(sentinelHostAndPorts, "SentinelHostAndPorts must not be null!");
+		Assert.hasText(master, "Master address must not be null or empty!");
+		Assert.notNull(sentinelHostAndPorts, "SentinelHostAndPorts must not be null!");
 
 		Map<String, Object> map = new HashMap<>();
 		map.put(REDIS_SENTINEL_MASTER_CONFIG_PROPERTY, master);

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java

@@ -1080,7 +1080,6 @@ private RedisURI getSentinelRedisURI() {
 
 		applyToAll(redisUri, it -> {
 
-			getRedisPassword().toOptional().ifPresent(it::setPassword);
 			clientConfiguration.getClientName().ifPresent(it::setClientName);
 
 			it.setSsl(clientConfiguration.isUseSsl());

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java

@@ -50,6 +50,7 @@
 import org.springframework.data.redis.connection.RedisListCommands.Position;
 import org.springframework.data.redis.connection.RedisNode;
 import org.springframework.data.redis.connection.RedisNode.NodeType;
+import org.springframework.data.redis.connection.RedisPassword;
 import org.springframework.data.redis.connection.RedisSentinelConfiguration;
 import org.springframework.data.redis.connection.RedisServer;
 import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
@@ -198,7 +199,8 @@ abstract public class LettuceConverters extends Converters {
 		};
 
 		SCORED_VALUE_TO_TUPLE = source -> source != null
-				? new DefaultTuple(source.getValue(), Double.valueOf(source.getScore())) : null;
+				? new DefaultTuple(source.getValue(), Double.valueOf(source.getScore()))
+				: null;
 
 		BYTES_LIST_TO_TUPLE_LIST_CONVERTER = source -> {
 
@@ -297,7 +299,8 @@ private Set<Flag> parseFlags(Set<NodeFlag> source) {
 		};
 
 		GEO_COORDINATE_TO_POINT_CONVERTER = geoCoordinate -> geoCoordinate != null
-				? new Point(geoCoordinate.getX().doubleValue(), geoCoordinate.getY().doubleValue()) : null;
+				? new Point(geoCoordinate.getX().doubleValue(), geoCoordinate.getY().doubleValue())
+				: null;
 		GEO_COORDINATE_LIST_TO_POINT_LIST_CONVERTER = new ListConverter<>(GEO_COORDINATE_TO_POINT_CONVERTER);
 
 		KEY_VALUE_UNWRAPPER = source -> source.getValueOrElse(null);
@@ -637,17 +640,25 @@ public static RedisURI sentinelConfigurationToRedisURI(RedisSentinelConfiguratio
 		Assert.notNull(sentinelConfiguration, "RedisSentinelConfiguration is required");
 
 		Set<RedisNode> sentinels = sentinelConfiguration.getSentinels();
-		RedisURI.Builder builder = null;
+		RedisPassword sentinelPassword = sentinelConfiguration.getSentinelPassword();
+		RedisURI.Builder builder = RedisURI.builder();
 		for (RedisNode sentinel : sentinels) {
 
-			if (builder == null) {
-				builder = RedisURI.Builder.sentinel(sentinel.getHost(), sentinel.getPort(),
-						sentinelConfiguration.getMaster().getName());
-			} else {
-				builder.withSentinel(sentinel.getHost(), sentinel.getPort());
+			RedisURI.Builder sentinelBuilder = RedisURI.Builder.redis(sentinel.getHost(), sentinel.getPort());
+
+			if (sentinelPassword.isPresent()) {
+				sentinelBuilder.withPassword(sentinelPassword.get());
 			}
+			builder.withSentinel(sentinelBuilder.build());
 		}
 
+		RedisPassword password = sentinelConfiguration.getPassword();
+		if (password.isPresent()) {
+			builder.withPassword(password.get());
+		}
+
+		builder.withSentinelMasterId(sentinelConfiguration.getMaster().getName());
+
 		return builder.build();
 	}
 

src/test/java/org/springframework/data/redis/connection/RedisSentinelConfigurationUnitTests.java

@@ -22,7 +22,6 @@
 import java.util.HashSet;
 
 import org.junit.Test;
-
 import org.springframework.core.env.PropertySource;
 import org.springframework.mock.env.MockPropertySource;
 import org.springframework.util.StringUtils;
@@ -51,8 +50,7 @@ public void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndSingleH
 	public void shouldCreateRedisSentinelConfigurationCorrectlyGivenMasterAndMultipleHostAndPortStrings() {
 
 		RedisSentinelConfiguration config = new RedisSentinelConfiguration("mymaster",
-				new HashSet<>(Arrays.asList(
-				HOST_AND_PORT_1, HOST_AND_PORT_2, HOST_AND_PORT_3)));
+				new HashSet<>(Arrays.asList(HOST_AND_PORT_1, HOST_AND_PORT_2, HOST_AND_PORT_3)));
 
 		assertThat(config.getSentinels().size()).isEqualTo(3);
 		assertThat(config.getSentinels()).contains(new RedisNode("127.0.0.1", 123), new RedisNode("localhost", 456),
@@ -121,4 +119,29 @@ public void shouldBeCreatedCorrecltyGivenValidPropertySourceWithMasterAndMultipl
 		assertThat(config.getSentinels()).contains(new RedisNode("127.0.0.1", 123), new RedisNode("localhost", 456),
 				new RedisNode("localhost", 789));
 	}
+
+	@Test // DATAREDIS-1060
+	public void dataNodePasswordDoesNotAffectSentinelPassword() {
+
+		RedisPassword password = RedisPassword.of("88888888-8x8-getting-creative-now");
+		RedisSentinelConfiguration configuration = new RedisSentinelConfiguration("myMaster",
+				Collections.singleton(HOST_AND_PORT_1));
+		configuration.setPassword(password);
+
+		assertThat(configuration.getSentinelPassword()).isEqualTo(RedisPassword.none());
+	}
+
+	@Test // DATAREDIS-1060
+	public void readSentinelPasswordFromConfigProperty() {
+
+		MockPropertySource propertySource = new MockPropertySource();
+		propertySource.setProperty("spring.redis.sentinel.master", "myMaster");
+		propertySource.setProperty("spring.redis.sentinel.nodes", HOST_AND_PORT_1);
+		propertySource.setProperty("spring.redis.sentinel.password", "computer-says-no");
+
+		RedisSentinelConfiguration config = new RedisSentinelConfiguration(propertySource);
+
+		assertThat(config.getSentinelPassword()).isEqualTo(RedisPassword.of("computer-says-no"));
+		assertThat(config.getSentinels()).hasSize(1).contains(new RedisNode("127.0.0.1", 123));
+	}
 }