From 2d907764c8beef80cf7112e4fffa097ded202630 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Fri, 7 Apr 2023 16:10:48 -0700 Subject: [PATCH 01/17] feat: PPL parser for ccs Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 6 +++- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 30 +++++++++++++++++++ .../sql/ppl/parser/AstBuilderTest.java | 7 +++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index cca99407bb..becbee5b5b 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -322,7 +322,7 @@ multiFieldRelevanceFunction /** tables */ tableSource - : qualifiedName + : clusterQualifiedName | ID_DATE_SUFFIX ; @@ -723,6 +723,10 @@ qualifiedName : ident (DOT ident)* #identsAsQualifiedName ; +clusterQualifiedName + : (ident COLON)? ident (DOT ident)* + ; + wcQualifiedName : wildcard (DOT wildcard)* #identsAsWildcardQualifiedName ; diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index dcf961dc24..d59e09125e 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -30,12 +30,42 @@ public void testSearchCommandIgnoreSearchKeywordShouldPass() { assertNotEquals(null, tree); } + @Test + public void testSearchCommandWithMultipleIndicesShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=t,u a=1 b=2"); + assertNotEquals(null, tree); + } + + @Test + public void testSearchCommandCrossClusterShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:t a=1 b=2"); + assertNotEquals(null, tree); + } + + @Test + public void testSearchCommandCrossClusterWithMultipleIndicesShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:t,d:u,v a=1 b=2"); + assertNotEquals(null, tree); + } + + @Test + public void testSearchCommandCrossClusterIgnoreSearchKeywordShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("source=c:t a=1 b=2"); + assertNotEquals(null, tree); + } + @Test public void testSearchFieldsCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("search source=t a=1 b=2 | fields a,b"); assertNotEquals(null, tree); } + @Test + public void testSearchFieldsCommandCrossClusterShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:t a=1 b=2 | fields a,b"); + assertNotEquals(null, tree); + } + @Test public void testSearchCommandWithoutSourceShouldFail() { exceptionRule.expect(RuntimeException.class); diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index 533254a599..03c187c6fb 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -77,6 +77,13 @@ public void testSearchCommand() { ); } + @Test + public void testSearchCrossClusterCommand() { + assertEqual("search source=c:t", + relation(qualifiedName("c:t")) + ); + } + @Test public void testPrometheusSearchCommand() { assertEqual("search source = prometheus.http_requests_total", From 24547d85e72dc185c344e1cf12050d90dbf6d084 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Fri, 7 Apr 2023 16:14:02 -0700 Subject: [PATCH 02/17] feat: disable describe remote cluster index in PPL Allowing the syntax will lead to misunderstanding in the query result, because we will do a local cluster query for index mapping, even for remote indices. This is due to the restriction that OpenSearch doesn't support remote cluster index mapping query at the moment. Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 13 +++++++++++-- .../org/opensearch/sql/ppl/parser/AstBuilder.java | 10 +++++++++- .../sql/ppl/parser/AstExpressionBuilder.java | 10 ++++++++++ .../sql/ppl/antlr/PPLSyntaxParserTest.java | 9 +++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index becbee5b5b..f63b3d3fc3 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -181,14 +181,18 @@ mlArg /** clauses */ fromClause - : SOURCE EQUAL tableSourceClause - | INDEX EQUAL tableSourceClause + : SOURCE EQUAL clusterTableSourceClause + | INDEX EQUAL clusterTableSourceClause ; tableSourceClause : tableSource (COMMA tableSource)* ; +clusterTableSourceClause + : clusterTableSource (COMMA clusterTableSource)* + ; + renameClasue : orignalField=wcFieldExpression AS renamedField=wcFieldExpression ; @@ -322,6 +326,11 @@ multiFieldRelevanceFunction /** tables */ tableSource + : qualifiedName + | ID_DATE_SUFFIX + ; + +clusterTableSource : clusterQualifiedName | ID_DATE_SUFFIX ; diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index 73c7238624..fd0972cb03 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -7,6 +7,7 @@ package org.opensearch.sql.ppl.parser; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; +import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ClusterTableSourceClauseContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DedupCommandContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DescribeCommandContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EvalCommandContext; @@ -346,7 +347,7 @@ public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { */ @Override public UnresolvedPlan visitFromClause(FromClauseContext ctx) { - return visitTableSourceClause(ctx.tableSourceClause()); + return visitClusterTableSourceClause(ctx.clusterTableSourceClause()); } @Override @@ -356,6 +357,13 @@ public UnresolvedPlan visitTableSourceClause(TableSourceClauseContext ctx) { .collect(Collectors.toList())); } + @Override + public UnresolvedPlan visitClusterTableSourceClause(ClusterTableSourceClauseContext ctx) { + return new Relation(ctx.clusterTableSource() + .stream().map(this::internalVisitExpression) + .collect(Collectors.toList())); + } + @Override @Generated //To exclude from jacoco..will remove https://github.com/opensearch-project/sql/issues/1019 public UnresolvedPlan visitTableFunction(OpenSearchPPLParser.TableFunctionContext ctx) { diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index c9823b67f9..abdb9a71e5 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -14,6 +14,7 @@ import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanLiteralContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BySpanClauseContext; +import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ClusterTableSourceContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.CompareExprContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ConvertedDataTypeContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.CountAllFunctionCallContext; @@ -287,6 +288,15 @@ public UnresolvedExpression visitTableSource(TableSourceContext ctx) { } } + @Override + public UnresolvedExpression visitClusterTableSource(ClusterTableSourceContext ctx) { + if (ctx.getChild(0) instanceof IdentsAsQualifiedNameContext) { + return visitIdentifiers(((IdentsAsQualifiedNameContext) ctx.getChild(0)).ident()); + } else { + return visitIdentifiers(Arrays.asList(ctx)); + } + } + @Override public UnresolvedExpression visitPositionFunction( OpenSearchPPLParser.PositionFunctionContext ctx) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index d59e09125e..8e16d24de4 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -207,6 +207,7 @@ public void can_parse_query_string_relevance_function() { + "analyzer=keyword, quote_field_suffix=\".exact\", fuzzy_prefix_length = 4)")); } + @Test public void testDescribeCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("describe t"); assertNotEquals(null, tree); @@ -218,6 +219,14 @@ public void testDescribeCommandWithMultipleIndicesShouldPass() { assertNotEquals(null, tree); } + @Test + public void testDescribeCommandCrossClusterShouldFail() { + exceptionRule.expect(RuntimeException.class); + exceptionRule.expectMessage("Failed to parse query due to offending symbol"); + + new PPLSyntaxParser().parse("describe c:t"); + } + @Test public void testDescribeFieldsCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("describe t | fields a,b"); From 4b1c00a32fc2637f778b461dd25a3ecee0d0133f Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Fri, 7 Apr 2023 16:19:23 -0700 Subject: [PATCH 03/17] feat: Query system index without cluster name We require system index query to happen at the local cluster. Currently, OpenSearch does not support cross cluster system index query. Thus, mapping of a remote index is unavailable. Therefore, we require the local cluster to have the system index of the remote cluster index. The full "cluster:index" name is still used to query OpenSearch for datarows, as CCS is natively supported. Signed-off-by: Sean Kao --- .../request/OpenSearchQueryRequest.java | 2 +- .../opensearch/request/OpenSearchRequest.java | 19 +++++++++++++++++-- .../request/OpenSearchScrollRequest.java | 3 +-- .../request/OpenSearchQueryRequestTest.java | 18 ++++++++++++++++++ .../OpenSearchDescribeIndexRequestTest.java | 16 ++++++++++++++++ 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java index 3976f854fd..8469377f97 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java @@ -123,7 +123,7 @@ public void clean(Consumer cleanAction) { @VisibleForTesting protected SearchRequest searchRequest() { return new SearchRequest() - .indices(indexName.getIndexNames()) + .indices(indexName.getIndexFullNames()) .source(sourceBuilder); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java index ce990780c1..23f33109e9 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java @@ -6,6 +6,7 @@ package org.opensearch.sql.opensearch.request; +import java.util.Arrays; import java.util.function.Consumer; import java.util.function.Function; import lombok.EqualsAndHashCode; @@ -57,11 +58,25 @@ OpenSearchResponse search(Function searchAction, @EqualsAndHashCode class IndexName { private static final String COMMA = ","; + private static final String COLON = ":"; + private final String[] indexFullNames; private final String[] indexNames; + /** + * Constructor. + * indexNames are indexFullNames without the "{cluster}:" prefix. + */ public IndexName(String indexName) { - this.indexNames = indexName.split(COMMA); + this.indexFullNames = indexName.split(COMMA); + // Remove all ":" prefix if they exist + this.indexNames = Arrays.stream(indexFullNames) + .map(name -> name.substring(name.indexOf(COLON) + 1)) + .toArray(String[]::new); + } + + public String[] getIndexFullNames() { + return indexFullNames; } public String[] getIndexNames() { @@ -70,7 +85,7 @@ public String[] getIndexNames() { @Override public String toString() { - return String.join(COMMA, indexNames); + return String.join(COMMA, indexFullNames); } } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java index dacbecc7b9..2b340623fc 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java @@ -13,7 +13,6 @@ import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; -import lombok.RequiredArgsConstructor; import lombok.Setter; import lombok.ToString; import org.opensearch.action.search.SearchRequest; @@ -115,7 +114,7 @@ public void clean(Consumer cleanAction) { */ public SearchRequest searchRequest() { return new SearchRequest() - .indices(indexName.getIndexNames()) + .indices(indexName.getIndexFullNames()) .scroll(DEFAULT_SCROLL_TIMEOUT) .source(sourceBuilder); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java index be83622578..2e1ded6322 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java @@ -67,6 +67,9 @@ public class OpenSearchQueryRequestTest { private final OpenSearchQueryRequest request = new OpenSearchQueryRequest("test", 200, factory); + private final OpenSearchQueryRequest remoteRequest = + new OpenSearchQueryRequest("ccs:test", 200, factory); + @Test void search() { OpenSearchQueryRequest request = new OpenSearchQueryRequest( @@ -152,4 +155,19 @@ void searchRequest() { .query(QueryBuilders.termQuery("name", "John"))), request.searchRequest()); } + + @Test + void searchCrossClusterRequest() { + remoteRequest.getSourceBuilder().query(QueryBuilders.termQuery("name", "John")); + + assertEquals( + new SearchRequest() + .indices("ccs:test") + .source(new SearchSourceBuilder() + .timeout(OpenSearchQueryRequest.DEFAULT_QUERY_TIMEOUT) + .from(0) + .size(200) + .query(QueryBuilders.termQuery("name", "John"))), + remoteRequest.searchRequest()); + } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequestTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequestTest.java index 111316a7ed..c19b3a3ccd 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequestTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequestTest.java @@ -49,6 +49,22 @@ void testSearch() { )); } + @Test + void testCrossClusterShouldSearchLocal() { + when(mapping.getFieldMappings()).thenReturn( + Map.of("name", OpenSearchDataType.of(OpenSearchDataType.MappingType.Keyword))); + when(client.getIndexMappings("index")).thenReturn(ImmutableMap.of("test", mapping)); + + final List results = + new OpenSearchDescribeIndexRequest(client, "ccs:index").search(); + assertEquals(1, results.size()); + assertThat(results.get(0).tupleValue(), anyOf( + hasEntry("TABLE_NAME", stringValue("index")), + hasEntry("COLUMN_NAME", stringValue("name")), + hasEntry("TYPE_NAME", stringValue("STRING")) + )); + } + @Test void testToString() { assertEquals("OpenSearchDescribeIndexRequest{indexName='index'}", From c918bf7cdaaef1e08ccaf1c9840e4678c05e2567 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 10 Apr 2023 14:44:16 -0700 Subject: [PATCH 04/17] fix: index name parsing for datasources To identify datasources in the index qualified names, they need to be parsed into parts (separated only by dots). clusterQualifiedName can't contain custom datasources, hence the distinction. Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index f63b3d3fc3..40c47d9fe6 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -331,7 +331,8 @@ tableSource ; clusterTableSource - : clusterQualifiedName + : qualifiedName + | clusterQualifiedName | ID_DATE_SUFFIX ; @@ -733,7 +734,7 @@ qualifiedName ; clusterQualifiedName - : (ident COLON)? ident (DOT ident)* + : ident COLON ident (DOT ident)* ; wcQualifiedName From 6c547e3254dd8d7940e64fa41023b84fdfe1ca82 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Tue, 18 Apr 2023 15:08:41 -0700 Subject: [PATCH 05/17] multi clusters setup for integration test Signed-off-by: Sean Kao --- integ-test/build.gradle | 25 ++++ .../sql/legacy/OpenSearchSQLRestTestCase.java | 15 ++- .../sql/legacy/SQLIntegTestCase.java | 33 +++-- .../OpenSearchMultiClustersRestTestCase.java | 126 ++++++++++++++++++ .../sql/ppl/CrossClusterSearchIT.java | 50 +++++++ 5 files changed, 237 insertions(+), 12 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java create mode 100644 integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 0a30e057ad..695e007f44 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -26,6 +26,7 @@ import org.opensearch.gradle.test.RestIntegTestTask import org.opensearch.gradle.testclusters.StandaloneRestIntegTestTask import java.util.concurrent.Callable +import java.util.stream.Collectors plugins { id "de.undercouch.download" version "5.3.0" @@ -131,6 +132,12 @@ testClusters.integTest { plugin ":opensearch-sql-plugin" } +testClusters { + remoteCluster { + plugin ":opensearch-sql-plugin" + } +} + task startPrometheus(type: SpawnProcessTask) { mustRunAfter ':doctest:doctest' @@ -209,9 +216,27 @@ task integJdbcTest(type: RestIntegTestTask) { // Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled integTest { + useCluster testClusters.remoteCluster + + // Set properties for connection to clusters and between clusters + doFirst { + getClusters().forEach { cluster -> + String allTransportSocketURI = cluster.nodes.stream().flatMap { node -> + node.getAllTransportPortURI().stream() + }.collect(Collectors.joining(",")) + String allHttpSocketURI = cluster.nodes.stream().flatMap { node -> + node.getAllHttpSocketURI().stream() + }.collect(Collectors.joining(",")) + + systemProperty "tests.rest.${cluster.name}.http_hosts", "${-> allHttpSocketURI}" + systemProperty "tests.rest.${cluster.name}.transport_hosts", "${-> allTransportSocketURI}" + } + } + testLogging { events "passed", "skipped", "failed" } + dependsOn ':opensearch-sql-plugin:bundlePlugin' if(getOSFamilyType() != "windows") { dependsOn startPrometheus diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java index dc18e8510d..9fe89583e4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java @@ -34,12 +34,12 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.test.rest.OpenSearchRestTestCase; +import org.opensearch.sql.multicluster.OpenSearchMultiClustersRestTestCase; /** * OpenSearch SQL integration test base class to support both security disabled and enabled OpenSearch cluster. */ -public abstract class OpenSearchSQLRestTestCase extends OpenSearchRestTestCase { +public abstract class OpenSearchSQLRestTestCase extends OpenSearchMultiClustersRestTestCase { private static final Logger LOG = LogManager.getLogger(); @@ -74,10 +74,17 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE } protected static void wipeAllOpenSearchIndices() throws IOException { + wipeAllOpenSearchIndices(client()); + if (remoteClient() != null) { + wipeAllOpenSearchIndices(remoteClient()); + } + } + + protected static void wipeAllOpenSearchIndices(RestClient client) throws IOException { // include all the indices, included hidden indices. // https://www.elastic.co/guide/en/elasticsearch/reference/current/cat-indices.html#cat-indices-api-query-params try { - Response response = client().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all")); + Response response = client.performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all")); JSONArray jsonArray = new JSONArray(EntityUtils.toString(response.getEntity(), "UTF-8")); for (Object object : jsonArray) { JSONObject jsonObject = (JSONObject) object; @@ -85,7 +92,7 @@ protected static void wipeAllOpenSearchIndices() throws IOException { try { // System index, mostly named .opensearch-xxx or .opendistro-xxx, are not allowed to delete if (!indexName.startsWith(".opensearch") && !indexName.startsWith(".opendistro")) { - client().performRequest(new Request("DELETE", "/" + indexName)); + client.performRequest(new Request("DELETE", "/" + indexName)); } } catch (Exception e) { // TODO: Ignore index delete error for now. Remove this if strict check on system index added above. diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 35ae5d3675..f6e4b23708 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -17,6 +17,7 @@ import org.opensearch.client.Request; import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; +import org.opensearch.client.RestClient; import org.opensearch.sql.common.setting.Settings; import javax.management.MBeanServerInvocationHandler; @@ -167,6 +168,10 @@ protected void resetQuerySizeLimit() throws IOException { protected static void wipeAllClusterSettings() throws IOException { updateClusterSettings(new ClusterSetting("persistent", "*", null)); updateClusterSettings(new ClusterSetting("transient", "*", null)); + if (remoteClient() != null) { + updateClusterSettings(new ClusterSetting("persistent", "*", null), remoteClient()); + updateClusterSettings(new ClusterSetting("transient", "*", null), remoteClient()); + } } protected void setMaxResultWindow(String indexName, Integer window) throws IOException { @@ -188,17 +193,21 @@ protected void init() throws Exception { * Make it thread-safe in case tests are running in parallel but does not guarantee * if test like DeleteIT that mutates cluster running in parallel. */ - protected synchronized void loadIndex(Index index) throws IOException { + protected synchronized void loadIndex(Index index, RestClient client) throws IOException { String indexName = index.getName(); String mapping = index.getMapping(); String dataSet = index.getDataSet(); - if (!isIndexExist(client(), indexName)) { - createIndexByRestClient(client(), indexName, mapping); - loadDataByRestClient(client(), indexName, dataSet); + if (!isIndexExist(client, indexName)) { + createIndexByRestClient(client, indexName, mapping); + loadDataByRestClient(client, indexName, dataSet); } } + protected synchronized void loadIndex(Index index) throws IOException { + loadIndex(index, client()); + } + protected Request getSqlRequest(String request, boolean explain) { return getSqlRequest(request, explain, "json"); } @@ -325,12 +334,16 @@ private String executeRequest(final String requestBody, final boolean isExplainQ return executeRequest(sqlRequest); } - protected static String executeRequest(final Request request) throws IOException { - Response response = client().performRequest(request); + protected static String executeRequest(final Request request, RestClient client) throws IOException { + Response response = client.performRequest(request); Assert.assertEquals(200, response.getStatusLine().getStatusCode()); return getResponseBody(response); } + protected static String executeRequest(final Request request) throws IOException { + return executeRequest(request, client()); + } + protected JSONObject executeQueryWithGetRequest(final String sqlQuery) throws IOException { final Request request = buildGetEndpointRequest(sqlQuery); @@ -350,7 +363,7 @@ protected JSONObject executeCursorCloseQuery(final String cursor) throws IOExcep return new JSONObject(executeRequest(sqlRequest)); } - protected static JSONObject updateClusterSettings(ClusterSetting setting) throws IOException { + protected static JSONObject updateClusterSettings(ClusterSetting setting, RestClient client) throws IOException { Request request = new Request("PUT", "/_cluster/settings"); String persistentSetting = String.format(Locale.ROOT, "{\"%s\": {\"%s\": %s}}", setting.type, setting.name, setting.value); @@ -358,7 +371,11 @@ protected static JSONObject updateClusterSettings(ClusterSetting setting) throws RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); restOptionsBuilder.addHeader("Content-Type", "application/json"); request.setOptions(restOptionsBuilder); - return new JSONObject(executeRequest(request)); + return new JSONObject(executeRequest(request, client)); + } + + protected static JSONObject updateClusterSettings(ClusterSetting setting) throws IOException { + return updateClusterSettings(setting, client()); } protected static JSONObject getAllClusterSettings() throws IOException { diff --git a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java new file mode 100644 index 0000000000..a8018045f8 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java @@ -0,0 +1,126 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.multicluster; + +import org.apache.hc.core5.http.HttpHost; +import org.opensearch.client.Request; +import org.opensearch.client.RestClient; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.test.rest.OpenSearchRestTestCase; +import org.junit.AfterClass; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static java.util.Collections.unmodifiableList; + +/** + * Superclass for tests that interact with multiple external test clusters using OpenSearch's {@link RestClient}. + */ +public abstract class OpenSearchMultiClustersRestTestCase extends OpenSearchRestTestCase { + + public static final String REMOTE_CLUSTER = "remoteCluster"; + + private static RestClient remoteClient; + /** + * A client for the running remote OpenSearch cluster configured to take test administrative actions + * like remove all indexes after the test completes + */ + private static RestClient remoteAdminClient; + + // modified from initClient in OpenSearchRestTestCase + public void initRemoteClient() throws IOException { + if (remoteClient == null) { + assert remoteAdminClient == null; + String cluster = getTestRestCluster(REMOTE_CLUSTER); + String[] stringUrls = cluster.split(","); + List hosts = new ArrayList<>(stringUrls.length); + for (String stringUrl : stringUrls) { + int portSeparator = stringUrl.lastIndexOf(':'); + if (portSeparator < 0) { + throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]"); + } + String host = stringUrl.substring(0, portSeparator); + int port = Integer.valueOf(stringUrl.substring(portSeparator + 1)); + hosts.add(buildHttpHost(host, port)); + } + final List clusterHosts = unmodifiableList(hosts); + logger.info("initializing REST clients against {}", clusterHosts); + remoteClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[0])); + remoteAdminClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[0])); + } + assert remoteClient != null; + assert remoteAdminClient != null; + } + + protected String getTestRestCluster(String clusterName) { + String cluster = System.getProperty("tests.rest." + clusterName + ".http_hosts"); + if (cluster == null) { + throw new RuntimeException( + "Must specify [tests.rest." + + clusterName + + ".http_hosts] system property with a comma delimited list of [host:port] " + + "to which to send REST requests" + ); + } + return cluster; + } + + protected String getTestTransportCluster(String clusterName) { + String cluster = System.getProperty("tests.rest." + clusterName + ".transport_hosts"); + if (cluster == null) { + throw new RuntimeException( + "Must specify [tests.rest." + + clusterName + + ".transport_hosts] system property with a comma delimited list of [host:port] " + + "for connections between clusters" + ); + } + return cluster; + } + + public void configureMultiClusters() throws IOException { + initRemoteClient(); + + Request connectionRequest = new Request("PUT", "_cluster/settings"); + String connectionSetting = "{\"persistent\": {\"cluster\": {\"remote\": {\"" + + REMOTE_CLUSTER + + "\": {\"seeds\": [\"" + + getTestTransportCluster(REMOTE_CLUSTER).split(",")[0] + + "\"]}}}}}"; + connectionRequest.setJsonEntity(connectionSetting); + logger.info("Creating connection from coordinating cluster to {}", REMOTE_CLUSTER); + adminClient().performRequest(connectionRequest); + } + + @AfterClass + public static void closeRemoteClients() throws IOException { + try { + IOUtils.close(remoteClient, remoteAdminClient); + } finally { + remoteClient = null; + remoteAdminClient = null; + } + } + + /** + * Get the client to remote cluster used for ordinary api calls while writing a test + */ + protected static RestClient remoteClient() { + return remoteClient; + } + + /** + * Get the client to remote cluster used for test administrative actions. + * Do not use this while writing a test. Only use it for cleaning up after tests. + */ + protected static RestClient remoteAdminClient() { + return remoteAdminClient; + } + +} diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java new file mode 100644 index 0000000000..c959c8a970 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.ppl; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; +import static org.opensearch.sql.util.MatcherUtils.columnName; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyColumn; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; +import org.opensearch.client.Request; +import org.opensearch.client.ResponseException; + +public class CrossClusterSearchIT extends PPLIntegTestCase { + + private final static String TEST_INDEX_BANK_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_BANK; + private final static String TEST_INDEX_DOG_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; + + @Override + public void init() throws IOException { + configureMultiClusters(); + loadIndex(Index.BANK); + loadIndex(Index.BANK, remoteClient()); + loadIndex(Index.DOG); + loadIndex(Index.DOG, remoteClient()); + } + + @Test + public void testCrossClusterSearchAllFields() throws IOException { + JSONObject result = executeQuery(String.format("search source=%s", TEST_INDEX_DOG_REMOTE)); + verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age")); + } + + @Test + public void testCrossClusterSearchCommandWithLogicalExpression() throws IOException { + JSONObject result = + executeQuery( + String.format( + "search source=%s firstname='Hattie' | fields firstname", TEST_INDEX_BANK_REMOTE)); + verifyDataRows(result, rows("Hattie")); + } +} From ca6b805fc363e0e12dedacfaa476cbe7e84a610e Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Tue, 18 Apr 2023 18:14:49 -0700 Subject: [PATCH 06/17] Add IT test case Signed-off-by: Sean Kao --- integ-test/build.gradle | 2 +- .../sql/ppl/CrossClusterSearchIT.java | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/integ-test/build.gradle b/integ-test/build.gradle index 695e007f44..bbda0ac255 100644 --- a/integ-test/build.gradle +++ b/integ-test/build.gradle @@ -232,7 +232,7 @@ integTest { systemProperty "tests.rest.${cluster.name}.transport_hosts", "${-> allTransportSocketURI}" } } - + testLogging { events "passed", "skipped", "failed" } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java index c959c8a970..55c871d4b0 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java @@ -16,8 +16,6 @@ import java.io.IOException; import org.json.JSONObject; import org.junit.jupiter.api.Test; -import org.opensearch.client.Request; -import org.opensearch.client.ResponseException; public class CrossClusterSearchIT extends PPLIntegTestCase { @@ -41,10 +39,17 @@ public void testCrossClusterSearchAllFields() throws IOException { @Test public void testCrossClusterSearchCommandWithLogicalExpression() throws IOException { - JSONObject result = - executeQuery( - String.format( - "search source=%s firstname='Hattie' | fields firstname", TEST_INDEX_BANK_REMOTE)); + JSONObject result = executeQuery(String.format( + "search source=%s firstname='Hattie' | fields firstname", TEST_INDEX_BANK_REMOTE)); verifyDataRows(result, rows("Hattie")); } + + @Test + public void testCrossClusterSearchMultiClusters() throws IOException { + JSONObject result = executeQuery(String.format( + "search source=%s,%s firstname='Hattie' | fields firstname", TEST_INDEX_BANK_REMOTE, TEST_INDEX_BANK)); + verifyDataRows(result, + rows("Hattie"), + rows("Hattie")); + } } From 6572a14951b70521bba65c85e8b289fa738654de Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Wed, 19 Apr 2023 12:57:02 -0700 Subject: [PATCH 07/17] Document ccs for ppl Signed-off-by: Sean Kao --- docs/user/ppl/admin/cross_cluster_search.rst | 108 ++++++++++++++++++ docs/user/ppl/admin/security.rst | 3 +- docs/user/ppl/cmd/search.rst | 2 +- .../OpenSearchMultiClustersRestTestCase.java | 14 ++- .../opensearch/request/OpenSearchRequest.java | 1 - 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 docs/user/ppl/admin/cross_cluster_search.rst diff --git a/docs/user/ppl/admin/cross_cluster_search.rst b/docs/user/ppl/admin/cross_cluster_search.rst new file mode 100644 index 0000000000..9d54a37207 --- /dev/null +++ b/docs/user/ppl/admin/cross_cluster_search.rst @@ -0,0 +1,108 @@ +.. highlight:: sh + +==================== +Cross-Cluster Search +==================== + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 1 + +Introduction +============ +Cross-cluster search lets any node in a cluster execute search requests against other clusters. +It makes searching easy across all connected clusters, allowing users to use multiple smaller clusters instead of a single large one. + + +Configuration +============= +On the local cluster, add the remote cluster name and the IP address with port 9300 for each seed node. :: + + PUT _cluster/settings + { + "persistent": { + "cluster.remote": { + "": { + "seeds": [":9300"] + } + } + } + } + + +Using Cross-Cluster Search in PPL +================================= +Perform cross-cluster search by using ":" as the index identifier. + +Example search command :: + + >> search source = my_remote_cluster:my_index + + +Limitation +========== +Since OpenSearch does not support cross cluster system index query, field mapping of a remote cluster index is not available to the local cluster. +Therefore, the query engine requires that for any remote cluster index that the users need to search, +the local cluster keep a field mapping system index with the same index name. +This can be done by creating an index on the local cluster with the same name and schema as the remote cluster index. + + +Authentication and Permission +============================= + +1. The security plugin authenticates the user on the local cluster. +2. The security plugin fetches the user’s backend roles on the local cluster. +3. The call, including the authenticated user, is forwarded to the remote cluster. +4. The user’s permissions are evaluated on the remote cluster. + +Check `Cross-cluster search access control `_ for more details. + +On the local cluster, create a new role and grant permission to access PPL plugin and access index, then map the user to this role:: + + PUT _plugins/_security/api/roles/ppl_role + { + "cluster_permissions":[ + "cluster:admin/opensearch/ppl" + ], + "index_permissions":[ + { + "index_patterns":["example_index"], + "allowed_actions":[ + "indices:data/read/search", + "indices:admin/mappings/get", + "indices:monitor/settings/get" + ] + } + ] + } + + PUT _plugins/_security/api/rolesmapping/ppl_role + { + "backend_roles" : [], + "hosts" : [], + "users" : ["test_user"] + } + +On the remote cluster, create a new role and grant permission to access index. Create a user the same as the local cluster, and map the user to this role:: + + PUT _plugins/_security/api/roles/example_index_ccs_role + { + "index_permissions":[ + { + "index_patterns":["example_index"], + "allowed_actions":[ + "indices:admin/shards/search_shards", + "indices:data/read/search" + ] + } + ] + } + + PUT _plugins/_security/api/rolesmapping/example_index_ccs_role + { + "backend_roles" : [], + "hosts" : [], + "users" : ["test_user"] + } diff --git a/docs/user/ppl/admin/security.rst b/docs/user/ppl/admin/security.rst index 529704574b..40fa72f095 100644 --- a/docs/user/ppl/admin/security.rst +++ b/docs/user/ppl/admin/security.rst @@ -34,7 +34,8 @@ Example: Create the ppl_role for test_user. then test_user could use PPL to quer ], "allowed_actions": [ "indices:data/read/search*", - "indices:admin/mappings/get" + "indices:admin/mappings/get", + "indices:monitor/settings/get" ] }] } diff --git a/docs/user/ppl/cmd/search.rst b/docs/user/ppl/cmd/search.rst index 120ce34d40..7e8afb16ef 100644 --- a/docs/user/ppl/cmd/search.rst +++ b/docs/user/ppl/cmd/search.rst @@ -19,7 +19,7 @@ Syntax search source= [boolean-expression] * search: search keywords, which could be ignore. -* index: mandatory. search command must specify which index to query from. +* index: mandatory. search command must specify which index to query from. The index name can be prefixed by ":" for cross-cluster search. * bool-expression: optional. any expression which could be evaluated to boolean value. diff --git a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java index a8018045f8..7ac74fac0a 100644 --- a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java @@ -33,7 +33,7 @@ public abstract class OpenSearchMultiClustersRestTestCase extends OpenSearchRest */ private static RestClient remoteAdminClient; - // modified from initClient in OpenSearchRestTestCase + // Modified from initClient in OpenSearchRestTestCase public void initRemoteClient() throws IOException { if (remoteClient == null) { assert remoteAdminClient == null; @@ -58,6 +58,9 @@ public void initRemoteClient() throws IOException { assert remoteAdminClient != null; } + /** + * Get a comma delimited list of [host:port] to which to send REST requests. + */ protected String getTestRestCluster(String clusterName) { String cluster = System.getProperty("tests.rest." + clusterName + ".http_hosts"); if (cluster == null) { @@ -71,6 +74,9 @@ protected String getTestRestCluster(String clusterName) { return cluster; } + /** + * Get a comma delimited list of [host:port] for connections between clusters. + */ protected String getTestTransportCluster(String clusterName) { String cluster = System.getProperty("tests.rest." + clusterName + ".transport_hosts"); if (cluster == null) { @@ -84,6 +90,10 @@ protected String getTestTransportCluster(String clusterName) { return cluster; } + /** + * Initialize rest client to remote cluster, + * and create a connection to it from the coordinating cluster. + */ public void configureMultiClusters() throws IOException { initRemoteClient(); @@ -109,7 +119,7 @@ public static void closeRemoteClients() throws IOException { } /** - * Get the client to remote cluster used for ordinary api calls while writing a test + * Get the client to remote cluster used for ordinary api calls while writing a test. */ protected static RestClient remoteClient() { return remoteClient; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java index 23f33109e9..1451b4bc43 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java @@ -69,7 +69,6 @@ class IndexName { */ public IndexName(String indexName) { this.indexFullNames = indexName.split(COMMA); - // Remove all ":" prefix if they exist this.indexNames = Arrays.stream(indexFullNames) .map(name -> name.substring(name.indexOf(COLON) + 1)) .toArray(String[]::new); From 23b414db205c634d36947246c14030b14cc932c7 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 24 Apr 2023 09:30:16 -0700 Subject: [PATCH 08/17] documentation update Signed-off-by: Sean Kao --- docs/user/ppl/admin/cross_cluster_search.rst | 35 ++++---------------- docs/user/ppl/admin/security.rst | 3 +- docs/user/ppl/cmd/search.rst | 2 +- docs/user/ppl/general/identifiers.rst | 19 +++++++++++ docs/user/ppl/index.rst | 2 ++ 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/docs/user/ppl/admin/cross_cluster_search.rst b/docs/user/ppl/admin/cross_cluster_search.rst index 9d54a37207..ad7e71f1c5 100644 --- a/docs/user/ppl/admin/cross_cluster_search.rst +++ b/docs/user/ppl/admin/cross_cluster_search.rst @@ -44,6 +44,7 @@ Example search command :: Limitation ========== Since OpenSearch does not support cross cluster system index query, field mapping of a remote cluster index is not available to the local cluster. +(`[Feature] Cross cluster field mappings query #6573 `_) Therefore, the query engine requires that for any remote cluster index that the users need to search, the local cluster keep a field mapping system index with the same index name. This can be done by creating an index on the local cluster with the same name and schema as the remote cluster index. @@ -59,39 +60,17 @@ Authentication and Permission Check `Cross-cluster search access control `_ for more details. -On the local cluster, create a new role and grant permission to access PPL plugin and access index, then map the user to this role:: +Example: Create the ppl_role for test_user on local cluster and the ccs_role for test_user on remote cluster. Then test_user could use PPL to query ``ppl-security-demo`` index on remote cluster. - PUT _plugins/_security/api/roles/ppl_role - { - "cluster_permissions":[ - "cluster:admin/opensearch/ppl" - ], - "index_permissions":[ - { - "index_patterns":["example_index"], - "allowed_actions":[ - "indices:data/read/search", - "indices:admin/mappings/get", - "indices:monitor/settings/get" - ] - } - ] - } - - PUT _plugins/_security/api/rolesmapping/ppl_role - { - "backend_roles" : [], - "hosts" : [], - "users" : ["test_user"] - } +1. On the local cluster, refer to `Security Settings `_ to create role and user for PPL plugin and index access permission. -On the remote cluster, create a new role and grant permission to access index. Create a user the same as the local cluster, and map the user to this role:: +2. On the remote cluster, create a new role and grant permission to access index. Create a user the same as the local cluster, and map the user to this role:: - PUT _plugins/_security/api/roles/example_index_ccs_role + PUT _plugins/_security/api/roles/ccs_role { "index_permissions":[ { - "index_patterns":["example_index"], + "index_patterns":["ppl-security-demo"], "allowed_actions":[ "indices:admin/shards/search_shards", "indices:data/read/search" @@ -100,7 +79,7 @@ On the remote cluster, create a new role and grant permission to access index. C ] } - PUT _plugins/_security/api/rolesmapping/example_index_ccs_role + PUT _plugins/_security/api/rolesmapping/ccs_role { "backend_roles" : [], "hosts" : [], diff --git a/docs/user/ppl/admin/security.rst b/docs/user/ppl/admin/security.rst index 40fa72f095..529704574b 100644 --- a/docs/user/ppl/admin/security.rst +++ b/docs/user/ppl/admin/security.rst @@ -34,8 +34,7 @@ Example: Create the ppl_role for test_user. then test_user could use PPL to quer ], "allowed_actions": [ "indices:data/read/search*", - "indices:admin/mappings/get", - "indices:monitor/settings/get" + "indices:admin/mappings/get" ] }] } diff --git a/docs/user/ppl/cmd/search.rst b/docs/user/ppl/cmd/search.rst index 7e8afb16ef..eb237ee51f 100644 --- a/docs/user/ppl/cmd/search.rst +++ b/docs/user/ppl/cmd/search.rst @@ -16,7 +16,7 @@ Description Syntax ============ -search source= [boolean-expression] +search source=[:] [boolean-expression] * search: search keywords, which could be ignore. * index: mandatory. search command must specify which index to query from. The index name can be prefixed by ":" for cross-cluster search. diff --git a/docs/user/ppl/general/identifiers.rst b/docs/user/ppl/general/identifiers.rst index b15f621af8..51fc36c40f 100644 --- a/docs/user/ppl/general/identifiers.rst +++ b/docs/user/ppl/general/identifiers.rst @@ -83,6 +83,25 @@ Here are examples for quoting an index name by back ticks:: +------------------+ +Cross-Cluster Index Identifiers +=================== + +Description +----------- + +A cross-cluster index identifier is an index identifier with a prefix ``:``. The cluster identifier could contain star ``*``. This is mostly an cluster pattern for wildcard match. + +Use Cases +--------- + +It is used to identify an index on a remote cluster for cross-cluster search. + +Examples +-------- + +For example, if you setup a connection between the local cluster and a remote cluster ``my_cluster``, then you can run ``source=my_cluster:accounts`` to query the ``accounts`` index at ``my_cluster``. + + Case Sensitivity ================ diff --git a/docs/user/ppl/index.rst b/docs/user/ppl/index.rst index a69136bb19..4ddc9fc9a5 100644 --- a/docs/user/ppl/index.rst +++ b/docs/user/ppl/index.rst @@ -38,6 +38,8 @@ The query start with search command and then flowing a set of command delimited - `Prometheus Connector `_ + - `Cross Cluster Search `_ + * **Commands** - `Syntax `_ From a69a87bfdaa433feac71969e9107feac84618b54 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 24 Apr 2023 11:37:20 -0700 Subject: [PATCH 09/17] feat: allow describe remote cluster index in PPL Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 13 ++----------- .../org/opensearch/sql/ppl/parser/AstBuilder.java | 10 +--------- .../sql/ppl/parser/AstExpressionBuilder.java | 10 ---------- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 10 ++++------ 4 files changed, 7 insertions(+), 36 deletions(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 40c47d9fe6..f95dcc4eaa 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -181,18 +181,14 @@ mlArg /** clauses */ fromClause - : SOURCE EQUAL clusterTableSourceClause - | INDEX EQUAL clusterTableSourceClause + : SOURCE EQUAL tableSourceClause + | INDEX EQUAL tableSourceClause ; tableSourceClause : tableSource (COMMA tableSource)* ; -clusterTableSourceClause - : clusterTableSource (COMMA clusterTableSource)* - ; - renameClasue : orignalField=wcFieldExpression AS renamedField=wcFieldExpression ; @@ -326,11 +322,6 @@ multiFieldRelevanceFunction /** tables */ tableSource - : qualifiedName - | ID_DATE_SUFFIX - ; - -clusterTableSource : qualifiedName | clusterQualifiedName | ID_DATE_SUFFIX diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index fd0972cb03..73c7238624 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -7,7 +7,6 @@ package org.opensearch.sql.ppl.parser; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; -import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ClusterTableSourceClauseContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DedupCommandContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.DescribeCommandContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EvalCommandContext; @@ -347,7 +346,7 @@ public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { */ @Override public UnresolvedPlan visitFromClause(FromClauseContext ctx) { - return visitClusterTableSourceClause(ctx.clusterTableSourceClause()); + return visitTableSourceClause(ctx.tableSourceClause()); } @Override @@ -357,13 +356,6 @@ public UnresolvedPlan visitTableSourceClause(TableSourceClauseContext ctx) { .collect(Collectors.toList())); } - @Override - public UnresolvedPlan visitClusterTableSourceClause(ClusterTableSourceClauseContext ctx) { - return new Relation(ctx.clusterTableSource() - .stream().map(this::internalVisitExpression) - .collect(Collectors.toList())); - } - @Override @Generated //To exclude from jacoco..will remove https://github.com/opensearch-project/sql/issues/1019 public UnresolvedPlan visitTableFunction(OpenSearchPPLParser.TableFunctionContext ctx) { diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index abdb9a71e5..c9823b67f9 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -14,7 +14,6 @@ import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanLiteralContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BySpanClauseContext; -import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ClusterTableSourceContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.CompareExprContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.ConvertedDataTypeContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.CountAllFunctionCallContext; @@ -288,15 +287,6 @@ public UnresolvedExpression visitTableSource(TableSourceContext ctx) { } } - @Override - public UnresolvedExpression visitClusterTableSource(ClusterTableSourceContext ctx) { - if (ctx.getChild(0) instanceof IdentsAsQualifiedNameContext) { - return visitIdentifiers(((IdentsAsQualifiedNameContext) ctx.getChild(0)).ident()); - } else { - return visitIdentifiers(Arrays.asList(ctx)); - } - } - @Override public UnresolvedExpression visitPositionFunction( OpenSearchPPLParser.PositionFunctionContext ctx) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 8e16d24de4..40caeac4ad 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -53,7 +53,7 @@ public void testSearchCommandCrossClusterIgnoreSearchKeywordShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("source=c:t a=1 b=2"); assertNotEquals(null, tree); } - + @Test public void testSearchFieldsCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("search source=t a=1 b=2 | fields a,b"); @@ -220,11 +220,9 @@ public void testDescribeCommandWithMultipleIndicesShouldPass() { } @Test - public void testDescribeCommandCrossClusterShouldFail() { - exceptionRule.expect(RuntimeException.class); - exceptionRule.expectMessage("Failed to parse query due to offending symbol"); - - new PPLSyntaxParser().parse("describe c:t"); + public void testDescribeCommandCrossClusterShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("describe c:t"); + assertNotEquals(null, tree); } @Test From 95f211bfb8462acee29fb2662b0a10ee1e09515e Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 24 Apr 2023 12:47:44 -0700 Subject: [PATCH 10/17] feat: allow "*:index" to match all remote clusters Signed-off-by: Sean Kao --- .../OpenSearchMultiClustersRestTestCase.java | 1 + .../sql/ppl/CrossClusterSearchIT.java | 71 +++++++++++++++++++ ppl/src/main/antlr/OpenSearchPPLParser.g4 | 7 +- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 12 ++++ .../sql/ppl/parser/AstBuilderTest.java | 13 ++++ 5 files changed, 103 insertions(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java index 7ac74fac0a..1452217e04 100644 --- a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java @@ -25,6 +25,7 @@ public abstract class OpenSearchMultiClustersRestTestCase extends OpenSearchRestTestCase { public static final String REMOTE_CLUSTER = "remoteCluster"; + public static final String MATCH_ALL_REMOTE_CLUSTER = "*"; private static RestClient remoteClient; /** diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java index 55c871d4b0..963d23c720 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java @@ -21,6 +21,7 @@ public class CrossClusterSearchIT extends PPLIntegTestCase { private final static String TEST_INDEX_BANK_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_BANK; private final static String TEST_INDEX_DOG_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; + private final static String TEST_INDEX_DOG_MATCH_ALL_REMOTE = MATCH_ALL_REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; @Override public void init() throws IOException { @@ -37,6 +38,12 @@ public void testCrossClusterSearchAllFields() throws IOException { verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age")); } + @Test + public void testMatchAllCrossClusterSearchAllFields() throws IOException { + JSONObject result = executeQuery(String.format("search source=%s", TEST_INDEX_DOG_MATCH_ALL_REMOTE)); + verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age")); + } + @Test public void testCrossClusterSearchCommandWithLogicalExpression() throws IOException { JSONObject result = executeQuery(String.format( @@ -52,4 +59,68 @@ public void testCrossClusterSearchMultiClusters() throws IOException { rows("Hattie"), rows("Hattie")); } + + @Test + public void testCrossClusterDescribeAllFields() throws IOException { + JSONObject result = executeQuery(String.format("describe %s", TEST_INDEX_DOG_REMOTE)); + verifyColumn( + result, + columnName("TABLE_CAT"), + columnName("TABLE_SCHEM"), + columnName("TABLE_NAME"), + columnName("COLUMN_NAME"), + columnName("DATA_TYPE"), + columnName("TYPE_NAME"), + columnName("COLUMN_SIZE"), + columnName("BUFFER_LENGTH"), + columnName("DECIMAL_DIGITS"), + columnName("NUM_PREC_RADIX"), + columnName("NULLABLE"), + columnName("REMARKS"), + columnName("COLUMN_DEF"), + columnName("SQL_DATA_TYPE"), + columnName("SQL_DATETIME_SUB"), + columnName("CHAR_OCTET_LENGTH"), + columnName("ORDINAL_POSITION"), + columnName("IS_NULLABLE"), + columnName("SCOPE_CATALOG"), + columnName("SCOPE_SCHEMA"), + columnName("SCOPE_TABLE"), + columnName("SOURCE_DATA_TYPE"), + columnName("IS_AUTOINCREMENT"), + columnName("IS_GENERATEDCOLUMN") + ); + } + + @Test + public void testMatchAllCrossClusterDescribeAllFields() throws IOException { + JSONObject result = executeQuery(String.format("describe %s", TEST_INDEX_DOG_MATCH_ALL_REMOTE)); + verifyColumn( + result, + columnName("TABLE_CAT"), + columnName("TABLE_SCHEM"), + columnName("TABLE_NAME"), + columnName("COLUMN_NAME"), + columnName("DATA_TYPE"), + columnName("TYPE_NAME"), + columnName("COLUMN_SIZE"), + columnName("BUFFER_LENGTH"), + columnName("DECIMAL_DIGITS"), + columnName("NUM_PREC_RADIX"), + columnName("NULLABLE"), + columnName("REMARKS"), + columnName("COLUMN_DEF"), + columnName("SQL_DATA_TYPE"), + columnName("SQL_DATETIME_SUB"), + columnName("CHAR_OCTET_LENGTH"), + columnName("ORDINAL_POSITION"), + columnName("IS_NULLABLE"), + columnName("SCOPE_CATALOG"), + columnName("SCOPE_SCHEMA"), + columnName("SCOPE_TABLE"), + columnName("SOURCE_DATA_TYPE"), + columnName("IS_AUTOINCREMENT"), + columnName("IS_GENERATEDCOLUMN") + ); + } } diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index f95dcc4eaa..9a641c0810 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -725,7 +725,7 @@ qualifiedName ; clusterQualifiedName - : ident COLON ident (DOT ident)* + : clusterIdent COLON ident (DOT ident)* ; wcQualifiedName @@ -739,6 +739,11 @@ ident | keywordsCanBeId ; +clusterIdent + : ident + | STAR + ; + wildcard : ident (MODULE ident)* (MODULE)? | SINGLE_QUOTE wildcard SINGLE_QUOTE diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 40caeac4ad..bb6aad378f 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -42,6 +42,12 @@ public void testSearchCommandCrossClusterShouldPass() { assertNotEquals(null, tree); } + @Test + public void testSearchCommandMatchAllCrossClusterShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=*:t a=1 b=2"); + assertNotEquals(null, tree); + } + @Test public void testSearchCommandCrossClusterWithMultipleIndicesShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("search source=c:t,d:u,v a=1 b=2"); @@ -225,6 +231,12 @@ public void testDescribeCommandCrossClusterShouldPass() { assertNotEquals(null, tree); } + @Test + public void testDescribeCommandMatchAllCrossClusterShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("describe *:t"); + assertNotEquals(null, tree); + } + @Test public void testDescribeFieldsCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("describe t | fields a,b"); diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index 03c187c6fb..8249619b37 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -84,6 +84,13 @@ public void testSearchCrossClusterCommand() { ); } + @Test + public void testSearchMatchAllCrossClusterCommand() { + assertEqual("search source=*:t", + relation(qualifiedName("*:t")) + ); + } + @Test public void testPrometheusSearchCommand() { assertEqual("search source = prometheus.http_requests_total", @@ -743,6 +750,12 @@ public void testDescribeCommand() { relation(mappingTable("t"))); } + @Test + public void testDescribeMatchAllCrossClusterSearchCommand() { + assertEqual("describe *:t", + relation(mappingTable("*:t"))); + } + @Test public void testDescribeCommandWithMultipleIndices() { assertEqual("describe t,u", From 2d179a708141436bdbdec7b2f3d93b1308cba1eb Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 24 Apr 2023 15:35:12 -0700 Subject: [PATCH 11/17] use local index names for field mappings request Signed-off-by: Sean Kao --- .../request/OpenSearchQueryRequest.java | 2 +- .../opensearch/request/OpenSearchRequest.java | 18 ++---------------- .../request/OpenSearchScrollRequest.java | 2 +- .../OpenSearchDescribeIndexRequest.java | 19 +++++++++++++++++-- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java index 8469377f97..3976f854fd 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java @@ -123,7 +123,7 @@ public void clean(Consumer cleanAction) { @VisibleForTesting protected SearchRequest searchRequest() { return new SearchRequest() - .indices(indexName.getIndexFullNames()) + .indices(indexName.getIndexNames()) .source(sourceBuilder); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java index 1451b4bc43..ce990780c1 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequest.java @@ -6,7 +6,6 @@ package org.opensearch.sql.opensearch.request; -import java.util.Arrays; import java.util.function.Consumer; import java.util.function.Function; import lombok.EqualsAndHashCode; @@ -58,24 +57,11 @@ OpenSearchResponse search(Function searchAction, @EqualsAndHashCode class IndexName { private static final String COMMA = ","; - private static final String COLON = ":"; - private final String[] indexFullNames; private final String[] indexNames; - /** - * Constructor. - * indexNames are indexFullNames without the "{cluster}:" prefix. - */ public IndexName(String indexName) { - this.indexFullNames = indexName.split(COMMA); - this.indexNames = Arrays.stream(indexFullNames) - .map(name -> name.substring(name.indexOf(COLON) + 1)) - .toArray(String[]::new); - } - - public String[] getIndexFullNames() { - return indexFullNames; + this.indexNames = indexName.split(COMMA); } public String[] getIndexNames() { @@ -84,7 +70,7 @@ public String[] getIndexNames() { @Override public String toString() { - return String.join(COMMA, indexFullNames); + return String.join(COMMA, indexNames); } } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java index 2b340623fc..9b0d6ca074 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java @@ -114,7 +114,7 @@ public void clean(Consumer cleanAction) { */ public SearchRequest searchRequest() { return new SearchRequest() - .indices(indexName.getIndexFullNames()) + .indices(indexName.getIndexNames()) .scroll(DEFAULT_SCROLL_TIMEOUT) .source(sourceBuilder); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java index 22ed8c2ffe..f4fd7b98d3 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java @@ -11,6 +11,7 @@ import static org.opensearch.sql.opensearch.client.OpenSearchClient.META_CLUSTER_NAME; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -82,7 +83,8 @@ public List search() { // TODO possible collision if two indices have fields with the same name and different mappings public Map getFieldTypes() { Map fieldTypes = new HashMap<>(); - Map indexMappings = client.getIndexMappings(indexName.getIndexNames()); + Map indexMappings = + client.getIndexMappings(getLocalIndexNames(indexName.getIndexNames())); for (IndexMapping indexMapping : indexMappings.values()) { fieldTypes.putAll(indexMapping.getFieldMappings()); } @@ -95,7 +97,7 @@ public Map getFieldTypes() { * @return max result window */ public Integer getMaxResultWindow() { - return client.getIndexMaxResultWindows(indexName.getIndexNames()) + return client.getIndexMaxResultWindows(getLocalIndexNames(indexName.getIndexNames())) .values().stream().min(Integer::compare).get(); } @@ -119,6 +121,19 @@ private ExprTupleValue row(String fieldName, String fieldType, int position, Str return new ExprTupleValue(valueMap); } + /** + * Return index names without "{cluster}:" prefix. + * Without the prefix, they refer to the indices at the local cluster. + * + * @param indexNames a string array of index names + * @return local cluster index names + */ + private String[] getLocalIndexNames(String[] indexNames) { + return Arrays.stream(indexNames) + .map(name -> name.substring(name.indexOf(":") + 1)) + .toArray(String[]::new); + } + private String clusterName(Map meta) { return meta.getOrDefault(META_CLUSTER_NAME, DEFAULT_TABLE_CAT); } From 572bdbb6cf8649d8864044f4ef9a00ae3053d13a Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Mon, 24 Apr 2023 16:43:45 -0700 Subject: [PATCH 12/17] allow ':' in index identifier Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 2 +- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 12c24bd531..7973c71546 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -367,7 +367,7 @@ INTEGER_LITERAL: DEC_DIGIT+; DECIMAL_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+; fragment DATE_SUFFIX: ([\-.][*0-9]+)*; -fragment ID_LITERAL: [@*A-Z]+?[*A-Z_\-0-9]*; +fragment ID_LITERAL: ([*A-Z]+?[*A-Z_\-0-9]*':')?[@*A-Z]+?[*A-Z_\-0-9]*; ID_DATE_SUFFIX: ID_LITERAL DATE_SUFFIX; DQUOTA_STRING: '"' ( '\\'. | '""' | ~('"'| '\\') )* '"'; SQUOTA_STRING: '\'' ('\\'. | '\'\'' | ~('\'' | '\\'))* '\''; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 9a641c0810..cca99407bb 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -323,7 +323,6 @@ multiFieldRelevanceFunction /** tables */ tableSource : qualifiedName - | clusterQualifiedName | ID_DATE_SUFFIX ; @@ -724,10 +723,6 @@ qualifiedName : ident (DOT ident)* #identsAsQualifiedName ; -clusterQualifiedName - : clusterIdent COLON ident (DOT ident)* - ; - wcQualifiedName : wildcard (DOT wildcard)* #identsAsWildcardQualifiedName ; @@ -739,11 +734,6 @@ ident | keywordsCanBeId ; -clusterIdent - : ident - | STAR - ; - wildcard : ident (MODULE ident)* (MODULE)? | SINGLE_QUOTE wildcard SINGLE_QUOTE From a283a5bb5876cbc7cac44271e9bb0baf9be631bd Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Thu, 27 Apr 2023 05:20:47 -0700 Subject: [PATCH 13/17] docs update Signed-off-by: Sean Kao --- docs/user/ppl/admin/cross_cluster_search.rst | 4 ++-- docs/user/ppl/admin/security.rst | 5 +++-- docs/user/ppl/cmd/search.rst | 5 +++++ docs/user/ppl/index.rst | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/user/ppl/admin/cross_cluster_search.rst b/docs/user/ppl/admin/cross_cluster_search.rst index ad7e71f1c5..5a0370ebe0 100644 --- a/docs/user/ppl/admin/cross_cluster_search.rst +++ b/docs/user/ppl/admin/cross_cluster_search.rst @@ -43,7 +43,7 @@ Example search command :: Limitation ========== -Since OpenSearch does not support cross cluster system index query, field mapping of a remote cluster index is not available to the local cluster. +Since OpenSearch does not support cross cluster index metadata retrieval, field mapping of a remote cluster index is not available to the local cluster. (`[Feature] Cross cluster field mappings query #6573 `_) Therefore, the query engine requires that for any remote cluster index that the users need to search, the local cluster keep a field mapping system index with the same index name. @@ -64,7 +64,7 @@ Example: Create the ppl_role for test_user on local cluster and the ccs_role for 1. On the local cluster, refer to `Security Settings `_ to create role and user for PPL plugin and index access permission. -2. On the remote cluster, create a new role and grant permission to access index. Create a user the same as the local cluster, and map the user to this role:: +2. On the remote cluster, create a new role and grant permission to access index. Create a user with the same name and credentials as the local cluster, and map the user to this role:: PUT _plugins/_security/api/roles/ccs_role { diff --git a/docs/user/ppl/admin/security.rst b/docs/user/ppl/admin/security.rst index 529704574b..e512cc259c 100644 --- a/docs/user/ppl/admin/security.rst +++ b/docs/user/ppl/admin/security.rst @@ -13,7 +13,7 @@ Security Settings Introduction ============ -User needs ``cluster:admin/opensearch/ppl`` permission to use PPL plugin. User also needs indices level permission ``indices:admin/mappings/get`` to get field mappings and ``indices:data/read/search*`` to search index. +User needs ``cluster:admin/opensearch/ppl`` permission to use PPL plugin. User also needs indices level permission ``indices:admin/mappings/get`` to get field mappings, ``indices:monitor/settings/get`` to get cluster settings, and ``indices:data/read/search*`` to search index. Using Rest API ============== @@ -34,7 +34,8 @@ Example: Create the ppl_role for test_user. then test_user could use PPL to quer ], "allowed_actions": [ "indices:data/read/search*", - "indices:admin/mappings/get" + "indices:admin/mappings/get", + "indices:monitor/settings/get" ] }] } diff --git a/docs/user/ppl/cmd/search.rst b/docs/user/ppl/cmd/search.rst index eb237ee51f..5299f9f78a 100644 --- a/docs/user/ppl/cmd/search.rst +++ b/docs/user/ppl/cmd/search.rst @@ -23,6 +23,11 @@ search source=[:] [boolean-expression] * bool-expression: optional. any expression which could be evaluated to boolean value. +Cross-Cluster Search +==================== +Cross-cluster search lets any node in a cluster execute search requests against other clusters. Refer to `Cross-Cluster Search `_ for configuration. + + Example 1: Fetch all the data ============================= diff --git a/docs/user/ppl/index.rst b/docs/user/ppl/index.rst index 4ddc9fc9a5..3fc094bddf 100644 --- a/docs/user/ppl/index.rst +++ b/docs/user/ppl/index.rst @@ -38,7 +38,7 @@ The query start with search command and then flowing a set of command delimited - `Prometheus Connector `_ - - `Cross Cluster Search `_ + - `Cross-Cluster Search `_ * **Commands** From c03b69e40c773fa81ae4af6388055377579743f7 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Thu, 27 Apr 2023 12:34:34 -0700 Subject: [PATCH 14/17] limit cluster prefix to table names only Signed-off-by: Sean Kao --- ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 8 +++++--- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 10 +++++++++- .../sql/ppl/parser/AstExpressionBuilder.java | 14 ++++++++++++-- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 18 ++++++++++++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 7973c71546..f412f29280 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -363,12 +363,14 @@ Y: 'Y'; // LITERALS AND VALUES //STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING; ID: ID_LITERAL; +CLUSTER: CLUSTER_PREFIX_LITERAL; INTEGER_LITERAL: DEC_DIGIT+; DECIMAL_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+; -fragment DATE_SUFFIX: ([\-.][*0-9]+)*; -fragment ID_LITERAL: ([*A-Z]+?[*A-Z_\-0-9]*':')?[@*A-Z]+?[*A-Z_\-0-9]*; -ID_DATE_SUFFIX: ID_LITERAL DATE_SUFFIX; +fragment DATE_SUFFIX: ([\-.][*0-9]+)+; +fragment ID_LITERAL: [@*A-Z]+?[*A-Z_\-0-9]*; +fragment CLUSTER_PREFIX_LITERAL: [*A-Z]+?[*A-Z_\-0-9]* COLON; +ID_DATE_SUFFIX: CLUSTER_PREFIX_LITERAL? ID_LITERAL DATE_SUFFIX; DQUOTA_STRING: '"' ( '\\'. | '""' | ~('"'| '\\') )* '"'; SQUOTA_STRING: '\'' ('\\'. | '\'\'' | ~('\'' | '\\'))* '\''; BQUOTA_STRING: '`' ( '\\'. | '``' | ~('`'|'\\'))* '`'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index cca99407bb..3a28210271 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -322,7 +322,7 @@ multiFieldRelevanceFunction /** tables */ tableSource - : qualifiedName + : tableQualifiedName | ID_DATE_SUFFIX ; @@ -723,6 +723,10 @@ qualifiedName : ident (DOT ident)* #identsAsQualifiedName ; +tableQualifiedName + : tableIdent (DOT ident)* #identsAsTableQualifiedName + ; + wcQualifiedName : wildcard (DOT wildcard)* #identsAsWildcardQualifiedName ; @@ -734,6 +738,10 @@ ident | keywordsCanBeId ; +tableIdent + : (CLUSTER)? ident + ; + wildcard : ident (MODULE ident)* (MODULE)? | SINGLE_QUOTE wildcard SINGLE_QUOTE diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index c9823b67f9..e56eae83a6 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -24,6 +24,7 @@ import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.EvalFunctionCallContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.FieldExpressionContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IdentsAsQualifiedNameContext; +import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IdentsAsTableQualifiedNameContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IdentsAsWildcardQualifiedNameContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.InExprContext; import static org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.IntegerLiteralContext; @@ -50,6 +51,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.RuleContext; import org.opensearch.sql.ast.dsl.AstDSL; @@ -280,8 +282,8 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( @Override public UnresolvedExpression visitTableSource(TableSourceContext ctx) { - if (ctx.getChild(0) instanceof IdentsAsQualifiedNameContext) { - return visitIdentifiers(((IdentsAsQualifiedNameContext) ctx.getChild(0)).ident()); + if (ctx.getChild(0) instanceof IdentsAsTableQualifiedNameContext) { + return visitIdentsAsTableQualifiedName((IdentsAsTableQualifiedNameContext) ctx.getChild(0)); } else { return visitIdentifiers(Arrays.asList(ctx)); } @@ -304,6 +306,14 @@ public UnresolvedExpression visitIdentsAsQualifiedName(IdentsAsQualifiedNameCont return visitIdentifiers(ctx.ident()); } + @Override + public UnresolvedExpression visitIdentsAsTableQualifiedName( + IdentsAsTableQualifiedNameContext ctx) { + return visitIdentifiers( + Stream.concat(Stream.of(ctx.tableIdent()), ctx.ident().stream()) + .collect(Collectors.toList())); + } + @Override public UnresolvedExpression visitIdentsAsWildcardQualifiedName( IdentsAsWildcardQualifiedNameContext ctx) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index bb6aad378f..bbc566e2ba 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -42,6 +42,24 @@ public void testSearchCommandCrossClusterShouldPass() { assertNotEquals(null, tree); } + @Test + public void testSearchCommandCrossClusterHiddenShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:.t a=1 b=2"); + assertNotEquals(null, tree); + } + + @Test + public void testSearchCommandCrossClusterQualifiedShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:t.u a=1 b=2"); + assertNotEquals(null, tree); + } + + @Test + public void testSearchCommandCrossClusterHiddenQualifiedShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("search source=c:.t.u a=1 b=2"); + assertNotEquals(null, tree); + } + @Test public void testSearchCommandMatchAllCrossClusterShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("search source=*:t a=1 b=2"); From 10d1ff60bf11d47c29dccdcaca2aa7743eb9c313 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Thu, 27 Apr 2023 13:08:33 -0700 Subject: [PATCH 15/17] move multicluster capability to sql rest test case Signed-off-by: Sean Kao --- .../sql/legacy/OpenSearchSQLRestTestCase.java | 120 ++++++++++++++- .../OpenSearchMultiClustersRestTestCase.java | 137 ------------------ 2 files changed, 118 insertions(+), 139 deletions(-) delete mode 100644 integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java index 9fe89583e4..61724b6100 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java @@ -7,6 +7,8 @@ package org.opensearch.sql.legacy; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Optional; import org.apache.hc.client5.http.auth.AuthScope; @@ -27,6 +29,7 @@ import org.apache.logging.log4j.Logger; import org.json.JSONArray; import org.json.JSONObject; +import org.junit.AfterClass; import org.opensearch.client.Request; import org.opensearch.client.Response; import org.opensearch.client.RestClient; @@ -34,14 +37,27 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.sql.multicluster.OpenSearchMultiClustersRestTestCase; +import org.opensearch.common.util.io.IOUtils; +import org.opensearch.test.rest.OpenSearchRestTestCase; + +import static java.util.Collections.unmodifiableList; /** * OpenSearch SQL integration test base class to support both security disabled and enabled OpenSearch cluster. + * Allows interaction with multiple external test clusters using OpenSearch's {@link RestClient}. */ -public abstract class OpenSearchSQLRestTestCase extends OpenSearchMultiClustersRestTestCase { +public abstract class OpenSearchSQLRestTestCase extends OpenSearchRestTestCase { private static final Logger LOG = LogManager.getLogger(); + public static final String REMOTE_CLUSTER = "remoteCluster"; + public static final String MATCH_ALL_REMOTE_CLUSTER = "*"; + + private static RestClient remoteClient; + /** + * A client for the running remote OpenSearch cluster configured to take test administrative actions + * like remove all indexes after the test completes + */ + private static RestClient remoteAdminClient; protected boolean isHttps() { boolean isHttps = Optional.ofNullable(System.getProperty("https")) @@ -61,6 +77,21 @@ protected String getProtocol() { return isHttps() ? "https" : "http"; } + /** + * Get the client to remote cluster used for ordinary api calls while writing a test. + */ + protected static RestClient remoteClient() { + return remoteClient; + } + + /** + * Get the client to remote cluster used for test administrative actions. + * Do not use this while writing a test. Only use it for cleaning up after tests. + */ + protected static RestClient remoteAdminClient() { + return remoteAdminClient; + } + protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException { RestClientBuilder builder = RestClient.builder(hosts); if (isHttps()) { @@ -73,6 +104,73 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE return builder.build(); } + // Modified from initClient in OpenSearchRestTestCase + public void initRemoteClient() throws IOException { + if (remoteClient == null) { + assert remoteAdminClient == null; + String cluster = getTestRestCluster(REMOTE_CLUSTER); + String[] stringUrls = cluster.split(","); + List hosts = new ArrayList<>(stringUrls.length); + for (String stringUrl : stringUrls) { + int portSeparator = stringUrl.lastIndexOf(':'); + if (portSeparator < 0) { + throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]"); + } + String host = stringUrl.substring(0, portSeparator); + int port = Integer.valueOf(stringUrl.substring(portSeparator + 1)); + hosts.add(buildHttpHost(host, port)); + } + final List clusterHosts = unmodifiableList(hosts); + logger.info("initializing REST clients against {}", clusterHosts); + remoteClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[0])); + remoteAdminClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[0])); + } + assert remoteClient != null; + assert remoteAdminClient != null; + } + + /** + * Get a comma delimited list of [host:port] to which to send REST requests. + */ + protected String getTestRestCluster(String clusterName) { + String cluster = System.getProperty("tests.rest." + clusterName + ".http_hosts"); + if (cluster == null) { + throw new RuntimeException( + "Must specify [tests.rest." + + clusterName + + ".http_hosts] system property with a comma delimited list of [host:port] " + + "to which to send REST requests" + ); + } + return cluster; + } + + /** + * Get a comma delimited list of [host:port] for connections between clusters. + */ + protected String getTestTransportCluster(String clusterName) { + String cluster = System.getProperty("tests.rest." + clusterName + ".transport_hosts"); + if (cluster == null) { + throw new RuntimeException( + "Must specify [tests.rest." + + clusterName + + ".transport_hosts] system property with a comma delimited list of [host:port] " + + "for connections between clusters" + ); + } + return cluster; + } + + @AfterClass + public static void closeRemoteClients() throws IOException { + try { + IOUtils.close(remoteClient, remoteAdminClient); + } finally { + remoteClient = null; + remoteAdminClient = null; + } + } + protected static void wipeAllOpenSearchIndices() throws IOException { wipeAllOpenSearchIndices(client()); if (remoteClient() != null) { @@ -150,4 +248,22 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX)); } } + + /** + * Initialize rest client to remote cluster, + * and create a connection to it from the coordinating cluster. + */ + public void configureMultiClusters() throws IOException { + initRemoteClient(); + + Request connectionRequest = new Request("PUT", "_cluster/settings"); + String connectionSetting = "{\"persistent\": {\"cluster\": {\"remote\": {\"" + + REMOTE_CLUSTER + + "\": {\"seeds\": [\"" + + getTestTransportCluster(REMOTE_CLUSTER).split(",")[0] + + "\"]}}}}}"; + connectionRequest.setJsonEntity(connectionSetting); + logger.info("Creating connection from coordinating cluster to {}", REMOTE_CLUSTER); + adminClient().performRequest(connectionRequest); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java deleted file mode 100644 index 1452217e04..0000000000 --- a/integ-test/src/test/java/org/opensearch/sql/multicluster/OpenSearchMultiClustersRestTestCase.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - - -package org.opensearch.sql.multicluster; - -import org.apache.hc.core5.http.HttpHost; -import org.opensearch.client.Request; -import org.opensearch.client.RestClient; -import org.opensearch.common.util.io.IOUtils; -import org.opensearch.test.rest.OpenSearchRestTestCase; -import org.junit.AfterClass; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -import static java.util.Collections.unmodifiableList; - -/** - * Superclass for tests that interact with multiple external test clusters using OpenSearch's {@link RestClient}. - */ -public abstract class OpenSearchMultiClustersRestTestCase extends OpenSearchRestTestCase { - - public static final String REMOTE_CLUSTER = "remoteCluster"; - public static final String MATCH_ALL_REMOTE_CLUSTER = "*"; - - private static RestClient remoteClient; - /** - * A client for the running remote OpenSearch cluster configured to take test administrative actions - * like remove all indexes after the test completes - */ - private static RestClient remoteAdminClient; - - // Modified from initClient in OpenSearchRestTestCase - public void initRemoteClient() throws IOException { - if (remoteClient == null) { - assert remoteAdminClient == null; - String cluster = getTestRestCluster(REMOTE_CLUSTER); - String[] stringUrls = cluster.split(","); - List hosts = new ArrayList<>(stringUrls.length); - for (String stringUrl : stringUrls) { - int portSeparator = stringUrl.lastIndexOf(':'); - if (portSeparator < 0) { - throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]"); - } - String host = stringUrl.substring(0, portSeparator); - int port = Integer.valueOf(stringUrl.substring(portSeparator + 1)); - hosts.add(buildHttpHost(host, port)); - } - final List clusterHosts = unmodifiableList(hosts); - logger.info("initializing REST clients against {}", clusterHosts); - remoteClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[0])); - remoteAdminClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[0])); - } - assert remoteClient != null; - assert remoteAdminClient != null; - } - - /** - * Get a comma delimited list of [host:port] to which to send REST requests. - */ - protected String getTestRestCluster(String clusterName) { - String cluster = System.getProperty("tests.rest." + clusterName + ".http_hosts"); - if (cluster == null) { - throw new RuntimeException( - "Must specify [tests.rest." - + clusterName - + ".http_hosts] system property with a comma delimited list of [host:port] " - + "to which to send REST requests" - ); - } - return cluster; - } - - /** - * Get a comma delimited list of [host:port] for connections between clusters. - */ - protected String getTestTransportCluster(String clusterName) { - String cluster = System.getProperty("tests.rest." + clusterName + ".transport_hosts"); - if (cluster == null) { - throw new RuntimeException( - "Must specify [tests.rest." - + clusterName - + ".transport_hosts] system property with a comma delimited list of [host:port] " - + "for connections between clusters" - ); - } - return cluster; - } - - /** - * Initialize rest client to remote cluster, - * and create a connection to it from the coordinating cluster. - */ - public void configureMultiClusters() throws IOException { - initRemoteClient(); - - Request connectionRequest = new Request("PUT", "_cluster/settings"); - String connectionSetting = "{\"persistent\": {\"cluster\": {\"remote\": {\"" - + REMOTE_CLUSTER - + "\": {\"seeds\": [\"" - + getTestTransportCluster(REMOTE_CLUSTER).split(",")[0] - + "\"]}}}}}"; - connectionRequest.setJsonEntity(connectionSetting); - logger.info("Creating connection from coordinating cluster to {}", REMOTE_CLUSTER); - adminClient().performRequest(connectionRequest); - } - - @AfterClass - public static void closeRemoteClients() throws IOException { - try { - IOUtils.close(remoteClient, remoteAdminClient); - } finally { - remoteClient = null; - remoteAdminClient = null; - } - } - - /** - * Get the client to remote cluster used for ordinary api calls while writing a test. - */ - protected static RestClient remoteClient() { - return remoteClient; - } - - /** - * Get the client to remote cluster used for test administrative actions. - * Do not use this while writing a test. Only use it for cleaning up after tests. - */ - protected static RestClient remoteAdminClient() { - return remoteAdminClient; - } - -} From 30971b1378e0c6b1d5eb43202fc95ac902590fe4 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Thu, 27 Apr 2023 13:37:37 -0700 Subject: [PATCH 16/17] add IT for failure case Signed-off-by: Sean Kao --- .../sql/ppl/CrossClusterSearchIT.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java index 963d23c720..a8e686a893 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java @@ -6,6 +6,7 @@ package org.opensearch.sql.ppl; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; import static org.opensearch.sql.util.MatcherUtils.columnName; @@ -15,13 +16,20 @@ import java.io.IOException; import org.json.JSONObject; +import org.junit.Rule; import org.junit.jupiter.api.Test; +import org.junit.rules.ExpectedException; +import org.opensearch.client.ResponseException; public class CrossClusterSearchIT extends PPLIntegTestCase { + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + private final static String TEST_INDEX_BANK_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_BANK; private final static String TEST_INDEX_DOG_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; private final static String TEST_INDEX_DOG_MATCH_ALL_REMOTE = MATCH_ALL_REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; + private final static String TEST_INDEX_ACCOUNT_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_ACCOUNT; @Override public void init() throws IOException { @@ -30,6 +38,7 @@ public void init() throws IOException { loadIndex(Index.BANK, remoteClient()); loadIndex(Index.DOG); loadIndex(Index.DOG, remoteClient()); + loadIndex(Index.ACCOUNT, remoteClient()); } @Test @@ -44,6 +53,15 @@ public void testMatchAllCrossClusterSearchAllFields() throws IOException { verifyColumn(result, columnName("dog_name"), columnName("holdersName"), columnName("age")); } + @Test + public void testCrossClusterSearchWithoutLocalFieldMappingShouldFail() throws IOException { + exceptionRule.expect(ResponseException.class); + exceptionRule.expectMessage("400 Bad Request"); + exceptionRule.expectMessage("IndexNotFoundException"); + + executeQuery(String.format("search source=%s", TEST_INDEX_ACCOUNT_REMOTE)); + } + @Test public void testCrossClusterSearchCommandWithLogicalExpression() throws IOException { JSONObject result = executeQuery(String.format( From 04d03eaf2eb82b5e23d4fb753f363b91ebe8ce60 Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Fri, 28 Apr 2023 09:21:46 -0700 Subject: [PATCH 17/17] remove logger info for connection in IT test case Signed-off-by: Sean Kao --- .../org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java index 61724b6100..e057c58969 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java @@ -121,7 +121,6 @@ public void initRemoteClient() throws IOException { hosts.add(buildHttpHost(host, port)); } final List clusterHosts = unmodifiableList(hosts); - logger.info("initializing REST clients against {}", clusterHosts); remoteClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[0])); remoteAdminClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[0])); } @@ -263,7 +262,6 @@ public void configureMultiClusters() throws IOException { + getTestTransportCluster(REMOTE_CLUSTER).split(",")[0] + "\"]}}}}}"; connectionRequest.setJsonEntity(connectionSetting); - logger.info("Creating connection from coordinating cluster to {}", REMOTE_CLUSTER); adminClient().performRequest(connectionRequest); } }