[Backport 2.x] Added validation for the new clusters field. (#633) (#636

) * Added validation for the new clusters field. (#633) * Added validation for the clusters field. Refactored ClusterMetricsInput validiation to throw 4xx-level CommonUtilsExceptions instead of 5xx-level IllegalArgumentException. Signed-off-by: AWSHurneyt <[email protected]> * Moved some regex from alerting plugin to common utils. Signed-off-by: AWSHurneyt <[email protected]> * Moved cluster-based regex to separate file. Signed-off-by: AWSHurneyt <[email protected]> * Fixed ktlint error. Signed-off-by: AWSHurneyt <[email protected]> * Fixed regex. Moved cluster-related regexes. Signed-off-by: AWSHurneyt <[email protected]> --------- Signed-off-by: AWSHurneyt <[email protected]> * Removed CommonUtilsException. Team decided IllegalArgumentExceptions should be caught in the plugins themselves. Signed-off-by: AWSHurneyt <[email protected]> --------- Signed-off-by: AWSHurneyt <[email protected]>
opensearch-project · Apr 13, 2024 · 4e81d64 · 4e81d64
1 parent b8f1285
commit 4e81d64
Show file tree

Hide file tree

Showing 4 changed files with 161 additions and 2 deletions.
diff --git a/src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt b/src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt
@@ -3,6 +3,7 @@ package org.opensearch.commons.alerting.model
 import org.apache.commons.validator.routines.UrlValidator
 import org.apache.http.client.utils.URIBuilder
 import org.opensearch.common.CheckedFunction
+import org.opensearch.commons.utils.CLUSTER_NAME_REGEX
 import org.opensearch.core.ParseField
 import org.opensearch.core.common.io.stream.StreamInput
 import org.opensearch.core.common.io.stream.StreamOutput
@@ -58,6 +59,12 @@ data class ClusterMetricsInput(
             "Only port '$SUPPORTED_PORT' is supported."
         }
 
+        if (clusters.isNotEmpty()) {
+            require(clusters.all { CLUSTER_NAME_REGEX.matches(it) }) {
+                "Cluster names are not valid."
+            }
+        }
+
         clusterMetricType = findApiType(constructedUri.path)
         this.parseEmptyFields()
     }
@@ -158,7 +165,7 @@ data class ClusterMetricsInput(
     /**
      * Isolates just the path parameters from the [ClusterMetricsInput] URI.
      * @return The path parameters portion of the [ClusterMetricsInput] URI.
-     * @throws IllegalArgumentException if the [ClusterMetricType] requires path parameters, but none are supplied;
+     * @throws [IllegalArgumentException] if the [ClusterMetricType] requires path parameters, but none are supplied;
      * or when path parameters are provided for an [ClusterMetricType] that does not use path parameters.
      */
     fun parsePathParams(): String {
@@ -199,7 +206,7 @@ data class ClusterMetricsInput(
      * Examines the path of a [ClusterMetricsInput] to determine which API is being called.
      * @param uriPath The path to examine.
      * @return The [ClusterMetricType] associated with the [ClusterMetricsInput] monitor.
-     * @throws IllegalArgumentException when the API to call cannot be determined from the URI.
+     * @throws [IllegalArgumentException] when the API to call cannot be determined from the URI.
      */
     private fun findApiType(uriPath: String): ClusterMetricType {
         var apiType = ClusterMetricType.BLANK

diff --git a/src/main/kotlin/org/opensearch/commons/alerting/util/IndexUtils.kt b/src/main/kotlin/org/opensearch/commons/alerting/util/IndexUtils.kt
@@ -11,6 +11,24 @@ import java.time.Instant
 
 class IndexUtils {
     companion object {
+        /**
+         * This regex asserts that the string:
+         *  The index does not start with an underscore _, hyphen -, or plus sign +
+         *  The index does not contain two consecutive periods (e.g., `..`)
+         *  The index does not contain any whitespace characters, commas, backslashes, forward slashes, asterisks,
+         *   question marks, double quotes, less than or greater than signs, pipes, colons, or periods.
+         *  The length of the index must be between 1 and 255 characters
+         */
+        val VALID_INDEX_NAME_REGEX = Regex("""^(?![_\-\+])(?!.*\.\.)[^\s,\\\/\*\?"<>|#:\.]{1,255}$""")
+
+        /**
+         * This regex asserts that the string:
+         *  The index pattern can start with an optional period
+         *  The index pattern can contain lowercase letters, digits, underscores, hyphens, asterisks, and periods
+         *  The length of the index pattern must be between 1 and 255 characters
+         */
+        val INDEX_PATTERN_REGEX = Regex("""^(?=.{1,255}$)\.?[a-z0-9_\-\*\.]+$""")
+
         const val NO_SCHEMA_VERSION = 0
 
         const val MONITOR_MAX_INPUTS = 1

diff --git a/src/main/kotlin/org/opensearch/commons/utils/ValidationHelpers.kt b/src/main/kotlin/org/opensearch/commons/utils/ValidationHelpers.kt
@@ -8,6 +8,24 @@ package org.opensearch.commons.utils
 import java.net.URL
 import java.util.regex.Pattern
 
+/**
+ * This regex asserts that the string:
+ *  Starts with a lowercase letter, or digit
+ *  Contains a sequence of characters followed by an optional colon and another sequence of characters
+ *  The sequences of characters can include lowercase letters, uppercase letters, digits, underscores, or hyphens
+ *  The total length of the string can range from 1 to 255 characters
+ */
+val CLUSTER_NAME_REGEX = Regex("^(?=.{1,255}$)[a-z0-9]([a-zA-Z0-9_-]*:?[a-zA-Z0-9_-]*)$")
+
+/**
+ * This regex asserts that the string:
+ *  Starts with a lowercase letter, digit, or asterisk
+ *  Contains a sequence of characters followed by an optional colon and another sequence of characters
+ *  The sequences of characters can include lowercase letters, uppercase letters, digits, underscores, asterisks, or hyphens
+ *  The total length of the string can range from 1 to 255 characters
+ */
+val CLUSTER_PATTERN_REGEX = Regex("^(?=.{1,255}$)[a-z0-9*]([a-zA-Z0-9_*-]*:?[a-zA-Z0-9_*-]*)$")
+
 // Valid ID characters = (All Base64 chars + "_-") to support UUID format and Base64 encoded IDs
 private val VALID_ID_CHARS: Set<Char> = (('a'..'z') + ('A'..'Z') + ('0'..'9') + '+' + '/' + '_' + '-').toSet()
 

diff --git a/src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt b/src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt
@@ -9,6 +9,26 @@ class ClusterMetricsInputTests {
     private var pathParams = ""
     private var url = ""
 
+    private val validClusters = listOf(
+        "cluster-name",
+        "cluster:name"
+    )
+
+    private val invalidClusters = listOf(
+        // Character length less than 1 should return FALSE
+        "",
+
+        // Character length greater than 255 should return FALSE
+        (0..255).joinToString(separator = "") { "a" },
+
+        // Invalid characters should return FALSE
+        "cluster-#name",
+        "cluster:#name",
+
+        // More than 1 `:` character should return FALSE
+        "bad:cluster:name"
+    )
+
     @Test
     fun `test valid ClusterMetricsInput creation using HTTP URI component fields`() {
         // GIVEN
@@ -21,6 +41,7 @@ class ClusterMetricsInputTests {
         assertEquals(path, clusterMetricsInput.path)
         assertEquals(pathParams, clusterMetricsInput.pathParams)
         assertEquals(testUrl, clusterMetricsInput.url)
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -34,6 +55,7 @@ class ClusterMetricsInputTests {
 
         // THEN
         assertEquals(url, clusterMetricsInput.url)
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -47,6 +69,7 @@ class ClusterMetricsInputTests {
 
         // THEN
         assertEquals(url, clusterMetricsInput.url)
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -84,6 +107,7 @@ class ClusterMetricsInputTests {
         assertEquals(pathParams, clusterMetricsInput.pathParams)
         assertEquals(url, clusterMetricsInput.url)
         assertEquals(url, clusterMetricsInput.constructedUri.toString())
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -101,6 +125,7 @@ class ClusterMetricsInputTests {
         assertEquals(pathParams, clusterMetricsInput.pathParams)
         assertEquals(url, clusterMetricsInput.url)
         assertEquals(url, clusterMetricsInput.constructedUri.toString())
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -200,6 +225,7 @@ class ClusterMetricsInputTests {
         // THEN
         assertEquals(pathParams, params)
         assertEquals(testUrl, clusterMetricsInput.constructedUri.toString())
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -216,6 +242,7 @@ class ClusterMetricsInputTests {
         // THEN
         assertEquals(pathParams, params)
         assertEquals(testUrl, clusterMetricsInput.constructedUri.toString())
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -232,6 +259,7 @@ class ClusterMetricsInputTests {
         // THEN
         assertEquals(testParams, params)
         assertEquals(url, clusterMetricsInput.constructedUri.toString())
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -422,6 +450,7 @@ class ClusterMetricsInputTests {
         assertEquals(testPath, clusterMetricsInput.path)
         assertEquals(testPathParams, clusterMetricsInput.pathParams)
         assertEquals(url, clusterMetricsInput.url)
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
     }
 
     @Test
@@ -438,5 +467,92 @@ class ClusterMetricsInputTests {
         assertEquals(path, clusterMetricsInput.path)
         assertEquals(pathParams, clusterMetricsInput.pathParams)
         assertEquals(testUrl, clusterMetricsInput.url)
+        assertEquals(emptyList(), clusterMetricsInput.clusters)
+    }
+
+    @Test
+    fun `test a single valid cluster`() {
+        validClusters.forEach {
+            // GIVEN
+            path = "/_cluster/health"
+            pathParams = "index1,index2,index3,index4,index5"
+            url = ""
+            val clusters = listOf(it)
+
+            // WHEN
+            val clusterMetricsInput = ClusterMetricsInput(
+                path = path,
+                pathParams = pathParams,
+                url = url,
+                clusters = clusters
+            )
+
+            // THEN
+            assertEquals(path, clusterMetricsInput.path)
+            assertEquals(pathParams, clusterMetricsInput.pathParams)
+            assertEquals(clusters, clusterMetricsInput.clusters)
+        }
+    }
+
+    @Test
+    fun `test multiple valid clusters`() {
+        // GIVEN
+        path = "/_cluster/health"
+        pathParams = "index1,index2,index3,index4,index5"
+        url = ""
+        val clusters = validClusters
+
+        // WHEN
+        val clusterMetricsInput = ClusterMetricsInput(
+            path = path,
+            pathParams = pathParams,
+            url = url,
+            clusters = clusters
+        )
+
+        // THEN
+        assertEquals(path, clusterMetricsInput.path)
+        assertEquals(pathParams, clusterMetricsInput.pathParams)
+        assertEquals(clusters, clusterMetricsInput.clusters)
+    }
+
+    @Test
+    fun `test a single invalid cluster`() {
+        invalidClusters.forEach {
+            // GIVEN
+            path = "/_cluster/health"
+            pathParams = "index1,index2,index3,index4,index5"
+            url = ""
+            val clusters = listOf(it)
+
+            // WHEN + THEN
+            assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
+                ClusterMetricsInput(
+                    path = path,
+                    pathParams = pathParams,
+                    url = url,
+                    clusters = clusters
+                )
+            }
+        }
+    }
+
+    @Test
+    fun `test multiple invalid clusters`() {
+        // GIVEN
+        path = "/_cluster/health"
+        pathParams = "index1,index2,index3,index4,index5"
+        url = ""
+        val clusters = invalidClusters
+
+        // WHEN + THEN
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
+            ClusterMetricsInput(
+                path = path,
+                pathParams = pathParams,
+                url = url,
+                clusters = clusters
+            )
+        }
     }
 }