fix chained findings monitor logic in update detector flow #1019

eirsep · 2024-05-08T23:07:15Z

Description

Currently when updating a detector with aggegation rules, the chained finding doc levelmonitor is getting deleted
This PR fixes the logic to update/retain the chained finding doc level monitor
Added to integ test to verify detector behaviour after updating detector.

Signed-off-by: Surya Sashank Nistala <[email protected]>

engechas · 2024-05-08T23:49:14Z

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

@@ -465,6 +465,18 @@ public void onResponse(Map<String, Map<String, String>> ruleFieldMappings) {
                                        new ActionListener<>() {
                                            @Override
                                            public void onResponse(Collection<IndexMonitorRequest> indexMonitorRequests) {
+                                                if (detector.getRuleIdMonitorIdMap().containsKey("chained_findings_monitor")) {


chained_findings_monitor should be a constant

engechas · 2024-05-08T23:51:16Z

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

@@ -465,6 +465,18 @@ public void onResponse(Map<String, Map<String, String>> ruleFieldMappings) {
                                        new ActionListener<>() {
                                            @Override
                                            public void onResponse(Collection<IndexMonitorRequest> indexMonitorRequests) {
+                                                if (detector.getRuleIdMonitorIdMap().containsKey("chained_findings_monitor")) {
+                                                    String cmfId = detector.getRuleIdMonitorIdMap().get("chained_findings_monitor");
+                                                    if (enabledWorkflowUsage && !indexMonitorRequests.isEmpty() && rulesById.stream().anyMatch(it -> it.getRight().isAggregationRule())) {


Triplicate condition + same logic on line 475. Should be refactored to a private method, something like workflowContainsAggregationRule()

engechas · 2024-05-08T23:52:24Z

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

+                                                if (detector.getRuleIdMonitorIdMap().containsKey("chained_findings_monitor")) {
+                                                    String cmfId = detector.getRuleIdMonitorIdMap().get("chained_findings_monitor");
+                                                    if (enabledWorkflowUsage && !indexMonitorRequests.isEmpty() && rulesById.stream().anyMatch(it -> it.getRight().isAggregationRule())) {
+                                                        //todo update not create chained findings monitor


Should create and link an issue for this. Or at least elaborate on why this todo exists

Do we know why the chained monitor gets deleted in the first place?

removing TODO. its resolved

…reate constant for chained findings monitor string literal Signed-off-by: Surya Sashank Nistala <[email protected]>

opensearch-trigger-bot · 2024-05-09T01:24:19Z

The backport to 2.11 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security-analytics/backport-2.11 2.11
# Navigate to the new working tree
pushd ../.worktrees/security-analytics/backport-2.11
# Create a new branch
git switch --create backport-1019-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 df5f7466c3388050654af4199d6437a4a2dc2b08
# Push it to GitHub
git push --set-upstream origin backport-1019-to-2.11
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security-analytics/backport-2.11

Then, create a pull request where the base branch is 2.11 and the compare/head branch is backport-1019-to-2.11.

opensearch-trigger-bot · 2024-05-09T01:24:20Z

The backport to 2.13 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security-analytics/backport-2.13 2.13
# Navigate to the new working tree
pushd ../.worktrees/security-analytics/backport-2.13
# Create a new branch
git switch --create backport-1019-to-2.13
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 df5f7466c3388050654af4199d6437a4a2dc2b08
# Push it to GitHub
git push --set-upstream origin backport-1019-to-2.13
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security-analytics/backport-2.13

Then, create a pull request where the base branch is 2.13 and the compare/head branch is backport-1019-to-2.13.

* fix chained findings monitor logic in update detector flow Signed-off-by: Surya Sashank Nistala <[email protected]> * extract check for chained findings monitor into a re-usable method. create constant for chained findings monitor string literal Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> (cherry picked from commit df5f746) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…1020) * fix chained findings monitor logic in update detector flow * extract check for chained findings monitor into a re-usable method. create constant for chained findings monitor string literal --------- (cherry picked from commit df5f746) Signed-off-by: Surya Sashank Nistala <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…1020) * fix chained findings monitor logic in update detector flow * extract check for chained findings monitor into a re-usable method. create constant for chained findings monitor string literal --------- (cherry picked from commit df5f746) Signed-off-by: Surya Sashank Nistala <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> (cherry picked from commit c162a6f)

…1020) (#1023) * fix chained findings monitor logic in update detector flow * extract check for chained findings monitor into a re-usable method. create constant for chained findings monitor string literal --------- (cherry picked from commit df5f746) Signed-off-by: Surya Sashank Nistala <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> (cherry picked from commit c162a6f) Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>

fix chained findings monitor logic in update detector flow

1efb9ae

Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep requested review from amsiglan, AWSHurneyt, getsaurabh02, lezzago, praveensameneni, sbcd90, jowg-amazon, engechas, goyamegh and riysaxen-amzn as code owners May 8, 2024 23:07

eirsep added backport 2.11 backport 2.13 labels May 8, 2024

engechas previously approved these changes May 8, 2024

View reviewed changes

jowg-amazon previously approved these changes May 8, 2024

View reviewed changes

praveensameneni previously approved these changes May 9, 2024

View reviewed changes

extract check for chained findings monitor into a re-usable method. c…

f9e276a

…reate constant for chained findings monitor string literal Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep dismissed stale reviews from praveensameneni, jowg-amazon, and engechas via f9e276a May 9, 2024 00:20

praveensameneni approved these changes May 9, 2024

View reviewed changes

jowg-amazon approved these changes May 9, 2024

View reviewed changes

engechas approved these changes May 9, 2024

View reviewed changes

eirsep merged commit df5f746 into opensearch-project:main May 9, 2024
16 checks passed

opensearch-trigger-bot bot added the failed backport label May 9, 2024

eirsep added the backport 2.x label May 9, 2024

opensearch-trigger-bot bot mentioned this pull request May 9, 2024

[Backport 2.x] fix chained findings monitor logic in update detector flow #1020

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix chained findings monitor logic in update detector flow #1019

fix chained findings monitor logic in update detector flow #1019

eirsep commented May 8, 2024

engechas May 8, 2024

eirsep May 9, 2024

engechas May 8, 2024

eirsep May 9, 2024

engechas May 8, 2024

jowg-amazon May 8, 2024

eirsep May 9, 2024

opensearch-trigger-bot bot commented May 9, 2024

opensearch-trigger-bot bot commented May 9, 2024

fix chained findings monitor logic in update detector flow #1019

fix chained findings monitor logic in update detector flow #1019

Conversation

eirsep commented May 8, 2024

Description

engechas May 8, 2024

Choose a reason for hiding this comment

eirsep May 9, 2024

Choose a reason for hiding this comment

engechas May 8, 2024

Choose a reason for hiding this comment

eirsep May 9, 2024

Choose a reason for hiding this comment

engechas May 8, 2024

Choose a reason for hiding this comment

jowg-amazon May 8, 2024

Choose a reason for hiding this comment

eirsep May 9, 2024

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented May 9, 2024

opensearch-trigger-bot bot commented May 9, 2024