Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Backport 2.11] Add validation check for doc level query name during monitor creation #1524

Open
wants to merge 1 commit into
base: 2.11
Choose a base branch
from
Open

[Backport 2.11] Add validation check for doc level query name during monitor creation #1524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@ import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.IndexMonitorRequest
import org.opensearch.commons.alerting.action.IndexMonitorResponse
import org.opensearch.commons.alerting.model.BucketLevelTrigger
import org.opensearch.commons.alerting.model.DocLevelMonitorInput
import org.opensearch.commons.alerting.model.DocumentLevelTrigger
import org.opensearch.commons.alerting.model.Monitor
import org.opensearch.commons.alerting.model.QueryLevelTrigger
import org.opensearch.commons.alerting.model.ScheduledJob
import org.opensearch.commons.utils.getInvalidNameChars
import org.opensearch.commons.utils.isValidName
import org.opensearch.core.rest.RestStatus
import org.opensearch.core.xcontent.ToXContent
import org.opensearch.core.xcontent.XContentParser.Token
Expand Down Expand Up @@ -118,8 +121,10 @@ class RestIndexMonitorAction : BaseRestHandler() {
throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor")
}
}
validateDocLevelQueryName(monitor)
}
}

val seqNo = request.paramAsLong(IF_SEQ_NO, SequenceNumbers.UNASSIGNED_SEQ_NO)
val primaryTerm = request.paramAsLong(IF_PRIMARY_TERM, SequenceNumbers.UNASSIGNED_PRIMARY_TERM)
val refreshPolicy = if (request.hasParam(REFRESH)) {
Expand All @@ -134,6 +139,19 @@ class RestIndexMonitorAction : BaseRestHandler() {
}
}

private fun validateDocLevelQueryName(monitor: Monitor) {
monitor.inputs.filterIsInstance<DocLevelMonitorInput>().forEach { docLevelMonitorInput ->
docLevelMonitorInput.queries.forEach { dlq ->
if (!isValidName(dlq.name)) {
throw IllegalArgumentException(
"Doc level query name may not start with [_, +, -], contain '..', or contain: " +
getInvalidNameChars().replace("\\", "")
)
}
}
}
}

private fun validateDataSources(monitor: Monitor) { // Data Sources will currently be supported only at transport layer.
if (monitor.dataSources != null) {
if (
Expand Down
47 changes: 47 additions & 0 deletions alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import org.opensearch.alerting.randomAnomalyDetector
import org.opensearch.alerting.randomAnomalyDetectorWithUser
import org.opensearch.alerting.randomBucketLevelMonitor
import org.opensearch.alerting.randomBucketLevelTrigger
import org.opensearch.alerting.randomDocLevelMonitorInput
import org.opensearch.alerting.randomDocLevelQuery
import org.opensearch.alerting.randomDocumentLevelMonitor
import org.opensearch.alerting.randomDocumentLevelTrigger
import org.opensearch.alerting.randomQueryLevelMonitor
Expand All @@ -48,6 +50,7 @@ import org.opensearch.commons.alerting.model.Monitor
import org.opensearch.commons.alerting.model.QueryLevelTrigger
import org.opensearch.commons.alerting.model.ScheduledJob
import org.opensearch.commons.alerting.model.SearchInput
import org.opensearch.commons.utils.getInvalidNameChars
import org.opensearch.core.common.bytes.BytesReference
import org.opensearch.core.rest.RestStatus
import org.opensearch.core.xcontent.ToXContent
Expand Down Expand Up @@ -1266,6 +1269,50 @@ class MonitorRestApiIT : AlertingRestTestCase() {
}
}

fun `test creating and updating a document monitor with invalid query name`() {
// creating a monitor with an invalid query name
val invalidQueryName = "_Invalid .. query ! n>ame"
val queries = listOf(randomDocLevelQuery(name = invalidQueryName))
val randomDocLevelMonitorInput = randomDocLevelMonitorInput(queries = queries)
val inputs = listOf(randomDocLevelMonitorInput)
val trigger = randomDocumentLevelTrigger()
var monitor = randomDocumentLevelMonitor(inputs = inputs, triggers = listOf(trigger))

try {
client().makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity())
fail("Doc level monitor with invalid query name should be rejected")
} catch (e: ResponseException) {
assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
val expectedMessage = "Doc level query name may not start with [_, +, -], contain '..', or contain: " +
getInvalidNameChars().replace("\\", "")
e.message?.let { assertTrue(it.contains(expectedMessage)) }
}

// successfully creating monitor with valid query name
val testIndex = createTestIndex()
val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "valid (name)", fields = listOf())
val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery))

monitor = createMonitor(randomDocumentLevelMonitor(inputs = listOf(docLevelInput), triggers = listOf(trigger)))

// updating monitor with invalid query name
val updatedDocQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = invalidQueryName, fields = listOf())
val updatedDocLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(updatedDocQuery))

try {
client().makeRequest(
"PUT", monitor.relativeUrl(),
emptyMap(), monitor.copy(inputs = listOf(updatedDocLevelInput)).toHttpEntity()
)
fail("Doc level monitor with invalid query name should be rejected")
} catch (e: ResponseException) {
assertEquals("Unexpected status", RestStatus.BAD_REQUEST, e.response.restStatus())
val expectedMessage = "Doc level query name may not start with [_, +, -], contain '..', or contain: " +
getInvalidNameChars().replace("\\", "")
e.message?.let { assertTrue(it.contains(expectedMessage)) }
}
}

/**
* This use case is needed by the frontend plugin for displaying alert counts on the Monitors list page.
* https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/server/services/MonitorService.js#L235
Expand Down
Loading