diff --git a/docs/sources/developers/http_api/annotations.md b/docs/sources/developers/http_api/annotations.md index 0fb5c4a5d1a..3210477738d 100644 --- a/docs/sources/developers/http_api/annotations.md +++ b/docs/sources/developers/http_api/annotations.md @@ -54,7 +54,7 @@ Query Parameters: - `to`: epoch datetime in milliseconds. Optional. - `limit`: number. Optional - default is 100. Max limit for results returned. - `alertId`: number. Optional. Find annotations for a specified alert. -- `dashboardId`: number. Optional. Find annotations that are scoped to a specific dashboard +- `dashboardId`: Deprecated. Use dashboardUID instead. - `dashboardUID`: string. Optional. Find annotations that are scoped to a specific dashboard, when dashboardUID presents, dashboardId would be ignored. - `panelId`: number. Optional. Find annotations that are scoped to a specific panel - `userId`: number. Optional. Find annotations created by a specific user @@ -113,7 +113,7 @@ Content-Type: application/json ## Create Annotation -Creates an annotation in the Grafana database. The `dashboardId` and `panelId` fields are optional. +Creates an annotation in the Grafana database. The `dashboardUid` and `panelId` fields are optional. If they are not specified then an organization annotation is created and can be queried in any dashboard that adds the Grafana annotations data source. When creating a region annotation include the timeEnd property. diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 93d68def008..31ee8d66810 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -61,34 +61,28 @@ func (hs *HTTPServer) GetAnnotations(c *contextmodel.ReqContext) response.Respon if err != nil { return response.Error(http.StatusBadRequest, "Invalid dashboard UID in annotation request", err) } else { - query.DashboardID = dqResult.ID + query.DashboardID = dqResult.ID // nolint:staticcheck } } + if query.DashboardID != 0 && query.DashboardUID == "" { // nolint:staticcheck + dq := dashboards.GetDashboardQuery{ID: query.DashboardID, OrgID: c.GetOrgID()} // nolint:staticcheck + dqResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &dq) + if err != nil { + return response.Error(http.StatusBadRequest, "Invalid dashboard ID in annotation request", err) + } + query.DashboardUID = dqResult.UID + } + items, err := hs.annotationsRepo.Find(c.Req.Context(), query) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to get annotations", err) } - // since there are several annotations per dashboard, we can cache dashboard uid - dashboardCache := make(map[int64]*string) for _, item := range items { if item.Email != "" { item.AvatarURL = dtos.GetGravatarUrl(hs.Cfg, item.Email) } - - if item.DashboardID != 0 { - if val, ok := dashboardCache[item.DashboardID]; ok { - item.DashboardUID = val - } else { - query := dashboards.GetDashboardQuery{ID: item.DashboardID, OrgID: c.GetOrgID()} - queryResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &query) - if err == nil && queryResult != nil { - item.DashboardUID = &queryResult.UID - dashboardCache[item.DashboardID] = &queryResult.UID - } - } - } } return response.JSON(http.StatusOK, items) @@ -131,7 +125,17 @@ func (hs *HTTPServer) PostAnnotation(c *contextmodel.ReqContext) response.Respon } } - if canSave, err := hs.canCreateAnnotation(c, cmd.DashboardId); err != nil || !canSave { + // get dashboard uid if not provided + if cmd.DashboardId != 0 && cmd.DashboardUID == "" { + query := dashboards.GetDashboardQuery{OrgID: c.GetOrgID(), ID: cmd.DashboardId} + queryResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &query) + if err != nil { + return response.Error(http.StatusBadRequest, "Invalid dashboard ID in annotation request", err) + } + cmd.DashboardUID = queryResult.UID + } + + if canSave, err := hs.canCreateAnnotation(c, cmd.DashboardUID); err != nil || !canSave { if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { return dashboardGuardianResponse(err) } else if err != nil { @@ -148,15 +152,16 @@ func (hs *HTTPServer) PostAnnotation(c *contextmodel.ReqContext) response.Respon userID, _ := identity.UserIdentifier(c.GetID()) item := annotations.Item{ - OrgID: c.GetOrgID(), - UserID: userID, - DashboardID: cmd.DashboardId, - PanelID: cmd.PanelId, - Epoch: cmd.Time, - EpochEnd: cmd.TimeEnd, - Text: cmd.Text, - Data: cmd.Data, - Tags: cmd.Tags, + OrgID: c.GetOrgID(), + UserID: userID, + DashboardID: cmd.DashboardId, + DashboardUID: cmd.DashboardUID, + PanelID: cmd.PanelId, + Epoch: cmd.Time, + EpochEnd: cmd.TimeEnd, + Text: cmd.Text, + Data: cmd.Data, + Tags: cmd.Tags, } if err := hs.annotationsRepo.Save(c.Req.Context(), &item); err != nil { @@ -402,6 +407,15 @@ func (hs *HTTPServer) MassDeleteAnnotations(c *contextmodel.ReqContext) response } } + if cmd.DashboardId != 0 && cmd.DashboardUID == "" { + query := dashboards.GetDashboardQuery{OrgID: c.GetOrgID(), ID: cmd.DashboardId} + queryResult, err := hs.DashboardService.GetDashboard(c.Req.Context(), &query) + if err != nil { + return response.Error(http.StatusBadRequest, "Invalid dashboard ID in annotation request", err) + } + cmd.DashboardUID = queryResult.UID + } + if (cmd.DashboardId != 0 && cmd.PanelId == 0) || (cmd.PanelId != 0 && cmd.DashboardId == 0) { err := &AnnotationError{message: "DashboardId and PanelId are both required for mass delete"} return response.Error(http.StatusBadRequest, "bad request data", err) @@ -411,28 +425,29 @@ func (hs *HTTPServer) MassDeleteAnnotations(c *contextmodel.ReqContext) response // validations only for RBAC. A user can mass delete all annotations in a (dashboard + panel) or a specific annotation // if has access to that dashboard. - var dashboardId int64 + var dashboardUID string if cmd.AnnotationId != 0 { annotation, respErr := findAnnotationByID(c.Req.Context(), hs.annotationsRepo, cmd.AnnotationId, c.SignedInUser) if respErr != nil { return respErr } - dashboardId = annotation.DashboardID + dashboardUID = *annotation.DashboardUID deleteParams = &annotations.DeleteParams{ OrgID: c.GetOrgID(), ID: cmd.AnnotationId, } } else { - dashboardId = cmd.DashboardId + dashboardUID = cmd.DashboardUID deleteParams = &annotations.DeleteParams{ - OrgID: c.GetOrgID(), - DashboardID: cmd.DashboardId, - PanelID: cmd.PanelId, + OrgID: c.GetOrgID(), + DashboardID: cmd.DashboardId, + DashboardUID: cmd.DashboardUID, + PanelID: cmd.PanelId, } } - canSave, err := hs.canMassDeleteAnnotations(c, dashboardId) + canSave, err := hs.canMassDeleteAnnotations(c, dashboardUID) if err != nil || !canSave { if !hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { return dashboardGuardianResponse(err) @@ -519,14 +534,14 @@ func (hs *HTTPServer) DeleteAnnotationByID(c *contextmodel.ReqContext) response. func (hs *HTTPServer) canSaveAnnotation(c *contextmodel.ReqContext, ac accesscontrol.AccessControl, annotation *annotations.ItemDTO) (bool, error) { if annotation.GetType() == annotations.Dashboard { - return canEditDashboard(c, ac, annotation.DashboardID) + return canEditDashboard(c, ac, *annotation.DashboardUID) } else { return true, nil } } -func canEditDashboard(c *contextmodel.ReqContext, ac accesscontrol.AccessControl, dashboardID int64) (bool, error) { - evaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashboards.ScopeDashboardsProvider.GetResourceScope(strconv.FormatInt(dashboardID, 10))) +func canEditDashboard(c *contextmodel.ReqContext, ac accesscontrol.AccessControl, dashboardUID string) (bool, error) { + evaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashboardUID)) return ac.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } @@ -630,11 +645,11 @@ func AnnotationTypeScopeResolver(annotationsRepo annotations.Repository, feature } } - if annotation.DashboardID == 0 { + if annotation.DashboardUID == nil || *annotation.DashboardUID == "" { return []string{accesscontrol.ScopeAnnotationsTypeOrganization}, nil } else { return identity.WithServiceIdentityFn(ctx, orgID, func(ctx context.Context) ([]string, error) { - dashboard, err := dashSvc.GetDashboard(ctx, &dashboards.GetDashboardQuery{ID: annotation.DashboardID, OrgID: orgID}) + dashboard, err := dashSvc.GetDashboard(ctx, &dashboards.GetDashboardQuery{UID: *annotation.DashboardUID, OrgID: orgID}) if err != nil { return nil, err } @@ -656,10 +671,10 @@ func AnnotationTypeScopeResolver(annotationsRepo annotations.Repository, feature }) } -func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardId int64) (bool, error) { +func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardUID string) (bool, error) { if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { - if dashboardId != 0 { - evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, dashboards.ScopeDashboardsProvider.GetResourceScope(strconv.FormatInt(dashboardId, 10))) + if dashboardUID != "" { + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashboardUID)) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } else { // organization annotations evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization) @@ -667,31 +682,31 @@ func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardI } } - if dashboardId != 0 { + if dashboardUID != "" { evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard) if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave { return canSave, err } - return canEditDashboard(c, hs.AccessControl, dashboardId) + return canEditDashboard(c, hs.AccessControl, dashboardUID) } else { // organization annotations evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } } -func (hs *HTTPServer) canMassDeleteAnnotations(c *contextmodel.ReqContext, dashboardID int64) (bool, error) { +func (hs *HTTPServer) canMassDeleteAnnotations(c *contextmodel.ReqContext, dashboardUID string) (bool, error) { if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagAnnotationPermissionUpdate) { - if dashboardID == 0 { + if dashboardUID == "" { evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, accesscontrol.ScopeAnnotationsTypeOrganization) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } else { - evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, dashboards.ScopeDashboardsProvider.GetResourceScope(strconv.FormatInt(dashboardID, 10))) + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(dashboardUID)) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } } - if dashboardID == 0 { + if dashboardUID == "" { evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsDelete, accesscontrol.ScopeAnnotationsTypeOrganization) return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } else { @@ -701,7 +716,7 @@ func (hs *HTTPServer) canMassDeleteAnnotations(c *contextmodel.ReqContext, dashb return false, err } - canSave, err = canEditDashboard(c, hs.AccessControl, dashboardID) + canSave, err = canEditDashboard(c, hs.AccessControl, dashboardUID) if err != nil || !canSave { return false, err } diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index 6eaf4317be6..cffc023b909 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -399,8 +399,8 @@ func TestAPI_Annotations(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() repo := annotationstest.NewFakeAnnotationsRepo() - _ = repo.Save(context.Background(), &annotations.Item{ID: 1, DashboardID: 0}) - _ = repo.Save(context.Background(), &annotations.Item{ID: 2, DashboardID: 1}) + _ = repo.Save(context.Background(), &annotations.Item{ID: 1, DashboardID: 0, DashboardUID: ""}) + _ = repo.Save(context.Background(), &annotations.Item{ID: 2, DashboardID: 1, DashboardUID: "dashuid1"}) hs.annotationsRepo = repo hs.Features = featuremgmt.WithFeatures(tt.featureFlags...) dashService := &dashboards.FakeDashboardService{} @@ -413,7 +413,7 @@ func TestAPI_Annotations(t *testing.T) { hs.folderService = folderService hs.AccessControl = acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) hs.AccessControl.RegisterScopeAttributeResolver(AnnotationTypeScopeResolver(hs.annotationsRepo, hs.Features, dashService, folderService)) - hs.AccessControl.RegisterScopeAttributeResolver(dashboards.NewDashboardIDScopeResolver(dashService, folderService)) + hs.AccessControl.RegisterScopeAttributeResolver(dashboards.NewDashboardUIDScopeResolver(dashService, folderService)) }) var body io.Reader if tt.body != "" { @@ -436,11 +436,11 @@ func TestService_AnnotationTypeScopeResolver(t *testing.T) { dashSvc := &dashboards.FakeDashboardService{} rootDash := &dashboards.Dashboard{ID: 1, OrgID: 1, UID: rootDashUID} folderDash := &dashboards.Dashboard{ID: 2, OrgID: 1, UID: folderDashUID, FolderUID: folderUID} - dashSvc.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{ID: rootDash.ID, OrgID: 1}).Return(rootDash, nil) - dashSvc.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{ID: folderDash.ID, OrgID: 1}).Return(folderDash, nil) + dashSvc.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{UID: rootDash.UID, OrgID: 1}).Return(rootDash, nil) + dashSvc.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{UID: folderDash.UID, OrgID: 1}).Return(folderDash, nil) - rootDashboardAnnotation := annotations.Item{ID: 1, DashboardID: rootDash.ID} - folderDashboardAnnotation := annotations.Item{ID: 3, DashboardID: folderDash.ID} + rootDashboardAnnotation := annotations.Item{ID: 1, DashboardID: rootDash.ID, DashboardUID: rootDash.UID} + folderDashboardAnnotation := annotations.Item{ID: 3, DashboardID: folderDash.ID, DashboardUID: folderDash.UID} organizationAnnotation := annotations.Item{ID: 2} fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo() diff --git a/pkg/services/annotations/accesscontrol/accesscontrol.go b/pkg/services/annotations/accesscontrol/accesscontrol.go index f2e89acd23b..d315e9076b8 100644 --- a/pkg/services/annotations/accesscontrol/accesscontrol.go +++ b/pkg/services/annotations/accesscontrol/accesscontrol.go @@ -63,11 +63,11 @@ func (authz *AuthService) Authorize(ctx context.Context, query annotations.ItemQ var err error if canAccessDashAnnotations { if query.AnnotationID != 0 { - annotationDashboardID, err := authz.getAnnotationDashboard(ctx, query) + annotationDashboardUID, err := authz.getAnnotationDashboard(ctx, query) if err != nil { return nil, ErrAccessControlInternal.Errorf("failed to fetch annotations: %w", err) } - query.DashboardID = annotationDashboardID + query.DashboardUID = annotationDashboardUID } visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, query) @@ -83,7 +83,7 @@ func (authz *AuthService) Authorize(ctx context.Context, query annotations.ItemQ }, nil } -func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query annotations.ItemQuery) (int64, error) { +func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query annotations.ItemQuery) (string, error) { var items []annotations.Item params := make([]any, 0) err := authz.db.WithDbSession(ctx, func(sess *db.Session) error { @@ -91,7 +91,7 @@ func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query anno SELECT a.id, a.org_id, - a.dashboard_id + a.dashboard_uid FROM annotation as a WHERE a.org_id = ? AND a.id = ? ` @@ -100,13 +100,13 @@ func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query anno return sess.SQL(sql, params...).Find(&items) }) if err != nil { - return 0, err + return "", err } if len(items) == 0 { - return 0, ErrAccessControlInternal.Errorf("annotation not found") + return "", ErrAccessControlInternal.Errorf("annotation not found") } - return items[0].DashboardID, nil + return items[0].DashboardUID, nil } func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, query annotations.ItemQuery) (map[string]int64, error) { @@ -130,11 +130,6 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, UIDs: []string{query.DashboardUID}, }) } - if query.DashboardID != 0 { - filters = append(filters, searchstore.DashboardIDFilter{ - IDs: []int64{query.DashboardID}, - }) - } dashs, err := authz.dashSvc.SearchDashboards(ctx, &dashboards.FindPersistedDashboardsQuery{ OrgId: query.SignedInUser.GetOrgID(), diff --git a/pkg/services/annotations/annotationsimpl/annotations.go b/pkg/services/annotations/annotationsimpl/annotations.go index 496b782be8f..07dff48325a 100644 --- a/pkg/services/annotations/annotationsimpl/annotations.go +++ b/pkg/services/annotations/annotationsimpl/annotations.go @@ -79,6 +79,7 @@ func (r *RepositoryImpl) Find(ctx context.Context, query *annotations.ItemQuery) } // Search without dashboard UID filter is expensive, so check without access control first + // nolint: staticcheck if query.DashboardID == 0 && query.DashboardUID == "" { // Return early if no annotations found, it's not necessary to perform expensive access control filtering res, err := r.reader.Get(ctx, *query, &accesscontrol.AccessResources{ diff --git a/pkg/services/annotations/annotationsimpl/annotations_test.go b/pkg/services/annotations/annotationsimpl/annotations_test.go index 28ee61f4afb..600e41ce269 100644 --- a/pkg/services/annotations/annotationsimpl/annotations_test.go +++ b/pkg/services/annotations/annotationsimpl/annotations_test.go @@ -81,7 +81,7 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) { }), }) - _ = testutil.CreateDashboard(t, sql, cfg, features, dashboards.SaveDashboardCommand{ + dashboard2 := testutil.CreateDashboard(t, sql, cfg, features, dashboards.SaveDashboardCommand{ UserID: 1, OrgID: 1, IsFolder: false, @@ -91,18 +91,20 @@ func TestIntegrationAnnotationListingWithRBAC(t *testing.T) { }) dash1Annotation := &annotations.Item{ - OrgID: 1, - DashboardID: 1, - Epoch: 10, + OrgID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard1.UID, + Epoch: 10, } err = repo.Save(context.Background(), dash1Annotation) require.NoError(t, err) dash2Annotation := &annotations.Item{ - OrgID: 1, - DashboardID: 2, - Epoch: 10, - Tags: []string{"foo:bar"}, + OrgID: 1, + DashboardID: 2, // nolint: staticcheck + DashboardUID: dashboard2.UID, + Epoch: 10, + Tags: []string{"foo:bar"}, } err = repo.Save(context.Background(), dash2Annotation) require.NoError(t, err) @@ -292,10 +294,11 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) { annotationTxt := fmt.Sprintf("annotation %d", i) dash1Annotation := &annotations.Item{ - OrgID: orgID, - DashboardID: dashboard.ID, - Epoch: 10, - Text: annotationTxt, + OrgID: orgID, + DashboardID: dashboard.ID, // nolint: staticcheck + DashboardUID: dashboard.UID, + Epoch: 10, + Text: annotationTxt, } err = store.Add(context.Background(), dash1Annotation) require.NoError(t, err) diff --git a/pkg/services/annotations/annotationsimpl/cleanup_test.go b/pkg/services/annotations/annotationsimpl/cleanup_test.go index 324699736b1..359ae0e8508 100644 --- a/pkg/services/annotations/annotationsimpl/cleanup_test.go +++ b/pkg/services/annotations/annotationsimpl/cleanup_test.go @@ -3,6 +3,7 @@ package annotationsimpl import ( "context" "errors" + "strconv" "testing" "time" @@ -238,24 +239,27 @@ func createTestAnnotations(t *testing.T, store db.DB, expectedCount int, oldAnno newAnnotationTags := make([]*annotationTag, 0, 2*expectedCount) for i := 0; i < expectedCount; i++ { a := &annotations.Item{ - ID: int64(i + 1), - DashboardID: 1, - OrgID: 1, - UserID: 1, - PanelID: 1, - Text: "", + ID: int64(i + 1), + DashboardID: 1, + DashboardUID: "uid" + strconv.Itoa(i), + OrgID: 1, + UserID: 1, + PanelID: 1, + Text: "", } // mark every third as an API annotation // that does not belong to a dashboard if i%3 == 1 { - a.DashboardID = 0 + a.DashboardID = 0 // nolint: staticcheck + a.DashboardUID = "" } // mark every third annotation as an alert annotation if i%3 == 0 { a.AlertID = 10 - a.DashboardID = 2 + a.DashboardID = 2 // nolint: staticcheck + a.DashboardUID = "dashboard2uid" } // create epoch as int annotations.go line 40 diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store.go b/pkg/services/annotations/annotationsimpl/loki/historian_store.go index 707e9c6bebb..933b7426ab1 100644 --- a/pkg/services/annotations/annotationsimpl/loki/historian_store.go +++ b/pkg/services/annotations/annotationsimpl/loki/historian_store.go @@ -85,6 +85,7 @@ func (r *LokiHistorianStore) Get(ctx context.Context, query annotations.ItemQuer // if the query is filtering on tags, but not on a specific dashboard, we shouldn't query loki // since state history won't have tags for annotations + // nolint: staticcheck if len(query.Tags) > 0 && query.DashboardID == 0 && query.DashboardUID == "" { return make([]*annotations.ItemDTO, 0), nil } @@ -178,7 +179,7 @@ func (r *LokiHistorianStore) annotationsFromStream(stream historian.Stream, ac a items = append(items, &annotations.ItemDTO{ AlertID: entry.RuleID, - DashboardID: ac.Dashboards[entry.DashboardUID], + DashboardID: ac.Dashboards[entry.DashboardUID], // nolint: staticcheck DashboardUID: &entry.DashboardUID, PanelID: entry.PanelID, NewState: entry.Current, @@ -280,8 +281,10 @@ func buildHistoryQuery(query *annotations.ItemQuery, dashboards map[string]int64 RuleUID: ruleUID, } + // nolint: staticcheck if historyQuery.DashboardUID == "" && query.DashboardID != 0 { for uid, id := range dashboards { + // nolint: staticcheck if query.DashboardID == id { historyQuery.DashboardUID = uid break diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go b/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go index 1fa642e3731..cf6c7af51e3 100644 --- a/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go +++ b/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go @@ -193,7 +193,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: dashboard1.ID, + DashboardID: dashboard1.ID, // nolint: staticcheck From: start.UnixMilli(), To: start.Add(time.Second * time.Duration(numTransitions+1)).UnixMilli(), } @@ -243,7 +243,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: dashboard1.ID, + DashboardID: dashboard1.ID, // nolint: staticcheck From: start.Add(-2 * time.Second).UnixMilli(), To: start.Add(-1 * time.Second).UnixMilli(), } @@ -273,7 +273,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: dashboard1.ID, + DashboardID: dashboard1.ID, // nolint: staticcheck From: start.Add(-1 * time.Second).UnixMilli(), // should clamp to start To: start.Add(1 * time.Second).UnixMilli(), } @@ -294,17 +294,17 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { fakeLokiClient.cfg.MaxQueryLength = oldMax }) - t.Run("should sort history by time", func(t *testing.T) { + t.Run("should sort history by time and be able to query by dashboard uid", func(t *testing.T) { fakeLokiClient.rangeQueryRes = []historian.Stream{ historian.StatesToStream(ruleMetaFromRule(t, dashboardRules[dashboard1.UID][0]), transitions, map[string]string{}, log.NewNopLogger()), historian.StatesToStream(ruleMetaFromRule(t, dashboardRules[dashboard1.UID][1]), transitions, map[string]string{}, log.NewNopLogger()), } query := annotations.ItemQuery{ - OrgID: 1, - DashboardID: dashboard1.ID, - From: start.UnixMilli(), - To: start.Add(time.Second * time.Duration(numTransitions+1)).UnixMilli(), + OrgID: 1, + DashboardUID: dashboard1.UID, + From: start.UnixMilli(), + To: start.Add(time.Second * time.Duration(numTransitions+1)).UnixMilli(), } res, err := store.Get( context.Background(), @@ -393,7 +393,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { expected := &annotations.ItemDTO{ AlertID: rule.ID, - DashboardID: dashboard1.ID, + DashboardID: dashboard1.ID, // nolint: staticcheck DashboardUID: &dashboard1.UID, PanelID: *rule.PanelID, Time: transition.LastEvaluationTime.UnixMilli(), @@ -433,6 +433,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { require.Len(t, items, numTransitions) for _, item := range items { + // nolint: staticcheck require.Equal(t, dashboard1.ID, item.DashboardID) require.Equal(t, dashboard1.UID, *item.DashboardUID) } @@ -464,7 +465,7 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) { for _, item := range items { require.Zero(t, *item.DashboardUID) - require.Zero(t, item.DashboardID) + require.Zero(t, item.DashboardID) // nolint: staticcheck } }) }) @@ -553,7 +554,7 @@ func TestBuildHistoryQuery(t *testing.T) { t.Run("should set dashboard UID from dashboard ID if query does not contain UID", func(t *testing.T) { query := buildHistoryQuery( &annotations.ItemQuery{ - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck }, map[string]int64{ "dashboard-uid": 1, @@ -566,7 +567,7 @@ func TestBuildHistoryQuery(t *testing.T) { t.Run("should skip dashboard UID if missing from query and dashboard map", func(t *testing.T) { query := buildHistoryQuery( &annotations.ItemQuery{ - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck }, map[string]int64{ "other-dashboard-uid": 2, @@ -794,7 +795,7 @@ func compareAnnotationItem(t *testing.T, expected, actual *annotations.ItemDTO) require.Equal(t, expected.PanelID, actual.PanelID) } if expected.DashboardUID != nil { - require.Equal(t, expected.DashboardID, actual.DashboardID) + require.Equal(t, expected.DashboardID, actual.DashboardID) // nolint: staticcheck require.Equal(t, *expected.DashboardUID, *actual.DashboardUID) } require.Equal(t, expected.NewState, actual.NewState) diff --git a/pkg/services/annotations/annotationsimpl/xorm_store.go b/pkg/services/annotations/annotationsimpl/xorm_store.go index 317f9915566..92fb80507c7 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store.go @@ -5,11 +5,11 @@ import ( "context" "errors" "fmt" - "strconv" "strings" "time" "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore/migrations" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -46,6 +46,13 @@ type xormRepositoryImpl struct { } func NewXormStore(cfg *setting.Cfg, l log.Logger, db db.DB, tagService tag.Service) *xormRepositoryImpl { + // populate dashboard_uid at startup, to ensure safe downgrades & upgrades after + // the initial migration occurs + err := migrations.RunDashboardUIDMigrations(db.GetEngine().NewSession(), db.GetEngine().DriverName()) + if err != nil { + l.Error("failed to populate dashboard_uid for annotations", "error", err) + } + return &xormRepositoryImpl{ cfg: cfg, db: db, @@ -255,6 +262,7 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer annotation.id, annotation.epoch as time, annotation.epoch_end as time_end, + annotation.dashboard_uid, annotation.dashboard_id, annotation.panel_id, annotation.new_state, @@ -292,11 +300,18 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer params = append(params, query.AlertUID, query.OrgID) } + // nolint: staticcheck if query.DashboardID != 0 { sql.WriteString(` AND a.dashboard_id = ?`) params = append(params, query.DashboardID) } + // note: orgID is already required above + if query.DashboardUID != "" { + sql.WriteString(` AND a.dashboard_uid = ?`) + params = append(params, query.DashboardUID) + } + if query.PanelID != 0 { sql.WriteString(` AND a.panel_id = ?`) params = append(params, query.PanelID) @@ -351,13 +366,11 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer } } - acFilter, err := r.getAccessControlFilter(query.SignedInUser, accessResources) - if err != nil { - return err - } + acFilter, acParams := r.getAccessControlFilter(query.SignedInUser, accessResources) if acFilter != "" { sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter)) } + params = append(params, acParams...) // order of ORDER BY arguments match the order of a sql index for performance orderBy := " ORDER BY a.org_id, a.epoch_end DESC, a.epoch DESC" @@ -377,41 +390,30 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer return items, err } -func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester, accessResources *accesscontrol.AccessResources) (string, error) { +func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester, accessResources *accesscontrol.AccessResources) (string, []any) { if accessResources.SkipAccessControlFilter { return "", nil } var filters []string + var params []any if accessResources.CanAccessOrgAnnotations { filters = append(filters, "a.dashboard_id = 0") } if accessResources.CanAccessDashAnnotations { - var dashboardIDs []int64 - for _, id := range accessResources.Dashboards { - dashboardIDs = append(dashboardIDs, id) - } - - var inClause string - if len(dashboardIDs) == 0 { - inClause = "SELECT * FROM (SELECT 0 LIMIT 0) tt" // empty set + if len(accessResources.Dashboards) == 0 { + filters = append(filters, "1=0") // empty set } else { - b := make([]byte, 0, 3*len(dashboardIDs)) - - b = strconv.AppendInt(b, dashboardIDs[0], 10) - for _, num := range dashboardIDs[1:] { - b = append(b, ',') - b = strconv.AppendInt(b, num, 10) + filters = append(filters, fmt.Sprintf("a.dashboard_uid IN (%s)", strings.Repeat("?,", len(accessResources.Dashboards)-1)+"?")) + for uid := range accessResources.Dashboards { + params = append(params, uid) } - - inClause = string(b) } - filters = append(filters, fmt.Sprintf("a.dashboard_id IN (%s)", inClause)) } - return strings.Join(filters, " OR "), nil + return strings.Join(filters, " OR "), params } func (r *xormRepositoryImpl) Delete(ctx context.Context, params *annotations.DeleteParams) error { @@ -433,14 +435,27 @@ func (r *xormRepositoryImpl) Delete(ctx context.Context, params *annotations.Del if _, err := sess.Exec(sql, params.ID, params.OrgID); err != nil { return err } + } else if params.DashboardUID != "" { + annoTagSQL = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE dashboard_uid = ? AND panel_id = ? AND org_id = ?)" + sql = "DELETE FROM annotation WHERE dashboard_uid = ? AND panel_id = ? AND org_id = ?" + + if _, err := sess.Exec(annoTagSQL, params.DashboardUID, params.PanelID, params.OrgID); err != nil { + return err + } + + if _, err := sess.Exec(sql, params.DashboardUID, params.PanelID, params.OrgID); err != nil { + return err + } } else { annoTagSQL = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE dashboard_id = ? AND panel_id = ? AND org_id = ?)" sql = "DELETE FROM annotation WHERE dashboard_id = ? AND panel_id = ? AND org_id = ?" + // nolint: staticcheck if _, err := sess.Exec(annoTagSQL, params.DashboardID, params.PanelID, params.OrgID); err != nil { return err } + // nolint: staticcheck if _, err := sess.Exec(sql, params.DashboardID, params.PanelID, params.OrgID); err != nil { return err } diff --git a/pkg/services/annotations/annotationsimpl/xorm_store_test.go b/pkg/services/annotations/annotationsimpl/xorm_store_test.go index 2dc394c392a..015c42a5075 100644 --- a/pkg/services/annotations/annotationsimpl/xorm_store_test.go +++ b/pkg/services/annotations/annotationsimpl/xorm_store_test.go @@ -78,14 +78,15 @@ func TestIntegrationAnnotations(t *testing.T) { var err error annotation := &annotations.Item{ - OrgID: 1, - UserID: 1, - DashboardID: dashboard.ID, - Text: "hello", - Type: "alert", - Epoch: 10, - Tags: []string{"outage", "error", "type:outage", "server:server-1"}, - Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), + OrgID: 1, + UserID: 1, + DashboardID: dashboard.ID, // nolint: staticcheck + DashboardUID: dashboard.UID, + Text: "hello", + Type: "alert", + Epoch: 10, + Tags: []string{"outage", "error", "type:outage", "server:server-1"}, + Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), } err = store.Add(context.Background(), annotation) require.NoError(t, err) @@ -93,14 +94,15 @@ func TestIntegrationAnnotations(t *testing.T) { assert.Equal(t, annotation.Epoch, annotation.EpochEnd) annotation2 := &annotations.Item{ - OrgID: 1, - UserID: 1, - DashboardID: dashboard2.ID, - Text: "hello", - Type: "alert", - Epoch: 21, // Should swap epoch & epochEnd - EpochEnd: 20, - Tags: []string{"outage", "type:outage", "server:server-1", "error"}, + OrgID: 1, + UserID: 1, + DashboardID: dashboard2.ID, // nolint: staticcheck + DashboardUID: dashboard2.UID, + Text: "hello", + Type: "alert", + Epoch: 21, // Should swap epoch & epochEnd + EpochEnd: 20, + Tags: []string{"outage", "type:outage", "server:server-1", "error"}, } err = store.Add(context.Background(), annotation2) require.NoError(t, err) @@ -135,7 +137,31 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can query for annotation by dashboard id", func(t *testing.T) { items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: dashboard.ID, + DashboardID: dashboard.ID, // nolint: staticcheck + From: 0, + To: 15, + SignedInUser: testUser, + }, &annotation_ac.AccessResources{ + Dashboards: map[string]int64{ + dashboard.UID: dashboard.ID, + }, + CanAccessDashAnnotations: true, + }) + + require.NoError(t, err) + assert.Len(t, items, 1) + + assert.Equal(t, []string{"outage", "error", "type:outage", "server:server-1"}, items[0].Tags) + + assert.GreaterOrEqual(t, items[0].Created, int64(0)) + assert.GreaterOrEqual(t, items[0].Updated, int64(0)) + assert.Equal(t, items[0].Updated, items[0].Created) + }) + + t.Run("Can query for annotation by dashboard uid", func(t *testing.T) { + items, err := store.Get(context.Background(), annotations.ItemQuery{ + OrgID: 1, + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, @@ -234,12 +260,13 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Should not find any when item is outside time range", func(t *testing.T) { accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 12, To: 15, SignedInUser: testUser, @@ -250,12 +277,13 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Should not find one when tag filter does not match", func(t *testing.T) { accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 1, To: 15, Tags: []string{"asd"}, @@ -267,12 +295,13 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Should not find one when type filter does not match", func(t *testing.T) { accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 1, To: 15, Type: "alert", @@ -284,12 +313,13 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Should find one when all tag filters does match", func(t *testing.T) { accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 1, To: 15, // this will exclude the second test annotation Tags: []string{"outage", "error"}, @@ -315,12 +345,13 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Should find one when all key value tag filters does match", func(t *testing.T) { accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 1, To: 15, Tags: []string{"type:outage", "server:server-1"}, @@ -333,13 +364,14 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can update annotation and remove all tags", func(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) @@ -368,13 +400,14 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can update annotation with new tags", func(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) @@ -401,13 +434,14 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can update annotation with additional tags", func(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) @@ -434,13 +468,14 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can update annotations with data", func(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) @@ -470,13 +505,14 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can delete annotation", func(t *testing.T) { query := annotations.ItemQuery{ OrgID: 1, - DashboardID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: dashboard.UID, From: 0, To: 15, SignedInUser: testUser, } accRes := &annotation_ac.AccessResources{ - Dashboards: map[string]int64{"foo": 1}, + Dashboards: map[string]int64{dashboard.UID: 1}, CanAccessDashAnnotations: true, } items, err := store.Get(context.Background(), query, accRes) @@ -493,14 +529,53 @@ func TestIntegrationAnnotations(t *testing.T) { t.Run("Can delete annotation using dashboard id and panel id", func(t *testing.T) { annotation3 := &annotations.Item{ - OrgID: 1, - UserID: 1, - DashboardID: dashboard2.ID, - Text: "toBeDeletedWithPanelId", - Type: "alert", - Epoch: 11, - Tags: []string{"test"}, - PanelID: 20, + OrgID: 1, + UserID: 1, + DashboardID: dashboard2.ID, // nolint: staticcheck + DashboardUID: dashboard2.UID, + Text: "toBeDeletedWithPanelId", + Type: "alert", + Epoch: 11, + Tags: []string{"test"}, + PanelID: 20, + } + err = store.Add(context.Background(), annotation3) + require.NoError(t, err) + + accRes := &annotation_ac.AccessResources{ + Dashboards: map[string]int64{ + dashboard2.UID: dashboard2.ID, + }, + CanAccessDashAnnotations: true, + } + + query := annotations.ItemQuery{ + OrgID: 1, + AnnotationID: annotation3.ID, + SignedInUser: testUser, + } + items, err := store.Get(context.Background(), query, accRes) + require.NoError(t, err) + + // nolint:staticcheck + err = store.Delete(context.Background(), &annotations.DeleteParams{DashboardID: items[0].DashboardID, PanelID: items[0].PanelID, OrgID: 1}) + require.NoError(t, err) + + items, err = store.Get(context.Background(), query, accRes) + require.NoError(t, err) + assert.Empty(t, items) + }) + + t.Run("Can delete annotation using dashboard uid and panel id", func(t *testing.T) { + annotation3 := &annotations.Item{ + OrgID: 1, + UserID: 1, + DashboardUID: dashboard2.UID, + Text: "toBeDeletedWithPanelId", + Type: "alert", + Epoch: 11, + Tags: []string{"test"}, + PanelID: 20, } err = store.Add(context.Background(), annotation3) require.NoError(t, err) @@ -520,9 +595,8 @@ func TestIntegrationAnnotations(t *testing.T) { items, err := store.Get(context.Background(), query, accRes) require.NoError(t, err) - dashboardId := items[0].DashboardID - panelId := items[0].PanelID - err = store.Delete(context.Background(), &annotations.DeleteParams{DashboardID: dashboardId, PanelID: panelId, OrgID: 1}) + // nolint:staticcheck + err = store.Delete(context.Background(), &annotations.DeleteParams{DashboardUID: *items[0].DashboardUID, PanelID: items[0].PanelID, OrgID: 1}) require.NoError(t, err) items, err = store.Get(context.Background(), query, accRes) @@ -635,15 +709,16 @@ func benchmarkFindTags(b *testing.B, numAnnotations int) { require.NoError(b, err) annotationWithTheTag := annotations.Item{ - ID: int64(numAnnotations) + 1, - OrgID: 1, - UserID: 1, - DashboardID: int64(1), - Text: "hello", - Type: "alert", - Epoch: 10, - Tags: []string{"outage", "error", "type:outage", "server:server-1"}, - Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), + ID: int64(numAnnotations) + 1, + OrgID: 1, + UserID: 1, + DashboardID: 1, // nolint: staticcheck + DashboardUID: "uid", + Text: "hello", + Type: "alert", + Epoch: 10, + Tags: []string{"outage", "error", "type:outage", "server:server-1"}, + Data: simplejson.NewFromAny(map[string]any{"data1": "I am a cool data", "data2": "I am another cool data"}), } err = store.Add(context.Background(), &annotationWithTheTag) require.NoError(b, err) diff --git a/pkg/services/annotations/annotationstest/fake.go b/pkg/services/annotations/annotationstest/fake.go index b4119e57b85..c7983bd4a36 100644 --- a/pkg/services/annotations/annotationstest/fake.go +++ b/pkg/services/annotations/annotationstest/fake.go @@ -26,7 +26,7 @@ func (repo *fakeAnnotationsRepo) Delete(_ context.Context, params *annotations.D delete(repo.annotations, params.ID) } else { for _, v := range repo.annotations { - if params.DashboardID == v.DashboardID && params.PanelID == v.PanelID { + if params.DashboardUID == v.DashboardUID && params.PanelID == v.PanelID { delete(repo.annotations, v.ID) } } @@ -70,7 +70,7 @@ func (repo *fakeAnnotationsRepo) Find(_ context.Context, query *annotations.Item defer repo.mtx.Unlock() if annotation, has := repo.annotations[query.AnnotationID]; has { - return []*annotations.ItemDTO{{ID: annotation.ID, DashboardID: annotation.DashboardID}}, nil + return []*annotations.ItemDTO{{ID: annotation.ID, DashboardID: annotation.DashboardID, DashboardUID: &annotation.DashboardUID}}, nil // nolint: staticcheck } annotations := []*annotations.ItemDTO{{ID: 1, DashboardID: 0}} return annotations, nil diff --git a/pkg/services/annotations/models.go b/pkg/services/annotations/models.go index 9dcd23755ea..b5e295f876c 100644 --- a/pkg/services/annotations/models.go +++ b/pkg/services/annotations/models.go @@ -6,12 +6,13 @@ import ( ) type ItemQuery struct { - OrgID int64 `json:"orgId"` - From int64 `json:"from"` - To int64 `json:"to"` - UserID int64 `json:"userId"` - AlertID int64 `json:"alertId"` - AlertUID string `json:"alertUID"` + OrgID int64 `json:"orgId"` + From int64 `json:"from"` + To int64 `json:"to"` + UserID int64 `json:"userId"` + AlertID int64 `json:"alertId"` + AlertUID string `json:"alertUID"` + // Deprecated: Use DashboardUID and OrgID instead DashboardID int64 `json:"dashboardId"` DashboardUID string `json:"dashboardUID"` PanelID int64 `json:"panelId"` @@ -72,28 +73,32 @@ type GetAnnotationTagsResponse struct { } type DeleteParams struct { - OrgID int64 - ID int64 - DashboardID int64 - PanelID int64 + OrgID int64 + ID int64 + // Deprecated: Use DashboardUID and OrgID instead + DashboardID int64 + DashboardUID string + PanelID int64 } type Item struct { - ID int64 `json:"id" xorm:"pk autoincr 'id'"` - OrgID int64 `json:"orgId" xorm:"org_id"` - UserID int64 `json:"userId" xorm:"user_id"` - DashboardID int64 `json:"dashboardId" xorm:"dashboard_id"` - PanelID int64 `json:"panelId" xorm:"panel_id"` - Text string `json:"text"` - AlertID int64 `json:"alertId" xorm:"alert_id"` - PrevState string `json:"prevState"` - NewState string `json:"newState"` - Epoch int64 `json:"epoch"` - EpochEnd int64 `json:"epochEnd"` - Created int64 `json:"created"` - Updated int64 `json:"updated"` - Tags []string `json:"tags"` - Data *simplejson.Json `json:"data"` + ID int64 `json:"id" xorm:"pk autoincr 'id'"` + OrgID int64 `json:"orgId" xorm:"org_id"` + UserID int64 `json:"userId" xorm:"user_id"` + // Deprecated: Use DashboardUID and OrgID instead + DashboardID int64 `json:"dashboardId" xorm:"dashboard_id"` + DashboardUID string `json:"dashboardUID" xorm:"dashboard_uid"` + PanelID int64 `json:"panelId" xorm:"panel_id"` + Text string `json:"text"` + AlertID int64 `json:"alertId" xorm:"alert_id"` + PrevState string `json:"prevState"` + NewState string `json:"newState"` + Epoch int64 `json:"epoch"` + EpochEnd int64 `json:"epochEnd"` + Created int64 `json:"created"` + Updated int64 `json:"updated"` + Tags []string `json:"tags"` + Data *simplejson.Json `json:"data"` // needed until we remove it from db Type string @@ -106,9 +111,10 @@ func (i Item) TableName() string { // swagger:model Annotation type ItemDTO struct { - ID int64 `json:"id" xorm:"id"` - AlertID int64 `json:"alertId" xorm:"alert_id"` - AlertName string `json:"alertName"` + ID int64 `json:"id" xorm:"id"` + AlertID int64 `json:"alertId" xorm:"alert_id"` + AlertName string `json:"alertName"` + // Deprecated: Use DashboardUID and OrgID instead DashboardID int64 `json:"dashboardId" xorm:"dashboard_id"` DashboardUID *string `json:"dashboardUID" xorm:"dashboard_uid"` PanelID int64 `json:"panelId" xorm:"panel_id"` @@ -164,7 +170,7 @@ func (a annotationType) String() string { } func (annotation *ItemDTO) GetType() annotationType { - if annotation.DashboardID != 0 { + if annotation.DashboardUID != nil && *annotation.DashboardUID != "" { return Dashboard } return Organization diff --git a/pkg/services/ngalert/state/historian/annotation_store.go b/pkg/services/ngalert/state/historian/annotation_store.go index d04f8395508..8e50447e8ad 100644 --- a/pkg/services/ngalert/state/historian/annotation_store.go +++ b/pkg/services/ngalert/state/historian/annotation_store.go @@ -38,7 +38,8 @@ func (s *AnnotationServiceStore) Save(ctx context.Context, panel *PanelKey, anno } for i := range annotations { - annotations[i].DashboardID = dashID + annotations[i].DashboardID = dashID // nolint: staticcheck + annotations[i].DashboardUID = panel.dashUID annotations[i].PanelID = panel.panelID } } diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index 676eb96c966..689809aa3e2 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -80,16 +80,17 @@ type AnnotationsDto struct { } type AnnotationEvent struct { - Id int64 `json:"id"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - Tags []string `json:"tags"` - IsRegion bool `json:"isRegion"` - Text string `json:"text"` - Color string `json:"color"` - Time int64 `json:"time"` - TimeEnd int64 `json:"timeEnd"` - Source dashboard.AnnotationQuery `json:"source"` + Id int64 `json:"id"` + DashboardId int64 `json:"dashboardId"` + DashboardUID string `json:"dashboardUID"` + PanelId int64 `json:"panelId"` + Tags []string `json:"tags"` + IsRegion bool `json:"isRegion"` + Text string `json:"text"` + Color string `json:"color"` + Time int64 `json:"time"` + TimeEnd int64 `json:"timeEnd"` + Source dashboard.AnnotationQuery `json:"source"` } func (pd PublicDashboard) TableName() string { diff --git a/pkg/services/publicdashboards/service/query.go b/pkg/services/publicdashboards/service/query.go index 446f74ab3b6..3e21e959722 100644 --- a/pkg/services/publicdashboards/service/query.go +++ b/pkg/services/publicdashboards/service/query.go @@ -55,7 +55,8 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT annoQuery.Limit = anno.Target.Limit annoQuery.MatchAny = anno.Target.MatchAny if anno.Target.Type == "tags" { - annoQuery.DashboardID = 0 + annoQuery.DashboardID = 0 // nolint: staticcheck + annoQuery.DashboardUID = "" annoQuery.Tags = anno.Target.Tags } } @@ -68,7 +69,7 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT for _, item := range annotationItems { event := models.AnnotationEvent{ Id: item.ID, - DashboardId: item.DashboardID, + DashboardId: item.DashboardID, // nolint: staticcheck Tags: item.Tags, IsRegion: item.TimeEnd > 0 && item.Time != item.TimeEnd, Text: item.Text, @@ -78,6 +79,10 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT Source: anno, } + if item.DashboardUID != nil { + event.DashboardUID = *item.DashboardUID + } + // We want dashboard annotations to reference the panel they're for. If no panelId is provided, they'll show up on all panels // which is only intended for tag and org annotations. if anno.Type != nil && *anno.Type == "dashboard" { diff --git a/pkg/services/sqlstore/migrations/annotation_mig.go b/pkg/services/sqlstore/migrations/annotation_mig.go index 6db2495ae06..896ef21aa8d 100644 --- a/pkg/services/sqlstore/migrations/annotation_mig.go +++ b/pkg/services/sqlstore/migrations/annotation_mig.go @@ -1,6 +1,8 @@ package migrations import ( + "fmt" + "github.com/grafana/grafana/pkg/util/xorm" . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -191,6 +193,12 @@ func addAnnotationMig(mg *Migrator) { mg.AddMigration("Increase new_state column to length 40 not null", NewRawSQLMigration(""). Postgres("ALTER TABLE annotation ALTER COLUMN new_state TYPE VARCHAR(40);"). // Does not modify nullability. Mysql("ALTER TABLE annotation MODIFY new_state VARCHAR(40) NOT NULL;")) + + mg.AddMigration("Add dashboard_uid column to annotation table", NewAddColumnMigration(table, &Column{ + Name: "dashboard_uid", Type: DB_NVarchar, Length: 40, Nullable: true, + })) + + mg.AddMigration("Add missing dashboard_uid to annotation table", &SetDashboardUIDMigration{}) } type AddMakeRegionSingleRowMigration struct { @@ -225,3 +233,40 @@ func (m *AddMakeRegionSingleRowMigration) Exec(sess *xorm.Session, mg *Migrator) _, err = sess.Exec("DELETE FROM annotation WHERE region_id > 0 AND id <> region_id") return err } + +type SetDashboardUIDMigration struct { + MigrationBase +} + +func (m *SetDashboardUIDMigration) SQL(dialect Dialect) string { + return "code migration" +} + +func (m *SetDashboardUIDMigration) Exec(sess *xorm.Session, mg *Migrator) error { + return RunDashboardUIDMigrations(sess, mg.Dialect.DriverName()) +} + +func RunDashboardUIDMigrations(sess *xorm.Session, driverName string) error { + sql := `UPDATE annotation + SET dashboard_uid = (SELECT uid FROM dashboard WHERE dashboard.id = annotation.dashboard_id) + WHERE dashboard_uid IS NULL AND dashboard_id != 0 AND EXISTS (SELECT 1 FROM dashboard WHERE dashboard.id = annotation.dashboard_id);` + switch driverName { + case Postgres: + sql = `UPDATE annotation + SET dashboard_uid = dashboard.uid + FROM dashboard + WHERE annotation.dashboard_id = dashboard.id + AND annotation.dashboard_id != 0 + AND annotation.dashboard_uid IS NULL;` + case MySQL: + sql = `UPDATE annotation + LEFT JOIN dashboard ON annotation.dashboard_id = dashboard.id + SET annotation.dashboard_uid = dashboard.uid + WHERE annotation.dashboard_uid IS NULL and annotation.dashboard_id != 0;` + } + if _, err := sess.Exec(sql); err != nil { + return fmt.Errorf("failed to set dashboard_uid for annotation: %w", err) + } + + return nil +} diff --git a/public/api-enterprise-spec.json b/public/api-enterprise-spec.json index 82f427d23a5..6f4adb798a2 100644 --- a/public/api-enterprise-spec.json +++ b/public/api-enterprise-spec.json @@ -2818,6 +2818,7 @@ "format": "int64" }, "dashboardId": { + "description": "Deprecated: Use DashboardUID and OrgID instead", "type": "integer", "format": "int64" }, @@ -2899,6 +2900,9 @@ "type": "integer", "format": "int64" }, + "dashboardUID": { + "type": "string" + }, "id": { "type": "integer", "format": "int64" diff --git a/public/api-merged.json b/public/api-merged.json index a2d768b53ed..8ec57956afe 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -13025,6 +13025,7 @@ "format": "int64" }, "dashboardId": { + "description": "Deprecated: Use DashboardUID and OrgID instead", "type": "integer", "format": "int64" }, @@ -13106,6 +13107,9 @@ "type": "integer", "format": "int64" }, + "dashboardUID": { + "type": "string" + }, "id": { "type": "integer", "format": "int64" diff --git a/public/openapi3.json b/public/openapi3.json index 9d9783ab1f8..dfd6ed136b4 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -3075,6 +3075,7 @@ "type": "integer" }, "dashboardId": { + "description": "Deprecated: Use DashboardUID and OrgID instead", "format": "int64", "type": "integer" }, @@ -3156,6 +3157,9 @@ "format": "int64", "type": "integer" }, + "dashboardUID": { + "type": "string" + }, "id": { "format": "int64", "type": "integer"