diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 016b6cf83ae..73fd132ca5a 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -225,8 +225,15 @@ func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *contextmodel.ReqContext, nam result := apimodels.NamespaceConfigResponse{} + ruleList := make([]*ngmodels.AlertRule, 0) + for _, ruleGroup := range ruleGroups { + ruleList = append(ruleList, ruleGroup...) + } + + userUIDmapping := srv.getUserUIDmapping(c.Req.Context(), ruleList) + for groupKey, rules := range ruleGroups { - result[namespace.Fullpath] = append(result[namespace.Fullpath], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords, srv.resolveUserIdToNameFn(c.Req.Context()))) + result[namespace.Fullpath] = append(result[namespace.Fullpath], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords, userUIDmapping)) } return response.JSON(http.StatusAccepted, result) @@ -263,9 +270,10 @@ func (srv RulerSrv) RouteGetRulesGroupConfig(c *contextmodel.ReqContext, namespa return ErrResp(http.StatusInternalServerError, err, "failed to get group alert rules") } + userUIDmapping := srv.getUserUIDmapping(c.Req.Context(), rules) result := apimodels.RuleGroupConfigResponse{ // nolint:staticcheck - GettableRuleGroupConfig: toGettableRuleGroupConfig(finalRuleGroup, rules, provenanceRecords, srv.resolveUserIdToNameFn(c.Req.Context())), + GettableRuleGroupConfig: toGettableRuleGroupConfig(finalRuleGroup, rules, provenanceRecords, userUIDmapping), } return response.JSON(http.StatusAccepted, result) @@ -285,9 +293,10 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res return ErrResp(http.StatusInternalServerError, err, "failed to get deleted rules") } result := apimodels.NamespaceConfigResponse{} + userUIDmapping := srv.getUserUIDmapping(c.Req.Context(), rules) if len(rules) > 0 { result[""] = []apimodels.GettableRuleGroupConfig{ - toGettableRuleGroupConfig("", rules, map[string]ngmodels.Provenance{}, srv.resolveUserIdToNameFn(c.Req.Context())), + toGettableRuleGroupConfig("", rules, map[string]ngmodels.Provenance{}, userUIDmapping), } } return response.JSON(http.StatusOK, result) @@ -332,6 +341,13 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res return ErrResp(http.StatusInternalServerError, err, "failed to get alert rules") } + ruleList := make([]*ngmodels.AlertRule, 0) + for _, ruleGroup := range configs { + ruleList = append(ruleList, ruleGroup...) + } + + userUIDmapping := srv.getUserUIDmapping(c.Req.Context(), ruleList) + for groupKey, rules := range configs { folder, ok := namespaceMap[groupKey.NamespaceUID] if !ok { @@ -340,7 +356,7 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res srv.log.Error("Namespace not visible to the user", "user", id, "userNamespace", userNamespace, "namespace", groupKey.NamespaceUID) continue } - result[folder.Fullpath] = append(result[folder.Fullpath], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords, srv.resolveUserIdToNameFn(c.Req.Context()))) + result[folder.Fullpath] = append(result[folder.Fullpath], toGettableRuleGroupConfig(groupKey.RuleGroup, rules, provenanceRecords, userUIDmapping)) } return response.JSON(http.StatusOK, result) } @@ -363,7 +379,8 @@ func (srv RulerSrv) RouteGetRuleByUID(c *contextmodel.ReqContext, ruleUID string return response.ErrOrFallback(http.StatusInternalServerError, "failed to get rule provenance", err) } - result := toGettableExtendedRuleNode(rule, map[string]ngmodels.Provenance{rule.ResourceID(): provenance}, srv.resolveUserIdToNameFn(ctx)) + userUIDmapping := srv.getUserUIDmapping(ctx, []*ngmodels.AlertRule{&rule}) + result := toGettableExtendedRuleNode(rule, map[string]ngmodels.Provenance{rule.ResourceID(): provenance}, userUIDmapping) return response.JSON(http.StatusOK, result) } @@ -385,9 +402,10 @@ func (srv RulerSrv) RouteGetRuleVersionsByUID(c *contextmodel.ReqContext, ruleUI } sort.Slice(rules, func(i, j int) bool { return rules[i].ID > rules[j].ID }) result := make(apimodels.GettableRuleVersions, 0, len(rules)) + userUIDmapping := srv.getUserUIDmapping(ctx, rules) for _, rule := range rules { // do not provide provenance status because we do not have historical changes for it - result = append(result, toGettableExtendedRuleNode(*rule, map[string]ngmodels.Provenance{}, srv.resolveUserIdToNameFn(ctx))) + result = append(result, toGettableExtendedRuleNode(*rule, map[string]ngmodels.Provenance{}, userUIDmapping)) } return response.JSON(http.StatusOK, result) } @@ -616,7 +634,7 @@ func changesToResponse(finalChanges *store.GroupDelta) response.Response { return response.JSON(http.StatusAccepted, body) } -func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, provenanceRecords map[string]ngmodels.Provenance, userIdToName userIDToUserInfoFn) apimodels.GettableRuleGroupConfig { +func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, provenanceRecords map[string]ngmodels.Provenance, userUIDmapping map[ngmodels.UserUID]*apimodels.UserInfo) apimodels.GettableRuleGroupConfig { rules.SortByGroupIndex() ruleNodes := make([]apimodels.GettableExtendedRuleNode, 0, len(rules)) var interval time.Duration @@ -624,7 +642,7 @@ func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, prov interval = time.Duration(rules[0].IntervalSeconds) * time.Second } for _, r := range rules { - ruleNodes = append(ruleNodes, toGettableExtendedRuleNode(*r, provenanceRecords, userIdToName)) + ruleNodes = append(ruleNodes, toGettableExtendedRuleNode(*r, provenanceRecords, userUIDmapping)) } return apimodels.GettableRuleGroupConfig{ Name: groupName, @@ -633,7 +651,7 @@ func toGettableRuleGroupConfig(groupName string, rules ngmodels.RulesGroup, prov } } -func toGettableExtendedRuleNode(r ngmodels.AlertRule, provenanceRecords map[string]ngmodels.Provenance, userIdToName userIDToUserInfoFn) apimodels.GettableExtendedRuleNode { +func toGettableExtendedRuleNode(r ngmodels.AlertRule, provenanceRecords map[string]ngmodels.Provenance, userUIDmapping map[ngmodels.UserUID]*apimodels.UserInfo) apimodels.GettableExtendedRuleNode { provenance := ngmodels.ProvenanceNone if prov, exists := provenanceRecords[r.ResourceID()]; exists { provenance = prov @@ -645,7 +663,7 @@ func toGettableExtendedRuleNode(r ngmodels.AlertRule, provenanceRecords map[stri Condition: r.Condition, Data: ApiAlertQueriesFromAlertQueries(r.Data), Updated: r.Updated, - UpdatedBy: userIdToName(r.UpdatedBy), + UpdatedBy: getUserFromMapping(userUIDmapping, r.UpdatedBy), IntervalSeconds: r.IntervalSeconds, Version: r.Version, UID: r.UID, @@ -811,11 +829,9 @@ func (srv RulerSrv) searchAuthorizedAlertRules(ctx context.Context, q authorized return byGroupKey, totalGroups, nil } -type userIDToUserInfoFn func(id *ngmodels.UserUID) *apimodels.UserInfo - -// getIdentityName returns name of either user or service account -func (srv RulerSrv) resolveUserIdToNameFn(ctx context.Context) userIDToUserInfoFn { - cache := map[ngmodels.UserUID]*apimodels.UserInfo{ +// getUserUIDmaping returns a UserUID->UserInfo mapping from the UpdatedBy users in the RulesGroup +func (srv RulerSrv) getUserUIDmapping(ctx context.Context, rules []*ngmodels.AlertRule) map[ngmodels.UserUID]*apimodels.UserInfo { + mapping := map[ngmodels.UserUID]*apimodels.UserInfo{ ngmodels.AlertingUserUID: { UID: string(ngmodels.AlertingUserUID), }, @@ -823,30 +839,56 @@ func (srv RulerSrv) resolveUserIdToNameFn(ctx context.Context) userIDToUserInfoF UID: string(ngmodels.FileProvisioningUserUID), }, } - return func(id *ngmodels.UserUID) *apimodels.UserInfo { - if id == nil { - return nil + userUIDs := []string{} + for _, rule := range rules { + if rule == nil { + continue } - if val, ok := cache[*id]; ok { - return val + + if rule.UpdatedBy == nil { + continue } - u, err := srv.userService.GetByUID(ctx, &user.GetUserByUIDQuery{ - UID: string(*id), - }) - var name string - if err != nil { - srv.log.FromContext(ctx).Warn("Failed to get user by uid. Defaulting to an empty name", "uid", id, "error", err) + + if _, ok := mapping[*rule.UpdatedBy]; ok { + // one of the system identifiers + continue } - if u != nil { - name = u.NameOrFallback() - } - result := &apimodels.UserInfo{ - UID: string(*id), - Name: name, - } - cache[*id] = result - return result + + userUIDs = append(userUIDs, string(*rule.UpdatedBy)) } + + if len(userUIDs) == 0 { + return mapping + } + + users, err := srv.userService.ListByIdOrUID(ctx, userUIDs, []int64{}) + if err != nil { + srv.log.FromContext(ctx).Warn("Failed to list users by uid. Defaulting to empty names", "uids", userUIDs, "error", err) + return mapping + } + + for _, user := range users { + mapping[ngmodels.UserUID(user.UID)] = &apimodels.UserInfo{ + UID: user.UID, + Name: user.NameOrFallback(), + } + } + + return mapping +} + +func getUserFromMapping(mapping map[ngmodels.UserUID]*apimodels.UserInfo, userUID *ngmodels.UserUID) *apimodels.UserInfo { + if userUID == nil { + return nil + } + + u, ok := mapping[*userUID] + if ok { + return u + } + + // if user is not found or we get an error building the mapping, return empty name by default + return &apimodels.UserInfo{UID: string(*userUID)} } func getPanelIDFromQuery(v url.Values) (int64, error) { diff --git a/pkg/services/ngalert/api/api_ruler_export_test.go b/pkg/services/ngalert/api/api_ruler_export_test.go index f976bcf0e4e..9d66158522f 100644 --- a/pkg/services/ngalert/api/api_ruler_export_test.go +++ b/pkg/services/ngalert/api/api_ruler_export_test.go @@ -40,7 +40,7 @@ func TestExportFromPayload(t *testing.T) { ruleStore := fakes.NewRuleStore(t) ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder) - srv := createService(ruleStore) + srv := createService(ruleStore, nil) requestFile := "post-rulegroup-101.json" rawBody, err := testData.ReadFile(path.Join("test-data", requestFile)) @@ -253,7 +253,7 @@ func TestExportRules(t *testing.T) { // overwrite the folders visible to user because PutRule automatically creates folders in the fake store. ruleStore.Folders[orgID] = []*folder2.Folder{f1, f2} - srv := createService(ruleStore) + srv := createService(ruleStore, nil) allRules := make([]*ngmodels.AlertRule, 0, len(hasAccess1)+len(hasAccess2)+len(noAccess1)) allRules = append(allRules, hasAccess1...) diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index d1df25af570..4b3c87910ab 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -87,7 +87,7 @@ func TestRouteDeleteAlertRules(t *testing.T) { request := createRequestContextWithPerms(orgID, map[int64]map[string][]string{}, nil) - response := createService(ruleStore).RouteDeleteAlertRules(request, folder.UID, "") + response := createService(ruleStore, nil).RouteDeleteAlertRules(request, folder.UID, "") require.Equalf(t, http.StatusForbidden, response.Status(), "Expected 403 but got %d: %v", response.Status(), string(response.Body())) require.Empty(t, getRecordedCommand(ruleStore)) @@ -145,7 +145,7 @@ func TestRouteDeleteAlertRules(t *testing.T) { ruleStore := initFakeRuleStore(t) requestCtx := createRequestContext(orgID, nil) - response := createService(ruleStore).RouteDeleteAlertRules(requestCtx, folder.UID, "") + response := createService(ruleStore, nil).RouteDeleteAlertRules(requestCtx, folder.UID, "") require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body())) require.Empty(t, getRecordedCommand(ruleStore)) @@ -165,7 +165,7 @@ func TestRouteDeleteAlertRules(t *testing.T) { permissions := createPermissionsForRules(authorizedRulesInGroup, orgID) requestCtx := createRequestContextWithPerms(orgID, permissions, nil) - response := createService(ruleStore).RouteDeleteAlertRules(requestCtx, folder.UID, authorizedRulesInGroup[0].RuleGroup) + response := createService(ruleStore, nil).RouteDeleteAlertRules(requestCtx, folder.UID, authorizedRulesInGroup[0].RuleGroup) require.Equalf(t, http.StatusForbidden, response.Status(), "Expected 403 but got %d: %v", response.Status(), string(response.Body())) deleteCommands := getRecordedCommand(ruleStore) @@ -217,9 +217,24 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { permissions := createPermissionsForRules(queryAccessRules, orgID) req := createRequestContextWithPerms(orgID, permissions, nil) - response := createService(ruleStore).RouteGetNamespaceRulesConfig(req, folder.UID) + fakeUserService := usertest.NewUserServiceFake() + userUids := make([]string, 0) + for _, rule := range allRules { + if rule.UpdatedBy != nil { + userUids = append(userUids, string(*rule.UpdatedBy)) + } + } + fakeUserServiceResponse := []*user.User{} + for i, uid := range userUids { + fakeUserServiceResponse = append(fakeUserServiceResponse, &user.User{ID: int64(i + 1), UID: uid}) + } + fakeUserService.ExpectedListUsersByIdOrUid = fakeUserServiceResponse + + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetNamespaceRulesConfig(req, folder.UID) require.Equal(t, http.StatusAccepted, response.Status()) + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) // only one call to the user service result := &apimodels.NamespaceConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) @@ -249,7 +264,20 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { expectedRules := gen.With(gen.WithOrgID(orgID), gen.WithNamespace(folder.ToFolderReference())).GenerateManyRef(2, 6) ruleStore.PutRule(context.Background(), expectedRules...) - svc := createService(ruleStore) + fakeUserService := usertest.NewUserServiceFake() + userUids := make([]string, 0) + for _, rule := range expectedRules { + if rule.UpdatedBy != nil { + userUids = append(userUids, string(*rule.UpdatedBy)) + } + } + fakeUserServiceResponse := []*user.User{} + for i, uid := range userUids { + fakeUserServiceResponse = append(fakeUserServiceResponse, &user.User{ID: int64(i + 1), UID: uid}) + } + fakeUserService.ExpectedListUsersByIdOrUid = fakeUserServiceResponse + + svc := createService(ruleStore, fakeUserService) // add provenance to the first generated rule rule := &models.AlertRule{ @@ -263,6 +291,11 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { response := svc.RouteGetNamespaceRulesConfig(req, folder.UID) require.Equal(t, http.StatusAccepted, response.Status()) + if len(userUids) > 0 { + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) + } else { + require.Equal(t, 0, len(fakeUserService.ListUsersByIdOrUidCalls)) + } result := &apimodels.NamespaceConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) @@ -295,9 +328,25 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { perms := createPermissionsForRules(expectedRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetNamespaceRulesConfig(req, folder.UID) + + fakeUserService := usertest.NewUserServiceFake() + userUids := make([]string, 0) + for _, rule := range expectedRules { + if rule.UpdatedBy != nil { + userUids = append(userUids, string(*rule.UpdatedBy)) + } + } + fakeUserServiceResponse := []*user.User{} + for i, uid := range userUids { + fakeUserServiceResponse = append(fakeUserServiceResponse, &user.User{ID: int64(i + 1), UID: uid}) + } + fakeUserService.ExpectedListUsersByIdOrUid = fakeUserServiceResponse + + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetNamespaceRulesConfig(req, folder.UID) require.Equal(t, http.StatusAccepted, response.Status()) + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) // only one call to the user service result := &apimodels.NamespaceConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) @@ -349,7 +398,9 @@ func TestRouteGetRuleByUID(t *testing.T) { req := createRequestContextWithPerms(orgID, perms, nil) expectedRule := createdRules[1] - response := createService(ruleStore).RouteGetRuleByUID(req, expectedRule.UID) + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetRuleByUID(req, expectedRule.UID) require.Equal(t, http.StatusOK, response.Status()) result := &apimodels.GettableExtendedRuleNode{} @@ -369,6 +420,7 @@ func TestRouteGetRuleByUID(t *testing.T) { UpdatedBy *models.UserUID User *user.User UserServiceError error + UserServiceCalls []usertest.ListUsersByIdOrUidCall Expected *apimodels.UserInfo }{ { @@ -376,6 +428,7 @@ func TestRouteGetRuleByUID(t *testing.T) { UpdatedBy: nil, User: nil, UserServiceError: nil, + UserServiceCalls: nil, Expected: nil, }, { @@ -383,6 +436,7 @@ func TestRouteGetRuleByUID(t *testing.T) { UpdatedBy: util.Pointer(models.UserUID("test-uid")), User: nil, UserServiceError: nil, + UserServiceCalls: []usertest.ListUsersByIdOrUidCall{{Uids: []string{"test-uid"}, Ids: []int64{}}}, Expected: &apimodels.UserInfo{ UID: "test-uid", }, @@ -391,6 +445,7 @@ func TestRouteGetRuleByUID(t *testing.T) { desc: "just UID if error", UpdatedBy: util.Pointer(models.UserUID("test-uid")), UserServiceError: errors.New("error"), + UserServiceCalls: []usertest.ListUsersByIdOrUidCall{{Uids: []string{"test-uid"}, Ids: []int64{}}}, Expected: &apimodels.UserInfo{ UID: "test-uid", }, @@ -399,9 +454,11 @@ func TestRouteGetRuleByUID(t *testing.T) { desc: "login if it's known user", UpdatedBy: util.Pointer(models.UserUID("test-uid")), User: &user.User{ + UID: "test-uid", Login: "Test", }, UserServiceError: nil, + UserServiceCalls: []usertest.ListUsersByIdOrUidCall{{Uids: []string{"test-uid"}, Ids: []int64{}}}, Expected: &apimodels.UserInfo{ UID: "test-uid", Name: "Test", @@ -412,6 +469,7 @@ func TestRouteGetRuleByUID(t *testing.T) { UpdatedBy: &models.AlertingUserUID, User: nil, UserServiceError: nil, + UserServiceCalls: nil, Expected: &apimodels.UserInfo{ UID: string(models.AlertingUserUID), }, @@ -421,6 +479,7 @@ func TestRouteGetRuleByUID(t *testing.T) { UpdatedBy: &models.FileProvisioningUserUID, User: nil, UserServiceError: nil, + UserServiceCalls: nil, Expected: &apimodels.UserInfo{ UID: string(models.FileProvisioningUserUID), }, @@ -429,11 +488,12 @@ func TestRouteGetRuleByUID(t *testing.T) { for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { expectedRule.UpdatedBy = tc.UpdatedBy - svc := createService(ruleStore) usvc := usertest.NewUserServiceFake() - usvc.ExpectedUser = tc.User + if tc.User != nil { + usvc.ExpectedListUsersByIdOrUid = []*user.User{tc.User} + } usvc.ExpectedError = tc.UserServiceError - svc.userService = usvc + svc := createService(ruleStore, usvc) response := svc.RouteGetRuleByUID(req, expectedRule.UID) @@ -441,6 +501,7 @@ func TestRouteGetRuleByUID(t *testing.T) { result := &apimodels.GettableExtendedRuleNode{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) + require.Equal(t, tc.UserServiceCalls, usvc.ListUsersByIdOrUidCalls) require.Equal(t, tc.Expected, result.GrafanaManagedAlert.UpdatedBy) }) @@ -463,13 +524,16 @@ func TestRouteGetRuleByUID(t *testing.T) { perms := createPermissionsForRules(createdRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRuleByUID(req, "foobar") + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetRuleByUID(req, "foobar") require.Equal(t, http.StatusNotFound, response.Status()) + require.Equal(t, 0, len(fakeUserService.ListUsersByIdOrUidCalls)) }) } -func TestRouteGetRuleHistoryByUID(t *testing.T) { +func TestRouteGetRuleVersionsByUID(t *testing.T) { orgID := rand.Int63() f := randFolder() groupKey := models.GenerateGroupKey(orgID) @@ -494,7 +558,7 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) { perms := createPermissionsForRules([]*models.AlertRule{rule}, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - svc := createService(ruleStore) + svc := createService(ruleStore, nil) response := svc.RouteGetRuleVersionsByUID(req, rule.UID) require.Equal(t, http.StatusOK, response.Status()) @@ -525,7 +589,7 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) { perms := createPermissionsForRules(history, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRuleVersionsByUID(req, ruleKey.UID) + response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID) require.Equal(t, http.StatusNotFound, response.Status()) }) @@ -544,7 +608,7 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) { perms := createPermissionsForRules([]*models.AlertRule{rule}, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRuleVersionsByUID(req, ruleKey.UID) + response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID) require.Equal(t, http.StatusOK, response.Status()) @@ -569,7 +633,7 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) { perms := createPermissionsForRules(history, orgID) // grant permissions to all records in history but not the rule itself req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRuleVersionsByUID(req, ruleKey.UID) + response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID) require.Equal(t, http.StatusForbidden, response.Status()) }) @@ -598,9 +662,13 @@ func TestRouteGetRulesConfig(t *testing.T) { permissions := createPermissionsForRules(append(group1, group2[1:]...), orgID) request := createRequestContextWithPerms(orgID, permissions, nil) - response := createService(ruleStore).RouteGetRulesConfig(request) + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetRulesConfig(request) require.Equal(t, http.StatusOK, response.Status()) + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) // only one call to the user service + result := &apimodels.NamespaceConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) @@ -629,12 +697,15 @@ func TestRouteGetRulesConfig(t *testing.T) { perms := createPermissionsForRules(expectedRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRulesConfig(req) + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetRulesConfig(req) require.Equal(t, http.StatusOK, response.Status()) result := &apimodels.NamespaceConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) // only one call to the user service models.RulesGroup(expectedRules).SortByGroupIndex() @@ -676,12 +747,15 @@ func TestRouteGetRulesGroupConfig(t *testing.T) { perms := createPermissionsForRules(expectedRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRulesGroupConfig(req, folder.UID, groupKey.RuleGroup) + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + response := svc.RouteGetRulesGroupConfig(req, folder.UID, groupKey.RuleGroup) require.Equal(t, http.StatusAccepted, response.Status()) result := &apimodels.RuleGroupConfigResponse{} require.NoError(t, json.Unmarshal(response.Body(), result)) require.NotNil(t, result) + require.Equal(t, 1, len(fakeUserService.ListUsersByIdOrUidCalls)) // only one call to the user service models.RulesGroup(expectedRules).SortByGroupIndex() @@ -714,9 +788,13 @@ func TestRouteGetRulesGroupConfig(t *testing.T) { perms := createPermissionsForRules(expectedRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) - response := createService(ruleStore).RouteGetRulesGroupConfig(req, folder.UID, "non-existent-rule-group") + fakeUserService := usertest.NewUserServiceFake() + svc := createService(ruleStore, fakeUserService) + + response := svc.RouteGetRulesGroupConfig(req, folder.UID, "non-existent-rule-group") require.Equal(t, http.StatusNotFound, response.Status()) + require.Equal(t, 0, len(fakeUserService.ListUsersByIdOrUidCalls)) }) } @@ -853,12 +931,16 @@ func TestValidateQueries(t *testing.T) { } func createServiceWithProvenanceStore(store *fakes.RuleStore, provenanceStore provisioning.ProvisioningStore) *RulerSrv { - svc := createService(store) + svc := createService(store, nil) svc.provenanceStore = provenanceStore return svc } -func createService(store *fakes.RuleStore) *RulerSrv { +func createService(store *fakes.RuleStore, _userService *usertest.FakeUserService) *RulerSrv { + userService := _userService + if _userService == nil { + userService = usertest.NewUserServiceFake() + } return &RulerSrv{ xactManager: store, store: store, @@ -872,7 +954,7 @@ func createService(store *fakes.RuleStore) *RulerSrv { amConfigStore: &fakeAMRefresher{}, amRefresher: &fakeAMRefresher{}, featureManager: featuremgmt.WithFeatures(featuremgmt.FlagGrafanaManagedRecordingRules), - userService: usertest.NewUserServiceFake(), + userService: userService, } }