Skip to content

Commit

Permalink
Added validation for the clusters field. Refactored ClusterMetricsInp…
Browse files Browse the repository at this point in the history
…ut validiation to throw 4xx-level CommonUtilsExceptions instead of 5xx-level IllegalArgumentException.

Signed-off-by: AWSHurneyt <[email protected]>
  • Loading branch information
AWSHurneyt committed Apr 12, 2024
1 parent c05715b commit 04ddb3d
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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.core.ParseField
import org.opensearch.core.common.io.stream.StreamInput
import org.opensearch.core.common.io.stream.StreamOutput
Expand All @@ -15,6 +16,14 @@ import java.io.IOException
import java.net.URI
import java.net.URISyntaxException

/**
* 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_-]*)$")
val ILLEGAL_PATH_PARAMETER_CHARACTERS = arrayOf(':', '"', '+', '\\', '|', '?', '#', '>', '<', ' ')

/**
Expand All @@ -31,35 +40,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."
}

if (clusters.isNotEmpty()) {
require(clusters.all { CLUSTER_NAME_REGEX.matches(it) }) {
"Cluster names are not valid."
}
}

clusterMetricType = findApiType(constructedUri.path)
this.parseEmptyFields()
clusterMetricType = findApiType(constructedUri.path)
this.parseEmptyFields()
} catch (exception: Exception) {
throw CommonUtilsException.wrap(exception)
}
}

@Throws(IOException::class)
Expand Down Expand Up @@ -158,7 +178,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 {
Expand All @@ -178,18 +198,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
Expand All @@ -199,7 +223,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
Expand All @@ -211,7 +235,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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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}"))
}
}
}
Loading

0 comments on commit 04ddb3d

Please sign in to comment.