From 54290f2ac44b976d813624daa68f4c8cd73509d2 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Mon, 22 Apr 2024 12:36:50 +0200 Subject: [PATCH] Alerting: Fix TestRouteGetRuleStatuses as much as possible. (#86666) This test has been skipped for a long time, so it doesn't work anymore. I've fixed the test so it works again, but left some tests disabled which were apparently flaky. If we see the other test cases flaking, we'll have to disable it again. Fixes: - Use fake access control for most test cases, and real one for FGAC test cases. - Check that "file" in API responses the full folder path, not folder title. --- pkg/services/ngalert/api/api_prometheus_test.go | 16 +++++++++------- pkg/services/ngalert/tests/fakes/rules.go | 8 +++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/services/ngalert/api/api_prometheus_test.go b/pkg/services/ngalert/api/api_prometheus_test.go index e08152d7441..86a3c328f82 100644 --- a/pkg/services/ngalert/api/api_prometheus_test.go +++ b/pkg/services/ngalert/api/api_prometheus_test.go @@ -283,7 +283,7 @@ func withLabels(labels data.Labels) forEachState { } func TestRouteGetRuleStatuses(t *testing.T) { - t.Skip() // TODO: Flaky test: https://github.com/grafana/grafana/issues/69146 + // t.Skip() // TODO: Flaky test: https://github.com/grafana/grafana/issues/69146 timeNow = func() time.Time { return time.Date(2022, 3, 10, 14, 0, 0, 0, time.UTC) } orgID := int64(1) @@ -363,7 +363,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { } } } -`, folder.Title), string(r.Body())) +`, folder.Fullpath), string(r.Body())) }) t.Run("with the inclusion of internal Labels", func(t *testing.T) { @@ -429,7 +429,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { } } } -`, folder.Title), string(r.Body())) +`, folder.Fullpath), string(r.Body())) }) t.Run("with a rule that has multiple queries", func(t *testing.T) { @@ -488,7 +488,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { } } } -`, folder.Title), string(r.Body())) +`, folder.Fullpath), string(r.Body())) }) t.Run("with many rules in a group", func(t *testing.T) { @@ -547,12 +547,11 @@ func TestRouteGetRuleStatuses(t *testing.T) { log: log.NewNopLogger(), manager: fakeAIM, store: ruleStore, - authz: &fakeRuleAccessControlService{}, + authz: accesscontrol.NewRuleService(acimpl.ProvideAccessControl(setting.NewCfg())), } c := &contextmodel.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &user.SignedInUser{OrgID: orgID, Permissions: createPermissionsForRules(rules, orgID)}} - //c.SignedInUser.Permissions[1] = createPermissionsForRules(rules) response := api.RouteGetRuleStatuses(c) require.Equal(t, http.StatusOK, response.Status()) result := &apimodels.RuleResponse{} @@ -924,6 +923,8 @@ func TestRouteGetRuleStatuses(t *testing.T) { }) t.Run("test with filters on state", func(t *testing.T) { + t.Skip() // TODO: Flaky test: https://github.com/grafana/grafana/issues/69146 + fakeStore, fakeAIM, api := setupAPI(t) // create two rules in the same Rule Group to keep assertions simple rules := ngmodels.GenerateAlertRules(3, ngmodels.AlertRuleGen(withOrgID(orgID), withGroup("Rule-Group-1"), withNamespace(&folder.Folder{ @@ -1255,12 +1256,13 @@ func TestRouteGetRuleStatuses(t *testing.T) { func setupAPI(t *testing.T) (*fakes.RuleStore, *fakeAlertInstanceManager, PrometheusSrv) { fakeStore := fakes.NewRuleStore(t) fakeAIM := NewFakeAlertInstanceManager(t) + fakeAuthz := &fakeRuleAccessControlService{} api := PrometheusSrv{ log: log.NewNopLogger(), manager: fakeAIM, store: fakeStore, - authz: accesscontrol.NewRuleService(acimpl.ProvideAccessControl(setting.NewCfg())), + authz: fakeAuthz, } return fakeStore, fakeAIM, api diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index 1714011d0bf..9f8bea0c31e 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -68,10 +68,12 @@ mainloop: } if existing == nil { metrics.MFolderIDsServiceCount.WithLabelValues(metrics.NGAlerts).Inc() + title := "TEST-FOLDER-" + util.GenerateShortUID() folders = append(folders, &folder.Folder{ - ID: rand.Int63(), // nolint:staticcheck - UID: r.NamespaceUID, - Title: "TEST-FOLDER-" + util.GenerateShortUID(), + ID: rand.Int63(), // nolint:staticcheck + UID: r.NamespaceUID, + Title: title, + Fullpath: "fullpath_" + title, }) f.Folders[r.OrgID] = folders }