From 9aa6ce2a506643816c92a5ec690c9b988af0919f Mon Sep 17 00:00:00 2001 From: Michael Mandrus <41969079+mmandrus@users.noreply.github.com> Date: Wed, 13 Jul 2022 09:27:03 -0400 Subject: [PATCH] Datasource: Propagate datasource secret decryption errors to the frontend (#52068) * update decrypt secrets function signature and add secrets error handling * remove a couple instances of unnecessary logging since errors are properly handled now * add unit test * fix linting issues --- pkg/api/datasources.go | 10 ++-- pkg/api/metrics.go | 9 +++- pkg/api/metrics_test.go | 48 +++++++++++++++++++ pkg/expr/transform.go | 10 ++-- pkg/plugins/adapters/adapters.go | 13 +++-- pkg/plugins/plugincontext/plugincontext.go | 10 ++-- .../fakes/fake_datasource_service.go | 8 +++- pkg/services/query/query.go | 12 ++--- 8 files changed, 84 insertions(+), 36 deletions(-) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index d2491fb863f..ef075b4b200 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -626,13 +626,9 @@ func (hs *HTTPServer) checkDatasourceHealth(c *models.ReqContext, ds *datasource return response.JSON(http.StatusOK, payload) } -func (hs *HTTPServer) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) map[string]string { - return func(ds *datasources.DataSource) map[string]string { - decryptedJsonData, err := hs.DataSourcesService.DecryptedValues(ctx, ds) - if err != nil { - hs.log.Error("Failed to decrypt secure json data", "error", err) - } - return decryptedJsonData +func (hs *HTTPServer) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { + return func(ds *datasources.DataSource) (map[string]string, error) { + return hs.DataSourcesService.DecryptedValues(ctx, ds) } } diff --git a/pkg/api/metrics.go b/pkg/api/metrics.go index 69bc2b8eb78..7e611bc85c2 100644 --- a/pkg/api/metrics.go +++ b/pkg/api/metrics.go @@ -2,6 +2,7 @@ package api import ( "errors" + "fmt" "net/http" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -24,7 +25,13 @@ func (hs *HTTPServer) handleQueryMetricsError(err error) *response.NormalRespons if errors.Is(err, datasources.ErrDataSourceNotFound) { return response.Error(http.StatusNotFound, "Data source not found", err) } - var badQuery *query.ErrBadQuery + + var secretsPlugin datasources.ErrDatasourceSecretsPluginUserFriendly + if errors.As(err, &secretsPlugin) { + return response.Error(http.StatusInternalServerError, fmt.Sprint("Secrets Plugin error: ", err.Error()), err) + } + + var badQuery query.ErrBadQuery if errors.As(err, &badQuery) { return response.Error(http.StatusBadRequest, util.Capitalize(badQuery.Message), err) } diff --git a/pkg/api/metrics_test.go b/pkg/api/metrics_test.go index fedffcc8414..60783307019 100644 --- a/pkg/api/metrics_test.go +++ b/pkg/api/metrics_test.go @@ -1,7 +1,9 @@ package api import ( + "bytes" "context" + "encoding/json" "fmt" "net/http" "strings" @@ -40,6 +42,11 @@ type fakePluginRequestValidator struct { err error } +type secretsErrorResponseBody struct { + Error string `json:"error"` + Message string `json:"message"` +} + func (rv *fakePluginRequestValidator) Validate(dsURL string, req *http.Request) error { return rv.err } @@ -104,3 +111,44 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { require.Equal(t, http.StatusMultiStatus, resp.StatusCode) }) } + +func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) { + qds := query.ProvideService( + nil, + nil, + nil, + &fakePluginRequestValidator{}, + &fakeDatasources.FakeDataSourceService{SimulatePluginFailure: true}, + &fakePluginClient{ + QueryDataHandlerFunc: func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { + resp := backend.Responses{ + "A": backend.DataResponse{ + Error: fmt.Errorf("query failed"), + }, + } + return &backend.QueryDataResponse{Responses: resp}, nil + }, + }, + &fakeOAuthTokenService{}, + ) + httpServer := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.queryDataService = qds + }) + + t.Run("Status code is 500 and a secrets plugin error is returned if there is a problem getting secrets from the remote plugin", func(t *testing.T) { + req := httpServer.NewPostRequest("/api/ds/query", strings.NewReader(queryDatasourceInput)) + webtest.RequestWithSignedInUser(req, &models.SignedInUser{UserId: 1, OrgId: 1, OrgRole: models.ROLE_VIEWER}) + resp, err := httpServer.SendJSON(req) + require.NoError(t, err) + require.Equal(t, http.StatusInternalServerError, resp.StatusCode) + buf := new(bytes.Buffer) + _, err = buf.ReadFrom(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + var resObj secretsErrorResponseBody + err = json.Unmarshal(buf.Bytes(), &resObj) + require.NoError(t, err) + require.Equal(t, "unknown error", resObj.Error) + require.Contains(t, resObj.Message, "Secrets Plugin error:") + }) +} diff --git a/pkg/expr/transform.go b/pkg/expr/transform.go index 51cd60ce055..b3ae0de2532 100644 --- a/pkg/expr/transform.go +++ b/pkg/expr/transform.go @@ -127,12 +127,8 @@ func hiddenRefIDs(queries []Query) (map[string]struct{}, error) { return hidden, nil } -func (s *Service) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) map[string]string { - return func(ds *datasources.DataSource) map[string]string { - decryptedJsonData, err := s.dataSourceService.DecryptedValues(ctx, ds) - if err != nil { - logger.Error("Failed to decrypt secure json data", "error", err) - } - return decryptedJsonData +func (s *Service) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { + return func(ds *datasources.DataSource) (map[string]string, error) { + return s.dataSourceService.DecryptedValues(ctx, ds) } } diff --git a/pkg/plugins/adapters/adapters.go b/pkg/plugins/adapters/adapters.go index 9f46067dadd..77b45c9c841 100644 --- a/pkg/plugins/adapters/adapters.go +++ b/pkg/plugins/adapters/adapters.go @@ -3,6 +3,7 @@ package adapters import ( "encoding/json" + "fmt" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -11,16 +12,20 @@ import ( ) // ModelToInstanceSettings converts a datasources.DataSource to a backend.DataSourceInstanceSettings. -func ModelToInstanceSettings(ds *datasources.DataSource, decryptFn func(ds *datasources.DataSource) map[string]string, +func ModelToInstanceSettings(ds *datasources.DataSource, decryptFn func(ds *datasources.DataSource) (map[string]string, error), ) (*backend.DataSourceInstanceSettings, error) { var jsonDataBytes json.RawMessage if ds.JsonData != nil { var err error jsonDataBytes, err = ds.JsonData.MarshalJSON() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to convert data source to instance settings: %w", err) } } + decrypted, err := decryptFn(ds) + if err != nil { + return nil, err + } return &backend.DataSourceInstanceSettings{ ID: ds.Id, @@ -32,9 +37,9 @@ func ModelToInstanceSettings(ds *datasources.DataSource, decryptFn func(ds *data BasicAuthEnabled: ds.BasicAuth, BasicAuthUser: ds.BasicAuthUser, JSONData: jsonDataBytes, - DecryptedSecureJSONData: decryptFn(ds), + DecryptedSecureJSONData: decrypted, Updated: ds.Updated, - }, nil + }, err } // BackendUserFromSignedInUser converts Grafana's SignedInUser model diff --git a/pkg/plugins/plugincontext/plugincontext.go b/pkg/plugins/plugincontext/plugincontext.go index 6ae57c650df..9e09a477236 100644 --- a/pkg/plugins/plugincontext/plugincontext.go +++ b/pkg/plugins/plugincontext/plugincontext.go @@ -127,12 +127,8 @@ func (p *Provider) getCachedPluginSettings(ctx context.Context, pluginID string, return ps, nil } -func (p *Provider) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) map[string]string { - return func(ds *datasources.DataSource) map[string]string { - decryptedJsonData, err := p.dataSourceService.DecryptedValues(ctx, ds) - if err != nil { - p.logger.Error("Failed to decrypt secure json data", "error", err) - } - return decryptedJsonData +func (p *Provider) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { + return func(ds *datasources.DataSource) (map[string]string, error) { + return p.dataSourceService.DecryptedValues(ctx, ds) } } diff --git a/pkg/services/datasources/fakes/fake_datasource_service.go b/pkg/services/datasources/fakes/fake_datasource_service.go index 415dce53d98..4263e313fba 100644 --- a/pkg/services/datasources/fakes/fake_datasource_service.go +++ b/pkg/services/datasources/fakes/fake_datasource_service.go @@ -11,8 +11,9 @@ import ( ) type FakeDataSourceService struct { - lastId int64 - DataSources []*datasources.DataSource + lastId int64 + DataSources []*datasources.DataSource + SimulatePluginFailure bool } var _ datasources.DataSourceService = &FakeDataSourceService{} @@ -112,6 +113,9 @@ func (s *FakeDataSourceService) GetHTTPTransport(ctx context.Context, ds *dataso } func (s *FakeDataSourceService) DecryptedValues(ctx context.Context, ds *datasources.DataSource) (map[string]string, error) { + if s.SimulatePluginFailure { + return nil, datasources.ErrDatasourceSecretsPluginUserFriendly{Err: "unknown error"} + } values := make(map[string]string) return values, nil } diff --git a/pkg/services/query/query.go b/pkg/services/query/query.go index 4f7a478c476..2ddb7dec1b9 100644 --- a/pkg/services/query/query.go +++ b/pkg/services/query/query.go @@ -151,7 +151,7 @@ func (s *Service) handleQueryData(ctx context.Context, user *models.SignedInUser instanceSettings, err := adapters.ModelToInstanceSettings(ds, s.decryptSecureJsonDataFn(ctx)) if err != nil { - return nil, fmt.Errorf("failed to convert data source to instance settings: %w", err) + return nil, err } req := &backend.QueryDataRequest{ @@ -343,12 +343,8 @@ func (s *Service) getDataSourceFromQuery(ctx context.Context, user *models.Signe return nil, NewErrBadQuery("missing data source ID/UID") } -func (s *Service) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) map[string]string { - return func(ds *datasources.DataSource) map[string]string { - decryptedJsonData, err := s.dataSourceService.DecryptedValues(ctx, ds) - if err != nil { - s.log.Error("Failed to decrypt secure json data", "error", err) - } - return decryptedJsonData +func (s *Service) decryptSecureJsonDataFn(ctx context.Context) func(ds *datasources.DataSource) (map[string]string, error) { + return func(ds *datasources.DataSource) (map[string]string, error) { + return s.dataSourceService.DecryptedValues(ctx, ds) } }