Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Backport 2.11] Backports PRs #585 and #597 to 2.11 #615

Merged
merged 2 commits into from
Mar 14, 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
9 changes: 7 additions & 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AcknowledgeChainedAlertRequest : ActionRequest {

constructor(
workflowId: String,
alertIds: List<String>,
alertIds: List<String>
) : super() {
this.workflowId = workflowId
this.alertIds = alertIds
Expand All @@ -28,7 +28,7 @@ class AcknowledgeChainedAlertRequest : ActionRequest {
@Throws(IOException::class)
constructor(sin: StreamInput) : this(
sin.readString(), // workflowId
Collections.unmodifiableList(sin.readStringList()), // alertIds
Collections.unmodifiableList(sin.readStringList()) // alertIds
)

override fun validate(): ActionRequestValidationException? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ object AlertingActions {
@JvmField
val INDEX_WORKFLOW_ACTION_TYPE =
ActionType(INDEX_WORKFLOW_ACTION_NAME, ::IndexWorkflowResponse)

@JvmField
val GET_ALERTS_ACTION_TYPE =
ActionType(GET_ALERTS_ACTION_NAME, ::GetAlertsResponse)
Expand All @@ -45,6 +46,7 @@ object AlertingActions {
@JvmField
val DELETE_WORKFLOW_ACTION_TYPE =
ActionType(DELETE_WORKFLOW_ACTION_NAME, ::DeleteWorkflowResponse)

@JvmField
val GET_FINDINGS_ACTION_TYPE =
ActionType(GET_FINDINGS_ACTION_NAME, ::GetFindingsResponse)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import java.io.IOException
class DeleteWorkflowRequest : ActionRequest {

val workflowId: String

/**
* Flag that indicates whether the delegate monitors should be deleted or not.
* If the flag is set to true, Delegate monitors will be deleted only in the case when they are part of the specified workflow and no other.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class GetWorkflowAlertsRequest : ActionRequest {
monitorIds: List<String>? = null,
workflowIds: List<String>? = null,
alertIds: List<String>? = null,
getAssociatedAlerts: Boolean,
getAssociatedAlerts: Boolean
) : super() {
this.table = table
this.severityLevel = severityLevel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import java.util.Collections
class GetWorkflowAlertsResponse : BaseResponse {
val alerts: List<Alert>
val associatedAlerts: List<Alert>

// totalAlerts is not the same as the size of alerts because there can be 30 alerts from the request, but
// the request only asked for 5 alerts, so totalAlerts will be 30, but alerts will only contain 5 alerts
val totalAlerts: Int?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@
sin.readEnum(RestStatus::class.java), // RestStatus
if (sin.readBoolean()) {
Workflow.readFrom(sin) // monitor
} else null
} else {
null

Check warning on line 55 in src/main/kotlin/org/opensearch/commons/alerting/action/GetWorkflowResponse.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/GetWorkflowResponse.kt#L55

Added line #L55 was not covered by tests
}
)

@Throws(IOException::class)
Expand All @@ -76,8 +78,9 @@
.field(_VERSION, version)
.field(_SEQ_NO, seqNo)
.field(_PRIMARY_TERM, primaryTerm)
if (workflow != null)
if (workflow != null) {
builder.field("workflow", workflow)
}

return builder.endObject()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,55 +57,61 @@

if (workflow.inputs.isEmpty()) {
validationException = ValidateActions.addValidationError(
"Input list can not be empty.", validationException
"Input list can not be empty.",
validationException
)
return validationException
}
if (workflow.inputs.size > 1) {
validationException = ValidateActions.addValidationError(
"Input list can contain only one element.", validationException
"Input list can contain only one element.",
validationException

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

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/IndexWorkflowRequest.kt#L67-L68

Added lines #L67 - L68 were not covered by tests
)
return validationException
}
if (workflow.inputs[0] !is CompositeInput) {
validationException = ValidateActions.addValidationError(
"When creating a workflow input must be CompositeInput", validationException
"When creating a workflow input must be CompositeInput",
validationException

Check warning on line 75 in src/main/kotlin/org/opensearch/commons/alerting/action/IndexWorkflowRequest.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/IndexWorkflowRequest.kt#L74-L75

Added lines #L74 - L75 were not covered by tests
)
}
val compositeInput = workflow.inputs[0] as CompositeInput
val monitorIds = compositeInput.sequence.delegates.stream().map { it.monitorId }.collect(Collectors.toList())

if (monitorIds.isNullOrEmpty()) {
validationException = ValidateActions.addValidationError(
"Delegates list can not be empty.", validationException
"Delegates list can not be empty.",
validationException
)
// Break the flow because next checks are dependant on non-null monitorIds
return validationException
}

if (monitorIds.size > MAX_DELEGATE_SIZE) {
validationException = ValidateActions.addValidationError(
"Delegates list can not be larger then $MAX_DELEGATE_SIZE.", validationException
"Delegates list can not be larger then $MAX_DELEGATE_SIZE.",
validationException
)
}

if (monitorIds.toSet().size != monitorIds.size) {
validationException = ValidateActions.addValidationError(
"Duplicate delegates not allowed", validationException
"Duplicate delegates not allowed",
validationException
)
}
val delegates = compositeInput.sequence.delegates
val orderSet = delegates.stream().filter { it.order > 0 }.map { it.order }.collect(Collectors.toSet())
if (orderSet.size != delegates.size) {
validationException = ValidateActions.addValidationError(
"Sequence ordering of delegate monitor shouldn't contain duplicate order values", validationException
"Sequence ordering of delegate monitor shouldn't contain duplicate order values",
validationException
)
}

val monitorIdOrderMap: Map<String, Int> = delegates.associate { it.monitorId to it.order }
delegates.forEach {
if (it.chainedMonitorFindings != null) {

if (it.chainedMonitorFindings.monitorId != null) {
if (monitorIdOrderMap.containsKey(it.chainedMonitorFindings.monitorId) == false) {
validationException = ValidateActions.addValidationError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
script: Script,
gapPolicy: BucketHelpers.GapPolicy,
filter: BucketSelectorExtFilter?,
metadata: Map<String, Any>?,
metadata: Map<String, Any>?
) : super(name, bucketsPathsMap.values.toTypedArray(), metadata) {
this.bucketsPathsMap = bucketsPathsMap
this.parentBucketPath = parentBucketPath
Expand Down Expand Up @@ -132,7 +132,7 @@
name(),
parentBucketPath,
selectedBucketsIndex,
originalAgg.metadata,
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#L135

Added line #L135 was not covered by tests
)
}

Expand Down
31 changes: 19 additions & 12 deletions src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@
val actionExecutionResults: List<ActionExecutionResult>,
val aggregationResultBucket: AggregationResultBucket? = null,
val executionId: String? = null,
val associatedAlertIds: List<String>,
val associatedAlertIds: List<String>
) : Writeable, ToXContent {

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

Check warning on line 51 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#L51

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

Expand All @@ -60,7 +62,7 @@
executionId: String,
chainedAlertTrigger: ChainedAlertTrigger,
workflow: Workflow,
associatedAlertIds: List<String>,
associatedAlertIds: List<String>
) : this(
monitorId = NO_ID,
monitorName = "",
Expand Down Expand Up @@ -96,7 +98,7 @@
actionExecutionResults: List<ActionExecutionResult> = mutableListOf(),
schemaVersion: Int = NO_SCHEMA_VERSION,
executionId: String? = null,
workflowId: String? = null,
workflowId: String? = null
) : this(
monitorId = monitor.id,
monitorName = monitor.name,
Expand Down Expand Up @@ -133,7 +135,7 @@
schemaVersion: Int = NO_SCHEMA_VERSION,
findingIds: List<String> = emptyList(),
executionId: String? = null,
workflowId: String? = null,
workflowId: String? = null

Check warning on line 138 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#L138

Added line #L138 was not covered by tests
) : this(
monitorId = monitor.id,
monitorName = monitor.name,
Expand Down Expand Up @@ -171,7 +173,7 @@
aggregationResultBucket: AggregationResultBucket,
findingIds: List<String> = emptyList(),
executionId: String? = null,
workflowId: String? = null,
workflowId: String? = null
) : this(
monitorId = monitor.id,
monitorName = monitor.name,
Expand Down Expand Up @@ -210,7 +212,7 @@
actionExecutionResults: List<ActionExecutionResult> = mutableListOf(),
schemaVersion: Int = NO_SCHEMA_VERSION,
executionId: String? = null,
workflowId: String? = null,
workflowId: String? = null

Check warning on line 215 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#L215

Added line #L215 was not covered by tests
) : this(
id = id,
monitorId = monitor.id,
Expand Down Expand Up @@ -247,7 +249,7 @@
errorHistory: List<AlertError> = mutableListOf(),
schemaVersion: Int = NO_SCHEMA_VERSION,
workflowId: String? = null,
executionId: String?,
executionId: String?
) : this(
id = id,
monitorId = monitor.id,
Expand Down Expand Up @@ -295,7 +297,9 @@
monitorVersion = sin.readLong(),
monitorUser = if (sin.readBoolean()) {
User(sin)
} else null,
} else {
null

Check warning on line 301 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#L301

Added line #L301 was not covered by tests
},
triggerId = sin.readString(),
triggerName = sin.readString(),
findingIds = sin.readStringList(),
Expand Down Expand Up @@ -422,8 +426,11 @@
MONITOR_NAME_FIELD -> monitorName = xcp.text()
MONITOR_VERSION_FIELD -> monitorVersion = xcp.longValue()
MONITOR_USER_FIELD ->
monitorUser = if (xcp.currentToken() == XContentParser.Token.VALUE_NULL) null
else User.parse(xcp)
monitorUser = if (xcp.currentToken() == XContentParser.Token.VALUE_NULL) {
null
} else {
User.parse(xcp)
}
TRIGGER_ID_FIELD -> triggerId = xcp.text()
FINDING_IDS -> {
ensureExpectedToken(XContentParser.Token.START_ARRAY, xcp.currentToken(), xcp)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import java.util.Collections
// TODO - Remove the class and move the monitorId to Delegate (as a chainedMonitorId property) if this class won't be updated by adding new properties
data class ChainedMonitorFindings(
val monitorId: String? = null,
val monitorIds: List<String> = emptyList(), // if monitorId field is non-null it would be given precendence for BWC
val monitorIds: List<String> = emptyList() // if monitorId field is non-null it would be given precendence for BWC
) : BaseModel {

init {
Expand Down Expand Up @@ -75,8 +75,9 @@ data class ChainedMonitorFindings(

when (fieldName) {
MONITOR_ID_FIELD -> {
if (!xcp.currentToken().equals(XContentParser.Token.VALUE_NULL))
if (!xcp.currentToken().equals(XContentParser.Token.VALUE_NULL)) {
monitorId = xcp.text()
}
}

MONITOR_IDS_FIELD -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import java.io.IOException

data class CompositeInput(
val sequence: Sequence,
val sequence: Sequence
) : WorkflowInput {
@Throws(IOException::class)
constructor(sin: StreamInput) : this(
Expand Down Expand Up @@ -53,7 +53,8 @@

val XCONTENT_REGISTRY = NamedXContentRegistry.Entry(
WorkflowInput::class.java,
ParseField(COMPOSITE_INPUT_FIELD), CheckedFunction { CompositeInput.parse(it) }
ParseField(COMPOSITE_INPUT_FIELD),
CheckedFunction { CompositeInput.parse(it) }

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

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/model/CompositeInput.kt#L57

Added line #L57 was not covered by tests
)

@JvmStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ data class Delegate(
monitorId = sin.readString(),
chainedMonitorFindings = if (sin.readBoolean()) {
ChainedMonitorFindings(sin)
} else null,
} else {
null
}
)

fun asTemplateArg(): Map<String, Any> {
return mapOf(
ORDER_FIELD to order,
MONITOR_ID_FIELD to monitorId,
MONITOR_ID_FIELD to monitorId
)
}

Expand Down
Loading
Loading