From 5c1ad6403e12a97a3c6ad8945bd02bd48445b88d Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Wed, 3 Aug 2022 16:31:00 +0530 Subject: [PATCH] fix: Elasticsearch DB Head request fails on some ES instances when used on /test API (#15332) * This commit changes two things: * Pom.xml --> ES- Restclient version to 7.17.5 from 7.9.2 * ElasticSearchPlugin.java ---> changed the testDatasource method to expect a 200 instead of a 200 or 404 HTTP status code This commit fixes a high-priority bug with github issue https://github.com/appsmithorg/appsmith/issues/14909 * it can now negotiate around a 403 forbidden error as it does try to send head only to user provided URI This commit has been tested: * manually for CRUD at local with ES version 7.9.2 * manually for CR at user provide aws instance running 7.9.3 on docker * | | BugFix: | | This commit changes two files: | | * ElasticSearchPlugin.java --> added feature to distinguish between unauthorized and not found datasources. | * ElasticSearchPluginTest.java ---> added testcases to verify the unauthorized and not found test cases | | This commit adds features on top of previous commit https://github.com/appsmithorg/appsmith/issues/14909 | * this commit adds the feature of more readable error messages while testing the elasticsearch datasource. i.e wrong endpoint errors and unauthorized issues | | This commit has been tested: | * Junit | * manually * removed leftover comments from the parent commit and updated response texts for better readability * removed wildcard imports from appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java --- .../elasticSearchPlugin/pom.xml | 2 +- .../external/plugins/ElasticSearchPlugin.java | 40 ++++++++- .../plugins/ElasticSearchPluginTest.java | 90 ++++++++++++++++--- 3 files changed, 117 insertions(+), 15 deletions(-) diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/pom.xml b/app/server/appsmith-plugins/elasticSearchPlugin/pom.xml index 05d637a03a..ec752dfe01 100644 --- a/app/server/appsmith-plugins/elasticSearchPlugin/pom.xml +++ b/app/server/appsmith-plugins/elasticSearchPlugin/pom.xml @@ -32,7 +32,7 @@ org.elasticsearch.client elasticsearch-rest-client - 7.9.2 + 7.17.5 diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java index 06ccc82594..c1226ac650 100644 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/com/external/plugins/ElasticSearchPlugin.java @@ -23,6 +23,7 @@ import org.apache.http.entity.ContentType; import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.http.message.BasicHeader; import org.apache.http.nio.entity.NStringEntity; +import org.eclipse.jgit.util.SystemReader; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -45,6 +46,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATION_BODY; import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATION_PATH; @@ -61,6 +63,15 @@ public class ElasticSearchPlugin extends BasePlugin { private final Scheduler scheduler = Schedulers.elastic(); + public static final String esDatasourceNotFoundMessage = "The Page you are tyring to access does not exist"; + + public static final String esDatasourceUnauthorizedMessage = "Your Username or Password is not correct"; + + public static final String esDatasourceUnauthorizedPattern = ".*unauthorized.*"; + + public static final String esDatasourceNotFoundPattern = ".*(?:not.?found)|(?:refused)|(?:not.?known)|(?:timed?\\s?out).*"; + + @Override public Mono execute(RestClient client, DatasourceConfiguration datasourceConfiguration, @@ -251,16 +262,32 @@ public class ElasticSearchPlugin extends BasePlugin { if (client == null) { return new DatasourceTestResult("Null client object to ElasticSearch."); } - - // This HEAD request is to check if an index exists. It response with 200 if the index exists, + // This HEAD request is to check if the base of datasource exists. It responds with 200 if the index exists, // 404 if it doesn't. We just check for either of these two. // Ref: https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-exists.html - Request request = new Request("HEAD", "/potentially-missing-index?local=true"); + Request request = new Request("HEAD", "/"); final Response response; try { response = client.performRequest(request); } catch (IOException e) { + + /* since the 401, and 403 are registered as IOException, but for the given connection it + * in the current rest-client. We will figure out with matching patterns with regexes. + */ + + Pattern patternForUnauthorized = Pattern.compile(esDatasourceUnauthorizedPattern, Pattern.CASE_INSENSITIVE); + Pattern patterForNotFound = Pattern.compile(esDatasourceNotFoundPattern,Pattern.CASE_INSENSITIVE); + + if (patternForUnauthorized.matcher(e.getMessage()).find()){ + return new DatasourceTestResult(esDatasourceUnauthorizedMessage); + } + + if (patterForNotFound.matcher(e.getMessage()).find()){ + return new DatasourceTestResult(esDatasourceNotFoundMessage); + } + + return new DatasourceTestResult("Error running HEAD request: " + e.getMessage()); } @@ -271,8 +298,13 @@ public class ElasticSearchPlugin extends BasePlugin { } catch (IOException e) { log.warn("Error closing ElasticSearch client that was made for testing.", e); } + // earlier it was 404 and 200, now it has been changed to just expect 200 status code + // here it checks if it is anything else than 200, even 404 is not allowed! + if (statusLine.getStatusCode() == 404){ + return new DatasourceTestResult(esDatasourceNotFoundMessage); + } - if (statusLine.getStatusCode() != 404 && statusLine.getStatusCode() != 200) { + if (statusLine.getStatusCode() != 200) { return new DatasourceTestResult( "Unexpected response from ElasticSearch: " + statusLine); } diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java index a2dadc3552..f25f711232 100755 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java @@ -1,21 +1,28 @@ package com.external.plugins; -import com.appsmith.external.models.ActionConfiguration; -import com.appsmith.external.models.ActionExecutionResult; +import com.appsmith.external.constants.Authentication; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Endpoint; +import com.appsmith.external.models.DBAuth; +import com.appsmith.external.models.ActionExecutionResult; +import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.RequestParamDTO; import lombok.extern.slf4j.Slf4j; import org.apache.http.HttpHost; +import org.apache.http.auth.AuthScope; +import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.client.CredentialsProvider; +import org.apache.http.impl.client.BasicCredentialsProvider; +import org.apache.http.impl.nio.client.HttpAsyncClientBuilder; import org.elasticsearch.client.Request; import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.RestClientBuilder; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.springframework.http.HttpMethod; import org.testcontainers.elasticsearch.ElasticsearchContainer; -import org.testcontainers.utility.DockerImageName; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -39,20 +46,37 @@ public class ElasticSearchPluginTest { @ClassRule public static final ElasticsearchContainer container = new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:7.12.1") - .withEnv("discovery.type", "single-node"); - + .withEnv("discovery.type", "single-node") + .withPassword("esPassword"); + private static String username ="elastic"; + private static String password = "esPassword"; private static final DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + private static DBAuth elasticInstanceCredentials = new DBAuth(DBAuth.Type.USERNAME_PASSWORD,username,password, null); private static String host; private static Integer port; + + @BeforeClass public static void setUp() throws IOException { port = container.getMappedPort(9200); host = "http://" + container.getContainerIpAddress(); - final RestClient client = RestClient.builder( - new HttpHost(container.getContainerIpAddress(), port, "http") - ).build(); + final CredentialsProvider credentialsProvider = + new BasicCredentialsProvider(); + credentialsProvider.setCredentials(AuthScope.ANY, + new UsernamePasswordCredentials(username,password)); + + RestClient client = RestClient.builder( + new HttpHost(container.getContainerIpAddress(),port,"http")) + .setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() { + @Override + public HttpAsyncClientBuilder customizeHttpClient( + HttpAsyncClientBuilder httpClientBuilder) { + return httpClientBuilder + .setDefaultCredentialsProvider(credentialsProvider); + } + }).build(); Request request; @@ -69,8 +93,11 @@ public class ElasticSearchPluginTest { client.performRequest(request); client.close(); - + elasticInstanceCredentials.setAuthenticationType(Authentication.BASIC); + elasticInstanceCredentials.setUsername(username); + elasticInstanceCredentials.setPassword(password); dsConfig.setEndpoints(List.of(new Endpoint(host, port.longValue()))); + dsConfig.setAuthentication(elasticInstanceCredentials); } private Mono execute(HttpMethod method, String path, String body) { @@ -230,6 +257,7 @@ public class ElasticSearchPluginTest { @Test public void itShouldValidateDatasourceWithNoEndpoints() { DatasourceConfiguration invalidDatasourceConfiguration = new DatasourceConfiguration(); + invalidDatasourceConfiguration.setAuthentication(elasticInstanceCredentials); Assert.assertEquals(Set.of("No endpoint provided. Please provide a host:port where ElasticSearch is reachable."), pluginExecutor.validateDatasource(invalidDatasourceConfiguration)); @@ -238,6 +266,7 @@ public class ElasticSearchPluginTest { @Test public void itShouldValidateDatasourceWithEmptyPort() { DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); Endpoint endpoint = new Endpoint(); endpoint.setHost(host); datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); @@ -249,6 +278,7 @@ public class ElasticSearchPluginTest { @Test public void itShouldValidateDatasourceWithEmptyHost() { DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); Endpoint endpoint = new Endpoint(); endpoint.setPort(Long.valueOf(port)); datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); @@ -260,7 +290,7 @@ public class ElasticSearchPluginTest { @Test public void itShouldValidateDatasourceWithMissingEndpoint() { DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); Endpoint endpoint = new Endpoint(); datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); @@ -272,6 +302,7 @@ public class ElasticSearchPluginTest { public void itShouldValidateDatasourceWithEndpointNoProtocol() { DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); Endpoint endpoint = new Endpoint(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); endpoint.setHost("localhost"); endpoint.setPort(Long.valueOf(port)); datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); @@ -284,6 +315,7 @@ public class ElasticSearchPluginTest { @Test public void itShouldTestDatasourceWithInvalidEndpoint() { DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); Endpoint endpoint = new Endpoint(); endpoint.setHost("localhost"); endpoint.setPort(Long.valueOf(port)); @@ -304,4 +336,42 @@ public class ElasticSearchPluginTest { }) .verifyComplete(); } + + @Test + public void shouldVerifyUnauthorized() { + final Integer secureHostPort = container.getMappedPort(9200); + final String secureHostEndpoint = "http://" + container.getHttpHostAddress(); + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + Endpoint endpoint = new Endpoint(secureHostEndpoint,Long.valueOf(secureHostPort)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration) + .map(result -> { + return (Set) result.getInvalids(); + })) + .expectNext(Set.of(ElasticSearchPlugin.ElasticSearchPluginExecutor.esDatasourceUnauthorizedMessage)) + .verifyComplete(); + + } + + + @Test + public void shouldVerifyNotFound() { + final Integer secureHostPort = container.getMappedPort(9200); + final String secureHostEndpoint = "http://esdatabasenotfound.co" ; + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + Endpoint endpoint = new Endpoint(secureHostEndpoint,Long.valueOf(secureHostPort)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration) + .map(result -> { + return (Set) result.getInvalids(); + })) + .expectNext(Set.of(ElasticSearchPlugin.ElasticSearchPluginExecutor.esDatasourceNotFoundMessage)) + .verifyComplete(); + + } + + }