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

alerting: batch user service calls (#103403)

Browse Source 
* update alerting endpoints to batch requests to the user service via ListByIdOrUID
This commit is contained in:
Will Assis
2025-04-07 07:33:15 -04:00
committed by GitHub
parent a05b35a13c
commit 9bc4fbe900
3 changed files with 184 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/api/api_ruler_test.go
+105 -23
View File
@@ -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,
}
}