Skip to content

Commit

Permalink
#709 Return empty response for empty mappings and no applied aliases (#…
Browse files Browse the repository at this point in the history 
…724)

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

Signed-off-by: Megha Goyal <[email protected]>

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

Signed-off-by: Megha Goyal <[email protected]>

* Fix unit tests for MappingsTraverser

Signed-off-by: Megha Goyal <[email protected]>

---------

Signed-off-by: Megha Goyal <[email protected]>
(cherry picked from commit c0f7bd9)
  • Loading branch information
@goyamegh @github-actions
goyamegh authored and github-actions[bot] committed Nov 30, 2023
1 parent 951d6e5 commit 12d6b2e
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 30 deletions.
7 changes: 0 additions & 7 deletions src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,13 +400,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
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,6 +39,8 @@
*/
public class MappingsTraverser {

private static final Logger log = LogManager.getLogger(MappingsTraverser.class);

/**
* Traverser listener used to process leaves
*/
Expand Down Expand Up @@ -157,7 +159,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();
Expand Down Expand Up @@ -193,6 +198,7 @@ public void traverse() {
// This is coming from listeners.
throw e;
} catch (Exception e) {
log.error("Error traversing mappings tree", e);

Check warning on line 201 in src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/mapper/MappingsTraverser.java#L201 

Added line #L201 was not covered by tests
notifyError("Error traversing mappings tree");
}
}
Expand Down
41 changes: 31 additions & 10 deletions src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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*";
Expand Down Expand Up @@ -235,35 +256,35 @@ 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(
"{ \"index_name\":\"" + testIndexName + "\"," +
" \"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 {
Expand Down
37 changes: 28 additions & 9 deletions src/test/java/org/opensearch/securityanalytics/mapper/MappingsTraverserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@

public class MappingsTraverserTests extends OpenSearchTestCase {





public void testTraverseValidMappings() {
// 1. Parse mappings from MappingMetadata
Map<String, MappingMetadata> mappings = new HashMap<>();
Expand Down Expand Up @@ -136,30 +132,53 @@ public void onError(String error) {

public void testTraverseInvalidMappings() {
// 1. Parse mappings from MappingMetadata
Map<String, MappingMetadata> mappings = new HashMap<>();
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() {
Expand Down

0 comments on commit 12d6b2e

Please sign in to comment.