From 9b326d59fe43edb0fb567080ce9100baedaa60a9 Mon Sep 17 00:00:00 2001 From: Tobias Laatsch Date: Fri, 1 Mar 2024 19:20:34 +0100 Subject: [PATCH 1/2] Fix handling of Redis nodes with IPv6 addresses See gh-39819 --- .../PropertiesRedisConnectionDetails.java | 6 +++-- .../redis/RedisConnectionConfiguration.java | 4 ++-- .../data/redis/RedisConnectionDetails.java | 3 +++ ...PropertiesRedisConnectionDetailsTests.java | 17 +++++++++++--- .../redis/RedisAutoConfigurationTests.java | 22 ++++++++++++------- 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java index 72e2c9efe46..8102f8f9414 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java @@ -113,8 +113,10 @@ class PropertiesRedisConnectionDetails implements RedisConnectionDetails { } private Node asNode(String node) { - String[] components = node.split(":"); - return new Node(components[0], Integer.parseInt(components[1])); + int portSeparatorIndex = node.lastIndexOf(':'); + String host = node.substring(0, portSeparatorIndex); + int port = Integer.parseInt(node.substring(portSeparatorIndex + 1)); + return new Node(host, port); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java index e3d59342d8e..32eea8d5975 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java @@ -139,7 +139,7 @@ abstract class RedisConnectionConfiguration { } private List getNodes(Cluster cluster) { - return cluster.getNodes().stream().map((node) -> "%s:%d".formatted(node.host(), node.port())).toList(); + return cluster.getNodes().stream().map(Node::asString).toList(); } protected final RedisProperties getProperties() { @@ -162,7 +162,7 @@ abstract class RedisConnectionConfiguration { private List createSentinels(Sentinel sentinel) { List nodes = new ArrayList<>(); for (Node node : sentinel.getNodes()) { - nodes.add(new RedisNode(node.host(), node.port())); + nodes.add(RedisNode.fromString(node.asString())); } return nodes; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java index 7aed831582c..cae5758f017 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java @@ -185,6 +185,9 @@ public interface RedisConnectionDetails extends ConnectionDetails { */ record Node(String host, int port) { + String asString() { + return this.host + ":" + this.port; + } } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java index e4c325e5241..9c7b42a7253 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java @@ -119,12 +119,23 @@ class PropertiesRedisConnectionDetailsTests { @Test void clusterIsConfigured() { RedisProperties.Cluster cluster = new RedisProperties.Cluster(); - cluster.setNodes(List.of("first:1111", "second:2222", "third:3333")); + cluster.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333")); this.properties.setCluster(cluster); PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties); assertThat(connectionDetails.getCluster().getNodes()).containsExactly( - new RedisConnectionDetails.Node("first", 1111), new RedisConnectionDetails.Node("second", 2222), - new RedisConnectionDetails.Node("third", 3333)); + new RedisConnectionDetails.Node("localhost", 1111), new RedisConnectionDetails.Node("127.0.0.1", 2222), + new RedisConnectionDetails.Node("[::1]", 3333)); + } + + @Test + void sentinelIsConfigured() { + RedisProperties.Sentinel sentinel = new RedisProperties.Sentinel(); + sentinel.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333")); + this.properties.setSentinel(sentinel); + PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties); + assertThat(connectionDetails.getSentinel().getNodes()).containsExactly( + new RedisConnectionDetails.Node("localhost", 1111), new RedisConnectionDetails.Node("127.0.0.1", 2222), + new RedisConnectionDetails.Node("[::1]", 3333)); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index 3e6ab344b29..fa197e0a5f6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -308,8 +308,14 @@ class RedisAutoConfigurationTests { this.contextRunner .withPropertyValues("spring.data.redis.sentinel.master:mymaster", "spring.data.redis.sentinel.nodes:" + StringUtils.collectionToCommaDelimitedString(sentinels)) - .run((context) -> assertThat(context.getBean(LettuceConnectionFactory.class).isRedisSentinelAware()) - .isTrue()); + .run((context) -> { + assertThat(context.getBean(LettuceConnectionFactory.class).isRedisSentinelAware()).isTrue(); + assertThat(context.getBean(LettuceConnectionFactory.class).getSentinelConfiguration()).isNotNull(); + assertThat(context.getBean(LettuceConnectionFactory.class).getSentinelConfiguration().getSentinels()) + .isNotNull() + .extracting(RedisNode::toString) + .containsExactlyInAnyOrder("[0:0:0:0:0:0:0:1]:26379", "[0:0:0:0:0:0:0:1]:26380"); + }); } @Test @@ -394,17 +400,17 @@ class RedisAutoConfigurationTests { @Test void testRedisConfigurationWithCluster() { - List clusterNodes = Arrays.asList("127.0.0.1:27379", "127.0.0.1:27380"); + List clusterNodes = Arrays.asList("127.0.0.1:27379", "127.0.0.1:27380", "[::1]:27381"); this.contextRunner .withPropertyValues("spring.data.redis.cluster.nodes[0]:" + clusterNodes.get(0), - "spring.data.redis.cluster.nodes[1]:" + clusterNodes.get(1)) + "spring.data.redis.cluster.nodes[1]:" + clusterNodes.get(1), + "spring.data.redis.cluster.nodes[2]:" + clusterNodes.get(2)) .run((context) -> { RedisClusterConfiguration clusterConfiguration = context.getBean(LettuceConnectionFactory.class) .getClusterConfiguration(); - assertThat(clusterConfiguration.getClusterNodes()).hasSize(2); - assertThat(clusterConfiguration.getClusterNodes()) - .extracting((node) -> node.getHost() + ":" + node.getPort()) - .containsExactlyInAnyOrder("127.0.0.1:27379", "127.0.0.1:27380"); + assertThat(clusterConfiguration.getClusterNodes()).hasSize(3); + assertThat(clusterConfiguration.getClusterNodes()).extracting(RedisNode::asString) + .containsExactlyInAnyOrder("127.0.0.1:27379", "127.0.0.1:27380", "[::1]:27381"); }); } From 34f53d48b951b218aa50e8e599af29d316ca42e0 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 22 Apr 2024 11:49:44 +0100 Subject: [PATCH 2/2] Polish "Fix handling of Redis nodes with IPv6 addresses" See gh-39819 --- .../PropertiesRedisConnectionDetails.java | 2 +- .../redis/RedisConnectionConfiguration.java | 14 +++++++++----- .../data/redis/RedisConnectionDetails.java | 5 +---- ...PropertiesRedisConnectionDetailsTests.java | 14 +++++++------- .../redis/RedisAutoConfigurationTests.java | 19 +++++++++---------- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java index 8102f8f9414..458baa93819 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetails.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java index 32eea8d5975..b64347ab22d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java @@ -123,8 +123,8 @@ abstract class RedisConnectionConfiguration { } RedisProperties.Cluster clusterProperties = this.properties.getCluster(); if (this.connectionDetails.getCluster() != null) { - RedisClusterConfiguration config = new RedisClusterConfiguration( - getNodes(this.connectionDetails.getCluster())); + RedisClusterConfiguration config = new RedisClusterConfiguration(); + config.setClusterNodes(getNodes(this.connectionDetails.getCluster())); if (clusterProperties != null && clusterProperties.getMaxRedirects() != null) { config.setMaxRedirects(clusterProperties.getMaxRedirects()); } @@ -138,8 +138,12 @@ abstract class RedisConnectionConfiguration { return null; } - private List getNodes(Cluster cluster) { - return cluster.getNodes().stream().map(Node::asString).toList(); + private List getNodes(Cluster cluster) { + return cluster.getNodes().stream().map(this::asRedisNode).toList(); + } + + private RedisNode asRedisNode(Node node) { + return new RedisNode(node.host(), node.port()); } protected final RedisProperties getProperties() { @@ -162,7 +166,7 @@ abstract class RedisConnectionConfiguration { private List createSentinels(Sentinel sentinel) { List nodes = new ArrayList<>(); for (Node node : sentinel.getNodes()) { - nodes.add(RedisNode.fromString(node.asString())); + nodes.add(asRedisNode(node)); } return nodes; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java index cae5758f017..b423e61ab2e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionDetails.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -185,9 +185,6 @@ public interface RedisConnectionDetails extends ConnectionDetails { */ record Node(String host, int port) { - String asString() { - return this.host + ":" + this.port; - } } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java index 9c7b42a7253..4d3aa55ab84 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/PropertiesRedisConnectionDetailsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.util.List; import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.data.redis.RedisConnectionDetails.Node; + import static org.assertj.core.api.Assertions.assertThat; /** @@ -122,9 +124,8 @@ class PropertiesRedisConnectionDetailsTests { cluster.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333")); this.properties.setCluster(cluster); PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties); - assertThat(connectionDetails.getCluster().getNodes()).containsExactly( - new RedisConnectionDetails.Node("localhost", 1111), new RedisConnectionDetails.Node("127.0.0.1", 2222), - new RedisConnectionDetails.Node("[::1]", 3333)); + assertThat(connectionDetails.getCluster().getNodes()).containsExactly(new Node("localhost", 1111), + new Node("127.0.0.1", 2222), new Node("[::1]", 3333)); } @Test @@ -133,9 +134,8 @@ class PropertiesRedisConnectionDetailsTests { sentinel.setNodes(List.of("localhost:1111", "127.0.0.1:2222", "[::1]:3333")); this.properties.setSentinel(sentinel); PropertiesRedisConnectionDetails connectionDetails = new PropertiesRedisConnectionDetails(this.properties); - assertThat(connectionDetails.getSentinel().getNodes()).containsExactly( - new RedisConnectionDetails.Node("localhost", 1111), new RedisConnectionDetails.Node("127.0.0.1", 2222), - new RedisConnectionDetails.Node("[::1]", 3333)); + assertThat(connectionDetails.getSentinel().getNodes()).containsExactly(new Node("localhost", 1111), + new Node("127.0.0.1", 2222), new Node("[::1]", 3333)); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index fa197e0a5f6..b1fd47fb37d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -309,12 +309,11 @@ class RedisAutoConfigurationTests { .withPropertyValues("spring.data.redis.sentinel.master:mymaster", "spring.data.redis.sentinel.nodes:" + StringUtils.collectionToCommaDelimitedString(sentinels)) .run((context) -> { - assertThat(context.getBean(LettuceConnectionFactory.class).isRedisSentinelAware()).isTrue(); - assertThat(context.getBean(LettuceConnectionFactory.class).getSentinelConfiguration()).isNotNull(); - assertThat(context.getBean(LettuceConnectionFactory.class).getSentinelConfiguration().getSentinels()) - .isNotNull() - .extracting(RedisNode::toString) - .containsExactlyInAnyOrder("[0:0:0:0:0:0:0:1]:26379", "[0:0:0:0:0:0:0:1]:26380"); + LettuceConnectionFactory connectionFactory = context.getBean(LettuceConnectionFactory.class); + assertThat(connectionFactory.isRedisSentinelAware()).isTrue(); + assertThat(connectionFactory.getSentinelConfiguration().getSentinels()).isNotNull() + .containsExactlyInAnyOrder(new RedisNode("[0:0:0:0:0:0:0:1]", 26379), + new RedisNode("[0:0:0:0:0:0:0:1]", 26380)); }); } @@ -409,10 +408,10 @@ class RedisAutoConfigurationTests { RedisClusterConfiguration clusterConfiguration = context.getBean(LettuceConnectionFactory.class) .getClusterConfiguration(); assertThat(clusterConfiguration.getClusterNodes()).hasSize(3); - assertThat(clusterConfiguration.getClusterNodes()).extracting(RedisNode::asString) - .containsExactlyInAnyOrder("127.0.0.1:27379", "127.0.0.1:27380", "[::1]:27381"); + assertThat(clusterConfiguration.getClusterNodes()).containsExactlyInAnyOrder( + new RedisNode("127.0.0.1", 27379), new RedisNode("127.0.0.1", 27380), + new RedisNode("[::1]", 27381)); }); - } @Test