From 1e380e869e8c017c7e780412938bf726ad6eae4f Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki Date: Thu, 29 Apr 2021 19:15:15 +0300 Subject: [PATCH] [Alerting]: some fixes (#33538) * Fix fialure when adding state annotations * Fix get org rules API Do not fail response if user has no access to view a namespace. Do not include the namespace in the response instead. * lint --- pkg/services/ngalert/api/api_ruler.go | 5 + pkg/services/ngalert/state/state.go | 18 +- pkg/tests/api/alerting/api_ruler_test.go | 292 +++++++++++++++++++++++ 3 files changed, 310 insertions(+), 5 deletions(-) create mode 100644 pkg/tests/api/alerting/api_ruler_test.go diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 97804c7387a..960f1ad3780 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -142,6 +142,11 @@ func (srv RulerSrv) RouteGetRulesConfig(c *models.ReqContext) response.Response for _, r := range q.Result { folder, err := srv.store.GetNamespaceByUID(r.NamespaceUID, c.SignedInUser.OrgId, c.SignedInUser) if err != nil { + if errors.Is(err, models.ErrFolderAccessDenied) { + // do not fail if used does not have access to a specific namespace + // just do not include it in the response + continue + } return toNamespaceErrorResponse(err) } namespace := folder.Title diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 1a445811499..31a045108f5 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -36,6 +36,14 @@ func resultNormal(alertState *State, result eval.Result) *State { return newState } +// addAnnotation adds an annotation to the state. +func (a *State) addAnnotation(key, value string) { + if a.Annotations == nil { + a.Annotations = make(map[string]string) + } + a.Annotations[key] = value +} + func (a *State) resultAlerting(alertRule *ngModels.AlertRule, result eval.Result) *State { switch a.State { case eval.Alerting: @@ -51,19 +59,19 @@ func (a *State) resultAlerting(alertRule *ngModels.AlertRule, result eval.Result a.State = eval.Alerting a.StartsAt = result.EvaluatedAt a.EndsAt = result.EvaluatedAt.Add(alertRule.For) - a.Annotations["alerting_at"] = result.EvaluatedAt.String() + a.addAnnotation("alerting_at", result.EvaluatedAt.String()) } default: a.StartsAt = result.EvaluatedAt if !(alertRule.For > 0) { a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second) a.State = eval.Alerting - a.Annotations["alerting_at"] = result.EvaluatedAt.String() + a.addAnnotation("alerting_at", result.EvaluatedAt.String()) } else { a.EndsAt = result.EvaluatedAt.Add(alertRule.For) if result.EvaluatedAt.Sub(a.StartsAt) > alertRule.For { a.State = eval.Alerting - a.Annotations["alerting_at"] = result.EvaluatedAt.String() + a.addAnnotation("alerting_at", result.EvaluatedAt.String()) } else { a.State = eval.Pending } @@ -82,7 +90,7 @@ func (a *State) resultError(alertRule *ngModels.AlertRule, result eval.Result) * a.EndsAt = result.EvaluatedAt.Add(alertRule.For) } if a.State != eval.Error { - a.Annotations["last_error"] = result.EvaluatedAt.String() + a.addAnnotation("last_error", result.EvaluatedAt.String()) } switch alertRule.ExecErrState { @@ -103,7 +111,7 @@ func (a *State) resultNoData(alertRule *ngModels.AlertRule, result eval.Result) a.EndsAt = result.EvaluatedAt.Add(alertRule.For) } if a.State != eval.NoData { - a.Annotations["no_data"] = result.EvaluatedAt.String() + a.addAnnotation("no_data", result.EvaluatedAt.String()) } switch alertRule.NoDataState { diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go new file mode 100644 index 00000000000..6b4750e88a8 --- /dev/null +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -0,0 +1,292 @@ +package alerting + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "testing" + "time" + + "github.com/grafana/grafana/pkg/models" + 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) { + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + EnableFeatureToggles: []string{"ngalert"}, + AnonymousUserRole: models.ROLE_EDITOR, + }) + store := testinfra.SetUpDatabase(t, dir) + grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store) + + // Create the namespace we'll save our alerts to. + require.NoError(t, createFolder(t, store, 0, "folder1")) + + // Create the namespace we'll save our alerts to. + require.NoError(t, createFolder(t, store, 0, "folder2")) + + // Create rule under folder1 + createRule(t, grafanaListedAddr, "folder1") + + // Create rule under folder2 + createRule(t, grafanaListedAddr, "folder2") + + // With the rules created, let's make sure that rule definitions are stored. + { + u := fmt.Sprintf("http://%s/api/ruler/grafana/api/v1/rules", grafanaListedAddr) + // nolint:gosec + resp, err := http.Get(u) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, resp.StatusCode, 202) + + body, _ := rulesNamespaceWithoutVariableValues(t, b) + expectedGetNamespaceResponseBody := ` + { + "folder1":[ + { + "name":"arulegroup", + "interval":"1m", + "rules":[ + { + "annotations": { + "annotation1": "val1" + }, + "expr":"", + "for": "2m", + "labels": { + "label1": "val1" + }, + "grafana_alert":{ + "id":1, + "orgId":2, + "title":"rule under folder folder1", + "condition":"A", + "data":[ + { + "refId":"A", + "queryType":"", + "relativeTimeRange":{ + "from":18000, + "to":10800 + }, + "datasourceUid":"-100", + "model":{ + "expression":"2 + 3 \u003E 1", + "intervalMs":1000, + "maxDataPoints":100, + "type":"math" + } + } + ], + "updated":"2021-02-21T01:10:30Z", + "intervalSeconds":60, + "version":1, + "uid":"uid", + "namespace_uid":"nsuid", + "namespace_id":1, + "rule_group":"arulegroup", + "no_data_state":"NoData", + "exec_err_state":"Alerting" + } + } + ] + } + ], + "folder2":[ + { + "name":"arulegroup", + "interval":"1m", + "rules":[ + { + "annotations": { + "annotation1": "val1" + }, + "expr":"", + "for": "2m", + "labels": { + "label1": "val1" + }, + "grafana_alert":{ + "id":2, + "orgId":2, + "title":"rule under folder folder2", + "condition":"A", + "data":[ + { + "refId":"A", + "queryType":"", + "relativeTimeRange":{ + "from":18000, + "to":10800 + }, + "datasourceUid":"-100", + "model":{ + "expression":"2 + 3 \u003E 1", + "intervalMs":1000, + "maxDataPoints":100, + "type":"math" + } + } + ], + "updated":"2021-02-21T01:10:30Z", + "intervalSeconds":60, + "version":1, + "uid":"uid", + "namespace_uid":"nsuid", + "namespace_id":2, + "rule_group":"arulegroup", + "no_data_state":"NoData", + "exec_err_state":"Alerting" + } + } + ] + } + ] + }` + assert.JSONEq(t, expectedGetNamespaceResponseBody, body) + + // remove permissions from folder2 + require.NoError(t, store.UpdateDashboardACL(2, nil)) + + // make sure that folder2 is not included in the response + // nolint:gosec + resp, err = http.Get(u) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err = ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, resp.StatusCode, 202) + + body, _ = rulesNamespaceWithoutVariableValues(t, b) + expectedGetNamespaceResponseBody = ` + { + "folder1":[ + { + "name":"arulegroup", + "interval":"1m", + "rules":[ + { + "annotations": { + "annotation1": "val1" + }, + "expr":"", + "for": "2m", + "labels": { + "label1": "val1" + }, + "grafana_alert":{ + "id":1, + "orgId":2, + "title":"rule under folder folder1", + "condition":"A", + "data":[ + { + "refId":"A", + "queryType":"", + "relativeTimeRange":{ + "from":18000, + "to":10800 + }, + "datasourceUid":"-100", + "model":{ + "expression":"2 + 3 \u003E 1", + "intervalMs":1000, + "maxDataPoints":100, + "type":"math" + } + } + ], + "updated":"2021-02-21T01:10:30Z", + "intervalSeconds":60, + "version":1, + "uid":"uid", + "namespace_uid":"nsuid", + "namespace_id":1, + "rule_group":"arulegroup", + "no_data_state":"NoData", + "exec_err_state":"Alerting" + } + } + ] + } + ] + }` + assert.JSONEq(t, expectedGetNamespaceResponseBody, body) + } +} + +func createRule(t *testing.T, grafanaListedAddr string, folder string) { + t.Helper() + + interval, err := model.ParseDuration("1m") + require.NoError(t, err) + + rules := apimodels.PostableRuleGroupConfig{ + Name: "arulegroup", + Interval: interval, + Rules: []apimodels.PostableExtendedRuleNode{ + { + ApiRuleNode: &apimodels.ApiRuleNode{ + For: 2 * interval, + Labels: map[string]string{"label1": "val1"}, + Annotations: map[string]string{"annotation1": "val1"}, + }, + GrafanaManagedAlert: &apimodels.PostableGrafanaRule{ + Title: fmt.Sprintf("rule under folder %s", folder), + Condition: "A", + Data: []ngmodels.AlertQuery{ + { + RefID: "A", + RelativeTimeRange: ngmodels.RelativeTimeRange{ + From: ngmodels.Duration(time.Duration(5) * time.Hour), + To: ngmodels.Duration(time.Duration(3) * time.Hour), + }, + DatasourceUID: "-100", + Model: json.RawMessage(`{ + "type": "math", + "expression": "2 + 3 > 1" + }`), + }, + }, + }, + }, + }, + } + buf := bytes.Buffer{} + enc := json.NewEncoder(&buf) + err = enc.Encode(&rules) + require.NoError(t, err) + + u := fmt.Sprintf("http://%s/api/ruler/grafana/api/v1/rules/%s", grafanaListedAddr, folder) + // nolint:gosec + resp, err := http.Post(u, "application/json", &buf) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, http.StatusAccepted, resp.StatusCode) + require.JSONEq(t, `{"message":"rule group updated successfully"}`, string(b)) +}