SQL: Fix show tables as admin (elastic/x-pack-elasticsearch#3597)

Fix show tables as JDBC with security enabled
This commit fixes the test SqlSecurityTestCase.testShowTablesWorksAsAdmin to
skip over any indices/aliases that exist starting with `.security`.
Use value comparison instead of the result sets
Fix an offset bug while at it (columns start at 1 not 0)
Resolves elastic/x-pack-elasticsearch#3423

Original commit: elastic/x-pack-elasticsearch@6fffda6070

qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/CliSecurityIT.java

@@ -9,6 +9,7 @@ import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.xpack.qa.sql.cli.RemoteCli;
 import org.elasticsearch.xpack.qa.sql.cli.RemoteCli.SecurityConfig;
+
 import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -128,10 +129,15 @@ public class CliSecurityIT extends SqlSecurityTestCase {
         @Override
         public void expectShowTables(List<String> tables, String user) throws Exception {
             try (RemoteCli cli = new RemoteCli(elasticsearchAddress(), true, userSecurity(user))) {
-                assertThat(cli.command("SHOW TABLES"), containsString("table"));
-                assertEquals("---------------", cli.readLine());
+                String tablesOutput = cli.command("SHOW TABLES");
+                assertThat(tablesOutput, containsString("name"));
+                assertThat(tablesOutput, containsString("type"));
+                assertEquals("---------------+---------------", cli.readLine());
                 for (String table : tables) {
-                    assertThat(cli.readLine(), containsString(table));
+                    String line = cli.readLine();
+                    if (!line.startsWith(".security")) {
+                        assertThat(line, containsString(table));
+                    }
                 }
                 assertEquals("", cli.readLine());
             }

qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcSecurityIT.java

@@ -10,6 +10,7 @@ import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.xpack.qa.sql.jdbc.LocalH2;
+
 import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -18,6 +19,7 @@ import java.sql.DriverManager;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -179,25 +181,22 @@ public class JdbcSecurityIT extends SqlSecurityTestCase {
 
         @Override
         public void expectShowTables(List<String> tables, String user) throws Exception {
-            try (Connection h2 = LocalH2.anonymousDb();
-                    Connection es = es(userProperties(user))) {
-                // h2 doesn't spit out the same columns we do so we emulate
-                h2.createStatement().executeUpdate("CREATE TABLE mock (table VARCHAR)");
-                StringBuilder insert = new StringBuilder();
-                insert.append("INSERT INTO mock (table) VALUES ");
-                boolean first = true;
-                for (String table : tables) {
-                    if (first) {
-                        first = false;
-                    } else {
-                        insert.append(", ");
-                    }
-                    insert.append("('").append(table).append("')");
-                }
-                h2.createStatement().executeUpdate(insert.toString());
+            try (Connection es = es(userProperties(user))) {
+                ResultSet actual = es.createStatement().executeQuery("SHOW TABLES");
 
-                ResultSet expected = h2.createStatement().executeQuery("SELECT * FROM mock ORDER BY table");
-                assertResultSets(expected, es.createStatement().executeQuery("SHOW TABLES"));
+                // depending on whether security is enabled and a test is run in isolation or suite
+                // .security or .security6 index can appear
+                // to filter these out, the result set is flatten to a list
+                List<String> actualList = new ArrayList<>();
+
+                while (actual.next()) {
+                    String name = actual.getString("name");
+                    if (!name.startsWith(".security")) {
+                        actualList.add(name);
+                    }
+                }
+
+                assertEquals(tables, actualList);
             }
         }
 

qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/RestSqlSecurityIT.java

@@ -27,6 +27,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
+import java.util.stream.Collectors;
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.xpack.qa.sql.rest.RestSqlTestCase.columnInfo;
 import static java.util.Collections.singletonList;
@@ -106,14 +107,27 @@ public class RestSqlSecurityIT extends SqlSecurityTestCase {
         @Override
         public void expectShowTables(List<String> tables, String user) throws Exception {
             String mode = randomMode();
+            List<Object> columns = new ArrayList<>();
+            columns.add(columnInfo(mode, "name", "keyword", JDBCType.VARCHAR, 0));
+            columns.add(columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 0));
             Map<String, Object> expected = new HashMap<>();
-            expected.put("columns", singletonList(columnInfo(mode, "table", "keyword", JDBCType.VARCHAR, 0)));
+            expected.put("columns", columns);
             List<List<String>> rows = new ArrayList<>();
             for (String table : tables) {
-                rows.add(singletonList(table));
+                List<String> fields = new ArrayList<>();
+                fields.add(table);
+                fields.add("INDEX");
+                rows.add(fields);
             }
             expected.put("rows", rows);
-            assertResponse(expected, runSql(user, mode, "SHOW TABLES"));
+
+            Map<String, Object> actual = runSql(user, mode, "SHOW TABLES");
+            List<List<String>> rowsNoSecurity = ((List<List<String>>) actual.get("rows"))
+                    .stream()
+                    .filter(ls -> ls.get(0).startsWith(".security") == false)
+                    .collect(Collectors.toList());
+            actual.put("rows", rowsNoSecurity);
+            assertResponse(expected, actual);
         }
 
         @Override

qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java

@@ -334,9 +334,8 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
             .assertLogs();
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/3423")
     public void testShowTablesWorksAsAdmin() throws Exception {
-        actions.expectShowTables(Arrays.asList(".security-6", "bort", "test"), null);
+        actions.expectShowTables(Arrays.asList("bort", "test"), null);
         new AuditLogAsserter()
             .expectSqlCompositeAction("test_admin", "bort", "test")
             .assertLogs();

sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java

@@ -64,7 +64,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
 
         List<ColumnInfo> columns = cursor.columns();
         for (int i = 0; i < columns.size(); i++) {
-            nameToIndex.put(columns.get(i).name, Integer.valueOf(i));
+            nameToIndex.put(columns.get(i).name, Integer.valueOf(i + 1));
         }
     }