diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java index da181981d..52a0af849 100644 --- a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java +++ b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java @@ -330,13 +330,6 @@ public void onResponse(GetMappingsResponse getMappingsResponse) { } } - if (appliedAliases.size() == 0) { - actionListener.onFailure(SecurityAnalyticsException.wrap( - new OpenSearchStatusException("No applied aliases found", RestStatus.NOT_FOUND)) - ); - return; - } - // Traverse mappings and do copy with excluded type=alias properties MappingsTraverser mappingsTraverser = new MappingsTraverser(mappingMetadata); // Resulting mapping after filtering diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java index fbddaa31c..67a9f0e67 100644 --- a/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java +++ b/src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java @@ -5,23 +5,23 @@ 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.json.JsonXContent; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.index.mapper.MapperService; -import org.opensearch.securityanalytics.rules.condition.ConditionListener; 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 +39,8 @@ */ public class MappingsTraverser { + private static final Logger log = LogManager.getLogger(MappingsTraverser.class); + /** * Traverser listener used to process leaves */ @@ -157,7 +159,10 @@ public void traverse() { try { Map rootProperties = (Map) 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 +198,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 22952b4e9..a64554000 100644 --- a/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java @@ -67,6 +67,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 respMap = (Map) responseAsMap(response); + Map props = (Map)((Map) 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*"; @@ -234,13 +255,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( @@ -248,21 +270,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 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..e25b7084e 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 mappings = ImmutableOpenMap.builder(); @@ -143,23 +139,47 @@ public void testTraverseInvalidMappings() { Map properties = Map.of("incorrect_properties", m); Map 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 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 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 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() {