Merge branch 'main' into fix_ocsf_mapping

Signed-off-by: Joanne Wang <[email protected]>
opensearch-project · Mar 8, 2024 · 19ef4e7 · 19ef4e7
2 parents 09cf6ab + 656a5fe
commit 19ef4e7
Show file tree

Hide file tree

Showing 30 changed files with 2,498 additions and 1,789 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -1 +1 @@
-* @amsiglan @AWSHurneyt @getsaurabh02 @lezzago @praveensameneni @sbcd90 @eirsep @jowg-amazon @engechas
+* @amsiglan @AWSHurneyt @getsaurabh02 @lezzago @praveensameneni @sbcd90 @eirsep @jowg-amazon @engechas @goyamegh @riysaxen-amzn
diff --git a/MAINTAINERS.md b/MAINTAINERS.md
@@ -15,3 +15,5 @@ This document contains a list of maintainers in this repo. See [opensearch-proje
 | Saurabh Singh         | [getsaurabh02](https://github.com/getsaurabh02)       | Amazon      |
 | Joanne Wang           | [jowg-amazon](https://github.com/jowg-amazon)         | Amazon      |
 | Chase Engelbrecht     | [engechas](https://github.com/engechas)               | Amazon      |
+| Megha Goyal           | [goyamegh](https://github.com/goyamegh)               | Amazon      |
+| Riya Saxena           | [riysaxen-amzn](https://github.com/riysaxen-amzn))    | Amazon      |
diff --git a/src/main/java/org/opensearch/securityanalytics/correlation/JoinEngine.java b/src/main/java/org/opensearch/securityanalytics/correlation/JoinEngine.java
diff --git a/src/main/java/org/opensearch/securityanalytics/correlation/VectorEmbeddingsEngine.java b/src/main/java/org/opensearch/securityanalytics/correlation/VectorEmbeddingsEngine.java
diff --git a/src/main/java/org/opensearch/securityanalytics/logtype/LogTypeService.java b/src/main/java/org/opensearch/securityanalytics/logtype/LogTypeService.java
@@ -282,7 +282,7 @@ public void onResponse(SearchResponse response) {
                 if (response.isTimedOut()) {
                     listener.onFailure(new OpenSearchStatusException("Search request timed out", RestStatus.REQUEST_TIMEOUT));
                 }
-                if (response.getHits().getTotalHits().value > 0) {
+                if (response.getHits().getHits().length > 0) {
                     listener.onResponse(null);
                 } else {
                     try {

diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java b/src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
@@ -8,7 +8,6 @@
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.admin.indices.get.GetIndexRequest;
 import org.opensearch.action.admin.indices.get.GetIndexResponse;
 import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
@@ -78,9 +77,11 @@ public void createMappingAction(String indexName, String logType, String aliasMa
         // since you can't update documents in non-write indices
         String index = indexName;
         boolean shouldUpsertIndexTemplate = IndexUtils.isConcreteIndex(indexName, this.clusterService.state()) == false;
-        if (IndexUtils.isDataStream(indexName, this.clusterService.state())) {
+        if (IndexUtils.isDataStream(indexName, this.clusterService.state()) || IndexUtils.isAlias(indexName, this.clusterService.state())) {
+            log.debug("{} is an alias or datastream. Fetching write index for create mapping action.", indexName);
             String writeIndex = IndexUtils.getWriteIndex(indexName, this.clusterService.state());
             if (writeIndex != null) {
+                log.debug("Write index for {} is {}", indexName, writeIndex);
                 index = writeIndex;
             }
         }
@@ -92,6 +93,7 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                 applyAliasMappings(getMappingsResponse.getMappings(), logType, aliasMappings, partial, new ActionListener<>() {
                     @Override
                     public void onResponse(Collection<CreateMappingResult> createMappingResponse) {
+                        log.debug("Completed create mappings for {}", indexName);
                         // We will return ack==false if one of the requests returned that
                         // else return ack==true
                         Optional<AcknowledgedResponse> notAckd = createMappingResponse.stream()
@@ -110,6 +112,7 @@ public void onResponse(Collection<CreateMappingResult> createMappingResponse) {
 
                     @Override
                     public void onFailure(Exception e) {
+                        log.debug("Failed to create mappings for {}", indexName );
                         actionListener.onFailure(e);
                     }
                 });
@@ -481,13 +484,16 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                             String rawPath = requiredField.getRawField();
                             String ocsfPath = requiredField.getOcsf();
                             if (allFieldsFromIndex.contains(rawPath)) {
-                                if (alias != null) {
-                                    // Maintain list of found paths in index
-                                    applyableAliases.add(alias);
-                                } else {
-                                    applyableAliases.add(rawPath);
+                                // if the alias was already added into applyable aliases, then skip to avoid duplicates
+                                if (!applyableAliases.contains(alias) && !applyableAliases.contains(rawPath)) {
+                                    if (alias != null) {
+                                        // Maintain list of found paths in index
+                                        applyableAliases.add(alias);
+                                    } else {
+                                        applyableAliases.add(rawPath);
+                                    }
+                                    pathsOfApplyableAliases.add(rawPath);
                                 }
-                                pathsOfApplyableAliases.add(rawPath);
                             } else if (allFieldsFromIndex.contains(ocsfPath)) {
                                 applyableAliases.add(alias);
                                 pathsOfApplyableAliases.add(ocsfPath);
@@ -501,13 +507,21 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                             }
                         }
 
+                        // turn unmappedFieldAliases into a set to remove duplicates
+                        Set<String> setOfUnmappedFieldAliases = new HashSet<>(unmappedFieldAliases);
+
+                        // filter out aliases that were included in applyableAliases already
+                        List<String> filteredUnmappedFieldAliases = setOfUnmappedFieldAliases.stream()
+                                .filter(e -> false == applyableAliases.contains(e))
+                                .collect(Collectors.toList());
+
                         Map<String, Map<String, String>> aliasMappingFields = new HashMap<>();
                         XContentBuilder aliasMappingsObj = XContentFactory.jsonBuilder().startObject();
                         for (LogType.Mapping mapping : requiredFields) {
                             if (allFieldsFromIndex.contains(mapping.getOcsf())) {
                                 aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getOcsf()));
                             } else if (mapping.getEcs() != null) {
-                                aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+                                shouldUpdateEcsMappingAndMaybeUpdates(mapping, aliasMappingFields, pathsOfApplyableAliases);
                             } else if (mapping.getEcs() == null) {
                                 aliasMappingFields.put(mapping.getRawField(), Map.of("type", "alias", "path", mapping.getRawField()));
                             }
@@ -523,7 +537,7 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
                                 .filter(e -> pathsOfApplyableAliases.contains(e) == false)
                                 .collect(Collectors.toList());
                         actionListener.onResponse(
-                                new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, unmappedFieldAliases, logTypeService.getIocFieldsList(logType))
+                                new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, filteredUnmappedFieldAliases, logTypeService.getIocFieldsList(logType))
                         );
                     } catch (Exception e) {
                         actionListener.onFailure(e);
@@ -538,6 +552,26 @@ public void onFailure(Exception e) {
         });
     }
 
+    /**
+     * Only updates the alias mapping fields if the ecs key has not been mapped yet
+     * or if pathOfApplyableAliases contains the raw field
+     *
+     * @param mapping
+     * @param aliasMappingFields
+     * @param pathsOfApplyableAliases
+     */
+    private static void shouldUpdateEcsMappingAndMaybeUpdates(LogType.Mapping mapping, Map<String, Map<String, String>> aliasMappingFields, List<String> pathsOfApplyableAliases) {
+        // check if aliasMappingFields already contains a key
+        if (aliasMappingFields.containsKey(mapping.getEcs())) {
+            // if the pathOfApplyableAliases contains the raw field, then override the existing map
+            if (pathsOfApplyableAliases.contains(mapping.getRawField())) {
+                aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+            }
+        } else {
+            aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
+        }
+    }
+
     /**
      * Given index name, resolves it to single concrete index, depending on what initial <code>indexName</code> is.
      * In case of Datastream or Alias, WriteIndex would be returned. In case of index pattern, newest index by creation date would be returned.

diff --git a/src/main/java/org/opensearch/securityanalytics/mapper/MapperUtils.java b/src/main/java/org/opensearch/securityanalytics/mapper/MapperUtils.java
@@ -5,16 +5,17 @@
 
 package org.opensearch.securityanalytics.mapper;
 
+import org.apache.commons.lang3.tuple.Pair;
+import org.opensearch.cluster.metadata.MappingMetadata;
+import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
-import org.apache.commons.lang3.tuple.Pair;
-import org.opensearch.cluster.metadata.MappingMetadata;
-import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
 
 public class MapperUtils {
 
@@ -246,7 +247,6 @@ public void onError(String error) {
             }
         });
         mappingsTraverser.traverse();
-
         return presentPathsMappings;
     }
 }
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		* @amsiglan @AWSHurneyt @getsaurabh02 @lezzago @praveensameneni @sbcd90 @eirsep @jowg-amazon @engechas
		* @amsiglan @AWSHurneyt @getsaurabh02 @lezzago @praveensameneni @sbcd90 @eirsep @jowg-amazon @engechas @goyamegh @riysaxen-amzn