Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Backport 585 and 582 to 2.5 #613

Merged
merged 5 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@ apply plugin: 'opensearch.repositories'
apply from: 'build-tools/opensearchplugin-coverage.gradle'

configurations {
ktlint
ktlint {
resolutionStrategy {
force "ch.qos.logback:logback-classic:1.3.14"
force "ch.qos.logback:logback-core:1.3.14"
}
}
}

dependencies {
Expand All @@ -86,7 +91,7 @@ dependencies {
testImplementation "commons-validator:commons-validator:1.7"
testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.2'

ktlint "com.pinterest:ktlint:0.44.0"
ktlint "com.pinterest:ktlint:0.47.1"
}

test {
Expand Down Expand Up @@ -224,4 +229,4 @@ task updateVersion {
// Include the required files that needs to be updated with new Version
ant.replaceregexp(file:'build.gradle', match: '"opensearch.version", "\\d.*"', replace: '"opensearch.version", "' + newVersion.tokenize('-')[0] + '-SNAPSHOT"', flags:'g', byline:true)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class AcknowledgeAlertResponse : BaseResponse {

@Throws(IOException::class)
override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder {

builder.startObject().startArray("success")
acknowledged.forEach { builder.value(it.id) }
builder.endArray().startArray("failed")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@ object AlertingActions {
@JvmField
val INDEX_MONITOR_ACTION_TYPE =
ActionType(INDEX_MONITOR_ACTION_NAME, ::IndexMonitorResponse)

@JvmField
val GET_ALERTS_ACTION_TYPE =
ActionType(GET_ALERTS_ACTION_NAME, ::GetAlertsResponse)

@JvmField
val DELETE_MONITOR_ACTION_TYPE =
ActionType(DELETE_MONITOR_ACTION_NAME, ::DeleteMonitorResponse)

@JvmField
val GET_FINDINGS_ACTION_TYPE =
ActionType(GET_FINDINGS_ACTION_NAME, ::GetFindingsResponse)

@JvmField
val ACKNOWLEDGE_ALERTS_ACTION_TYPE =
ActionType(ACKNOWLEDGE_ALERTS_ACTION_NAME, ::AcknowledgeAlertResponse)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@
}

return BucketSelectorIndices(
name(), parentBucketPath, selectedBucketsIndex, originalAgg.metadata
name(),
parentBucketPath,
selectedBucketsIndex,
originalAgg.metadata

Check warning on line 135 in src/main/kotlin/org/opensearch/commons/alerting/aggregation/bucketselectorext/BucketSelectorExtAggregator.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/aggregation/bucketselectorext/BucketSelectorExtAggregator.kt#L132-L135

Added lines #L132 - L135 were not covered by tests
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import java.io.IOException
class BucketSelectorExtFilter : BaseModel {
// used for composite aggregations
val filtersMap: HashMap<String, IncludeExclude>?

// used for filtering string term aggregation
val filters: IncludeExclude?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ data class AlertError(val timestamp: Instant, val message: String) : Writeable,
@JvmStatic
@Throws(IOException::class)
fun parse(xcp: XContentParser): AlertError {

lateinit var timestamp: Instant
lateinit var message: String

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@
throw ParsingException(
xcp.tokenLocation,
String.format(
Locale.ROOT, "Failed to parse object: expecting token with name [%s] but found [%s]",
CONFIG_NAME, xcp.currentName()
Locale.ROOT,
"Failed to parse object: expecting token with name [%s] but found [%s]",
CONFIG_NAME,
xcp.currentName()

Check warning on line 65 in src/main/kotlin/org/opensearch/commons/alerting/model/AggregationResultBucket.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/AggregationResultBucket.kt#L62-L65

Added lines #L62 - L65 were not covered by tests
)
)
}
Expand Down
14 changes: 9 additions & 5 deletions src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@
) : Writeable, ToXContent {

init {
if (errorMessage != null) require(state == State.DELETED || state == State.ERROR) {
"Attempt to create an alert with an error in state: $state"
if (errorMessage != null) {
require(state == State.DELETED || state == State.ERROR) {
"Attempt to create an alert with an error in state: $state"

Check warning on line 47 in src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt#L47

Added line #L47 was not covered by tests
}
}
}

Expand Down Expand Up @@ -139,7 +141,9 @@
monitorVersion = sin.readLong(),
monitorUser = if (sin.readBoolean()) {
User(sin)
} else null,
} else {
null

Check warning on line 145 in src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt#L145

Added line #L145 was not covered by tests
},
triggerId = sin.readString(),
triggerName = sin.readString(),
findingIds = sin.readStringList(),
Expand Down Expand Up @@ -216,10 +220,10 @@
const val NO_ID = ""
const val NO_VERSION = Versions.NOT_FOUND

@JvmStatic @JvmOverloads
@JvmStatic
@JvmOverloads
@Throws(IOException::class)
fun parse(xcp: XContentParser, id: String = NO_ID, version: Long = NO_VERSION): Alert {

lateinit var monitorId: String
var schemaVersion = NO_SCHEMA_VERSION
lateinit var monitorName: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ data class BucketLevelTrigger(
const val PARENT_BUCKET_PATH = "parentBucketPath"

val XCONTENT_REGISTRY = NamedXContentRegistry.Entry(
Trigger::class.java, ParseField(BUCKET_LEVEL_TRIGGER_FIELD),
Trigger::class.java,
ParseField(BUCKET_LEVEL_TRIGGER_FIELD),
CheckedFunction { parseInner(it) }
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ data class ClusterMetricsInput(
"Invalid URI constructed from the path and path_params inputs, or the url input."
}

if (url.isNotEmpty() && validateFieldsNotEmpty())
if (url.isNotEmpty() && validateFieldsNotEmpty()) {
require(constructedUri == constructUrlFromInputs()) {
"The provided URL and URI fields form different URLs."
}
}

require(constructedUri.host.lowercase() == SUPPORTED_HOST) {
"Only host '$SUPPORTED_HOST' is supported."
Expand Down Expand Up @@ -104,7 +105,8 @@ data class ClusterMetricsInput(
/**
* This parse function uses [XContentParser] to parse JSON input and store corresponding fields to create a [ClusterMetricsInput] object
*/
@JvmStatic @Throws(IOException::class)
@JvmStatic
@Throws(IOException::class)
fun parseInner(xcp: XContentParser): ClusterMetricsInput {
var path = ""
var pathParams = ""
Expand Down Expand Up @@ -161,17 +163,20 @@ data class ClusterMetricsInput(
if (pathParams.isNotEmpty()) {
pathParams = pathParams.trim('/')
ILLEGAL_PATH_PARAMETER_CHARACTERS.forEach { character ->
if (pathParams.contains(character))
if (pathParams.contains(character)) {
throw IllegalArgumentException(
"The provided path parameters contain invalid characters or spaces. Please omit: " + "${ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")}"
)
}
}
}

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

return pathParams
}
Expand All @@ -187,11 +192,13 @@ data class ClusterMetricsInput(
ClusterMetricType.values()
.filter { option -> option != ClusterMetricType.BLANK }
.forEach { option ->
if (uriPath.startsWith(option.prependPath) || uriPath.startsWith(option.defaultPath))
if (uriPath.startsWith(option.prependPath) || uriPath.startsWith(option.defaultPath)) {
apiType = option
}
}
if (apiType.isBlank())
if (apiType.isBlank()) {
throw IllegalArgumentException("The API could not be determined from the provided URI.")
}
return apiType
}

Expand All @@ -213,12 +220,15 @@ data class ClusterMetricsInput(
* If [path] and [pathParams] are empty, populates them with values from [url].
*/
private fun parseEmptyFields() {
if (pathParams.isEmpty())
if (pathParams.isEmpty()) {
pathParams = this.parsePathParams()
if (path.isEmpty())
}
if (path.isEmpty()) {
path = if (pathParams.isEmpty()) clusterMetricType.defaultPath else clusterMetricType.prependPath
if (url.isEmpty())
}
if (url.isEmpty()) {
url = constructedUri.toString()
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
ALERTS_HISTORY_INDEX_FIELD to alertsHistoryIndex,
ALERTS_HISTORY_INDEX_PATTERN_FIELD to alertsHistoryIndexPattern,
QUERY_INDEX_MAPPINGS_BY_TYPE to queryIndexMappingsByType,
FINDINGS_ENABLED_FIELD to findingsEnabled,
FINDINGS_ENABLED_FIELD to findingsEnabled

Check warning on line 91 in src/main/kotlin/org/opensearch/commons/alerting/model/DataSources.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/DataSources.kt#L91

Added line #L91 was not covered by tests
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@

val XCONTENT_REGISTRY = NamedXContentRegistry.Entry(
Input::class.java,
ParseField(DOC_LEVEL_INPUT_FIELD), CheckedFunction { parse(it) }
ParseField(DOC_LEVEL_INPUT_FIELD),
CheckedFunction { parse(it) }

Check warning on line 68 in src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelMonitorInput.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelMonitorInput.kt#L68

Added line #L68 was not covered by tests
)

@JvmStatic @Throws(IOException::class)
@JvmStatic
@Throws(IOException::class)
fun parse(xcp: XContentParser): DocLevelMonitorInput {
var description: String = NO_DESCRIPTION
val indices: MutableList<String> = mutableListOf()
Expand Down Expand Up @@ -106,7 +108,8 @@
return DocLevelMonitorInput(description = description, indices = indices, queries = docLevelQueries)
}

@JvmStatic @Throws(IOException::class)
@JvmStatic
@Throws(IOException::class)
fun readFrom(sin: StreamInput): DocLevelMonitorInput {
return DocLevelMonitorInput(sin)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ data class DocLevelQuery(
val id: String = UUID.randomUUID().toString(),
val name: String,
val query: String,
val tags: List<String> = mutableListOf()
val tags: List<String> = mutableListOf(),
val queryFieldNames: List<String> = mutableListOf()
) : BaseModel {

init {
Expand All @@ -31,15 +32,17 @@ data class DocLevelQuery(
sin.readString(), // id
sin.readString(), // name
sin.readString(), // query
sin.readStringList() // tags
sin.readStringList(), // tags,
sin.readStringList() // fieldsBeingQueried
)

fun asTemplateArg(): Map<String, Any> {
return mapOf(
QUERY_ID_FIELD to id,
NAME_FIELD to name,
QUERY_FIELD to query,
TAGS_FIELD to tags
TAGS_FIELD to tags,
QUERY_FIELD_NAMES_FIELD to queryFieldNames
)
}

Expand All @@ -49,6 +52,7 @@ data class DocLevelQuery(
out.writeString(name)
out.writeString(query)
out.writeStringCollection(tags)
out.writeStringCollection(queryFieldNames)
}

override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder {
Expand All @@ -57,6 +61,7 @@ data class DocLevelQuery(
.field(NAME_FIELD, name)
.field(QUERY_FIELD, query)
.field(TAGS_FIELD, tags.toTypedArray())
.field(QUERY_FIELD_NAMES_FIELD, queryFieldNames.toTypedArray())
.endObject()
return builder
}
Expand All @@ -66,15 +71,18 @@ data class DocLevelQuery(
const val NAME_FIELD = "name"
const val QUERY_FIELD = "query"
const val TAGS_FIELD = "tags"
const val QUERY_FIELD_NAMES_FIELD = "query_field_names"
const val NO_ID = ""
val INVALID_CHARACTERS: List<String> = listOf(" ", "[", "]", "{", "}", "(", ")")

@JvmStatic @Throws(IOException::class)
@JvmStatic
@Throws(IOException::class)
fun parse(xcp: XContentParser): DocLevelQuery {
var id: String = UUID.randomUUID().toString()
lateinit var query: String
lateinit var name: String
val tags: MutableList<String> = mutableListOf()
val queryFieldNames: MutableList<String> = mutableListOf()

XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp)
while (xcp.nextToken() != XContentParser.Token.END_OBJECT) {
Expand All @@ -87,6 +95,7 @@ data class DocLevelQuery(
name = xcp.text()
validateQuery(name)
}

QUERY_FIELD -> query = xcp.text()
TAGS_FIELD -> {
XContentParserUtils.ensureExpectedToken(
Expand All @@ -100,18 +109,32 @@ data class DocLevelQuery(
tags.add(tag)
}
}

QUERY_FIELD_NAMES_FIELD -> {
XContentParserUtils.ensureExpectedToken(
XContentParser.Token.START_ARRAY,
xcp.currentToken(),
xcp
)
while (xcp.nextToken() != XContentParser.Token.END_ARRAY) {
val field = xcp.text()
queryFieldNames.add(field)
}
}
}
}

return DocLevelQuery(
id = id,
name = name,
query = query,
tags = tags
tags = tags,
queryFieldNames = queryFieldNames
)
}

@JvmStatic @Throws(IOException::class)
@JvmStatic
@Throws(IOException::class)
fun readFrom(sin: StreamInput): DocLevelQuery {
return DocLevelQuery(sin)
}
Expand All @@ -127,4 +150,18 @@ data class DocLevelQuery(
}
}
}

// constructor for java plugins' convenience to optionally avoid passing empty list for 'fieldsBeingQueried' field
constructor(
id: String,
name: String,
query: String,
tags: MutableList<String>
) : this(
id = id,
name = name,
query = query,
tags = tags,
queryFieldNames = emptyList()
)
}
Loading
Loading