From e7a1ecca289a17a56d6c175a9ff073e1ae61ee1a Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Wed, 21 Feb 2024 12:30:26 +0300 Subject: [PATCH] Annotations: Improve query performance when using dashboard filter (#83112) * Annotations: Improve query performance when using dashboard filter * Add dashboard id filter --- .../accesscontrol/accesscontrol.go | 21 ++++++++++++++----- .../accesscontrol/accesscontrol_test.go | 7 +++++-- .../annotationsimpl/annotations.go | 2 +- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/services/annotations/accesscontrol/accesscontrol.go b/pkg/services/annotations/accesscontrol/accesscontrol.go index 071cf21e870..9102e62b9fa 100644 --- a/pkg/services/annotations/accesscontrol/accesscontrol.go +++ b/pkg/services/annotations/accesscontrol/accesscontrol.go @@ -6,7 +6,6 @@ import ( "github.com/grafana/grafana/pkg/infra/db" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/annotations" - "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore/permissions" @@ -40,7 +39,8 @@ func NewAuthService(db db.DB, features featuremgmt.FeatureToggles) *AuthService } // Authorize checks if the user has permission to read annotations, then returns a struct containing dashboards and scope types that the user has access to. -func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user identity.Requester) (*AccessResources, error) { +func (authz *AuthService) Authorize(ctx context.Context, orgID int64, query *annotations.ItemQuery) (*AccessResources, error) { + user := query.SignedInUser if user == nil || user.IsNil() { return nil, ErrReadForbidden.Errorf("missing user") } @@ -59,7 +59,7 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user ident var visibleDashboards map[string]int64 var err error if canAccessDashAnnotations { - visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, user, orgID) + visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, query, orgID) if err != nil { return nil, ErrAccessControlInternal.Errorf("failed to fetch dashboards: %w", err) } @@ -72,7 +72,7 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, user ident }, nil } -func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, user identity.Requester, orgID int64) (map[string]int64, error) { +func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, query *annotations.ItemQuery, orgID int64) (map[string]int64, error) { recursiveQueriesSupported, err := authz.db.RecursiveQueriesAreSupported() if err != nil { return nil, err @@ -84,10 +84,21 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, } filters := []any{ - permissions.NewAccessControlDashboardPermissionFilter(user, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported), + permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported), searchstore.OrgFilter{OrgId: orgID}, } + if query.DashboardUID != "" { + filters = append(filters, searchstore.DashboardFilter{ + UIDs: []string{query.DashboardUID}, + }) + } + if query.DashboardID != 0 { + filters = append(filters, searchstore.DashboardIDFilter{ + IDs: []int64{query.DashboardID}, + }) + } + sb := &searchstore.Builder{Dialect: authz.db.GetDialect(), Filters: filters, Features: authz.features} visibleDashboards := make(map[string]int64) diff --git a/pkg/services/annotations/accesscontrol/accesscontrol_test.go b/pkg/services/annotations/accesscontrol/accesscontrol_test.go index 12ece80dd2c..02674e42b24 100644 --- a/pkg/services/annotations/accesscontrol/accesscontrol_test.go +++ b/pkg/services/annotations/accesscontrol/accesscontrol_test.go @@ -5,15 +5,17 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/annotations/testutil" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/tests/testsuite" - "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { @@ -173,7 +175,8 @@ func TestIntegrationAuthorize(t *testing.T) { authz := NewAuthService(sql, featuremgmt.WithFeatures(tc.featureToggle)) - resources, err := authz.Authorize(context.Background(), 1, u) + query := &annotations.ItemQuery{SignedInUser: u} + resources, err := authz.Authorize(context.Background(), 1, query) require.NoError(t, err) if tc.expectedResources.Dashboards != nil { diff --git a/pkg/services/annotations/annotationsimpl/annotations.go b/pkg/services/annotations/annotationsimpl/annotations.go index 42ff9d20cf8..f9640a51e97 100644 --- a/pkg/services/annotations/annotationsimpl/annotations.go +++ b/pkg/services/annotations/annotationsimpl/annotations.go @@ -68,7 +68,7 @@ func (r *RepositoryImpl) Update(ctx context.Context, item *annotations.Item) err } func (r *RepositoryImpl) Find(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) { - resources, err := r.authZ.Authorize(ctx, query.OrgID, query.SignedInUser) + resources, err := r.authZ.Authorize(ctx, query.OrgID, query) if err != nil { return make([]*annotations.ItemDTO, 0), err }