Percent-encode invalid flint index characters (#215) (#216)

* percent-encode invalid flint index characters Signed-off-by: Sean Kao <[email protected]> * formatting Signed-off-by: Sean Kao <[email protected]> * move encoding logic to FlintOpenSearchClient Signed-off-by: Sean Kao <[email protected]> * minor arg fix and string format fix Signed-off-by: Sean Kao <[email protected]> * fix import order Signed-off-by: Sean Kao <[email protected]> --------- Signed-off-by: Sean Kao <[email protected]> (cherry picked from commit 3528b66)
opensearch-project · Jan 9, 2024 · 3dbb4cb · 3dbb4cb
1 parent 68bcce8
commit 3dbb4cb
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 7 deletions.
diff --git a/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java b/flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java
@@ -17,6 +17,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -71,6 +72,13 @@ public class FlintOpenSearchClient implements FlintClient {
       new NamedXContentRegistry(new SearchModule(Settings.builder().build(),
           new ArrayList<>()).getNamedXContents());
 
+  /**
+   * Invalid index name characters to percent-encode,
+   * excluding '*' because it's reserved for pattern matching.
+   */
+  private final static Set<Character> INVALID_INDEX_NAME_CHARS =
+      Set.of(' ', ',', ':', '"', '+', '/', '\\', '|', '?', '#', '>', '<');
+
   /**
    * Metadata log index name prefix
    */
@@ -121,7 +129,7 @@ public void createIndex(String indexName, FlintMetadata metadata) {
 
   protected void createIndex(String indexName, String mapping, Option<String> settings) {
     LOG.info("Creating Flint index " + indexName);
-    String osIndexName = toLowercase(indexName);
+    String osIndexName = sanitizeIndexName(indexName);
     try (RestHighLevelClient client = createClient()) {
       CreateIndexRequest request = new CreateIndexRequest(osIndexName);
       request.mapping(mapping, XContentType.JSON);
@@ -137,7 +145,7 @@ protected void createIndex(String indexName, String mapping, Option<String> sett
   @Override
   public boolean exists(String indexName) {
     LOG.info("Checking if Flint index exists " + indexName);
-    String osIndexName = toLowercase(indexName);
+    String osIndexName = sanitizeIndexName(indexName);
     try (RestHighLevelClient client = createClient()) {
       return client.indices().exists(new GetIndexRequest(osIndexName), RequestOptions.DEFAULT);
     } catch (IOException e) {
@@ -148,7 +156,7 @@ public boolean exists(String indexName) {
   @Override
   public List<FlintMetadata> getAllIndexMetadata(String indexNamePattern) {
     LOG.info("Fetching all Flint index metadata for pattern " + indexNamePattern);
-    String osIndexNamePattern = toLowercase(indexNamePattern);
+    String osIndexNamePattern = sanitizeIndexName(indexNamePattern);
     try (RestHighLevelClient client = createClient()) {
       GetIndexRequest request = new GetIndexRequest(osIndexNamePattern);
       GetIndexResponse response = client.indices().get(request, RequestOptions.DEFAULT);
@@ -166,7 +174,7 @@ public List<FlintMetadata> getAllIndexMetadata(String indexNamePattern) {
   @Override
   public FlintMetadata getIndexMetadata(String indexName) {
     LOG.info("Fetching Flint index metadata for " + indexName);
-    String osIndexName = toLowercase(indexName);
+    String osIndexName = sanitizeIndexName(indexName);
     try (RestHighLevelClient client = createClient()) {
       GetIndexRequest request = new GetIndexRequest(osIndexName);
       GetIndexResponse response = client.indices().get(request, RequestOptions.DEFAULT);
@@ -182,7 +190,7 @@ public FlintMetadata getIndexMetadata(String indexName) {
   @Override
   public void deleteIndex(String indexName) {
     LOG.info("Deleting Flint index " + indexName);
-    String osIndexName = toLowercase(indexName);
+    String osIndexName = sanitizeIndexName(indexName);
     try (RestHighLevelClient client = createClient()) {
       DeleteIndexRequest request = new DeleteIndexRequest(osIndexName);
 
@@ -211,7 +219,7 @@ public FlintReader createReader(String indexName, String query) {
         queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(parser);
       }
       return new OpenSearchScrollReader(createClient(),
-          toLowercase(indexName),
+          sanitizeIndexName(indexName),
           new SearchSourceBuilder().query(queryBuilder),
           options);
     } catch (IOException e) {
@@ -221,7 +229,7 @@ public FlintReader createReader(String indexName, String query) {
 
   public FlintWriter createWriter(String indexName) {
     LOG.info("Creating Flint index writer for " + indexName);
-    return new OpenSearchWriter(createClient(), toLowercase(indexName), options.getRefreshPolicy());
+    return new OpenSearchWriter(createClient(), sanitizeIndexName(indexName), options.getRefreshPolicy());
   }
 
   @Override
@@ -287,4 +295,31 @@ private String toLowercase(String indexName) {
 
     return indexName.toLowerCase(Locale.ROOT);
   }
+
+  /*
+   * Percent-encode invalid OpenSearch index name characters.
+   */
+  private String percentEncode(String indexName) {
+    Objects.requireNonNull(indexName);
+
+    StringBuilder builder = new StringBuilder(indexName.length());
+    for (char ch : indexName.toCharArray()) {
+      if (INVALID_INDEX_NAME_CHARS.contains(ch)) {
+        builder.append(String.format("%%%02X", (int) ch));
+      } else {
+        builder.append(ch);
+      }
+    }
+    return builder.toString();
+  }
+
+  /*
+   * Sanitize index name to comply with OpenSearch index name restrictions.
+   */
+  private String sanitizeIndexName(String indexName) {
+    Objects.requireNonNull(indexName);
+
+    String encoded = percentEncode(indexName);
+    return toLowercase(encoded);
+  }
 }
diff --git a/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala b/integ-test/src/test/scala/org/opensearch/flint/core/FlintOpenSearchClientSuite.scala
@@ -114,6 +114,30 @@ class FlintOpenSearchClientSuite extends AnyFlatSpec with OpenSearchSuite with M
     flintClient.exists(indexName) shouldBe false
   }
 
+  it should "percent-encode invalid index name characters" in {
+    val indexName = "test ,:\"+/\\|?#><"
+    flintClient.createIndex(
+      indexName,
+      FlintMetadata("""{"properties": {"test": { "type": "integer" } } }"""))
+
+    flintClient.exists(indexName) shouldBe true
+    flintClient.getIndexMetadata(indexName) should not be null
+    flintClient.getAllIndexMetadata("test *") should not be empty
+
+    // Read write test
+    val writer = flintClient.createWriter(indexName)
+    writer.write("""{"create":{}}""")
+    writer.write("\n")
+    writer.write("""{"test":1}""")
+    writer.write("\n")
+    writer.flush()
+    writer.close()
+    flintClient.createReader(indexName, "").hasNext shouldBe true
+
+    flintClient.deleteIndex(indexName)
+    flintClient.exists(indexName) shouldBe false
+  }
+
   it should "return false if index not exist" in {
     flintClient.exists("non-exist-index") shouldBe false
   }