Skip to content

Commit

Permalink
acknowledge api hangs when plugins.alerting.filter_by_backend_roles i…
Browse files Browse the repository at this point in the history
…s enabled

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>
  • Loading branch information
sbcd90 committed Dec 20, 2023
1 parent 903a6d9 commit 99d4329
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import org.opensearch.action.search.SearchResponse
import org.opensearch.action.support.ActionFilters
import org.opensearch.action.support.HandledTransportAction
import org.opensearch.action.update.UpdateRequest
import org.opensearch.alerting.opensearchapi.InjectorContextElement
import org.opensearch.alerting.opensearchapi.suspendUntil
import org.opensearch.alerting.opensearchapi.withClosableContext
import org.opensearch.alerting.settings.AlertingSettings
import org.opensearch.alerting.util.AlertingException
import org.opensearch.alerting.util.use
Expand Down Expand Up @@ -66,12 +68,19 @@ class TransportAcknowledgeAlertAction @Inject constructor(
val xContentRegistry: NamedXContentRegistry,
val transportGetMonitorAction: TransportGetMonitorAction
) : HandledTransportAction<ActionRequest, AcknowledgeAlertResponse>(
AlertingActions.ACKNOWLEDGE_ALERTS_ACTION_NAME, transportService, actionFilters, ::AcknowledgeAlertRequest
) {
AlertingActions.ACKNOWLEDGE_ALERTS_ACTION_NAME,
transportService,
actionFilters,
::AcknowledgeAlertRequest
),
SecureTransportAction {

@Volatile
private var isAlertHistoryEnabled = AlertingSettings.ALERT_HISTORY_ENABLED.get(settings)

@Volatile
override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings)

init {
clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.ALERT_HISTORY_ENABLED) { isAlertHistoryEnabled = it }
}
Expand All @@ -83,8 +92,9 @@ class TransportAcknowledgeAlertAction @Inject constructor(
) {
val request = acknowledgeAlertRequest as? AcknowledgeAlertRequest
?: recreateObject(acknowledgeAlertRequest) { AcknowledgeAlertRequest(it) }
client.threadPool().threadContext.stashContext().use {
scope.launch {
val user = readUserFromThreadContext(client)
scope.launch {
withClosableContext(InjectorContextElement(request.monitorId, settings, client.threadPool().threadContext, user?.roles)) {
val getMonitorResponse: GetMonitorResponse =
transportGetMonitorAction.client.suspendUntil {
val getMonitorRequest = GetMonitorRequest(
Expand All @@ -108,7 +118,9 @@ class TransportAcknowledgeAlertAction @Inject constructor(
)
)
} else {
AcknowledgeHandler(client, actionListener, request).start(getMonitorResponse.monitor!!)
client.threadPool().threadContext.stashContext().use {
AcknowledgeHandler(client, actionListener, request).start(getMonitorResponse.monitor!!)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,24 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}
}

fun `test ack alert with enabled filter by`() {
enableFilterBy()
putAlertMappings()
val adminUser = User(ADMIN, listOf(ADMIN), listOf(ALL_ACCESS_ROLE), listOf())
val monitor = createRandomMonitor(refresh = true).copy(user = adminUser)
val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE))

val inputMap = HashMap<String, Any>()
inputMap["missing"] = "_last"
inputMap["monitorId"] = monitor.id

// search as "admin" - must get 4 docs
val adminResponseMap = getAlerts(client(), inputMap).asMap()
assertEquals(1, adminResponseMap["totalAlerts"])

acknowledgeAlerts(monitor, alert)
}

fun `test get alerts with an user with get alerts role`() {

putAlertMappings()
Expand Down

0 comments on commit 99d4329

Please sign in to comment.