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]>
opensearch-project · Apr 12, 2024 · abc69cd · abc69cd
1 parent 9beae87
commit abc69cd
Show file tree

Hide file tree

Showing 5 changed files with 287 additions and 46 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,8 @@ package org.opensearch.commons.alerting.model
 import org.apache.commons.validator.routines.UrlValidator
 import org.apache.hc.core5.net.URIBuilder
 import org.opensearch.common.CheckedFunction
+import org.opensearch.commons.alerting.util.CommonUtilsException
+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
@@ -31,35 +33,46 @@ data class ClusterMetricsInput(
 
     // Verify parameters are valid during creation
     init {
-        require(validateFields()) {
-            "The uri.api_type field, uri.path field, or uri.uri field must be defined."
-        }
+        // Wrap any validation exceptions in CommonUtilsException.
+        try {
+            require(validateFields()) {
+                "The uri.api_type field, uri.path field, or uri.uri field must be defined."
+            }
 
-        // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs.
-        val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS)
+            // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs.
+            val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS)
 
-        // Build url field by field if not provided as whole.
-        constructedUri = toConstructedUri()
+            // Build url field by field if not provided as whole.
+            constructedUri = toConstructedUri()
 
-        require(urlValidator.isValid(constructedUri.toString())) {
-            "Invalid URI constructed from the path and path_params inputs, or the url input."
-        }
+            require(urlValidator.isValid(constructedUri.toString())) {
+                "Invalid URI constructed from the path and path_params inputs, or the url input."
+            }
 
-        if (url.isNotEmpty() && validateFieldsNotEmpty()) {
-            require(constructedUri == constructUrlFromInputs()) {
-                "The provided URL and URI fields form different URLs."
+            if (url.isNotEmpty() && validateFieldsNotEmpty()) {
+                require(constructedUri == constructUrlFromInputs()) {
+                    "The provided URL and URI fields form different URLs."
+                }
             }
-        }
 
-        require(constructedUri.host.lowercase() == SUPPORTED_HOST) {
-            "Only host '$SUPPORTED_HOST' is supported."
-        }
-        require(constructedUri.port == SUPPORTED_PORT) {
-            "Only port '$SUPPORTED_PORT' is supported."
-        }
+            require(constructedUri.host.lowercase() == SUPPORTED_HOST) {
+                "Only host '$SUPPORTED_HOST' is supported."
+            }
+            require(constructedUri.port == SUPPORTED_PORT) {
+                "Only port '$SUPPORTED_PORT' is supported."
+            }
 
-        clusterMetricType = findApiType(constructedUri.path)
-        this.parseEmptyFields()
+            if (clusters.isNotEmpty()) {
+                require(clusters.all { CLUSTER_NAME_REGEX.matches(it) }) {
+                    "Cluster names are not valid."
+                }
+            }
+
+            clusterMetricType = findApiType(constructedUri.path)
+            this.parseEmptyFields()
+        } catch (exception: Exception) {
+            throw CommonUtilsException.wrap(exception)
+        }
     }
 
     @Throws(IOException::class)
@@ -158,7 +171,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 CommonUtilsException 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 {
@@ -178,18 +191,22 @@ data class ClusterMetricsInput(
             pathParams = pathParams.trim('/')
             ILLEGAL_PATH_PARAMETER_CHARACTERS.forEach { character ->
                 if (pathParams.contains(character)) {
-                    throw IllegalArgumentException(
-                        "The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")
+                    throw CommonUtilsException.wrap(
+                        IllegalArgumentException(
+                            "The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(
+                                " "
+                            )
+                        )
                     )
                 }
             }
         }
 
         if (apiType.requiresPathParams && pathParams.isEmpty()) {
-            throw IllegalArgumentException("The API requires path parameters.")
+            throw CommonUtilsException.wrap(IllegalArgumentException("The API requires path parameters."))
         }
         if (!apiType.supportsPathParams && pathParams.isNotEmpty()) {
-            throw IllegalArgumentException("The API does not use path parameters.")
+            throw CommonUtilsException.wrap(IllegalArgumentException("The API does not use path parameters."))
         }
 
         return pathParams
@@ -199,7 +216,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 CommonUtilsException when the API to call cannot be determined from the URI.
      */
     private fun findApiType(uriPath: String): ClusterMetricType {
         var apiType = ClusterMetricType.BLANK
@@ -211,7 +228,7 @@ data class ClusterMetricsInput(
                 }
             }
         if (apiType.isBlank()) {
-            throw IllegalArgumentException("The API could not be determined from the provided URI.")
+            throw CommonUtilsException.wrap(IllegalArgumentException("The API could not be determined from the provided URI."))
         }
         return apiType
     }

diff --git a/src/main/kotlin/org/opensearch/commons/alerting/util/CommonUtilsException.kt b/src/main/kotlin/org/opensearch/commons/alerting/util/CommonUtilsException.kt
@@ -0,0 +1,71 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.commons.alerting.util
+
+import org.apache.logging.log4j.LogManager
+import org.opensearch.OpenSearchException
+import org.opensearch.OpenSearchSecurityException
+import org.opensearch.OpenSearchStatusException
+import org.opensearch.core.common.Strings
+import org.opensearch.core.rest.RestStatus
+import org.opensearch.index.IndexNotFoundException
+import org.opensearch.index.engine.VersionConflictEngineException
+import org.opensearch.indices.InvalidIndexNameException
+
+private val log = LogManager.getLogger(CommonUtilsException::class.java)
+
+class CommonUtilsException(message: String, val status: RestStatus, ex: Exception) : OpenSearchException(message, ex) {
+
+    override fun status(): RestStatus {
+        return status
+    }
+
+    companion object {
+
+        @JvmStatic
+        fun wrap(ex: Exception): OpenSearchException {
+            log.error("Common utils error: $ex")
+
+            var friendlyMsg = "Unknown error"
+            var status = RestStatus.INTERNAL_SERVER_ERROR
+            when (ex) {
+                is IndexNotFoundException -> {
+                    status = ex.status()
+                    friendlyMsg = "Configured indices are not found: ${ex.index}"
+                }
+                is OpenSearchSecurityException -> {
+                    status = ex.status()
+                    friendlyMsg = "User doesn't have permissions to execute this action. Contact administrator."
+                }
+                is OpenSearchStatusException -> {
+                    status = ex.status()
+                    friendlyMsg = ex.message as String
+                }
+                is IllegalArgumentException -> {
+                    status = RestStatus.BAD_REQUEST
+                    friendlyMsg = ex.message as String
+                }
+                is VersionConflictEngineException -> {
+                    status = ex.status()
+                    friendlyMsg = ex.message as String
+                }
+                is InvalidIndexNameException -> {
+                    status = RestStatus.BAD_REQUEST
+                    friendlyMsg = ex.message as String
+                }
+                else -> {
+                    if (!Strings.isNullOrEmpty(ex.message)) {
+                        friendlyMsg = ex.message as String
+                    }
+                }
+            }
+            // Wrapping the origin exception as runtime to avoid it being formatted.
+            // Currently, alerting-kibana is using `error.root_cause.reason` as text in the toast message.
+            // Below logic is to set friendly message to error.root_cause.reason.
+            return CommonUtilsException(friendlyMsg, status, Exception("${ex.javaClass.name}: ${ex.message}"))
+        }
+    }
+}
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()