Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.10] #709 Return empty response for empty mappings and no applied aliases #750

Merged
merged 1 commit into from
Dec 1, 2023
Merged

[Backport 2.10] #709 Return empty response for empty mappings and no applied aliases #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,13 +398,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
Open in desktop
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 @@
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 @@
// 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,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 @@ -232,35 +253,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
Open in desktop
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
Loading