CNV-80608: management: add delete alert rule API by sradco · Pull Request #942 · openshift/monitoring-plugin

sradco · 2026-05-14T07:16:56Z

Add DELETE /api/v1/alerting/rules endpoint
for bulk deletion of user-defined alert
rules with full test coverage.

Also removes .github/workflows/unit-tests.yaml
as the OpenShift org runs all tests via Prow.

Signed-off-by: Shirly Radco sradco@redhat.com
Signed-off-by: João Vilaça jvilaca@redhat.com
Signed-off-by: Aviv Litman alitman@redhat.com
Co-authored-by: AI Assistant noreply@cursor.com

openshift-ci-robot · 2026-05-14T07:17:01Z

@sradco: This pull request references CNV-80608 which is a valid jira issue.

Details

In response to this:

Add DELETE /api/v1/alerting/rules endpoint
for bulk deletion of user-defined alert
rules with full test coverage.

Also removes .github/workflows/unit-tests.yaml
as the OpenShift org runs all tests via Prow.

Signed-off-by: Shirly Radco sradco@redhat.com
Signed-off-by: João Vilaça jvilaca@redhat.com
Signed-off-by: Aviv Litman alitman@redhat.com
Co-authored-by: AI Assistant noreply@cursor.com

Made with Cursor

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

sradco · 2026-05-18T08:34:57Z

Hi @jgbernalp , @jan--f , @simonpasquier, I would appreciate your review of this PR

jgbernalp · 2026-05-19T10:15:45Z

+      type: object
+      required:
+        - id
+        - status_code


This casing seems inconsistent, should we stick with camelCase?

yes I agree to change to statusCode

jgbernalp · 2026-05-19T10:45:52Z

+      properties:
+        ruleIds:
+          type: array
+          minItems: 1


should we set a max here?, it seems a large number of ids would execute sequential calls to the k8s api.

jgbernalp · 2026-05-19T10:48:29Z

+	"github.com/openshift/monitoring-plugin/pkg/managementlabels"
+)
+
+func (c *client) DeleteUserDefinedAlertRuleById(ctx context.Context, alertRuleId string) error {


It seems this function is also deleting platform managed rules, is the name correct or the logic needs adjustment?

jgbernalp · 2026-05-19T10:55:19Z

+		} else if len(newRules) != len(group.Rules) {
+			updated = true
+		}


is this needed, it seems the filterRulesById changes the updated value already. Can we simplify this logic to make it more readable?

jgbernalp · 2026-05-19T10:59:22Z

+	results := make([]DeleteAlertRuleResult, 0, len(payload.RuleIds))
+
+	for _, rawId := range payload.RuleIds {
+		id, err := parseParam(rawId, "ruleId")


IIUC, this is parsed from a JSON input. why do we need to decode the rawId?

sradco · 2026-05-20T11:13:43Z

Thank you @jgbernalp and @avlitman , I updated the PR based on your comments

avlitman · 2026-05-20T15:02:49Z

/lgtm

openshift-ci · 2026-05-20T15:03:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avlitman, sradco
Once this PR has been reviewed and has the lgtm label, please assign kyoto for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

simonpasquier · 2026-05-20T15:36:45Z

+	"github.com/openshift/monitoring-plugin/test/e2e/framework"
+)
+
+func TestDeleteAlertRule(t *testing.T) {


Are the tests executed somewhere?

There is no existing Prow job for this yet, the Go e2e tests are currently only runnable manually, and we need a new step in openshift/release.

@simonpasquier updated the code and created a PR openshift/release#79673

Add DELETE /api/v1/alerting/rules endpoint for bulk deletion of user-defined alert rules with full test coverage. Also removes .github/workflows/unit-tests.yaml as the OpenShift org runs all tests via Prow. Fix e2e tests to skip gracefully when KUBECONFIG or PLUGIN_URL env vars are not set, instead of failing. Introduce framework.ErrSkip sentinel so callers can distinguish missing-env from real errors. Remove hardcoded PLUGIN_URL from the test-e2e Makefile target so it can be supplied by CI or the developer. Signed-off-by: Shirly Radco <sradco@redhat.com> Signed-off-by: João Vilaça <jvilaca@redhat.com> Signed-off-by: Aviv Litman <alitman@redhat.com> Co-authored-by: AI Assistant <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>

openshift-ci · 2026-05-25T13:10:25Z

New changes are detected. LGTM label has been removed.

openshift-ci · 2026-05-25T16:06:58Z

@sradco: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 14, 2026

openshift-ci Bot requested review from PeterYurkovich and jgbernalp May 14, 2026 07:17

sradco mentioned this pull request May 14, 2026

CNV-80608: PR03 add delete alert rule API sradco/monitoring-plugin#17

Closed

jgbernalp reviewed May 19, 2026

View reviewed changes

avlitman reviewed May 19, 2026

View reviewed changes

Comment thread api/openapi.yaml Outdated

jgbernalp reviewed May 19, 2026

View reviewed changes

sradco force-pushed the alert-mgmt-restructured-03-delete-rule branch from 8c8f530 to c6bf5a5 Compare May 20, 2026 11:00

avlitman approved these changes May 20, 2026

View reviewed changes

openshift-ci Bot assigned avlitman May 20, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 20, 2026

simonpasquier reviewed May 20, 2026

View reviewed changes

sradco force-pushed the alert-mgmt-restructured-03-delete-rule branch from c6bf5a5 to 9ffd2eb Compare May 25, 2026 13:10

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 25, 2026

sradco mentioned this pull request May 25, 2026

monitoring-plugin: add e2e-management-api Prow job openshift/release#79673

Open

Conversation

sradco commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 14, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sradco commented May 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sradco commented May 20, 2026

Uh oh!

avlitman commented May 20, 2026

Uh oh!

openshift-ci Bot commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sradco May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented May 25, 2026

Uh oh!

openshift-ci Bot commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sradco commented May 14, 2026 •

edited

Loading

openshift-ci-robot commented May 14, 2026 •

edited by openshift-ci Bot

Loading

sradco May 25, 2026 •

edited

Loading