From 381321716889a8187952cb7a06bf8f753253e5f8 Mon Sep 17 00:00:00 2001 From: Riya Saxena Date: Mon, 11 Mar 2024 16:14:45 -0700 Subject: [PATCH 1/3] solution to fix integ tests Signed-off-by: Riya Saxena --- .../alerting/resthandler/RestGetFindingsAction.kt | 6 +----- .../alerting/transport/TransportGetFindingsAction.kt | 8 ++++---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt index 1270e3cab..75607a701 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetFindingsAction.kt @@ -45,8 +45,6 @@ class RestGetFindingsAction : BaseRestHandler() { val size = request.paramAsInt("size", 20) val startIndex = request.paramAsInt("startIndex", 0) val searchString = request.param("searchString", "") - val severity: String? = request.param("severity", "ALL") - val detectionType: String? = request.param("detectionType", "rules") val table = Table( sortOrder, @@ -59,9 +57,7 @@ class RestGetFindingsAction : BaseRestHandler() { val getFindingsSearchRequest = GetFindingsRequest( findingID, - table, - severity, - detectionType + table ) return RestChannelConsumer { channel -> diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt index 0357889aa..c9b6c8458 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt @@ -82,8 +82,6 @@ class TransportGetFindingsSearchAction @Inject constructor( val getFindingsRequest = request as? GetFindingsRequest ?: recreateObject(request) { GetFindingsRequest(it) } val tableProp = getFindingsRequest.table - val severity = getFindingsRequest.severity - val detectionType = getFindingsRequest.detectionType val searchString = tableProp.searchString val sortBuilder = SortBuilders @@ -125,7 +123,8 @@ class TransportGetFindingsSearchAction @Inject constructor( queryBuilder.filter(timeRangeQuery) } - if (!detectionType.isNullOrBlank()) { + if (!getFindingsRequest.detectionType.isNullOrBlank()) { + val detectionType = getFindingsRequest.detectionType val nestedQueryBuilder = QueryBuilders.nestedQuery( "queries", when { @@ -161,7 +160,8 @@ class TransportGetFindingsSearchAction @Inject constructor( .minimumShouldMatch(1) } - if (!severity.isNullOrBlank()) { + if (!getFindingsRequest.severity.isNullOrBlank()) { + val severity = getFindingsRequest.severity queryBuilder .must( QueryBuilders.nestedQuery( From 70d1193484c1dac0e7f21e00d381c8e9876505e1 Mon Sep 17 00:00:00 2001 From: Riya Saxena Date: Mon, 11 Mar 2024 17:57:11 -0700 Subject: [PATCH 2/3] fix flaky DocumentMonitor Runner tests Signed-off-by: Riya Saxena --- .../org/opensearch/alerting/DocumentMonitorRunnerIT.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt index cc65aa849..69b40e021 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/DocumentMonitorRunnerIT.kt @@ -2119,8 +2119,10 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() { val findings = searchFindings(monitor) assertEquals("Findings saved for test monitor", 2, findings.size) - assertTrue("Findings saved for test monitor", findings[0].relatedDocIds.contains("1")) - assertTrue("Findings saved for test monitor", findings[1].relatedDocIds.contains("5")) + val findings0 = findings[0].relatedDocIds.contains("1") || findings[0].relatedDocIds.contains("5") + val findings1 = findings[1].relatedDocIds.contains("5") || findings[1].relatedDocIds.contains("1") + assertTrue("Findings saved for test monitor", findings0) + assertTrue("Findings saved for test monitor", findings1) } fun `test document-level monitor when index alias contain docs that do match a NOT EQUALS query and EXISTS query`() { From 1efdc273522a52190db0ef770aa47eb098eaaeda Mon Sep 17 00:00:00 2001 From: Riya Saxena Date: Tue, 12 Mar 2024 18:59:33 -0700 Subject: [PATCH 3/3] fix findings API enhancemnts Signed-off-by: Riya Saxena --- .../transport/TransportGetFindingsAction.kt | 74 ++----------------- 1 file changed, 5 insertions(+), 69 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt index c9b6c8458..479f5e09d 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt @@ -40,6 +40,7 @@ import org.opensearch.commons.alerting.model.FindingWithDocs import org.opensearch.commons.utils.recreateObject import org.opensearch.core.action.ActionListener import org.opensearch.core.common.Strings +import org.opensearch.core.common.io.stream.NamedWriteableRegistry import org.opensearch.core.xcontent.NamedXContentRegistry import org.opensearch.core.xcontent.XContentParser import org.opensearch.core.xcontent.XContentParserUtils @@ -62,7 +63,8 @@ class TransportGetFindingsSearchAction @Inject constructor( clusterService: ClusterService, actionFilters: ActionFilters, val settings: Settings, - val xContentRegistry: NamedXContentRegistry + val xContentRegistry: NamedXContentRegistry, + val namedWriteableRegistry: NamedWriteableRegistry ) : HandledTransportAction ( AlertingActions.GET_FINDINGS_ACTION_NAME, transportService, actionFilters, ::GetFindingsRequest ), @@ -80,9 +82,8 @@ class TransportGetFindingsSearchAction @Inject constructor( actionListener: ActionListener ) { val getFindingsRequest = request as? GetFindingsRequest - ?: recreateObject(request) { GetFindingsRequest(it) } + ?: recreateObject(request, namedWriteableRegistry) { GetFindingsRequest(it) } val tableProp = getFindingsRequest.table - val searchString = tableProp.searchString val sortBuilder = SortBuilders .fieldSort(tableProp.sortString) @@ -99,81 +100,16 @@ class TransportGetFindingsSearchAction @Inject constructor( .seqNoAndPrimaryTerm(true) .version(true) - val queryBuilder = QueryBuilders.boolQuery() + val queryBuilder = getFindingsRequest.boolQueryBuilder ?: QueryBuilders.boolQuery() if (!getFindingsRequest.findingId.isNullOrBlank()) queryBuilder.filter(QueryBuilders.termQuery("_id", getFindingsRequest.findingId)) - - if (!getFindingsRequest.findingIds.isNullOrEmpty()) { - queryBuilder.filter(QueryBuilders.termsQuery("id", getFindingsRequest.findingIds)) - } - if (getFindingsRequest.monitorId != null) { queryBuilder.filter(QueryBuilders.termQuery("monitor_id", getFindingsRequest.monitorId)) } else if (getFindingsRequest.monitorIds.isNullOrEmpty() == false) { queryBuilder.filter(QueryBuilders.termsQuery("monitor_id", getFindingsRequest.monitorIds)) } - if (getFindingsRequest.startTime != null && getFindingsRequest.endTime != null) { - val startTime = getFindingsRequest.startTime!!.toEpochMilli() - val endTime = getFindingsRequest.endTime!!.toEpochMilli() - val timeRangeQuery = QueryBuilders.rangeQuery("timestamp") - .from(startTime) // Greater than or equal to start time - .to(endTime) // Less than or equal to end time - queryBuilder.filter(timeRangeQuery) - } - - if (!getFindingsRequest.detectionType.isNullOrBlank()) { - val detectionType = getFindingsRequest.detectionType - val nestedQueryBuilder = QueryBuilders.nestedQuery( - "queries", - when { - detectionType.equals("threat", ignoreCase = true) -> { - QueryBuilders.boolQuery().filter( - QueryBuilders.prefixQuery("queries.id", "threat_intel_") - ) - } - else -> { - QueryBuilders.boolQuery().mustNot( - QueryBuilders.prefixQuery("queries.id", "threat_intel_") - ) - } - }, - ScoreMode.None - ) - - // Add the nestedQueryBuilder to the main queryBuilder - queryBuilder.must(nestedQueryBuilder) - } - - if (!searchString.isNullOrBlank()) { - queryBuilder - .should(QueryBuilders.matchQuery("index", searchString)) - .should( - QueryBuilders.nestedQuery( - "queries", - QueryBuilders.matchQuery("queries.tags", searchString), - ScoreMode.None - ) - ) - .should(QueryBuilders.regexpQuery("monitor_name", searchString + ".*")) - .minimumShouldMatch(1) - } - - if (!getFindingsRequest.severity.isNullOrBlank()) { - val severity = getFindingsRequest.severity - queryBuilder - .must( - QueryBuilders.nestedQuery( - "queries", - QueryBuilders.boolQuery().should( - QueryBuilders.matchQuery("queries.tags", severity) - ), - ScoreMode.None - ) - ) - } - if (!tableProp.searchString.isNullOrBlank()) { queryBuilder .should(