From d3366fec2f6a53e292b9a386b7648347144af4da Mon Sep 17 00:00:00 2001 From: Chase <62891993+engechas@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:57:49 -0800 Subject: [PATCH] Delete detector successfully if workflow is missing (#790) * Delete detector successfully if workflow is missing Signed-off-by: Chase Engelbrecht * Refactor to use existing NotFound exception checker Signed-off-by: Chase Engelbrecht --------- Signed-off-by: Chase Engelbrecht (cherry picked from commit 0ad91ccecd846439bb3136390e2defbf59eb9e10) --- .../TransportDeleteDetectorAction.java | 39 ++++++++--- .../SecurityAnalyticsRestTestCase.java | 8 +++ .../resthandler/DetectorRestApiIT.java | 69 ++++++++++++++----- 3 files changed, 88 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java index 1e8a9880d..2371f84c0 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportDeleteDetectorAction.java @@ -204,7 +204,7 @@ public void onResponse(Collection 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); @@ -231,15 +231,25 @@ private void deleteWorkflow(Detector detector, ActionListener 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 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<>() { @@ -296,7 +306,7 @@ private void finishHim(String detectorId, Exception t) { })); } - private boolean isOnlyMonitorOrIndexMissingExceptionThrownByGroupedActionListener( + private boolean isOnlyWorkflowOrMonitorOrIndexMissingExceptionThrownByGroupedActionListener( Exception ex, String detectorId ) { @@ -305,12 +315,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 { @@ -321,6 +328,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; } diff --git a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java index 5b29285ee..81cd4767d 100644 --- a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java +++ b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java @@ -398,6 +398,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 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 0a7263509..3ed83c85e 100644 --- a/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/DetectorRestApiIT.java @@ -72,10 +72,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 detectorSourceAsMap = getDetectorSourceAsMap(detectorId); + + final String monitorId = ((List) 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 detectorSourceAsMap = getDetectorSourceAsMap(detectorId); + + final String workflowId = ((List) 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 +108,40 @@ 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()))); - String detectorId1 = createDetector(detector1); - String request = "{\n" + + // 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()); + final Detector detector = randomDetectorWithTriggers(getRandomPrePackagedRules(), List.of(detectorTrigger)); + return createDetector(detector); + } + + private Map getDetectorSourceAsMap(final String detectorId) throws IOException { + final String request = getDetectorQuery(detectorId); + final List hits = executeSearch(Detector.DETECTORS_INDEX, request); + final SearchHit hit = hits.get(0); + return (Map) 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 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); + 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 hits = executeSearch(Detector.DETECTORS_INDEX, request); Assert.assertEquals(0, hits.size()); }