Skip to content

Commit

Permalink
Delete detector successfully if workflow is missing (#790)
Browse files Browse the repository at this point in the history 
* Delete detector successfully if workflow is missing

Signed-off-by: Chase Engelbrecht <[email protected]>

* Refactor to use existing NotFound exception checker

Signed-off-by: Chase Engelbrecht <[email protected]>

---------

Signed-off-by: Chase Engelbrecht <[email protected]>
  • Loading branch information
@engechas
engechas authored Jan 3, 2024
1 parent c373343 commit 0ad91cc
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 28 deletions.
40 changes: 30 additions & 10 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.ActionRunnable;
import org.opensearch.action.StepListener;
Expand Down Expand Up @@ -204,7 +205,7 @@ public void onResponse(Collection<DeleteMonitorResponse> responses) {

@Override
public void onFailure(Exception e) {
if (isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListener(e, detector.getId())) {
if (isOnlyWorkflowOrMonitorOrIndexMissingExceptionThrownByGroupedActionListener(e, detector.getId())) {
deleteDetectorFromConfig(detector.getId(), request.getRefreshPolicy());
} else {
log.error(String.format(Locale.ROOT, "Failed to delete detector %s", detector.getId()), e);
Expand All @@ -231,15 +232,25 @@ private void deleteWorkflow(Detector detector, ActionListener<AcknowledgedRespon
log.debug(String.format("Deleting the workflow %s before deleting the detector", workflowId));
StepListener<DeleteWorkflowResponse> onDeleteWorkflowStep = new StepListener<>();
workflowService.deleteWorkflow(workflowId, onDeleteWorkflowStep);
onDeleteWorkflowStep.whenComplete(deleteWorkflowResponse -> {
actionListener.onResponse(new AcknowledgedResponse(true));
}, actionListener::onFailure);
onDeleteWorkflowStep.whenComplete(
deleteWorkflowResponse -> actionListener.onResponse(new AcknowledgedResponse(true)),
deleteWorkflowResponse -> handleDeleteWorkflowFailure(detector.getId(), deleteWorkflowResponse, actionListener)
);
} else {
// If detector doesn't have the workflows it means that older version of the plugin is used and just skip the step
actionListener.onResponse(new AcknowledgedResponse(true));
}
}

private void handleDeleteWorkflowFailure(final String detectorId, final Exception deleteWorkflowException,
final ActionListener<AcknowledgedResponse> actionListener) {
if (isOnlyWorkflowOrMonitorOrIndexMissingExceptionThrownByGroupedActionListener(deleteWorkflowException, detectorId)) {
actionListener.onResponse(new AcknowledgedResponse(true));
} else {
actionListener.onFailure(deleteWorkflowException);
}
}

private void deleteDetectorFromConfig(String detectorId, WriteRequest.RefreshPolicy refreshPolicy) {
deleteDetector(detectorId, refreshPolicy,
new ActionListener<>() {
Expand Down Expand Up @@ -296,7 +307,7 @@ private void finishHim(String detectorId, Exception t) {
}));
}

private boolean isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListener(
private boolean isOnlyWorkflowOrMonitorOrIndexMissingExceptionThrownByGroupedActionListener(
Exception ex,
String detectorId
) {
Expand All @@ -305,12 +316,9 @@ private boolean isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListene
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]")
) {
if (isMonitorNotFoundException(e) || isWorkflowNotFoundException(e) || isAlertingConfigIndexNotFoundException(e)) {
log.error(
String.format(Locale.ROOT, "Monitor or jobs index already deleted." +
String.format(Locale.ROOT, "Workflow, monitor, or jobs index already deleted." +
" Proceeding with detector %s deletion", detectorId),
e);
} else {
Expand All @@ -321,6 +329,18 @@ private boolean isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListene
}
}

private boolean isMonitorNotFoundException(final Throwable e) {
return e.getMessage().matches("(.*)Monitor(.*) is not found(.*)");
}

private boolean isWorkflowNotFoundException(final Throwable e) {
return e.getMessage().matches("(.*)Workflow(.*) not found(.*)");
}

private boolean isAlertingConfigIndexNotFoundException(final Throwable e) {
return e.getMessage().contains("Configured indices are not found: [.opendistro-alerting-config]");
}

private void setEnabledWorkflowUsage(boolean enabledWorkflowUsage) {
this.enabledWorkflowUsage = enabledWorkflowUsage;
}
Expand Down
8 changes: 8 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 @@ -403,6 +403,14 @@ protected Response executeAlertingWorkflow(RestClient client, String workflowId,
return makeRequest(client, "POST", String.format(Locale.getDefault(), "/_plugins/_alerting/workflows/%s/_execute", workflowId), params, null);
}

protected Response deleteAlertingWorkflow(String workflowId) throws IOException {
return deleteAlertingWorkflow(client(), workflowId);
}

protected Response deleteAlertingWorkflow(RestClient client, String workflowId) throws IOException {
return makeRequest(client, "DELETE", String.format(Locale.getDefault(), "/_plugins/_alerting/workflows/%s", workflowId), new HashMap<>(), null);
}

protected List<SearchHit> executeSearch(String index, String request) throws IOException {
return executeSearch(index, request, true);
}
Expand Down
69 changes: 51 additions & 18 deletions src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.opensearch.client.ResponseException;
import org.opensearch.commons.alerting.model.IntervalSchedule;
import org.opensearch.commons.alerting.model.Monitor.MonitorType;
import org.opensearch.commons.alerting.model.ScheduledJob;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.search.SearchHit;
Expand Down Expand Up @@ -72,10 +73,34 @@ public void testNewLogTypes() throws IOException {
@SuppressWarnings("unchecked")
public void testDeletingADetector_MonitorNotExists() throws IOException {
updateClusterSetting(ENABLE_WORKFLOW_USAGE.getKey(), "false");
String index = createTestIndex(randomIndex(), windowsIndexMapping());
final String detectorId = setupDetector();
final Map<String, Object> detectorSourceAsMap = getDetectorSourceAsMap(detectorId);

final String monitorId = ((List<String>) detectorSourceAsMap.get("monitor_id")).get(0);
final Response deleteMonitorResponse = deleteAlertingMonitor(monitorId);
assertEquals(200, deleteMonitorResponse.getStatusLine().getStatusCode());
entityAsMap(deleteMonitorResponse);

validateDetectorDeletion(detectorId);
}

public void testDeletingADetector_WorkflowUsageEnabled_WorkflowDoesntExist() throws IOException {
final String detectorId = setupDetector();
final Map<String, Object> detectorSourceAsMap = getDetectorSourceAsMap(detectorId);

final String workflowId = ((List<String>) detectorSourceAsMap.get("workflow_ids")).get(0);
final Response deleteWorkflowResponse = deleteAlertingWorkflow(workflowId);
assertEquals(200, deleteWorkflowResponse.getStatusLine().getStatusCode());
entityAsMap(deleteWorkflowResponse);

validateDetectorDeletion(detectorId);
}

private String setupDetector() throws IOException {
final String index = createTestIndex(randomIndex(), windowsIndexMapping());

// Execute CreateMappingsAction to add alias mapping for index
Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
final Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
// both req params and req body are supported
createMappingRequest.setJsonEntity(
"{ \"index_name\":\"" + index + "\"," +
Expand All @@ -84,31 +109,39 @@ public void testDeletingADetector_MonitorNotExists() throws IOException {
"}"
);

Response response = client().performRequest(createMappingRequest);
final 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(), List.of())));
String detectorId1 = createDetector(detector1);
// Create detector of type test_windows
final DetectorTrigger detectorTrigger = new DetectorTrigger(null, "test-trigger", "1", List.of(randomDetectorType()),
List.of(), List.of(), List.of(), List.of(), List.of());
final Detector detector = randomDetectorWithTriggers(getRandomPrePackagedRules(), List.of(detectorTrigger));
return createDetector(detector);
}

String request = "{\n" +
private Map<String, Object> getDetectorSourceAsMap(final String detectorId) throws IOException {
final String request = getDetectorQuery(detectorId);
final List<SearchHit> hits = executeSearch(Detector.DETECTORS_INDEX, request);
final SearchHit hit = hits.get(0);
return (Map<String, Object>) hit.getSourceAsMap().get("detector");
}

private String getDetectorQuery(final String detectorId) {
return "{\n" +
" \"query\" : {\n" +
" \"match\":{\n" +
" \"_id\": \"" + detectorId1 + "\"\n" +
" \"_id\": \"" + detectorId + "\"\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);
private void validateDetectorDeletion(final String detectorId) throws IOException {
final Response deleteResponse = makeRequest(client(), "DELETE", SecurityAnalyticsPlugin.DETECTOR_BASE_URI + "/" + detectorId,
Collections.emptyMap(), null);
Assert.assertEquals("Delete detector failed", RestStatus.OK, restStatus(deleteResponse));
hits = executeSearch(Detector.DETECTORS_INDEX, request);

final String request = getDetectorQuery(detectorId);
final List<SearchHit> hits = executeSearch(Detector.DETECTORS_INDEX, request);
Assert.assertEquals(0, hits.size());
}

Expand Down

0 comments on commit 0ad91cc

Please sign in to comment.