From 4f2d8a1a3b6d9995f5a7a85f4f0191d0fc8b33e3 Mon Sep 17 00:00:00 2001 From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com> Date: Thu, 9 Jan 2025 10:03:04 +0100 Subject: [PATCH 1/7] Remove access to folders directly from library elements service (#98665) * Remove access to folders directly from library elements service * Lint * Remove unwanted comment --- pkg/services/libraryelements/database.go | 16 ++++++++++------ .../libraryelements_permissions_test.go | 2 +- pkg/services/librarypanels/librarypanels_test.go | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index 123ace40c28..cd64949b7e5 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/search" @@ -288,7 +289,6 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D builder := db.NewSqlBuilder(cfg, features, store.GetDialect(), recursiveQueriesAreSupported) builder.Write(selectLibraryElementDTOWithMeta) builder.Write(", ? as folder_name ", cmd.FolderName) - builder.Write(", COALESCE((SELECT folder.uid FROM folder WHERE folder.id = le.folder_id), '') as folder_uid ") builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect())) metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() // nolint:staticcheck @@ -296,7 +296,6 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D builder.Write(" UNION ") builder.Write(selectLibraryElementDTOWithMeta) builder.Write(", dashboard.title as folder_name ") - builder.Write(", dashboard.uid as folder_uid ") builder.Write(getFromLibraryElementDTOWithMeta(store.GetDialect())) builder.Write(" INNER JOIN dashboard AS dashboard on le.folder_id = dashboard.id AND le.folder_id <> 0") writeParamSelectorSQL(&builder, params...) @@ -322,6 +321,11 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D leDtos := make([]model.LibraryElementDTO, len(libraryElements)) for i, libraryElement := range libraryElements { + // nolint:staticcheck + f, err := l.folderService.Get(c, &folder.GetFolderQuery{OrgID: signedInUser.GetOrgID(), ID: &libraryElement.FolderID, SignedInUser: signedInUser}) + if err != nil { + return []model.LibraryElementDTO{}, err + } var updatedModel json.RawMessage if libraryElement.Kind == int64(model.PanelElement) { updatedModel, err = l.addUidToLibraryPanel(libraryElement.Model, libraryElement.UID) @@ -331,8 +335,8 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D } metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc() - folderUID := libraryElement.FolderUID - if libraryElement.FolderID == 0 { // nolint:staticcheck + folderUID := f.UID + if f.ID == 0 { // nolint:staticcheck folderUID = ac.GeneralFolderUID } leDtos[i] = model.LibraryElementDTO{ @@ -348,8 +352,8 @@ func (l *LibraryElementService) getLibraryElements(c context.Context, store db.D Model: updatedModel, Version: libraryElement.Version, Meta: model.LibraryElementDTOMeta{ - FolderName: libraryElement.FolderName, - FolderUID: libraryElement.FolderUID, + FolderName: f.Title, + FolderUID: folderUID, ConnectedDashboards: libraryElement.ConnectedDashboards, Created: libraryElement.Created, Updated: libraryElement.Updated, diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index 327366115ae..302936dec17 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -95,7 +95,7 @@ func TestLibraryElementPermissionsGeneralFolder(t *testing.T) { result.Result.Meta.UpdatedBy.Name = userInDbName result.Result.Meta.UpdatedBy.AvatarUrl = userInDbAvatar result.Result.Meta.FolderName = "General" - result.Result.Meta.FolderUID = "" + result.Result.Meta.FolderUID = "general" result.Result.FolderUID = "general" sc.reqContext.SignedInUser.OrgRole = testCase.role diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index a83b1b38735..31f6f79b110 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -695,7 +695,7 @@ func getExpected(t *testing.T, res model.LibraryElementDTO, UID string, name str Version: 1, Meta: model.LibraryElementDTOMeta{ FolderName: "General", - FolderUID: "", + FolderUID: res.FolderUID, ConnectedDashboards: 0, Created: res.Meta.Created, Updated: res.Meta.Updated, From 4ae627e420b1ac557d7869235aacaab63d721b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agn=C3=A8s=20Toulet?= <35176601+AgnesToulet@users.noreply.github.com> Date: Thu, 9 Jan 2025 10:19:11 +0100 Subject: [PATCH 2/7] Render Service: Add trace ID to logs (#98006) * Rendering: add logs * fix tests and lint issues * add context to logs * update timed out logs to error logs --- pkg/services/rendering/http_mode.go | 22 ++++++++++++++-------- pkg/services/rendering/plugin_mode.go | 12 ++++++++---- pkg/services/rendering/rendering.go | 22 +++++++++++++--------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 0900f9fd90f..f278acca7d8 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -102,6 +102,8 @@ func (rs *RenderingService) generateImageRendererURL(renderType RenderType, opts } func (rs *RenderingService) doRequestAndWriteToFile(ctx context.Context, renderType RenderType, rendererURL *url.URL, timeoutOpts TimeoutOpts, headers map[string][]string) (*Result, error) { + logger := rs.log.FromContext(ctx) + filePath, err := rs.getNewFilePath(renderType) if err != nil { return nil, err @@ -118,13 +120,13 @@ func (rs *RenderingService) doRequestAndWriteToFile(ctx context.Context, renderT defer func() { if err := resp.Body.Close(); err != nil { - rs.log.Warn("Failed to close response body", "err", err) + logger.Warn("Failed to close response body", "err", err) } }() // if we didn't get a 200 response, something went wrong. if resp.StatusCode != http.StatusOK { - rs.log.Error("Remote rendering request failed", "error", resp.Status, "url", rendererURL.Query().Get("url")) + logger.Error("Remote rendering request failed", "error", resp.Status, "url", rendererURL.Query().Get("url")) return nil, fmt.Errorf("remote rendering request failed, status code: %d, status: %s", resp.StatusCode, resp.Status) } @@ -148,6 +150,8 @@ func (rs *RenderingService) doRequestAndWriteToFile(ctx context.Context, renderT } func (rs *RenderingService) doRequest(ctx context.Context, u *url.URL, headers map[string][]string) (*http.Response, error) { + logger := rs.log.FromContext(ctx) + req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil) if err != nil { return nil, err @@ -159,12 +163,12 @@ func (rs *RenderingService) doRequest(ctx context.Context, u *url.URL, headers m req.Header[k] = v } - rs.log.Debug("calling remote rendering service", "url", u) + logger.Debug("calling remote rendering service", "url", u) // make request to renderer server resp, err := netClient.Do(req) if err != nil { - rs.log.Error("Failed to send request to remote rendering service", "error", err) + logger.Error("Failed to send request to remote rendering service", "error", err) var urlErr *url.Error if errors.As(err, &urlErr) { if urlErr.Timeout() { @@ -178,9 +182,11 @@ func (rs *RenderingService) doRequest(ctx context.Context, u *url.URL, headers m } func (rs *RenderingService) writeResponseToFile(ctx context.Context, resp *http.Response, filePath string) error { + logger := rs.log.FromContext(ctx) + // check for timeout first if errors.Is(ctx.Err(), context.DeadlineExceeded) { - rs.log.Info("Rendering timed out") + logger.Error("Rendering timed out") return ErrTimeout } @@ -193,7 +199,7 @@ func (rs *RenderingService) writeResponseToFile(ctx context.Context, resp *http. defer func() { if err := out.Close(); err != nil && !errors.Is(err, fs.ErrClosed) { // We already close the file explicitly in the non-error path, so shouldn't be a problem - rs.log.Warn("Failed to close file", "path", filePath, "err", err) + logger.Warn("Failed to close file", "path", filePath, "err", err) } }() @@ -201,11 +207,11 @@ func (rs *RenderingService) writeResponseToFile(ctx context.Context, resp *http. if err != nil { // check that we didn't timeout while receiving the response. if errors.Is(ctx.Err(), context.DeadlineExceeded) { - rs.log.Info("Rendering timed out") + logger.Error("Rendering timed out") return ErrTimeout } - rs.log.Error("Remote rendering request failed", "error", err) + logger.Error("Remote rendering request failed", "error", err) return fmt.Errorf("remote rendering request failed: %w", err) } if err := out.Close(); err != nil { diff --git a/pkg/services/rendering/plugin_mode.go b/pkg/services/rendering/plugin_mode.go index 6a69d1c56d0..98eea55eb46 100644 --- a/pkg/services/rendering/plugin_mode.go +++ b/pkg/services/rendering/plugin_mode.go @@ -9,6 +9,8 @@ import ( ) func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType RenderType, renderKey string, opts Opts) (*RenderResult, error) { + logger := rs.log.FromContext(ctx) + // gives plugin some additional time to timeout and return possible errors. ctx, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) defer cancel() @@ -40,7 +42,7 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType Rend AuthToken: rs.Cfg.RendererAuthToken, Encoding: string(renderType), } - rs.log.Debug("Calling renderer plugin", "req", req) + logger.Debug("Calling renderer plugin", "req", req) rc, err := rs.plugin.Client() if err != nil { @@ -48,7 +50,7 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType Rend } rsp, err := rc.Render(ctx, req) if errors.Is(ctx.Err(), context.DeadlineExceeded) { - rs.log.Info("Rendering timed out") + logger.Error("Rendering timed out") return nil, ErrTimeout } if err != nil { @@ -62,6 +64,8 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, renderType Rend } func (rs *RenderingService) renderCSVViaPlugin(ctx context.Context, renderKey string, opts CSVOpts) (*RenderCSVResult, error) { + logger := rs.log.FromContext(ctx) + // gives plugin some additional time to timeout and return possible errors. ctx, cancel := context.WithTimeout(ctx, getRequestTimeout(opts.TimeoutOpts)) defer cancel() @@ -88,7 +92,7 @@ func (rs *RenderingService) renderCSVViaPlugin(ctx context.Context, renderKey st Headers: headers, AuthToken: rs.Cfg.RendererAuthToken, } - rs.log.Debug("Calling renderer plugin", "req", req) + logger.Debug("Calling renderer plugin", "req", req) rc, err := rs.plugin.Client() if err != nil { @@ -98,7 +102,7 @@ func (rs *RenderingService) renderCSVViaPlugin(ctx context.Context, renderKey st rsp, err := rc.RenderCSV(ctx, req) if err != nil { if errors.Is(ctx.Err(), context.DeadlineExceeded) { - rs.log.Info("Rendering timed out") + logger.Error("Rendering timed out") return nil, ErrTimeout } diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index 36799f69a7a..cb25268154f 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -271,8 +271,10 @@ func (rs *RenderingService) Render(ctx context.Context, renderType RenderType, o } func (rs *RenderingService) render(ctx context.Context, renderType RenderType, opts Opts, renderKeyProvider renderKeyProvider) (*RenderResult, error) { + logger := rs.log.FromContext(ctx) + if !rs.IsAvailable(ctx) { - rs.log.Warn("Could not render image, no image renderer found/installed. " + + logger.Warn("Could not render image, no image renderer found/installed. " + "For image rendering support please install the grafana-image-renderer plugin. " + "Read more at https://grafana.com/docs/grafana/latest/administration/image_rendering/") if opts.ErrorRenderUnavailable { @@ -282,7 +284,7 @@ func (rs *RenderingService) render(ctx context.Context, renderType RenderType, o } if int(atomic.LoadInt32(&rs.inProgressCount)) > opts.ConcurrentLimit { - rs.log.Warn("Could not render image, hit the currency limit", "concurrencyLimit", opts.ConcurrentLimit, "path", opts.Path) + logger.Warn("Could not render image, hit the currency limit", "concurrencyLimit", opts.ConcurrentLimit, "path", opts.Path) if opts.ErrorConcurrentLimitReached { return nil, ErrConcurrentLimitReached } @@ -312,7 +314,7 @@ func (rs *RenderingService) render(ctx context.Context, renderType RenderType, o } } - rs.log.Info("Rendering", "path", opts.Path, "userID", opts.AuthOpts.UserID) + logger.Info("Rendering", "path", opts.Path, "userID", opts.AuthOpts.UserID) if math.IsInf(opts.DeviceScaleFactor, 0) || math.IsNaN(opts.DeviceScaleFactor) || opts.DeviceScaleFactor == 0 { opts.DeviceScaleFactor = 1 } @@ -325,10 +327,10 @@ func (rs *RenderingService) render(ctx context.Context, renderType RenderType, o res, err := rs.renderAction(ctx, renderType, renderKey, opts) if err != nil { - rs.log.Error("Failed to render image", "path", opts.Path, "error", err) + logger.Error("Failed to render image", "path", opts.Path, "error", err) return nil, err } - rs.log.Debug("Successfully rendered image", "path", opts.Path) + logger.Debug("Successfully rendered image", "path", opts.Path) return res, nil } @@ -367,15 +369,17 @@ func (rs *RenderingService) SanitizeSVG(ctx context.Context, req *SanitizeSVGReq } func (rs *RenderingService) renderCSV(ctx context.Context, opts CSVOpts, renderKeyProvider renderKeyProvider) (*RenderCSVResult, error) { - if int(atomic.LoadInt32(&rs.inProgressCount)) > opts.ConcurrentLimit { - return nil, ErrConcurrentLimitReached - } + logger := rs.log.FromContext(ctx) if !rs.IsAvailable(ctx) { return nil, ErrRenderUnavailable } - rs.log.Info("Rendering", "path", opts.Path) + if int(atomic.LoadInt32(&rs.inProgressCount)) > opts.ConcurrentLimit { + return nil, ErrConcurrentLimitReached + } + + logger.Info("Rendering", "path", opts.Path) renderKey, err := renderKeyProvider.get(ctx, opts.AuthOpts) if err != nil { return nil, err From fb5783691df348b2f8fc28ea9aeb189a9c9e08aa Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 9 Jan 2025 10:40:18 +0100 Subject: [PATCH 3/7] Zanzana: Fix reconciliation of fixed roles (#98696) Remove "globalReconciler" and reuse the same one but only run them for cluster namespace --- .../accesscontrol/dualwrite/collectors.go | 16 ++- .../dualwrite/global_reconciler.go | 112 ------------------ .../accesscontrol/dualwrite/reconciler.go | 13 +- pkg/services/authz/zanzana/zanzana.go | 2 +- 4 files changed, 22 insertions(+), 121 deletions(-) delete mode 100644 pkg/services/accesscontrol/dualwrite/global_reconciler.go diff --git a/pkg/services/accesscontrol/dualwrite/collectors.go b/pkg/services/accesscontrol/dualwrite/collectors.go index 929c0e2a062..c5fee947a73 100644 --- a/pkg/services/accesscontrol/dualwrite/collectors.go +++ b/pkg/services/accesscontrol/dualwrite/collectors.go @@ -414,8 +414,8 @@ func rolePermissionsCollector(store db.DB) legacyTupleCollector { } } -func fixedRolePermissionsCollector(store db.DB) globalTupleCollector { - return func(ctx context.Context) (map[string]map[string]*openfgav1.TupleKey, error) { +func fixedRolePermissionsCollector(store db.DB) legacyTupleCollector { + return func(ctx context.Context, _ int64) (map[string]map[string]*openfgav1.TupleKey, error) { var query = ` SELECT r.uid as role_uid, p.action, p.kind, p.identifier FROM permission p @@ -457,6 +457,18 @@ func fixedRolePermissionsCollector(store db.DB) globalTupleCollector { tuples[tuple.Object] = make(map[string]*openfgav1.TupleKey) } + // For resource actions on folders we need to merge the tuples into one with combined + // group_resources. + if zanzana.IsFolderResourceTuple(tuple) { + key := tupleStringWithoutCondition(tuple) + if t, ok := tuples[tuple.Object][key]; ok { + zanzana.MergeFolderResourceTuples(t, tuple) + } else { + tuples[tuple.Object][key] = tuple + } + continue + } + tuples[tuple.Object][tuple.String()] = tuple } diff --git a/pkg/services/accesscontrol/dualwrite/global_reconciler.go b/pkg/services/accesscontrol/dualwrite/global_reconciler.go deleted file mode 100644 index 04b381cffa6..00000000000 --- a/pkg/services/accesscontrol/dualwrite/global_reconciler.go +++ /dev/null @@ -1,112 +0,0 @@ -package dualwrite - -import ( - "context" - "fmt" - - openfgav1 "github.com/openfga/api/proto/openfga/v1" - - authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/grafana/grafana/pkg/services/authz/zanzana" -) - -type globalTupleCollector func(ctx context.Context) (map[string]map[string]*openfgav1.TupleKey, error) - -type globalReconciler struct { - name string - globalCollector globalTupleCollector - zanzana zanzanaTupleCollector - client zanzana.Client -} - -func newGlobalReconciler(name string, globalCollector globalTupleCollector, zanzana zanzanaTupleCollector, client zanzana.Client) globalReconciler { - return globalReconciler{name, globalCollector, zanzana, client} -} - -func (r globalReconciler) reconcile(ctx context.Context) error { - namespace := zanzana.ClusterNamespace - - // 1. Fetch grafana resources stored in grafana db. - res, err := r.globalCollector(ctx) - if err != nil { - return fmt.Errorf("failed to collect legacy tuples for %s: %w", r.name, err) - } - - var ( - writes = []*openfgav1.TupleKey{} - deletes = []*openfgav1.TupleKeyWithoutCondition{} - ) - - for object, tuples := range res { - // 2. Fetch all tuples for given object. - // Due to limitations in open fga api we need to collect tuples per object - zanzanaTuples, err := r.zanzana(ctx, r.client, object, namespace) - if err != nil { - return fmt.Errorf("failed to collect zanzanaa tuples for %s: %w", r.name, err) - } - - // 3. Check if tuples from grafana db exists in zanzana and if not add them to writes - for key, t := range tuples { - stored, ok := zanzanaTuples[key] - if !ok { - writes = append(writes, t) - continue - } - - // 4. For folder resource tuples we also need to compare the stored group_resources - if zanzana.IsFolderResourceTuple(t) && t.String() != stored.String() { - deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - User: t.User, - Relation: t.Relation, - Object: t.Object, - }) - - writes = append(writes, t) - } - } - - // 5. Check if tuple from zanzana don't exists in grafana db, if not add them to deletes. - for key, tuple := range zanzanaTuples { - _, ok := tuples[key] - if !ok { - deletes = append(deletes, &openfgav1.TupleKeyWithoutCondition{ - User: tuple.User, - Relation: tuple.Relation, - Object: tuple.Object, - }) - } - } - } - - if len(writes) == 0 && len(deletes) == 0 { - return nil - } - - if len(deletes) > 0 { - err := batch(deletes, 100, func(items []*openfgav1.TupleKeyWithoutCondition) error { - return r.client.Write(ctx, &authzextv1.WriteRequest{ - Namespace: namespace, - Deletes: &authzextv1.WriteRequestDeletes{TupleKeys: zanzana.ToAuthzExtTupleKeysWithoutCondition(items)}, - }) - }) - - if err != nil { - return err - } - } - - if len(writes) > 0 { - err := batch(writes, 100, func(items []*openfgav1.TupleKey) error { - return r.client.Write(ctx, &authzextv1.WriteRequest{ - Namespace: namespace, - Writes: &authzextv1.WriteRequestWrites{TupleKeys: zanzana.ToAuthzExtTupleKeys(items)}, - }) - }) - - if err != nil { - return err - } - } - - return nil -} diff --git a/pkg/services/accesscontrol/dualwrite/reconciler.go b/pkg/services/accesscontrol/dualwrite/reconciler.go index 88461ed17bf..ac2c979c4d6 100644 --- a/pkg/services/accesscontrol/dualwrite/reconciler.go +++ b/pkg/services/accesscontrol/dualwrite/reconciler.go @@ -32,8 +32,9 @@ type ZanzanaReconciler struct { lock *serverlock.ServerLockService // reconcilers are migrations that tries to reconcile the state of grafana db to zanzana store. // These are run periodically to try to maintain a consistent state. - reconcilers []resourceReconciler - globalReconcilers []globalReconciler + reconcilers []resourceReconciler + // globalReconcilers are reconcilers that should only run for cluster namespace + globalReconcilers []resourceReconciler } func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, lock *serverlock.ServerLockService, folderService folder.Service) *ZanzanaReconciler { @@ -93,11 +94,11 @@ func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, client, ), }, - globalReconcilers: []globalReconciler{ - newGlobalReconciler( + globalReconcilers: []resourceReconciler{ + newResourceReconciler( "fixed role pemissions", fixedRolePermissionsCollector(store), - zanzanaCollector([]string{zanzana.RelationAssignee}), + zanzanaCollector(zanzana.RelationsFolder), client, ), }, @@ -146,7 +147,7 @@ func (r *ZanzanaReconciler) reconcile(ctx context.Context) { runGlobal := func(ctx context.Context) { for _, reconciler := range r.globalReconcilers { r.log.Debug("Performing zanzana reconciliation", "reconciler", reconciler.name) - if err := reconciler.reconcile(ctx); err != nil { + if err := reconciler.reconcile(ctx, zanzana.ClusterNamespace); err != nil { r.log.Warn("Failed to perform reconciliation for resource", "err", err) } } diff --git a/pkg/services/authz/zanzana/zanzana.go b/pkg/services/authz/zanzana/zanzana.go index 696f75c1e82..26b82d0151a 100644 --- a/pkg/services/authz/zanzana/zanzana.go +++ b/pkg/services/authz/zanzana/zanzana.go @@ -49,8 +49,8 @@ const ( var ( RelationsFolder = common.RelationsFolder - RelationsFolderResource = common.RelationsFolder RelationsResouce = common.RelationsResource + RelationsFolderResource = common.RelationsFolderResource ) const ( From f04d72a49c52fbd251290798dbeba6c68901ae39 Mon Sep 17 00:00:00 2001 From: Alexa V <239999+axelavargas@users.noreply.github.com> Date: Thu, 9 Jan 2025 10:51:53 +0100 Subject: [PATCH 4/7] Dashboards: SchemaV2 - Fix default datasource logic (#98573) Get defaultDatasource config option from config.bootData.settings.defaultDatasource --- .../transformSceneToSaveModelSchemaV2.ts | 2 +- .../api/ResponseTransformers.test.ts | 62 ++++++++++++++++++- .../dashboard/api/ResponseTransformers.ts | 32 ++++++---- public/app/features/dashboard/api/v0.test.ts | 2 + public/app/features/dashboard/api/v2.test.ts | 5 +- 5 files changed, 89 insertions(+), 14 deletions(-) diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts index 12ee69e9f69..605cb307c82 100644 --- a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts +++ b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts @@ -443,7 +443,7 @@ export function getAnnotationQueryKind(annotationQuery: AnnotationQuery): string } } -function getDefaultDataSourceRef(): DataSourceRef | undefined { +export function getDefaultDataSourceRef(): DataSourceRef | undefined { // we need to return the default datasource configured in the BootConfig const defaultDatasource = config.bootData.settings.defaultDatasource; diff --git a/public/app/features/dashboard/api/ResponseTransformers.test.ts b/public/app/features/dashboard/api/ResponseTransformers.test.ts index 24074a3f845..f7c5c66328b 100644 --- a/public/app/features/dashboard/api/ResponseTransformers.test.ts +++ b/public/app/features/dashboard/api/ResponseTransformers.test.ts @@ -9,12 +9,72 @@ import { AnnoKeyUpdatedBy, AnnoKeyUpdatedTimestamp, } from 'app/features/apiserver/types'; +import { getDefaultDataSourceRef } from 'app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2'; import { DashboardDataDTO, DashboardDTO } from 'app/types'; -import { getPanelQueries, ResponseTransformers } from './ResponseTransformers'; +import { getDefaultDatasource, getPanelQueries, ResponseTransformers } from './ResponseTransformers'; import { DashboardWithAccessInfo } from './types'; +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + config: { + ...jest.requireActual('@grafana/runtime').config, + bootData: { + ...jest.requireActual('@grafana/runtime').config.bootData, + settings: { + ...jest.requireActual('@grafana/runtime').config.bootData.settings, + datasources: { + PromTest: { + uid: 'xyz-abc', + name: 'PromTest', + id: 'prometheus', + meta: { + id: 'prometheus', + name: 'PromTest', + type: 'datasource', + }, + isDefault: true, + apiVersion: 'v2', + }, + '-- Grafana --': { + uid: 'grafana', + name: '-- Grafana --', + id: 'grafana', + meta: { + id: 'grafana', + name: '-- Grafana --', + type: 'datasource', + }, + isDefault: false, + }, + }, + + defaultDatasource: 'PromTest', + }, + }, + }, +})); + describe('ResponseTransformers', () => { + describe('getDefaultDataSource', () => { + it('should return prometheus as default', () => { + expect(getDefaultDatasource()).toEqual({ + apiVersion: 'v2', + uid: 'PromTest', + type: 'prometheus', + }); + }); + }); + + describe('getDefaultDataSourceRef', () => { + it('should return prometheus as default', () => { + expect(getDefaultDataSourceRef()).toEqual({ + uid: 'PromTest', + type: 'prometheus', + }); + }); + }); + describe('v1 -> v2 transformation', () => { it('should transform DashboardDTO to DashboardWithAccessInfo', () => { const dashboardV1: DashboardDataDTO = { diff --git a/public/app/features/dashboard/api/ResponseTransformers.ts b/public/app/features/dashboard/api/ResponseTransformers.ts index a5d454bbdf3..1f46a08b280 100644 --- a/public/app/features/dashboard/api/ResponseTransformers.ts +++ b/public/app/features/dashboard/api/ResponseTransformers.ts @@ -22,6 +22,7 @@ import { AnnoKeyUpdatedBy, AnnoKeyUpdatedTimestamp, } from 'app/features/apiserver/types'; +import { getDefaultDataSourceRef } from 'app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2'; import { transformCursorSyncV2ToV1 } from 'app/features/dashboard-scene/serialization/transformToV1TypesUtils'; import { transformCursorSynctoEnum, @@ -226,6 +227,11 @@ function getElementsFromPanels(panels: Panel[]): [DashboardV2Spec['elements'], D // iterate over panels for (const p of panels) { + // FIXME: for now we should skip row panels + if (p.type === 'row') { + continue; + } + const queries = getPanelQueries( (p.targets as unknown as DataQuery[]) || [], p.datasource || getDefaultDatasource() @@ -291,20 +297,23 @@ function getElementsFromPanels(panels: Panel[]): [DashboardV2Spec['elements'], D } function getDefaultDatasourceType() { - const datasources = config.datasources; - // find default datasource in datasources - return Object.values(datasources).find((ds) => ds.isDefault)!.type; + // if there is no default datasource, return 'grafana' as default + return getDefaultDataSourceRef()?.type ?? 'grafana'; } -function getDefaultDatasource(): DataSourceRef { - const datasources = config.datasources; +export function getDefaultDatasource(): DataSourceRef { + const configDefaultDS = getDefaultDataSourceRef() ?? { type: 'grafana', uid: '-- Grafana --' }; + + if (configDefaultDS.uid && !configDefaultDS.apiVersion) { + // get api version from config + const dsInstance = config.bootData.settings.datasources[configDefaultDS.uid]; + configDefaultDS.apiVersion = dsInstance.apiVersion ?? undefined; + } - // find default datasource in datasources - const defaultDs = Object.values(datasources).find((ds) => ds.isDefault)!; return { - apiVersion: defaultDs.apiVersion, - type: defaultDs.type, - uid: defaultDs.uid, + apiVersion: configDefaultDS.apiVersion, + type: configDefaultDS.type, + uid: configDefaultDS.uid, }; } @@ -407,7 +416,8 @@ function getVariables(vars: VariableModel[]): DashboardV2Spec['variables'] { variables.push(dv); break; default: - throw new Error(`Variable transformation not implemented: ${v.type}`); + // do not throw error, just log it + console.error(`Variable transformation not implemented: ${v.type}`); } } return variables; diff --git a/public/app/features/dashboard/api/v0.test.ts b/public/app/features/dashboard/api/v0.test.ts index c21e103bb10..82d82ecc0d3 100644 --- a/public/app/features/dashboard/api/v0.test.ts +++ b/public/app/features/dashboard/api/v0.test.ts @@ -88,12 +88,14 @@ const saveDashboardResponse = { }, }; jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => ({ get: () => mockDashboardDto, put: jest.fn().mockResolvedValue(saveDashboardResponse), post: jest.fn().mockResolvedValue(saveDashboardResponse), }), config: { + ...jest.requireActual('@grafana/runtime').config, buildInfo: { version: '11.5.0-test-version-string', }, diff --git a/public/app/features/dashboard/api/v2.test.ts b/public/app/features/dashboard/api/v2.test.ts index ec0d744a307..d9420a36b37 100644 --- a/public/app/features/dashboard/api/v2.test.ts +++ b/public/app/features/dashboard/api/v2.test.ts @@ -27,10 +27,13 @@ const mockDashboardDto: DashboardWithAccessInfo = { }; jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => ({ get: () => mockDashboardDto, }), - config: {}, + config: { + ...jest.requireActual('@grafana/runtime').config, + }, })); jest.mock('app/features/live/dashboard/dashboardWatcher', () => ({ From 42894ba8407f341dc8710567405f38a81ead17f7 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Thu, 9 Jan 2025 10:02:15 +0000 Subject: [PATCH 5/7] Alerting: Use common i18n phrases for cancel and save (#98600) --- .betterer.results | 44 ++++++------------- .../contact-points/components/Modals.tsx | 3 +- .../components/export/FileExportPreview.tsx | 3 +- .../receivers/AlertInstanceModalSelector.tsx | 3 +- .../components/receivers/GlobalConfigForm.tsx | 3 +- .../receivers/form/ReceiverForm.tsx | 3 +- .../form/fields/TemplateSelector.tsx | 4 +- .../rule-editor/DashboardPicker.tsx | 3 +- .../alert-rule-form/AlertRuleForm.tsx | 3 +- .../alert-rule-form/ModifyExportRuleForm.tsx | 3 +- .../rule-editor/labels/LabelsField.tsx | 4 +- .../QueryAndExpressionsStep.tsx | 2 +- .../components/rules/EditRuleGroupModal.tsx | 3 +- .../settings/AlertmanagerConfig.tsx | 5 ++- .../components/silences/SilencesEditor.tsx | 3 +- 15 files changed, 42 insertions(+), 47 deletions(-) diff --git a/.betterer.results b/.betterer.results index 360862b8111..ba09360a954 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1596,8 +1596,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "5"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/alerting/unified/components/contact-points/components/UnusedBadge.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -1611,8 +1610,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "8"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], "public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -1797,14 +1795,12 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "4"], [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "6"] ], "public/app/features/alerting/unified/components/receivers/GlobalConfigForm.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "2"] ], "public/app/features/alerting/unified/components/receivers/PayloadEditor.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -1899,8 +1895,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], [0, 0, 0, "No untranslated strings. Wrap text with ", "8"], [0, 0, 0, "No untranslated strings. Wrap text with ", "9"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "10"], - [0, 0, 0, "Unexpected any. Specify a different type.", "11"] + [0, 0, 0, "Unexpected any. Specify a different type.", "10"] ], "public/app/features/alerting/unified/components/receivers/form/TestContactPointModal.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -1946,9 +1941,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "6"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/alerting/unified/components/rule-editor/AlertRuleNameInput.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2003,8 +1996,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "12"], [0, 0, 0, "No untranslated strings. Wrap text with ", "13"], [0, 0, 0, "No untranslated strings. Wrap text with ", "14"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "15"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "16"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "15"] ], "public/app/features/alerting/unified/components/rule-editor/ExpressionEditor.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], @@ -2118,14 +2110,12 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "1"], [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "5"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "2"] ], "public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2168,8 +2158,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "8"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], "public/app/features/alerting/unified/components/rule-editor/labels/LabelsFieldInForm.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2214,8 +2203,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "3"], [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "6"] ], "public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/SimpleCondition.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] @@ -2325,8 +2313,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], [0, 0, 0, "No untranslated strings. Wrap text with ", "8"], [0, 0, 0, "No untranslated strings. Wrap text with ", "9"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "10"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "11"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "10"] ], "public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v1.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2499,9 +2486,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "5"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "5"] ], "public/app/features/alerting/unified/components/settings/ConfigurationDrawer.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2566,8 +2551,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "5"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "6"], [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "8"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "9"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "8"] ], "public/app/features/alerting/unified/components/silences/SilencesFilter.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], diff --git a/public/app/features/alerting/unified/components/contact-points/components/Modals.tsx b/public/app/features/alerting/unified/components/contact-points/components/Modals.tsx index d1d91762662..bdba364879c 100644 --- a/public/app/features/alerting/unified/components/contact-points/components/Modals.tsx +++ b/public/app/features/alerting/unified/components/contact-points/components/Modals.tsx @@ -1,6 +1,7 @@ import { useCallback, useMemo, useState } from 'react'; import { Button, Modal, ModalProps } from '@grafana/ui'; +import { Trans } from 'app/core/internationalization'; import { stringifyErrorLike } from '../../../utils/misc'; @@ -63,7 +64,7 @@ export const useDeleteContactPointModal = ( {isLoading ? 'Deleting...' : 'Yes, delete contact point'} diff --git a/public/app/features/alerting/unified/components/export/FileExportPreview.tsx b/public/app/features/alerting/unified/components/export/FileExportPreview.tsx index 8f23aaccdc9..5670ddc8376 100644 --- a/public/app/features/alerting/unified/components/export/FileExportPreview.tsx +++ b/public/app/features/alerting/unified/components/export/FileExportPreview.tsx @@ -6,6 +6,7 @@ import AutoSizer from 'react-virtualized-auto-sizer'; import { GrafanaTheme2 } from '@grafana/data'; import { Alert, Button, ClipboardButton, CodeEditor, TextLink, useStyles2 } from '@grafana/ui'; +import { Trans } from 'app/core/internationalization'; import { ExportFormats, ExportProvider, ProvisioningType, allGrafanaExportProviders } from './providers'; @@ -59,7 +60,7 @@ export function FileExportPreview({ format, textDefinition, downloadFileName, on
textDefinition}> Copy code diff --git a/public/app/features/alerting/unified/components/receivers/AlertInstanceModalSelector.tsx b/public/app/features/alerting/unified/components/receivers/AlertInstanceModalSelector.tsx index d67a709b4f6..05ace6725db 100644 --- a/public/app/features/alerting/unified/components/receivers/AlertInstanceModalSelector.tsx +++ b/public/app/features/alerting/unified/components/receivers/AlertInstanceModalSelector.tsx @@ -15,6 +15,7 @@ import { clearButtonStyles, useStyles2, } from '@grafana/ui'; +import { Trans } from 'app/core/internationalization'; import { AlertmanagerAlert, TestTemplateAlert } from 'app/plugins/datasource/alertmanager/types'; import { alertmanagerApi } from '../../api/alertmanagerApi'; @@ -259,7 +260,7 @@ export function AlertInstanceModalSelector({
diff --git a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx index 7b44968ee36..14eea33da9d 100644 --- a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx @@ -17,6 +17,7 @@ import { clearButtonStyles, useStyles2, } from '@grafana/ui'; +import { Trans } from 'app/core/internationalization'; import { DashboardModel } from '../../../../dashboard/state/DashboardModel'; import { dashboardApi } from '../../api/dashboardApi'; @@ -243,7 +244,7 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis {existing ? ( diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index 7b0f94b9866..f43e3a14b67 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -652,7 +652,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P {isPreviewLoading && ( )} {!isPreviewLoading && ( diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index f6c4a116005..76427e7d5fa 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -6,6 +6,7 @@ import { FieldValues, FormProvider, RegisterOptions, useForm, useFormContext } f import { GrafanaTheme2 } from '@grafana/data'; import { Alert, Badge, Button, Field, Input, Label, LinkButton, Modal, Stack, useStyles2 } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; +import { Trans } from 'app/core/internationalization'; import { dispatch } from 'app/store/store'; import { CombinedRuleGroup, CombinedRuleNamespace, RuleGroupIdentifier } from 'app/types/unified-alerting'; import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; @@ -384,7 +385,7 @@ export function EditRuleGroupModal(props: ModalProps): React.ReactElement { onClick={() => onClose(false)} fill="outline" > - Cancel + Cancel {!readOnly && ( )} diff --git a/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx b/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx index c08f87076be..c72cb1e5521 100644 --- a/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencesEditor.tsx @@ -27,6 +27,7 @@ import { useStyles2, withErrorBoundary, } from '@grafana/ui'; +import { Trans } from 'app/core/internationalization'; import { SilenceCreatedResponse, alertSilencesApi } from 'app/features/alerting/unified/api/alertSilencesApi'; import { MATCHER_ALERT_RULE_UID } from 'app/features/alerting/unified/utils/constants'; import { GRAFANA_RULES_SOURCE_NAME, getDatasourceAPIUid } from 'app/features/alerting/unified/utils/datasource'; @@ -258,7 +259,7 @@ export const SilencesEditor = ({ )} {!isLoading && } - Cancel + Cancel From f75bbe9f43ba33d111d09f1d6cfe1a1853bb5545 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Thu, 9 Jan 2025 10:02:52 +0000 Subject: [PATCH 6/7] Alerting: Add self closing comp rule to alerting eslint config (#98603) --- eslint.config.js | 2 ++ .../unified/components/permissions/ManagePermissions.tsx | 2 +- .../alerting/unified/components/receivers/TemplateForm.tsx | 2 +- .../alerting/unified/components/receivers/TemplatesTable.tsx | 4 ++-- .../components/receivers/form/GenerateAlertDataModal.tsx | 2 +- .../components/receivers/form/fields/KeyValueMapInput.tsx | 2 +- .../alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx | 4 ++-- .../notificaton-preview/NotificationPreviewByAlertManager.tsx | 4 ++-- .../unified/components/rules/Filter/RulesFilter.v2.tsx | 4 ++-- .../unified/components/silences/SilencedAlertsTable.tsx | 4 ++-- .../unified/components/silences/SilencedAlertsTableRow.tsx | 2 +- .../app/features/alerting/unified/rule-list/GroupedView.tsx | 2 +- 12 files changed, 18 insertions(+), 16 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 7c1b5d23fb4..17be1ebc4e0 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -249,6 +249,7 @@ module.exports = [ name: 'grafana/alerting-overrides', plugins: { unicorn: unicornPlugin, + react: reactPlugin, }, files: ['public/app/features/alerting/**/*.{ts,tsx,js,jsx}'], rules: { @@ -256,6 +257,7 @@ module.exports = [ 'dot-notation': 'error', 'prefer-const': 'error', 'react/no-unused-prop-types': 'error', + 'react/self-closing-comp': 'error', 'unicorn/no-unused-properties': 'error', }, }, diff --git a/public/app/features/alerting/unified/components/permissions/ManagePermissions.tsx b/public/app/features/alerting/unified/components/permissions/ManagePermissions.tsx index 6bbe527fafd..aa82b4b794c 100644 --- a/public/app/features/alerting/unified/components/permissions/ManagePermissions.tsx +++ b/public/app/features/alerting/unified/components/permissions/ManagePermissions.tsx @@ -30,7 +30,7 @@ export const ManagePermissionsDrawer = ({ const defaultTitle = t('alerting.manage-permissions.title', 'Manage permissions'); return ( - + ); }; diff --git a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx index d9304ae90c6..96ca2dedd46 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx @@ -324,7 +324,7 @@ export const TemplateForm = ({ originalTemplate, prefill, alertmanager }: Props) {isGrafanaAlertManager && ( <>
-
+
{ - + Template group {isExpanded && ( - + -
+
{alerts.length > 0 && (
Review alert data to add to the payload:
diff --git a/public/app/features/alerting/unified/components/receivers/form/fields/KeyValueMapInput.tsx b/public/app/features/alerting/unified/components/receivers/form/fields/KeyValueMapInput.tsx index 0bd18538cf1..5a1bccdd34e 100644 --- a/public/app/features/alerting/unified/components/receivers/form/fields/KeyValueMapInput.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/fields/KeyValueMapInput.tsx @@ -47,7 +47,7 @@ export const KeyValueMapInput = ({ value, onChange, readOnly = false }: Props) = Name Value - {!readOnly && } + {!readOnly && } diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx index 521a4c8e7c5..1d6b7c0b3f3 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/AlertManagerRouting.tsx @@ -40,13 +40,13 @@ export function AlertManagerManualRouting({ alertManager }: AlertManagerManualRo return ( -
+
Alertmanager: Alert manager logo {alertManagerName}
-
+
diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewByAlertManager.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewByAlertManager.tsx index 813187556cb..ddf78bef3fe 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewByAlertManager.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewByAlertManager.tsx @@ -49,13 +49,13 @@ function NotificationPreviewByAlertManager({
{!onlyOneAM && ( -
+
Alertmanager: {alertManagerSource.name}
-
+
)} diff --git a/public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v2.tsx b/public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v2.tsx index 7faa69f1f83..1c682cc689e 100644 --- a/public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v2.tsx +++ b/public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v2.tsx @@ -84,7 +84,7 @@ const FilterOptions = () => { - + {}}> +