From ad6b374fd3293edb0992f4660d57efb233f1b7b1 Mon Sep 17 00:00:00 2001
From: Megha Goyal <goyamegh@amazon.com>
Date: Thu, 30 Nov 2023 09:29:07 -0800
Subject: [PATCH 1/3] #709 Return empty response for empty mappings and no
 applied aliases (#724)

* #709 Return empty response for empty mappings and no applied aliases

Signed-off-by: Megha Goyal <goyamegh@amazon.com>

* Adding integ tests for empty mappings/aliases use-cases

Signed-off-by: Megha Goyal <goyamegh@amazon.com>

* Fix unit tests for MappingsTraverser

Signed-off-by: Megha Goyal <goyamegh@amazon.com>

---------

Signed-off-by: Megha Goyal <goyamegh@amazon.com>
(cherry picked from commit c0f7bd934b79f7c3a911229fa63e157116ec5ca9)
---
 .../mapper/MapperService.java                 | 19 ++++++---
 .../mapper/MappingsTraverser.java             | 17 ++++++--
 .../mapper/MapperRestApiIT.java               | 41 ++++++++++++++-----
 .../mapper/MappingsTraverserTests.java        | 37 +++++++++++++----
 4 files changed, 85 insertions(+), 29 deletions(-)

diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
index 0f55daecd..6d42cb5db 100644
--- a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
+++ b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
@@ -290,13 +290,20 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                                 }
                             }
                         }
-                    }
 
-                    if (appliedAliases.size() == 0) {
-                        actionListener.onFailure(SecurityAnalyticsException.wrap(
-                                new OpenSearchStatusException("No applied aliases not found", RestStatus.NOT_FOUND))
-                        );
-                        return;
+                        // Traverse mappings and do copy with excluded type=alias properties
+                        MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
+                        // Resulting mapping after filtering
+                        Map<String, Object> filteredMapping = mappingsTraverser.traverseAndCopyWithFilter(appliedAliases);
+
+
+                        // Construct filtered mappings and return them as result
+                        Map<String, MappingMetadata> outIndexMappings = new HashMap<>();
+                        Map<String, Object> root = Map.of(org.opensearch.index.mapper.MapperService.SINGLE_MAPPING_NAME, filteredMapping);
+                        MappingMetadata outMappingMetadata = new MappingMetadata(org.opensearch.index.mapper.MapperService.SINGLE_MAPPING_NAME, root);
+                        outIndexMappings.put(indexName, outMappingMetadata);
+
+                        actionListener.onResponse(new GetIndexMappingsResponse(outIndexMappings));
                     }
 
                     // Traverse mappings and do copy with excluded type=alias properties
diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
index 16335894b..e6f82eb47 100644
--- a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
+++ b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
@@ -5,23 +5,26 @@
 
 package org.opensearch.securityanalytics.mapper;
 
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.ListIterator;
 import org.apache.commons.lang3.tuple.Pair;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.opensearch.cluster.metadata.MappingMetadata;
 import org.opensearch.common.xcontent.DeprecationHandler;
 import org.opensearch.common.xcontent.NamedXContentRegistry;
 import org.opensearch.common.xcontent.XContentParser;
 import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.index.mapper.MapperService;
-import org.opensearch.securityanalytics.rules.condition.ConditionListener;
+import org.opensearch.core.xcontent.DeprecationHandler;
+import org.opensearch.core.xcontent.NamedXContentRegistry;
+import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.HashMap;
@@ -39,6 +42,8 @@
  */
 public class MappingsTraverser {
 
+    private static final Logger log = LogManager.getLogger(MappingsTraverser.class);
+
     /**
      * Traverser listener used to process leaves
      */
@@ -157,7 +162,10 @@ public void traverse() {
         try {
 
             Map<String, Object> rootProperties = (Map<String, Object>) this.mappingsMap.get(PROPERTIES);
-            rootProperties.forEach((k, v) -> nodeStack.push(new Node(Map.of(k, v), null, rootProperties, "", "")));
+
+            if (Objects.nonNull(rootProperties)) {
+                rootProperties.forEach((k, v) -> nodeStack.push(new Node(Map.of(k, v), null, rootProperties, "", "")));
+            }
 
             while (nodeStack.size() > 0) {
                 Node node = nodeStack.pop();
@@ -193,6 +201,7 @@ public void traverse() {
             // This is coming from listeners.
             throw e;
         } catch (Exception e) {
+            log.error("Error traversing mappings tree", e);
             notifyError("Error traversing mappings tree");
         }
     }
diff --git a/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java
index 9e7337a92..992166bb2 100644
--- a/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java
+++ b/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java
@@ -60,6 +60,27 @@ public void testGetMappingSuccess() throws IOException {
         assertTrue(respMap.containsKey(testIndexPattern));
     }
 
+    // Tests the case when the mappings map is empty
+    public void testGetMappings_emptyIndex_Success() throws IOException {
+        String testIndexName1 = "my_index_1";
+        String testIndexName2 = "my_index_2";
+        String testIndexPattern = "my_index*";
+
+        createSampleIndex(testIndexName1);
+        createSampleIndex(testIndexName2);
+
+        Request request = new Request("GET", MAPPER_BASE_URI + "?index_name=" + testIndexPattern);
+        Response response = client().performRequest(request);
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+        Map<String, Object> respMap = (Map<String, Object>) responseAsMap(response);
+        Map<String, Object> props = (Map<String, Object>)((Map<String, Object>) respMap.get(testIndexPattern)).get("mappings");
+
+        // Assert that indexName returned is one passed by user
+        assertTrue(respMap.containsKey(testIndexPattern));
+        //Assert that mappings map is also present in the output
+        assertTrue(props.containsKey("properties"));
+    }
+
     public void testGetMappingSuccess_1() throws IOException {
         String testIndexName1 = "my_index_1";
         String testIndexPattern = "my_index*";
@@ -227,13 +248,14 @@ public void testUpdateAndGetMappingSuccess() throws IOException {
         assertEquals(1L, searchResponse.getHits().getTotalHits().value);
     }
 
+    // Tests the case when alias mappings are not present on the index
     public void testUpdateAndGetMapping_notFound_Success() throws IOException {
 
         String testIndexName = "my_index";
 
         createSampleIndex(testIndexName);
 
-        // Execute UpdateMappingsAction to add alias mapping for index
+        // Execute UpdateMappingsAction to add other settings except alias mapping for index
         Request updateRequest = new Request("PUT", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
         // both req params and req body are supported
         updateRequest.setJsonEntity(
@@ -241,21 +263,20 @@ public void testUpdateAndGetMapping_notFound_Success() throws IOException {
                         "  \"field\":\"netflow.source_transport_port\","+
                         "  \"alias\":\"\\u0000\" }"
         );
-        // request.addParameter("indexName", testIndexName);
-        // request.addParameter("ruleTopic", "netflow");
+
         Response response = client().performRequest(updateRequest);
         assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
 
         // Execute GetIndexMappingsAction and verify mappings
         Request getRequest = new Request("GET", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
         getRequest.addParameter("index_name", testIndexName);
-        try {
-            client().performRequest(getRequest);
-            fail();
-        } catch (ResponseException e) {
-            assertEquals(HttpStatus.SC_NOT_FOUND, e.getResponse().getStatusLine().getStatusCode());
-            assertTrue(e.getMessage().contains("No applied aliases not found"));
-        }
+        response = client().performRequest(getRequest);
+        XContentParser parser = createParser(JsonXContent.jsonXContent, new String(response.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8));
+        assertTrue(
+                (((Map)((Map)parser.map()
+                        .get(testIndexName))
+                        .get("mappings"))
+                        .containsKey("properties")));
     }
 
     public void testExistingMappingsAreUntouched() throws IOException {
diff --git a/src/test/java/org/opensearch/securityanalytics/mapper/MappingsTraverserTests.java b/src/test/java/org/opensearch/securityanalytics/mapper/MappingsTraverserTests.java
index 1c71df93c..c5d7d077c 100644
--- a/src/test/java/org/opensearch/securityanalytics/mapper/MappingsTraverserTests.java
+++ b/src/test/java/org/opensearch/securityanalytics/mapper/MappingsTraverserTests.java
@@ -19,10 +19,6 @@
 
 public class MappingsTraverserTests extends OpenSearchTestCase {
 
-
-
-
-
     public void testTraverseValidMappings() {
         // 1. Parse mappings from MappingMetadata
         ImmutableOpenMap.Builder<String, MappingMetadata> mappings = ImmutableOpenMap.builder();
@@ -136,30 +132,53 @@ public void onError(String error) {
 
     public void testTraverseInvalidMappings() {
         // 1. Parse mappings from MappingMetadata
-        ImmutableOpenMap.Builder<String, MappingMetadata> mappings = ImmutableOpenMap.builder();
         Map<String, Object> m = new HashMap<>();
         m.put("netflow.event_data.SourceAddress", Map.of("type", "ip"));
         m.put("netflow.event_data.SourcePort", Map.of("type", "integer"));
         Map<String, Object> properties = Map.of("incorrect_properties", m);
         Map<String, Object> root = Map.of(MapperService.SINGLE_MAPPING_NAME, properties);
         MappingMetadata mappingMetadata = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, root);
-        mappings.put("my_index", mappingMetadata);
 
         MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
-        final boolean[] errorHappend = new boolean[1];
+        List<String> paths = new ArrayList<>();
         mappingsTraverser.addListener(new MappingsTraverser.MappingsTraverserListener() {
             @Override
             public void onLeafVisited(MappingsTraverser.Node node) {
                 assertNotNull(node);
+                paths.add(node.currentPath);
+            }
+
+            @Override
+            public void onError(String error) {
+                fail("Failed traversing invalid mappings");
+            }
+        });
+        mappingsTraverser.traverse();
+        assertEquals(0, paths.size());
+    }
+
+   public void testTraverseEmptyMappings() {
+        Map<String, Object> root = Map.of(MapperService.SINGLE_MAPPING_NAME, new HashMap<>());
+        MappingMetadata mappingMetadata = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, root);
+
+        MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
+        final boolean[] errorHappend = new boolean[1];
+        List<String> paths = new ArrayList<>();
+
+       mappingsTraverser.addListener(new MappingsTraverser.MappingsTraverserListener() {
+            @Override
+            public void onLeafVisited(MappingsTraverser.Node node) {
+                assertNotNull(node);
+                paths.add(node.currentPath);
             }
 
             @Override
             public void onError(String error) {
-                errorHappend[0] = true;
+                fail("Failed traversing empty mappings");
             }
         });
         mappingsTraverser.traverse();
-        assertTrue(errorHappend[0]);
+        assertEquals(0, paths.size());
     }
 
     public void testTraverseValidMappingsWithTypeFilter() {

From db2ee003ccb832e9d91eba5c8596a805b6fae9b9 Mon Sep 17 00:00:00 2001
From: Megha Goyal <56077967+goyamegh@users.noreply.github.com>
Date: Wed, 29 Nov 2023 17:27:23 -0800
Subject: [PATCH 2/3] #709 Fix merge conflicts while backporting

Signed-off-by: Megha Goyal <goyamegh@amazon.com>
---
 .../securityanalytics/mapper/MapperService.java    | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
index 6d42cb5db..ff7479372 100644
--- a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
+++ b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
@@ -290,20 +290,6 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                                 }
                             }
                         }
-
-                        // Traverse mappings and do copy with excluded type=alias properties
-                        MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata);
-                        // Resulting mapping after filtering
-                        Map<String, Object> filteredMapping = mappingsTraverser.traverseAndCopyWithFilter(appliedAliases);
-
-
-                        // Construct filtered mappings and return them as result
-                        Map<String, MappingMetadata> outIndexMappings = new HashMap<>();
-                        Map<String, Object> root = Map.of(org.opensearch.index.mapper.MapperService.SINGLE_MAPPING_NAME, filteredMapping);
-                        MappingMetadata outMappingMetadata = new MappingMetadata(org.opensearch.index.mapper.MapperService.SINGLE_MAPPING_NAME, root);
-                        outIndexMappings.put(indexName, outMappingMetadata);
-
-                        actionListener.onResponse(new GetIndexMappingsResponse(outIndexMappings));
                     }
 
                     // Traverse mappings and do copy with excluded type=alias properties

From 4201f35c32d3004d097a254ad8c5e74dd810fcde Mon Sep 17 00:00:00 2001
From: Megha Goyal <56077967+goyamegh@users.noreply.github.com>
Date: Wed, 29 Nov 2023 17:27:23 -0800
Subject: [PATCH 3/3] #709 Fix merge conflicts while backporting

Signed-off-by: Megha Goyal <goyamegh@amazon.com>
---
 .../opensearch/securityanalytics/mapper/MappingsTraverser.java | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
index e6f82eb47..010e21e1b 100644
--- a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
+++ b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
@@ -15,9 +15,6 @@
 import org.opensearch.common.xcontent.NamedXContentRegistry;
 import org.opensearch.common.xcontent.XContentParser;
 import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.core.xcontent.DeprecationHandler;
-import org.opensearch.core.xcontent.NamedXContentRegistry;
-import org.opensearch.core.xcontent.XContentParser;
 
 import java.io.IOException;