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

@@ -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

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
 			}
 

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

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{}

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 {