Skip to content

Commit

Permalink
Add validation check for doc level query name during monitor creation (
Browse files Browse the repository at this point in the history
…#1506)

* add validation check for doc level query name during monitor creation

Signed-off-by: Joanne Wang <jowg@amazon.com>

* change to 0-256 chars

Signed-off-by: Joanne Wang <jowg@amazon.com>

* change validation message and move validation loc

Signed-off-by: Joanne Wang <jowg@amazon.com>

---------

Signed-off-by: Joanne Wang <jowg@amazon.com>
(cherry picked from commit 636b43f)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Apr 19, 2024
1 parent 274b125 commit 81f4b3e
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
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
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

0 comments on commit 81f4b3e

Please sign in to comment.