From 5eaf6509c09ea9e9e44cb631787cb245edd98d93 Mon Sep 17 00:00:00 2001 From: Selene Date: Wed, 2 Mar 2022 11:04:29 +0100 Subject: [PATCH] Dashboard Extractor: Don't fail when using default OSS implementation (#46024) * Don't fail when using default OSS implementation * Check correct error --- pkg/api/datasources.go | 5 +- pkg/services/alerting/extractor.go | 7 +- pkg/services/alerting/extractor_test.go | 65 +++++++++++++++++++ .../permissions/datasource_permissions.go | 5 +- .../datasource_permissions_mocks.go | 5 +- 5 files changed, 79 insertions(+), 8 deletions(-) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 9af7d1c247b..0476e9172e3 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -9,11 +9,12 @@ import ( "sort" "strconv" + "github.com/grafana/grafana/pkg/services/datasources/permissions" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/api/datasource" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins/adapters" @@ -577,7 +578,7 @@ func (hs *HTTPServer) filterDatasourcesByQueryPermission(ctx context.Context, us query.Result = datasources if err := hs.DatasourcePermissionsService.FilterDatasourcesBasedOnQueryPermissions(ctx, &query); err != nil { - if !errors.Is(err, bus.ErrHandlerNotFound) { + if !errors.Is(err, permissions.ErrNotImplemented) { return nil, err } return datasources, nil diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index f8af2b8f858..3238b5b60ac 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -212,9 +212,10 @@ func (e *DashAlertExtractorService) getAlertFromPanels(ctx context.Context, json } if err := e.datasourcePermissionsService.FilterDatasourcesBasedOnQueryPermissions(ctx, &dsFilterQuery); err != nil { - return nil, err - } - if len(dsFilterQuery.Result) == 0 { + if !errors.Is(err, permissions.ErrNotImplemented) { + return nil, err + } + } else if len(dsFilterQuery.Result) == 0 { return nil, models.ErrDataSourceAccessDenied } diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 6b8ea7861a6..90c7499d9e3 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/pkg/services/alerting/extractor_test.go @@ -2,6 +2,7 @@ package alerting import ( "context" + "errors" "io/ioutil" "testing" "time" @@ -11,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/datasources/permissions" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -295,6 +297,69 @@ func TestAlertRuleExtraction(t *testing.T) { }) } +func TestFilterPermissionsErrors(t *testing.T) { + RegisterCondition("query", func(model *simplejson.Json, index int) (Condition, error) { + return &FakeCondition{}, nil + }) + + // mock data + defaultDs := &models.DataSource{Id: 12, OrgId: 1, Name: "I am default", IsDefault: true, Uid: "def-uid"} + + json, err := ioutil.ReadFile("./testdata/graphite-alert.json") + require.Nil(t, err) + dashJSON, err := simplejson.NewJson(json) + require.Nil(t, err) + + dsPermissions := permissions.NewMockDatasourcePermissionService() + dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs} + extractor := ProvideDashAlertExtractorService(dsPermissions, dsService) + + tc := []struct { + name string + result []*models.DataSource + err error + expectedErr error + }{ + { + "Data sources are filtered and return results don't return an error", + []*models.DataSource{defaultDs}, + nil, + nil, + }, + { + "Data sources are filtered but return empty results should return error", + nil, + nil, + models.ErrDataSourceAccessDenied, + }, + { + "Using default OSS implementation doesn't return an error", + nil, + permissions.ErrNotImplemented, + nil, + }, + { + "Returning an error different from ErrNotImplemented should fails", + nil, + errors.New("random error"), + errors.New("random error"), + }, + } + + for _, test := range tc { + t.Run(test.name, func(t *testing.T) { + dsPermissions.DsResult = test.result + dsPermissions.ErrResult = test.err + _, err = extractor.GetAlerts(WithUAEnabled(context.Background(), true), DashAlertInfo{ + User: nil, + Dash: models.NewDashboardFromJson(dashJSON), + OrgID: 1, + }) + assert.Equal(t, err, test.expectedErr) + }) + } +} + type fakeDatasourceService struct { ExpectedDatasource *models.DataSource datasources.DataSourceService diff --git a/pkg/services/datasources/permissions/datasource_permissions.go b/pkg/services/datasources/permissions/datasource_permissions.go index 9dee1b2d0b5..906ef83713d 100644 --- a/pkg/services/datasources/permissions/datasource_permissions.go +++ b/pkg/services/datasources/permissions/datasource_permissions.go @@ -2,17 +2,20 @@ package permissions import ( "context" + "errors" "github.com/grafana/grafana/pkg/models" ) +var ErrNotImplemented = errors.New("not implemented") + type DatasourcePermissionsService interface { FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *models.DatasourcesPermissionFilterQuery) error } // dummy method func (hs *OSSDatasourcePermissionsService) FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *models.DatasourcesPermissionFilterQuery) error { - return nil + return ErrNotImplemented } type OSSDatasourcePermissionsService struct{} diff --git a/pkg/services/datasources/permissions/datasource_permissions_mocks.go b/pkg/services/datasources/permissions/datasource_permissions_mocks.go index 767fdda5e5d..69896c8398f 100644 --- a/pkg/services/datasources/permissions/datasource_permissions_mocks.go +++ b/pkg/services/datasources/permissions/datasource_permissions_mocks.go @@ -7,12 +7,13 @@ import ( ) type mockDatasourcePermissionService struct { - DsResult []*models.DataSource + DsResult []*models.DataSource + ErrResult error } func (m *mockDatasourcePermissionService) FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *models.DatasourcesPermissionFilterQuery) error { cmd.Result = m.DsResult - return nil + return m.ErrResult } func NewMockDatasourcePermissionService() *mockDatasourcePermissionService {