Remove opendistro settings and endpoints (#3326)

* Remove opendistro settings

Signed-off-by: Louis Chu <clingzhi@amazon.com>

* Remove OpenDistro endpoints

Signed-off-by: Louis Chu <clingzhi@amazon.com>

* Fix bwc IT gracefully

Signed-off-by: Louis Chu <clingzhi@amazon.com>

---------

Signed-off-by: Louis Chu <clingzhi@amazon.com>

Author: noCharger

common/src/main/java/org/opensearch/sql/common/setting/LegacySettings.java

@@ -16,36 +16,6 @@ Introduction
 
 When OpenSearch bootstraps, SQL plugin will register a few settings in OpenSearch cluster settings. Most of the settings are able to change dynamically so you can control the behavior of SQL plugin without need to bounce your cluster. You can update the settings by sending requests to either ``_cluster/settings`` or ``_plugins/_query/settings`` endpoint, though the examples are sending to the latter.
 
-Breaking Change
-===============
-opendistro.sql.engine.new.enabled
----------------------------------
-The opendistro.sql.engine.new.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the new engine is always enabled.
-
-opendistro.sql.query.analysis.enabled
--------------------------------------
-The opendistro.sql.query.analysis.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis in legacy engine is disabled.
-
-opendistro.sql.query.analysis.semantic.suggestion
--------------------------------------------------
-The opendistro.sql.query.analysis.semantic.suggestion setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis suggestion in legacy engine is disabled.
-
-opendistro.sql.query.analysis.semantic.threshold
-------------------------------------------------
-The opendistro.sql.query.analysis.semantic.threshold setting is deprecated and will be removed then. From OpenSearch 1.0, the query analysis threshold in legacy engine is disabled.
-
-opendistro.sql.query.response.format
-------------------------------------
-The opendistro.sql.query.response.format setting is deprecated and will be removed then. From OpenSearch 1.0, the query response format is default to JDBC format. `You can change the format by using query parameters<../interfaces/protocol.rst>`_.
-
-opendistro.sql.cursor.enabled
------------------------------
-The opendistro.sql.cursor.enabled setting is deprecated and will be removed then. From OpenSearch 1.0, the cursor feature is enabled by default.
-
-opendistro.sql.cursor.fetch_size
---------------------------------
-The opendistro.sql.cursor.fetch_size setting is deprecated and will be removed then. From OpenSearch 1.0, the fetch_size in query body will decide whether create the cursor context. No cursor will be created if the fetch_size = 0.
-
 plugins.sql.enabled
 ======================
 
@@ -86,8 +56,6 @@ Result set::
 	  }
 	}
 
-Note: the legacy settings of ``opendistro.sql.enabled`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.
-
 Example 2
 ---------
 
@@ -150,8 +118,6 @@ Result set::
 	  }
 	}
 
-Note: the legacy settings of ``opendistro.sql.slowlog`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.
-
 plugins.sql.cursor.keep_alive
 ================================
 
@@ -194,8 +160,6 @@ Result set::
 	  }
 	}
 
-Note: the legacy settings of ``opendistro.sql.cursor.keep_alive`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.
-
 plugins.sql.pagination.api
 ================================
 
@@ -268,8 +232,6 @@ Result set::
       }
     }
 
-Note: the legacy settings of ``opendistro.query.size_limit`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.
-
 plugins.query.memory_limit
 ==========================
 
@@ -298,9 +260,6 @@ Result set::
       "transient": {}
     }
 
-Note: the legacy settings of ``opendistro.ppl.query.memory_limit`` is deprecated, it will fallback to the new settings if you request an update with the legacy name.
-
-
 plugins.sql.delete.enabled
 ======================
 

docs/user/admin/settings.rst

@@ -8,9 +8,9 @@
 import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient;
 import static org.opensearch.sql.legacy.TestUtils.isIndexExist;
 import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient;
-import static org.opensearch.sql.legacy.plugin.RestSqlAction.LEGACY_QUERY_API_ENDPOINT;
 import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
-import static org.opensearch.sql.plugin.rest.RestQuerySettingsAction.LEGACY_SQL_SETTINGS_API_ENDPOINT;
+import static org.opensearch.sql.sql.LegacyAPICompatibilityIT.LEGACY_QUERY_API_ENDPOINT;
+import static org.opensearch.sql.sql.LegacyAPICompatibilityIT.LEGACY_SQL_SETTINGS_API_ENDPOINT;
 import static org.opensearch.sql.util.MatcherUtils.rows;
 import static org.opensearch.sql.util.MatcherUtils.schema;
 import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -93,21 +93,33 @@ public void testBackwardsCompatibility() throws Exception {
       List<Map<String, Object>> plugins = (List<Map<String, Object>>) response.get("plugins");
       Set<Object> pluginNames =
           plugins.stream().map(map -> map.get("name")).collect(Collectors.toSet());
+      String version = (String) response.get("version");
+
+      boolean isBackwardsIncompatibleVersion = version.startsWith("2.");
+
       switch (CLUSTER_TYPE) {
         case OLD:
           Assert.assertTrue(pluginNames.contains("opensearch-sql"));
-          updateLegacySQLSettings();
+          if (isBackwardsIncompatibleVersion) {
+            updateLegacySQLSettings();
+          }
           loadIndex(Index.ACCOUNT);
-          verifySQLQueries(LEGACY_QUERY_API_ENDPOINT);
+          verifySQLQueries(
+              isBackwardsIncompatibleVersion ? LEGACY_QUERY_API_ENDPOINT : QUERY_API_ENDPOINT);
           break;
         case MIXED:
           Assert.assertTrue(pluginNames.contains("opensearch-sql"));
-          verifySQLSettings();
-          verifySQLQueries(LEGACY_QUERY_API_ENDPOINT);
+          if (isBackwardsIncompatibleVersion) {
+            verifySQLSettings();
+          } else {
+            // For upgraded nodes, we don't need to verify legacy settings
+          }
+          verifySQLQueries(
+              isBackwardsIncompatibleVersion ? LEGACY_QUERY_API_ENDPOINT : QUERY_API_ENDPOINT);
           break;
         case UPGRADED:
           Assert.assertTrue(pluginNames.contains("opensearch-sql"));
-          verifySQLSettings();
+          // For fully upgraded clusters, we don't need to verify legacy settings
           verifySQLQueries(QUERY_API_ENDPOINT);
           break;
       }

integ-test/src/test/java/org/opensearch/sql/bwc/SQLBackwardsCompatibilityIT.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<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());
   }

integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.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,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);