public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Extend PUT rule-group route to write the entire rule group rather than top-level fields only (#53078) (#53554)

Browse Source 
* Wire up to full alert rule struct

* Extract group change detection logic to dedicated file

* GroupDiff -> GroupDelta for consistency

* Calculate deltas and handle backwards compatible requests

* Separate changes and insert/update/delete as needed

* Regenerate files

* Don't touch the DB if there are no changes

* Quota checking, delete unused file

* Mark modified records as provisioned

* Validation + a couple API layer tests

* Address linter errors

* Fix issue with UID assignment and rule creation

* Propagate top level group fields to all rules

* Tests for repeated updates and versioning

* Tests for quota and provenance checks

* Fix linter errors

* Regenerate

* Factor out some shared logic

* Drop unnecessary multiple nilchecks

* Use alternative strategy for rolling UIDs on inserted rules

* Fix tests, add back nilcheck, refresh UIDs during test

* Address feedback

* Add missing nil-check

(cherry picked from commit b198559225)

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>
This commit is contained in:
Grot (@grafanabot)
2022-08-10 17:32:54 -05:00
committed by GitHub
co-authored by Alexander Weaver
parent 57e60ffed6
commit aa321e74f6
12 changed files with 373 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/api/api_provisioning.go
+8 -3
View File
@@ -56,7 +56,7 @@ type AlertRuleService interface {
UpdateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error)
DeleteAlertRule(ctx context.Context, orgID int64, ruleUID string, provenance alerting_models.Provenance) error
GetRuleGroup(ctx context.Context, orgID int64, folder, group string) (definitions.AlertRuleGroup, error)
UpdateRuleGroup(ctx context.Context, orgID int64, folderUID, rulegroup string, interval int64) error
ReplaceRuleGroup(ctx context.Context, orgID int64, group definitions.AlertRuleGroup, userID int64, provenance alerting_models.Provenance) error
}
func (srv *ProvisioningSrv) RouteGetPolicyTree(c *models.ReqContext) response.Response {
@@ -312,8 +312,13 @@ func (srv *ProvisioningSrv) RouteGetAlertRuleGroup(c *models.ReqContext, folder
return response.JSON(http.StatusOK, g)
}
func (srv *ProvisioningSrv) RoutePutAlertRuleGroup(c *models.ReqContext, ag definitions.AlertRuleGroupMetadata, folderUID string, group string) response.Response {
err := srv.alertRules.UpdateRuleGroup(c.Req.Context(), c.OrgId, folderUID, group, ag.Interval)
func (srv *ProvisioningSrv) RoutePutAlertRuleGroup(c *models.ReqContext, ag definitions.AlertRuleGroup, folderUID string, group string) response.Response {
ag.FolderUID = folderUID
ag.Title = group
err := srv.alertRules.ReplaceRuleGroup(c.Req.Context(), c.OrgId, ag, c.UserId, alerting_models.ProvenanceAPI)
if errors.Is(err, alerting_models.ErrAlertRuleFailedValidation) {
return ErrResp(http.StatusBadRequest, err, "")
}
if err != nil {
if errors.Is(err, store.ErrOptimisticLock) {
return ErrResp(http.StatusConflict, err, "")
pkg/services/ngalert/api/api_provisioning_test.go
+39
View File
@@ -298,6 +298,37 @@ func TestProvisioningApi(t *testing.T) {
require.Equal(t, 404, response.Status())
})
t.Run("are invalid at group level", func(t *testing.T) {
t.Run("PUT returns 400", func(t *testing.T) {
sut := createProvisioningSrvSut(t)
rc := createTestRequestCtx()
insertRule(t, sut, createTestAlertRule("rule", 1))
group := createInvalidAlertRuleGroup()
group.Interval = 0
response := sut.RoutePutAlertRuleGroup(&rc, group, "folder-uid", group.Title)
require.Equal(t, 400, response.Status())
require.NotEmpty(t, response.Body())
require.Contains(t, string(response.Body()), "invalid alert rule")
})
})
t.Run("are invalid at rule level", func(t *testing.T) {
t.Run("PUT returns 400", func(t *testing.T) {
sut := createProvisioningSrvSut(t)
rc := createTestRequestCtx()
insertRule(t, sut, createTestAlertRule("rule", 1))
group := createInvalidAlertRuleGroup()
response := sut.RoutePutAlertRuleGroup(&rc, group, "folder-uid", group.Title)
require.Equal(t, 400, response.Status())
require.NotEmpty(t, response.Body())
require.Contains(t, string(response.Body()), "invalid alert rule")
})
})
})
}
@@ -476,6 +507,14 @@ func createInvalidAlertRule() definitions.ProvisionedAlertRule {
return definitions.ProvisionedAlertRule{}
}
func createInvalidAlertRuleGroup() definitions.AlertRuleGroup {
return definitions.AlertRuleGroup{
Title: "invalid",
Interval: 10,
Rules: []models.AlertRule{{}},
}
}
func createTestAlertRule(title string, orgID int64) definitions.ProvisionedAlertRule {
return definitions.ProvisionedAlertRule{
OrgID: orgID,
pkg/services/ngalert/api/generated_base_api_provisioning.go
+1 -1
View File
@@ -135,7 +135,7 @@ func (f *ProvisioningApiHandler) RoutePutAlertRuleGroup(ctx *models.ReqContext)
folderUIDParam := web.Params(ctx.Req)[":FolderUID"]
groupParam := web.Params(ctx.Req)[":Group"]
// Parse Request Body
conf := apimodels.AlertRuleGroupMetadata{}
conf := apimodels.AlertRuleGroup{}
if err := web.Bind(ctx.Req, &conf); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
pkg/services/ngalert/api/provisioning.go
+1 -1
View File
@@ -100,6 +100,6 @@ func (f *ProvisioningApiHandler) handleRouteGetAlertRuleGroup(ctx *models.ReqCon
return f.svc.RouteGetAlertRuleGroup(ctx, folder, group)
}
func (f *ProvisioningApiHandler) handleRoutePutAlertRuleGroup(ctx *models.ReqContext, ag apimodels.AlertRuleGroupMetadata, folder, group string) response.Response {
func (f *ProvisioningApiHandler) handleRoutePutAlertRuleGroup(ctx *models.ReqContext, ag apimodels.AlertRuleGroup, folder, group string) response.Response {
return f.svc.RoutePutAlertRuleGroup(ctx, ag, folder, group)
}
pkg/services/ngalert/api/tooling/api.json
+3 -5
View File
@@ -3092,7 +3092,6 @@
"type": "object"
},
"gettableAlerts": {
"description": "GettableAlerts gettable alerts",
"items": {
"$ref": "#/definitions/gettableAlert"
},
@@ -3258,7 +3257,6 @@
"type": "array"
},
"postableSilence": {
"description": "PostableSilence postable silence",
"properties": {
"comment": {
"description": "comment",
@@ -3707,15 +3705,15 @@
"in": "body",
"name": "Body",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
}
],
"responses": {
"200": {
"description": "AlertRuleGroupMetadata",
"description": "AlertRuleGroup",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
},
"400": {
pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go
+2 -2
View File
@@ -151,7 +151,7 @@ func NewAlertRule(rule models.AlertRule, provenance models.Provenance) Provision
// - application/json
//
// Responses:
// 200: AlertRuleGroupMetadata
// 200: AlertRuleGroup
// 400: ValidationError
// swagger:parameters RouteGetAlertRuleGroup RoutePutAlertRuleGroup
@@ -169,7 +169,7 @@ type RuleGroupPathParam struct {
// swagger:parameters RoutePutAlertRuleGroup
type AlertRuleGroupPayload struct {
// in:body
Body AlertRuleGroupMetadata
Body AlertRuleGroup
}
// swagger:model
pkg/services/ngalert/api/tooling/post.json
+4 -4
View File
@@ -2904,6 +2904,7 @@
"type": "object"
},
"alertGroup": {
"description": "AlertGroup alert group",
"properties": {
"alerts": {
"description": "alerts",
@@ -3144,7 +3145,6 @@
"type": "object"
},
"gettableSilences": {
"description": "GettableSilences gettable silences",
"items": {
"$ref": "#/definitions/gettableSilence"
},
@@ -5329,15 +5329,15 @@
"in": "body",
"name": "Body",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
}
],
"responses": {
"200": {
"description": "AlertRuleGroupMetadata",
"description": "AlertRuleGroup",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
},
"400": {
pkg/services/ngalert/api/tooling/spec.json
+3 -5
View File
@@ -1961,15 +1961,15 @@
"name": "Body",
"in": "body",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
}
],
"responses": {
"200": {
"description": "AlertRuleGroupMetadata",
"description": "AlertRuleGroup",
"schema": {
"$ref": "#/definitions/AlertRuleGroupMetadata"
"$ref": "#/definitions/AlertRuleGroup"
}
},
"400": {
@@ -5480,7 +5480,6 @@
"$ref": "#/definitions/gettableAlert"
},
"gettableAlerts": {
"description": "GettableAlerts gettable alerts",
"type": "array",
"items": {
"$ref": "#/definitions/gettableAlert"
@@ -5648,7 +5647,6 @@
}
},
"postableSilence": {
"description": "PostableSilence postable silence",
"type": "object",
"required": [
"comment",