CNV-80608: management: add update alert rule APIs #1047
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,223 @@ | ||||||
| package managementrouter | ||||||
|
|
||||||
| import ( | ||||||
| "encoding/json" | ||||||
| "errors" | ||||||
| "io" | ||||||
| "net/http" | ||||||
| "strings" | ||||||
|
|
||||||
| monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" | ||||||
|
|
||||||
| "github.com/openshift/monitoring-plugin/pkg/management" | ||||||
| ) | ||||||
|
|
||||||
| func (hr *httpRouter) BulkUpdateAlertRules(w http.ResponseWriter, req *http.Request) { | ||||||
| req.Body = http.MaxBytesReader(w, req.Body, maxRequestBodyBytes) | ||||||
|
|
||||||
| body, err := io.ReadAll(req.Body) | ||||||
| if err != nil { | ||||||
| writeError(w, http.StatusRequestEntityTooLarge, "request body too large") | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // BulkUpdateAlertRulesRequest.Classification is typed as | ||||||
| // *AlertRuleClassificationPatch (via x-go-type in the spec), so the | ||||||
| // three-state omitted/null/string semantics are preserved on decode. | ||||||
| var payload BulkUpdateAlertRulesRequest | ||||||
| if err := json.Unmarshal(body, &payload); err != nil { | ||||||
| writeError(w, http.StatusBadRequest, "invalid request body") | ||||||
|
Contributor
There was a problem hiding this comment. can we include the error in the response as well so that rather than trying to guess at what went wrong the caller can directly make an adjustment |
||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if len(payload.RuleIds) == 0 { | ||||||
| writeError(w, http.StatusBadRequest, "ruleIds is required") | ||||||
| return | ||||||
| } | ||||||
| if len(payload.RuleIds) > maxBulkUpdateRuleIds { | ||||||
| writeError(w, http.StatusBadRequest, "ruleIds exceeds maximum of 100") | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if payload.AlertingRuleEnabled == nil && payload.Labels == nil && payload.Classification == nil { | ||||||
| writeError(w, http.StatusBadRequest, "alertingRuleEnabled (toggle drop/restore) or labels (set/unset) or classification is required") | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| return | ||||||
| } | ||||||
|
|
||||||
| var haveToggle bool | ||||||
| var enabled bool | ||||||
| if payload.AlertingRuleEnabled != nil { | ||||||
| enabled = *payload.AlertingRuleEnabled | ||||||
| haveToggle = true | ||||||
| } | ||||||
|
|
||||||
| results := make([]UpdateAlertRuleResult, 0, len(payload.RuleIds)) | ||||||
|
|
||||||
| for _, rawId := range payload.RuleIds { | ||||||
|
Contributor
There was a problem hiding this comment. If we are looping through up to 100 alert rules I think rather than repeatedly trying to perform an update only to get a permission failure it would be best to either cache the permission failures or perform the permission checks ahead of time |
||||||
| id := strings.TrimSpace(rawId) | ||||||
| if id == "" { | ||||||
| msg := "missing ruleId" | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: rawId, | ||||||
|
Contributor
There was a problem hiding this comment. Won't this just be a whitespace only string? |
||||||
| StatusCode: int32(http.StatusBadRequest), | ||||||
| Message: &msg, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| notAllowedEnabled := false | ||||||
| if haveToggle { | ||||||
| var derr error | ||||||
|
Contributor
There was a problem hiding this comment. Other checks use err, I don't see any reason to have this one have a different name |
||||||
| if !enabled { | ||||||
| derr = hr.managementClient.DropPlatformAlertRule(req.Context(), id) | ||||||
| } else { | ||||||
| derr = hr.managementClient.RestorePlatformAlertRule(req.Context(), id) | ||||||
|
Contributor
There was a problem hiding this comment. Is it not possible to drop/restore user ar's? For example if an ar is gitops or operator managed? |
||||||
| } | ||||||
| if derr != nil { | ||||||
| var na *management.NotAllowedError | ||||||
| if errors.As(derr, &na) { | ||||||
| notAllowedEnabled = true | ||||||
| } else { | ||||||
| status, message := parseError(derr) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if payload.Classification != nil { | ||||||
| cl := payload.Classification | ||||||
| update := management.UpdateRuleClassificationRequest{RuleId: id} | ||||||
| if cl.ComponentSet { | ||||||
| update.Component = cl.Component | ||||||
| update.ComponentSet = true | ||||||
| } | ||||||
| if cl.LayerSet { | ||||||
| update.Layer = cl.Layer | ||||||
| update.LayerSet = true | ||||||
| } | ||||||
| if cl.ComponentFromSet { | ||||||
| update.ComponentFrom = cl.ComponentFrom | ||||||
| update.ComponentFromSet = true | ||||||
| } | ||||||
| if cl.LayerFromSet { | ||||||
| update.LayerFrom = cl.LayerFrom | ||||||
| update.LayerFromSet = true | ||||||
| } | ||||||
|
|
||||||
| if update.ComponentSet || update.LayerSet || update.ComponentFromSet || update.LayerFromSet { | ||||||
| if err := hr.managementClient.UpdateAlertRuleClassification(req.Context(), update); err != nil { | ||||||
|
Contributor
There was a problem hiding this comment. I think looking more at the code it would be best to have the label update code and the drop/restore code also flow through a single function similar to |
||||||
| status, message := parseError(err) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if payload.Labels != nil { | ||||||
| currentRule, err := hr.managementClient.GetRuleById(req.Context(), id) | ||||||
| if err != nil { | ||||||
| status, message := parseError(err) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| // platformLabels uses "" to signal "drop this label"; the management | ||||||
| // layer's UpdatePlatformAlertRule interprets "" as a delete directive. | ||||||
| // userLabels is the fully-merged map for user-defined rules where we | ||||||
| // simply omit deleted keys rather than set them to "". | ||||||
| platformLabels := make(map[string]string) | ||||||
| userLabels := make(map[string]string) | ||||||
| for k, v := range currentRule.Labels { | ||||||
| userLabels[k] = v | ||||||
| } | ||||||
| for k, pv := range *payload.Labels { | ||||||
| if pv == nil || *pv == "" { | ||||||
| platformLabels[k] = "" | ||||||
| delete(userLabels, k) | ||||||
| } else { | ||||||
| platformLabels[k] = *pv | ||||||
| userLabels[k] = *pv | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| updatedPlatformRule := monitoringv1.Rule{Labels: platformLabels} | ||||||
|
|
||||||
| err = hr.managementClient.UpdatePlatformAlertRule(req.Context(), id, updatedPlatformRule) | ||||||
| if err != nil { | ||||||
| var ve *management.ValidationError | ||||||
| var nf *management.NotFoundError | ||||||
| if errors.As(err, &ve) || errors.As(err, &nf) { | ||||||
| status, message := parseError(err) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| var na *management.NotAllowedError | ||||||
| if errors.As(err, &na) { | ||||||
| updatedUserRule := currentRule | ||||||
| updatedUserRule.Labels = userLabels | ||||||
|
|
||||||
| newRuleId, err := hr.managementClient.UpdateUserDefinedAlertRule(req.Context(), id, updatedUserRule) | ||||||
|
Contributor
There was a problem hiding this comment. Shouldn't we determine if an alert rule is a platform or user defined ar before just trying platform and then falling back to user on a not allowed error? |
||||||
| if err != nil { | ||||||
| status, message := parseError(err) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: newRuleId, | ||||||
| StatusCode: int32(http.StatusNoContent), | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| status, message := parseError(err) | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(status), | ||||||
| Message: &message, | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if notAllowedEnabled && payload.Labels == nil && payload.Classification == nil { | ||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(http.StatusMethodNotAllowed), | ||||||
| }) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| results = append(results, UpdateAlertRuleResult{ | ||||||
| Id: id, | ||||||
| StatusCode: int32(http.StatusNoContent), | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| w.Header().Set("Content-Type", "application/json") | ||||||
| w.WriteHeader(http.StatusOK) | ||||||
| if err := json.NewEncoder(w).Encode(BulkUpdateAlertRulesResponse{Rules: results}); err != nil { | ||||||
| log.WithError(err).Warn("failed to encode bulk update response") | ||||||
| } | ||||||
| } | ||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is accurate to state that dropping an alert silences it, they are two separate actions