Skip to content

Commit

Permalink
Remove OpenDistro endpoints
Browse files Browse the repository at this point in the history
Signed-off-by: Louis Chu <[email protected]>
  • Loading branch information
noCharger committed Feb 18, 2025
1 parent 50239aa commit 30586c5
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -252,6 +254,18 @@ protected Request getSqlCursorCloseRequest(String cursorRequest) {
return sqlRequest;
}

protected void assertBadRequest(Callable<Response> 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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,49 +15,60 @@

/** 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 {
loadIndex(Index.ACCOUNT);
}

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -94,21 +91,10 @@ public RestSqlAction(Settings settings, Injector injector) {

@Override
public List<Route> routes() {
return ImmutableList.of();
}

@Override
public List<ReplacedRoute> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -52,18 +50,9 @@ public String getName() {

@Override
public List<Route> routes() {
return ImmutableList.of();
}

@Override
public List<ReplacedRoute> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -66,18 +64,9 @@ private static boolean isClientError(Exception e) {

@Override
public List<Route> routes() {
return ImmutableList.of();
}

@Override
public List<ReplacedRoute> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -47,18 +45,9 @@ public String getName() {

@Override
public List<Route> routes() {
return ImmutableList.of();
}

@Override
public List<ReplacedRoute> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String> 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();
Expand All @@ -67,15 +57,7 @@ public String getName() {

@Override
public List<Route> routes() {
return ImmutableList.of();
}

@Override
public List<ReplacedRoute> 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")
Expand Down

0 comments on commit 30586c5

Please sign in to comment.