Delete detector successfully if workflow is missing

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

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

@@ -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;
@@ -231,15 +232,34 @@ 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(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 Exception deleteWorkflowException,
+                                                 final ActionListener<AcknowledgedResponse> actionListener) {
+            if (isNotFoundOpenSearchException(deleteWorkflowException)) {
+                actionListener.onResponse(new AcknowledgedResponse(true));
+            } else {
+                actionListener.onFailure(deleteWorkflowException);
+            }
+        }
+
+        private boolean isNotFoundOpenSearchException(final Exception e) {
+            if (!(e instanceof OpenSearchException)) {
+                return false;
+            }
+
+            final OpenSearchException openSearchException = (OpenSearchException) e;
+            return RestStatus.NOT_FOUND.equals(openSearchException.status());
+        }
+
         private void deleteDetectorFromConfig(String detectorId, WriteRequest.RefreshPolicy refreshPolicy) {
             deleteDetector(detectorId, refreshPolicy,
                     new ActionListener<>() {

src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java

@@ -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);
     }

src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java

@@ -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;
@@ -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 + "\"," +
@@ -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());
     }