diff --git a/pkg/services/publicdashboards/api/api.go b/pkg/services/publicdashboards/api/api.go index 848fe025610..6f7247ba695 100644 --- a/pkg/services/publicdashboards/api/api.go +++ b/pkg/services/publicdashboards/api/api.go @@ -1,6 +1,7 @@ package api import ( + "context" "errors" "net/http" "strconv" @@ -88,7 +89,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response { ) if err != nil { - return api.handleError(http.StatusInternalServerError, "failed to get public dashboard", err) + return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboard: failed to get public dashboard", err) } meta := dtos.DashboardMeta{ @@ -118,7 +119,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response { func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response { resp, err := api.PublicDashboardService.ListPublicDashboards(c.Req.Context(), c.OrgID) if err != nil { - return api.handleError(http.StatusInternalServerError, "failed to list public dashboards", err) + return api.handleError(c.Req.Context(), http.StatusInternalServerError, "ListPublicDashboards: failed to list public dashboards", err) } return response.JSON(http.StatusOK, resp) } @@ -128,7 +129,7 @@ func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response { func (api *Api) GetPublicDashboardConfig(c *models.ReqContext) response.Response { pdc, err := api.PublicDashboardService.GetPublicDashboardConfig(c.Req.Context(), c.OrgID, web.Params(c.Req)[":uid"]) if err != nil { - return api.handleError(http.StatusInternalServerError, "failed to get public dashboard config", err) + return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboardConfig: failed to get public dashboard config", err) } return response.JSON(http.StatusOK, pdc) } @@ -139,12 +140,12 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons // exit if we don't have a valid dashboardUid dashboardUid := web.Params(c.Req)[":uid"] if dashboardUid == "" || !util.IsValidShortUID(dashboardUid) { - api.handleError(http.StatusBadRequest, "no dashboardUid", dashboards.ErrDashboardIdentifierNotSet) + api.handleError(c.Req.Context(), http.StatusBadRequest, "SavePublicDashboardConfig: no dashboardUid", dashboards.ErrDashboardIdentifierNotSet) } pubdash := &PublicDashboard{} if err := web.Bind(c.Req, pubdash); err != nil { - return response.Error(http.StatusBadRequest, "bad request data", err) + return response.Error(http.StatusBadRequest, "SavePublicDashboardConfig: bad request data", err) } // Always set the orgID and userID from the session @@ -159,7 +160,7 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons // Save the public dashboard pubdash, err := api.PublicDashboardService.SavePublicDashboardConfig(c.Req.Context(), c.SignedInUser, &dto) if err != nil { - return api.handleError(http.StatusInternalServerError, "failed to save public dashboard configuration", err) + return api.handleError(c.Req.Context(), http.StatusInternalServerError, "SavePublicDashboardConfig: failed to save public dashboard configuration", err) } return response.JSON(http.StatusOK, pubdash) @@ -170,17 +171,17 @@ func (api *Api) SavePublicDashboardConfig(c *models.ReqContext) response.Respons func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response { panelId, err := strconv.ParseInt(web.Params(c.Req)[":panelId"], 10, 64) if err != nil { - return response.Error(http.StatusBadRequest, "invalid panel ID", err) + return response.Error(http.StatusBadRequest, "QueryPublicDashboard: invalid panel ID", err) } reqDTO := PublicDashboardQueryDTO{} if err = web.Bind(c.Req, &reqDTO); err != nil { - return response.Error(http.StatusBadRequest, "bad request data", err) + return response.Error(http.StatusBadRequest, "QueryPublicDashboard: bad request data", err) } resp, err := api.PublicDashboardService.GetQueryDataResponse(c.Req.Context(), c.SkipCache, reqDTO, panelId, web.Params(c.Req)[":accessToken"]) if err != nil { - return api.handleError(http.StatusInternalServerError, "error running public dashboard panel queries", err) + return api.handleError(c.Req.Context(), http.StatusInternalServerError, "QueryPublicDashboard: error running public dashboard panel queries", err) } return toJsonStreamingResponse(api.Features, resp) @@ -189,10 +190,10 @@ func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response { // util to help us unpack dashboard and publicdashboard errors or use default http code and message // we should look to do some future refactoring of these errors as publicdashboard err is the same as a dashboarderr, just defined in a // different package. -func (api *Api) handleError(code int, message string, err error) response.Response { +func (api *Api) handleError(ctx context.Context, code int, message string, err error) response.Response { var publicDashboardErr PublicDashboardErr - - api.Log.Error(message, "error", err.Error()) + ctxLogger := api.Log.FromContext(ctx) + ctxLogger.Error(message, "error", err.Error()) // handle public dashboard error if ok := errors.As(err, &publicDashboardErr); ok { diff --git a/pkg/services/publicdashboards/database/database.go b/pkg/services/publicdashboards/database/database.go index 8abfad412f7..4f467fe4e41 100644 --- a/pkg/services/publicdashboards/database/database.go +++ b/pkg/services/publicdashboards/database/database.go @@ -100,7 +100,6 @@ func (d *PublicDashboardStoreImpl) GetPublicDashboard(ctx context.Context, acces // find dashboard dashRes, err := d.GetDashboard(ctx, pdRes.DashboardUid) - if err != nil { return nil, nil, err } @@ -272,7 +271,6 @@ func (d *PublicDashboardStoreImpl) PublicDashboardEnabled(ctx context.Context, d } hasPublicDashboard = result > 0 - return err }) @@ -292,7 +290,6 @@ func (d *PublicDashboardStoreImpl) AccessTokenExists(ctx context.Context, access } hasPublicDashboard = result > 0 - return err }) diff --git a/pkg/services/publicdashboards/public_dashboard_service_mock.go b/pkg/services/publicdashboards/public_dashboard_service_mock.go index 8be2bef0a1d..c7429829950 100644 --- a/pkg/services/publicdashboards/public_dashboard_service_mock.go +++ b/pkg/services/publicdashboards/public_dashboard_service_mock.go @@ -47,7 +47,7 @@ func (_m *FakePublicDashboardService) AccessTokenExists(ctx context.Context, acc } // BuildAnonymousUser provides a mock function with given fields: ctx, dashboard -func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error) { +func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser { ret := _m.Called(ctx, dashboard) var r0 *user.SignedInUser @@ -59,14 +59,7 @@ func (_m *FakePublicDashboardService) BuildAnonymousUser(ctx context.Context, da } } - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *models.Dashboard) error); ok { - r1 = rf(ctx, dashboard) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // GetDashboard provides a mock function with given fields: ctx, dashboardUid diff --git a/pkg/services/publicdashboards/public_dashboard_store_mock.go b/pkg/services/publicdashboards/public_dashboard_store_mock.go index 4cb15bef0fd..d8b71e38851 100644 --- a/pkg/services/publicdashboards/public_dashboard_store_mock.go +++ b/pkg/services/publicdashboards/public_dashboard_store_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.14.0. DO NOT EDIT. +// Code generated by mockery v2.12.1. DO NOT EDIT. package publicdashboards @@ -9,6 +9,8 @@ import ( mock "github.com/stretchr/testify/mock" publicdashboardsmodels "github.com/grafana/grafana/pkg/services/publicdashboards/models" + + testing "testing" ) // FakePublicDashboardStore is an autogenerated mock type for the Store type @@ -273,13 +275,8 @@ func (_m *FakePublicDashboardStore) UpdatePublicDashboardConfig(ctx context.Cont return r0 } -type mockConstructorTestingTNewFakePublicDashboardStore interface { - mock.TestingT - Cleanup(func()) -} - -// NewFakePublicDashboardStore creates a new instance of FakePublicDashboardStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewFakePublicDashboardStore(t mockConstructorTestingTNewFakePublicDashboardStore) *FakePublicDashboardStore { +// NewFakePublicDashboardStore creates a new instance of FakePublicDashboardStore. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations. +func NewFakePublicDashboardStore(t testing.TB) *FakePublicDashboardStore { mock := &FakePublicDashboardStore{} mock.Mock.Test(t) diff --git a/pkg/services/publicdashboards/publicdashboard.go b/pkg/services/publicdashboards/publicdashboard.go index 2da091d4cb2..2631b2409a4 100644 --- a/pkg/services/publicdashboards/publicdashboard.go +++ b/pkg/services/publicdashboards/publicdashboard.go @@ -15,7 +15,7 @@ import ( //go:generate mockery --name Service --structname FakePublicDashboardService --inpackage --filename public_dashboard_service_mock.go type Service interface { AccessTokenExists(ctx context.Context, accessToken string) (bool, error) - BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error) + BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser GetDashboard(ctx context.Context, dashboardUid string) (*models.Dashboard, error) GetMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64, reqDTO PublicDashboardQueryDTO) (dtos.MetricRequest, error) GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index ffa1afb3aa5..15167904110 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -72,16 +72,24 @@ func (pd *PublicDashboardServiceImpl) GetDashboard(ctx context.Context, dashboar // Gets public dashboard via access token func (pd *PublicDashboardServiceImpl) GetPublicDashboard(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) { pubdash, dash, err := pd.store.GetPublicDashboard(ctx, accessToken) + ctxLogger := pd.log.FromContext(ctx) if err != nil { return nil, nil, err } - if pubdash == nil || dash == nil { + if pubdash == nil { + ctxLogger.Error("GetPublicDashboard: Public dashboard not found", "accessToken", accessToken) + return nil, nil, ErrPublicDashboardNotFound + } + + if dash == nil { + ctxLogger.Error("GetPublicDashboard: Dashboard not found", "accessToken", accessToken) return nil, nil, ErrPublicDashboardNotFound } if !pubdash.IsEnabled { + ctxLogger.Error("GetPublicDashboard: Public dashboard is disabled", "accessToken", accessToken) return nil, nil, ErrPublicDashboardNotFound } @@ -205,11 +213,7 @@ func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context, return nil, err } - anonymousUser, err := pd.BuildAnonymousUser(ctx, dashboard) - if err != nil { - return nil, err - } - + anonymousUser := pd.BuildAnonymousUser(ctx, dashboard) res, err := pd.QueryDataService.QueryData(ctx, anonymousUser, skipCache, metricReq) reqDatasources := metricReq.GetUniqueDatasourceTypes() @@ -225,8 +229,9 @@ func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context, } func (pd *PublicDashboardServiceImpl) GetMetricRequest(ctx context.Context, dashboard *models.Dashboard, publicDashboard *PublicDashboard, panelId int64, queryDto PublicDashboardQueryDTO) (dtos.MetricRequest, error) { - if err := validation.ValidateQueryPublicDashboardRequest(queryDto); err != nil { - return dtos.MetricRequest{}, ErrPublicDashboardBadRequest + err := validation.ValidateQueryPublicDashboardRequest(queryDto) + if err != nil { + return dtos.MetricRequest{}, err } metricReqDTO, err := pd.buildMetricRequest( @@ -270,7 +275,7 @@ func (pd *PublicDashboardServiceImpl) buildMetricRequest(ctx context.Context, da } // BuildAnonymousUser creates a user with permissions to read from all datasources used in the dashboard -func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) (*user.SignedInUser, error) { +func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, dashboard *models.Dashboard) *user.SignedInUser { datasourceUids := queries.GetUniqueDashboardDatasourceUids(dashboard.Data) // Create a temp user with read-only datasource permissions @@ -286,7 +291,7 @@ func (pd *PublicDashboardServiceImpl) BuildAnonymousUser(ctx context.Context, da permissions[datasources.ActionRead] = readScopes anonymousUser.Permissions[dashboard.OrgId] = permissions - return anonymousUser, nil + return anonymousUser } func (pd *PublicDashboardServiceImpl) PublicDashboardEnabled(ctx context.Context, dashboardUid string) (bool, error) { @@ -337,7 +342,7 @@ func (pd *PublicDashboardServiceImpl) logIsEnabledChanged(existingPubdash *Publi if newPubdash.IsEnabled { verb = "enabled" } - pd.log.Info(fmt.Sprintf("Public dashboard %v: dashboardUid: %v, user:%v", verb, newPubdash.Uid, u.Login)) + pd.log.Info("Public dashboard "+verb, "publicDashboardUid", newPubdash.Uid, "dashboardUid", newPubdash.DashboardUid, "user", u.Login) } } diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 94e4b2a7461..4c8a2a00c04 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -373,8 +373,7 @@ func TestBuildAnonymousUser(t *testing.T) { } t.Run("will add datasource read and query permissions to user for each datasource in dashboard", func(t *testing.T) { - user, err := service.BuildAnonymousUser(context.Background(), dashboard) - require.NoError(t, err) + user := service.BuildAnonymousUser(context.Background(), dashboard) require.Equal(t, dashboard.OrgId, user.OrgID) require.Equal(t, "datasources:uid:ds1", user.Permissions[user.OrgID]["datasources:query"][0]) require.Equal(t, "datasources:uid:ds3", user.Permissions[user.OrgID]["datasources:query"][1])