Skip to content

Commit

Permalink
Handle monitor or monitor index not exists during detector deletion
Browse files Browse the repository at this point in the history 
Signed-off-by: Surya Sashank Nistala <[email protected]>
  • Loading branch information
@eirsep
eirsep committed Apr 3, 2023
1 parent 2647ef9 commit a25107d
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 3 deletions.
35 changes: 32 additions & 3 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ public void onFailure(Exception t) {

private void onGetResponse(Detector detector) {
List<String> monitorIds = detector.getMonitorIds();
String ruleIndex = detector.getRuleIndex();
ActionListener<DeleteMonitorResponse> deletesListener = new GroupedActionListener<>(new ActionListener<>() {
@Override
public void onResponse(Collection<DeleteMonitorResponse> responses) {
Expand All @@ -158,8 +157,13 @@ public void onResponse(Collection<DeleteMonitorResponse> 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());
Expand Down Expand Up @@ -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;
}
Expand All @@ -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("Monitor or jobs index already deleted." +
" Proceeding with detector %s deletion", detectorId),
e);
} else {
return false;
}
}
return true;
}
}
}
12 changes: 12 additions & 0 deletions src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchHit> executeSearch(String index, String request) throws IOException {
return executeSearch(index, request, true);
}
Expand Down
86 changes: 86 additions & 0 deletions src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,92 @@ 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<SearchHit> hits = executeSearch(Detector.DETECTORS_INDEX, request);
SearchHit hit = hits.get(0);

String monitorId = ((List<String>) ((Map<String, Object>) 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());
}



@SuppressWarnings("unchecked")
public void testDeletingADetector_MonitorIndexNotExists() 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<SearchHit> hits = executeSearch(Detector.DETECTORS_INDEX, request);
SearchHit hit = hits.get(0);

String monitorId = ((List<String>) ((Map<String, Object>) hit.getSourceAsMap().get("detector")).get("monitor_id")).get(0);

Response deleteMonitorResponse = deleteAlertingMonitorIndex();
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);
Expand Down

0 comments on commit a25107d

Please sign in to comment.