From 38bf22c1c577190ae5a56c4ceafd69e0a0160a0e Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Mon, 3 Apr 2023 16:15:57 -0700 Subject: [PATCH 1/2] Handle monitor or monitor index not exists during detector deletion Signed-off-by: Surya Sashank Nistala --- .../TransportDeleteDetectorAction.java | 35 ++++++++++++++-- .../SecurityAnalyticsRestTestCase.java | 12 ++++++ .../resthandler/DetectorRestApiIT.java | 42 +++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java index 899f47991..7f2d9b05f 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java @@ -138,7 +138,6 @@ public void onFailure(Exception t) { private void onGetResponse(Detector detector) { List monitorIds = detector.getMonitorIds(); - String ruleIndex = detector.getRuleIndex(); ActionListener deletesListener = new GroupedActionListener<>(new ActionListener<>() { @Override public void onResponse(Collection responses) { @@ -158,8 +157,13 @@ public void onResponse(Collection responses) { @Override public void onFailure(Exception e) { - if (counter.compareAndSet(false, true)) { - finishHim(null, e); + if(isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListener(e, detector.getId())) { + deleteDetectorFromConfig(detector.getId(), request.getRefreshPolicy()); + } else { + log.error(String.format(Locale.ROOT, "Failed to delete detector %s", detector.getId()), e); + if (counter.compareAndSet(false, true)) { + finishHim(null, e); + } } } }, monitorIds.size()); @@ -212,6 +216,7 @@ private void onFailures(Exception t) { private void finishHim(String detectorId, Exception t) { threadPool.executor(ThreadPool.Names.GENERIC).execute(ActionRunnable.supply(listener, () -> { if (t != null) { + log.error(String.format(Locale.ROOT, "Failed to delete detector %s",detectorId), t); if (t instanceof OpenSearchStatusException) { throw t; } @@ -221,5 +226,29 @@ private void finishHim(String detectorId, Exception t) { } })); } + + private boolean isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListener( + Exception ex, + String detectorId + ) { + // grouped action listener listens on mutliple listeners but throws only one exception. If multiple + // listeners fail the other exceptions are added as suppressed exceptions to the first failure. + int len = ex.getSuppressed().length; + for (int i = 0; i <= len; i++) { + Throwable e = i == len ? ex : ex.getSuppressed()[i]; + if (e.getMessage().matches("(.*)Monitor(.*) is not found(.*)") + || e.getMessage().contains( + "Configured indices are not found: [.opendistro-alerting-config]") + ) { + log.error( + String.format(Locale.ROOT, "Monitor or jobs index already deleted." + + " Proceeding with detector %s deletion", detectorId), + e); + } else { + return false; + } + } + return true; + } } } \ No newline at end of file diff --git a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java index 59f250545..d680cb0dc 100644 --- a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java +++ b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java @@ -238,6 +238,18 @@ protected Response executeAlertingMonitor(RestClient client, String monitorId, M return makeRequest(client, "POST", String.format(Locale.getDefault(), "/_plugins/_alerting/monitors/%s/_execute", monitorId), params, null); } + protected Response deleteAlertingMonitorIndex() throws IOException { + return makeRequest(client(), "DELETE", String.format(Locale.getDefault(), "/.opendistro-alerting-config"), new HashMap<>(), null); + } + + protected Response deleteAlertingMonitor(String monitorId) throws IOException { + return deleteAlertingMonitor(client(), monitorId); + } + + protected Response deleteAlertingMonitor(RestClient client, String monitorId) throws IOException { + return makeRequest(client, "DELETE", String.format(Locale.getDefault(), "/_plugins/_alerting/monitors/%s", monitorId), new HashMap<>(), null); + } + protected List executeSearch(String index, String request) throws IOException { return executeSearch(index, request, true); } diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java index 16a852ca7..d2c849120 100644 --- a/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java @@ -710,6 +710,48 @@ public void testDeletingADetector_single_ruleTopicIndex() throws IOException { Assert.assertEquals(0, hits.size()); } + @SuppressWarnings("unchecked") + public void testDeletingADetector_MonitorNotExists() throws IOException { + String index = createTestIndex(randomIndex(), windowsIndexMapping()); + + // Execute CreateMappingsAction to add alias mapping for index + Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI); + // both req params and req body are supported + createMappingRequest.setJsonEntity( + "{ \"index_name\":\"" + index + "\"," + + " \"rule_topic\":\"" + randomDetectorType() + "\", " + + " \"partial\":true" + + "}" + ); + + Response response = client().performRequest(createMappingRequest); + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + // Create detector #1 of type test_windows + Detector detector1 = randomDetectorWithTriggers(getRandomPrePackagedRules(), List.of(new DetectorTrigger(null, "test-trigger", "1", List.of(randomDetectorType()), List.of(), List.of(), List.of(), List.of()))); + String detectorId1 = createDetector(detector1); + + String request = "{\n" + + " \"query\" : {\n" + + " \"match\":{\n" + + " \"_id\": \"" + detectorId1 + "\"\n" + + " }\n" + + " }\n" + + "}"; + List hits = executeSearch(Detector.DETECTORS_INDEX, request); + SearchHit hit = hits.get(0); + + String monitorId = ((List) ((Map) hit.getSourceAsMap().get("detector")).get("monitor_id")).get(0); + + Response deleteMonitorResponse = deleteAlertingMonitor(monitorId); + assertEquals(200, deleteMonitorResponse.getStatusLine().getStatusCode()); + entityAsMap(deleteMonitorResponse); + + Response deleteResponse = makeRequest(client(), "DELETE", SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/" + detectorId1, Collections.emptyMap(), null); + Assert.assertEquals("Delete detector failed", RestStatus.OK, restStatus(deleteResponse)); + hits = executeSearch(Detector.DETECTORS_INDEX, request); + Assert.assertEquals(0, hits.size()); + } + public void testDeletingADetector_oneDetectorType_multiple_ruleTopicIndex() throws IOException { String index1 = "test_index_1"; createIndex(index1, Settings.EMPTY); From 5988f43de11d49e7348c20df7365c7013d46b156 Mon Sep 17 00:00:00 2001 From: Subhobrata Dey Date: Wed, 5 Apr 2023 01:05:51 +0000 Subject: [PATCH 2/2] Handle monitor or monitor index not exists during detector deletion Signed-off-by: Subhobrata Dey --- build.gradle | 2 +- .../transport/TransportDeleteDetectorAction.java | 2 +- .../transport/TransportIndexDetectorAction.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 41cc45a9f..5f0b43a40 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ import org.opensearch.gradle.test.RestIntegTestTask buildscript { ext { - opensearch_version = System.getProperty("opensearch.version", "2.5.0-SNAPSHOT") + opensearch_version = System.getProperty("opensearch.version", "2.6.0-SNAPSHOT") isSnapshot = "true" == System.getProperty("build.snapshot", "true") buildVersionQualifier = System.getProperty("build.version_qualifier", "") version_tokens = opensearch_version.tokenize('-') diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java index 7f2d9b05f..3014cca0e 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java @@ -11,7 +11,6 @@ import java.util.concurrent.atomic.AtomicReference; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.lucene.util.SetOnce; import org.opensearch.OpenSearchStatusException; import org.opensearch.action.ActionListener; import org.opensearch.action.ActionRunnable; @@ -25,6 +24,7 @@ import org.opensearch.action.support.WriteRequest; import org.opensearch.client.Client; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.SetOnce; import org.opensearch.common.inject.Inject; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java index 5fda9de28..78fab2afb 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java @@ -8,7 +8,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.search.join.ScoreMode; -import org.apache.lucene.util.SetOnce; import org.opensearch.OpenSearchStatusException; import org.opensearch.action.ActionListener; import org.opensearch.action.ActionRunnable; @@ -32,6 +31,7 @@ import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.SetOnce; import org.opensearch.common.inject.Inject; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.settings.Settings;