Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 13, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Removed CommonUtilsException. Team decided IllegalArgumentExceptions …
…should be caught in the plugins themselves.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
  • Loading branch information
AWSHurneyt committed Apr 13, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit ab6db13159e5a42899c63704743d4496b75a0be5
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@ 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.alerting.util.CommonUtilsException
import org.opensearch.commons.utils.CLUSTER_NAME_REGEX
import org.opensearch.core.ParseField
import org.opensearch.core.common.io.stream.StreamInput
@@ -33,46 +32,41 @@ data class ClusterMetricsInput(

// Verify parameters are valid during creation
init {
// Wrap any validation exceptions in CommonUtilsException.
try {
require(validateFields()) {
"The uri.api_type field, uri.path field, or uri.uri field must be defined."
}
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."
}
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)
}

clusterMetricType = findApiType(constructedUri.path)
this.parseEmptyFields()
}

@Throws(IOException::class)
@@ -171,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 CommonUtilsException 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 {
@@ -191,22 +185,18 @@ data class ClusterMetricsInput(
pathParams = pathParams.trim('/')
ILLEGAL_PATH_PARAMETER_CHARACTERS.forEach { character ->
if (pathParams.contains(character)) {
throw CommonUtilsException.wrap(
IllegalArgumentException(
"The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(
" "
)
)
throw IllegalArgumentException(
"The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")
)
}
}
}

if (apiType.requiresPathParams && pathParams.isEmpty()) {
throw CommonUtilsException.wrap(IllegalArgumentException("The API requires path parameters."))
throw IllegalArgumentException("The API requires path parameters.")
}
if (!apiType.supportsPathParams && pathParams.isNotEmpty()) {
throw CommonUtilsException.wrap(IllegalArgumentException("The API does not use path parameters."))
throw IllegalArgumentException("The API does not use path parameters.")
}

return pathParams
@@ -216,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 CommonUtilsException 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
@@ -228,7 +218,7 @@ data class ClusterMetricsInput(
}
}
if (apiType.isBlank()) {
throw CommonUtilsException.wrap(IllegalArgumentException("The API could not be determined from the provided URI."))
throw IllegalArgumentException("The API could not be determined from the provided URI.")
}
return apiType
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.opensearch.commons.alerting.model

import org.junit.jupiter.api.Test
import org.opensearch.commons.alerting.util.CommonUtilsException
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith

@@ -79,7 +78,7 @@ class ClusterMetricsInputTests {
path = "///"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Invalid URL.") {
assertFailsWith<IllegalArgumentException>("Invalid URL.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -90,7 +89,7 @@ class ClusterMetricsInputTests {
url = "///"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Invalid URL.") {
assertFailsWith<IllegalArgumentException>("Invalid URL.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -135,7 +134,7 @@ class ClusterMetricsInputTests {
url = "http://localhost:9200/_cluster/stats"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The provided URL and URI fields form different URLs.") {
assertFailsWith<IllegalArgumentException>("The provided URL and URI fields form different URLs.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -147,7 +146,7 @@ class ClusterMetricsInputTests {
url = "http://localhost:9200/_cluster/stats/index1,index2,index3,index4,index5"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The provided URL and URI fields form different URLs.") {
assertFailsWith<IllegalArgumentException>("The provided URL and URI fields form different URLs.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -160,7 +159,7 @@ class ClusterMetricsInputTests {
url = ""

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
assertFailsWith<IllegalArgumentException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -173,7 +172,7 @@ class ClusterMetricsInputTests {
url = ""

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
assertFailsWith<IllegalArgumentException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -185,7 +184,7 @@ class ClusterMetricsInputTests {
url = "invalidScheme://localhost:9200/_cluster/health"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Invalid URL.") {
assertFailsWith<IllegalArgumentException>("Invalid URL.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -197,7 +196,7 @@ class ClusterMetricsInputTests {
url = "http://127.0.0.1:9200/_cluster/health"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Only host '${ClusterMetricsInput.SUPPORTED_HOST}' is supported.") {
assertFailsWith<IllegalArgumentException>("Only host '${ClusterMetricsInput.SUPPORTED_HOST}' is supported.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -209,7 +208,7 @@ class ClusterMetricsInputTests {
url = "http://localhost:${ClusterMetricsInput.SUPPORTED_PORT + 1}/_cluster/health"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Only port '${ClusterMetricsInput.SUPPORTED_PORT}' is supported.") {
assertFailsWith<IllegalArgumentException>("Only port '${ClusterMetricsInput.SUPPORTED_PORT}' is supported.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -269,7 +268,7 @@ class ClusterMetricsInputTests {
path = "/_cat/snapshots"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API requires path parameters.") {
assertFailsWith<IllegalArgumentException>("The API requires path parameters.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -282,7 +281,7 @@ class ClusterMetricsInputTests {
val clusterMetricsInput = ClusterMetricsInput(path, pathParams, url)

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API does not use path parameters.") {
assertFailsWith<IllegalArgumentException>("The API does not use path parameters.") {
clusterMetricsInput.parsePathParams()
}
}
@@ -296,7 +295,7 @@ class ClusterMetricsInputTests {
val clusterMetricsInput = ClusterMetricsInput(path, pathParams, url)

// WHEN + THEN
assertFailsWith<CommonUtilsException>(
assertFailsWith<IllegalArgumentException>(
"The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")
) {
clusterMetricsInput.parsePathParams()
@@ -394,7 +393,7 @@ class ClusterMetricsInputTests {
path = "/_cat/paws"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -406,7 +405,7 @@ class ClusterMetricsInputTests {
pathParams = "index1,index2,index3,index4,index5"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -418,7 +417,7 @@ class ClusterMetricsInputTests {
url = "http://localhost:9200/_cat/paws"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -430,7 +429,7 @@ class ClusterMetricsInputTests {
url = "http://localhost:9200/_cat/paws/index1,index2,index3,index4,index5"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(path, pathParams, url)
}
}
@@ -527,7 +526,7 @@ class ClusterMetricsInputTests {
val clusters = listOf(it)

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(
path = path,
pathParams = pathParams,
@@ -547,7 +546,7 @@ class ClusterMetricsInputTests {
val clusters = invalidClusters

// WHEN + THEN
assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
ClusterMetricsInput(
path = path,
pathParams = pathParams,
Loading