From 30586c50cc7f4f6798ba269b76f6be5c7efe2872 Mon Sep 17 00:00:00 2001 From: Louis Chu Date: Mon, 17 Feb 2025 14:52:30 -0800 Subject: [PATCH] Remove OpenDistro endpoints Signed-off-by: Louis Chu --- .../sql/legacy/SQLIntegTestCase.java | 14 ++++++ .../sql/ppl/LegacyAPICompatibilityIT.java | 46 +++++++++++-------- .../sql/sql/LegacyAPICompatibilityIT.java | 41 +++++++---------- .../sql/legacy/plugin/RestSqlAction.java | 20 ++------ .../sql/legacy/plugin/RestSqlStatsAction.java | 15 +----- .../sql/plugin/rest/RestPPLQueryAction.java | 17 ++----- .../sql/plugin/rest/RestPPLStatsAction.java | 15 +----- .../plugin/rest/RestQuerySettingsAction.java | 22 +-------- 8 files changed, 70 insertions(+), 120 deletions(-) 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 328e76013f..f59fe82fa0 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 @@ -53,6 +53,7 @@ import java.nio.file.Paths; import java.util.Locale; import java.util.Map; +import java.util.concurrent.Callable; import javax.management.MBeanServerInvocationHandler; import javax.management.ObjectName; import javax.management.remote.JMXConnector; @@ -68,6 +69,7 @@ import org.opensearch.client.Request; import org.opensearch.client.RequestOptions; import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; import org.opensearch.client.RestClient; import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.datasource.model.DataSourceMetadata; @@ -252,6 +254,18 @@ protected Request getSqlCursorCloseRequest(String cursorRequest) { return sqlRequest; } + protected void assertBadRequest(Callable operation) { + try { + operation.call(); + Assert.fail("Expected ResponseException was not thrown"); + } catch (ResponseException e) { + Assert.assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); + Assert.assertEquals("Bad Request", e.getResponse().getStatusLine().getReasonPhrase()); + } catch (Exception e) { + Assert.fail("Unexpected exception: " + e.getMessage()); + } + } + protected String executeQuery(String query, String requestType) { return executeQuery(query, requestType, Map.of()); } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/LegacyAPICompatibilityIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/LegacyAPICompatibilityIT.java index c14b9baa35..f119efbe70 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/LegacyAPICompatibilityIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/LegacyAPICompatibilityIT.java @@ -4,9 +4,6 @@ */ package org.opensearch.sql.ppl; -import static org.opensearch.sql.plugin.rest.RestPPLQueryAction.LEGACY_EXPLAIN_API_ENDPOINT; -import static org.opensearch.sql.plugin.rest.RestPPLQueryAction.LEGACY_QUERY_API_ENDPOINT; -import static org.opensearch.sql.plugin.rest.RestPPLStatsAction.PPL_LEGACY_STATS_API_ENDPOINT; import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.SETTINGS_API_ENDPOINT; import java.io.IOException; @@ -18,6 +15,10 @@ /** For backward compatibility, check if legacy API endpoints are accessible. */ public class LegacyAPICompatibilityIT extends PPLIntegTestCase { + public static final String LEGACY_PPL_API_ENDPOINT = "/_opendistro/_ppl"; + public static final String LEGACY_PPL_EXPLAIN_API_ENDPOINT = "/_opendistro/_ppl/_explain"; + public static final String LEGACY_PPL_SETTINGS_API_ENDPOINT = "/_opendistro/_ppl/settings"; + public static final String LEGACY_PPL_STATS_API_ENDPOINT = "/_opendistro/_ppl/stats"; @Override public void init() throws IOException { @@ -25,42 +26,49 @@ public void init() throws IOException { } @Test - public void query() throws IOException { + public void query() { String query = "source=opensearch-sql_test_index_account | where age > 30"; - Request request = buildRequest(query, LEGACY_QUERY_API_ENDPOINT); - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + Request request = buildRequest(query, LEGACY_PPL_API_ENDPOINT); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void explain() throws IOException { + public void explain() { String query = "source=opensearch-sql_test_index_account | where age > 30"; - Request request = buildRequest(query, LEGACY_EXPLAIN_API_ENDPOINT); - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + Request request = buildRequest(query, LEGACY_PPL_EXPLAIN_API_ENDPOINT); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void stats() throws IOException { - Request request = new Request("GET", PPL_LEGACY_STATS_API_ENDPOINT); - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + public void stats() { + Request request = new Request("GET", LEGACY_PPL_STATS_API_ENDPOINT); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void legacySettingNewEndpoint() throws IOException { + public void legacyPPLSettingNewEndpoint() { String requestBody = "{" + " \"persistent\": {" + " \"opendistro.ppl.query.memory_limit\": \"80%\"" + " }" + "}"; - Response response = updateSetting(SETTINGS_API_ENDPOINT, requestBody); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> updateSetting(SETTINGS_API_ENDPOINT, requestBody)); + } + + @Test + public void newPPLSettingsLegacyEndpoint() { + String requestBody = + "{" + + " \"persistent\": {" + + " \"plugins.ppl.query.memory_limit\": \"90%\"" + + " }" + + "}"; + assertBadRequest(() -> updateSetting(LEGACY_PPL_SETTINGS_API_ENDPOINT, requestBody)); } @Test - public void newSettingNewEndpoint() throws IOException { + public void newPPLSettingNewEndpoint() throws IOException { String requestBody = "{" + " \"persistent\": {" + " \"plugins.query.size_limit\": \"100\"" + " }" + "}"; Response response = updateSetting(SETTINGS_API_ENDPOINT, requestBody); diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java index e76a13822b..155d9002ae 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/LegacyAPICompatibilityIT.java @@ -5,13 +5,7 @@ package org.opensearch.sql.sql; -import static org.hamcrest.Matchers.equalTo; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; -import static org.opensearch.sql.legacy.plugin.RestSqlAction.LEGACY_CURSOR_CLOSE_ENDPOINT; -import static org.opensearch.sql.legacy.plugin.RestSqlAction.LEGACY_EXPLAIN_API_ENDPOINT; -import static org.opensearch.sql.legacy.plugin.RestSqlAction.LEGACY_QUERY_API_ENDPOINT; -import static org.opensearch.sql.legacy.plugin.RestSqlStatsAction.LEGACY_STATS_API_ENDPOINT; -import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.LEGACY_SQL_SETTINGS_API_ENDPOINT; import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.SETTINGS_API_ENDPOINT; import java.io.IOException; @@ -27,29 +21,32 @@ /** For backward compatibility, check if legacy API endpoints are accessible. */ public class LegacyAPICompatibilityIT extends SQLIntegTestCase { + public static final String LEGACY_QUERY_API_ENDPOINT = "/_opendistro/_sql"; + public static final String LEGACY_CURSOR_CLOSE_ENDPOINT = LEGACY_QUERY_API_ENDPOINT + "/close"; + public static final String LEGACY_EXPLAIN_API_ENDPOINT = LEGACY_QUERY_API_ENDPOINT + "/_explain"; + public static final String LEGACY_SQL_SETTINGS_API_ENDPOINT = + LEGACY_QUERY_API_ENDPOINT + "/settings"; + public static final String LEGACY_STATS_API_ENDPOINT = LEGACY_QUERY_API_ENDPOINT + "/stats"; + @Override protected void init() throws Exception { loadIndex(Index.ACCOUNT); } @Test - public void query() throws IOException { + public void query() { String requestBody = makeRequest("SELECT 1"); Request request = new Request("POST", LEGACY_QUERY_API_ENDPOINT); request.setJsonEntity(requestBody); - - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void explain() throws IOException { + public void explain() { String requestBody = makeRequest("SELECT 1"); Request request = new Request("POST", LEGACY_EXPLAIN_API_ENDPOINT); request.setJsonEntity(requestBody); - - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> client().performRequest(request)); } @Test @@ -61,31 +58,27 @@ public void closeCursor() throws IOException { Request request = new Request("POST", LEGACY_CURSOR_CLOSE_ENDPOINT); request.setJsonEntity(makeCursorRequest(result.getString("cursor"))); request.setOptions(buildJsonOption()); - JSONObject response = new JSONObject(executeRequest(request)); - assertThat(response.getBoolean("succeeded"), equalTo(true)); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void stats() throws IOException { + public void stats() { Request request = new Request("GET", LEGACY_STATS_API_ENDPOINT); - Response response = client().performRequest(request); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> client().performRequest(request)); } @Test - public void legacySettingNewEndpoint() throws IOException { + public void legacySettingNewEndpoint() { String requestBody = "{" + " \"persistent\": {" + " \"opendistro.query.size_limit\": \"100\"" + " }" + "}"; - Response response = updateSetting(SETTINGS_API_ENDPOINT, requestBody); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> updateSetting(SETTINGS_API_ENDPOINT, requestBody)); } @Test public void newSettingsLegacyEndpoint() throws IOException { String requestBody = "{" + " \"persistent\": {" + " \"plugins.sql.slowlog\": \"10\"" + " }" + "}"; - Response response = updateSetting(LEGACY_SQL_SETTINGS_API_ENDPOINT, requestBody); - Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + assertBadRequest(() -> updateSetting(LEGACY_SQL_SETTINGS_API_ENDPOINT, requestBody)); } @Test diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index c47e5f05bd..31a28eb943 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -79,9 +79,6 @@ public class RestSqlAction extends BaseRestHandler { public static final String EXPLAIN_API_ENDPOINT = QUERY_API_ENDPOINT + "/_explain"; public static final String CURSOR_CLOSE_ENDPOINT = QUERY_API_ENDPOINT + "/close"; - public static final String LEGACY_QUERY_API_ENDPOINT = "/_opendistro/_sql"; - public static final String LEGACY_EXPLAIN_API_ENDPOINT = LEGACY_QUERY_API_ENDPOINT + "/_explain"; - public static final String LEGACY_CURSOR_CLOSE_ENDPOINT = LEGACY_QUERY_API_ENDPOINT + "/close"; /** New SQL query request handler. */ private final RestSQLQueryAction newSqlQueryHandler; @@ -94,21 +91,10 @@ public RestSqlAction(Settings settings, Injector injector) { @Override public List routes() { - return ImmutableList.of(); - } - - @Override - public List replacedRoutes() { return ImmutableList.of( - new ReplacedRoute( - RestRequest.Method.POST, QUERY_API_ENDPOINT, - RestRequest.Method.POST, LEGACY_QUERY_API_ENDPOINT), - new ReplacedRoute( - RestRequest.Method.POST, EXPLAIN_API_ENDPOINT, - RestRequest.Method.POST, LEGACY_EXPLAIN_API_ENDPOINT), - new ReplacedRoute( - RestRequest.Method.POST, CURSOR_CLOSE_ENDPOINT, - RestRequest.Method.POST, LEGACY_CURSOR_CLOSE_ENDPOINT)); + new Route(RestRequest.Method.POST, QUERY_API_ENDPOINT), + new Route(RestRequest.Method.POST, EXPLAIN_API_ENDPOINT), + new Route(RestRequest.Method.POST, CURSOR_CLOSE_ENDPOINT)); } @Override diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java index 6f9d1e4117..493115a726 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlStatsAction.java @@ -39,8 +39,6 @@ public class RestSqlStatsAction extends BaseRestHandler { /** API endpoint path */ public static final String STATS_API_ENDPOINT = "/_plugins/_sql/stats"; - public static final String LEGACY_STATS_API_ENDPOINT = "/_opendistro/_sql/stats"; - public RestSqlStatsAction(Settings settings, RestController restController) { super(); } @@ -52,18 +50,9 @@ public String getName() { @Override public List routes() { - return ImmutableList.of(); - } - - @Override - public List replacedRoutes() { return ImmutableList.of( - new ReplacedRoute( - RestRequest.Method.POST, STATS_API_ENDPOINT, - RestRequest.Method.POST, LEGACY_STATS_API_ENDPOINT), - new ReplacedRoute( - RestRequest.Method.GET, STATS_API_ENDPOINT, - RestRequest.Method.GET, LEGACY_STATS_API_ENDPOINT)); + new Route(RestRequest.Method.POST, STATS_API_ENDPOINT), + new Route(RestRequest.Method.GET, STATS_API_ENDPOINT)); } @Override diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java index 7e6d3c1422..4663d79c86 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java @@ -41,8 +41,6 @@ public class RestPPLQueryAction extends BaseRestHandler { public static final String QUERY_API_ENDPOINT = "/_plugins/_ppl"; public static final String EXPLAIN_API_ENDPOINT = "/_plugins/_ppl/_explain"; - public static final String LEGACY_QUERY_API_ENDPOINT = "/_opendistro/_ppl"; - public static final String LEGACY_EXPLAIN_API_ENDPOINT = "/_opendistro/_ppl/_explain"; private static final Logger LOG = LogManager.getLogger(); @@ -66,18 +64,9 @@ private static boolean isClientError(Exception e) { @Override public List routes() { - return ImmutableList.of(); - } - - @Override - public List replacedRoutes() { - return Arrays.asList( - new ReplacedRoute( - RestRequest.Method.POST, QUERY_API_ENDPOINT, - RestRequest.Method.POST, LEGACY_QUERY_API_ENDPOINT), - new ReplacedRoute( - RestRequest.Method.POST, EXPLAIN_API_ENDPOINT, - RestRequest.Method.POST, LEGACY_EXPLAIN_API_ENDPOINT)); + return ImmutableList.of( + new Route(RestRequest.Method.POST, QUERY_API_ENDPOINT), + new Route(RestRequest.Method.POST, EXPLAIN_API_ENDPOINT)); } @Override diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java index d3d7074b20..feeb0713a2 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLStatsAction.java @@ -34,8 +34,6 @@ public class RestPPLStatsAction extends BaseRestHandler { /** API endpoint path. */ public static final String PPL_STATS_API_ENDPOINT = "/_plugins/_ppl/stats"; - public static final String PPL_LEGACY_STATS_API_ENDPOINT = "/_opendistro/_ppl/stats"; - public RestPPLStatsAction(Settings settings, RestController restController) { super(); } @@ -47,18 +45,9 @@ public String getName() { @Override public List routes() { - return ImmutableList.of(); - } - - @Override - public List replacedRoutes() { return ImmutableList.of( - new ReplacedRoute( - RestRequest.Method.POST, PPL_STATS_API_ENDPOINT, - RestRequest.Method.POST, PPL_LEGACY_STATS_API_ENDPOINT), - new ReplacedRoute( - RestRequest.Method.GET, PPL_STATS_API_ENDPOINT, - RestRequest.Method.GET, PPL_LEGACY_STATS_API_ENDPOINT)); + new Route(RestRequest.Method.POST, PPL_STATS_API_ENDPOINT), + new Route(RestRequest.Method.GET, PPL_STATS_API_ENDPOINT)); } @Override diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java index f9080051b4..8fd6e6d5ce 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java @@ -36,25 +36,15 @@ public class RestQuerySettingsAction extends BaseRestHandler { private static final String SQL_SETTINGS_PREFIX = "plugins.sql."; private static final String PPL_SETTINGS_PREFIX = "plugins.ppl."; private static final String COMMON_SETTINGS_PREFIX = "plugins.query."; - private static final String LEGACY_SQL_SETTINGS_PREFIX = "opendistro.sql."; - private static final String LEGACY_PPL_SETTINGS_PREFIX = "opendistro.ppl."; - private static final String LEGACY_COMMON_SETTINGS_PREFIX = "opendistro.query."; private static final String EXECUTION_ENGINE_SETTINGS_PREFIX = "plugins.query.executionengine"; public static final String DATASOURCES_SETTINGS_PREFIX = "plugins.query.datasources"; private static final List SETTINGS_PREFIX = - ImmutableList.of( - SQL_SETTINGS_PREFIX, - PPL_SETTINGS_PREFIX, - COMMON_SETTINGS_PREFIX, - LEGACY_SQL_SETTINGS_PREFIX, - LEGACY_PPL_SETTINGS_PREFIX, - LEGACY_COMMON_SETTINGS_PREFIX); + ImmutableList.of(SQL_SETTINGS_PREFIX, PPL_SETTINGS_PREFIX, COMMON_SETTINGS_PREFIX); private static final List DENY_LIST_SETTINGS_PREFIX = ImmutableList.of(EXECUTION_ENGINE_SETTINGS_PREFIX, DATASOURCES_SETTINGS_PREFIX); public static final String SETTINGS_API_ENDPOINT = "/_plugins/_query/settings"; - public static final String LEGACY_SQL_SETTINGS_API_ENDPOINT = "/_opendistro/_sql/settings"; public RestQuerySettingsAction(Settings settings, RestController restController) { super(); @@ -67,15 +57,7 @@ public String getName() { @Override public List routes() { - return ImmutableList.of(); - } - - @Override - public List replacedRoutes() { - return ImmutableList.of( - new ReplacedRoute( - RestRequest.Method.PUT, SETTINGS_API_ENDPOINT, - RestRequest.Method.PUT, LEGACY_SQL_SETTINGS_API_ENDPOINT)); + return ImmutableList.of(new Route(RestRequest.Method.PUT, SETTINGS_API_ENDPOINT)); } @SuppressWarnings("unchecked")