updated integ tests

Signed-off-by: Amit Galitzky <[email protected]>
opensearch-project · Jul 10, 2023 · bf2c90f · bf2c90f
1 parent 25df4be
commit bf2c90f
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 38 deletions.
diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt b/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt
@@ -15,7 +15,7 @@ import org.opensearch.alerting.opensearchapi.suspendUntil
 import org.opensearch.alerting.util.AggregationQueryRewriter
 import org.opensearch.alerting.util.addUserBackendRolesFilter
 import org.opensearch.alerting.util.executeTransportAction
-import org.opensearch.alerting.util.getADBackendRoleFilterEnabled
+import org.opensearch.alerting.util.getRoleFilterEnabled
 import org.opensearch.alerting.util.toMap
 import org.opensearch.alerting.util.use
 import org.opensearch.alerting.workflow.WorkflowRunContext
@@ -207,7 +207,7 @@ class InputService(
                 // Monitor runner will send transport request to check permission first. If security plugin response
                 // is yes, user has permission to query AD result. If AD role filter enabled, we will add user role
                 // filter to protect data at user role level; otherwise, user can query any AD result.
-                if (getADBackendRoleFilterEnabled(clusterService, settings)) {
+                if (getRoleFilterEnabled(clusterService, settings, "plugins.anomaly_detection.filter_by_backend_roles")) {
                     addUserBackendRolesFilter(monitor.user, searchRequest.source())
                 }
                 val searchResponse: SearchResponse = client.suspendUntil { client.search(searchRequest, it) }

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt
@@ -45,7 +45,7 @@ import org.opensearch.alerting.util.AlertingException
 import org.opensearch.alerting.util.DocLevelMonitorQueries
 import org.opensearch.alerting.util.IndexUtils
 import org.opensearch.alerting.util.addUserBackendRolesFilter
-import org.opensearch.alerting.util.getADBackendRoleFilterEnabled
+import org.opensearch.alerting.util.getRoleFilterEnabled
 import org.opensearch.alerting.util.isADMonitor
 import org.opensearch.alerting.util.use
 import org.opensearch.client.Client
@@ -270,7 +270,7 @@ class TransportIndexMonitorAction @Inject constructor(
                     request.monitor = request.monitor
                         .copy(user = User(user.name, user.backendRoles, user.roles, user.customAttNames))
                     val searchSourceBuilder = SearchSourceBuilder().size(0)
-                    if (getADBackendRoleFilterEnabled(clusterService, settings)) {
+                    if (getRoleFilterEnabled(clusterService, settings, "plugins.anomaly_detection.filter_by_backend_roles")) {
                         addUserBackendRolesFilter(user, searchSourceBuilder)
                     }
                     val searchRequest = SearchRequest().indices(".opendistro-anomaly-detectors").source(searchSourceBuilder)

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt b/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt
@@ -9,6 +9,8 @@ import org.apache.logging.log4j.LogManager
 import org.opensearch.alerting.model.BucketLevelTriggerRunResult
 import org.opensearch.alerting.model.destination.Destination
 import org.opensearch.alerting.settings.DestinationSettings
+import org.opensearch.cluster.service.ClusterService
+import org.opensearch.common.settings.Settings
 import org.opensearch.common.util.concurrent.ThreadContext
 import org.opensearch.commons.alerting.model.AggregationResultBucket
 import org.opensearch.commons.alerting.model.Monitor
@@ -39,6 +41,28 @@ fun isValidEmail(email: String): Boolean {
     return validEmailPattern.matches(email)
 }
 
+fun getRoleFilterEnabled(clusterService: ClusterService, settings: Settings, settingPath: String): Boolean {
+    var adBackendRoleFilterEnabled: Boolean
+    val metaData = clusterService.state().metadata()
+
+    // get default value for setting
+    if (clusterService.clusterSettings.get(settingPath) != null) {
+        adBackendRoleFilterEnabled = clusterService.clusterSettings.get(settingPath).getDefault(settings) as Boolean
+    } else {
+        // default setting doesn't exist, so returning false as it means AD plugins isn't in cluster anyway
+        return false
+    }
+
+    // Transient settings are prioritized so those are checked first.
+    return if (metaData.transientSettings().get(settingPath) != null) {
+        metaData.transientSettings().getAsBoolean(settingPath, adBackendRoleFilterEnabled)
+    } else if (metaData.persistentSettings().get(settingPath) != null) {
+        metaData.persistentSettings().getAsBoolean(settingPath, adBackendRoleFilterEnabled)
+    } else {
+        adBackendRoleFilterEnabled
+    }
+}
+
 /** Allowed Destinations are ones that are specified in the [DestinationSettings.ALLOW_LIST] setting. */
 fun Destination.isAllowed(allowList: List<String>): Boolean = allowList.contains(this.type.value)
 

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt b/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt
@@ -6,8 +6,6 @@
 package org.opensearch.alerting.util
 
 import org.apache.lucene.search.join.ScoreMode
-import org.opensearch.cluster.service.ClusterService
-import org.opensearch.common.settings.Settings
 import org.opensearch.commons.alerting.model.Monitor
 import org.opensearch.commons.alerting.model.SearchInput
 import org.opensearch.commons.authuser.User
@@ -34,29 +32,6 @@ fun isADMonitor(monitor: Monitor): Boolean {
     return false
 }
 
-fun getADBackendRoleFilterEnabled(clusterService: ClusterService, settings: Settings): Boolean {
-    var adBackendRoleFilterEnabled: Boolean
-    val metaData = clusterService.state().metadata()
-    val adFilterString = "plugins.anomaly_detection.filter_by_backend_roles"
-
-    // get default value for setting
-    if (clusterService.clusterSettings.get(adFilterString) != null) {
-        adBackendRoleFilterEnabled = clusterService.clusterSettings.get(adFilterString).getDefault(settings) as Boolean
-    } else {
-        // default setting doesn't exist, so returning false as it means AD plugins isn't in cluster anyway
-        return false
-    }
-
-    // Transient settings are prioritized so those are checked first.
-    return if (metaData.transientSettings().get(adFilterString) != null) {
-        metaData.transientSettings().getAsBoolean(adFilterString, adBackendRoleFilterEnabled)
-    } else if (metaData.persistentSettings().get(adFilterString) != null) {
-        metaData.persistentSettings().getAsBoolean(adFilterString, adBackendRoleFilterEnabled)
-    } else {
-        adBackendRoleFilterEnabled
-    }
-}
-
 fun addUserBackendRolesFilter(user: User?, searchSourceBuilder: SearchSourceBuilder): SearchSourceBuilder {
     var boolQueryBuilder = BoolQueryBuilder()
     val userFieldName = "user"

diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerServiceIT.kt
@@ -1024,7 +1024,7 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
         }
     }
 
-    fun `test execute AD monitor doesn't return search result without user`() {
+    fun `test execute AD monitor returns search result without user`() {
         // TODO: change to REST API call to test security enabled case
         if (!securityEnabled()) {
             val user = randomUser()
@@ -1044,14 +1044,14 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
             val searchResult = (output.objectMap("input_results")["results"] as List<Map<String, Any>>).first()
             @Suppress("UNCHECKED_CAST")
             val total = searchResult.stringMap("hits")?.get("total") as Map<String, String>
-            assertEquals("Incorrect search result", 1, total["value"])
+            assertEquals("Incorrect search result", 5, total["value"])
             @Suppress("UNCHECKED_CAST")
             val maxAnomalyGrade = searchResult.stringMap("aggregations")?.get("max_anomaly_grade") as Map<String, String>
-            assertEquals("Incorrect search result", 0.75, maxAnomalyGrade["value"])
+            assertEquals("Incorrect search result", 0.9, maxAnomalyGrade["value"])
         }
     }
 
-    fun `test execute AD monitor doesn't return search result with empty backend role`() {
+    fun `test execute AD monitor returns search result with empty backend role`() {
         // TODO: change to REST API call to test security enabled case
         if (!securityEnabled()) {
             val user = randomUser()
@@ -1074,7 +1074,7 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
             val searchResult = (output.objectMap("input_results")["results"] as List<Map<String, Any>>).first()
             @Suppress("UNCHECKED_CAST")
             val total = searchResult.stringMap("hits")?.get("total") as Map<String, String>
-            assertEquals("Incorrect search result", 1, total["value"])
+            assertEquals("Incorrect search result", 5, total["value"])
             @Suppress("UNCHECKED_CAST")
             val maxAnomalyGrade = searchResult.stringMap("aggregations")?.get("max_anomaly_grade") as Map<String, String>
             assertEquals("Incorrect search result", 0.9, maxAnomalyGrade["value"])
@@ -1100,10 +1100,10 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
             val searchResult = (output.objectMap("input_results")["results"] as List<Map<String, Any>>).first()
             @Suppress("UNCHECKED_CAST")
             val total = searchResult.stringMap("hits")?.get("total") as Map<String, String>
-            assertEquals("Incorrect search result", 3, total["value"])
+            assertEquals("Incorrect search result", 5, total["value"])
             @Suppress("UNCHECKED_CAST")
             val maxAnomalyGrade = searchResult.stringMap("aggregations")?.get("max_anomaly_grade") as Map<String, String>
-            assertEquals("Incorrect search result", 0.8, maxAnomalyGrade["value"])
+            assertEquals("Incorrect search result", 0.9, maxAnomalyGrade["value"])
         }
     }
 
@@ -1123,13 +1123,13 @@ class MonitorRunnerServiceIT : AlertingRestTestCase() {
             @Suppress("UNCHECKED_CAST")
             (output["trigger_results"] as HashMap<String, Any>).forEach {
                     _, v ->
-                assertFalse((v as HashMap<String, Boolean>)["triggered"] as Boolean)
+                assertTrue((v as HashMap<String, Boolean>)["triggered"] as Boolean)
             }
             @Suppress("UNCHECKED_CAST")
             val searchResult = (output.objectMap("input_results")["results"] as List<Map<String, Any>>).first()
             @Suppress("UNCHECKED_CAST")
             val total = searchResult.stringMap("hits")?.get("total") as Map<String, String>
-            assertEquals("Incorrect search result", 0, total["value"])
+            assertEquals("Incorrect search result", 5, total["value"])
         }
     }