Alerting: validate rules and calculate changes in API controller (#45072)

* Update API controller
   - add validation of rules API model
   - add function to calculate changes between the submitted alerts and existing alerts
   - update RoutePostNameRulesConfig to validate input models, calculate changes and apply in a transaction

* Update DBStore
   - delete unused storage method. All the logic is moved upstream.
   - upsert to not modify fields of new by values from the existing alert
   - if rule has UID do not try to pull it from db. (it is done upstream)

* Add rule generator
This commit is contained in:
Yuriy Tseretyan
2022-02-23 11:30:04 -05:00
committed by GitHub
parent a3a852be81
commit f75bea481d
15 changed files with 1824 additions and 275 deletions

View File

@@ -908,7 +908,7 @@ func TestAlertRuleCRUD(t *testing.T) {
Data: []ngmodels.AlertQuery{},
},
},
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: no queries or expressions are found"}`,
expectedResponse: `{"message": "invalid rule specification at index [0]: invalid alert rule: no queries or expressions are found"}`,
},
{
desc: "alert rule with empty title",
@@ -938,7 +938,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: title is empty"}`,
expectedResponse: `{"message": "invalid rule specification at index [0]: alert rule title cannot be empty"}`,
},
{
desc: "alert rule with too long name",
@@ -968,7 +968,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: name length should not be greater than 190"}`,
expectedResponse: `{"message": "invalid rule specification at index [0]: alert rule title is too long. Max length is 190"}`,
},
{
desc: "alert rule with too long rulegroup",
@@ -998,7 +998,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: rule group name length should not be greater than 190"}`,
expectedResponse: `{"message": "rule group name is too long. Max length is 190"}`,
},
{
desc: "alert rule with invalid interval",
@@ -1029,7 +1029,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to update rule group: invalid alert rule: interval (1s) should be non-zero and divided exactly by scheduler interval: 10s"}`,
expectedResponse: `{"message": "rule evaluation interval (1 second) should be positive number that is multiple of the base interval of 10 seconds"}`,
},
{
desc: "alert rule with unknown datasource",
@@ -1059,7 +1059,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to validate alert rule \"AlwaysFiring\": invalid query A: data source not found: unknown"}`,
expectedResponse: `{"message": "invalid rule specification at index [0]: failed to validate condition of alert rule AlwaysFiring: invalid query A: data source not found: unknown"}`,
},
{
desc: "alert rule with invalid condition",
@@ -1089,7 +1089,7 @@ func TestAlertRuleCRUD(t *testing.T) {
},
},
},
expectedResponse: `{"message": "failed to validate alert rule \"AlwaysFiring\": condition B not found in any query or expression: it should be one of: [A]"}`,
expectedResponse: `{"message": "invalid rule specification at index [0]: failed to validate condition of alert rule AlwaysFiring: condition B not found in any query or expression: it should be one of: [A]"}`,
},
}
@@ -1379,7 +1379,7 @@ func TestAlertRuleCRUD(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
require.JSONEq(t, `{"message": "failed to update rule group: failed to get alert rule unknown: could not find alert rule"}`, string(b))
require.JSONEq(t, `{"message": "failed to update rule group: failed to update rule with UID unknown because could not find alert rule"}`, string(b))
// let's make sure that rule definitions are not affected by the failed POST request.
u = fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
@@ -1498,7 +1498,7 @@ func TestAlertRuleCRUD(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.JSONEq(t, fmt.Sprintf(`{"message": "failed to validate alert rule \"AlwaysAlerting\": conflicting UID \"%s\" found"}`, ruleUID), string(b))
require.JSONEq(t, fmt.Sprintf(`{"message": "rule [1] has UID %s that is already assigned to another rule at index 0"}`, ruleUID), string(b))
// let's make sure that rule definitions are not affected by the failed POST request.
u = fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
@@ -1847,6 +1847,7 @@ func TestAlertRuleCRUD(t *testing.T) {
"rules":[
{
"expr":"",
"for": "30s",
"grafana_alert":{
"id":1,
"orgId":1,
@@ -2115,7 +2116,7 @@ func TestQuota(t *testing.T) {
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
assert.Equal(t, http.StatusForbidden, resp.StatusCode)
require.JSONEq(t, `{"message": "quota reached"}`, string(b))
require.JSONEq(t, `{"message": "quota has been exceeded"}`, string(b))
})
t.Run("when quota limit exceed updating existing rule should succeed", func(t *testing.T) {
@@ -2193,6 +2194,7 @@ func TestQuota(t *testing.T) {
"rules":[
{
"expr":"",
"for": "2m",
"grafana_alert":{
"id":1,
"orgId":1,

View File

@@ -10,6 +10,10 @@ import (
"testing"
"time"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/models"
@@ -17,9 +21,6 @@ import (
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/tests/testinfra"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPrometheusRules(t *testing.T) {
@@ -208,7 +209,7 @@ func TestPrometheusRules(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, 400, resp.StatusCode)
require.JSONEq(t, `{"message": "failed to update rule group: invalid alert rule: cannot have Panel ID without a Dashboard UID"}`, string(b))
require.JSONEq(t, `{"message": "invalid rule specification at index [0]: both annotations __dashboardUid__ and __panelId__ must be specified"}`, string(b))
}
// Now, let's see how this looks like.

View File

@@ -10,6 +10,10 @@ import (
"testing"
"time"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/models"
@@ -17,9 +21,6 @@ import (
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/tests/testinfra"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAlertRulePermissions(t *testing.T) {
@@ -432,7 +433,7 @@ func TestAlertRuleConflictingTitle(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
require.JSONEq(t, `{"message": "failed to update rule group: a conflicting alert rule is found: rule title under the same organisation and folder should be unique"}`, string(b))
require.JSONEq(t, `{"message": "failed to update rule group: failed to add or update rules: a conflicting alert rule is found: rule title under the same organisation and folder should be unique"}`, string(b))
})
t.Run("trying to create alert with same title under another folder should succeed", func(t *testing.T) {