From 941162ca79b5947456d80f6388788a69f1babc3a Mon Sep 17 00:00:00 2001 From: Moustafa Baiou Date: Thu, 5 Jun 2025 09:43:06 -0400 Subject: [PATCH] Alerting: Optimize prometheus api permission checks (#106299) * Alerting: Optimize prometheus api permission checks This improves the performance of the Prometheus API by performing the permission checks for rule read permission in a folder upfront, rather than checking permissions for each rule group individually. This reduces the number of permission checks and should speed up the API response time. * refactor vars --------- Co-authored-by: Konrad Lalik --- .../ngalert/api/prometheus/api_prometheus.go | 57 +++++++++---------- pkg/services/ngalert/api/testing.go | 4 ++ 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/pkg/services/ngalert/api/prometheus/api_prometheus.go b/pkg/services/ngalert/api/prometheus/api_prometheus.go index 99387809fa3..59b002d4256 100644 --- a/pkg/services/ngalert/api/prometheus/api_prometheus.go +++ b/pkg/services/ngalert/api/prometheus/api_prometheus.go @@ -34,7 +34,7 @@ type RuleStoreReader interface { } type RuleGroupAccessControlService interface { - HasAccessToRuleGroup(ctx context.Context, user identity.Requester, rules ngmodels.RulesGroup) (bool, error) + HasAccessInFolder(ctx context.Context, user identity.Requester, folder ngmodels.Namespaced) (bool, error) } type StatusReader interface { @@ -215,11 +215,10 @@ func getStatesFromQuery(v url.Values) ([]eval.State, error) { } type RuleGroupStatusesOptions struct { - Ctx context.Context - OrgID int64 - Query url.Values - Namespaces map[string]string - AuthorizeRuleGroup func(rules []*ngmodels.AlertRule) (bool, error) + Ctx context.Context + OrgID int64 + Query url.Values + AllowedNamespaces map[string]string } type ListAlertRulesStore interface { @@ -247,19 +246,26 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *contextmodel.ReqContext) respon return response.JSON(ruleResponse.HTTPStatusCode(), ruleResponse) } - namespaces := map[string]string{} + allowedNamespaces := map[string]string{} for namespaceUID, folder := range namespaceMap { - namespaces[namespaceUID] = folder.Fullpath + // only add namespaces that the user has access to rules in + hasAccess, err := srv.authz.HasAccessInFolder(c.Req.Context(), c.SignedInUser, ngmodels.Namespace(*folder.ToFolderReference())) + if err != nil { + ruleResponse.Status = "error" + ruleResponse.Error = fmt.Sprintf("failed to get namespaces visible to the user: %s", err.Error()) + ruleResponse.ErrorType = apiv1.ErrServer + return response.JSON(ruleResponse.HTTPStatusCode(), ruleResponse) + } + if hasAccess { + allowedNamespaces[namespaceUID] = folder.Fullpath + } } ruleResponse = PrepareRuleGroupStatuses(srv.log, srv.store, RuleGroupStatusesOptions{ - Ctx: c.Req.Context(), - OrgID: c.OrgID, - Query: c.Req.Form, - Namespaces: namespaces, - AuthorizeRuleGroup: func(rules []*ngmodels.AlertRule) (bool, error) { - return srv.authz.HasAccessToRuleGroup(c.Req.Context(), c.SignedInUser, rules) - }, + Ctx: c.Req.Context(), + OrgID: c.OrgID, + Query: c.Req.Form, + AllowedNamespaces: allowedNamespaces, }, RuleStatusMutatorGenerator(srv.status), RuleAlertStateMutatorGenerator(srv.manager)) return response.JSON(ruleResponse.HTTPStatusCode(), ruleResponse) @@ -411,19 +417,19 @@ func PrepareRuleGroupStatuses(log log.Logger, store ListAlertRulesStore, opts Ru labelOptions = append(labelOptions, ngmodels.WithoutInternalLabels()) } - if len(opts.Namespaces) == 0 { + if len(opts.AllowedNamespaces) == 0 { log.Debug("User does not have access to any namespaces") return ruleResponse } - namespaceUIDs := make([]string, 0, len(opts.Namespaces)) + namespaceUIDs := make([]string, 0, len(opts.AllowedNamespaces)) folderUID := opts.Query.Get("folder_uid") - _, exists := opts.Namespaces[folderUID] + _, exists := opts.AllowedNamespaces[folderUID] if folderUID != "" && exists { namespaceUIDs = append(namespaceUIDs, folderUID) } else { - for k := range opts.Namespaces { + for k := range opts.AllowedNamespaces { namespaceUIDs = append(namespaceUIDs, k) } } @@ -459,22 +465,11 @@ func PrepareRuleGroupStatuses(log log.Logger, store ListAlertRulesStore, opts Ru } } - groupedRules := getGroupedRules(log, ruleList, ruleNamesSet, opts.Namespaces) + groupedRules := getGroupedRules(log, ruleList, ruleNamesSet, opts.AllowedNamespaces) rulesTotals := make(map[string]int64, len(groupedRules)) var newToken string foundToken := false for _, rg := range groupedRules { - ok, err := opts.AuthorizeRuleGroup(rg.Rules) - if err != nil { - ruleResponse.Status = "error" - ruleResponse.Error = fmt.Sprintf("cannot authorize access to rule group: %s", err.Error()) - ruleResponse.ErrorType = apiv1.ErrServer - return ruleResponse - } - if !ok { - continue - } - if nextToken != "" && !foundToken { if !tokenGreaterThanOrEqual(getRuleGroupNextToken(rg.Folder, rg.GroupKey.RuleGroup), nextToken) { continue diff --git a/pkg/services/ngalert/api/testing.go b/pkg/services/ngalert/api/testing.go index fa81a5d64a7..f5a9e86b10c 100644 --- a/pkg/services/ngalert/api/testing.go +++ b/pkg/services/ngalert/api/testing.go @@ -129,6 +129,10 @@ var _ ac.AccessControl = &recordingAccessControlFake{} type fakeRuleAccessControlService struct { } +func (f *fakeRuleAccessControlService) HasAccessInFolder(ctx context.Context, user identity.Requester, folder models.Namespaced) (bool, error) { + return true, nil +} + func (f fakeRuleAccessControlService) HasAccessToRuleGroup(ctx context.Context, user identity.Requester, rules models.RulesGroup) (bool, error) { return true, nil }