From b490cf7a299c461963aa1008bee67b4a331b4d1b Mon Sep 17 00:00:00 2001 From: Joanne Wang Date: Mon, 8 Apr 2024 15:23:38 -0700 Subject: [PATCH] change validation message and move validation loc Signed-off-by: Joanne Wang --- .../resthandler/RestIndexMonitorAction.kt | 15 ++++++--------- .../alerting/resthandler/MonitorRestApiIT.kt | 11 +++++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt index edc834a40..03b5e09be 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt @@ -21,6 +21,8 @@ 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 @@ -47,11 +49,6 @@ private val log = LogManager.getLogger(RestIndexMonitorAction::class.java) */ class RestIndexMonitorAction : BaseRestHandler() { - // allowed characters [- : , ( ) [ ] ' _] - private val allowedChars = "-:,\\(\\)\\[\\]\'_" - // regex to restrict string to alphanumeric and allowed chars, must be between 0 - 256 characters - val regex = "[\\w\\s$allowedChars]{0,256}" - override fun getName(): String { return "index_monitor_action" } @@ -122,8 +119,8 @@ class RestIndexMonitorAction : BaseRestHandler() { if (it !is DocumentLevelTrigger) { throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor") } - validateDocLevelQueryName(monitor) } + validateDocLevelQueryName(monitor) } } @@ -144,10 +141,10 @@ class RestIndexMonitorAction : BaseRestHandler() { private fun validateDocLevelQueryName(monitor: Monitor) { monitor.inputs.filterIsInstance().forEach { docLevelMonitorInput -> docLevelMonitorInput.queries.forEach { dlq -> - if (!dlq.name.matches(Regex(regex))) { + if (!isValidName(dlq.name)) { throw IllegalArgumentException( - "Doc level query name, ${dlq.name}, may only contain alphanumeric values and " + - "these special characters: ${allowedChars.replace("\\","")}" + "Doc level query name may not start with [_, +, -], contain '..', or contain: " + + getInvalidNameChars().replace("\\", "") ) } } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt index a298b4b3f..79c871a97 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -50,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 @@ -1289,7 +1290,7 @@ 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 ! name" + val invalidQueryName = "_Invalid .. query ! n>ame" val queries = listOf(randomDocLevelQuery(name = invalidQueryName)) val randomDocLevelMonitorInput = randomDocLevelMonitorInput(queries = queries) val inputs = listOf(randomDocLevelMonitorInput) @@ -1301,13 +1302,14 @@ class MonitorRestApiIT : AlertingRestTestCase() { 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, $invalidQueryName, may only contain alphanumeric values" + 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 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))) @@ -1324,7 +1326,8 @@ class MonitorRestApiIT : AlertingRestTestCase() { 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, $invalidQueryName, may only contain alphanumeric values" + val expectedMessage = "Doc level query name may not start with [_, +, -], contain '..', or contain: " + + getInvalidNameChars().replace("\\", "") e.message?.let { assertTrue(it.contains(expectedMessage)) } } }