Skip to content

feat(alerting): Block non-PPL monitor CRUD on pluggable dataformat domains #2141

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.

Sign up for GitHub

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

Merged
eirsep merged 1 commit into from
May 16, 2026
Merged

feat(alerting): Block non-PPL monitor CRUD on pluggable dataformat domains #2141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 40 additions & 0 deletions alerting/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,46 @@ integTest {
excludeTestsMatching "org.opensearch.alerting.bwc.*IT"
}
}

task integTestPluggableDataformat(type: RestIntegTestTask) {
Copy link
Copy Markdown
Member

@eirsep eirsep May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will this task run in our github CIs
can you edit github workflow.yml and add this too

Copy link
Copy Markdown
Contributor Author

@yashgupte21 yashgupte21 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In build.gradle added integTest.finalizedBy which automatically runs integTestPluggableDataFormat task after integTest task , they're included in the CIs. Adding it to workflows could be taken in follow up PR to double guard this .

description = "Runs integration tests with pluggable dataformat feature flag enabled"
dependsOn bundlePlugin
testClassesDirs = sourceSets.test.output.classesDirs
classpath = sourceSets.test.runtimeClasspath

systemProperty 'tests.security.manager', 'false'
systemProperty 'java.io.tmpdir', opensearch_tmp_dir.absolutePath

filter {
includeTestsMatching "org.opensearch.alerting.transport.PluggableDataFormatMonitorBlockIT"
}
}

testClusters.integTestPluggableDataformat {
testDistribution = "ARCHIVE"
systemProperty 'opensearch.experimental.feature.pluggable.dataformat.enabled', 'true'

// Include same plugins as main integTest cluster
plugin(provider({ new RegularFile() { @Override File getAsFile() { return configurations.zipArchive.asFileTree.matching { include '**/opensearch-notifications-core*' }.singleFile } } }))
plugin(provider({ new RegularFile() { @Override File getAsFile() { return configurations.zipArchive.asFileTree.matching { include '**/notifications*' }.singleFile } } }))
plugin(provider({ new RegularFile() { @Override File getAsFile() { return configurations.zipArchive.asFileTree.matching { include '**/opensearch-job-scheduler*' }.singleFile } } }))
plugin(provider({ new RegularFile() { @Override File getAsFile() { return configurations.zipArchive.asFileTree.matching { include '**/opensearch-sql-plugin*' }.singleFile } } }))

}

// Install alerting plugin on the pluggable dataformat test cluster
integTestPluggableDataformat.dependsOn(bundle)
integTestPluggableDataformat.getClusters().forEach{c -> c.plugin(project.getObjects().fileProperty().value(bundle.getArchiveFile()))}

// Exclude pluggable dataformat tests from the main integTest task
integTest {
filter {
excludeTestsMatching "org.opensearch.alerting.transport.PluggableDataFormatMonitorBlockIT"
}
}

check.dependsOn integTestPluggableDataformat
integTest.finalizedBy integTestPluggableDataformat
}

task integTestRemote(type: RestIntegTestTask) {
Expand Down
18 changes: 18 additions & 0 deletions alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportExecuteMonitorAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.opensearch.alerting.util.use
import org.opensearch.cluster.service.ClusterService
import org.opensearch.common.inject.Inject
import org.opensearch.common.settings.Settings
import org.opensearch.common.util.FeatureFlags
import org.opensearch.common.xcontent.LoggingDeprecationHandler
import org.opensearch.common.xcontent.XContentHelper
import org.opensearch.common.xcontent.XContentType
Expand All @@ -35,6 +36,7 @@ import org.opensearch.commons.alerting.model.Monitor
import org.opensearch.commons.alerting.model.ScheduledJob
import org.opensearch.commons.alerting.util.AlertingException
import org.opensearch.commons.alerting.util.isMonitorOfStandardType
import org.opensearch.commons.alerting.util.isPPLMonitor
import org.opensearch.commons.authuser.User
import org.opensearch.commons.utils.TenantContext
import org.opensearch.core.action.ActionListener
Expand Down Expand Up @@ -207,6 +209,22 @@ class TransportExecuteMonitorAction @Inject constructor(
return@use
}

// Block non-PPL monitors on pluggable dataformat domains
if (FeatureFlags.isEnabled(FeatureFlags.PLUGGABLE_DATAFORMAT_EXPERIMENTAL_FLAG) &&
!monitor.isPPLMonitor()
) {
actionListener.onFailure(
AlertingException.wrap(
OpenSearchStatusException(
"Only PPL monitors are supported on this domain." +
" ${monitor.monitorType} monitors are not allowed.",
RestStatus.FORBIDDEN
)
)
)
return@use
}

if (
monitor.isMonitorOfStandardType() &&
Monitor.MonitorType.valueOf(monitor.monitorType.uppercase(Locale.ROOT)) == Monitor.MonitorType.DOC_LEVEL_MONITOR
Expand Down
18 changes: 18 additions & 0 deletions alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import org.opensearch.cluster.service.ClusterService
import org.opensearch.common.inject.Inject
import org.opensearch.common.settings.Settings
import org.opensearch.common.unit.TimeValue
import org.opensearch.common.util.FeatureFlags
import org.opensearch.common.xcontent.LoggingDeprecationHandler
import org.opensearch.common.xcontent.XContentFactory
import org.opensearch.common.xcontent.XContentHelper
Expand All @@ -85,6 +86,7 @@ import org.opensearch.commons.alerting.model.remote.monitors.RemoteDocLevelMonit
import org.opensearch.commons.alerting.model.userErrorMessage
import org.opensearch.commons.alerting.util.AlertingException
import org.opensearch.commons.alerting.util.isMonitorOfStandardType
import org.opensearch.commons.alerting.util.isPPLMonitor
import org.opensearch.commons.authuser.User
import org.opensearch.commons.utils.TenantContext
import org.opensearch.commons.utils.recreateObject
Expand Down Expand Up @@ -213,6 +215,22 @@ class TransportIndexMonitorAction @Inject constructor(
return
}

// Block non-PPL monitors on pluggable dataformat domains
if (FeatureFlags.isEnabled(FeatureFlags.PLUGGABLE_DATAFORMAT_EXPERIMENTAL_FLAG) &&
!transformedRequest.monitor.isPPLMonitor()
) {
actionListener.onFailure(
AlertingException.wrap(
OpenSearchStatusException(
"Only PPL monitors are supported on this domain." +
" ${transformedRequest.monitor.monitorType} monitors are not allowed.",
RestStatus.FORBIDDEN
)
)
)
return
}
Comment on lines +219 to +232
Copy link
Copy Markdown
Collaborator

@vikhy-aws vikhy-aws May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Code repetition. Can we pull this out into a utility method?


val user = readUserFromThreadContext(client)

if (!validateUserBackendRoles(user, actionListener)) {
Expand Down
17 changes: 17 additions & 0 deletions alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexWorkflowAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import org.opensearch.alerting.workflow.CompositeWorkflowRunner
import org.opensearch.cluster.service.ClusterService
import org.opensearch.common.inject.Inject
import org.opensearch.common.settings.Settings
import org.opensearch.common.util.FeatureFlags
import org.opensearch.common.xcontent.LoggingDeprecationHandler
import org.opensearch.common.xcontent.XContentFactory.jsonBuilder
import org.opensearch.common.xcontent.XContentHelper
Expand Down Expand Up @@ -150,6 +151,22 @@ class TransportIndexWorkflowAction @Inject constructor(
return
}

// Block workflow creation on pluggable dataformat domains
if (FeatureFlags.isEnabled(
Copy link
Copy Markdown
Member

@eirsep eirsep May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we be blocking get workflow alerts, acknowledge workflow alerts, chained alerts etc.

Copy link
Copy Markdown
Member

@eirsep eirsep May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be taken up on followup pr

Copy link
Copy Markdown
Contributor Author

@yashgupte21 yashgupte21 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree since workflows can not be created on pluggable dataformat domains, those API would never have data to operate on, but adding explicit blocks for completeness makes sense and can take that as a follow up PR.

FeatureFlags.PLUGGABLE_DATAFORMAT_EXPERIMENTAL_FLAG
)
) {
actionListener.onFailure(
AlertingException.wrap(
OpenSearchStatusException(
"Workflow operations are not supported on this domain.",
RestStatus.FORBIDDEN
)
)
)
return
}

val transformedRequest = request as? IndexWorkflowRequest
?: recreateObject(request, namedWriteableRegistry) {
IndexWorkflowRequest(it)
Expand Down
Loading
Loading