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
This commit is contained in:
parent
787ec15b0c
commit
5c1ad6403e
|
|
@ -32,7 +32,7 @@
|
|||
<dependency>
|
||||
<groupId>org.elasticsearch.client</groupId>
|
||||
<artifactId>elasticsearch-rest-client</artifactId>
|
||||
<version>7.9.2</version>
|
||||
<version>7.17.5</version>
|
||||
</dependency>
|
||||
|
||||
<!-- Test Dependencies -->
|
||||
|
|
|
|||
|
|
@ -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<ActionExecutionResult> 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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ActionExecutionResult> 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<String>) 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<String>) result.getInvalids();
|
||||
}))
|
||||
.expectNext(Set.of(ElasticSearchPlugin.ElasticSearchPluginExecutor.esDatasourceNotFoundMessage))
|
||||
.verifyComplete();
|
||||
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user