Skip to content

Commit ace6077

Browse files
spencergibbspencergibb
committed
Continues migration away from ecwid consul client.
This fixes bugs and tests See gh-475
1 parent dd8c476 commit ace6077

File tree

17 files changed

+148
-97
lines changed

17 files changed

+148
-97
lines changed

‎spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySource.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ public void init() {
6565
if (this.context.startsWith("/")) {
6666
this.context = this.context.substring(1);
6767
}
68+
if (this.context.contains("//")) {
69+
this.context = this.context.replace("//", "/");
70+
}
6871

6972
ResponseEntity<List<GetValue>> response = this.source.getKVValues(this.context,
7073
this.configProperties.getAclToken());

‎spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceTests.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828

2929
import org.springframework.cloud.consul.ConsulClient;
3030
import org.springframework.cloud.consul.test.ConsulTestcontainers;
31+
import org.springframework.http.HttpStatus;
32+
import org.springframework.http.ResponseEntity;
3133

3234
import static org.assertj.core.api.Assertions.assertThat;
3335

@@ -47,7 +49,7 @@ public class ConsulPropertySourceTests {
4749
@Before
4850
public void setup() throws CertificateException, KeyStoreException, IOException, NoSuchAlgorithmException {
4951
ConsulTestcontainers.start();
50-
this.prefix = "/consulPropertySourceTests" + new Random().nextInt(Integer.MAX_VALUE);
52+
this.prefix = "consulPropertySourceTests" + new Random().nextInt(Integer.MAX_VALUE);
5153
this.consulClient = ConsulTestcontainers.client();
5254
}
5355

@@ -60,8 +62,15 @@ public void teardown() {
6062
public void testKv() {
6163
// key value properties
6264
this.kvContext = this.prefix + "/kv";
63-
this.consulClient.setKVValue(this.kvContext + "/fooprop", "fookvval");
64-
this.consulClient.setKVValue(this.prefix + "/kv" + "/bar/prop", "8080");
65+
ResponseEntity<Boolean> fookvval = this.consulClient.setKVValue(this.kvContext + "/fooprop", "fookvval");
66+
assertThat(fookvval).isNotNull();
67+
assertThat(fookvval.getStatusCode()).isEqualTo(HttpStatus.OK);
68+
assertThat(fookvval.getBody()).isTrue();
69+
ResponseEntity<Boolean> listResponseEntity = this.consulClient.setKVValue(this.prefix + "/kv" + "/bar/prop",
70+
"8080");
71+
assertThat(listResponseEntity).isNotNull();
72+
assertThat(listResponseEntity.getStatusCode()).isEqualTo(HttpStatus.OK);
73+
assertThat(listResponseEntity.getBody()).isTrue();
6574

6675
ConsulPropertySource source = getConsulPropertySource(new ConsulConfigProperties(), this.kvContext);
6776

‎spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulAutoConfiguration.java‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import org.springframework.web.service.invoker.HttpServiceArgumentResolver;
6767
import org.springframework.web.service.invoker.HttpServiceProxyFactory;
6868
import org.springframework.web.util.DefaultUriBuilderFactory;
69+
import org.springframework.web.util.DefaultUriBuilderFactory.EncodingMode;
6970
import org.springframework.web.util.UriBuilder;
7071

7172
/**
@@ -84,13 +85,20 @@ public ConsulProperties consulProperties() {
8485
return new ConsulProperties();
8586
}
8687

88+
@Bean
89+
@ConditionalOnMissingBean
90+
public ConsulClientSettings consulClientRestClientAdapter(ConsulProperties consulProperties) {
91+
String baseUrl = createConsulClientBaseUrl(consulProperties);
92+
return createConsulClientSettings(baseUrl, consulProperties.getTls());
93+
}
94+
8795
@Bean
8896
@ConditionalOnMissingBean
8997
public ConsulClient coreConsulClient(ConsulProperties consulProperties) {
9098
return createNewConsulClient(consulProperties);
9199
}
92100

93-
public static ConsulClient createNewConsulClient(ConsulProperties consulProperties) {
101+
public static String createConsulClientBaseUrl(ConsulProperties consulProperties) {
94102
UriBuilder uriBuilder = new DefaultUriBuilderFactory().builder();
95103

96104
if (StringUtils.hasLength(consulProperties.getScheme())) {
@@ -111,8 +119,16 @@ public static ConsulClient createNewConsulClient(ConsulProperties consulProperti
111119
}
112120

113121
String baseUrl = uriBuilder.build().toString();
122+
return baseUrl;
123+
}
114124

115-
HttpExchangeAdapter adapter = createConsulRestClientAdapter(baseUrl, consulProperties.getTls());
125+
public static ConsulClient createNewConsulClient(ConsulProperties consulProperties) {
126+
String baseUrl = createConsulClientBaseUrl(consulProperties);
127+
HttpExchangeAdapter adapter = createConsulClientSettings(baseUrl, consulProperties.getTls()).adapter();
128+
return createNewConsulClient(adapter);
129+
}
130+
131+
public static ConsulClient createNewConsulClient(HttpExchangeAdapter adapter) {
116132
HttpServiceProxyFactory factory = HttpServiceProxyFactory.builderFor(adapter)
117133
.customArgumentResolver(new QueryParamsArgumentResolver())
118134
.conversionService(createConsulClientConversionService())
@@ -121,17 +137,21 @@ public static ConsulClient createNewConsulClient(ConsulProperties consulProperti
121137
return factory.createClient(ConsulClient.class);
122138
}
123139

124-
public static RestClientAdapter createConsulRestClientAdapter(String baseUrl,
140+
// TODO: migrate to boot managed sslbundle
141+
public static ConsulClientSettings createConsulClientSettings(String baseUrl,
125142
ConsulProperties.TLSConfig tlsConfig) {
126143
try {
144+
DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory(baseUrl);
145+
uriBuilderFactory.setEncodingMode(EncodingMode.NONE);
127146
RestClient.Builder builder = RestClient.builder()
128147
.defaultStatusHandler(HttpStatusCode::is4xxClientError, (request, response) -> {
129148
})
130149
.defaultStatusHandler(HttpStatusCode::is5xxServerError,
131150
(request, response) -> LOGGER
132151
.error(new String(response.getBody().readAllBytes(), StandardCharsets.UTF_8)))
133-
.baseUrl(baseUrl);
152+
.uriBuilderFactory(uriBuilderFactory);
134153

154+
HttpClientSettings settings = null;
135155
if (tlsConfig != null) {
136156
KeyStore clientStore = KeyStore.getInstance(tlsConfig.getKeyStoreInstanceType().name());
137157
clientStore.load(Files.newInputStream(Paths.get(tlsConfig.getCertificatePath())),
@@ -144,12 +164,12 @@ public static RestClientAdapter createConsulRestClientAdapter(String baseUrl,
144164
SslStoreBundle sslStoreBundle = SslStoreBundle.of(clientStore, tlsConfig.getKeyStorePassword(),
145165
trustStore);
146166
SslBundle sslBundle = SslBundle.of(sslStoreBundle);
147-
HttpClientSettings settings = HttpClientSettings.ofSslBundle(sslBundle);
167+
settings = HttpClientSettings.ofSslBundle(sslBundle);
148168
ClientHttpRequestFactory requestFactory = ClientHttpRequestFactoryBuilder.detect().build(settings);
149169
builder.requestFactory(requestFactory);
150170
}
151171

152-
return RestClientAdapter.create(builder.build());
172+
return new ConsulClientSettings(baseUrl, settings, RestClientAdapter.create(builder.build()));
153173
}
154174
catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) {
155175
throw new RuntimeException(e);
@@ -162,6 +182,10 @@ public static ConversionService createConsulClientConversionService() {
162182
return conversionService;
163183
}
164184

185+
public record ConsulClientSettings(String baseUrl, HttpClientSettings httpClientSettings,
186+
RestClientAdapter adapter) {
187+
}
188+
165189
@Configuration(proxyBeanMethods = false)
166190
@ConditionalOnClass({ Endpoint.class, Health.class })
167191
@EnableConfigurationProperties(ConsulHealthIndicatorProperties.class)

‎spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulClient.java‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.springframework.cloud.consul.model.http.health.Check;
2929
import org.springframework.cloud.consul.model.http.health.HealthService;
3030
import org.springframework.cloud.consul.model.http.kv.GetValue;
31+
import org.springframework.http.MediaType;
3132
import org.springframework.http.ResponseEntity;
3233
import org.springframework.web.bind.annotation.PathVariable;
3334
import org.springframework.web.bind.annotation.RequestBody;
@@ -55,56 +56,57 @@ public interface ConsulClient {
5556
ResponseEntity<List<String>> getCatalogDatacenters(
5657
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken);
5758

59+
@GetExchange("/v1/catalog/services")
60+
ResponseEntity<Map<String, List<String>>> getCatalogServices();
61+
5862
@GetExchange("/v1/catalog/services")
5963
ResponseEntity<Map<String, List<String>>> getCatalogServices(
6064
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken, QueryParams queryParams);
6165

6266
@PutExchange("/v1/agent/check/fail/{checkId}")
63-
ResponseEntity<Void> agentCheckFail(String checkId, @RequestParam(name = "note", required = false) String note,
67+
ResponseEntity<Void> agentCheckFail(String checkId, @RequestParam(required = false) String note,
6468
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken);
6569

6670
@PutExchange("/v1/agent/check/pass/{checkId}")
67-
ResponseEntity<Void> agentCheckPass(String checkId, @RequestParam(name = "note", required = false) String note,
71+
ResponseEntity<Void> agentCheckPass(String checkId, @RequestParam(required = false) String note,
6872
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken);
6973

7074
@PutExchange("/v1/agent/check/warn/{checkId}")
71-
ResponseEntity<Void> agentCheckWarn(String checkId, @RequestParam(name = "note", required = false) String note,
75+
ResponseEntity<Void> agentCheckWarn(String checkId, @RequestParam(required = false) String note,
7276
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken);
7377

7478
@GetExchange("/v1/agent/services")
7579
ResponseEntity<Map<String, Service>> getAgentServices();
7680

7781
@PutExchange("/v1/agent/service/deregister/{serviceId}")
78-
ResponseEntity<Void> agentServiceDeregister(String serviceId,
82+
ResponseEntity<Void> agentServiceDeregister(@PathVariable String serviceId,
7983
@RequestHeader(name = ACL_TOKEN_HEADER) String aclToken);
8084

8185
@PutExchange("/v1/agent/service/register")
82-
ResponseEntity<Void> agentServiceRegister(@RequestHeader(name = ACL_TOKEN_HEADER) String aclToken,
83-
NewService newService);
86+
ResponseEntity<Void> agentServiceRegister(@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken,
87+
@RequestBody NewService newService);
8488

8589
@PutExchange("/v1/agent/service/maintenance/{serviceId}")
86-
ResponseEntity<Void> agentServiceSetMaintenance(String serviceId,
87-
@RequestParam(name = "maintenanceEnabled", required = false) Boolean maintenanceEnabled,
88-
@RequestParam(name = "reason", required = false) String reason,
90+
ResponseEntity<Void> agentServiceSetMaintenance(@PathVariable String serviceId,
91+
@RequestParam(required = false) Boolean enable, @RequestParam(required = false) String reason,
8992
@RequestHeader(name = ACL_TOKEN_HEADER) String aclToken);
9093

9194
@GetExchange("/v1/catalog/service/{serviceId}")
92-
ResponseEntity<List<CatalogService>> getCatalogService(String serviceId);
95+
ResponseEntity<List<CatalogService>> getCatalogService(@PathVariable String serviceId);
9396

9497
@GetExchange("/v1/catalog/nodes")
9598
ResponseEntity<List<Node>> getCatalogNodes();
9699

97-
@GetExchange("/v1/health/service/{serviceName}")
98-
ResponseEntity<List<HealthService>> getHealthServices(@PathVariable String serviceName);
99-
100100
@GetExchange("/v1/health/checks/{serviceName}")
101101
ResponseEntity<List<Check>> getHealthChecksForService(@PathVariable String serviceName);
102102

103+
@GetExchange("/v1/health/service/{serviceName}")
104+
ResponseEntity<List<HealthService>> getHealthServices(@PathVariable String serviceName);
105+
103106
@GetExchange("/v1/health/service/{serviceName}")
104107
ResponseEntity<List<HealthService>> getHealthServices(@PathVariable String serviceName,
105-
@RequestParam("passing") boolean passing,
106-
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken,
107-
@RequestParam(name = "tags", required = false) List<String> tags, QueryParams queryParams);
108+
@RequestParam boolean passing, @RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken,
109+
@RequestParam(required = false) List<String> tag, QueryParams queryParams);
108110

109111
@DeleteExchange("/v1/kv/{context}")
110112
ResponseEntity<Void> deleteKVValues(@PathVariable String context);
@@ -126,17 +128,17 @@ ResponseEntity<List<GetValue>> getKVValues(@PathVariable String context,
126128
@RequestHeader(name = ACL_TOKEN_HEADER, required = false) String aclToken,
127129
@RequestParam("wait") @WaitTimeFormat Long waitTime, @RequestParam("index") long index);
128130

129-
@PutExchange("/v1/kv/{context}")
130-
ResponseEntity<List<GetValue>> setKVValue(@PathVariable String context, @RequestBody String value);
131+
@PutExchange(url = "/v1/kv/{context}", contentType = MediaType.TEXT_PLAIN_VALUE)
132+
ResponseEntity<Boolean> setKVValue(@PathVariable String context, @RequestBody String value);
131133

132134
@GetExchange("/v1/events")
133135
ResponseEntity<List<Event>> eventList();
134136

135137
@GetExchange("/v1/events")
136138
ResponseEntity<List<Event>> eventList(int eventTimeout, long index);
137139

138-
@PostExchange("/v1/events")
139-
ResponseEntity<Event> eventFire(String name, @RequestBody String payload);
140+
@PostExchange("/v1/event/fire/{name}")
141+
ResponseEntity<Event> eventFire(@PathVariable String name, @RequestBody String payload);
140142

141143
class QueryParams {
142144

‎spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulEndpoint.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public ConsulData invoke() {
4646
Map<String, Service> agentServices = this.consul.getAgentServices().getBody();
4747
data.setAgentServices(agentServices);
4848

49-
Map<String, List<String>> catalogServices = this.consul.getCatalogServices(null, null).getBody();
49+
Map<String, List<String>> catalogServices = this.consul.getCatalogServices().getBody();
5050

5151
for (String serviceId : catalogServices.keySet()) {
5252
List<CatalogService> response = this.consul.getCatalogService(serviceId).getBody();

‎spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ protected void doHealthCheck(Health.Builder builder) {
4242
final String leaderStatus = this.consul.getStatusLeader().getBody();
4343
builder.up().withDetail("leader", leaderStatus);
4444
if (properties.isIncludeServicesQuery()) {
45-
ResponseEntity<Map<String, List<String>>> response = this.consul.getCatalogServices(null, null);
45+
ResponseEntity<Map<String, List<String>>> response = this.consul.getCatalogServices();
4646
final Map<String, List<String>> services = response.getBody();
4747
builder.withDetail("services", services);
4848
}

‎spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/model/http/agent/NewService.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ public String toString() {
140140

141141
public static class Check {
142142

143-
@JsonProperty("Script")
144-
private String script;
143+
@JsonProperty("Args")
144+
private List<String> args;
145145

146146
@JsonProperty("DockerContainerID")
147147
private String dockerContainerID;
@@ -185,12 +185,12 @@ public static class Check {
185185
@JsonProperty("GRPCUseTLS")
186186
private Boolean grpcUseTLS;
187187

188-
public String getScript() {
189-
return script;
188+
public List<String> getArgs() {
189+
return args;
190190
}
191191

192-
public void setScript(String script) {
193-
this.script = script;
192+
public void setArgs(List<String> args) {
193+
this.args = args;
194194
}
195195

196196
public String getDockerContainerID() {
@@ -307,7 +307,7 @@ public void setGrpcUseTLS(Boolean grpcUseTLS) {
307307

308308
@Override
309309
public String toString() {
310-
return new ToStringCreator(this).append("script", script)
310+
return new ToStringCreator(this).append("args", args)
311311
.append("dockerContainerID", dockerContainerID)
312312
.append("shell", shell)
313313
.append("interval", interval)

‎spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulAutoConfigurationTests.java‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222

2323
import org.springframework.boot.actuate.endpoint.annotation.Endpoint;
2424
import org.springframework.boot.autoconfigure.AutoConfigurations;
25+
import org.springframework.boot.ssl.SslBundle;
2526
import org.springframework.boot.test.context.FilteredClassLoader;
2627
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
28+
import org.springframework.cloud.consul.ConsulAutoConfiguration.ConsulClientSettings;
2729
import org.springframework.cloud.consul.test.ConsulTestcontainers;
2830

2931
import static org.assertj.core.api.Assertions.assertThat;
@@ -61,20 +63,13 @@ public void consulDisabled() {
6163
@Test
6264
public void customPathConfigured() {
6365
appContextRunner.withPropertyValues("spring.cloud.consul.path=/consul/proxy/").run(context -> {
64-
assertThat(context).hasNotFailed().hasSingleBean(ConsulClient.class);
65-
66-
ConsulClient consulClient = context.getBean(ConsulClient.class);
67-
// CatalogConsulClient client = (CatalogConsulClient)
68-
// ReflectionTestUtils.getField(consulClient,
69-
// "catalogClient");
70-
// ConsulRawClient rawClient = (ConsulRawClient)
71-
// ReflectionTestUtils.getField(client, "rawClient");
72-
String agentAddress = null; // (String)
73-
// ReflectionTestUtils.getField(rawClient,
74-
// "agentAddress");
75-
76-
assertThat(agentAddress).isNotNull();
77-
assertThat(new URL(agentAddress).getPath()).isEqualTo("/consul/proxy");
66+
assertThat(context).hasNotFailed()
67+
.hasSingleBean(ConsulClient.class)
68+
.hasSingleBean(ConsulClientSettings.class);
69+
70+
ConsulClientSettings settings = context.getBean(ConsulClientSettings.class);
71+
assertThat(settings).isNotNull();
72+
assertThat(new URL(settings.baseUrl()).getPath()).isEqualTo("/consul/proxy");
7873
});
7974
}
8075

@@ -87,18 +82,18 @@ public void tlsConfigured() {
8782
"spring.cloud.consul.tls.certificate-path=src/test/resources/trustStore.jks",
8883
"spring.cloud.consul.tls.certificate-password=change_me")
8984
.run(context -> {
90-
assertThat(context).hasNotFailed().hasSingleBean(ConsulClient.class);
91-
92-
ConsulClient consulClient = context.getBean(ConsulClient.class);
93-
// CatalogConsulClient client = (CatalogConsulClient)
94-
// ReflectionTestUtils.getField(consulClient,
95-
// "catalogClient");
96-
// ConsulRawClient rawClient = (ConsulRawClient)
97-
// ReflectionTestUtils.getField(client, "rawClient");
98-
// HttpTransport httpTransport = (HttpTransport)
99-
// ReflectionTestUtils.getField(rawClient, "httpTransport");
100-
// assertThat(httpTransport).isInstanceOf(DefaultHttpsTransport.class);
101-
assertThat(consulClient).isNull();
85+
assertThat(context).hasNotFailed()
86+
.hasSingleBean(ConsulClient.class)
87+
.hasSingleBean(ConsulClientSettings.class);
88+
89+
ConsulClientSettings settings = context.getBean(ConsulClientSettings.class);
90+
assertThat(settings).isNotNull();
91+
assertThat(settings.httpClientSettings()).isNotNull();
92+
SslBundle sslBundle = settings.httpClientSettings().sslBundle();
93+
assertThat(sslBundle).isNotNull();
94+
assertThat(sslBundle.getStores().getKeyStore()).isNotNull();
95+
assertThat(sslBundle.getStores().getKeyStorePassword()).isEqualTo("letmein");
96+
assertThat(sslBundle.getStores().getTrustStore()).isNotNull();
10297
});
10398
}
10499

‎spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorDownTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ public void statusIsDownWhenConsulClientFailsToGetLeaderStatus() {
5656
public void statusIsDownWhenConsulClientFailsToGetServices() {
5757
String leaderStatus = "OK";
5858
when(consulClient.getStatusLeader()).thenReturn(ResponseEntity.ok(leaderStatus));
59-
when(consulClient.getCatalogServices(null, null)).thenThrow(new RuntimeException("no services"));
59+
when(consulClient.getCatalogServices()).thenThrow(new RuntimeException("no services"));
6060
assertThat(this.healthEndpoint.health().getStatus()).as("health status was not DOWN").isEqualTo(Status.DOWN);
61-
verify(consulClient).getCatalogServices(null, null);
61+
verify(consulClient).getCatalogServices();
6262
}
6363

6464
@EnableAutoConfiguration

‎spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorLightweightUpTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public class ConsulHealthIndicatorLightweightUpTest {
5555
public void statusIsUp() {
5656
assertThat(this.healthEndpoint.health().getStatus()).as("health status was not UP").isEqualTo(Status.UP);
5757
verify(consulClient).getStatusLeader();
58-
verify(consulClient, never()).getCatalogServices(null, null);
58+
verify(consulClient, never()).getCatalogServices();
5959
}
6060

6161
@EnableAutoConfiguration

0 commit comments

Comments
 (0)