diff --git a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryDiscoveryProperties.java b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryDiscoveryProperties.java index d5df358..a2ff8d2 100644 --- a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryDiscoveryProperties.java +++ b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryDiscoveryProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2018 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. @@ -35,6 +35,11 @@ public class CloudFoundryDiscoveryProperties { */ private long heartbeatFrequency = 5000; + /** + * Port to use when no port is defined by ribbon. + */ + private int defaultServerPort = 80; + public boolean isEnabled() { return enabled; } @@ -50,4 +55,21 @@ public class CloudFoundryDiscoveryProperties { public void setHeartbeatFrequency(long heartbeatFrequency) { this.heartbeatFrequency = heartbeatFrequency; } -} \ No newline at end of file + + public int getDefaultServerPort() { + return defaultServerPort; + } + + public void setDefaultServerPort(int defaultServerPort) { + this.defaultServerPort = defaultServerPort; + } + + @Override + public String toString() { + return "CloudFoundryDiscoveryProperties{" + + "enabled=" + enabled + + ", heartbeatFrequency=" + heartbeatFrequency + + ", defaultServerPort=" + defaultServerPort + + '}'; + } +} diff --git a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryRibbonClientConfiguration.java b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryRibbonClientConfiguration.java index 4a0ec71..4929b54 100644 --- a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryRibbonClientConfiguration.java +++ b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryRibbonClientConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2018 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. @@ -16,19 +16,17 @@ package org.springframework.cloud.cloudfoundry.discovery; -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.IClientConfig; -import com.netflix.config.ConfigurationManager; -import com.netflix.config.DynamicPropertyFactory; -import com.netflix.config.DynamicStringProperty; -import com.netflix.loadbalancer.ServerList; -import org.springframework.beans.factory.annotation.Value; +import javax.annotation.PostConstruct; + import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.cloud.cloudfoundry.CloudFoundryService; +import org.springframework.cloud.netflix.ribbon.RibbonClientName; +import org.springframework.cloud.netflix.ribbon.RibbonUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import javax.annotation.PostConstruct; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ServerList; /** * @author Josh Long @@ -36,10 +34,7 @@ import javax.annotation.PostConstruct; @Configuration public class CloudFoundryRibbonClientConfiguration { - protected static final String DEFAULT_NAMESPACE = "ribbon"; - protected static final String VALUE_NOT_SET = "__not__set__"; - - @Value("${ribbon.client.name}") + @RibbonClientName private String serviceId; public CloudFoundryRibbonClientConfiguration() { @@ -51,36 +46,16 @@ public class CloudFoundryRibbonClientConfiguration { @Bean @ConditionalOnMissingBean - public ServerList ribbonServerList(CloudFoundryService svc, - IClientConfig config) { - CloudFoundryServerList cloudFoundryServerList = new CloudFoundryServerList(svc); + public ServerList ribbonServerList(CloudFoundryService svc, IClientConfig config, + CloudFoundryDiscoveryProperties properties) { + CloudFoundryServerList cloudFoundryServerList = new CloudFoundryServerList(svc, properties); cloudFoundryServerList.initWithNiwsConfig(config); return cloudFoundryServerList; } @PostConstruct public void postConstruct() { - // FIXME: what should this be? - setProp(this.serviceId, - CommonClientConfigKey.DeploymentContextBasedVipAddresses.key(), - this.serviceId); - setProp(this.serviceId, CommonClientConfigKey.EnableZoneAffinity.key(), "true"); + RibbonUtils.initializeRibbonDefaults(this.serviceId); } - protected void setProp(String serviceId, String suffix, String value) { - // how to set the namespace properly? - String key = getKey(serviceId, suffix); - DynamicStringProperty property = getProperty(key); - if (property.get().equals(VALUE_NOT_SET)) { - ConfigurationManager.getConfigInstance().setProperty(key, value); - } - } - - protected DynamicStringProperty getProperty(String key) { - return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET); - } - - protected String getKey(String serviceId, String suffix) { - return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix; - } } \ No newline at end of file diff --git a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerList.java b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerList.java index 3ffc4d5..0cc6c22 100644 --- a/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerList.java +++ b/spring-cloud-cloudfoundry-discovery/src/main/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerList.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2018 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. @@ -16,13 +16,17 @@ package org.springframework.cloud.cloudfoundry.discovery; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.AbstractServerList; -import org.springframework.cloud.cloudfoundry.CloudFoundryService; - import java.util.ArrayList; import java.util.List; +import org.springframework.cloud.cloudfoundry.CloudFoundryService; +import org.springframework.cloud.netflix.ribbon.RibbonUtils; +import org.springframework.cloud.netflix.ribbon.RibbonUtils.RibbonProperties; +import org.springframework.util.Assert; + +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.AbstractServerList; + /** * @author Josh Long */ @@ -31,14 +35,18 @@ public class CloudFoundryServerList extends AbstractServerList cloudFoundryServers() { + Assert.notNull(this.clientConfig, "clientConfig may not be null"); + + RibbonProperties ribbon = RibbonUtils.from(clientConfig); + + Boolean secure = ribbon.getSecure(); + Integer securePort = ribbon.getSecurePort(); + Integer nonSecurePort = ribbon.getPort(); + + final int port; + if (secure != null && secure && securePort != null) { + port = securePort; + } else if (nonSecurePort != null) { + port = nonSecurePort; + } else { + port = this.properties.getDefaultServerPort(); + } + return cloudFoundryService .getApplicationInstances(this.serviceId) - .map(tpl -> new CloudFoundryServer(tpl.getT1().getName(), tpl.getT1().getUrls().get(0), 80)) + .map(tpl -> new CloudFoundryServer(tpl.getT1().getName(), tpl.getT1().getUrls().get(0), port)) .collectList() .blockOptional() .orElse(new ArrayList<>()); } + + /** for testing */ String getServiceId() { + return serviceId; + } } \ No newline at end of file diff --git a/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerListTest.java b/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerListTest.java index b373492..9474cc0 100644 --- a/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerListTest.java +++ b/spring-cloud-cloudfoundry-discovery/src/test/java/org/springframework/cloud/cloudfoundry/discovery/CloudFoundryServerListTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2018 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. @@ -16,20 +16,18 @@ package org.springframework.cloud.cloudfoundry.discovery; -import java.lang.reflect.Field; import java.util.List; import org.cloudfoundry.operations.applications.ApplicationDetail; import org.cloudfoundry.operations.applications.InstanceDetail; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; import org.springframework.cloud.cloudfoundry.CloudFoundryService; -import org.springframework.util.ReflectionUtils; +import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,11 +45,22 @@ public class CloudFoundryServerListTest { @Before public void setUp() { + IClientConfig iClientConfig = IClientConfig.Builder.newBuilder(this.serviceId) + .withSecure(true) + .build(); + iClientConfig.set(CommonClientConfigKey.SecurePort, 443); - IClientConfig iClientConfig = mock(IClientConfig.class); - when(iClientConfig.getClientName()).thenReturn(this.serviceId); + Tuple2 tuple2 = getInstanceDetail(); CloudFoundryService cfs = mock(CloudFoundryService.class); + when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2)); + + this.cloudFoundryServerList = new CloudFoundryServerList(cfs, new CloudFoundryDiscoveryProperties()); + this.cloudFoundryServerList.initWithNiwsConfig(iClientConfig); + } + + private Tuple2 getInstanceDetail() { + // @formatter:off ApplicationDetail applicationDetail = ApplicationDetail .builder() .instances(2) @@ -61,7 +70,7 @@ public class CloudFoundryServerListTest { .id("id") .requestedState("requestedState") .runningInstances(2) - .url("http://my-app.cfapps.io") + .url("https://my-app.cfapps.io") .diskQuota(20) .build(); @@ -69,28 +78,43 @@ public class CloudFoundryServerListTest { .builder() .index("0") .build(); + // @formatter:on - Tuple2 tuple2 = Tuples.of(applicationDetail, instanceDetail); - Mockito.when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2)); - - this.cloudFoundryServerList = new CloudFoundryServerList(cfs); - this.cloudFoundryServerList.initWithNiwsConfig(iClientConfig); + return Tuples.of(applicationDetail, instanceDetail); } @Test public void testListOfServers() { List initialListOfServers = this.cloudFoundryServerList.getInitialListOfServers(); List updatedListOfServers = this.cloudFoundryServerList.getUpdatedListOfServers(); - Assert.assertEquals(updatedListOfServers, initialListOfServers); - Assert.assertTrue(initialListOfServers.size() == 1); + assertThat(initialListOfServers) + .containsExactly(updatedListOfServers.toArray(new CloudFoundryServer[0])) + .hasSize(1); + + CloudFoundryServer server = initialListOfServers.get(0); + assertThat(server.getPort()).isEqualTo(443); } @Test - public void testInit() throws Exception { - Field field = ReflectionUtils.findField(this.cloudFoundryServerList.getClass(), "serviceId"); - assert field != null; - ReflectionUtils.makeAccessible(field); - Assert.assertEquals(String.class.cast(field.get(this.cloudFoundryServerList)), this.serviceId); + public void testDefaultServerPort() { + IClientConfig iClientConfig = mock(IClientConfig.class); + when(iClientConfig.getClientName()).thenReturn(this.serviceId); + + Tuple2 tuple2 = getInstanceDetail(); + + CloudFoundryService cfs = mock(CloudFoundryService.class); + when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2)); + + CloudFoundryServerList serverList = new CloudFoundryServerList(cfs, new CloudFoundryDiscoveryProperties()); + serverList.initWithNiwsConfig(iClientConfig); + + CloudFoundryServer server = serverList.getInitialListOfServers().get(0); + assertThat(server.getPort()).isEqualTo(80); + } + + @Test + public void testInit() { + assertThat(this.cloudFoundryServerList.getServiceId()).isEqualTo(this.serviceId); } }