From ccf8f86c8dfb9acd2b8c0166bd13a72b0ce4b233 Mon Sep 17 00:00:00 2001 From: Rico Pahlisch Date: Wed, 31 May 2017 12:04:09 +0200 Subject: [PATCH 1/5] use only default https ports for https check spring boot uses all high ports and there are some trouble if ports end with 443 (e.g. 24443, ...) --- .../cloud/netflix/ribbon/DefaultServerIntrospector.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java index a228544f..8c9053d6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java @@ -16,7 +16,9 @@ package org.springframework.cloud.netflix.ribbon; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import com.netflix.loadbalancer.Server; @@ -25,10 +27,11 @@ import com.netflix.loadbalancer.Server; * @author Spencer Gibb */ public class DefaultServerIntrospector implements ServerIntrospector { + private static final List SECURE_PORTS = Arrays.asList(443, 8443); + @Override public boolean isSecure(Server server) { - // Can we do better? - return (""+server.getPort()).endsWith("443"); + return SECURE_PORTS.contains(server.getPort()); } @Override From bdfbe3a87ef7d2b693d5ecb7dcf870d3ed53caed Mon Sep 17 00:00:00 2001 From: pahli Date: Wed, 31 May 2017 22:09:35 +0200 Subject: [PATCH 2/5] make secure ports for DefaultServerIntrospector configurable --- .../ribbon/DefaultServerIntrospector.java | 12 ++-- .../DefaultServerIntrospectorDefaultTest.java | 61 +++++++++++++++++++ .../ribbon/DefaultServerIntrospectorTest.java | 61 +++++++++++++++++++ 3 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java index 8c9053d6..11e515e7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java @@ -16,22 +16,24 @@ package org.springframework.cloud.netflix.ribbon; -import java.util.Arrays; +import com.netflix.loadbalancer.Server; +import org.springframework.beans.factory.annotation.Value; + import java.util.Collections; import java.util.List; import java.util.Map; -import com.netflix.loadbalancer.Server; - /** * @author Spencer Gibb */ public class DefaultServerIntrospector implements ServerIntrospector { - private static final List SECURE_PORTS = Arrays.asList(443, 8443); + + @Value("#{T(java.util.Arrays).asList('${ribbon.securePorts:443,8443}')}") + private List securePorts; @Override public boolean isSecure(Server server) { - return SECURE_PORTS.contains(server.getPort()); + return securePorts.contains(server.getPort()); } @Override diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java new file mode 100644 index 00000000..0636d066 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2013-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.ribbon; + +import com.netflix.loadbalancer.Server; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author Rico Pahlisch + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = DefaultServerIntrospectorDefaultTest.TestConfiguration.class) +public class DefaultServerIntrospectorDefaultTest { + + @Autowired + private ServerIntrospector serverIntrospector; + + @Test + public void testDefaultSslPorts(){ + Server serverMock = mock(Server.class); + when(serverMock.getPort()).thenReturn(443); + Assert.assertTrue(serverIntrospector.isSecure(serverMock)); + when(serverMock.getPort()).thenReturn(8443); + Assert.assertTrue(serverIntrospector.isSecure(serverMock)); + + when(serverMock.getPort()).thenReturn(16443); + Assert.assertFalse(serverIntrospector.isSecure(serverMock)); + } + + @Configuration + protected static class TestConfiguration { + @Bean + public DefaultServerIntrospector defaultServerIntrospector(){ + return new DefaultServerIntrospector(); + } + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java new file mode 100644 index 00000000..3bc9f404 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2013-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.ribbon; + +import com.netflix.loadbalancer.Server; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author Rico Pahlisch + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = DefaultServerIntrospectorTest.TestConfiguration.class) +@TestPropertySource(properties = { "ribbon.securePorts=12345" }) +public class DefaultServerIntrospectorTest { + + @Autowired + private ServerIntrospector serverIntrospector; + + @Test + public void testSecurePortConfiguration(){ + Server serverMock = mock(Server.class); + when(serverMock.getPort()).thenReturn(12345); + Assert.assertTrue(serverIntrospector.isSecure(serverMock)); + + when(serverMock.getPort()).thenReturn(443); + Assert.assertFalse(serverIntrospector.isSecure(serverMock)); + } + + @Configuration + protected static class TestConfiguration { + @Bean + public DefaultServerIntrospector defaultServerIntrospector(){ + return new DefaultServerIntrospector(); + } + } +} From 6d22a407cc5cc70e539d1b119caf427c6e1f054d Mon Sep 17 00:00:00 2001 From: pahli Date: Wed, 31 May 2017 22:25:51 +0200 Subject: [PATCH 3/5] use @ConfigurationProperties --- .../ribbon/DefaultServerIntrospector.java | 8 +++-- .../ribbon/ServerIntrospectorProperties.java | 32 +++++++++++++++++++ .../DefaultServerIntrospectorDefaultTest.java | 2 ++ .../ribbon/DefaultServerIntrospectorTest.java | 8 +++-- 4 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/ServerIntrospectorProperties.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java index 11e515e7..39ef4724 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java @@ -17,7 +17,9 @@ package org.springframework.cloud.netflix.ribbon; import com.netflix.loadbalancer.Server; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Configuration; import java.util.Collections; import java.util.List; @@ -28,12 +30,12 @@ import java.util.Map; */ public class DefaultServerIntrospector implements ServerIntrospector { - @Value("#{T(java.util.Arrays).asList('${ribbon.securePorts:443,8443}')}") - private List securePorts; + @Autowired + ServerIntrospectorProperties serverIntrospectorProperties; @Override public boolean isSecure(Server server) { - return securePorts.contains(server.getPort()); + return serverIntrospectorProperties.getSecurePorts().contains(server.getPort()); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/ServerIntrospectorProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/ServerIntrospectorProperties.java new file mode 100644 index 00000000..e5492f78 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/ServerIntrospectorProperties.java @@ -0,0 +1,32 @@ +/* + * Copyright 2013-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.ribbon; + +import lombok.Data; +import org.springframework.boot.context.properties.ConfigurationProperties; + +import java.util.Arrays; +import java.util.List; + +/** + * @author Rico Pahlisch + */ +@Data +@ConfigurationProperties("ribbon") +public class ServerIntrospectorProperties { + private List securePorts = Arrays.asList(443,8443); +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java index 0636d066..d36945d8 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorDefaultTest.java @@ -21,6 +21,7 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -52,6 +53,7 @@ public class DefaultServerIntrospectorDefaultTest { } @Configuration + @EnableConfigurationProperties(ServerIntrospectorProperties.class) protected static class TestConfiguration { @Bean public DefaultServerIntrospector defaultServerIntrospector(){ diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java index 3bc9f404..4e44a1e3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospectorTest.java @@ -21,6 +21,8 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -35,7 +37,7 @@ import static org.mockito.Mockito.when; */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = DefaultServerIntrospectorTest.TestConfiguration.class) -@TestPropertySource(properties = { "ribbon.securePorts=12345" }) +@TestPropertySource(properties = { "ribbon.securePorts=12345,556" }) public class DefaultServerIntrospectorTest { @Autowired @@ -46,12 +48,14 @@ public class DefaultServerIntrospectorTest { Server serverMock = mock(Server.class); when(serverMock.getPort()).thenReturn(12345); Assert.assertTrue(serverIntrospector.isSecure(serverMock)); - + when(serverMock.getPort()).thenReturn(556); + Assert.assertTrue(serverIntrospector.isSecure(serverMock)); when(serverMock.getPort()).thenReturn(443); Assert.assertFalse(serverIntrospector.isSecure(serverMock)); } @Configuration + @EnableConfigurationProperties(ServerIntrospectorProperties.class) protected static class TestConfiguration { @Bean public DefaultServerIntrospector defaultServerIntrospector(){ From 71c906b00b0ebe3054f450945671fb7d39f15ea8 Mon Sep 17 00:00:00 2001 From: pahli Date: Wed, 31 May 2017 22:33:10 +0200 Subject: [PATCH 4/5] add private accessor --- .../cloud/netflix/ribbon/DefaultServerIntrospector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java index 39ef4724..7f37a3b3 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java @@ -31,7 +31,7 @@ import java.util.Map; public class DefaultServerIntrospector implements ServerIntrospector { @Autowired - ServerIntrospectorProperties serverIntrospectorProperties; + private ServerIntrospectorProperties serverIntrospectorProperties; @Override public boolean isSecure(Server server) { From a1d089038da249774c569843e1e245b9b7be1469 Mon Sep 17 00:00:00 2001 From: pahli Date: Wed, 31 May 2017 23:55:17 +0200 Subject: [PATCH 5/5] make property injection optional --- .../cloud/netflix/ribbon/DefaultServerIntrospector.java | 8 ++++++-- .../cloud/netflix/ribbon/RibbonAutoConfiguration.java | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java index 7f37a3b3..b4180e34 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/DefaultServerIntrospector.java @@ -30,8 +30,12 @@ import java.util.Map; */ public class DefaultServerIntrospector implements ServerIntrospector { - @Autowired - private ServerIntrospectorProperties serverIntrospectorProperties; + private ServerIntrospectorProperties serverIntrospectorProperties = new ServerIntrospectorProperties(); + + @Autowired(required = false) + public void setServerIntrospectorProperties(ServerIntrospectorProperties serverIntrospectorProperties){ + this.serverIntrospectorProperties = serverIntrospectorProperties; + } @Override public boolean isSecure(Server server) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java index 26b99997..8d03633c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java @@ -61,7 +61,7 @@ import com.netflix.ribbon.Ribbon; @RibbonClients @AutoConfigureAfter(name = "org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration") @AutoConfigureBefore({LoadBalancerAutoConfiguration.class, AsyncLoadBalancerAutoConfiguration.class}) -@EnableConfigurationProperties(RibbonEagerLoadProperties.class) +@EnableConfigurationProperties({RibbonEagerLoadProperties.class, ServerIntrospectorProperties.class}) public class RibbonAutoConfiguration { @Autowired(required = false)