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

Update dependency com.pinterest:ktlint to 0.47.1 and fix CVE-2023-6378 for common-utils #585

Merged
merged 3 commits into from
Feb 6, 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 @@ -226,4 +231,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 @@ -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 @@ -29,6 +29,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 @@ -48,6 +49,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 @@ -38,7 +38,9 @@
sin.readEnum(RestRequest.Method::class.java), // method
if (sin.readBoolean()) {
FetchSourceContext(sin) // srcContext
} else null
} else {
null

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

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/GetMonitorRequest.kt#L42

Added line #L42 was not covered by tests
}
)

override fun validate(): ActionRequestValidationException? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
seqNo: Long,
primaryTerm: Long,
monitor: Monitor?,
associatedCompositeMonitors: List<AssociatedWorkflow>?,
associatedCompositeMonitors: List<AssociatedWorkflow>?
) : super() {
this.id = id
this.version = version
Expand All @@ -50,8 +50,10 @@
primaryTerm = sin.readLong(), // primaryTerm
monitor = if (sin.readBoolean()) {
Monitor.readFrom(sin) // monitor
} else null,
associatedCompositeMonitors = sin.readList((AssociatedWorkflow)::readFrom),
} else {
null

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

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/GetMonitorResponse.kt#L54

Added line #L54 was not covered by tests
},
associatedCompositeMonitors = sin.readList((AssociatedWorkflow)::readFrom)

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

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/commons/alerting/action/GetMonitorResponse.kt#L56

Added line #L56 was not covered by tests
)

@Throws(IOException::class)
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 @@ -108,7 +108,10 @@
}

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

Check warning on line 114 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#L111-L114

Added lines #L111 - L114 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 @@ -36,7 +36,6 @@ data class AlertError(val timestamp: Instant, var 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
20 changes: 13 additions & 7 deletions src/main/kotlin/org/opensearch/commons/alerting/model/Alert.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@
val aggregationResultBucket: AggregationResultBucket? = null,
val executionId: String? = null,
val associatedAlertIds: List<String>,
val clusters: List<String>? = null,
val clusters: List<String>? = null
) : 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 52 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#L52

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

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

Check warning on line 314 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#L314

Added line #L314 was not covered by tests
},
triggerId = sin.readString(),
triggerName = sin.readString(),
findingIds = sin.readStringList(),
Expand Down Expand Up @@ -402,7 +406,6 @@
@JvmOverloads
@Throws(IOException::class)
fun parse(xcp: XContentParser, id: String = NO_ID, version: Long = NO_VERSION): Alert {

lateinit var monitorId: String
var workflowId = ""
var workflowName = ""
Expand Down Expand Up @@ -440,8 +443,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 @@ -88,7 +88,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 @@ -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
Loading
Loading