From c97aa530482eabbf4c85041f31b0f05accba2e5e Mon Sep 17 00:00:00 2001 From: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> Date: Tue, 21 Jan 2025 14:38:56 -0800 Subject: [PATCH 1/4] fix: add input validations to monitor fields before creating/updating a monitor. add tests to ensure that functionality of existing monitors do not break. Signed-off-by: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> --- .../resthandler/RestIndexMonitorAction.kt | 80 ++++++- .../alerting/resthandler/MonitorRestApiIT.kt | 200 ++++++++++++++++++ 2 files changed, 279 insertions(+), 1 deletion(-) 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 e9b973c9c..1134c43c3 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt @@ -24,7 +24,9 @@ import org.opensearch.commons.alerting.model.ScheduledJob import org.opensearch.commons.alerting.util.AlertingException import org.opensearch.commons.alerting.util.isMonitorOfStandardType import org.opensearch.commons.utils.getInvalidNameChars +import org.opensearch.commons.utils.isValidId import org.opensearch.commons.utils.isValidName +import org.opensearch.commons.utils.isValidQueryName import org.opensearch.core.rest.RestStatus import org.opensearch.core.xcontent.ToXContent import org.opensearch.core.xcontent.XContentParser.Token @@ -86,6 +88,14 @@ class RestIndexMonitorAction : BaseRestHandler() { throw AlertingException.wrap(IllegalArgumentException("Missing monitor ID")) } + // Check if the ID is valid + if (request.method() == PUT && !isValidId(id)) { + throw IllegalArgumentException( + "Invalid monitor ID [$id]. " + + "Monitor ID should be alphanumeric string with +, /, _, or - characters only" + ) + } + // Validate request by parsing JSON to Monitor val xcp = request.contentParser() ensureExpectedToken(Token.START_OBJECT, xcp.nextToken(), xcp) @@ -95,6 +105,14 @@ class RestIndexMonitorAction : BaseRestHandler() { try { monitor = Monitor.parse(xcp, id).copy(lastUpdateTime = Instant.now()) + // Validate if the monitor name is valid + if (!isValidName(monitor.name)) { + throw IllegalArgumentException( + "Invalid monitor name [${monitor.name}]. " + + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore" + ) + } + rbacRoles = request.contentParser().map()["rbac_roles"] as List? validateDataSources(monitor) @@ -108,6 +126,21 @@ class RestIndexMonitorAction : BaseRestHandler() { if (it !is QueryLevelTrigger) { throw (IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for query level monitor")) } + if (!isValidName(it.name)) { + throw IllegalArgumentException( + "Invalid trigger name [${it.name}]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + ) + } + it.actions.forEach { action -> + val destinationId = action.destinationId + if (!isValidId(destinationId)) { + throw IllegalArgumentException( + "Invalid destination ID [$destinationId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only" + ) + } + } } } @@ -116,6 +149,21 @@ class RestIndexMonitorAction : BaseRestHandler() { if (it !is BucketLevelTrigger) { throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for bucket level monitor") } + if (!isValidName(it.name)) { + throw IllegalArgumentException( + "Invalid trigger name [${it.name}]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + ) + } + it.actions.forEach { action -> + val destinationId = action.destinationId + if (!isValidId(destinationId)) { + throw IllegalArgumentException( + "Invalid destination ID [$destinationId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only" + ) + } + } } } @@ -124,6 +172,21 @@ class RestIndexMonitorAction : BaseRestHandler() { if (it !is QueryLevelTrigger) { throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for cluster metrics monitor") } + if (!isValidName(it.name)) { + throw IllegalArgumentException( + "Invalid trigger name [${it.name}]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + ) + } + it.actions.forEach { action -> + val destinationId = action.destinationId + if (!isValidId(destinationId)) { + throw IllegalArgumentException( + "Invalid destination ID [$destinationId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only" + ) + } + } } } @@ -133,6 +196,21 @@ class RestIndexMonitorAction : BaseRestHandler() { if (it !is DocumentLevelTrigger) { throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor") } + if (!isValidName(it.name)) { + throw IllegalArgumentException( + "Invalid trigger name [${it.name}]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + ) + } + it.actions.forEach { action -> + val destinationId = action.destinationId + if (!isValidId(destinationId)) { + throw IllegalArgumentException( + "Invalid destination ID [$destinationId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only" + ) + } + } } } } @@ -158,7 +236,7 @@ class RestIndexMonitorAction : BaseRestHandler() { private fun validateDocLevelQueryName(monitor: Monitor) { monitor.inputs.filterIsInstance().forEach { docLevelMonitorInput -> docLevelMonitorInput.queries.forEach { dlq -> - if (!isValidName(dlq.name)) { + if (!isValidQueryName(dlq.name)) { throw IllegalArgumentException( "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 79c871a97..75eb889e0 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -39,23 +39,35 @@ import org.opensearch.alerting.toJsonString import org.opensearch.alerting.util.DestinationType import org.opensearch.client.ResponseException import org.opensearch.client.WarningFailureException +import org.opensearch.common.UUIDs import org.opensearch.common.unit.TimeValue +import org.opensearch.common.xcontent.LoggingDeprecationHandler +import org.opensearch.common.xcontent.XContentFactory.jsonBuilder import org.opensearch.common.xcontent.XContentType +import org.opensearch.common.xcontent.json.JsonXContent.jsonXContent import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.alerting.model.CronSchedule +import org.opensearch.commons.alerting.model.DataSources import org.opensearch.commons.alerting.model.DocLevelMonitorInput import org.opensearch.commons.alerting.model.DocLevelQuery import org.opensearch.commons.alerting.model.DocumentLevelTrigger +import org.opensearch.commons.alerting.model.IntervalSchedule import org.opensearch.commons.alerting.model.Monitor +import org.opensearch.commons.alerting.model.Monitor.Companion.NO_ID 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.alerting.util.IndexUtils.Companion.NO_SCHEMA_VERSION +import org.opensearch.commons.alerting.util.string import org.opensearch.commons.utils.getInvalidNameChars import org.opensearch.core.common.bytes.BytesReference import org.opensearch.core.rest.RestStatus +import org.opensearch.core.xcontent.NamedXContentRegistry import org.opensearch.core.xcontent.ToXContent import org.opensearch.core.xcontent.XContentBuilder +import org.opensearch.index.query.BoolQueryBuilder import org.opensearch.index.query.QueryBuilders +import org.opensearch.index.query.RangeQueryBuilder import org.opensearch.script.Script import org.opensearch.search.aggregations.AggregationBuilders import org.opensearch.search.builder.SearchSourceBuilder @@ -66,6 +78,8 @@ import org.opensearch.test.rest.OpenSearchRestTestCase import java.time.Instant import java.time.ZoneId import java.time.temporal.ChronoUnit +import java.time.temporal.ChronoUnit.MINUTES +import java.util.* import java.util.concurrent.TimeUnit @TestLogging("level:DEBUG", reason = "Debug for tests.") @@ -249,6 +263,192 @@ class MonitorRestApiIT : AlertingRestTestCase() { } } + fun `test creating a monitor with invalid monitor name`() { + val invalidName = """1~`!@#$%^&*()_+-=[]/<>?;':\"""" + val exception = assertThrows(ResponseException::class.java) { + createMonitor(randomQueryLevelMonitor(name = invalidName), refresh = true) + } + val errorResponse = createParser(XContentType.JSON.xContent(), exception.response.entity.content).map() + // Expected error + val expectedError = mapOf( + "error" to mapOf( + "reason" to "Invalid monitor name [$invalidName]. " + + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "caused_by" to mapOf( + "reason" to "java.lang.IllegalArgumentException: Invalid monitor name [$invalidName]. " + + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "type" to "exception" + ), + "type" to "alerting_exception", + "root_cause" to listOf( + mapOf( + "reason" to "Invalid monitor name [$invalidName]. " + + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "type" to "alerting_exception", + ) + ) + ), + "status" to 400 + ) + assertEquals(expectedError, errorResponse) + } + + fun `test creating a monitor with invalid trigger name`() { + val invalidName = """1~`!@#$%^&*()_+-=[]/<>?;':\"""" + val trigger = randomQueryLevelTrigger(name = invalidName) + val exception = assertThrows(ResponseException::class.java) { + createMonitor(randomQueryLevelMonitor(triggers = listOf(trigger)), refresh = true) + } + val errorResponse = createParser(XContentType.JSON.xContent(), exception.response.entity.content).map() + // Expected error + val expectedError = mapOf( + "error" to mapOf( + "reason" to "Invalid trigger name [$invalidName]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "caused_by" to mapOf( + "reason" to "java.lang.IllegalArgumentException: Invalid trigger name [$invalidName]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "type" to "exception" + ), + "type" to "alerting_exception", + "root_cause" to listOf( + mapOf( + "reason" to "Invalid trigger name [$invalidName]. " + + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "type" to "alerting_exception", + ) + ) + ), + "status" to 400 + ) + assertEquals(expectedError, errorResponse) + } + + fun `test creating a monitor with invalid destination id`() { + val invalidId = """1~`!@#$%^&*()_+-=[]/<>?;':\"""" + val trigger = randomQueryLevelTrigger(destinationId = invalidId) + val exception = assertThrows(ResponseException::class.java) { + createMonitor(randomQueryLevelMonitor(triggers = listOf(trigger)), refresh = true) + } + val errorResponse = createParser(XContentType.JSON.xContent(), exception.response.entity.content).map() + // Expected error + val expectedError = mapOf( + "error" to mapOf( + "reason" to "Invalid destination ID [$invalidId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "caused_by" to mapOf( + "reason" to "java.lang.IllegalArgumentException: Invalid destination ID [$invalidId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "type" to "exception" + ), + "type" to "alerting_exception", + "root_cause" to listOf( + mapOf( + "reason" to "Invalid destination ID [$invalidId]. " + + "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "type" to "alerting_exception", + ) + ) + ), + "status" to 400 + ) + assertEquals(expectedError, errorResponse) + } + + protected fun Monitor.toJsonStringWithType(): String { + val builder = jsonBuilder() + return shuffleXContent( + toXContent(builder, ToXContent.MapParams(mapOf("with_type" to "true"))) + ).string() + } + + protected fun createMonitorUsingAdminClient(monitor: Monitor, refresh: Boolean = true): Monitor { + createAlertingConfigIndex() + + val response = indexDocWithAdminClient( + ScheduledJob.SCHEDULED_JOBS_INDEX, + UUIDs.base64UUID(), + monitor.toJsonStringWithType(), + refresh + + ) + + val monitorJson = jsonXContent.createParser( + NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, + response.entity.content + ).map() + + return monitor.copy( + id = monitorJson["_id"] as String, + version = (monitorJson["_version"] as Int).toLong() + ) + } + + fun `test existing monitors do not break with new validations`() { + val invalidValue = "~'`!@#$%^&*()_+-=[]/<>?;':\"" + val monitor = Monitor( + id = NO_ID, + version = 1L, + name = invalidValue, + enabled = true, + schedule = IntervalSchedule( + interval = 1, + unit = MINUTES + ), + lastUpdateTime = Instant.now(), + enabledTime = Instant.now(), + monitorType = Monitor.MonitorType.QUERY_LEVEL_MONITOR.value, + user = null, + schemaVersion = NO_SCHEMA_VERSION, + inputs = listOf( + SearchInput( + indices = listOf(invalidValue), + query = SearchSourceBuilder().apply { + size(2147483647) + query( + BoolQueryBuilder().filter( + RangeQueryBuilder("order_date") + .gte(invalidValue) + .lte(invalidValue) + ) + ) + aggregations() + } + ) + ), + triggers = listOf( + QueryLevelTrigger( + id = UUID.randomUUID().toString(), + name = invalidValue, + severity = invalidValue, + condition = ALWAYS_RUN, + actions = listOf() + ) + ), + uiMetadata = mapOf(), + dataSources = DataSources(), + deleteQueryIndexInEveryRun = false, + shouldCreateSingleAlertForFindings = false, + owner = "alerting" + ) + + // Monitor should be created + val createdMonitor = createMonitorUsingAdminClient(monitor) + assertNotNull("Created monitor should have an ID", createdMonitor.id) + + // getMonitor should work + val retrievedMonitor = getMonitor(createdMonitor.id) + assertEquals("Retrieved monitor should have the same name", retrievedMonitor.name, invalidValue) + + // executeMonitor should work + val executedMonitor = executeMonitor(createdMonitor.id) + assertEquals("Monitor execution should return OK status", RestStatus.OK, executedMonitor.restStatus()) + + // searchAlerts should work + val alerts = searchAlerts(createdMonitor, ScheduledJob.SCHEDULED_JOBS_INDEX) + assertEquals("No alerts raised, but searchAlerts must work", 0, alerts.size) + } + /* fun `test creating an AD monitor with detector has monitor backend role`() { createAnomalyDetectorIndex() From 0a073966e75e0cd155243e1472028f247c6f200a Mon Sep 17 00:00:00 2001 From: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> Date: Fri, 31 Jan 2025 16:09:58 -0800 Subject: [PATCH 2/4] fix: update tests to conform to new validations enforced Signed-off-by: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> --- .../kotlin/org/opensearch/alerting/TestHelpers.kt | 14 +++++++------- .../alerting/resthandler/MonitorRestApiIT.kt | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt b/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt index 2330974f4..346546d7a 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt @@ -298,7 +298,7 @@ fun randomQueryLevelTrigger( severity: String = "1", condition: Script = randomScript(), actions: List = mutableListOf(), - destinationId: String = "" + destinationId: String = "sample" ): QueryLevelTrigger { return QueryLevelTrigger( id = id, @@ -315,7 +315,7 @@ fun randomBucketLevelTrigger( severity: String = "1", bucketSelector: BucketSelectorExtAggregationBuilder = randomBucketSelectorExtAggregationBuilder(name = id), actions: List = mutableListOf(), - destinationId: String = "" + destinationId: String = "sample" ): BucketLevelTrigger { return BucketLevelTrigger( id = id, @@ -326,7 +326,7 @@ fun randomBucketLevelTrigger( ) } -fun randomActionsForBucketLevelTrigger(min: Int = 0, max: Int = 10, destinationId: String = ""): List = +fun randomActionsForBucketLevelTrigger(min: Int = 0, max: Int = 10, destinationId: String = "sample"): List = (min..randomInt(max)).map { randomActionWithPolicy(destinationId = destinationId) } fun randomDocumentLevelTrigger( @@ -335,7 +335,7 @@ fun randomDocumentLevelTrigger( severity: String = "1", condition: Script = randomScript(), actions: List = mutableListOf(), - destinationId: String = "" + destinationId: String = "sample" ): DocumentLevelTrigger { return DocumentLevelTrigger( id = id, @@ -424,7 +424,7 @@ fun randomTemplateScript( fun randomAction( name: String = OpenSearchRestTestCase.randomUnicodeOfLength(10), template: Script = randomTemplateScript("Hello World"), - destinationId: String = "", + destinationId: String = "sample", throttleEnabled: Boolean = false, throttle: Throttle = randomThrottle() ) = Action(name, destinationId, template, template, throttleEnabled, throttle, actionExecutionPolicy = null) @@ -432,7 +432,7 @@ fun randomAction( fun randomActionWithPolicy( name: String = OpenSearchRestTestCase.randomUnicodeOfLength(10), template: Script = randomTemplateScript("Hello World"), - destinationId: String = "", + destinationId: String = "sample", throttleEnabled: Boolean = false, throttle: Throttle = randomThrottle(), actionExecutionPolicy: ActionExecutionPolicy? = randomActionExecutionPolicy() @@ -773,7 +773,7 @@ fun randomChainedAlertTrigger( severity: String = "1", condition: Script = randomScript(), actions: List = mutableListOf(), - destinationId: String = "" + destinationId: String = "sample" ): ChainedAlertTrigger { return ChainedAlertTrigger( id = id, 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 75eb889e0..ec946d870 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -537,7 +537,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { val updatedTriggers = listOf( QueryLevelTrigger( - name = "foo", + name = "sample", severity = "1", condition = Script("return true"), actions = emptyList() @@ -1422,7 +1422,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { val updatedTriggers = listOf( DocumentLevelTrigger( - name = "foo", + name = "sample", severity = "1", condition = Script("return true"), actions = emptyList() From fb915f51e7ee2ccb2e050e93a9ae8e081369bcf7 Mon Sep 17 00:00:00 2001 From: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:14:05 -0800 Subject: [PATCH 3/4] fix: update tests to conform to the validations Signed-off-by: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> --- .../alerting/resthandler/MonitorRestApiIT.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 ec946d870..05ec6b556 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -1377,7 +1377,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { @Throws(Exception::class) fun `test creating a document monitor`() { val testIndex = createTestIndex() - val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf()) + val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "query-3", fields = listOf()) val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -1398,7 +1398,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { @Throws(Exception::class) fun `test getting a document level monitor`() { val testIndex = createTestIndex() - val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf()) + val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "query-3", fields = listOf()) val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -1414,7 +1414,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { @Throws(Exception::class) fun `test updating conditions for a doc-level monitor`() { val testIndex = createTestIndex() - val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf()) + val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "query-3", fields = listOf()) val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -1445,7 +1445,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { @Throws(Exception::class) fun `test deleting a document level monitor`() { val testIndex = createTestIndex() - val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf()) + val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "query-3", fields = listOf()) val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery)) val trigger = randomDocumentLevelTrigger(condition = ALWAYS_RUN) @@ -1509,7 +1509,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { // 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 = "validName)", fields = listOf()) val docLevelInput = DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery)) monitor = createMonitor(randomDocumentLevelMonitor(inputs = listOf(docLevelInput), triggers = listOf(trigger))) From 1cb8cd9ba042b7b466bd376095f507c8e0645a23 Mon Sep 17 00:00:00 2001 From: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:41:09 -0800 Subject: [PATCH 4/4] fix: fix nitpicks Signed-off-by: vikhy-aws <191836418+vikhy-aws@users.noreply.github.com> --- .../resthandler/RestIndexMonitorAction.kt | 28 +++++++++---------- .../alerting/resthandler/MonitorRestApiIT.kt | 18 ++++++------ 2 files changed, 23 insertions(+), 23 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 1134c43c3..0562eca13 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt @@ -92,7 +92,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (request.method() == PUT && !isValidId(id)) { throw IllegalArgumentException( "Invalid monitor ID [$id]. " + - "Monitor ID should be alphanumeric string with +, /, _, or - characters only" + "Monitor ID should be alphanumeric string with +, /, _, or - characters only." ) } @@ -109,7 +109,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (!isValidName(monitor.name)) { throw IllegalArgumentException( "Invalid monitor name [${monitor.name}]. " + - "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore" + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore." ) } @@ -124,12 +124,12 @@ class RestIndexMonitorAction : BaseRestHandler() { Monitor.MonitorType.QUERY_LEVEL_MONITOR -> { triggers.forEach { if (it !is QueryLevelTrigger) { - throw (IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for query level monitor")) + throw (IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for query level monitor.")) } if (!isValidName(it.name)) { throw IllegalArgumentException( "Invalid trigger name [${it.name}]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore." ) } it.actions.forEach { action -> @@ -137,7 +137,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (!isValidId(destinationId)) { throw IllegalArgumentException( "Invalid destination ID [$destinationId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only" + "Destination ID should be alphanumeric string with +, /, _, or - characters only." ) } } @@ -147,12 +147,12 @@ class RestIndexMonitorAction : BaseRestHandler() { Monitor.MonitorType.BUCKET_LEVEL_MONITOR -> { triggers.forEach { if (it !is BucketLevelTrigger) { - throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for bucket level monitor") + throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for bucket level monitor.") } if (!isValidName(it.name)) { throw IllegalArgumentException( "Invalid trigger name [${it.name}]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore." ) } it.actions.forEach { action -> @@ -160,7 +160,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (!isValidId(destinationId)) { throw IllegalArgumentException( "Invalid destination ID [$destinationId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only" + "Destination ID should be alphanumeric string with +, /, _, or - characters only." ) } } @@ -170,12 +170,12 @@ class RestIndexMonitorAction : BaseRestHandler() { Monitor.MonitorType.CLUSTER_METRICS_MONITOR -> { triggers.forEach { if (it !is QueryLevelTrigger) { - throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for cluster metrics monitor") + throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for cluster metrics monitor.") } if (!isValidName(it.name)) { throw IllegalArgumentException( "Invalid trigger name [${it.name}]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore." ) } it.actions.forEach { action -> @@ -183,7 +183,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (!isValidId(destinationId)) { throw IllegalArgumentException( "Invalid destination ID [$destinationId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only" + "Destination ID should be alphanumeric string with +, /, _, or - characters only." ) } } @@ -194,12 +194,12 @@ class RestIndexMonitorAction : BaseRestHandler() { validateDocLevelQueryName(monitor) triggers.forEach { if (it !is DocumentLevelTrigger) { - throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor") + throw IllegalArgumentException("Illegal trigger type, ${it.javaClass.name}, for document level monitor.") } if (!isValidName(it.name)) { throw IllegalArgumentException( "Invalid trigger name [${it.name}]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore" + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore." ) } it.actions.forEach { action -> @@ -207,7 +207,7 @@ class RestIndexMonitorAction : BaseRestHandler() { if (!isValidId(destinationId)) { throw IllegalArgumentException( "Invalid destination ID [$destinationId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only" + "Destination ID should be alphanumeric string with +, /, _, or - characters only." ) } } 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 05ec6b556..62b067584 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -273,17 +273,17 @@ class MonitorRestApiIT : AlertingRestTestCase() { val expectedError = mapOf( "error" to mapOf( "reason" to "Invalid monitor name [$invalidName]. " + - "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "caused_by" to mapOf( "reason" to "java.lang.IllegalArgumentException: Invalid monitor name [$invalidName]. " + - "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "type" to "exception" ), "type" to "alerting_exception", "root_cause" to listOf( mapOf( "reason" to "Invalid monitor name [$invalidName]. " + - "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Monitor Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "type" to "alerting_exception", ) ) @@ -304,17 +304,17 @@ class MonitorRestApiIT : AlertingRestTestCase() { val expectedError = mapOf( "error" to mapOf( "reason" to "Invalid trigger name [$invalidName]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "caused_by" to mapOf( "reason" to "java.lang.IllegalArgumentException: Invalid trigger name [$invalidName]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "type" to "exception" ), "type" to "alerting_exception", "root_cause" to listOf( mapOf( "reason" to "Invalid trigger name [$invalidName]. " + - "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore", + "Trigger Name should be alphanumeric (4-50 chars) starting with letter or underscore.", "type" to "alerting_exception", ) ) @@ -335,17 +335,17 @@ class MonitorRestApiIT : AlertingRestTestCase() { val expectedError = mapOf( "error" to mapOf( "reason" to "Invalid destination ID [$invalidId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "Destination ID should be alphanumeric string with +, /, _, or - characters only.", "caused_by" to mapOf( "reason" to "java.lang.IllegalArgumentException: Invalid destination ID [$invalidId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "Destination ID should be alphanumeric string with +, /, _, or - characters only.", "type" to "exception" ), "type" to "alerting_exception", "root_cause" to listOf( mapOf( "reason" to "Invalid destination ID [$invalidId]. " + - "Destination ID should be alphanumeric string with +, /, _, or - characters only", + "Destination ID should be alphanumeric string with +, /, _, or - characters only.", "type" to "alerting_exception", ) )