fix: Ecwid#186 token as request param is deprecated

Consul now warns aggressively when token is provided as a request param and the X-Consul-Token is required. Also as already mentioned in Ecwid#186 token request param will be removed in Consul v1.17. By reading the code and related PRs I've seen that in the so called "new http architecture" this is addressed although not all clients fully utilize the new Request class. This PR uses the Request class for both Catalog and KV clients. Should address both Ecwid#186 and Ecwid#237.
lucwillems · Mar 26, 2024 · 65cb928 · 65cb928
1 parent 232550b
commit 65cb928
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 25 deletions.
diff --git a/src/main/java/com/ecwid/consul/v1/ConsulRawClient.java b/src/main/java/com/ecwid/consul/v1/ConsulRawClient.java
@@ -170,6 +170,7 @@ public HttpResponse makePutRequest(Request request) {
 
 		HttpRequest httpRequest = HttpRequest.Builder.newBuilder()
 			.setUrl(url)
+			.setContent(request.getContent()) // if content is not null then it has priority over binaryContent
 			.setBinaryContent(request.getBinaryContent())
 			.addHeaders(Utils.createTokenMap(request.getToken()))
 			.build();

diff --git a/src/main/java/com/ecwid/consul/v1/catalog/CatalogConsulClient.java b/src/main/java/com/ecwid/consul/v1/catalog/CatalogConsulClient.java
@@ -140,7 +140,13 @@ public Response<Map<String, List<String>>> getCatalogServices(QueryParams queryP
 
 	@Override
 	public Response<Map<String, List<String>>> getCatalogServices(CatalogServicesRequest catalogServicesRequest) {
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/catalog/services", catalogServicesRequest.asUrlParameters());
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/catalog/services")
+			.addUrlParameters(catalogServicesRequest.asUrlParameters())
+			.setToken(catalogServicesRequest.getToken())
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			Map<String, List<String>> value = GsonFactory.getGson().fromJson(httpResponse.getContent(),
@@ -188,7 +194,13 @@ public Response<List<com.ecwid.consul.v1.catalog.model.CatalogService>> getCatal
 
 	@Override
 	public Response<List<CatalogService>> getCatalogService(String serviceName, CatalogServiceRequest catalogServiceRequest) {
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/catalog/service/" + serviceName, catalogServiceRequest.asUrlParameters());
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/catalog/service/" + serviceName)
+			.addUrlParameters(catalogServiceRequest.asUrlParameters())
+			.setToken(catalogServiceRequest.getToken())
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<com.ecwid.consul.v1.catalog.model.CatalogService> value = GsonFactory.getGson().fromJson(httpResponse.getContent(),

diff --git a/src/main/java/com/ecwid/consul/v1/catalog/CatalogServiceRequest.java b/src/main/java/com/ecwid/consul/v1/catalog/CatalogServiceRequest.java
@@ -144,10 +144,6 @@ public List<UrlParameters> asUrlParameters() {
 			params.add(queryParams);
 		}
 
-		if (token != null) {
-			params.add(new SingleUrlParameters("token", token));
-		}
-
 		return params;
 	}
 

diff --git a/src/main/java/com/ecwid/consul/v1/catalog/CatalogServicesRequest.java b/src/main/java/com/ecwid/consul/v1/catalog/CatalogServicesRequest.java
@@ -101,10 +101,6 @@ public List<UrlParameters> asUrlParameters() {
 			params.add(queryParams);
 		}
 
-		if (token != null) {
-			params.add(new SingleUrlParameters("token", token));
-		}
-
 		return params;
 	}
 

diff --git a/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java b/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java
@@ -1,7 +1,5 @@
 package com.ecwid.consul.v1.kv;
 
-import java.util.List;
-
 import com.ecwid.consul.ConsulException;
 import com.ecwid.consul.SingleUrlParameters;
 import com.ecwid.consul.UrlParameters;
@@ -14,6 +12,8 @@
 import com.ecwid.consul.v1.kv.model.PutParams;
 import com.google.gson.reflect.TypeToken;
 
+import java.util.List;
+
 /**
  * @author Vasily Vasilkov ([email protected])
  */
@@ -66,8 +66,12 @@ public Response<GetValue> getKVValue(String key, QueryParams queryParams) {
 
 	@Override
 	public Response<GetValue> getKVValue(String key, String token, QueryParams queryParams) {
-		UrlParameters tokenParams = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/kv/" + key, tokenParams, queryParams);
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + key)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<GetValue> value = GsonFactory.getGson().fromJson(httpResponse.getContent(), new TypeToken<List<GetValue>>() {
@@ -104,8 +108,13 @@ public Response<GetBinaryValue> getKVBinaryValue(String key, QueryParams queryPa
 
 	@Override
 	public Response<GetBinaryValue> getKVBinaryValue(String key, String token, QueryParams queryParams) {
-		UrlParameters tokenParams = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/kv/" + key, tokenParams, queryParams);
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + key)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<GetBinaryValue> value = GsonFactory.getGson().fromJson(httpResponse.getContent(), new TypeToken<List<GetBinaryValue>>() {
@@ -143,8 +152,15 @@ public Response<List<GetValue>> getKVValues(String keyPrefix, QueryParams queryP
 	@Override
 	public Response<List<GetValue>> getKVValues(String keyPrefix, String token, QueryParams queryParams) {
 		UrlParameters recurseParam = new SingleUrlParameters("recurse");
-		UrlParameters tokenParam = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/kv/" + keyPrefix, recurseParam, tokenParam, queryParams);
+
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + keyPrefix)
+			.addUrlParameter(recurseParam)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<GetValue> value = GsonFactory.getGson().fromJson(httpResponse.getContent(), new TypeToken<List<GetValue>>() {
@@ -175,8 +191,15 @@ public Response<List<GetBinaryValue>> getKVBinaryValues(String keyPrefix, QueryP
 	@Override
 	public Response<List<GetBinaryValue>> getKVBinaryValues(String keyPrefix, String token, QueryParams queryParams) {
 		UrlParameters recurseParam = new SingleUrlParameters("recurse");
-		UrlParameters tokenParam = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/kv/" + keyPrefix, recurseParam, tokenParam, queryParams);
+
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + keyPrefix)
+			.addUrlParameter(recurseParam)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<GetBinaryValue> value = GsonFactory.getGson().fromJson(httpResponse.getContent(), new TypeToken<List<GetBinaryValue>>() {
@@ -208,8 +231,16 @@ public Response<List<String>> getKVKeysOnly(String keyPrefix, QueryParams queryP
 	public Response<List<String>> getKVKeysOnly(String keyPrefix, String separator, String token, QueryParams queryParams) {
 		UrlParameters keysParam = new SingleUrlParameters("keys");
 		UrlParameters separatorParam = separator != null ? new SingleUrlParameters("separator", separator) : null;
-		UrlParameters tokenParam = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makeGetRequest("/v1/kv/" + keyPrefix, keysParam, separatorParam, tokenParam, queryParams);
+
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + keyPrefix)
+			.addUrlParameter(keysParam)
+			.addUrlParameter(separatorParam)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+
+		HttpResponse httpResponse = rawClient.makeGetRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			List<String> value = GsonFactory.getGson().fromJson(httpResponse.getContent(), new TypeToken<List<String>>() {
@@ -250,7 +281,16 @@ public Response<Boolean> setKVValue(String key, String value, PutParams putParam
 	@Override
 	public Response<Boolean> setKVValue(String key, String value, String token, PutParams putParams, QueryParams queryParams) {
 		UrlParameters tokenParam = token != null ? new SingleUrlParameters("token", token) : null;
-		HttpResponse httpResponse = rawClient.makePutRequest("/v1/kv/" + key, value, putParams, tokenParam, queryParams);
+
+		Request request = Request.Builder.newBuilder()
+			.setEndpoint("/v1/kv/" + key)
+			.setContent(value)
+			.addUrlParameter(putParams)
+			.addUrlParameter(queryParams)
+			.setToken(token)
+			.build();
+
+		HttpResponse httpResponse = rawClient.makePutRequest(request);
 
 		if (httpResponse.getStatusCode() == 200) {
 			boolean result = GsonFactory.getGson().fromJson(httpResponse.getContent(), boolean.class);

diff --git a/src/test/java/com/ecwid/consul/v1/catalog/CatalogConsulClientTest.java b/src/test/java/com/ecwid/consul/v1/catalog/CatalogConsulClientTest.java
@@ -3,7 +3,6 @@
 import com.ecwid.consul.ConsulTestConstants;
 import com.ecwid.consul.v1.Response;
 import com.ecwid.consul.v1.catalog.model.Node;
-import com.ecwid.consul.v1.kv.KeyValueConsulClient;
 import com.pszymczyk.consul.ConsulProcess;
 import com.pszymczyk.consul.ConsulStarterBuilder;
 import com.pszymczyk.consul.infrastructure.Ports;
@@ -12,9 +11,10 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
+import java.util.Map;
 import java.util.Random;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 class CatalogConsulClientTest {
 
@@ -48,5 +48,14 @@ void testGetCatalogNodes() {
 		assertEquals(1, response.getValue().size());
 	}
 
+	@Test
+	void testGetCatalogServices() {
+		CatalogServicesRequest request = CatalogServicesRequest.newBuilder().build();
+		Response<Map<String, List<String>>> response = consulClient.getCatalogServices(request);
+
+		// We should find only one node – this
+		assertEquals(1, response.getValue().size());
+	}
+
 
 }