From dc0c4490d2659223d9d949ca33f6d4f034ade6e9 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Mon, 29 Mar 2021 18:40:21 -0700 Subject: [PATCH 01/12] Implemented LocalUriInput and extension class --- .../alerting/AlertingPlugin.kt | 3 +- core/build.gradle | 1 + .../alerting/core/model/LocalUriInput.kt | 144 ++++++++++++++++++ .../localuriapi/LocalUriExtensions.kt | 23 +++ 4 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt create mode 100644 core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/AlertingPlugin.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/AlertingPlugin.kt index 23f2ea6f..8c770bbc 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/AlertingPlugin.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/AlertingPlugin.kt @@ -37,6 +37,7 @@ import com.amazon.opendistroforelasticsearch.alerting.core.JobSweeper import com.amazon.opendistroforelasticsearch.alerting.core.ScheduledJobIndices import com.amazon.opendistroforelasticsearch.alerting.core.action.node.ScheduledJobsStatsAction import com.amazon.opendistroforelasticsearch.alerting.core.action.node.ScheduledJobsStatsTransportAction +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.ScheduledJob import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput import com.amazon.opendistroforelasticsearch.alerting.core.resthandler.RestScheduledJobStatsHandler @@ -200,7 +201,7 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R } override fun getNamedXContent(): List { - return listOf(Monitor.XCONTENT_REGISTRY, SearchInput.XCONTENT_REGISTRY) + return listOf(Monitor.XCONTENT_REGISTRY, SearchInput.XCONTENT_REGISTRY, LocalUriInput.XCONTENT_REGISTRY) } override fun createComponents( diff --git a/core/build.gradle b/core/build.gradle index 066d4098..d79c6c5c 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -26,6 +26,7 @@ dependencies { compile "org.elasticsearch.client:elasticsearch-rest-client:${es_version}" compile 'com.google.googlejavaformat:google-java-format:1.3' compile "com.amazon.opendistroforelasticsearch:common-utils:1.13.0.0" + compile 'commons-validator:commons-validator:1.7' testImplementation "org.elasticsearch.test:framework:${es_version}" testImplementation "org.jetbrains.kotlin:kotlin-test:${kotlin_version}" diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt new file mode 100644 index 00000000..2866f451 --- /dev/null +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -0,0 +1,144 @@ +package com.amazon.opendistroforelasticsearch.alerting.core.model + +import com.amazon.opendistroforelasticsearch.alerting.localuriapi.toConstructedUrl +import org.apache.commons.validator.routines.UrlValidator +import org.apache.http.client.utils.URIBuilder +import org.elasticsearch.common.CheckedFunction +import org.elasticsearch.common.ParseField +import org.elasticsearch.common.Strings +import org.elasticsearch.common.io.stream.StreamOutput +import org.elasticsearch.common.xcontent.NamedXContentRegistry +import org.elasticsearch.common.xcontent.ToXContent +import org.elasticsearch.common.xcontent.XContentBuilder +import org.elasticsearch.common.xcontent.XContentParser +import org.elasticsearch.common.xcontent.XContentParserUtils +import java.io.IOException + +/** + * This is a data class for URI type of input for Monitors. + */ +data class LocalUriInput( + val scheme: String, + val host: String, + val port: Int, + val path: String, + val params: Map, + val url: String, + val connection_timeout: Int, + val socket_timeout: Int +) : Input { + + // Verify parameters are valid during creation + init { + require(validateFields()) { + "Either one of url or scheme + host + port+ + path + params can be set." + } + require(connection_timeout in 1..5) { + "Connection timeout: $connection_timeout is not in the range of 1 - 5" + } + require(socket_timeout in 1..60) { + "Socket timeout: $socket_timeout is not in the range of 1 - 60" + } + + // 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. + val constructedUrl = if (Strings.isEmpty(url)) { + toConstructedUrl() + } else { + URIBuilder(url).build() + } + + require(urlValidator.isValid(constructedUrl.toString())) { + "Invalid url: $constructedUrl" + } + } + + override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return builder.startObject() + .startObject(URI_FIELD) + .field(SCHEME_FIELD, scheme) + .field(HOST_FIELD, host) + .field(PORT_FIELD, port) + .field(PATH_FIELD, path) + .field(PARAMS_FIELD, this.params) + .field(URL_FIELD, url) + .field(CONNECTION_TIMEOUT_FIELD, connection_timeout) + .field(SOCKET_TIMEOUT_FIELD, socket_timeout) + .endObject() + .endObject() + } + + override fun name(): String { + return URI_FIELD + } + + override fun writeTo(out: StreamOutput) { + out.writeString(scheme) + out.writeString(host) + out.writeInt(port) + out.writeString(path) + out.writeMap(params) + out.writeString(url) + out.writeInt(connection_timeout) + out.writeInt(socket_timeout) + } + + companion object { + const val SCHEME_FIELD = "scheme" + const val HOST_FIELD = "host" + const val PORT_FIELD = "port" + const val PATH_FIELD = "path" + const val PARAMS_FIELD = "params" + const val URL_FIELD = "url" + const val CONNECTION_TIMEOUT_FIELD = "connection_timeout" + const val SOCKET_TIMEOUT_FIELD = "socket_timeout" + const val URI_FIELD = "uri" + + val XCONTENT_REGISTRY = NamedXContentRegistry.Entry(Input::class.java, ParseField("uri"), CheckedFunction { parseInner(it) }) + + /** + * This parse function uses [XContentParser] to parse JSON input and store corresponding fields to create a [LocalUriInput] object + */ + @JvmStatic @Throws(IOException::class) + private fun parseInner(xcp: XContentParser): LocalUriInput { + var scheme = "http" + var host = "" + var port: Int = -1 + var path = "" + var params: Map = mutableMapOf() + var url = "" + var connectionTimeout = 5 + var socketTimeout = 10 + + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) + + while (xcp.nextToken() != XContentParser.Token.END_OBJECT) { + val fieldName = xcp.currentName() + xcp.nextToken() + when (fieldName) { + SCHEME_FIELD -> scheme = xcp.text() + HOST_FIELD -> host = xcp.text() + PORT_FIELD -> port = xcp.intValue() + PATH_FIELD -> path = xcp.text() + PARAMS_FIELD -> params = xcp.mapStrings() + URL_FIELD -> url = xcp.text() + CONNECTION_TIMEOUT_FIELD -> connectionTimeout = xcp.intValue() + SOCKET_TIMEOUT_FIELD -> socketTimeout = xcp.intValue() + } + } + return LocalUriInput(scheme, host, port, path, params, url, connectionTimeout, socketTimeout) + } + } + + /** + * Helper function to check whether one of url or scheme+host+port+path+params is defined. + */ + private fun validateFields(): Boolean { + if (url.isNotEmpty()) { + return (host.isEmpty() && (port == -1) && path.isEmpty() && params.isEmpty()) + } + return true + } +} \ No newline at end of file diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt new file mode 100644 index 00000000..42afedb5 --- /dev/null +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt @@ -0,0 +1,23 @@ +package com.amazon.opendistroforelasticsearch.alerting.localuriapi + +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput +import org.apache.http.client.utils.URIBuilder +import java.net.URI + +/** + * Construct url either by url or by scheme+host+port+path+params. + */ +fun LocalUriInput.toConstructedUrl(): URI { + return if (url.isEmpty()) { + val uriBuilder = URIBuilder() + uriBuilder.scheme = scheme + uriBuilder.host = host + uriBuilder.port = port + uriBuilder.path = path + for (e in params.entries) + uriBuilder.addParameter(e.key, e.value) + uriBuilder.build() + } else { + URIBuilder(url).build() + } +} \ No newline at end of file From f33f9a4991c1c4ad7d5a9e222e0b7d8f600ac6a6 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Mon, 29 Mar 2021 19:24:36 -0700 Subject: [PATCH 02/12] Implemented SupportedApiSettings and extensions. Implemented logic for creating ClusterHealth and ClusterStats monitors. --- .../alerting/MonitorRunner.kt | 10 +++ .../alerting/model/Monitor.kt | 8 ++- .../alerting/settings/SupportedApiSettings.kt | 67 +++++++++++++++++++ .../util/SupportedApiSettingsExtensions.kt | 32 +++++++++ .../alerting/core/model/LocalUriInput.kt | 31 ++++++--- .../localuriapi/LocalUriExtensions.kt | 23 ------- 6 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt create mode 100644 alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt delete mode 100644 core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt index 872589d1..9b3812fc 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt @@ -19,6 +19,7 @@ import com.amazon.opendistroforelasticsearch.alerting.alerts.AlertError import com.amazon.opendistroforelasticsearch.alerting.alerts.AlertIndices import com.amazon.opendistroforelasticsearch.alerting.alerts.moveAlerts import com.amazon.opendistroforelasticsearch.alerting.core.JobRunner +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.ScheduledJob import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.InjectorContextElement @@ -56,8 +57,10 @@ import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettin import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings.Companion.loadDestinationSettings import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils import com.amazon.opendistroforelasticsearch.alerting.util.addUserBackendRolesFilter +import com.amazon.opendistroforelasticsearch.alerting.util.executeTransportAction import com.amazon.opendistroforelasticsearch.alerting.util.isADMonitor import com.amazon.opendistroforelasticsearch.alerting.util.isAllowed +import com.amazon.opendistroforelasticsearch.alerting.util.toMap import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -336,6 +339,13 @@ class MonitorRunner( val searchResponse: SearchResponse = client.suspendUntil { client.search(searchRequest, it) } results += searchResponse.convertToMap() } + is LocalUriInput -> { + logger.debug("LocalUriInput path: ${input.toConstructedUri().path}") + val response = executeTransportAction(input, client) + results += withContext(Dispatchers.IO) { + response.toMap() + } + } else -> { throw IllegalArgumentException("Unsupported input type: ${input.name()}.") } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt index 6aa9d711..9a9e96ed 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt @@ -17,6 +17,7 @@ package com.amazon.opendistroforelasticsearch.alerting.model import com.amazon.opendistroforelasticsearch.alerting.core.model.CronSchedule import com.amazon.opendistroforelasticsearch.alerting.core.model.Input +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.Schedule import com.amazon.opendistroforelasticsearch.alerting.core.model.ScheduledJob import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput @@ -25,6 +26,7 @@ import com.amazon.opendistroforelasticsearch.alerting.elasticapi.optionalTimeFie import com.amazon.opendistroforelasticsearch.alerting.elasticapi.optionalUserField import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MONITOR_MAX_INPUTS import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MONITOR_MAX_TRIGGERS +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils.Companion.NO_SCHEMA_VERSION import com.amazon.opendistroforelasticsearch.alerting.util._ID import com.amazon.opendistroforelasticsearch.alerting.util._VERSION @@ -197,7 +199,11 @@ data class Monitor( INPUTS_FIELD -> { ensureExpectedToken(Token.START_ARRAY, xcp.currentToken(), xcp) while (xcp.nextToken() != Token.END_ARRAY) { - inputs.add(Input.parse(xcp)) + val input = Input.parse(xcp) + if (input is LocalUriInput) { + SupportedApiSettings.validateLocalUriInput(input) + } + inputs.add(input) } } TRIGGERS_FIELD -> { diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt new file mode 100644 index 00000000..392a1ed2 --- /dev/null +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -0,0 +1,67 @@ +package com.amazon.opendistroforelasticsearch.alerting.settings + +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput + +/** + * A class that supports storing a unique set of API paths that can be accessed by general users. + */ +class SupportedApiSettings { + companion object { + const val CLUSTER_HEALTH_PATH = "/_cluster/health" + const val CLUSTER_STATS_PATH = "/_cluster/stats" + + /** + * Each String represents the path to call an API. + * NOTE: Paths should conform to the following pattern: + * "/_cluster/health" + * + * The maps of supported JSON payloads were pulled from + * https://code.amazon.com/packages/CloudSearchSolrAuthService/blobs/4d6e5d39771dc6de6a6c46d9b359f42436505edf/--/etc/es/jetty-web.xml#L1702 + */ + private var supportedApiList = HashMap>>() + + /** + * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. + */ + // TODO HURNEYT: Currently set to TRUE for testing purposes. + // Should likely be set to FALSE by default. + private var supportedApiListEnabled = true + + init { + supportedApiList[CLUSTER_HEALTH_PATH] = hashMapOf() + supportedApiList[CLUSTER_STATS_PATH] = hashMapOf() + } + + /** + * Returns the map of all supported json payload associated with the provided path from supportedApiList. + * @param path The path for the requested API. + * @return The map of all supported json payload for the requested API. + * @throws IllegalArgumentException When supportedApiList does not contain a value for the provided key. + */ + fun getSupportedJsonPayload(path: String): Map> { + return supportedApiList[path] ?: throw IllegalArgumentException("API path not in supportedApiList: $path") + } + + /** + * If [supportedApiListEnabled] is TRUE, calls [validatePath] to confirm whether the provided path + * is in supportedApiList. Will otherwise take no actions. + * @param localUriInput The [LocalUriInput] to validate. + * @return The path that was validated. + */ + fun validateLocalUriInput(localUriInput: LocalUriInput): String { + val path = localUriInput.toConstructedUri().path + if (supportedApiListEnabled) validatePath(path) + return path + } + + /** + * Confirms whether the provided path is in supportedApiList. + * Throws an exception if the provided path is not on the list; otherwise performs no action. + * @param path The path to validate. + * @throws IllegalArgumentException When supportedApiList does not contain the provided path. + */ + private fun validatePath(path: String) { + if (!supportedApiList.contains(path)) throw IllegalArgumentException("API path not in supportedApiList: $path") + } + } +} diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt new file mode 100644 index 00000000..f21ef898 --- /dev/null +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -0,0 +1,32 @@ +package com.amazon.opendistroforelasticsearch.alerting.util + +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput +import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings +import org.elasticsearch.action.ActionResponse +import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest +import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse +import org.elasticsearch.client.Client + +fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { + val path = SupportedApiSettings.validateLocalUriInput(localUriInput) + if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { + return client.admin().cluster().health(ClusterHealthRequest()).get() + } + if (path == SupportedApiSettings.CLUSTER_STATS_PATH) { + return client.admin().cluster().clusterStats(ClusterStatsRequest()).get() + } + throw IllegalArgumentException("Unsupported API: $path") +} + +fun ActionResponse.toMap(): Map { + if (this is ClusterHealthResponse) { + return this.convertToMap() + } + if (this is ClusterStatsResponse) { + return this.convertToMap() + } + throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") +} diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 2866f451..3a0bde4a 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -1,11 +1,9 @@ package com.amazon.opendistroforelasticsearch.alerting.core.model -import com.amazon.opendistroforelasticsearch.alerting.localuriapi.toConstructedUrl import org.apache.commons.validator.routines.UrlValidator import org.apache.http.client.utils.URIBuilder import org.elasticsearch.common.CheckedFunction import org.elasticsearch.common.ParseField -import org.elasticsearch.common.Strings import org.elasticsearch.common.io.stream.StreamOutput import org.elasticsearch.common.xcontent.NamedXContentRegistry import org.elasticsearch.common.xcontent.ToXContent @@ -13,9 +11,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder import org.elasticsearch.common.xcontent.XContentParser import org.elasticsearch.common.xcontent.XContentParserUtils import java.io.IOException +import java.net.URI /** - * This is a data class for URI type of input for Monitors. + * This is a data class for a URI type of input for Monitors. */ data class LocalUriInput( val scheme: String, @@ -44,11 +43,7 @@ data class LocalUriInput( val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS) // Build url field by field if not provided as whole. - val constructedUrl = if (Strings.isEmpty(url)) { - toConstructedUrl() - } else { - URIBuilder(url).build() - } + val constructedUrl = toConstructedUri() require(urlValidator.isValid(constructedUrl.toString())) { "Invalid url: $constructedUrl" @@ -133,7 +128,25 @@ data class LocalUriInput( } /** - * Helper function to check whether one of url or scheme+host+port+path+params is defined. + * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[params]. + */ + fun toConstructedUri(): URI { + return if (url.isEmpty()) { + val uriBuilder = URIBuilder() + uriBuilder.scheme = scheme + uriBuilder.host = host + uriBuilder.port = port + uriBuilder.path = path + for (e in params.entries) + uriBuilder.addParameter(e.key, e.value) + uriBuilder.build() + } else { + URIBuilder(url).build() + } + } + + /** + * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[params] is defined. */ private fun validateFields(): Boolean { if (url.isNotEmpty()) { diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt deleted file mode 100644 index 42afedb5..00000000 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt +++ /dev/null @@ -1,23 +0,0 @@ -package com.amazon.opendistroforelasticsearch.alerting.localuriapi - -import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput -import org.apache.http.client.utils.URIBuilder -import java.net.URI - -/** - * Construct url either by url or by scheme+host+port+path+params. - */ -fun LocalUriInput.toConstructedUrl(): URI { - return if (url.isEmpty()) { - val uriBuilder = URIBuilder() - uriBuilder.scheme = scheme - uriBuilder.host = host - uriBuilder.port = port - uriBuilder.path = path - for (e in params.entries) - uriBuilder.addParameter(e.key, e.value) - uriBuilder.build() - } else { - URIBuilder(url).build() - } -} \ No newline at end of file From 546b384676a7178bc5ed7cbf14377746c6e2d23f Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Mon, 29 Mar 2021 19:24:36 -0700 Subject: [PATCH 03/12] Implemented SupportedApiSettings and extensions. Implemented logic for creating ClusterHealth and ClusterStats monitors. --- .../alerting/MonitorRunner.kt | 10 +++ .../alerting/model/Monitor.kt | 8 ++- .../alerting/settings/SupportedApiSettings.kt | 66 +++++++++++++++++++ .../util/SupportedApiSettingsExtensions.kt | 32 +++++++++ .../alerting/core/model/LocalUriInput.kt | 31 ++++++--- .../localuriapi/LocalUriExtensions.kt | 23 ------- 6 files changed, 137 insertions(+), 33 deletions(-) create mode 100644 alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt create mode 100644 alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt delete mode 100644 core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt index 872589d1..9b3812fc 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt @@ -19,6 +19,7 @@ import com.amazon.opendistroforelasticsearch.alerting.alerts.AlertError import com.amazon.opendistroforelasticsearch.alerting.alerts.AlertIndices import com.amazon.opendistroforelasticsearch.alerting.alerts.moveAlerts import com.amazon.opendistroforelasticsearch.alerting.core.JobRunner +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.ScheduledJob import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.InjectorContextElement @@ -56,8 +57,10 @@ import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettin import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings.Companion.loadDestinationSettings import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils import com.amazon.opendistroforelasticsearch.alerting.util.addUserBackendRolesFilter +import com.amazon.opendistroforelasticsearch.alerting.util.executeTransportAction import com.amazon.opendistroforelasticsearch.alerting.util.isADMonitor import com.amazon.opendistroforelasticsearch.alerting.util.isAllowed +import com.amazon.opendistroforelasticsearch.alerting.util.toMap import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -336,6 +339,13 @@ class MonitorRunner( val searchResponse: SearchResponse = client.suspendUntil { client.search(searchRequest, it) } results += searchResponse.convertToMap() } + is LocalUriInput -> { + logger.debug("LocalUriInput path: ${input.toConstructedUri().path}") + val response = executeTransportAction(input, client) + results += withContext(Dispatchers.IO) { + response.toMap() + } + } else -> { throw IllegalArgumentException("Unsupported input type: ${input.name()}.") } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt index 6aa9d711..9a9e96ed 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt @@ -17,6 +17,7 @@ package com.amazon.opendistroforelasticsearch.alerting.model import com.amazon.opendistroforelasticsearch.alerting.core.model.CronSchedule import com.amazon.opendistroforelasticsearch.alerting.core.model.Input +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.Schedule import com.amazon.opendistroforelasticsearch.alerting.core.model.ScheduledJob import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput @@ -25,6 +26,7 @@ import com.amazon.opendistroforelasticsearch.alerting.elasticapi.optionalTimeFie import com.amazon.opendistroforelasticsearch.alerting.elasticapi.optionalUserField import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MONITOR_MAX_INPUTS import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MONITOR_MAX_TRIGGERS +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils.Companion.NO_SCHEMA_VERSION import com.amazon.opendistroforelasticsearch.alerting.util._ID import com.amazon.opendistroforelasticsearch.alerting.util._VERSION @@ -197,7 +199,11 @@ data class Monitor( INPUTS_FIELD -> { ensureExpectedToken(Token.START_ARRAY, xcp.currentToken(), xcp) while (xcp.nextToken() != Token.END_ARRAY) { - inputs.add(Input.parse(xcp)) + val input = Input.parse(xcp) + if (input is LocalUriInput) { + SupportedApiSettings.validateLocalUriInput(input) + } + inputs.add(input) } } TRIGGERS_FIELD -> { diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt new file mode 100644 index 00000000..88065c20 --- /dev/null +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -0,0 +1,66 @@ +package com.amazon.opendistroforelasticsearch.alerting.settings + +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput + +/** + * A class that supports storing a unique set of API paths that can be accessed by general users. + */ +class SupportedApiSettings { + companion object { + const val CLUSTER_HEALTH_PATH = "/_cluster/health" + const val CLUSTER_STATS_PATH = "/_cluster/stats" + + /** + * Each String represents the path to call an API. + * NOTE: Paths should conform to the following pattern: + * "/_cluster/health" + * + * Each Set represents the supported JSON payload for the respective API. + */ + private var supportedApiList = HashMap>>() + + /** + * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. + */ + // TODO HURNEYT: Currently set to TRUE for testing purposes. + // Should likely be set to FALSE by default. + private var supportedApiListEnabled = true + + init { + supportedApiList[CLUSTER_HEALTH_PATH] = hashMapOf() + supportedApiList[CLUSTER_STATS_PATH] = hashMapOf() + } + + /** + * Returns the map of all supported json payload associated with the provided path from supportedApiList. + * @param path The path for the requested API. + * @return The map of all supported json payload for the requested API. + * @throws IllegalArgumentException When supportedApiList does not contain a value for the provided key. + */ + fun getSupportedJsonPayload(path: String): Map> { + return supportedApiList[path] ?: throw IllegalArgumentException("API path not in supportedApiList: $path") + } + + /** + * If [supportedApiListEnabled] is TRUE, calls [validatePath] to confirm whether the provided path + * is in supportedApiList. Will otherwise take no actions. + * @param localUriInput The [LocalUriInput] to validate. + * @return The path that was validated. + */ + fun validateLocalUriInput(localUriInput: LocalUriInput): String { + val path = localUriInput.toConstructedUri().path + if (supportedApiListEnabled) validatePath(path) + return path + } + + /** + * Confirms whether the provided path is in supportedApiList. + * Throws an exception if the provided path is not on the list; otherwise performs no action. + * @param path The path to validate. + * @throws IllegalArgumentException When supportedApiList does not contain the provided path. + */ + private fun validatePath(path: String) { + if (!supportedApiList.contains(path)) throw IllegalArgumentException("API path not in supportedApiList: $path") + } + } +} diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt new file mode 100644 index 00000000..f21ef898 --- /dev/null +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -0,0 +1,32 @@ +package com.amazon.opendistroforelasticsearch.alerting.util + +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput +import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings +import org.elasticsearch.action.ActionResponse +import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest +import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse +import org.elasticsearch.client.Client + +fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { + val path = SupportedApiSettings.validateLocalUriInput(localUriInput) + if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { + return client.admin().cluster().health(ClusterHealthRequest()).get() + } + if (path == SupportedApiSettings.CLUSTER_STATS_PATH) { + return client.admin().cluster().clusterStats(ClusterStatsRequest()).get() + } + throw IllegalArgumentException("Unsupported API: $path") +} + +fun ActionResponse.toMap(): Map { + if (this is ClusterHealthResponse) { + return this.convertToMap() + } + if (this is ClusterStatsResponse) { + return this.convertToMap() + } + throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") +} diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 2866f451..3a0bde4a 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -1,11 +1,9 @@ package com.amazon.opendistroforelasticsearch.alerting.core.model -import com.amazon.opendistroforelasticsearch.alerting.localuriapi.toConstructedUrl import org.apache.commons.validator.routines.UrlValidator import org.apache.http.client.utils.URIBuilder import org.elasticsearch.common.CheckedFunction import org.elasticsearch.common.ParseField -import org.elasticsearch.common.Strings import org.elasticsearch.common.io.stream.StreamOutput import org.elasticsearch.common.xcontent.NamedXContentRegistry import org.elasticsearch.common.xcontent.ToXContent @@ -13,9 +11,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder import org.elasticsearch.common.xcontent.XContentParser import org.elasticsearch.common.xcontent.XContentParserUtils import java.io.IOException +import java.net.URI /** - * This is a data class for URI type of input for Monitors. + * This is a data class for a URI type of input for Monitors. */ data class LocalUriInput( val scheme: String, @@ -44,11 +43,7 @@ data class LocalUriInput( val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS) // Build url field by field if not provided as whole. - val constructedUrl = if (Strings.isEmpty(url)) { - toConstructedUrl() - } else { - URIBuilder(url).build() - } + val constructedUrl = toConstructedUri() require(urlValidator.isValid(constructedUrl.toString())) { "Invalid url: $constructedUrl" @@ -133,7 +128,25 @@ data class LocalUriInput( } /** - * Helper function to check whether one of url or scheme+host+port+path+params is defined. + * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[params]. + */ + fun toConstructedUri(): URI { + return if (url.isEmpty()) { + val uriBuilder = URIBuilder() + uriBuilder.scheme = scheme + uriBuilder.host = host + uriBuilder.port = port + uriBuilder.path = path + for (e in params.entries) + uriBuilder.addParameter(e.key, e.value) + uriBuilder.build() + } else { + URIBuilder(url).build() + } + } + + /** + * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[params] is defined. */ private fun validateFields(): Boolean { if (url.isNotEmpty()) { diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt deleted file mode 100644 index 42afedb5..00000000 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/localuriapi/LocalUriExtensions.kt +++ /dev/null @@ -1,23 +0,0 @@ -package com.amazon.opendistroforelasticsearch.alerting.localuriapi - -import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput -import org.apache.http.client.utils.URIBuilder -import java.net.URI - -/** - * Construct url either by url or by scheme+host+port+path+params. - */ -fun LocalUriInput.toConstructedUrl(): URI { - return if (url.isEmpty()) { - val uriBuilder = URIBuilder() - uriBuilder.scheme = scheme - uriBuilder.host = host - uriBuilder.port = port - uriBuilder.path = path - for (e in params.entries) - uriBuilder.addParameter(e.key, e.value) - uriBuilder.build() - } else { - URIBuilder(url).build() - } -} \ No newline at end of file From c9a73fdc1366cbc1a20929e85f2e321b757b56a6 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Tue, 6 Apr 2021 12:31:41 -0700 Subject: [PATCH 04/12] Implemented unit tests and integration tests for LocalUriInput --- .../alerting/settings/SupportedApiSettings.kt | 2 +- .../util/SupportedApiSettingsExtensions.kt | 3 +- .../alerting/MonitorRunnerIT.kt | 127 +++++++++++ .../alerting/TestHelpers.kt | 14 ++ .../alerting/core/model/LocalUriInput.kt | 48 +++-- .../alerting/core/model/LocalUriInputTests.kt | 199 ++++++++++++++++++ 6 files changed, 373 insertions(+), 20 deletions(-) create mode 100644 core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index 88065c20..ccdd9b33 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -22,7 +22,7 @@ class SupportedApiSettings { /** * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. */ - // TODO HURNEYT: Currently set to TRUE for testing purposes. + // TODO: Currently set to TRUE for testing purposes. // Should likely be set to FALSE by default. private var supportedApiListEnabled = true diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index f21ef898..70e4e678 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -3,6 +3,7 @@ package com.amazon.opendistroforelasticsearch.alerting.util import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.validateLocalUriInput import org.elasticsearch.action.ActionResponse import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse @@ -11,7 +12,7 @@ import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse import org.elasticsearch.client.Client fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { - val path = SupportedApiSettings.validateLocalUriInput(localUriInput) + val path = validateLocalUriInput(localUriInput) if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { return client.admin().cluster().health(ClusterHealthRequest()).get() } diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt index 5e8c1261..e2fa8d06 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt @@ -807,6 +807,133 @@ class MonitorRunnerIT : AlertingRestTestCase() { } } + fun `test LocalUriInput monitor with ClusterHealth API`() { + // GIVEN + val path = "/_cluster/health" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + val inputResults = output.stringMap("input_results") + val resultsContent = (inputResults?.get("results") as ArrayList<*>)[0] + val errorMessage = inputResults["error"] + + assertEquals(monitor.name, output["monitor_name"]) + assertTrue("Monitor results should contain cluster_name, but found: $resultsContent", + resultsContent.toString().contains("cluster_name")) + assertNull("There should not be an error message, but found: $errorMessage", errorMessage) + } + + fun `test LocalUriInput monitor with ClusterStats API`() { + // GIVEN + val path = "/_cluster/stats" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + val inputResults = output.stringMap("input_results") + val resultsContent = (inputResults?.get("results") as ArrayList<*>)[0] + val errorMessage = inputResults["error"] + + assertEquals(monitor.name, output["monitor_name"]) + assertTrue("Monitor results should contain cluster_name, but found: $resultsContent", + resultsContent.toString().contains("memory_size_in_bytes")) + assertNull("There should not be an error message, but found: $errorMessage", errorMessage) + } + + fun `test LocalUriInput monitor with alert triggered`() { + // GIVEN + putAlertMappings() + val trigger = randomTrigger(condition = Script(""" + return ctx.results[0].number_of_pending_tasks < 1 + """.trimIndent()), destinationId = createDestination().id) + val path = "/_cluster/health" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + assertEquals(monitor.name, output["monitor_name"]) + + val triggerResults = output.objectMap("trigger_results").values + for (triggerResult in triggerResults) { + assertTrue("This triggerResult should be triggered: $triggerResult", + triggerResult.objectMap("action_results").isNotEmpty()) + } + + val alerts = searchAlerts(monitor) + assertEquals("Alert not saved, $output", 1, alerts.size) + verifyAlert(alerts.single(), monitor, ACTIVE) + } + + fun `test LocalUriInput monitor with no alert triggered`() { + // GIVEN + putAlertMappings() + val trigger = randomTrigger(condition = Script(""" + return ctx.results[0].status.equals("red") + """.trimIndent())) + val path = "/_cluster/stats" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + assertEquals(monitor.name, output["monitor_name"]) + + val triggerResults = output.objectMap("trigger_results").values + for (triggerResult in triggerResults) { + assertTrue("This triggerResult should not be triggered: $triggerResult", + triggerResult.objectMap("action_results").isEmpty()) + } + + val alerts = searchAlerts(monitor) + assertEquals("Alert saved for test monitor, output: $output", 0, alerts.size) + } + + // TODO: Once an API is implemented that supports adding/removing entries on the + // SupportedApiSettings::supportedApiList, create an test that simulates executing + // a preexisting LocalUriInput monitor for an API that has been removed from the supportedApiList. + // This will likely involve adding an API to the list before creating the monitor, and then removing + // the API from the list before executing the monitor. + private fun prepareTestAnomalyResult(detectorId: String, user: User) { val adResultIndex = ".opendistro-anomaly-results-history-2020.10.17" try { diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt index 235e5c46..8464b917 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt @@ -16,6 +16,7 @@ package com.amazon.opendistroforelasticsearch.alerting import com.amazon.opendistroforelasticsearch.alerting.core.model.Input import com.amazon.opendistroforelasticsearch.alerting.core.model.IntervalSchedule +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.Schedule import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.string @@ -229,6 +230,19 @@ fun randomUserEmpty(): User { return User("", listOf(), listOf(), listOf()) } +fun randomLocalUriInput( + scheme: String = if (randomInt(3) >= 2) "http" else "https", + host: String = "localhost", + port: Int = randomInt(LocalUriInput.MAX_PORT), + path: String, + queryParams: Map = hashMapOf(), + url: String = "", + connectionTimeout: Int = 1 + randomInt(LocalUriInput.MAX_CONNECTION_TIMEOUT - 1), + socketTimeout: Int = 1 + randomInt(LocalUriInput.MAX_SOCKET_TIMEOUT - 1) +): LocalUriInput { + return LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) +} + fun EmailAccount.toJsonString(): String { val builder = XContentFactory.jsonBuilder() return this.toXContent(builder).string() diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 3a0bde4a..76278b27 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -21,7 +21,7 @@ data class LocalUriInput( val host: String, val port: Int, val path: String, - val params: Map, + val query_params: Map, val url: String, val connection_timeout: Int, val socket_timeout: Int @@ -30,13 +30,18 @@ data class LocalUriInput( // Verify parameters are valid during creation init { require(validateFields()) { - "Either one of url or scheme + host + port+ + path + params can be set." + "Either the url field, or scheme + host + port + path + params can be set." } - require(connection_timeout in 1..5) { - "Connection timeout: $connection_timeout is not in the range of 1 - 5" + if (port != -1) { + require(port in MIN_PORT..MAX_PORT) { + "Port: $port is not in the range of $MIN_PORT - $MAX_PORT" + } + } + require(connection_timeout in MIN_CONNECTION_TIMEOUT..MAX_CONNECTION_TIMEOUT) { + "Connection timeout: $connection_timeout is not in the range of $MIN_CONNECTION_TIMEOUT - $MAX_CONNECTION_TIMEOUT" } - require(socket_timeout in 1..60) { - "Socket timeout: $socket_timeout is not in the range of 1 - 60" + require(socket_timeout in MIN_SOCKET_TIMEOUT..MAX_SOCKET_TIMEOUT) { + "Socket timeout: $socket_timeout is not in the range of $MIN_SOCKET_TIMEOUT - $MAX_SOCKET_TIMEOUT" } // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs. @@ -57,7 +62,7 @@ data class LocalUriInput( .field(HOST_FIELD, host) .field(PORT_FIELD, port) .field(PATH_FIELD, path) - .field(PARAMS_FIELD, this.params) + .field(PARAMS_FIELD, this.query_params) .field(URL_FIELD, url) .field(CONNECTION_TIMEOUT_FIELD, connection_timeout) .field(SOCKET_TIMEOUT_FIELD, socket_timeout) @@ -74,13 +79,20 @@ data class LocalUriInput( out.writeString(host) out.writeInt(port) out.writeString(path) - out.writeMap(params) + out.writeMap(query_params) out.writeString(url) out.writeInt(connection_timeout) out.writeInt(socket_timeout) } companion object { + const val MIN_CONNECTION_TIMEOUT = 1 + const val MAX_CONNECTION_TIMEOUT = 5 + const val MIN_PORT = 1 + const val MAX_PORT = 65535 + const val MIN_SOCKET_TIMEOUT = 1 + const val MAX_SOCKET_TIMEOUT = 60 + const val SCHEME_FIELD = "scheme" const val HOST_FIELD = "host" const val PORT_FIELD = "port" @@ -104,8 +116,8 @@ data class LocalUriInput( var path = "" var params: Map = mutableMapOf() var url = "" - var connectionTimeout = 5 - var socketTimeout = 10 + var connectionTimeout = MAX_CONNECTION_TIMEOUT + var socketTimeout = MAX_SOCKET_TIMEOUT XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) @@ -128,16 +140,16 @@ data class LocalUriInput( } /** - * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[params]. + * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[query_params]. */ fun toConstructedUri(): URI { return if (url.isEmpty()) { val uriBuilder = URIBuilder() - uriBuilder.scheme = scheme - uriBuilder.host = host - uriBuilder.port = port - uriBuilder.path = path - for (e in params.entries) + .setScheme(scheme) + .setHost(host) + .setPort(port) + .setPath(path) + for (e in query_params.entries) uriBuilder.addParameter(e.key, e.value) uriBuilder.build() } else { @@ -146,11 +158,11 @@ data class LocalUriInput( } /** - * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[params] is defined. + * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[query_params] is defined. */ private fun validateFields(): Boolean { if (url.isNotEmpty()) { - return (host.isEmpty() && (port == -1) && path.isEmpty() && params.isEmpty()) + return (host.isEmpty() && (port == -1) && path.isEmpty() && query_params.isEmpty()) } return true } diff --git a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt new file mode 100644 index 00000000..cf66658b --- /dev/null +++ b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt @@ -0,0 +1,199 @@ +package com.amazon.opendistroforelasticsearch.alerting.core.model + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class LocalUriInputTests { + private var scheme = "http" + private var host = "localhost" + private var port = 9200 + private var path = "/_cluster/health" + private var queryParams = hashMapOf() + private var url = "" + private var connectionTimeout = 5 + private var socketTimeout = 5 + + @Test + fun `test valid LocalUriInput creation using HTTP URI component fields`() { + // GIVEN + WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(scheme, localUriInput.scheme) + assertEquals(host, localUriInput.host) + assertEquals(port, localUriInput.port) + assertEquals(path, localUriInput.path) + assertEquals(queryParams, localUriInput.query_params) + assertEquals(url, localUriInput.url) + assertEquals(connectionTimeout, localUriInput.connection_timeout) + assertEquals(socketTimeout, localUriInput.socket_timeout) + } + + @Test + fun `test valid LocalUriInput creation using HTTP url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "http://localost:9200/_cluster/health" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(url, localUriInput.url) + } + + @Test + fun `test valid LocalUriInput creation using HTTPS URI component fields`() { + // GIVEN + scheme = "https" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(scheme, localUriInput.scheme) + assertEquals(host, localUriInput.host) + assertEquals(port, localUriInput.port) + assertEquals(path, localUriInput.path) + assertEquals(queryParams, localUriInput.query_params) + assertEquals(url, localUriInput.url) + assertEquals(connectionTimeout, localUriInput.connection_timeout) + assertEquals(socketTimeout, localUriInput.socket_timeout) + } + + @Test + fun `test valid LocalUriInput creation using HTTPS url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "https://localost:9200/_cluster/health" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(url, localUriInput.url) + } + + @Test + fun `test invalid scheme`() { + // GIVEN + scheme = "invalidScheme" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid host`() { + // GIVEN + host = "loco//host" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid path`() { + // GIVEN + path = "///" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid port that's too low`() { + // GIVEN + port = LocalUriInput.MIN_PORT - 1 + + // WHEN + THEN + assertFailsWith( + "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid port that's too high`() { + // GIVEN + port = LocalUriInput.MAX_PORT + 1 + + // WHEN + THEN + assertFailsWith( + "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid connection timeout that's too low`() { + // GIVEN + connectionTimeout = LocalUriInput.MIN_CONNECTION_TIMEOUT - 1 + + // WHEN + THEN + assertFailsWith( + "Connection timeout: $connectionTimeout is not in the range of ${LocalUriInput.MIN_CONNECTION_TIMEOUT} - ${LocalUriInput.MIN_CONNECTION_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid connection timeout that's too high`() { + // GIVEN + connectionTimeout = LocalUriInput.MAX_CONNECTION_TIMEOUT + 1 + + // WHEN + THEN + assertFailsWith( + "Connection timeout: $connectionTimeout is not in the range of ${LocalUriInput.MIN_CONNECTION_TIMEOUT} - ${LocalUriInput.MIN_CONNECTION_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid socket timeout that's too low`() { + // GIVEN + socketTimeout = LocalUriInput.MIN_SOCKET_TIMEOUT - 1 + + // WHEN + THEN + assertFailsWith( + "Socket timeout: $socketTimeout is not in the range of ${LocalUriInput.MIN_SOCKET_TIMEOUT} - ${LocalUriInput.MAX_SOCKET_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid socket timeout that's too high`() { + // GIVEN + socketTimeout = LocalUriInput.MAX_SOCKET_TIMEOUT + 1 + + // WHEN + THEN + assertFailsWith( + "Socket timeout: $socketTimeout is not in the range of ${LocalUriInput.MIN_SOCKET_TIMEOUT} - ${LocalUriInput.MAX_SOCKET_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid url`() { + // GIVEN + url = "///" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test setting other fields in addition to url field`() { + // GIVEN + url = "http://localhost:9200/_cluster/health" + + // WHEN + THEN + assertFailsWith("Either the url field, or scheme + host + port + path + params can be set.") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } +} \ No newline at end of file From 708d00bbb8a7aef9f65847c1445f8a0574f8135b Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Tue, 13 Apr 2021 17:17:35 -0700 Subject: [PATCH 05/12] Refactored SupportedApiSettings and SupportedApiSettingsExtensions based on PR feedback --- .../alerting/model/Monitor.kt | 2 +- .../alerting/settings/SupportedApiSettings.kt | 27 ++++++++++--------- .../util/SupportedApiSettingsExtensions.kt | 24 +++++++---------- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt index 9a9e96ed..690c612a 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt @@ -201,7 +201,7 @@ data class Monitor( while (xcp.nextToken() != Token.END_ARRAY) { val input = Input.parse(xcp) if (input is LocalUriInput) { - SupportedApiSettings.validateLocalUriInput(input) + SupportedApiSettings.validatePath(input.toConstructedUri().path) } inputs.add(input) } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index ccdd9b33..59b579c8 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -1,6 +1,9 @@ package com.amazon.opendistroforelasticsearch.alerting.settings import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput +import org.elasticsearch.action.ActionRequest +import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest /** * A class that supports storing a unique set of API paths that can be accessed by general users. @@ -19,13 +22,6 @@ class SupportedApiSettings { */ private var supportedApiList = HashMap>>() - /** - * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. - */ - // TODO: Currently set to TRUE for testing purposes. - // Should likely be set to FALSE by default. - private var supportedApiListEnabled = true - init { supportedApiList[CLUSTER_HEALTH_PATH] = hashMapOf() supportedApiList[CLUSTER_STATS_PATH] = hashMapOf() @@ -42,15 +38,20 @@ class SupportedApiSettings { } /** - * If [supportedApiListEnabled] is TRUE, calls [validatePath] to confirm whether the provided path - * is in supportedApiList. Will otherwise take no actions. + * Calls [validatePath] to confirm whether the provided path is in supportedApiList. + * Will otherwise throw an exception. * @param localUriInput The [LocalUriInput] to validate. + * @throws IllegalArgumentException When the requested API is not supported. * @return The path that was validated. */ - fun validateLocalUriInput(localUriInput: LocalUriInput): String { + fun resolveToActionRequest(localUriInput: LocalUriInput): ActionRequest { val path = localUriInput.toConstructedUri().path - if (supportedApiListEnabled) validatePath(path) - return path + validatePath(path) + return when (path) { + CLUSTER_HEALTH_PATH -> ClusterHealthRequest() + CLUSTER_STATS_PATH -> ClusterStatsRequest() + else -> throw IllegalArgumentException("Unsupported API: $path") + } } /** @@ -59,7 +60,7 @@ class SupportedApiSettings { * @param path The path to validate. * @throws IllegalArgumentException When supportedApiList does not contain the provided path. */ - private fun validatePath(path: String) { + fun validatePath(path: String) { if (!supportedApiList.contains(path)) throw IllegalArgumentException("API path not in supportedApiList: $path") } } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index 70e4e678..4e4bf577 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -2,8 +2,7 @@ package com.amazon.opendistroforelasticsearch.alerting.util import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap -import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings -import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.validateLocalUriInput +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.resolveToActionRequest import org.elasticsearch.action.ActionResponse import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse @@ -12,22 +11,17 @@ import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse import org.elasticsearch.client.Client fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { - val path = validateLocalUriInput(localUriInput) - if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { - return client.admin().cluster().health(ClusterHealthRequest()).get() + return when (val request = resolveToActionRequest(localUriInput)) { + is ClusterHealthRequest -> client.admin().cluster().health(request).get() + is ClusterStatsRequest -> client.admin().cluster().clusterStats(request).get() + else -> throw IllegalArgumentException("Unsupported API request: ${request.javaClass.name}") } - if (path == SupportedApiSettings.CLUSTER_STATS_PATH) { - return client.admin().cluster().clusterStats(ClusterStatsRequest()).get() - } - throw IllegalArgumentException("Unsupported API: $path") } fun ActionResponse.toMap(): Map { - if (this is ClusterHealthResponse) { - return this.convertToMap() - } - if (this is ClusterStatsResponse) { - return this.convertToMap() + return when (this) { + is ClusterHealthResponse -> this.convertToMap() + is ClusterStatsResponse -> this.convertToMap() + else -> throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") } - throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") } From 3ec3d71e0811efeea94358a29dad38d3cb8886d9 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Tue, 6 Apr 2021 12:31:41 -0700 Subject: [PATCH 06/12] Implemented unit tests and integration tests for LocalUriInput --- .../alerting/settings/SupportedApiSettings.kt | 2 +- .../util/SupportedApiSettingsExtensions.kt | 3 +- .../alerting/MonitorRunnerIT.kt | 127 +++++++++++ .../alerting/TestHelpers.kt | 14 ++ .../alerting/core/model/LocalUriInput.kt | 48 +++-- .../alerting/core/model/LocalUriInputTests.kt | 199 ++++++++++++++++++ 6 files changed, 373 insertions(+), 20 deletions(-) create mode 100644 core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index 392a1ed2..aad73e5c 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -23,7 +23,7 @@ class SupportedApiSettings { /** * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. */ - // TODO HURNEYT: Currently set to TRUE for testing purposes. + // TODO: Currently set to TRUE for testing purposes. // Should likely be set to FALSE by default. private var supportedApiListEnabled = true diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index f21ef898..70e4e678 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -3,6 +3,7 @@ package com.amazon.opendistroforelasticsearch.alerting.util import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.validateLocalUriInput import org.elasticsearch.action.ActionResponse import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse @@ -11,7 +12,7 @@ import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse import org.elasticsearch.client.Client fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { - val path = SupportedApiSettings.validateLocalUriInput(localUriInput) + val path = validateLocalUriInput(localUriInput) if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { return client.admin().cluster().health(ClusterHealthRequest()).get() } diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt index 5e8c1261..e2fa8d06 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt @@ -807,6 +807,133 @@ class MonitorRunnerIT : AlertingRestTestCase() { } } + fun `test LocalUriInput monitor with ClusterHealth API`() { + // GIVEN + val path = "/_cluster/health" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + val inputResults = output.stringMap("input_results") + val resultsContent = (inputResults?.get("results") as ArrayList<*>)[0] + val errorMessage = inputResults["error"] + + assertEquals(monitor.name, output["monitor_name"]) + assertTrue("Monitor results should contain cluster_name, but found: $resultsContent", + resultsContent.toString().contains("cluster_name")) + assertNull("There should not be an error message, but found: $errorMessage", errorMessage) + } + + fun `test LocalUriInput monitor with ClusterStats API`() { + // GIVEN + val path = "/_cluster/stats" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + val inputResults = output.stringMap("input_results") + val resultsContent = (inputResults?.get("results") as ArrayList<*>)[0] + val errorMessage = inputResults["error"] + + assertEquals(monitor.name, output["monitor_name"]) + assertTrue("Monitor results should contain cluster_name, but found: $resultsContent", + resultsContent.toString().contains("memory_size_in_bytes")) + assertNull("There should not be an error message, but found: $errorMessage", errorMessage) + } + + fun `test LocalUriInput monitor with alert triggered`() { + // GIVEN + putAlertMappings() + val trigger = randomTrigger(condition = Script(""" + return ctx.results[0].number_of_pending_tasks < 1 + """.trimIndent()), destinationId = createDestination().id) + val path = "/_cluster/health" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + assertEquals(monitor.name, output["monitor_name"]) + + val triggerResults = output.objectMap("trigger_results").values + for (triggerResult in triggerResults) { + assertTrue("This triggerResult should be triggered: $triggerResult", + triggerResult.objectMap("action_results").isNotEmpty()) + } + + val alerts = searchAlerts(monitor) + assertEquals("Alert not saved, $output", 1, alerts.size) + verifyAlert(alerts.single(), monitor, ACTIVE) + } + + fun `test LocalUriInput monitor with no alert triggered`() { + // GIVEN + putAlertMappings() + val trigger = randomTrigger(condition = Script(""" + return ctx.results[0].status.equals("red") + """.trimIndent())) + val path = "/_cluster/stats" + val clusterIndex = randomInt(clusterHosts.size - 1) + val input = randomLocalUriInput( + scheme = clusterHosts[clusterIndex].schemeName, + host = clusterHosts[clusterIndex].hostName, + port = clusterHosts[clusterIndex].port, + path = path + ) + val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) + + // WHEN + val response = executeMonitor(monitor.id) + + // THEN + val output = entityAsMap(response) + assertEquals(monitor.name, output["monitor_name"]) + + val triggerResults = output.objectMap("trigger_results").values + for (triggerResult in triggerResults) { + assertTrue("This triggerResult should not be triggered: $triggerResult", + triggerResult.objectMap("action_results").isEmpty()) + } + + val alerts = searchAlerts(monitor) + assertEquals("Alert saved for test monitor, output: $output", 0, alerts.size) + } + + // TODO: Once an API is implemented that supports adding/removing entries on the + // SupportedApiSettings::supportedApiList, create an test that simulates executing + // a preexisting LocalUriInput monitor for an API that has been removed from the supportedApiList. + // This will likely involve adding an API to the list before creating the monitor, and then removing + // the API from the list before executing the monitor. + private fun prepareTestAnomalyResult(detectorId: String, user: User) { val adResultIndex = ".opendistro-anomaly-results-history-2020.10.17" try { diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt index 235e5c46..8464b917 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt @@ -16,6 +16,7 @@ package com.amazon.opendistroforelasticsearch.alerting import com.amazon.opendistroforelasticsearch.alerting.core.model.Input import com.amazon.opendistroforelasticsearch.alerting.core.model.IntervalSchedule +import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.core.model.Schedule import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.string @@ -229,6 +230,19 @@ fun randomUserEmpty(): User { return User("", listOf(), listOf(), listOf()) } +fun randomLocalUriInput( + scheme: String = if (randomInt(3) >= 2) "http" else "https", + host: String = "localhost", + port: Int = randomInt(LocalUriInput.MAX_PORT), + path: String, + queryParams: Map = hashMapOf(), + url: String = "", + connectionTimeout: Int = 1 + randomInt(LocalUriInput.MAX_CONNECTION_TIMEOUT - 1), + socketTimeout: Int = 1 + randomInt(LocalUriInput.MAX_SOCKET_TIMEOUT - 1) +): LocalUriInput { + return LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) +} + fun EmailAccount.toJsonString(): String { val builder = XContentFactory.jsonBuilder() return this.toXContent(builder).string() diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 3a0bde4a..76278b27 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -21,7 +21,7 @@ data class LocalUriInput( val host: String, val port: Int, val path: String, - val params: Map, + val query_params: Map, val url: String, val connection_timeout: Int, val socket_timeout: Int @@ -30,13 +30,18 @@ data class LocalUriInput( // Verify parameters are valid during creation init { require(validateFields()) { - "Either one of url or scheme + host + port+ + path + params can be set." + "Either the url field, or scheme + host + port + path + params can be set." } - require(connection_timeout in 1..5) { - "Connection timeout: $connection_timeout is not in the range of 1 - 5" + if (port != -1) { + require(port in MIN_PORT..MAX_PORT) { + "Port: $port is not in the range of $MIN_PORT - $MAX_PORT" + } + } + require(connection_timeout in MIN_CONNECTION_TIMEOUT..MAX_CONNECTION_TIMEOUT) { + "Connection timeout: $connection_timeout is not in the range of $MIN_CONNECTION_TIMEOUT - $MAX_CONNECTION_TIMEOUT" } - require(socket_timeout in 1..60) { - "Socket timeout: $socket_timeout is not in the range of 1 - 60" + require(socket_timeout in MIN_SOCKET_TIMEOUT..MAX_SOCKET_TIMEOUT) { + "Socket timeout: $socket_timeout is not in the range of $MIN_SOCKET_TIMEOUT - $MAX_SOCKET_TIMEOUT" } // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs. @@ -57,7 +62,7 @@ data class LocalUriInput( .field(HOST_FIELD, host) .field(PORT_FIELD, port) .field(PATH_FIELD, path) - .field(PARAMS_FIELD, this.params) + .field(PARAMS_FIELD, this.query_params) .field(URL_FIELD, url) .field(CONNECTION_TIMEOUT_FIELD, connection_timeout) .field(SOCKET_TIMEOUT_FIELD, socket_timeout) @@ -74,13 +79,20 @@ data class LocalUriInput( out.writeString(host) out.writeInt(port) out.writeString(path) - out.writeMap(params) + out.writeMap(query_params) out.writeString(url) out.writeInt(connection_timeout) out.writeInt(socket_timeout) } companion object { + const val MIN_CONNECTION_TIMEOUT = 1 + const val MAX_CONNECTION_TIMEOUT = 5 + const val MIN_PORT = 1 + const val MAX_PORT = 65535 + const val MIN_SOCKET_TIMEOUT = 1 + const val MAX_SOCKET_TIMEOUT = 60 + const val SCHEME_FIELD = "scheme" const val HOST_FIELD = "host" const val PORT_FIELD = "port" @@ -104,8 +116,8 @@ data class LocalUriInput( var path = "" var params: Map = mutableMapOf() var url = "" - var connectionTimeout = 5 - var socketTimeout = 10 + var connectionTimeout = MAX_CONNECTION_TIMEOUT + var socketTimeout = MAX_SOCKET_TIMEOUT XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) @@ -128,16 +140,16 @@ data class LocalUriInput( } /** - * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[params]. + * Constructs the [URI] either using [url] or using [scheme]+[host]+[port]+[path]+[query_params]. */ fun toConstructedUri(): URI { return if (url.isEmpty()) { val uriBuilder = URIBuilder() - uriBuilder.scheme = scheme - uriBuilder.host = host - uriBuilder.port = port - uriBuilder.path = path - for (e in params.entries) + .setScheme(scheme) + .setHost(host) + .setPort(port) + .setPath(path) + for (e in query_params.entries) uriBuilder.addParameter(e.key, e.value) uriBuilder.build() } else { @@ -146,11 +158,11 @@ data class LocalUriInput( } /** - * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[params] is defined. + * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[query_params] is defined. */ private fun validateFields(): Boolean { if (url.isNotEmpty()) { - return (host.isEmpty() && (port == -1) && path.isEmpty() && params.isEmpty()) + return (host.isEmpty() && (port == -1) && path.isEmpty() && query_params.isEmpty()) } return true } diff --git a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt new file mode 100644 index 00000000..cf66658b --- /dev/null +++ b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt @@ -0,0 +1,199 @@ +package com.amazon.opendistroforelasticsearch.alerting.core.model + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class LocalUriInputTests { + private var scheme = "http" + private var host = "localhost" + private var port = 9200 + private var path = "/_cluster/health" + private var queryParams = hashMapOf() + private var url = "" + private var connectionTimeout = 5 + private var socketTimeout = 5 + + @Test + fun `test valid LocalUriInput creation using HTTP URI component fields`() { + // GIVEN + WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(scheme, localUriInput.scheme) + assertEquals(host, localUriInput.host) + assertEquals(port, localUriInput.port) + assertEquals(path, localUriInput.path) + assertEquals(queryParams, localUriInput.query_params) + assertEquals(url, localUriInput.url) + assertEquals(connectionTimeout, localUriInput.connection_timeout) + assertEquals(socketTimeout, localUriInput.socket_timeout) + } + + @Test + fun `test valid LocalUriInput creation using HTTP url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "http://localost:9200/_cluster/health" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(url, localUriInput.url) + } + + @Test + fun `test valid LocalUriInput creation using HTTPS URI component fields`() { + // GIVEN + scheme = "https" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(scheme, localUriInput.scheme) + assertEquals(host, localUriInput.host) + assertEquals(port, localUriInput.port) + assertEquals(path, localUriInput.path) + assertEquals(queryParams, localUriInput.query_params) + assertEquals(url, localUriInput.url) + assertEquals(connectionTimeout, localUriInput.connection_timeout) + assertEquals(socketTimeout, localUriInput.socket_timeout) + } + + @Test + fun `test valid LocalUriInput creation using HTTPS url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "https://localost:9200/_cluster/health" + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(url, localUriInput.url) + } + + @Test + fun `test invalid scheme`() { + // GIVEN + scheme = "invalidScheme" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid host`() { + // GIVEN + host = "loco//host" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid path`() { + // GIVEN + path = "///" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid port that's too low`() { + // GIVEN + port = LocalUriInput.MIN_PORT - 1 + + // WHEN + THEN + assertFailsWith( + "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid port that's too high`() { + // GIVEN + port = LocalUriInput.MAX_PORT + 1 + + // WHEN + THEN + assertFailsWith( + "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid connection timeout that's too low`() { + // GIVEN + connectionTimeout = LocalUriInput.MIN_CONNECTION_TIMEOUT - 1 + + // WHEN + THEN + assertFailsWith( + "Connection timeout: $connectionTimeout is not in the range of ${LocalUriInput.MIN_CONNECTION_TIMEOUT} - ${LocalUriInput.MIN_CONNECTION_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid connection timeout that's too high`() { + // GIVEN + connectionTimeout = LocalUriInput.MAX_CONNECTION_TIMEOUT + 1 + + // WHEN + THEN + assertFailsWith( + "Connection timeout: $connectionTimeout is not in the range of ${LocalUriInput.MIN_CONNECTION_TIMEOUT} - ${LocalUriInput.MIN_CONNECTION_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid socket timeout that's too low`() { + // GIVEN + socketTimeout = LocalUriInput.MIN_SOCKET_TIMEOUT - 1 + + // WHEN + THEN + assertFailsWith( + "Socket timeout: $socketTimeout is not in the range of ${LocalUriInput.MIN_SOCKET_TIMEOUT} - ${LocalUriInput.MAX_SOCKET_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid socket timeout that's too high`() { + // GIVEN + socketTimeout = LocalUriInput.MAX_SOCKET_TIMEOUT + 1 + + // WHEN + THEN + assertFailsWith( + "Socket timeout: $socketTimeout is not in the range of ${LocalUriInput.MIN_SOCKET_TIMEOUT} - ${LocalUriInput.MAX_SOCKET_TIMEOUT}") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid url`() { + // GIVEN + url = "///" + + // WHEN + THEN + assertFailsWith("Invalid url: $scheme://$host:$port$path") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test setting other fields in addition to url field`() { + // GIVEN + url = "http://localhost:9200/_cluster/health" + + // WHEN + THEN + assertFailsWith("Either the url field, or scheme + host + port + path + params can be set.") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } +} \ No newline at end of file From aa789dd07e3dda4a6a5b51cb4079a818166cca7b Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Tue, 13 Apr 2021 16:50:30 -0700 Subject: [PATCH 07/12] Refactored LocalUriInput and tests to confirm host is always localhost and port is always 9200 --- .../alerting/MonitorRunnerIT.kt | 8 --- .../alerting/TestHelpers.kt | 4 +- .../alerting/core/model/LocalUriInput.kt | 18 +++++-- .../alerting/core/model/LocalUriInputTests.kt | 52 ++++++++++++++----- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt index e2fa8d06..a0725f11 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt @@ -813,8 +813,6 @@ class MonitorRunnerIT : AlertingRestTestCase() { val clusterIndex = randomInt(clusterHosts.size - 1) val input = randomLocalUriInput( scheme = clusterHosts[clusterIndex].schemeName, - host = clusterHosts[clusterIndex].hostName, - port = clusterHosts[clusterIndex].port, path = path ) val monitor = createMonitor(randomMonitor(inputs = listOf(input))) @@ -840,8 +838,6 @@ class MonitorRunnerIT : AlertingRestTestCase() { val clusterIndex = randomInt(clusterHosts.size - 1) val input = randomLocalUriInput( scheme = clusterHosts[clusterIndex].schemeName, - host = clusterHosts[clusterIndex].hostName, - port = clusterHosts[clusterIndex].port, path = path ) val monitor = createMonitor(randomMonitor(inputs = listOf(input))) @@ -871,8 +867,6 @@ class MonitorRunnerIT : AlertingRestTestCase() { val clusterIndex = randomInt(clusterHosts.size - 1) val input = randomLocalUriInput( scheme = clusterHosts[clusterIndex].schemeName, - host = clusterHosts[clusterIndex].hostName, - port = clusterHosts[clusterIndex].port, path = path ) val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) @@ -905,8 +899,6 @@ class MonitorRunnerIT : AlertingRestTestCase() { val clusterIndex = randomInt(clusterHosts.size - 1) val input = randomLocalUriInput( scheme = clusterHosts[clusterIndex].schemeName, - host = clusterHosts[clusterIndex].hostName, - port = clusterHosts[clusterIndex].port, path = path ) val monitor = createMonitor(randomMonitor(inputs = listOf(input), triggers = listOf(trigger))) diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt index 8464b917..1c69e1ad 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/TestHelpers.kt @@ -232,8 +232,8 @@ fun randomUserEmpty(): User { fun randomLocalUriInput( scheme: String = if (randomInt(3) >= 2) "http" else "https", - host: String = "localhost", - port: Int = randomInt(LocalUriInput.MAX_PORT), + host: String = LocalUriInput.SUPPORTED_HOST, + port: Int = LocalUriInput.SUPPORTED_PORT, path: String, queryParams: Map = hashMapOf(), url: String = "", diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 76278b27..7c32e0ea 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -32,10 +32,11 @@ data class LocalUriInput( require(validateFields()) { "Either the url field, or scheme + host + port + path + params can be set." } - if (port != -1) { - require(port in MIN_PORT..MAX_PORT) { - "Port: $port is not in the range of $MIN_PORT - $MAX_PORT" - } + require(host == "" || host.toLowerCase() == SUPPORTED_HOST) { + "Only host '$SUPPORTED_HOST' is supported. Host: $host" + } + require(port == -1 || port == SUPPORTED_PORT) { + "Only port '$SUPPORTED_PORT' is supported. Port: $port" } require(connection_timeout in MIN_CONNECTION_TIMEOUT..MAX_CONNECTION_TIMEOUT) { "Connection timeout: $connection_timeout is not in the range of $MIN_CONNECTION_TIMEOUT - $MAX_CONNECTION_TIMEOUT" @@ -53,6 +54,12 @@ data class LocalUriInput( require(urlValidator.isValid(constructedUrl.toString())) { "Invalid url: $constructedUrl" } + require(constructedUrl.host.toLowerCase() == SUPPORTED_HOST) { + "Only host '$SUPPORTED_HOST' is supported. Host: $host" + } + require(constructedUrl.port == SUPPORTED_PORT) { + "Only port '$SUPPORTED_PORT' is supported. Port: $port" + } } override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { @@ -93,6 +100,9 @@ data class LocalUriInput( const val MIN_SOCKET_TIMEOUT = 1 const val MAX_SOCKET_TIMEOUT = 60 + const val SUPPORTED_HOST = "localhost" + const val SUPPORTED_PORT = 9200 + const val SCHEME_FIELD = "scheme" const val HOST_FIELD = "host" const val PORT_FIELD = "port" diff --git a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt index cf66658b..711d4e11 100644 --- a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt +++ b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt @@ -37,7 +37,7 @@ class LocalUriInputTests { host = "" port = -1 path = "" - url = "http://localost:9200/_cluster/health" + url = "http://localhost:9200/_cluster/health" // WHEN val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) @@ -72,7 +72,7 @@ class LocalUriInputTests { host = "" port = -1 path = "" - url = "https://localost:9200/_cluster/health" + url = "https://localhost:9200/_cluster/health" // WHEN val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) @@ -102,34 +102,34 @@ class LocalUriInputTests { } @Test - fun `test invalid path`() { + fun `test invalid host is not localhost`() { // GIVEN - path = "///" + host = "127.0.0.1" // WHEN + THEN - assertFailsWith("Invalid url: $scheme://$host:$port$path") { + assertFailsWith( + "Only host '${LocalUriInput.SUPPORTED_HOST}' is supported. Host: $host") { LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } } @Test - fun `test invalid port that's too low`() { + fun `test invalid path`() { // GIVEN - port = LocalUriInput.MIN_PORT - 1 + path = "///" // WHEN + THEN - assertFailsWith( - "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + assertFailsWith("Invalid url: $scheme://$host:$port$path") { LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } } @Test - fun `test invalid port that's too high`() { + fun `test invalid port`() { // GIVEN - port = LocalUriInput.MAX_PORT + 1 + port = LocalUriInput.SUPPORTED_PORT + 1 // WHEN + THEN assertFailsWith( - "Port: $port is not in the range of ${LocalUriInput.MIN_PORT} - ${LocalUriInput.MAX_PORT}") { + "Only port '${LocalUriInput.SUPPORTED_PORT}' is supported. Port: $port") { LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } } @@ -196,4 +196,32 @@ class LocalUriInputTests { assertFailsWith("Either the url field, or scheme + host + port + path + params can be set.") { LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } } + + @Test + fun `test invalid host in url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "http://127.0.0.1:9200/_cluster/health" + + // WHEN + THEN + assertFailsWith("Only host '${LocalUriInput.SUPPORTED_HOST}' is supported. Host: $host") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + + @Test + fun `test invalid port in url field`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "http://localhost:${LocalUriInput.SUPPORTED_PORT + 1}/_cluster/health" + + // WHEN + THEN + assertFailsWith("Only port '${LocalUriInput.SUPPORTED_PORT}' is supported. Port: $port") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } } \ No newline at end of file From e819bfb96a437944fcdea62ad79aa839c0d78f74 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Tue, 13 Apr 2021 17:17:35 -0700 Subject: [PATCH 08/12] Refactored SupportedApiSettings and SupportedApiSettingsExtensions based on PR feedback --- .../alerting/model/Monitor.kt | 2 +- .../alerting/settings/SupportedApiSettings.kt | 30 +++++++++---------- .../util/SupportedApiSettingsExtensions.kt | 24 ++++++--------- .../alerting/core/model/LocalUriInput.kt | 7 ++--- .../alerting/core/model/LocalUriInputTests.kt | 14 +++++++++ 5 files changed, 42 insertions(+), 35 deletions(-) diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt index 9a9e96ed..690c612a 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/Monitor.kt @@ -201,7 +201,7 @@ data class Monitor( while (xcp.nextToken() != Token.END_ARRAY) { val input = Input.parse(xcp) if (input is LocalUriInput) { - SupportedApiSettings.validateLocalUriInput(input) + SupportedApiSettings.validatePath(input.toConstructedUri().path) } inputs.add(input) } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index aad73e5c..59b579c8 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -1,6 +1,9 @@ package com.amazon.opendistroforelasticsearch.alerting.settings import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput +import org.elasticsearch.action.ActionRequest +import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest +import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest /** * A class that supports storing a unique set of API paths that can be accessed by general users. @@ -15,18 +18,10 @@ class SupportedApiSettings { * NOTE: Paths should conform to the following pattern: * "/_cluster/health" * - * The maps of supported JSON payloads were pulled from - * https://code.amazon.com/packages/CloudSearchSolrAuthService/blobs/4d6e5d39771dc6de6a6c46d9b359f42436505edf/--/etc/es/jetty-web.xml#L1702 + * Each Set represents the supported JSON payload for the respective API. */ private var supportedApiList = HashMap>>() - /** - * Set to TRUE to enable the supportedApiList check. Set to FALSE to disable. - */ - // TODO: Currently set to TRUE for testing purposes. - // Should likely be set to FALSE by default. - private var supportedApiListEnabled = true - init { supportedApiList[CLUSTER_HEALTH_PATH] = hashMapOf() supportedApiList[CLUSTER_STATS_PATH] = hashMapOf() @@ -43,15 +38,20 @@ class SupportedApiSettings { } /** - * If [supportedApiListEnabled] is TRUE, calls [validatePath] to confirm whether the provided path - * is in supportedApiList. Will otherwise take no actions. + * Calls [validatePath] to confirm whether the provided path is in supportedApiList. + * Will otherwise throw an exception. * @param localUriInput The [LocalUriInput] to validate. + * @throws IllegalArgumentException When the requested API is not supported. * @return The path that was validated. */ - fun validateLocalUriInput(localUriInput: LocalUriInput): String { + fun resolveToActionRequest(localUriInput: LocalUriInput): ActionRequest { val path = localUriInput.toConstructedUri().path - if (supportedApiListEnabled) validatePath(path) - return path + validatePath(path) + return when (path) { + CLUSTER_HEALTH_PATH -> ClusterHealthRequest() + CLUSTER_STATS_PATH -> ClusterStatsRequest() + else -> throw IllegalArgumentException("Unsupported API: $path") + } } /** @@ -60,7 +60,7 @@ class SupportedApiSettings { * @param path The path to validate. * @throws IllegalArgumentException When supportedApiList does not contain the provided path. */ - private fun validatePath(path: String) { + fun validatePath(path: String) { if (!supportedApiList.contains(path)) throw IllegalArgumentException("API path not in supportedApiList: $path") } } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index 70e4e678..4e4bf577 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -2,8 +2,7 @@ package com.amazon.opendistroforelasticsearch.alerting.util import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap -import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings -import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.validateLocalUriInput +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.resolveToActionRequest import org.elasticsearch.action.ActionResponse import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse @@ -12,22 +11,17 @@ import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse import org.elasticsearch.client.Client fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { - val path = validateLocalUriInput(localUriInput) - if (path == SupportedApiSettings.CLUSTER_HEALTH_PATH) { - return client.admin().cluster().health(ClusterHealthRequest()).get() + return when (val request = resolveToActionRequest(localUriInput)) { + is ClusterHealthRequest -> client.admin().cluster().health(request).get() + is ClusterStatsRequest -> client.admin().cluster().clusterStats(request).get() + else -> throw IllegalArgumentException("Unsupported API request: ${request.javaClass.name}") } - if (path == SupportedApiSettings.CLUSTER_STATS_PATH) { - return client.admin().cluster().clusterStats(ClusterStatsRequest()).get() - } - throw IllegalArgumentException("Unsupported API: $path") } fun ActionResponse.toMap(): Map { - if (this is ClusterHealthResponse) { - return this.convertToMap() - } - if (this is ClusterStatsResponse) { - return this.convertToMap() + return when (this) { + is ClusterHealthResponse -> this.convertToMap() + is ClusterStatsResponse -> this.convertToMap() + else -> throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") } - throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") } diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 7c32e0ea..4741d166 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -169,11 +169,10 @@ data class LocalUriInput( /** * Helper function to confirm at least [url], or [scheme]+[host]+[port]+[path]+[query_params] is defined. + * The ELSE statement currently only checks [path] as it's the only field without a default value. */ private fun validateFields(): Boolean { - if (url.isNotEmpty()) { - return (host.isEmpty() && (port == -1) && path.isEmpty() && query_params.isEmpty()) - } - return true + return if (url.isNotEmpty()) host.isEmpty() && (port == -1) && path.isEmpty() && query_params.isEmpty() + else path.isNotEmpty() } } \ No newline at end of file diff --git a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt index 711d4e11..1d7a1f03 100644 --- a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt +++ b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt @@ -197,6 +197,20 @@ class LocalUriInputTests { LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } } + @Test + fun `test LocalUriInput creation when all inputs are empty`() { + // GIVEN + scheme = "" + host = "" + port = -1 + path = "" + url = "" + + // WHEN + THEN + assertFailsWith("Either the url field, or scheme + host + port + path + params can be set.") { + LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) } + } + @Test fun `test invalid host in url field`() { // GIVEN From 61111843667a461159f44d11ef2fa59682d73caf Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Thu, 15 Apr 2021 12:48:51 -0700 Subject: [PATCH 09/12] Refactored LocalUriInput to support receiving only a path as input, and added unit test. Implemented method to redact fields from a response, and added unit tests --- .../alerting/settings/SupportedApiSettings.kt | 13 +-- .../util/SupportedApiSettingsExtensions.kt | 40 +++++++- .../SupportedApiSettingsExtensionsTests.kt | 97 +++++++++++++++++++ .../alerting/core/model/LocalUriInput.kt | 6 +- .../alerting/core/model/LocalUriInputTests.kt | 17 +++- 5 files changed, 160 insertions(+), 13 deletions(-) create mode 100644 alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index 59b579c8..ce9e5d91 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -38,11 +38,12 @@ class SupportedApiSettings { } /** - * Calls [validatePath] to confirm whether the provided path is in supportedApiList. + * Calls [validatePath] to confirm whether the provided [LocalUriInput]'s path is in [supportedApiList]. + * Will then return an [ActionRequest] for the API associated with that path. * Will otherwise throw an exception. - * @param localUriInput The [LocalUriInput] to validate. - * @throws IllegalArgumentException When the requested API is not supported. - * @return The path that was validated. + * @param localUriInput The [LocalUriInput] to resolve. + * @throws IllegalArgumentException when the requested API is not supported. + * @return The [ActionRequest] for the API associated with the provided [LocalUriInput]. */ fun resolveToActionRequest(localUriInput: LocalUriInput): ActionRequest { val path = localUriInput.toConstructedUri().path @@ -55,10 +56,10 @@ class SupportedApiSettings { } /** - * Confirms whether the provided path is in supportedApiList. + * Confirms whether the provided path is in [supportedApiList]. * Throws an exception if the provided path is not on the list; otherwise performs no action. * @param path The path to validate. - * @throws IllegalArgumentException When supportedApiList does not contain the provided path. + * @throws IllegalArgumentException when supportedApiList does not contain the provided path. */ fun validatePath(path: String) { if (!supportedApiList.contains(path)) throw IllegalArgumentException("API path not in supportedApiList: $path") diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index 4e4bf577..dd3208e0 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -2,6 +2,7 @@ package com.amazon.opendistroforelasticsearch.alerting.util import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import com.amazon.opendistroforelasticsearch.alerting.elasticapi.convertToMap +import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings import com.amazon.opendistroforelasticsearch.alerting.settings.SupportedApiSettings.Companion.resolveToActionRequest import org.elasticsearch.action.ActionResponse import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest @@ -9,7 +10,14 @@ import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest import org.elasticsearch.action.admin.cluster.stats.ClusterStatsResponse import org.elasticsearch.client.Client +import org.elasticsearch.common.xcontent.support.XContentMapValues +/** + * Calls the appropriate transport action for the API requested in the [localUriInput]. + * @param localUriInput The [LocalUriInput] to resolve. + * @param client The [Client] used to call the respective transport action. + * @throws IllegalArgumentException When the requested API is not supported by this feature. + */ fun executeTransportAction(localUriInput: LocalUriInput, client: Client): ActionResponse { return when (val request = resolveToActionRequest(localUriInput)) { is ClusterHealthRequest -> client.admin().cluster().health(request).get() @@ -18,10 +26,38 @@ fun executeTransportAction(localUriInput: LocalUriInput, client: Client): Action } } +/** + * Populates a [HashMap] with the values in the [ActionResponse]. + * @throws IllegalArgumentException when the [ActionResponse] is not supported by this feature. + */ fun ActionResponse.toMap(): Map { return when (this) { - is ClusterHealthResponse -> this.convertToMap() - is ClusterStatsResponse -> this.convertToMap() + is ClusterHealthResponse -> redactFieldsFromResponse(this.convertToMap(), + SupportedApiSettings.getSupportedJsonPayload(SupportedApiSettings.CLUSTER_HEALTH_PATH)) + is ClusterStatsResponse -> redactFieldsFromResponse(this.convertToMap(), + SupportedApiSettings.getSupportedJsonPayload(SupportedApiSettings.CLUSTER_STATS_PATH)) else -> throw IllegalArgumentException("Unsupported ActionResponse type: ${this.javaClass.name}") } } + +/** + * Populates a [HashMap] with only the values that support being exposed to users. + */ +fun redactFieldsFromResponse(mappedActionResponse: Map, supportedJsonPayload: Map>): Map { + return when { + supportedJsonPayload.isEmpty() -> mappedActionResponse + else -> { + val output = hashMapOf() + for ((key, value) in supportedJsonPayload) { + when (mappedActionResponse[key]) { + is Map<*, *> -> output[key] = XContentMapValues.filter( + mappedActionResponse[key] as MutableMap?, + value.toTypedArray(), arrayOf() + ) + else -> output[key] = value + } + } + output + } + } +} diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt new file mode 100644 index 00000000..751d6a0e --- /dev/null +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt @@ -0,0 +1,97 @@ +package com.amazon.opendistroforelasticsearch.alerting.util + +import org.elasticsearch.test.ESTestCase + +class SupportedApiSettingsExtensionsTests : ESTestCase() { + private var expectedResponse = hashMapOf() + private var mappedResponse = hashMapOf() + private var supportedJsonPayload = hashMapOf>() + + fun `test redactFieldsFromResponse with non-empty supportedJsonPayload`() { + // GIVEN + mappedResponse = hashMapOf( + ("pathRoot1" to hashMapOf( + ("pathRoot1_subPath1" to 11), + ("pathRoot1_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath1" to 121), + ("pathRoot1_subPath2_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath2_subPath1" to 1221) + )) + )) + )), + ("pathRoot2" to hashMapOf( + ("pathRoot2_subPath1" to 21), + ("pathRoot2_subPath2" to setOf(221, 222, "223string")) + )), + ("pathRoot3" to 3)) + + supportedJsonPayload = hashMapOf( + ("pathRoot1" to setOf( + "pathRoot1_subPath1", + "pathRoot1_subPath2.pathRoot1_subPath2_subPath2.pathRoot1_subPath2_subPath2_subPath1" + )), + ("pathRoot2" to setOf( + "pathRoot2_subPath2" + ))) + + expectedResponse = hashMapOf( + ("pathRoot1" to hashMapOf( + ("pathRoot1_subPath1" to 11), + ("pathRoot1_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath2_subPath1" to 1221) + )) + )) + )), + ("pathRoot2" to hashMapOf( + ("pathRoot2_subPath2" to setOf(221, 222, "223string")) + ))) + + // WHEN + val result = redactFieldsFromResponse(mappedResponse, supportedJsonPayload) + + // THEN + assertEquals(expectedResponse, result) + } + + fun `test redactFieldsFromResponse with empty supportedJsonPayload`() { + // GIVEN + mappedResponse = hashMapOf( + ("pathRoot1" to hashMapOf( + ("pathRoot1_subPath1" to 11), + ("pathRoot1_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath1" to 121), + ("pathRoot1_subPath2_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath2_subPath1" to 1221) + )) + )) + )), + ("pathRoot2" to hashMapOf( + ("pathRoot2_subPath1" to 21), + ("pathRoot2_subPath2" to setOf(221, 222, "223string")) + )), + ("pathRoot3" to 3)) + + expectedResponse = hashMapOf( + ("pathRoot1" to hashMapOf( + ("pathRoot1_subPath1" to 11), + ("pathRoot1_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath1" to 121), + ("pathRoot1_subPath2_subPath2" to hashMapOf( + ("pathRoot1_subPath2_subPath2_subPath1" to 1221) + )) + )) + )), + ("pathRoot2" to hashMapOf( + ("pathRoot2_subPath1" to 21), + ("pathRoot2_subPath2" to setOf(221, 222, "223string")) + )), + ("pathRoot3" to 3)) + + // WHEN + val result = redactFieldsFromResponse(mappedResponse, supportedJsonPayload) + + // THEN + assertEquals(expectedResponse, result) + } +} \ No newline at end of file diff --git a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt index 4741d166..dda3dbee 100644 --- a/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt +++ b/core/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInput.kt @@ -155,9 +155,9 @@ data class LocalUriInput( fun toConstructedUri(): URI { return if (url.isEmpty()) { val uriBuilder = URIBuilder() - .setScheme(scheme) - .setHost(host) - .setPort(port) + .setScheme(if (scheme.isNotEmpty()) scheme else "http") + .setHost(if (host.isNotEmpty()) host else SUPPORTED_HOST) + .setPort(if (port != -1) port else SUPPORTED_PORT) .setPath(path) for (e in query_params.entries) uriBuilder.addParameter(e.key, e.value) diff --git a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt index 1d7a1f03..0bbcba78 100644 --- a/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt +++ b/core/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/core/model/LocalUriInputTests.kt @@ -71,8 +71,6 @@ class LocalUriInputTests { scheme = "" host = "" port = -1 - path = "" - url = "https://localhost:9200/_cluster/health" // WHEN val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) @@ -81,6 +79,21 @@ class LocalUriInputTests { assertEquals(url, localUriInput.url) } + @Test + fun `test valid LocalUriInput creation with path, but empty scheme, host, and port fields`() { + // GIVEN + scheme = "" + host = "" + port = -1 + + // WHEN + val localUriInput = LocalUriInput(scheme, host, port, path, queryParams, url, connectionTimeout, socketTimeout) + + // THEN + assertEquals(path, localUriInput.path) + assertEquals(localUriInput.toConstructedUri().toString(), "http://localhost:9200/_cluster/health") + } + @Test fun `test invalid scheme`() { // GIVEN From 28d3fbab6e9e8d129ce07f49c370b0761956d719 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Thu, 15 Apr 2021 16:39:08 -0700 Subject: [PATCH 10/12] Implemented support for configuring SupportedApiSettings::supportedApiList using a JSON resource file --- .../alerting/settings/SupportedApiSettings.kt | 37 +++++++++++++++---- .../util/SupportedApiSettingsExtensions.kt | 3 +- .../settings/supported_json_payloads.json | 4 ++ .../SupportedApiSettingsExtensionsTests.kt | 6 +-- 4 files changed, 39 insertions(+), 11 deletions(-) create mode 100644 alerting/src/main/resources/com/amazon/opendistroforelasticsearch/alerting/settings/supported_json_payloads.json diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index ce9e5d91..76ac089c 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -4,6 +4,8 @@ import com.amazon.opendistroforelasticsearch.alerting.core.model.LocalUriInput import org.elasticsearch.action.ActionRequest import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest import org.elasticsearch.action.admin.cluster.stats.ClusterStatsRequest +import org.elasticsearch.common.xcontent.XContentHelper +import org.elasticsearch.common.xcontent.json.JsonXContent /** * A class that supports storing a unique set of API paths that can be accessed by general users. @@ -13,18 +15,39 @@ class SupportedApiSettings { const val CLUSTER_HEALTH_PATH = "/_cluster/health" const val CLUSTER_STATS_PATH = "/_cluster/stats" + private const val RESOURCE_FILE = "supported_json_payloads.json" + /** - * Each String represents the path to call an API. + * The key in this map represents the path to call an API. * NOTE: Paths should conform to the following pattern: - * "/_cluster/health" + * "/_cluster/stats" + * + * The value in these maps represents a path root mapped to a list of paths to field values. + * NOTE: Keys in this map should consist of root components of the response body; e.g.,: + * "indices" + * + * Values in these maps should consist of the remaining fields in the path + * to the supported value separated by periods; e.g.,: + * "shards.total", + * "shards.index.shards.min" * - * Each Set represents the supported JSON payload for the respective API. + * In this example for ClusterStats, the response will only include + * the values at the end of these two paths: + * "/_cluster/stats": { + * "indices": [ + * "shards.total", + * "shards.index.shards.min" + * ] + * } */ - private var supportedApiList = HashMap>>() + private var supportedApiList = HashMap>>() init { - supportedApiList[CLUSTER_HEALTH_PATH] = hashMapOf() - supportedApiList[CLUSTER_STATS_PATH] = hashMapOf() + val supportedJsonPayloads = SupportedApiSettings::class.java.getResource(RESOURCE_FILE) + @Suppress("UNCHECKED_CAST") + if (supportedJsonPayloads != null) supportedApiList = + XContentHelper.convertToMap( + JsonXContent.jsonXContent, supportedJsonPayloads.readText(), false) as HashMap>> } /** @@ -33,7 +56,7 @@ class SupportedApiSettings { * @return The map of all supported json payload for the requested API. * @throws IllegalArgumentException When supportedApiList does not contain a value for the provided key. */ - fun getSupportedJsonPayload(path: String): Map> { + fun getSupportedJsonPayload(path: String): Map> { return supportedApiList[path] ?: throw IllegalArgumentException("API path not in supportedApiList: $path") } diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index dd3208e0..065d082d 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -43,7 +43,8 @@ fun ActionResponse.toMap(): Map { /** * Populates a [HashMap] with only the values that support being exposed to users. */ -fun redactFieldsFromResponse(mappedActionResponse: Map, supportedJsonPayload: Map>): Map { +@Suppress("UNCHECKED_CAST") +fun redactFieldsFromResponse(mappedActionResponse: Map, supportedJsonPayload: Map>): Map { return when { supportedJsonPayload.isEmpty() -> mappedActionResponse else -> { diff --git a/alerting/src/main/resources/com/amazon/opendistroforelasticsearch/alerting/settings/supported_json_payloads.json b/alerting/src/main/resources/com/amazon/opendistroforelasticsearch/alerting/settings/supported_json_payloads.json new file mode 100644 index 00000000..b2b63f15 --- /dev/null +++ b/alerting/src/main/resources/com/amazon/opendistroforelasticsearch/alerting/settings/supported_json_payloads.json @@ -0,0 +1,4 @@ +{ + "/_cluster/health": {}, + "/_cluster/stats": {} +} \ No newline at end of file diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt index 751d6a0e..4c886ac3 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt @@ -5,7 +5,7 @@ import org.elasticsearch.test.ESTestCase class SupportedApiSettingsExtensionsTests : ESTestCase() { private var expectedResponse = hashMapOf() private var mappedResponse = hashMapOf() - private var supportedJsonPayload = hashMapOf>() + private var supportedJsonPayload = hashMapOf>() fun `test redactFieldsFromResponse with non-empty supportedJsonPayload`() { // GIVEN @@ -26,11 +26,11 @@ class SupportedApiSettingsExtensionsTests : ESTestCase() { ("pathRoot3" to 3)) supportedJsonPayload = hashMapOf( - ("pathRoot1" to setOf( + ("pathRoot1" to arrayListOf( "pathRoot1_subPath1", "pathRoot1_subPath2.pathRoot1_subPath2_subPath2.pathRoot1_subPath2_subPath2_subPath1" )), - ("pathRoot2" to setOf( + ("pathRoot2" to arrayListOf( "pathRoot2_subPath2" ))) From c05884a8d900ee8b7e9b4a1047885ca53ebdecf9 Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Thu, 22 Apr 2021 13:33:18 -0700 Subject: [PATCH 11/12] Refactored resolveToActionRequest to remove redundant call to validatePath, and javadocs to be more informative. --- .../alerting/settings/SupportedApiSettings.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt index 76ac089c..a2891888 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/SupportedApiSettings.kt @@ -19,10 +19,13 @@ class SupportedApiSettings { /** * The key in this map represents the path to call an API. + * * NOTE: Paths should conform to the following pattern: * "/_cluster/stats" * * The value in these maps represents a path root mapped to a list of paths to field values. + * If the value mapped to an API is an empty map, no fields will be redacted from the API response. + * * NOTE: Keys in this map should consist of root components of the response body; e.g.,: * "indices" * @@ -61,7 +64,6 @@ class SupportedApiSettings { } /** - * Calls [validatePath] to confirm whether the provided [LocalUriInput]'s path is in [supportedApiList]. * Will then return an [ActionRequest] for the API associated with that path. * Will otherwise throw an exception. * @param localUriInput The [LocalUriInput] to resolve. @@ -69,9 +71,7 @@ class SupportedApiSettings { * @return The [ActionRequest] for the API associated with the provided [LocalUriInput]. */ fun resolveToActionRequest(localUriInput: LocalUriInput): ActionRequest { - val path = localUriInput.toConstructedUri().path - validatePath(path) - return when (path) { + return when (val path = localUriInput.toConstructedUri().path) { CLUSTER_HEALTH_PATH -> ClusterHealthRequest() CLUSTER_STATS_PATH -> ClusterStatsRequest() else -> throw IllegalArgumentException("Unsupported API: $path") From 1d03c611b77831829477009dbe9033ff202c71ae Mon Sep 17 00:00:00 2001 From: Thomas Hurney Date: Mon, 3 May 2021 10:43:34 -0700 Subject: [PATCH 12/12] Refactored an erroneous value assignment in the SupportedApiSettingsExtensions::redactFieldsFromResponse output. --- .../alerting/util/SupportedApiSettingsExtensions.kt | 4 ++-- .../util/SupportedApiSettingsExtensionsTests.kt | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt index 065d082d..a6cb53b6 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt +++ b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensions.kt @@ -50,12 +50,12 @@ fun redactFieldsFromResponse(mappedActionResponse: Map, supportedJs else -> { val output = hashMapOf() for ((key, value) in supportedJsonPayload) { - when (mappedActionResponse[key]) { + when (val mappedValue = mappedActionResponse[key]) { is Map<*, *> -> output[key] = XContentMapValues.filter( mappedActionResponse[key] as MutableMap?, value.toTypedArray(), arrayOf() ) - else -> output[key] = value + else -> output[key] = mappedValue ?: hashMapOf() } } output diff --git a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt index 4c886ac3..a01bd7aa 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/SupportedApiSettingsExtensionsTests.kt @@ -23,7 +23,10 @@ class SupportedApiSettingsExtensionsTests : ESTestCase() { ("pathRoot2_subPath1" to 21), ("pathRoot2_subPath2" to setOf(221, 222, "223string")) )), - ("pathRoot3" to 3)) + ("pathRoot3" to hashMapOf( + ("pathRoot3_subPath1" to 31), + ("pathRoot3_subPath2" to setOf(321, 322, "323string")) + ))) supportedJsonPayload = hashMapOf( ("pathRoot1" to arrayListOf( @@ -32,7 +35,8 @@ class SupportedApiSettingsExtensionsTests : ESTestCase() { )), ("pathRoot2" to arrayListOf( "pathRoot2_subPath2" - ))) + )), + ("pathRoot3" to arrayListOf())) expectedResponse = hashMapOf( ("pathRoot1" to hashMapOf( @@ -45,6 +49,10 @@ class SupportedApiSettingsExtensionsTests : ESTestCase() { )), ("pathRoot2" to hashMapOf( ("pathRoot2_subPath2" to setOf(221, 222, "223string")) + )), + ("pathRoot3" to hashMapOf( + ("pathRoot3_subPath1" to 31), + ("pathRoot3_subPath2" to setOf(321, 322, "323string")) ))) // WHEN