From 0ec3110d41adbf58afdd82a9a6ecd709588ec4df Mon Sep 17 00:00:00 2001 From: Collin Fingar Date: Wed, 24 Sep 2025 12:16:23 -0400 Subject: [PATCH 01/33] Dashboard Panel: Expose Image Renderer URL to Copy (#111502) * Dashboard Panel: Expose Image Renderer URL to Copy * Fixed i18n * Feedback fixes * Tweaked analytics event --- .../panel-share/SharePanelInternally.test.tsx | 14 ++++++++++++++ .../sharing/panel-share/SharePanelPreview.tsx | 12 +++++++++++- .../features/dashboard-scene/utils/interactions.ts | 3 +++ public/locales/en-US/grafana.json | 1 + 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/public/app/features/dashboard-scene/sharing/panel-share/SharePanelInternally.test.tsx b/public/app/features/dashboard-scene/sharing/panel-share/SharePanelInternally.test.tsx index a02ed3e5a1d..8f26a824e19 100644 --- a/public/app/features/dashboard-scene/sharing/panel-share/SharePanelInternally.test.tsx +++ b/public/app/features/dashboard-scene/sharing/panel-share/SharePanelInternally.test.tsx @@ -30,6 +30,7 @@ describe('SharePanelInternally', () => { expect(screen.getByPlaceholderText('1')).toBeDisabled(); // Scale factor input expect(screen.getByRole('button', { name: /generate image/i })).toBeDisabled(); expect(screen.getByRole('button', { name: /download image/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /copy image link/i })).toBeDisabled(); }); it('should enable all image generation inputs when renderer is available', async () => { @@ -55,6 +56,19 @@ describe('SharePanelInternally', () => { expect(screen.getByRole('button', { name: /generate image/i })).toBeEnabled(); expect(screen.getByRole('button', { name: /download image/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /copy image link/i })).toBeEnabled(); + }); + + it('should copy image link when copy image link button is clicked', async () => { + document.execCommand = jest.fn(); + + config.rendererAvailable = true; + buildAndRenderScenario(); + + const copyImageLinkButton = screen.getByRole('button', { name: /copy image link/i }); + await userEvent.click(copyImageLinkButton); + + expect(document.execCommand).toHaveBeenCalledWith('copy'); }); }); diff --git a/public/app/features/dashboard-scene/sharing/panel-share/SharePanelPreview.tsx b/public/app/features/dashboard-scene/sharing/panel-share/SharePanelPreview.tsx index 57ea1bde7af..2fdc7bb89b2 100644 --- a/public/app/features/dashboard-scene/sharing/panel-share/SharePanelPreview.tsx +++ b/public/app/features/dashboard-scene/sharing/panel-share/SharePanelPreview.tsx @@ -8,7 +8,7 @@ import { lastValueFrom } from 'rxjs'; import { GrafanaTheme2, UrlQueryMap } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { config, getBackendSrv } from '@grafana/runtime'; -import { Button, Field, FieldSet, Icon, Input, Stack, Text, Tooltip, useStyles2 } from '@grafana/ui'; +import { Button, ClipboardButton, Field, FieldSet, Icon, Input, Stack, Text, Tooltip, useStyles2 } from '@grafana/ui'; import { DashboardInteractions } from '../../utils/interactions'; import { ImagePreview } from '../components/ImagePreview'; @@ -196,6 +196,16 @@ export function SharePanelPreview({ title, imageUrl, buildUrl, disabled, theme } > Download image + imageUrl} + onClipboardCopy={() => DashboardInteractions.copyImageUrlClicked({ shareResource: 'panel' })} + > + Copy image link + {disabled && ( diff --git a/public/app/features/dashboard-scene/utils/interactions.ts b/public/app/features/dashboard-scene/utils/interactions.ts index 9990c4e197e..67615b083d1 100644 --- a/public/app/features/dashboard-scene/utils/interactions.ts +++ b/public/app/features/dashboard-scene/utils/interactions.ts @@ -187,6 +187,9 @@ export const DashboardInteractions = { downloadDashboardImageClicked: (properties?: Record) => { reportDashboardInteraction('dashboard_image_downloaded', properties); }, + copyImageUrlClicked: (properties?: Record) => { + reportDashboardInteraction('dashboard_image_url_copied', properties); + }, }; const reportDashboardInteraction: typeof reportInteraction = (name, properties) => { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 95d771db840..c9dfbd95d55 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -9347,6 +9347,7 @@ }, "share-panel": { "config-description": "Create a personalized, direct link to share your panel within your organization, with the following customization settings:", + "copy-image-link": "Copy image link", "download-image": "Download image", "render-image": "Generate image" } From c59aff3bb9b592fedbec47a93be539cee57e2bd5 Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Wed, 24 Sep 2025 19:22:43 +0300 Subject: [PATCH 02/33] Provisioning: Disable editing options for read only repositories (#111547) --- .../dashgrid/DashboardEmpty.test.tsx | 33 +++++++++++++++++++ .../dashboard/dashgrid/DashboardEmpty.tsx | 12 +++++-- .../GettingStarted/GettingStartedPage.tsx | 20 ++++------- .../features/provisioning/utils/repository.ts | 4 +-- public/locales/en-US/grafana.json | 4 +-- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/DashboardEmpty.test.tsx b/public/app/features/dashboard/dashgrid/DashboardEmpty.test.tsx index 7dd3f6c4165..06f1438646d 100644 --- a/public/app/features/dashboard/dashgrid/DashboardEmpty.test.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardEmpty.test.tsx @@ -28,6 +28,18 @@ jest.mock('app/features/dashboard/utils/dashboard', () => ({ onAddLibraryPanel: jest.fn(), })); +jest.mock('app/features/provisioning/hooks/useGetResourceRepositoryView', () => ({ + useGetResourceRepositoryView: jest.fn(() => ({ + isReadOnlyRepo: false, + isInstanceManaged: false, + isLoading: false, + })), +})); + +const mockUseGetResourceRepositoryView = jest.mocked( + require('app/features/provisioning/hooks/useGetResourceRepositoryView').useGetResourceRepositoryView +); + function setup(options?: Partial) { const props = { dashboard: createDashboardModelFixture(defaultDashboard), @@ -40,6 +52,12 @@ function setup(options?: Partial) { beforeEach(() => { jest.clearAllMocks(); + // Reset the mock to default state + mockUseGetResourceRepositoryView.mockReturnValue({ + isReadOnlyRepo: false, + isInstanceManaged: false, + isLoading: false, + }); }); it('renders page with correct title for an empty dashboard', () => { @@ -117,3 +135,18 @@ it('renders page without Add Widget button when feature flag is disabled', () => expect(screen.getByRole('button', { name: 'Add library panel' })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Add widget' })).not.toBeInTheDocument(); }); + +it('renders with buttons disabled when repository is read-only', () => { + // Mock the hook to return read-only repository + mockUseGetResourceRepositoryView.mockReturnValue({ + isReadOnlyRepo: true, + isInstanceManaged: false, + isLoading: false, + }); + + setup({ canCreate: true }); + + expect(screen.getByRole('button', { name: 'Add visualization' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Import dashboard' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Add library panel' })).toBeDisabled(); +}); diff --git a/public/app/features/dashboard/dashgrid/DashboardEmpty.tsx b/public/app/features/dashboard/dashgrid/DashboardEmpty.tsx index 9555bea733e..f5f8a6856d1 100644 --- a/public/app/features/dashboard/dashgrid/DashboardEmpty.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardEmpty.tsx @@ -14,6 +14,7 @@ import { import { buildPanelEditScene } from 'app/features/dashboard-scene/panel-edit/PanelEditor'; import { DashboardScene } from 'app/features/dashboard-scene/scene/DashboardScene'; import { DashboardInteractions } from 'app/features/dashboard-scene/utils/interactions'; +import { useGetResourceRepositoryView } from 'app/features/provisioning/hooks/useGetResourceRepositoryView'; import { useDispatch, useSelector } from 'app/types/store'; import { setInitialDatasource } from '../state/reducers'; @@ -28,6 +29,11 @@ const DashboardEmpty = ({ dashboard, canCreate }: Props) => { const dispatch = useDispatch(); const initialDatasource = useSelector((state) => state.dashboard.initialDatasource); + // Get repository information to check if it's read-only + const { isReadOnlyRepo } = useGetResourceRepositoryView({ + folderName: dashboard instanceof DashboardScene ? dashboard.state.meta.folderUid : dashboard.meta.folderUid, + }); + const onAddVisualization = () => { let id; if (dashboard instanceof DashboardScene) { @@ -77,7 +83,7 @@ const DashboardEmpty = ({ dashboard, canCreate }: Props) => { icon="plus" data-testid={selectors.pages.AddDashboard.itemButton('Create new panel button')} onClick={onAddVisualization} - disabled={!canCreate} + disabled={!canCreate || isReadOnlyRepo} > Add visualization @@ -101,7 +107,7 @@ const DashboardEmpty = ({ dashboard, canCreate }: Props) => { fill="outline" data-testid={selectors.pages.AddDashboard.itemButton('Add a panel from the panel library button')} onClick={onAddLibraryPanel} - disabled={!canCreate || isProvisioned} + disabled={!canCreate || isProvisioned || isReadOnlyRepo} > Add library panel @@ -131,7 +137,7 @@ const DashboardEmpty = ({ dashboard, canCreate }: Props) => { DashboardInteractions.emptyDashboardButtonClicked({ item: 'import_dashboard' }); onImportDashboard(); }} - disabled={!canCreate} + disabled={!canCreate || isReadOnlyRepo} > Import dashboard diff --git a/public/app/features/provisioning/GettingStarted/GettingStartedPage.tsx b/public/app/features/provisioning/GettingStarted/GettingStartedPage.tsx index ff9999698ba..0a9d6822780 100644 --- a/public/app/features/provisioning/GettingStarted/GettingStartedPage.tsx +++ b/public/app/features/provisioning/GettingStarted/GettingStartedPage.tsx @@ -1,7 +1,7 @@ import { GrafanaEdition } from '@grafana/data/internal'; import { Trans, t } from '@grafana/i18n'; import { config } from '@grafana/runtime'; -import { Box, Text, TextLink } from '@grafana/ui'; +import { Alert, Stack, Text, TextLink } from '@grafana/ui'; import { Repository } from 'app/api/clients/provisioning/v0alpha1'; import { Page } from 'app/core/components/Page/Page'; @@ -24,8 +24,10 @@ export default function GettingStartedPage({ items }: Props) { }} > - - + + + + ); @@ -39,15 +41,7 @@ function Banner() { } return ( - + This feature is currently under active development. For the best experience and latest improvements, we @@ -58,6 +52,6 @@ function Banner() { of Grafana. - + ); } diff --git a/public/app/features/provisioning/utils/repository.ts b/public/app/features/provisioning/utils/repository.ts index 0f3d39728f6..f8990c14897 100644 --- a/public/app/features/provisioning/utils/repository.ts +++ b/public/app/features/provisioning/utils/repository.ts @@ -25,11 +25,11 @@ export const getReadOnlyTooltipText = ({ isLocal = false }) => { return isLocal ? t( 'provisioning.read-only-local-tooltip', - 'This folder is read-only and provisioned through file provisioning. To make any changes in the folder, update the connected file repository. To modify the folder settings go to Administration > Provisioning > Repositories.' + 'This resource is read-only and provisioned through file provisioning. To make any changes, update the connected repository. To modify the settings go to Administration > Provisioning > Repositories.' ) : t( 'provisioning.read-only-remote-tooltip', - 'This folder is read-only and provisioned through Git. To make any changes in the folder, update the connected repository. To modify the folder settings go to Administration > Provisioning > Repositories.' + 'This resource is read-only and provisioned through Git. To make any changes, update the connected repository. To modify the settings go to Administration > Provisioning > Repositories.' ); }; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index c9dfbd95d55..298ac1a1e84 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11526,8 +11526,8 @@ "subtitle": "Use this option if you want to sync and manage your entire Grafana instance through external storage." } }, - "read-only-local-tooltip": "This folder is read-only and provisioned through file provisioning. To make any changes in the folder, update the connected file repository. To modify the folder settings go to Administration > Provisioning > Repositories.", - "read-only-remote-tooltip": "This folder is read-only and provisioned through Git. To make any changes in the folder, update the connected repository. To modify the folder settings go to Administration > Provisioning > Repositories.", + "read-only-local-tooltip": "This resource is read-only and provisioned through file provisioning. To make any changes, update the connected repository. To modify the settings go to Administration > Provisioning > Repositories.", + "read-only-remote-tooltip": "This resource is read-only and provisioned through Git. To make any changes, update the connected repository. To modify the settings go to Administration > Provisioning > Repositories.", "recent-jobs": { "active-jobs": "active jobs", "column-action": "Action", From 61bf3d9899bdcad08bf2797ae8fd8a7675816ff0 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 24 Sep 2025 19:01:45 +0200 Subject: [PATCH 03/33] Alerting: Use route matching hook (#111028) --- eslint-suppressions.json | 5 - .../hooks/useMatchPolicies.test.ts | 74 +++ .../hooks/useMatchPolicies.ts | 99 ++-- .../notificationPolicies/utils.test.ts | 8 +- .../src/grafana/notificationPolicies/utils.ts | 14 +- packages/grafana-alerting/src/unstable.ts | 5 +- .../__mocks__/useRouteGroupsMatcher.ts | 6 +- .../NotificationPoliciesPage.test.tsx.snap | 458 +++++++++++++++--- .../alerting/unified/api/alertRuleApi.ts | 12 +- .../components/contact-points/utils.ts | 9 +- .../export/GrafanaModifyExport.test.tsx | 6 +- .../notification-policies/Matchers.tsx | 2 +- .../NotificationPoliciesList.tsx | 8 +- .../notification-policies/Policy.tsx | 15 +- .../useNotificationPolicyRoute.test.tsx.snap | 2 + .../useNotificationPolicyRoute.ts | 5 +- .../SimplifiedRuleEditor.test.tsx | 7 +- .../notificaton-preview/ConnectionLine.tsx | 22 + .../notificaton-preview/ContactPointGroup.tsx | 158 ++++++ .../JourneyPolicyCard.test.tsx | 103 ++++ .../notificaton-preview/JourneyPolicyCard.tsx | 112 +++++ .../notificaton-preview/MatchDetails.tsx | 56 +++ .../NotificationPolicyDrawer.tsx | 110 +++++ .../NotificationPolicyMatchers.tsx | 38 +- .../NotificationPreview.test.tsx | 401 +++------------ .../NotificationPreview.tsx | 88 +++- .../NotificationPreviewByAlertManager.tsx | 126 ++--- .../NotificationPreviewGrafanaManaged.tsx | 90 ++++ .../notificaton-preview/NotificationRoute.tsx | 279 ++--------- .../NotificationRouteDetailsModal.tsx | 200 -------- .../rule-editor/notificaton-preview/route.ts | 26 - ...eAlertmanagerNotificationRoutingPreview.ts | 53 +- .../alerting/unified/routeGroupsMatcher.ts | 59 ++- .../rule-editor/CloneRuleEditor.test.tsx | 3 +- .../rule-editor/RuleEditorExisting.test.tsx | 5 +- .../RuleEditorGrafanaRules.test.tsx | 11 +- .../alerting/unified/useRouteGroupsMatcher.ts | 10 +- .../notification-policies.test.ts.snap | 56 --- .../alerting/unified/utils/alertmanager.ts | 12 +- .../alerting/unified/utils/k8s/utils.ts | 5 + .../alerting/unified/utils/matchers.ts | 89 +--- .../alerting/unified/utils/navigation.ts | 10 + .../utils/notification-policies.test.ts | 457 +---------------- .../unified/utils/notification-policies.ts | 172 +------ .../unified/utils/routeAdapter.test.ts | 267 ++++++++++ .../alerting/unified/utils/routeAdapter.ts | 159 ++++++ public/locales/en-US/grafana.json | 32 +- 47 files changed, 2013 insertions(+), 1931 deletions(-) create mode 100644 packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.test.ts create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ConnectionLine.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ContactPointGroup.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.test.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/MatchDetails.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyDrawer.tsx create mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewGrafanaManaged.tsx delete mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRouteDetailsModal.tsx delete mode 100644 public/app/features/alerting/unified/components/rule-editor/notificaton-preview/route.ts delete mode 100644 public/app/features/alerting/unified/utils/__snapshots__/notification-policies.test.ts.snap create mode 100644 public/app/features/alerting/unified/utils/routeAdapter.test.ts create mode 100644 public/app/features/alerting/unified/utils/routeAdapter.ts diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e66f3ac2486..d6951a21011 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -24,11 +24,6 @@ "count": 1 } }, - "packages/grafana-alerting/src/grafana/notificationPolicies/utils.ts": { - "@typescript-eslint/consistent-type-assertions": { - "count": 1 - } - }, "packages/grafana-data/src/dataframe/ArrayDataFrame.ts": { "@typescript-eslint/no-explicit-any": { "count": 1 diff --git a/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.test.ts b/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.test.ts new file mode 100644 index 00000000000..9bf6cd1c7b1 --- /dev/null +++ b/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.test.ts @@ -0,0 +1,74 @@ +import { RoutingTree } from '../../api/v0alpha1/api.gen'; +import { LabelMatcherFactory, RouteFactory } from '../../api/v0alpha1/mocks/fakes/Routes'; +import { Label } from '../../matchers/types'; + +import { matchInstancesToRouteTrees } from './useMatchPolicies'; + +describe('matchInstancesToRouteTrees', () => { + it('should return root route when child routes do not match instances', () => { + const route = RouteFactory.build({ + matchers: [LabelMatcherFactory.build({ label: 'service', type: '=', value: 'web' })], + receiver: 'web-team', + }); + + const treeName = 'test-tree'; + const trees: RoutingTree[] = [ + { + kind: 'RoutingTree', + metadata: { name: treeName }, + spec: { + defaults: { + receiver: 'receiver 1', + }, + routes: [route], + }, + status: {}, + }, + ]; + + const instanceLabels: Label[] = [['service', 'api']]; + const instances: Label[][] = [instanceLabels]; // Different service - should not match + + const result = matchInstancesToRouteTrees(trees, instances); + + expect(result).toHaveLength(1); + expect(result[0].labels).toBe(instanceLabels); + expect(result[0].matchedRoutes).toHaveLength(1); + // The root route should match as it's a catch-all + expect(result[0].matchedRoutes[0].route.receiver).toBe('receiver 1'); + expect(result[0].matchedRoutes[0].routeTree.metadata.name).toBe(treeName); + }); + + it('should return matched routes when trees match instances', () => { + const route = RouteFactory.build({ + matchers: [LabelMatcherFactory.build({ label: 'service', type: '=', value: 'web' })], + receiver: 'web-team', + }); + + const treeName = 'test-tree'; + const trees: RoutingTree[] = [ + { + kind: 'RoutingTree', + metadata: { name: treeName }, + spec: { + defaults: { + receiver: 'receiver 1', + }, + routes: [route], + }, + status: {}, + }, + ]; + + const instanceLabels: Label[] = [['service', 'web']]; + const instances: Label[][] = [instanceLabels]; + + const result = matchInstancesToRouteTrees(trees, instances); + + expect(result).toHaveLength(1); + expect(result[0].labels).toBe(instanceLabels); + expect(result[0].matchedRoutes.length).toBeGreaterThan(0); + expect(result[0].matchedRoutes[0].routeTree.metadata.name).toBe(treeName); + expect(result[0].matchedRoutes[0].matchDetails.labels).toBe(instanceLabels); + }); +}); diff --git a/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.ts b/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.ts index 14516375a86..1e830830701 100644 --- a/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.ts +++ b/packages/grafana-alerting/src/grafana/notificationPolicies/hooks/useMatchPolicies.ts @@ -4,7 +4,7 @@ import { RoutingTree, alertingAPI } from '../../api/v0alpha1/api.gen'; import { Label } from '../../matchers/types'; import { USER_DEFINED_TREE_NAME } from '../consts'; import { Route, RouteWithID } from '../types'; -import { RouteMatchResult, convertRoutingTreeToRoute, matchAlertInstancesToPolicyTree } from '../utils'; +import { RouteMatchResult, TreeMatch, convertRoutingTreeToRoute, matchInstancesToRoute } from '../utils'; export type RouteMatch = { route: Route; @@ -32,11 +32,10 @@ export type InstanceMatchResult = { * 2. Compute the inherited properties for each node in the tree * 3. Find routes within each tree that match the given set of labels * - * @returns An object containing a `matchInstancesToPolicies` function that takes alert instances + * @returns An object containing a `matchInstancesToRoutingTrees` function that takes alert instances * and returns an array of InstanceMatchResult objects, each containing the matched routes and matching details */ -export function useMatchAlertInstancesToNotificationPolicies() { - // fetch the routing trees from the API +export function useMatchInstancesToRouteTrees() { const { data, ...rest } = alertingAPI.endpoints.listRoutingTree.useQuery( {}, { @@ -45,52 +44,52 @@ export function useMatchAlertInstancesToNotificationPolicies() { } ); - const matchInstancesToPolicies = useCallback( - (instances: Label[][]): InstanceMatchResult[] => { - if (!data) { - return []; - } - - // the routing trees are returned as an array of items because there can be several - const trees = data.items; - - return instances.map((labels) => { - // Collect all matched routes from all trees - const allMatchedRoutes: RouteMatch[] = []; - - // Process each tree for this instance - trees.forEach((tree) => { - const treeName = tree.metadata.name ?? USER_DEFINED_TREE_NAME; - // We have to convert the RoutingTree structure to a Route structure to be able to use the matching functions - const rootRoute = convertRoutingTreeToRoute(tree); - - // Match this single instance against the route tree - const { expandedTree, matchedPolicies } = matchAlertInstancesToPolicyTree([labels], rootRoute); - - // Process each matched route from the tree - matchedPolicies.forEach((results, route) => { - // For each match result, create a RouteMatch object - results.forEach((matchDetails) => { - allMatchedRoutes.push({ - route, - routeTree: { - metadata: { name: treeName }, - expandedSpec: expandedTree, - }, - matchDetails, - }); - }); - }); - }); - - return { - labels, - matchedRoutes: allMatchedRoutes, - }; - }); - }, - [data] + const memoizedFunction = useCallback( + (instances: Label[][]) => matchInstancesToRouteTrees(data?.items ?? [], instances), + [data?.items] ); - return { matchInstancesToPolicies, ...rest }; + return { + matchInstancesToRouteTrees: memoizedFunction, + ...rest, + }; +} + +/** + * This function will match a set of labels to multiple routing trees. Assumes a list of routing trees has already been fetched. + * + * Use "useMatchInstancesToRouteTrees" if you want the hook to automatically fetch the latest definition of routing trees. + */ +export function matchInstancesToRouteTrees(trees: RoutingTree[], instances: Label[][]): InstanceMatchResult[] { + // Process each tree and get matches for all instances + const treeMatches = trees.map((tree) => { + const rootRoute = convertRoutingTreeToRoute(tree); + return matchInstancesToRoute(rootRoute, instances); + }); + + // Group results by instance + return instances.map((labels) => { + // Collect matches for this specific instance from all trees + const allMatchedRoutes = treeMatches.flatMap(({ expandedTree, matchedPolicies }, index) => { + const tree = trees[index]; + + return Array.from(matchedPolicies.entries()).flatMap(([route, results]) => + results + .filter((matchDetails) => matchDetails.labels === labels) + .map((matchDetails) => ({ + route, + routeTree: { + metadata: { name: tree.metadata.name ?? USER_DEFINED_TREE_NAME }, + expandedSpec: expandedTree, + }, + matchDetails, + })) + ); + }); + + return { + labels, + matchedRoutes: allMatchedRoutes, + }; + }); } diff --git a/packages/grafana-alerting/src/grafana/notificationPolicies/utils.test.ts b/packages/grafana-alerting/src/grafana/notificationPolicies/utils.test.ts index 3911a4ba676..2c23b899a64 100644 --- a/packages/grafana-alerting/src/grafana/notificationPolicies/utils.test.ts +++ b/packages/grafana-alerting/src/grafana/notificationPolicies/utils.test.ts @@ -12,7 +12,7 @@ import { computeInheritedTree, findMatchingRoutes, getInheritedProperties, - matchAlertInstancesToPolicyTree, + matchInstancesToRoute, } from './utils'; describe('findMatchingRoutes', () => { @@ -1072,7 +1072,7 @@ describe('matchAlertInstancesToPolicyTree', () => { ], // Should match parent only ]; - const result = matchAlertInstancesToPolicyTree(instances, parentRoute); + const result = matchInstancesToRoute(parentRoute, instances); // Should return expanded tree with identifiers expect(result.expandedTree).toHaveProperty('id'); @@ -1109,13 +1109,13 @@ describe('matchAlertInstancesToPolicyTree', () => { }); // Empty instances array - const result1 = matchAlertInstancesToPolicyTree([], route); + const result1 = matchInstancesToRoute(route, []); expect(result1.expandedTree).toHaveProperty('id'); expect(result1.matchedPolicies.size).toBe(0); // Instances that don't match const instances: Label[][] = [[['service', 'api']]]; - const result2 = matchAlertInstancesToPolicyTree(instances, route); + const result2 = matchInstancesToRoute(route, instances); expect(result2.expandedTree).toHaveProperty('id'); expect(result2.matchedPolicies.size).toBe(0); }); diff --git a/packages/grafana-alerting/src/grafana/notificationPolicies/utils.ts b/packages/grafana-alerting/src/grafana/notificationPolicies/utils.ts index 2cf5babe025..444244fe5fe 100644 --- a/packages/grafana-alerting/src/grafana/notificationPolicies/utils.ts +++ b/packages/grafana-alerting/src/grafana/notificationPolicies/utils.ts @@ -1,7 +1,7 @@ import { groupBy, isArray, pick, reduce, uniqueId } from 'lodash'; import { RoutingTree, RoutingTreeRoute } from '../api/v0alpha1/api.gen'; -import { Label, LabelMatcher } from '../matchers/types'; +import { Label } from '../matchers/types'; import { LabelMatchDetails, matchLabels } from '../matchers/utils'; import { Route, RouteWithID } from './types'; @@ -150,6 +150,7 @@ export function addUniqueIdentifier(route: Route): RouteWithID { }; } +// all policies that were matched of a single tree export type TreeMatch = { /* we'll include the entire expanded policy tree for diagnostics */ expandedTree: RouteWithID; @@ -166,13 +167,13 @@ export type TreeMatch = { * @param instances - A set of labels for which you want to determine the matching policies * @param routingTree - A notification policy tree (or subtree) */ -export function matchAlertInstancesToPolicyTree(instances: Label[][], routingTree: Route): TreeMatch { +export function matchInstancesToRoute(rootRoute: Route, instances: Label[][]): TreeMatch { // initially empty map of matches policies const matchedPolicies = new Map(); // compute the entire expanded tree for matching routes and diagnostics // this will include inherited properties from parent nodes - const expandedTree = addUniqueIdentifier(computeInheritedTree(routingTree)); + const expandedTree = addUniqueIdentifier(computeInheritedTree(rootRoute)); // let's first find all matching routes for the provided instances const matchesArray = instances.flatMap((labels) => findMatchingRoutes(expandedTree, labels)); @@ -202,13 +203,6 @@ export function convertRoutingTreeToRoute(routingTree: RoutingTree): Route { return routes.map( (route): Route => ({ ...route, - matchers: route.matchers?.map( - (matcher): LabelMatcher => ({ - ...matcher, - // sadly we use type narrowing for this on Route but the codegen has it as a string - type: matcher.type as LabelMatcher['type'], - }) - ), routes: route.routes ? convertRoutingTreeRoutes(route.routes) : [], }) ); diff --git a/packages/grafana-alerting/src/unstable.ts b/packages/grafana-alerting/src/unstable.ts index 5f9bc060350..1b5fd591ee4 100644 --- a/packages/grafana-alerting/src/unstable.ts +++ b/packages/grafana-alerting/src/unstable.ts @@ -10,14 +10,15 @@ export { getContactPointDescription } from './grafana/contactPoints/utils'; // Notification Policies export { - useMatchAlertInstancesToNotificationPolicies, + useMatchInstancesToRouteTrees, + matchInstancesToRouteTrees, type RouteMatch, type InstanceMatchResult, } from './grafana/notificationPolicies/hooks/useMatchPolicies'; export { type TreeMatch, type RouteMatchResult, - matchAlertInstancesToPolicyTree, + matchInstancesToRoute, findMatchingRoutes, getInheritedProperties, computeInheritedTree, diff --git a/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts b/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts index fc4b41be7b7..9d0d7373a82 100644 --- a/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts +++ b/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts @@ -10,9 +10,9 @@ export function useRouteGroupsMatcher() { return routeGroupsMatcher.getRouteGroupsMap(route, groups); }, []); - const matchInstancesToRoute = useCallback(async (rootRoute: RouteWithID, instancesToMatch: Labels[]) => { - return routeGroupsMatcher.matchInstancesToRoute(rootRoute, instancesToMatch); + const matchInstancesToRoutes = useCallback(async (rootRoute: RouteWithID, instancesToMatch: Labels[]) => { + return routeGroupsMatcher.matchInstancesToRoutes(rootRoute, instancesToMatch); }, []); - return { getRouteGroupsMap, matchInstancesToRoute }; + return { getRouteGroupsMap, matchInstancesToRoutes }; } diff --git a/public/app/features/alerting/unified/__snapshots__/NotificationPoliciesPage.test.tsx.snap b/public/app/features/alerting/unified/__snapshots__/NotificationPoliciesPage.test.tsx.snap index b6c6a2c31fb..89a949d8533 100644 --- a/public/app/features/alerting/unified/__snapshots__/NotificationPoliciesPage.test.tsx.snap +++ b/public/app/features/alerting/unified/__snapshots__/NotificationPoliciesPage.test.tsx.snap @@ -19,41 +19,101 @@ exports[`findRoutesMatchingFilters should work with all filters 1`] = ` "filtersApplied": true, "matchedRoutesWithPath": Map { { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], } => [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "0", + "mute_time_intervals": undefined, + "object_matchers": [], "receiver": "default-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -61,19 +121,45 @@ exports[`findRoutesMatchingFilters should work with all filters 1`] = ` ], }, { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -95,41 +181,101 @@ exports[`findRoutesMatchingFilters should work with only contact point and inher "filtersApplied": true, "matchedRoutesWithPath": Map { { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], } => [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "0", + "mute_time_intervals": undefined, + "object_matchers": [], "receiver": "default-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -137,50 +283,121 @@ exports[`findRoutesMatchingFilters should work with only contact point and inher ], }, { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], }, ], { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, } => [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "0", + "mute_time_intervals": undefined, + "object_matchers": [], "receiver": "default-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -188,29 +405,66 @@ exports[`findRoutesMatchingFilters should work with only contact point and inher ], }, { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], }, { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -223,41 +477,101 @@ exports[`findRoutesMatchingFilters should work with only label matchers 1`] = ` "filtersApplied": true, "matchedRoutesWithPath": Map { { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], } => [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "0", + "mute_time_intervals": undefined, + "object_matchers": [], "receiver": "default-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], @@ -265,19 +579,45 @@ exports[`findRoutesMatchingFilters should work with only label matchers 1`] = ` ], }, { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "1", - "matchers": [ - "hello=world", - "foo!=bar", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "hello", + "=", + "world", + ], + [ + "foo", + "!=", + "bar", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": [ { + "active_time_intervals": undefined, + "continue": false, + "group_by": undefined, + "group_interval": undefined, + "group_wait": undefined, "id": "2", - "matchers": [ - "bar=baz", + "mute_time_intervals": undefined, + "object_matchers": [ + [ + "bar", + "=", + "baz", + ], ], "receiver": "simple-receiver", + "repeat_interval": undefined, "routes": undefined, }, ], diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index aaee307716b..19f2b95d537 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -1,6 +1,6 @@ import { RelativeTimeRange } from '@grafana/data'; import { t } from '@grafana/i18n'; -import { Matcher } from 'app/plugins/datasource/alertmanager/types'; +import { AlertmanagerAlert, Matcher } from 'app/plugins/datasource/alertmanager/types'; import { RuleIdentifier, RuleNamespace, RulerDataSourceConfig } from 'app/types/unified-alerting'; import { AlertQuery, @@ -33,11 +33,9 @@ import { } from './prometheus'; import { FetchRulerRulesFilter, rulerUrlBuilder } from './ruler'; -export type ResponseLabels = { - labels: AlertInstances[]; -}; - -export type PreviewResponse = ResponseLabels[]; +export type PreviewResponse = Array< + Pick +>; export interface Datasource { type: string; @@ -83,8 +81,6 @@ export interface Rule { annotations: Annotations; } -export type AlertInstances = Record; - interface ExportRulesParams { format: ExportFormats; folderUid?: string; diff --git a/public/app/features/alerting/unified/components/contact-points/utils.ts b/public/app/features/alerting/unified/components/contact-points/utils.ts index 24441564615..7c75e015412 100644 --- a/public/app/features/alerting/unified/components/contact-points/utils.ts +++ b/public/app/features/alerting/unified/components/contact-points/utils.ts @@ -1,6 +1,7 @@ import { difference, groupBy, take, trim, upperFirst } from 'lodash'; import { ReactNode } from 'react'; +import { computeInheritedTree } from '@grafana/alerting/unstable'; import { t } from '@grafana/i18n'; import { NotifierDTO, NotifierStatus, ReceiversStateDTO } from 'app/features/alerting/unified/types/alerting'; import { canAdminEntity, shouldUseK8sApi } from 'app/features/alerting/unified/utils/k8s/utils'; @@ -14,8 +15,8 @@ import { } from 'app/plugins/datasource/alertmanager/types'; import { OnCallIntegrationDTO } from '../../api/onCallApi'; -import { computeInheritedTree } from '../../utils/notification-policies'; import { extractReceivers } from '../../utils/receivers'; +import { routeAdapter } from '../../utils/routeAdapter'; import { ReceiverTypes } from '../receivers/grafanaAppReceivers/onCall/onCall'; import { ReceiverPluginMetadata, getOnCallMetadata } from '../receivers/grafanaAppReceivers/useReceiversMetadata'; @@ -132,8 +133,10 @@ export function enhanceContactPointsWithMetadata({ alertmanagerConfiguration, }: EnhanceContactPointsArgs): ContactPointWithMetadata[] { // compute the entire inherited tree before finding what notification policies are using a particular contact point - const fullyInheritedTree = computeInheritedTree(alertmanagerConfiguration?.alertmanager_config?.route ?? {}); - const usedContactPoints = getUsedContactPoints(fullyInheritedTree); + const fullyInheritedTree = computeInheritedTree( + routeAdapter.toPackage(alertmanagerConfiguration?.alertmanager_config?.route ?? {}) + ); + const usedContactPoints = getUsedContactPoints(routeAdapter.fromPackage(fullyInheritedTree)); const usedContactPointsByName = groupBy(usedContactPoints, 'receiver'); const enhanced = contactPoints.map((contactPoint) => { diff --git a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx index 93dfdfe1989..93cb7db63da 100644 --- a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx +++ b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx @@ -6,7 +6,7 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { mockExportApi, setupMswServer } from '../../mockApi'; import { mockDataSource } from '../../mocks'; -import { grafanaRulerRule } from '../../mocks/grafanaRulerApi'; +import { grafanaRulerRule, mockPreviewApiResponse } from '../../mocks/grafanaRulerApi'; import { setupDataSources } from '../../testSetup/datasources'; import GrafanaModifyExport from './GrafanaModifyExport'; @@ -59,6 +59,10 @@ function renderModifyExport(ruleId: string) { const server = setupMswServer(); +beforeEach(() => { + mockPreviewApiResponse(server, []); +}); + describe('GrafanaModifyExport', () => { setupDataSources(dataSources.default); diff --git a/public/app/features/alerting/unified/components/notification-policies/Matchers.tsx b/public/app/features/alerting/unified/components/notification-policies/Matchers.tsx index a82e9dfed4f..b1fcde8caf0 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Matchers.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Matchers.tsx @@ -55,7 +55,7 @@ interface MatcherBadgeProps { formatter?: MatcherFormatter; } -const MatcherBadge: FC = ({ matcher, formatter = 'default' }) => { +export const MatcherBadge: FC = ({ matcher, formatter = 'default' }) => { const styles = useStyles2(getStyles); return ( diff --git a/public/app/features/alerting/unified/components/notification-policies/NotificationPoliciesList.tsx b/public/app/features/alerting/unified/components/notification-policies/NotificationPoliciesList.tsx index 2e57a1960d2..72dace0fc9d 100644 --- a/public/app/features/alerting/unified/components/notification-policies/NotificationPoliciesList.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/NotificationPoliciesList.tsx @@ -2,6 +2,7 @@ import { defaults } from 'lodash'; import { useEffect, useMemo, useState } from 'react'; import { useAsyncFn } from 'react-use'; +import { computeInheritedTree } from '@grafana/alerting/unstable'; import { Trans, t } from '@grafana/i18n'; import { Alert, Button, Stack } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; @@ -10,12 +11,12 @@ import { AlertmanagerAction, useAlertmanagerAbility } from 'app/features/alertin import { FormAmRoute } from 'app/features/alerting/unified/types/amroutes'; import { addUniqueIdentifierToRoute } from 'app/features/alerting/unified/utils/amroutes'; import { getErrorCode, stringifyErrorLike } from 'app/features/alerting/unified/utils/misc'; -import { computeInheritedTree } from 'app/features/alerting/unified/utils/notification-policies'; import { ObjectMatcher, ROUTES_META_SYMBOL, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; import { anyOfRequestState, isError } from '../../hooks/useAsync'; import { useAlertmanager } from '../../state/AlertmanagerContext'; import { ERROR_NEWER_CONFIGURATION } from '../../utils/k8s/errors'; +import { routeAdapter } from '../../utils/routeAdapter'; import { alertmanagerApi } from './../../api/alertmanagerApi'; import { useGetContactPointsState } from './../../api/receiversApi'; @@ -297,7 +298,10 @@ export const findRoutesMatchingFilters = (rootRoute: RouteWithID, filters: Route const matchedRoutes: RouteWithID[][] = []; // compute fully inherited tree so all policies have their inherited receiver - const fullRoute = computeInheritedTree(rootRoute); + const adaptedRootRoute = routeAdapter.toPackage(rootRoute); + const adaptedFullTree = computeInheritedTree(adaptedRootRoute); + + const fullRoute = routeAdapter.fromPackage(adaptedFullTree); // find all routes for our contact point filter const matchingRoutesForContactPoint = contactPointFilter diff --git a/public/app/features/alerting/unified/components/notification-policies/Policy.tsx b/public/app/features/alerting/unified/components/notification-policies/Policy.tsx index 2eded5fb40f..abd12efd135 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Policy.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Policy.tsx @@ -5,7 +5,8 @@ import * as React from 'react'; import { FC, Fragment, ReactNode, useState } from 'react'; import { useToggle } from 'react-use'; -import { AlertLabel } from '@grafana/alerting/unstable'; +import { InheritableProperties } from '@grafana/alerting/internal'; +import { AlertLabel, getInheritedProperties } from '@grafana/alerting/unstable'; import { GrafanaTheme2 } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { @@ -39,7 +40,7 @@ import { AlertmanagerAction, useAlertmanagerAbilities, useAlertmanagerAbility } import { getAmMatcherFormatter } from '../../utils/alertmanager'; import { MatcherFormatter, normalizeMatchers } from '../../utils/matchers'; import { createContactPointLink, createContactPointSearchLink, createMuteTimingLink } from '../../utils/misc'; -import { InheritableProperties, getInheritedProperties } from '../../utils/notification-policies'; +import { routeAdapter } from '../../utils/routeAdapter'; import { InsertPosition } from '../../utils/routeTree'; import { Authorize } from '../Authorize'; import { PopupCard } from '../HoverCard'; @@ -59,7 +60,7 @@ interface PolicyComponentProps { contactPointsState?: ReceiversState; readOnly?: boolean; provisioned?: boolean; - inheritedProperties?: Partial; + inheritedProperties?: InheritableProperties; routesMatchingFilters?: RoutesMatchingFilters; matchingInstancesPreview?: { @@ -346,7 +347,11 @@ const Policy = (props: PolicyComponentProps) => { {showPolicyChildren && ( <> {pageOfChildren.map((child) => { - const childInheritedProperties = getInheritedProperties(currentRoute, child, inheritedProperties); + const childInheritedProperties = getInheritedProperties( + routeAdapter.toPackage(currentRoute), + routeAdapter.toPackage(child), + inheritedProperties + ); // This child is autogenerated if it's the autogenerated root or if it's a child of an autogenerated policy. const isThisChildAutoGenerated = isAutoGeneratedRoot(child) || isAutoGenerated; /* pass the "readOnly" prop from the parent, because for any child policy , if its parent it's not editable, @@ -685,7 +690,7 @@ const AllMatchesIndicator: FC = () => { ); }; -function DefaultPolicyIndicator() { +export function DefaultPolicyIndicator() { const styles = useStyles2(getStyles); return ( <> diff --git a/public/app/features/alerting/unified/components/notification-policies/__snapshots__/useNotificationPolicyRoute.test.tsx.snap b/public/app/features/alerting/unified/components/notification-policies/__snapshots__/useNotificationPolicyRoute.test.tsx.snap index cfd5f04b6af..f4e5ae1f88f 100644 --- a/public/app/features/alerting/unified/components/notification-policies/__snapshots__/useNotificationPolicyRoute.test.tsx.snap +++ b/public/app/features/alerting/unified/components/notification-policies/__snapshots__/useNotificationPolicyRoute.test.tsx.snap @@ -11,6 +11,8 @@ exports[`createKubernetesRoutingTreeSpec 1`] = ` "group_by": [ "alertname", ], + "group_interval": undefined, + "group_wait": undefined, "receiver": "default-receiver", "repeat_interval": "4h", }, diff --git a/public/app/features/alerting/unified/components/notification-policies/useNotificationPolicyRoute.ts b/public/app/features/alerting/unified/components/notification-policies/useNotificationPolicyRoute.ts index f927fc49d70..e6a0b7a0cc5 100644 --- a/public/app/features/alerting/unified/components/notification-policies/useNotificationPolicyRoute.ts +++ b/public/app/features/alerting/unified/components/notification-policies/useNotificationPolicyRoute.ts @@ -1,6 +1,7 @@ import { pick } from 'lodash'; import memoize from 'micro-memoize'; +import { INHERITABLE_KEYS, type InheritableProperties } from '@grafana/alerting/internal'; import { BaseAlertmanagerArgs, Skippable } from 'app/features/alerting/unified/types/hooks'; import { MatcherOperator, ROUTES_META_SYMBOL, Route } from 'app/plugins/datasource/alertmanager/types'; @@ -23,7 +24,7 @@ import { FormAmRoute } from '../../types/amroutes'; import { addUniqueIdentifierToRoute } from '../../utils/amroutes'; import { PROVENANCE_NONE, ROOT_ROUTE_NAME } from '../../utils/k8s/constants'; import { isK8sEntityProvisioned, shouldUseK8sApi } from '../../utils/k8s/utils'; -import { INHERITABLE_KEYS, InheritableProperties } from '../../utils/notification-policies'; +import { routeAdapter } from '../../utils/routeAdapter'; import { InsertPosition, addRouteToReferenceRoute, @@ -279,7 +280,7 @@ export function routeToK8sSubRoute(route: Route): ComGithubGrafanaGrafanaPkgApis export function createKubernetesRoutingTreeSpec( rootRoute: Route ): ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1RoutingTree { - const inheritableDefaultProperties: InheritableProperties = pick(rootRoute, INHERITABLE_KEYS); + const inheritableDefaultProperties: InheritableProperties = pick(routeAdapter.toPackage(rootRoute), INHERITABLE_KEYS); const defaults: ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1RouteDefaults = { ...inheritableDefaultProperties, diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx index 400ea551a25..c5b364a20d9 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx @@ -20,7 +20,7 @@ import { MANUAL_ROUTING_KEY, SIMPLIFIED_QUERY_EDITOR_KEY } from 'app/features/al import { AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types/accessControl'; -import { grafanaRulerGroup } from '../../../../mocks/grafanaRulerApi'; +import { grafanaRulerGroup, mockPreviewApiResponse } from '../../../../mocks/grafanaRulerApi'; jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ AppChromeUpdate: ({ actions }: { actions: ReactNode }) =>
{actions}
, @@ -63,6 +63,8 @@ const selectContactPoint = async (contactPointName: string) => { await clickSelectOption(contactPointInput, contactPointName); }; +const server = setupMswServer(); + // combobox hack beforeEach(() => { const mockGetBoundingClientRect = jest.fn(() => ({ @@ -77,9 +79,10 @@ beforeEach(() => { Object.defineProperty(Element.prototype, 'getBoundingClientRect', { value: mockGetBoundingClientRect, }); + + mockPreviewApiResponse(server, []); }); -setupMswServer(); setupDataSources(dataSources.default, dataSources.am); // Setup plugin extensions hook to prevent setPluginLinksHook errors diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ConnectionLine.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ConnectionLine.tsx new file mode 100644 index 00000000000..9e2881cdc35 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ConnectionLine.tsx @@ -0,0 +1,22 @@ +import { css } from '@emotion/css'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Box, useStyles2 } from '@grafana/ui'; + +export function ConnectionLine() { + const styles = useStyles2(getStyles); + + return ( + +
+ + ); +} + +const getStyles = (theme: GrafanaTheme2) => ({ + line: css({ + width: '1px', + height: '100%', + background: theme.colors.border.medium, + }), +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ContactPointGroup.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ContactPointGroup.tsx new file mode 100644 index 00000000000..ba69db35766 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/ContactPointGroup.tsx @@ -0,0 +1,158 @@ +import { css } from '@emotion/css'; +import { PropsWithChildren, ReactNode } from 'react'; +import Skeleton from 'react-loading-skeleton'; +import { useToggle } from 'react-use'; + +import { alertingAPI, getContactPointDescription } from '@grafana/alerting/unstable'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Trans, t } from '@grafana/i18n'; +import { Stack, Text, TextLink, useStyles2 } from '@grafana/ui'; + +import { stringifyFieldSelector } from '../../../utils/k8s/utils'; +import { createContactPointLink } from '../../../utils/misc'; +import { CollapseToggle } from '../../CollapseToggle'; +import { MetaText } from '../../MetaText'; +import { ContactPointLink } from '../../rule-viewer/ContactPointLink'; + +import UnknownContactPointDetails from './UnknownContactPointDetails'; + +interface ContactPointGroupProps extends PropsWithChildren { + name: string; + matchedInstancesCount: number; +} + +export function GrafanaContactPointGroup({ name, matchedInstancesCount, children }: ContactPointGroupProps) { + // find receiver by name – since this is what we store in the alert rule definition + const { data, isLoading } = alertingAPI.endpoints.listReceiver.useQuery({ + fieldSelector: stringifyFieldSelector([['spec.title', name]]), + }); + + // grab the first result from the fieldSelector result + const contactPoint = data?.items.at(0); + + return ( + + ) : ( + + ) + } + description={contactPoint ? getContactPointDescription(contactPoint) : null} + > + {children} + + ); +} + +export function ExternalContactPointGroup({ + name, + alertmanagerSourceName, + matchedInstancesCount, + children, +}: ContactPointGroupProps & { alertmanagerSourceName: string }) { + const link = ( + + {name} + + ); + return ( + + {children} + + ); +} + +interface ContactPointGroupInnerProps extends Omit { + name: NonNullable; + description?: ReactNode; + isLoading?: boolean; + children: ReactNode; +} + +export function ContactPointGroup({ + name, + description, + matchedInstancesCount, + isLoading = false, + children, +}: ContactPointGroupInnerProps) { + const styles = useStyles2(getStyles); + const [isExpanded, toggleExpanded] = useToggle(false); + + return ( + +
+ + toggleExpanded()} + aria-label={t('alerting.notification-route-header.aria-label-expand-policy-route', 'Expand policy route')} + /> + {isLoading && loader} + {!isLoading && ( + <> + {name && ( + <> + + Delivered to {name} + + {description && ( + + ⋅ {description} + + )} + + )} + {matchedInstancesCount && ( + <> + + | + + + {/* @TODO pluralization */} + {matchedInstancesCount}{' '} + instances + + + )} + + )} + +
+ {isExpanded &&
{children}
} +
+ ); +} + +const loader = ( + + + + +); + +const getStyles = (theme: GrafanaTheme2) => ({ + contactPointRow: css({ + padding: theme.spacing(0.5), + + ':hover': { + background: theme.components.table.rowHoverBackground, + }, + }), + notificationPolicies: css({ + marginLeft: theme.spacing(2), + borderLeftStyle: 'solid', + borderLeftWidth: 1, + borderLeftColor: theme.colors.border.weak, + }), +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.test.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.test.tsx new file mode 100644 index 00000000000..ab8bd49ede0 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.test.tsx @@ -0,0 +1,103 @@ +import { render, screen } from '@testing-library/react'; + +import { LabelMatcher, RouteWithID } from '@grafana/alerting/unstable'; + +import { JourneyPolicyCard } from './JourneyPolicyCard'; + +describe('JourneyPolicyCard', () => { + const mockMatchers: LabelMatcher[] = [ + { label: 'severity', type: '=', value: 'critical' }, + { label: 'team', type: '=', value: 'backend' }, + ]; + + const mockRoute: RouteWithID = { + id: 'test-route', + receiver: 'test-receiver', + group_by: ['alertname', 'severity'], + matchers: mockMatchers, + routes: [], + continue: false, + }; + + it('should render basic route information', () => { + render(); + + expect(screen.getByText('test-receiver')).toBeInTheDocument(); + expect(screen.getByText('alertname, severity')).toBeInTheDocument(); + expect(screen.getByTestId('label-matchers')).toBeInTheDocument(); + }); + + it('should render "No matchers" when route has no matchers', () => { + const routeWithoutMatchers: RouteWithID = { + id: 'test-route', + receiver: 'test-receiver', + matchers: [], + routes: [], + continue: false, + }; + + render(); + + expect(screen.getByText('No matchers')).toBeInTheDocument(); + expect(screen.queryByTestId('label-matchers')).not.toBeInTheDocument(); + }); + + it('should show continue matching indicator when route.continue is true', () => { + const routeWithContinue: RouteWithID = { + ...mockRoute, + continue: true, + }; + + render(); + + expect(screen.getByTestId('continue-matching')).toBeInTheDocument(); + }); + + it('should not show continue matching indicator when route.continue is false or undefined', () => { + render(); + + expect(screen.queryByTestId('continue-matching')).not.toBeInTheDocument(); + }); + + it('should not render receiver or group_by when they are not provided', () => { + const minimalRoute: RouteWithID = { + id: 'test-route', + routes: [], + continue: false, + }; + + render(); + + expect(screen.queryByText(/test-receiver/)).not.toBeInTheDocument(); + expect(screen.queryByText(/alertname/)).not.toBeInTheDocument(); + }); + + it('should show DefaultPolicyIndicator when isRoot is true', () => { + render(); + + expect(screen.getByRole('heading', { name: 'Default policy' })).toBeInTheDocument(); + }); + + describe('isFinalRoute prop', () => { + it('should set aria-current="true" and aria-label when isFinalRoute is true', () => { + render(); + + const card = screen.getByRole('article', { current: true }); + expect(card).toBeInTheDocument(); + }); + + it('should not set aria-current when isFinalRoute is false', () => { + render(); + + const card = screen.getByRole('article', { current: false }); + expect(card).toBeInTheDocument(); + }); + + it('should not set aria-current when isFinalRoute is undefined (default)', () => { + render(); + + const card = screen.getByRole('article', { current: false }); + expect(card).toBeInTheDocument(); + }); + }); +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.tsx new file mode 100644 index 00000000000..bab2fb31e8e --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/JourneyPolicyCard.tsx @@ -0,0 +1,112 @@ +import { css } from '@emotion/css'; + +import { RouteWithID } from '@grafana/alerting/unstable'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Trans } from '@grafana/i18n'; +import { Icon, Stack, Text, Tooltip, useStyles2 } from '@grafana/ui'; +import { ObjectMatcher } from 'app/plugins/datasource/alertmanager/types'; + +import { labelMatcherToObjectMatcher } from '../../../utils/routeAdapter'; +import { Matchers } from '../../notification-policies/Matchers'; +import { DefaultPolicyIndicator } from '../../notification-policies/Policy'; + +interface JourneyPolicyCardProps { + route: RouteWithID; + isRoot?: boolean; + isFinalRoute?: boolean; +} + +export function JourneyPolicyCard({ route, isRoot = false, isFinalRoute = false }: JourneyPolicyCardProps) { + const styles = useStyles2(getStyles); + + // Convert route matchers to ObjectMatcher format + const matchers: ObjectMatcher[] = route.matchers?.map(labelMatcherToObjectMatcher) ?? []; + + const hasMatchers = matchers.length > 0; + const continueMatching = route.continue ?? false; + + return ( +
+ {continueMatching && } + + {/* root route indicator */} + {isRoot && } + + {/* Matchers */} + {hasMatchers ? ( + + ) : ( + + No matchers + + )} + + {/* Route metadata */} + + {route.receiver && ( + + {route.receiver} + + )} + {route.group_by && route.group_by.length > 0 && ( + + {route.group_by.join(', ')} + + )} + + +
+ ); +} + +const ContinueMatchingIndicator = () => { + const styles = useStyles2(getStyles); + + return ( + + This route will continue matching other policies + + } + > +
+ +
+
+ ); +}; + +const getStyles = (theme: GrafanaTheme2) => ({ + policyWrapper: (hasFocus = false) => + css({ + position: 'relative', + background: theme.colors.background.secondary, + borderRadius: theme.shape.radius.default, + border: `solid 1px ${theme.colors.border.weak}`, + ...(hasFocus && { + borderColor: theme.colors.primary.border, + background: theme.colors.primary.transparent, + }), + padding: theme.spacing(1), + }), + gutterIcon: css({ + position: 'absolute', + left: `-${theme.spacing(3.5)}`, + top: theme.spacing(2.25), + + color: theme.colors.text.secondary, + background: theme.colors.background.primary, + + width: '20px', + height: '20px', + + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + + border: `solid 1px ${theme.colors.border.weak}`, + borderRadius: theme.shape.radius.default, + }), +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/MatchDetails.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/MatchDetails.tsx new file mode 100644 index 00000000000..40cbf60da62 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/MatchDetails.tsx @@ -0,0 +1,56 @@ +import { css } from '@emotion/css'; + +import { AlertLabel, LabelMatchDetails } from '@grafana/alerting/unstable'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Trans } from '@grafana/i18n'; +import { Box, Text, useStyles2 } from '@grafana/ui'; + +import { labelMatcherToObjectMatcher } from '../../../utils/routeAdapter'; +import { MatcherBadge } from '../../notification-policies/Matchers'; + +interface MatchDetailsProps { + matchDetails: LabelMatchDetails[]; + labels: Array<[string, string]>; +} + +export function MatchDetails({ matchDetails, labels }: MatchDetailsProps) { + const styles = useStyles2(getStyles); + const matchingLabels = matchDetails.filter((detail) => detail.match); + + const noMatchingLabels = matchingLabels.length === 0; + + return ( +
+ {noMatchingLabels ? ( + + Policy matches all labels + + ) : ( + matchingLabels.map((detail) => ( + + + + matched + + {detail.matcher && } + + )) + )} +
+ ); +} + +const getStyles = (theme: GrafanaTheme2) => ({ + container: css({ + padding: `${theme.spacing(1)} ${theme.spacing(2)}`, + display: 'flex', + flexDirection: 'column', + gap: theme.spacing(1), + background: theme.colors.background.secondary, + borderRadius: theme.shape.radius.pill, + border: `solid 1px ${theme.colors.border.weak}`, + + width: 'fit-content', + alignSelf: 'center', + }), +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyDrawer.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyDrawer.tsx new file mode 100644 index 00000000000..e43bb432b82 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyDrawer.tsx @@ -0,0 +1,110 @@ +import { Fragment, useState } from 'react'; + +import { AlertLabel, RouteMatchResult, RouteWithID } from '@grafana/alerting/unstable'; +import { Trans } from '@grafana/i18n'; +import { Button, Drawer, Text, TextLink } from '@grafana/ui'; + +import { Stack } from '../../../../../../plugins/datasource/parca/QueryEditor/Stack'; +import { createRelativeUrl } from '../../../utils/url'; + +import { ConnectionLine } from './ConnectionLine'; +import { JourneyPolicyCard } from './JourneyPolicyCard'; +import { MatchDetails } from './MatchDetails'; + +type NotificationPolicyDrawerProps = { + policyName?: string; + matchedRootRoute: boolean; + journey: RouteMatchResult['matchingJourney']; + labels: Array<[string, string]>; +}; + +export function NotificationPolicyDrawer({ + policyName, + matchedRootRoute, + journey, + labels, +}: NotificationPolicyDrawerProps) { + const [isDrawerOpen, setIsDrawerOpen] = useState(false); + + const handleOpenDrawer = () => { + setIsDrawerOpen(true); + }; + + const handleCloseDrawer = () => { + setIsDrawerOpen(false); + }; + + // Process the journey data to extract the information we need + const finalRouteMatchInfo = journey.at(-1); + const nonMatchingLabels = finalRouteMatchInfo?.matchDetails.filter((detail) => !detail.match) ?? []; + + return ( + <> + + + {isDrawerOpen && ( + + Notification policy + {policyName && ( + + {' '} + ⋅ {policyName} + + )} + + } + onClose={handleCloseDrawer} + > + + + + {journey.map((routeInfo, index) => ( + + {index > 0 && ( + <> + + + + + )} + + + ))} + + + {nonMatchingLabels.length > 0 && ( + + + Non-matching labels + + + {nonMatchingLabels.map((detail) => ( + + + + ))} + + + )} + + + + + View notification policy tree + + + + + )} + + ); +} diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyMatchers.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyMatchers.tsx index 2da056f7b2a..7ea07fdc3ec 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyMatchers.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPolicyMatchers.tsx @@ -1,45 +1,23 @@ -import { css } from '@emotion/css'; - -import { GrafanaTheme2 } from '@grafana/data'; import { Trans } from '@grafana/i18n'; -import { useStyles2 } from '@grafana/ui'; +import { Text } from '@grafana/ui'; +import { ObjectMatcher } from 'app/plugins/datasource/alertmanager/types'; import { MatcherFormatter } from '../../../utils/matchers'; import { Matchers } from '../../notification-policies/Matchers'; -import { RouteWithPath, hasEmptyMatchers, isDefaultPolicy } from './route'; - interface Props { - route: RouteWithPath; + matchers?: ObjectMatcher[]; matcherFormatter: MatcherFormatter; } -export function NotificationPolicyMatchers({ route, matcherFormatter }: Props) { - const styles = useStyles2(getStyles); - if (isDefaultPolicy(route)) { +export function NotificationPolicyMatchers({ matchers = [], matcherFormatter }: Props) { + if (matchers.length === 0) { return ( -
- Default policy -
- ); - } else if (hasEmptyMatchers(route)) { - return ( -
+ No matchers -
+ ); } else { - return ; + return ; } } - -const getStyles = (theme: GrafanaTheme2) => ({ - defaultPolicy: css({ - padding: theme.spacing(0.5), - background: theme.colors.background.secondary, - width: 'fit-content', - }), - textMuted: css({ - color: theme.colors.text.secondary, - }), -}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx index fa87aef23a4..ab95bb4b679 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx @@ -1,13 +1,12 @@ -import { render, screen, waitFor, within } from 'test/test-utils'; -import { byRole, byTestId, byText } from 'testing-library-selector'; +import { render, waitFor, within } from 'test/test-utils'; +import { byRole, byText } from 'testing-library-selector'; import { setAlertmanagerConfig } from 'app/features/alerting/unified/mocks/server/entities/alertmanagers'; import { AccessControlAction } from 'app/types/accessControl'; import { MatcherOperator } from '../../../../../../plugins/datasource/alertmanager/types'; -import { Labels } from '../../../../../../types/unified-alerting-dto'; import { getMockConfig, setupMswServer } from '../../../mockApi'; -import { grantUserPermissions, mockAlertQuery } from '../../../mocks'; +import { grantUserPermissions, mockAlertQuery, mockAlertmanagerAlert } from '../../../mocks'; import { mockPreviewApiResponse } from '../../../mocks/grafanaRulerApi'; import { Folder } from '../../../types/rule-form'; import * as dataSource from '../../../utils/datasource'; @@ -18,7 +17,6 @@ import { } from '../../../utils/datasource'; import { NotificationPreview } from './NotificationPreview'; -import NotificationPreviewByAlertManager from './NotificationPreviewByAlertManager'; jest.mock('../../../useRouteGroupsMatcher'); @@ -34,18 +32,14 @@ const getAlertManagerDataSourcesByPermissionAndConfigMock = >; const ui = { - route: byTestId('matching-policy-route'), - routeButton: byRole('button', { name: /Expand policy route/ }), - routeMatchingInstances: byTestId('route-matching-instance'), - loadingIndicator: byText(/Loading routing preview/i), - previewButton: byRole('button', { name: /preview routing/i }), + contactPointGroup: byRole('list'), grafanaAlertManagerLabel: byText(/alertmanager:grafana/i), otherAlertManagerLabel: byText(/alertmanager:other_am/i), - seeDetails: byText(/see details/i), + expandButton: byRole('button', { name: 'Expand policy route' }), + seeDetails: byRole('button', { name: 'View route' }), details: { - title: byRole('heading', { name: /routing details/i }), - modal: byRole('dialog'), - linkToContactPoint: byRole('link', { name: /see details/i }), + drawer: byRole('dialog'), + linkToPolicyTree: byRole('link', { name: /view notification policy tree/i }), }, }; @@ -118,360 +112,91 @@ describe('NotificationPreview', () => { it('should render notification preview without alert manager label, when having only one alert manager configured to receive alerts', async () => { mockOneAlertManager(); - mockPreviewApiResponse(server, [{ labels: [{ tomato: 'red', avocate: 'green' }] }]); + mockPreviewApiResponse(server, [ + mockAlertmanagerAlert({ + labels: { tomato: 'red', avocate: 'green' }, + }), + ]); - const { user } = render( - - ); + render(); - await user.click(ui.previewButton.get()); - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); + // wait for loading to finish + await waitFor(async () => { + const matchingContactPoint = await ui.contactPointGroup.findAll(); + expect(matchingContactPoint).toHaveLength(1); }); // we expect the alert manager label to be missing as there is only one alert manager configured to receive alerts - await waitFor(() => { - expect(ui.grafanaAlertManagerLabel.query()).not.toBeInTheDocument(); - }); + expect(ui.grafanaAlertManagerLabel.query()).not.toBeInTheDocument(); expect(ui.otherAlertManagerLabel.query()).not.toBeInTheDocument(); - const matchingPoliciesElements = ui.route.queryAll; - await waitFor(() => { - expect(matchingPoliciesElements()).toHaveLength(1); - }); - expect(matchingPoliciesElements()[0]).toHaveTextContent(/tomato = red/); + const matchingContactPoint = await ui.contactPointGroup.findAll(); + expect(matchingContactPoint[0]).toHaveTextContent(/Delivered to slack/); + expect(matchingContactPoint[0]).toHaveTextContent(/1 instance/); }); + it('should render notification preview with alert manager sections, when having more than one alert manager configured to receive alerts', async () => { // two alert managers configured to receive alerts mockTwoAlertManagers(); - mockPreviewApiResponse(server, [{ labels: [{ tomato: 'red', avocate: 'green' }] }]); + mockPreviewApiResponse(server, [ + mockAlertmanagerAlert({ + labels: { tomato: 'red', avocate: 'green' }, + }), + ]); - const { user } = render( - - ); + render(); - await user.click(await ui.previewButton.find()); + // wait for loading to finish + await waitFor(async () => { + const matchingContactPoint = await ui.contactPointGroup.findAll(); + expect(matchingContactPoint).toHaveLength(2); + }); // we expect the alert manager label to be present as there is more than one alert manager configured to receive alerts expect(await ui.grafanaAlertManagerLabel.find()).toBeInTheDocument(); expect(await ui.otherAlertManagerLabel.find()).toBeInTheDocument(); - const matchingPoliciesElements = await ui.route.findAll(); + const matchingContactPoint = await ui.contactPointGroup.findAll(); - expect(matchingPoliciesElements).toHaveLength(2); - expect(matchingPoliciesElements[0]).toHaveTextContent(/tomato = red/); - expect(matchingPoliciesElements[1]).toHaveTextContent(/tomato = red/); + expect(matchingContactPoint).toHaveLength(2); + expect(matchingContactPoint[0]).toHaveTextContent(/Delivered to slack/); + expect(matchingContactPoint[0]).toHaveTextContent(/1 instance/); + + expect(matchingContactPoint[1]).toHaveTextContent(/Delivered to slack/); + expect(matchingContactPoint[1]).toHaveTextContent(/1 instance/); }); - it('should render details modal when clicking see details button', async () => { - // two alert managers configured to receive alerts - mockOneAlertManager(); - mockPreviewApiResponse(server, [{ labels: [{ tomato: 'red', avocate: 'green' }] }]); + + it('should render details when clicking see details button', async () => { mockHasEditPermission(true); - - const { user } = render( - - ); - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); - }); - - await user.click(ui.previewButton.get()); - await user.click(await ui.seeDetails.find()); - expect(ui.details.title.query()).toBeInTheDocument(); - //we expect seeing the default policy - expect(screen.getByText(/default policy/i)).toBeInTheDocument(); - const matchingPoliciesElements = within(ui.details.modal.get()).getAllByTestId('label-matchers'); - expect(matchingPoliciesElements).toHaveLength(1); - expect(matchingPoliciesElements[0]).toHaveTextContent(/tomato = red/); - expect(within(ui.details.modal.get()).getByText(/slack/i)).toBeInTheDocument(); - expect(ui.details.linkToContactPoint.get()).toBeInTheDocument(); - }); - it('should not render contact point link in details modal if user has no permissions for editing contact points', async () => { - // two alert managers configured to receive alerts mockOneAlertManager(); - mockPreviewApiResponse(server, [{ labels: [{ tomato: 'red', avocate: 'green' }] }]); - mockHasEditPermission(false); + mockPreviewApiResponse(server, [ + mockAlertmanagerAlert({ + labels: { tomato: 'red', avocate: 'green' }, + }), + ]); const { user } = render( ); - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); + // wait for loading to finish + await waitFor(async () => { + const matchingContactPoint = await ui.contactPointGroup.findAll(); + expect(matchingContactPoint).toHaveLength(1); }); - await user.click(ui.previewButton.get()); + // expand the matching contact point to show instances + await user.click(await ui.expandButton.find()); + + // click "view route" await user.click(await ui.seeDetails.find()); - expect(ui.details.title.query()).toBeInTheDocument(); - //we expect seeing the default policy - expect(screen.getByText(/default policy/i)).toBeInTheDocument(); - const matchingPoliciesElements = within(ui.details.modal.get()).getAllByTestId('label-matchers'); - expect(matchingPoliciesElements).toHaveLength(1); - expect(matchingPoliciesElements[0]).toHaveTextContent(/tomato = red/); - expect(within(ui.details.modal.get()).getByText(/slack/i)).toBeInTheDocument(); - expect(ui.details.linkToContactPoint.query()).not.toBeInTheDocument(); - }); -}); -describe('NotificationPreviewByAlertmanager', () => { - it('should render route matching preview for alertmanager', async () => { - const potentialInstances: Labels[] = [ - { foo: 'bar', severity: 'critical' }, - { job: 'prometheus', severity: 'warning' }, - ]; + // grab drawer and assert within + const drawer = ui.details.drawer.getAll()[0]; + expect(drawer).toBeInTheDocument(); - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => rb.withReceiver('slack').addMatcher('severity', MatcherOperator.equal, 'critical')) - .addRoute((rb) => rb.withReceiver('opsgenie').addMatcher('team', MatcherOperator.equal, 'operations')) - ) - .addReceivers((b) => b.withName('email').addEmailConfig((eb) => eb.withTo('test@example.com'))) - .addReceivers((b) => b.withName('slack')) - .addReceivers((b) => b.withName('opsgenie')) - ); - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - const { user } = render( - - ); - - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); - }); - - const routeElements = ui.route.getAll(); - - expect(routeElements).toHaveLength(2); - expect(routeElements[0]).toHaveTextContent(/slack/); - expect(routeElements[1]).toHaveTextContent(/email/); - - await user.click(ui.routeButton.get(routeElements[0])); - await user.click(ui.routeButton.get(routeElements[1])); - - const matchingInstances0 = ui.routeMatchingInstances.get(routeElements[0]); - const matchingInstances1 = ui.routeMatchingInstances.get(routeElements[1]); - - expect(matchingInstances0).toHaveTextContent(/severity=critical/); - expect(matchingInstances0).toHaveTextContent(/foo=bar/); - - expect(matchingInstances1).toHaveTextContent(/job=prometheus/); - expect(matchingInstances1).toHaveTextContent(/severity=warning/); - }); - it('should render route matching preview for alertmanager without errors if receiver is inherited from parent route (no receiver) ', async () => { - const potentialInstances: Labels[] = [ - { foo: 'bar', severity: 'critical' }, - { job: 'prometheus', severity: 'warning' }, - ]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => { - rb.addRoute((rb) => rb.withoutReceiver().addMatcher('foo', MatcherOperator.equal, 'bar')); - return rb.withReceiver('slack').addMatcher('severity', MatcherOperator.equal, 'critical'); - }) - .addRoute((rb) => rb.withReceiver('opsgenie').addMatcher('team', MatcherOperator.equal, 'operations')) - ) - .addReceivers((b) => b.withName('email').addEmailConfig((eb) => eb.withTo('test@example.com'))) - .addReceivers((b) => b.withName('slack')) - .addReceivers((b) => b.withName('opsgenie')) - ); - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - const { user } = render( - - ); - - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); - }); - - const routeElements = ui.route.getAll(); - - expect(routeElements).toHaveLength(2); - expect(routeElements[0]).toHaveTextContent(/slack/); - expect(routeElements[1]).toHaveTextContent(/email/); - - await user.click(ui.routeButton.get(routeElements[0])); - await user.click(ui.routeButton.get(routeElements[1])); - - const matchingInstances0 = ui.routeMatchingInstances.get(routeElements[0]); - const matchingInstances1 = ui.routeMatchingInstances.get(routeElements[1]); - - expect(matchingInstances0).toHaveTextContent(/severity=critical/); - expect(matchingInstances0).toHaveTextContent(/foo=bar/); - - expect(matchingInstances1).toHaveTextContent(/job=prometheus/); - expect(matchingInstances1).toHaveTextContent(/severity=warning/); - }); - it('should render route matching preview for alertmanager without errors if receiver is inherited from parent route (empty string receiver)', async () => { - const potentialInstances: Labels[] = [ - { foo: 'bar', severity: 'critical' }, - { job: 'prometheus', severity: 'warning' }, - ]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => { - rb.addRoute((rb) => rb.withEmptyReceiver().addMatcher('foo', MatcherOperator.equal, 'bar')); - return rb.withReceiver('slack').addMatcher('severity', MatcherOperator.equal, 'critical'); - }) - .addRoute((rb) => rb.withReceiver('opsgenie').addMatcher('team', MatcherOperator.equal, 'operations')) - ) - .addReceivers((b) => b.withName('email').addEmailConfig((eb) => eb.withTo('test@example.com'))) - .addReceivers((b) => b.withName('slack')) - .addReceivers((b) => b.withName('opsgenie')) - ); - - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - const { user } = render( - - ); - - await waitFor(() => { - expect(ui.loadingIndicator.query()).not.toBeInTheDocument(); - }); - - const routeElements = ui.route.getAll(); - - expect(routeElements).toHaveLength(2); - expect(routeElements[0]).toHaveTextContent(/slack/); - expect(routeElements[1]).toHaveTextContent(/email/); - - await user.click(ui.routeButton.get(routeElements[0])); - await user.click(ui.routeButton.get(routeElements[1])); - - const matchingInstances0 = ui.routeMatchingInstances.get(routeElements[0]); - const matchingInstances1 = ui.routeMatchingInstances.get(routeElements[1]); - - expect(matchingInstances0).toHaveTextContent(/severity=critical/); - expect(matchingInstances0).toHaveTextContent(/foo=bar/); - - expect(matchingInstances1).toHaveTextContent(/job=prometheus/); - expect(matchingInstances1).toHaveTextContent(/severity=warning/); - }); - - describe('regex matching', () => { - it('does not match regex in middle of the word as alertmanager will anchor when queried via API', async () => { - const potentialInstances: Labels[] = [{ regexfield: 'foobarfoo' }]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .addReceivers((b) => b.withName('email')) - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => rb.withReceiver('email').addMatcher('regexfield', MatcherOperator.regex, 'bar')) - ) - ); - - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - render( - - ); - - expect(await screen.findByText(/default policy/i)).toBeInTheDocument(); - expect(screen.queryByText(/regexfield/)).not.toBeInTheDocument(); - }); - - it('matches regex at the start of the word', async () => { - const potentialInstances: Labels[] = [{ regexfield: 'baaaaaaah' }]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .addReceivers((b) => b.withName('email')) - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => rb.withReceiver('email').addMatcher('regexfield', MatcherOperator.regex, 'ba.*h')) - ) - ); - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - render( - - ); - - expect(await screen.findByText(/regexfield/i)).toBeInTheDocument(); - }); - - it('handles negated regex correctly', async () => { - const potentialInstances: Labels[] = [{ regexfield: 'thing' }]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .addReceivers((b) => b.withName('email')) - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => rb.withReceiver('email').addMatcher('regexfield', MatcherOperator.notRegex, 'thing')) - ) - ); - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - render( - - ); - - expect(await screen.findByText(/default policy/i)).toBeInTheDocument(); - expect(screen.queryByText(/regexfield/i)).not.toBeInTheDocument(); - }); - }); - it('matches regex with flags', async () => { - const potentialInstances: Labels[] = [{ regexfield: 'baaaaaaah' }]; - - const mockConfig = getMockConfig((amConfigBuilder) => - amConfigBuilder - .addReceivers((b) => b.withName('email')) - .withRoute((routeBuilder) => - routeBuilder - .withReceiver('email') - .addRoute((rb) => rb.withReceiver('email').addMatcher('regexfield', MatcherOperator.regex, '(?i)BA.*h')) - ) - ); - setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, mockConfig); - - render( - - ); - - expect(await screen.findByText(/regexfield/i)).toBeInTheDocument(); + // assert within the drawer + expect(within(drawer).getByRole('heading', { name: 'Default policy' })).toBeInTheDocument(); + expect(within(drawer).getByText(/non-matching labels/i)).toBeInTheDocument(); + expect(ui.details.linkToPolicyTree.get()).toBeInTheDocument(); }); }); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx index 248ef0c00c2..c1396464c86 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx @@ -1,15 +1,18 @@ -import { compact } from 'lodash'; -import { Suspense, lazy } from 'react'; +import { css } from '@emotion/css'; +import { Fragment, Suspense, lazy } from 'react'; +import { useEffectOnce } from 'react-use'; +import { GrafanaTheme2 } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; -import { Button, LoadingPlaceholder, Stack, Text } from '@grafana/ui'; +import { Button, LoadingPlaceholder, Stack, Text, useStyles2 } from '@grafana/ui'; import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; -import { AlertQuery } from 'app/types/unified-alerting-dto'; +import { AlertQuery, Labels } from 'app/types/unified-alerting-dto'; import { Folder, KBObjectArray } from '../../../types/rule-form'; import { useGetAlertManagerDataSourcesByPermissionAndConfig } from '../../../utils/datasource'; const NotificationPreviewByAlertManager = lazy(() => import('./NotificationPreviewByAlertManager')); +const NotificationPreviewForGrafanaManaged = lazy(() => import('./NotificationPreviewGrafanaManaged')); interface NotificationPreviewProps { customLabels: KBObjectArray; @@ -20,6 +23,8 @@ interface NotificationPreviewProps { alertUid?: string; } +const { preview } = alertRuleApi.endpoints; + // TODO the scroll position keeps resetting when we preview // this is to be expected because the list of routes dissapears as we start the request but is very annoying export const NotificationPreview = ({ @@ -30,15 +35,20 @@ export const NotificationPreview = ({ alertName, alertUid, }: NotificationPreviewProps) => { + const styles = useStyles2(getStyles); const disabled = !condition || !folder; - const previewEndpoint = alertRuleApi.endpoints.preview; - - const [trigger, { data = [], isLoading, isUninitialized: previewUninitialized }] = previewEndpoint.useMutation(); + const [trigger, { data = [], isLoading, isUninitialized: previewUninitialized }] = preview.useMutation(); // potential instances are the instances that are going to be routed to the notification policies // convert data to list of labels: are the representation of the potential instances - const potentialInstances = compact(data.flatMap((label) => label?.labels)); + const potentialInstances = data.reduce((acc = [], instance) => { + if (instance.labels) { + acc.push(instance.labels); + } + + return acc; + }, []); const onPreview = () => { if (!folder || !condition) { @@ -56,10 +66,13 @@ export const NotificationPreview = ({ }); }; + useEffectOnce(() => { + onPreview(); + }); + // Get alert managers's data source information const alertManagerDataSources = useGetAlertManagerDataSourcesByPermissionAndConfig('notification'); - - const onlyOneAM = alertManagerDataSources.length === 1; + const singleAlertManagerConfigured = alertManagerDataSources.length === 1; return ( @@ -93,22 +106,63 @@ export const NotificationPreview = ({ Preview routing - {!isLoading && !previewUninitialized && potentialInstances.length > 0 && ( + {potentialInstances.length > 0 && ( } > {alertManagerDataSources.map((alertManagerSource) => ( - + + {!singleAlertManagerConfigured && ( + +
+
+ Alertmanager: + + {alertManagerSource.name} +
+
+ + )} + {alertManagerSource.name === 'grafana' ? ( + + ) : ( + + )} + ))} )} ); }; + +const getStyles = (theme: GrafanaTheme2) => ({ + firstAlertManagerLine: css({ + height: '1px', + width: theme.spacing(4), + backgroundColor: theme.colors.secondary.main, + }), + alertManagerName: css({ + width: 'fit-content', + }), + secondAlertManagerLine: css({ + height: '1px', + width: '100%', + flex: 1, + backgroundColor: theme.colors.secondary.main, + }), + img: css({ + marginLeft: theme.spacing(2), + width: theme.spacing(3), + height: theme.spacing(3), + marginRight: theme.spacing(1), + }), +}); 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 d40f2f3de67..564b8eb3a92 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 @@ -1,31 +1,29 @@ -import { css } from '@emotion/css'; +import { groupBy } from 'lodash'; -import { GrafanaTheme2 } from '@grafana/data'; -import { Trans, t } from '@grafana/i18n'; -import { Alert, LoadingPlaceholder, useStyles2, withErrorBoundary } from '@grafana/ui'; +import { t } from '@grafana/i18n'; +import { Alert, Box, LoadingPlaceholder, withErrorBoundary } from '@grafana/ui'; import { stringifyErrorLike } from 'app/features/alerting/unified/utils/misc'; import { Stack } from '../../../../../../plugins/datasource/parca/QueryEditor/Stack'; import { Labels } from '../../../../../../types/unified-alerting-dto'; import { AlertManagerDataSource } from '../../../utils/datasource'; -import { NotificationRoute } from './NotificationRoute'; +import { ExternalContactPointGroup } from './ContactPointGroup'; +import { InstanceMatch } from './NotificationRoute'; import { useAlertmanagerNotificationRoutingPreview } from './useAlertmanagerNotificationRoutingPreview'; +const UNKNOWN_RECEIVER = 'unknown'; + function NotificationPreviewByAlertManager({ alertManagerSource, - potentialInstances, - onlyOneAM, + instances, }: { alertManagerSource: AlertManagerDataSource; - potentialInstances: Labels[]; - onlyOneAM: boolean; + instances: Labels[]; }) { - const styles = useStyles2(getStyles); - - const { routesByIdMap, receiversByName, matchingMap, loading, error } = useAlertmanagerNotificationRoutingPreview( + const { treeMatchingResults, isLoading, error } = useAlertmanagerNotificationRoutingPreview( alertManagerSource.name, - potentialInstances + instances ); if (error) { @@ -39,7 +37,7 @@ function NotificationPreviewByAlertManager({ ); } - if (loading) { + if (isLoading) { return ( 0; + const matchingPoliciesFound = treeMatchingResults.some((result) => result.matchedRoutes.length > 0); + + // Group results by receiver name + // We need to flatten the structure first to group by receiver + const flattenedResults = treeMatchingResults.flatMap(({ labels, matchedRoutes }) => { + return Array.from(matchedRoutes).map(({ route, routeTree, matchDetails }) => ({ + labels, + receiver: route.receiver || UNKNOWN_RECEIVER, + routeTree, + matchDetails, + })); + }); + + const contactPointGroups = groupBy(flattenedResults, 'receiver'); return matchingPoliciesFound ? ( -
- {!onlyOneAM && ( - -
-
- Alertmanager: - - {alertManagerSource.name} -
-
- - )} - - {Array.from(matchingMap.entries()).map(([routeId, instanceMatches]) => { - const route = routesByIdMap.get(routeId); - const receiver = route?.receiver && receiversByName.get(route.receiver); - - if (!route) { - return null; - } - return ( - - ); - })} + + + {Object.entries(contactPointGroups).map(([receiver, resultsForReceiver]) => ( + + + {resultsForReceiver.map(({ routeTree, matchDetails }) => ( + + ))} + + + ))} -
+ ) : null; } // export default because we want to load the component dynamically using React.lazy // Due to loading of the web worker we don't want to load this component when not necessary export default withErrorBoundary(NotificationPreviewByAlertManager); - -const getStyles = (theme: GrafanaTheme2) => ({ - alertManagerRow: css({ - display: 'flex', - flexDirection: 'column', - gap: theme.spacing(1), - width: '100%', - }), - firstAlertManagerLine: css({ - height: '1px', - width: theme.spacing(4), - backgroundColor: theme.colors.secondary.main, - }), - alertManagerName: css({ - width: 'fit-content', - }), - secondAlertManagerLine: css({ - height: '1px', - width: '100%', - flex: 1, - backgroundColor: theme.colors.secondary.main, - }), - img: css({ - marginLeft: theme.spacing(2), - width: theme.spacing(3), - height: theme.spacing(3), - marginRight: theme.spacing(1), - }), -}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewGrafanaManaged.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewGrafanaManaged.tsx new file mode 100644 index 00000000000..2a5cc020996 --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreviewGrafanaManaged.tsx @@ -0,0 +1,90 @@ +import { groupBy } from 'lodash'; + +import { t } from '@grafana/i18n'; +import { Alert, Box, LoadingPlaceholder, withErrorBoundary } from '@grafana/ui'; +import { stringifyErrorLike } from 'app/features/alerting/unified/utils/misc'; + +import { Stack } from '../../../../../../plugins/datasource/parca/QueryEditor/Stack'; +import { Labels } from '../../../../../../types/unified-alerting-dto'; +import { AlertManagerDataSource } from '../../../utils/datasource'; + +import { GrafanaContactPointGroup } from './ContactPointGroup'; +import { InstanceMatch } from './NotificationRoute'; +import { useAlertmanagerNotificationRoutingPreview } from './useAlertmanagerNotificationRoutingPreview'; + +const UNKNOWN_RECEIVER = 'unknown'; + +function NotificationPreviewGrafanaManaged({ + alertManagerSource, + instances, +}: { + alertManagerSource: AlertManagerDataSource; + instances: Labels[]; +}) { + const { treeMatchingResults, isLoading, error } = useAlertmanagerNotificationRoutingPreview( + alertManagerSource.name, + instances + ); + + if (error) { + const title = t('alerting.notification-preview.error', 'Could not load routing preview for {{alertmanager}}', { + alertmanager: alertManagerSource.name, + }); + return ( + + {stringifyErrorLike(error)} + + ); + } + + if (isLoading) { + return ( + + ); + } + + const matchingPoliciesFound = treeMatchingResults.some((result) => result.matchedRoutes.length > 0); + + // Group results by receiver name + // We need to flatten the structure first to group by receiver + const flattenedResults = treeMatchingResults.flatMap(({ labels, matchedRoutes }) => { + return Array.from(matchedRoutes).map(({ route, routeTree, matchDetails }) => ({ + labels, + receiver: route.receiver || UNKNOWN_RECEIVER, + routeTree, + matchDetails, + })); + }); + + const contactPointGroups = groupBy(flattenedResults, 'receiver'); + + return matchingPoliciesFound ? ( + + + {Object.entries(contactPointGroups).map(([receiver, resultsForReceiver]) => ( + + + {resultsForReceiver.map(({ routeTree, matchDetails }) => ( + + ))} + + + ))} + + + ) : null; +} + +// export default because we want to load the component dynamically using React.lazy +// Due to loading of the web worker we don't want to load this component when not necessary +export default withErrorBoundary(NotificationPreviewGrafanaManaged); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx index 421c25b2ec3..67bdce2d4d2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx @@ -1,263 +1,66 @@ -import { css, cx } from '@emotion/css'; -import { uniqueId } from 'lodash'; -import pluralize from 'pluralize'; -import { useState } from 'react'; -import { useToggle } from 'react-use'; +import { css } from '@emotion/css'; +import { AlertLabels, RouteMatchResult, RouteWithID } from '@grafana/alerting/unstable'; import { GrafanaTheme2 } from '@grafana/data'; -import { Trans, t } from '@grafana/i18n'; -import { Button, TagList, getTagColorIndexFromName, useStyles2 } from '@grafana/ui'; +import { Trans } from '@grafana/i18n'; +import { Text, useStyles2 } from '@grafana/ui'; -import { Receiver } from '../../../../../../plugins/datasource/alertmanager/types'; import { Stack } from '../../../../../../plugins/datasource/parca/QueryEditor/Stack'; -import { getAmMatcherFormatter } from '../../../utils/alertmanager'; -import { AlertInstanceMatch } from '../../../utils/notification-policies'; -import { CollapseToggle } from '../../CollapseToggle'; -import { MetaText } from '../../MetaText'; +import { arrayLabelsToObject } from '../../../utils/labels'; import { Spacer } from '../../Spacer'; -import { NotificationPolicyMatchers } from './NotificationPolicyMatchers'; -import { NotificationRouteDetailsModal } from './NotificationRouteDetailsModal'; -import UnknownContactPointDetails from './UnknownContactPointDetails'; -import { RouteWithPath } from './route'; +import { NotificationPolicyDrawer } from './NotificationPolicyDrawer'; -export interface ReceiverNameProps { - /** Receiver name taken from route definition. Used as a fallback when full receiver details cannot be found (in case of RBAC restrictions) */ - receiverNameFromRoute?: string; -} +type TreeMeta = { + name?: string; +}; -interface NotificationRouteHeaderProps extends ReceiverNameProps { - route: RouteWithPath; - receiver?: Receiver; - routesByIdMap: Map; - instancesCount: number; - alertManagerSourceName: string; - expandRoute: boolean; - onExpandRouteClick: (expand: boolean) => void; -} +type InstanceMatchProps = { + matchedInstance: RouteMatchResult; + policyTreeSpec: RouteWithID; + policyTreeMetadata: TreeMeta; +}; -function NotificationRouteHeader({ - route, - receiver, - receiverNameFromRoute, - routesByIdMap, - instancesCount, - alertManagerSourceName, - expandRoute, - onExpandRouteClick, -}: NotificationRouteHeaderProps) { +export function InstanceMatch({ matchedInstance, policyTreeSpec, policyTreeMetadata }: InstanceMatchProps) { const styles = useStyles2(getStyles); - const [showDetails, setShowDetails] = useState(false); - const onClickDetails = () => { - setShowDetails(true); - }; + const { labels, matchingJourney, route } = matchedInstance; - // @TODO: re-use component ContactPointsHoverDetails from Policy once we have it for cloud AMs. - - return ( -
- onExpandRouteClick(!isCollapsed)} - aria-label={t('alerting.notification-route-header.aria-label-expand-policy-route', 'Expand policy route')} - /> - - - {/* TODO: fix keyboard a11y */} - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} -
onExpandRouteClick(!expandRoute)} className={styles.expandable}> - - Notification policy - - -
- - - - {instancesCount ?? '-'} {pluralize('instance', instancesCount)} - - -
- - @ Delivered to - {' '} - {receiver ? receiver.name : } -
- -
- - - - - - {showDetails && ( - setShowDetails(false)} - route={route} - receiver={receiver} - receiverNameFromRoute={receiverNameFromRoute} - routesByIdMap={routesByIdMap} - alertManagerSourceName={alertManagerSourceName} - /> - )} -
+ // Get all match details from the final matched route in the journey + const finalRouteMatchInfo = matchingJourney.at(-1); + const routeMatchLabels = arrayLabelsToObject( + finalRouteMatchInfo?.matchDetails.map((detail) => labels[detail.labelIndex]) ?? [] ); -} - -interface NotificationRouteProps extends ReceiverNameProps { - route: RouteWithPath; - receiver?: Receiver; - instanceMatches: AlertInstanceMatch[]; - routesByIdMap: Map; - alertManagerSourceName: string; -} - -export function NotificationRoute({ - route, - instanceMatches, - receiver, - receiverNameFromRoute, - routesByIdMap, - alertManagerSourceName, -}: NotificationRouteProps) { - const styles = useStyles2(getStyles); - const [expandRoute, setExpandRoute] = useToggle(false); - // @TODO: The color index might be updated at some point in the future.Maybe we should roll our own tag component, - // one that supports a custom function to define the color and allow manual color overrides - const GREY_COLOR_INDEX = 9; + const matchedRootRoute = route.id === policyTreeSpec.id; return ( -
- - {expandRoute && ( - -
- {instanceMatches.map((instanceMatch) => { - const matchArray = Array.from(instanceMatch.labelsMatch); - const matchResult = matchArray.map(([label, matchResult]) => ({ - label: `${label[0]}=${label[1]}`, - match: matchResult.match, - colorIndex: matchResult.match ? getTagColorIndexFromName(label[0]) : GREY_COLOR_INDEX, - })); - - const matchingLabels = matchResult.filter((mr) => mr.match); - const nonMatchingLabels = matchResult.filter((mr) => !mr.match); - - return ( -
- {matchArray.length > 0 ? ( - <> - {matchingLabels.length > 0 ? ( - mr.label)} - className={styles.labelList} - getColorIndex={(_, index) => matchingLabels[index].colorIndex} - /> - ) : ( -
- No matching labels -
- )} -
- mr.label)} - className={styles.labelList} - getColorIndex={(_, index) => nonMatchingLabels[index].colorIndex} - /> - - ) : ( -
- No labels -
- )} -
- ); - })} -
- - )} +
+ + {labels.length > 0 ? ( + + ) : ( + + No labels + + )} + + +
); } const getStyles = (theme: GrafanaTheme2) => ({ - textMuted: css({ - color: theme.colors.text.secondary, - }), - textItalic: css({ - fontStyle: 'italic', - }), - expandable: css({ - cursor: 'pointer', - }), - routeHeader: css({ - display: 'flex', - flexDirection: 'row', - gap: theme.spacing(1), - alignItems: 'center', - borderBottom: `1px solid ${theme.colors.border.weak}`, - padding: theme.spacing(0.5, 0.5, 0.5, 0), + instanceListItem: css({ + padding: theme.spacing(1, 2), + '&:hover': { backgroundColor: theme.components.table.rowHoverBackground, }, }), - labelList: css({ - flex: '0 1 auto', - justifyContent: 'flex-start', - }), - labelSeparator: css({ - width: '1px', - backgroundColor: theme.colors.border.weak, - }), - tagListCard: css({ - display: 'flex', - flexDirection: 'row', - gap: theme.spacing(2), - - position: 'relative', - background: theme.colors.background.secondary, - padding: theme.spacing(1), - - borderRadius: theme.shape.borderRadius(2), - border: `solid 1px ${theme.colors.border.weak}`, - }), - routeInstances: css({ - padding: theme.spacing(1, 0, 1, 4), - position: 'relative', - - display: 'flex', - flexDirection: 'column', - gap: theme.spacing(1), - - '&:before': { - content: '""', - position: 'absolute', - left: theme.spacing(2), - height: `calc(100% - ${theme.spacing(2)})`, - width: theme.spacing(4), - borderLeft: `solid 1px ${theme.colors.border.weak}`, - }, - }), - verticalBar: css({ - width: '1px', - height: '20px', - backgroundColor: theme.colors.secondary.main, - marginLeft: theme.spacing(1), - marginRight: theme.spacing(1), - }), }); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRouteDetailsModal.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRouteDetailsModal.tsx deleted file mode 100644 index 74394a57677..00000000000 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRouteDetailsModal.tsx +++ /dev/null @@ -1,200 +0,0 @@ -import { css, cx } from '@emotion/css'; -import { compact } from 'lodash'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { Trans, t } from '@grafana/i18n'; -import { Button, Modal, Stack, TextLink, useStyles2 } from '@grafana/ui'; - -import { Receiver } from '../../../../../../plugins/datasource/alertmanager/types'; -import { AlertmanagerAction } from '../../../hooks/useAbilities'; -import { AlertmanagerProvider } from '../../../state/AlertmanagerContext'; -import { getAmMatcherFormatter } from '../../../utils/alertmanager'; -import { MatcherFormatter } from '../../../utils/matchers'; -import { createContactPointSearchLink } from '../../../utils/misc'; -import { Authorize } from '../../Authorize'; -import { Matchers } from '../../notification-policies/Matchers'; - -import { ReceiverNameProps } from './NotificationRoute'; -import UnknownContactPointDetails from './UnknownContactPointDetails'; -import { RouteWithPath, hasEmptyMatchers, isDefaultPolicy } from './route'; - -interface Props { - routesByIdMap: Map; - route: RouteWithPath; - matcherFormatter: MatcherFormatter; -} - -function PolicyPath({ route, routesByIdMap, matcherFormatter }: Props) { - const styles = useStyles2(getStyles); - const routePathIds = route.path?.slice(1) ?? []; - const routePathObjects = [...compact(routePathIds.map((id) => routesByIdMap.get(id))), route]; - - return ( -
-
- Default policy -
- {routePathObjects.map((pathRoute, index) => { - return ( -
-
- {hasEmptyMatchers(pathRoute) ? ( -
- No matchers -
- ) : ( - - )} -
-
- ); - })} -
- ); -} - -interface NotificationRouteDetailsModalProps extends ReceiverNameProps { - onClose: () => void; - route: RouteWithPath; - receiver?: Receiver; - routesByIdMap: Map; - alertManagerSourceName: string; -} - -export function NotificationRouteDetailsModal({ - onClose, - route, - receiver, - receiverNameFromRoute, - routesByIdMap, - alertManagerSourceName, -}: NotificationRouteDetailsModalProps) { - const styles = useStyles2(getStyles); - - const isDefault = isDefaultPolicy(route); - - return ( - - - -
- - Your alert instances are routed as follows. - -
-
- - Notification policy path - -
- {isDefault && ( -
- Default policy -
- )} -
- {!isDefault && ( - - )} -
-
- - Contact point - - - {receiver ? receiver.name : } - - - - - {receiver ? ( - - See details - - ) : null} - - -
-
- -
- - - - ); -} - -const getStyles = (theme: GrafanaTheme2) => ({ - textMuted: css({ - color: theme.colors.text.secondary, - }), - link: css({ - display: 'block', - color: theme.colors.text.link, - }), - button: css({ - justifyContent: 'flex-end', - display: 'flex', - }), - detailsModal: css({ - maxWidth: '560px', - }), - defaultPolicy: css({ - padding: theme.spacing(0.5), - background: theme.colors.background.secondary, - width: 'fit-content', - }), - contactPoint: css({ - display: 'flex', - flexDirection: 'row', - gap: theme.spacing(1), - alignItems: 'center', - justifyContent: 'space-between', - marginBottom: theme.spacing(1), - }), - policyPathWrapper: css({ - display: 'flex', - flexDirection: 'column', - marginTop: theme.spacing(1), - }), - separator: (units: number) => - css({ - marginTop: theme.spacing(units), - }), - marginBottom: (units: number) => - css({ - marginBottom: theme.spacing(theme.spacing(units)), - }), - policyInPath: (index = 0, highlight = false) => - css({ - marginLeft: `${30 + index * 30}px`, - padding: theme.spacing(1), - marginTop: theme.spacing(1), - border: `solid 1px ${highlight ? theme.colors.info.border : theme.colors.border.weak}`, - background: theme.colors.background.secondary, - width: 'fit-content', - position: 'relative', - '&:before': { - content: '""', - position: 'absolute', - height: 'calc(100% - 10px)', - width: theme.spacing(1), - borderLeft: `solid 1px ${theme.colors.border.weak}`, - borderBottom: `solid 1px ${theme.colors.border.weak}`, - marginTop: theme.spacing(-2), - marginLeft: `-17px`, - }, - }), -}); diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/route.ts b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/route.ts deleted file mode 100644 index 8e2647400be..00000000000 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/route.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { RouteWithID } from '../../../../../../plugins/datasource/alertmanager/types'; - -export interface RouteWithPath extends RouteWithID { - path: string[]; // path from root route to this route -} - -export function isDefaultPolicy(route: RouteWithPath) { - return route.path?.length === 0; -} - -// we traverse the whole tree and we create a map with -export function getRoutesByIdMap(rootRoute: RouteWithID): Map { - const map = new Map(); - - function addRoutesToMap(route: RouteWithID, path: string[] = []) { - map.set(route.id, { ...route, path: path }); - route.routes?.forEach((r) => addRoutesToMap(r, [...path, route.id])); - } - - addRoutesToMap(rootRoute, []); - return map; -} - -export function hasEmptyMatchers(route: RouteWithID) { - return route.object_matchers?.length === 0; -} diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts index aeec03ffea1..2f95fe40d52 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts @@ -1,36 +1,23 @@ import { useMemo } from 'react'; import { useAsync } from 'react-use'; -import { useContactPointsWithStatus } from 'app/features/alerting/unified/components/contact-points/useContactPoints'; import { useNotificationPolicyRoute } from 'app/features/alerting/unified/components/notification-policies/useNotificationPolicyRoute'; -import { Receiver } from '../../../../../../plugins/datasource/alertmanager/types'; import { Labels } from '../../../../../../types/unified-alerting-dto'; import { useRouteGroupsMatcher } from '../../../useRouteGroupsMatcher'; import { addUniqueIdentifierToRoute } from '../../../utils/amroutes'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; -import { AlertInstanceMatch, computeInheritedTree, normalizeRoute } from '../../../utils/notification-policies'; +import { normalizeRoute } from '../../../utils/notification-policies'; -import { RouteWithPath, getRoutesByIdMap } from './route'; - -export const useAlertmanagerNotificationRoutingPreview = (alertmanager: string, potentialInstances: Labels[]) => { +export const useAlertmanagerNotificationRoutingPreview = (alertmanager: string, instances: Labels[]) => { const { data: currentData, isLoading: isPoliciesLoading, error: policiesError, } = useNotificationPolicyRoute({ alertmanager }); - const { - contactPoints, - isLoading: contactPointsLoading, - error: contactPointsError, - } = useContactPointsWithStatus({ - alertmanager, - fetchPolicies: false, - fetchStatuses: false, - }); - - const { matchInstancesToRoute } = useRouteGroupsMatcher(); + // this function will use a web worker to compute matching routes + const { matchInstancesToRoutes } = useRouteGroupsMatcher(); const [defaultPolicy] = currentData ?? []; const rootRoute = useMemo(() => { @@ -40,27 +27,9 @@ export const useAlertmanagerNotificationRoutingPreview = (alertmanager: string, return normalizeRoute(addUniqueIdentifierToRoute(defaultPolicy)); }, [defaultPolicy]); - // create maps for routes to be get by id, this map also contains the path to the route - // ⚠️ don't forget to compute the inherited tree before using this map - const routesByIdMap = rootRoute - ? getRoutesByIdMap(computeInheritedTree(rootRoute)) - : new Map(); - - // to create the list of matching contact points we need to first get the rootRoute - const receiversByName = useMemo(() => { - if (!contactPoints) { - return new Map(); - } - - // create map for receivers to be get by name - return contactPoints.reduce((map, receiver) => { - return map.set(receiver.name, receiver); - }, new Map()); - }, [contactPoints]); - // match labels in the tree => map of notification policies and the alert instances (list of labels) in each one const { - value: matchingMap = new Map(), + value: treeMatchingResults = [], loading: matchingLoading, error: matchingError, } = useAsync(async () => { @@ -68,16 +37,14 @@ export const useAlertmanagerNotificationRoutingPreview = (alertmanager: string, return; } - return await matchInstancesToRoute(rootRoute, potentialInstances, { + return await matchInstancesToRoutes(rootRoute, instances, { unquoteMatchers: alertmanager !== GRAFANA_RULES_SOURCE_NAME, }); - }, [rootRoute, potentialInstances]); + }, [rootRoute, instances]); return { - routesByIdMap, - receiversByName, - matchingMap, - loading: isPoliciesLoading || contactPointsLoading || matchingLoading, - error: policiesError ?? contactPointsError ?? matchingError, + treeMatchingResults, + isLoading: isPoliciesLoading || matchingLoading, + error: policiesError ?? matchingError, }; }; diff --git a/public/app/features/alerting/unified/routeGroupsMatcher.ts b/public/app/features/alerting/unified/routeGroupsMatcher.ts index bce330c7f78..61b74cea8ff 100644 --- a/public/app/features/alerting/unified/routeGroupsMatcher.ts +++ b/public/app/features/alerting/unified/routeGroupsMatcher.ts @@ -1,13 +1,10 @@ +import { InstanceMatchResult, matchInstancesToRoute } from '@grafana/alerting/unstable'; + import { AlertmanagerGroup, RouteWithID } from '../../../plugins/datasource/alertmanager/types'; import { Labels } from '../../../types/unified-alerting-dto'; -import { - AlertInstanceMatch, - findMatchingAlertGroups, - findMatchingRoutes, - normalizeRoute, - unquoteRouteMatchers, -} from './utils/notification-policies'; +import { findMatchingAlertGroups, normalizeRoute, unquoteRouteMatchers } from './utils/notification-policies'; +import { routeAdapter } from './utils/routeAdapter'; export interface MatchOptions { unquoteMatchers?: boolean; @@ -34,29 +31,39 @@ export const routeGroupsMatcher = { return routeGroupsMap; }, - matchInstancesToRoute( - routeTree: RouteWithID, - instancesToMatch: Labels[], - options?: MatchOptions - ): Map { - const result = new Map(); + matchInstancesToRoutes(routeTree: RouteWithID, instances: Labels[], options?: MatchOptions): InstanceMatchResult[] { + const normalizedRouteTree = getNormalizedRoute(routeTree, options); - const normalizedRootRoute = getNormalizedRoute(routeTree, options); + // Convert all instances to labels format and match them all at once + const allLabels = instances.map((instance) => Object.entries(instance)); - instancesToMatch.forEach((instance) => { - const matchingRoutes = findMatchingRoutes(normalizedRootRoute, Object.entries(instance)); - matchingRoutes.forEach(({ route, labelsMatch }) => { - const currentRoute = result.get(route.id); + // Convert the RouteWithID to the alerting package format to ensure compatibility + const convertedRoute = routeAdapter.toPackage(normalizedRouteTree); + const { expandedTree, matchedPolicies } = matchInstancesToRoute(convertedRoute, allLabels); - if (currentRoute) { - currentRoute.push({ instance, labelsMatch }); - } else { - result.set(route.id, [{ instance, labelsMatch }]); - } - }); + // Group results by instance + return instances.map((instance, index) => { + const labels = allLabels[index]; + + // Collect matches for this specific instance + const allMatchedRoutes = Array.from(matchedPolicies.entries()).flatMap(([route, results]) => + results + .filter((matchDetails) => matchDetails.labels === labels) + .map((matchDetails) => ({ + route, + routeTree: { + metadata: { name: 'user-defined' }, + expandedSpec: expandedTree, + }, + matchDetails, + })) + ); + + return { + labels, + matchedRoutes: allMatchedRoutes, + }; }); - - return result; }, }; diff --git a/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx index 8bd3b28e933..8b31e3a946a 100644 --- a/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx @@ -24,7 +24,7 @@ import { mockRulerGrafanaRule, mockRulerRuleGroup, } from '../mocks'; -import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { grafanaRulerRule, mockPreviewApiResponse } from '../mocks/grafanaRulerApi'; import { mockRulerRulesApiResponse, mockRulerRulesGroupApiResponse } from '../mocks/rulerApi'; import { setFolderResponse } from '../mocks/server/configure'; import { AlertingQueryRunner } from '../state/AlertingQueryRunner'; @@ -103,6 +103,7 @@ describe('CloneRuleEditor', function () { }; setupDataSources(dataSources.default); setFolderResponse(mockFolder(folder)); + mockPreviewApiResponse(server, []); }); describe('Grafana-managed rules', function () { diff --git a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx index eab7a703499..073fca99df5 100644 --- a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx @@ -10,7 +10,7 @@ import { AccessControlAction } from 'app/types/accessControl'; import { setupMswServer } from '../mockApi'; import { grantUserPermissions, mockDataSource, mockFolder } from '../mocks'; -import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { grafanaRulerRule, mockPreviewApiResponse } from '../mocks/grafanaRulerApi'; import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; import { setupDataSources } from '../testSetup/datasources'; import { Annotation } from '../utils/constants'; @@ -23,7 +23,7 @@ jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ jest.setTimeout(60 * 1000); -setupMswServer(); +const server = setupMswServer(); function renderRuleEditor(identifier: string) { return render( @@ -85,6 +85,7 @@ describe('RuleEditor grafana managed rules', () => { setupDataSources(dataSources.default); setFolderResponse(mockFolder(folder)); setFolderResponse(mockFolder(slashedFolder)); + mockPreviewApiResponse(server, []); }); it('can edit grafana managed rule', async () => { diff --git a/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx index 1e8d537b3b7..d5ed0d39b6b 100644 --- a/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx @@ -12,7 +12,12 @@ import { DashboardSearchItemType } from 'app/features/search/types'; import { AccessControlAction } from 'app/types/accessControl'; import { grantUserPermissions, mockDataSource, mockFolder } from '../mocks'; -import { grafanaRulerGroup, grafanaRulerGroup2, grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { + grafanaRulerGroup, + grafanaRulerGroup2, + grafanaRulerRule, + mockPreviewApiResponse, +} from '../mocks/grafanaRulerApi'; import { setFolderResponse } from '../mocks/server/configure'; import { captureRequests, serializeRequests } from '../mocks/server/events'; import { setupDataSources } from '../testSetup/datasources'; @@ -25,7 +30,7 @@ jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ jest.setTimeout(60 * 1000); -setupMswServer(); +const server = setupMswServer(); const dataSources = { default: mockDataSource( @@ -62,6 +67,8 @@ describe('RuleEditor grafana managed rules', () => { AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleExternalWrite, ]); + + mockPreviewApiResponse(server, []); }); it('can create new grafana managed alert', async () => { diff --git a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts index 421afe78bde..3a157b66534 100644 --- a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts +++ b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts @@ -75,19 +75,19 @@ export function useRouteGroupsMatcher() { [] ); - const matchInstancesToRoute = useCallback( - async (rootRoute: RouteWithID, instancesToMatch: Labels[], options?: MatchOptions) => { + const matchInstancesToRoutes = useCallback( + async (rootRoute: RouteWithID, instances: Labels[], options?: MatchOptions) => { validateWorker(routeMatcher); const startTime = performance.now(); - const result = await routeMatcher.matchInstancesToRoute(rootRoute, instancesToMatch, options); + const result = await routeMatcher.matchInstancesToRoutes(rootRoute, instances, options); const timeSpent = performance.now() - startTime; logInfo(`Instances Matched in ${timeSpent} ms`, { matchingTime: timeSpent.toString(), - instancesToMatchCount: instancesToMatch.length.toString(), + instancesToMatchCount: instances.length.toString(), // Counting all nested routes might be too time-consuming, so we only count the first level topLevelRoutesCount: rootRoute.routes?.length.toString() ?? '0', }); @@ -97,5 +97,5 @@ export function useRouteGroupsMatcher() { [] ); - return { getRouteGroupsMap, matchInstancesToRoute }; + return { getRouteGroupsMap, matchInstancesToRoutes }; } diff --git a/public/app/features/alerting/unified/utils/__snapshots__/notification-policies.test.ts.snap b/public/app/features/alerting/unified/utils/__snapshots__/notification-policies.test.ts.snap deleted file mode 100644 index 891dc986133..00000000000 --- a/public/app/features/alerting/unified/utils/__snapshots__/notification-policies.test.ts.snap +++ /dev/null @@ -1,56 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`matchLabels should match with non-equal matchers 1`] = ` -Map { - [ - "team", - "operations", - ] => { - "match": true, - "matcher": [ - "team", - "=", - "operations", - ], - }, -} -`; - -exports[`matchLabels should match with non-matching matchers 1`] = ` -Map { - [ - "team", - "operations", - ] => { - "match": true, - "matcher": [ - "team", - "=", - "operations", - ], - }, -} -`; - -exports[`matchLabels should not match with a set of matchers 1`] = ` -Map { - [ - "team", - "operations", - ] => { - "match": true, - "matcher": [ - "team", - "=", - "operations", - ], - }, - [ - "foo", - "bar", - ] => { - "match": false, - "matcher": null, - }, -} -`; diff --git a/public/app/features/alerting/unified/utils/alertmanager.ts b/public/app/features/alerting/unified/utils/alertmanager.ts index 7b2e4f163b7..b7b3f28576a 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.ts @@ -1,5 +1,6 @@ import { isEqual, uniqWith } from 'lodash'; +import { matchLabelsSet } from '@grafana/alerting/unstable'; import { SelectableValue } from '@grafana/data'; import { AlertManagerCortexConfig, @@ -17,7 +18,12 @@ import { MatcherFieldValue } from '../types/silence-form'; import { getAllDataSources } from './config'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './datasource'; import { objectLabelsToArray } from './labels'; -import { MatcherFormatter, matchLabelsSet, parsePromQLStyleMatcherLooseSafe, unquoteWithUnescape } from './matchers'; +import { + MatcherFormatter, + convertObjectMatcherToAlertingPackageMatcher, + parsePromQLStyleMatcherLooseSafe, + unquoteWithUnescape, +} from './matchers'; export function addDefaultsToAlertmanagerConfig(config: AlertManagerCortexConfig): AlertManagerCortexConfig { // add default receiver if it does not exist @@ -127,9 +133,9 @@ export function matcherToObjectMatcher(matcher: Matcher): ObjectMatcher { export function labelsMatchMatchers(labels: Labels, matchers: Matcher[]): boolean { const labelsArray = objectLabelsToArray(labels); - const objectMatchers = matchers.map(matcherToObjectMatcher); + const labelMatchers = matchers.map(matcherToObjectMatcher).map(convertObjectMatcherToAlertingPackageMatcher); - return matchLabelsSet(objectMatchers, labelsArray); + return matchLabelsSet(labelMatchers, labelsArray); } export function combineMatcherStrings(...matcherStrings: string[]): string { diff --git a/public/app/features/alerting/unified/utils/k8s/utils.ts b/public/app/features/alerting/unified/utils/k8s/utils.ts index 1a99f52b522..e75818cde66 100644 --- a/public/app/features/alerting/unified/utils/k8s/utils.ts +++ b/public/app/features/alerting/unified/utils/k8s/utils.ts @@ -49,3 +49,8 @@ export const canDeleteEntity = (k8sEntity: EntityToCheck) => export const encodeFieldSelector = (value: string): string => { return value.replaceAll(/\\/g, '\\\\').replaceAll(/\=/g, '\\=').replaceAll(/,/g, '\\,'); }; + +type FieldSelector = [string, string] | [string, string, '=' | '!=']; +export const stringifyFieldSelector = (fieldSelectors: FieldSelector[]): string => { + return fieldSelectors.map(([key, value, operator = '=']) => `${key}${operator}${value}`).join(','); +}; diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index 32e0f08555a..2103ee71eb4 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -7,7 +7,7 @@ import { chain, compact } from 'lodash'; -import { parseFlags } from '@grafana/data'; +import { type LabelMatcher } from '@grafana/alerting/unstable'; import { Matcher, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from '../../../../types/unified-alerting-dto'; @@ -228,6 +228,8 @@ export const matcherFormatter = { }, } as const; +export type MatcherFormatter = keyof typeof matcherFormatter; + export function isPromQLStyleMatcher(input: string): boolean { return input.startsWith('{') && input.endsWith('}'); } @@ -251,81 +253,12 @@ function matcherToOperator(matcher: Matcher): MatcherOperator { } } -// Compare set of matchers to set of label -export function matchLabelsSet(matchers: ObjectMatcher[], labels: Label[]): boolean { - for (const matcher of matchers) { - if (!isLabelMatchInSet(matcher, labels)) { - return false; - } - } - return true; +export function convertObjectMatcherToAlertingPackageMatcher(matcher: ObjectMatcher): LabelMatcher { + const [label, type, value] = matcher; + + return { + label, + type, + value, + }; } - -type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean; -const OperatorFunctions: Record = { - [MatcherOperator.equal]: (lv, mv) => lv === mv, - [MatcherOperator.notEqual]: (lv, mv) => lv !== mv, - // At the time of writing, Alertmanager compiles to another (anchored) Regular Expression, - // so we should also anchor our UI matches for consistency with this behaviour - // https://github.com/prometheus/alertmanager/blob/fd37ce9c95898ca68be1ab4d4529517174b73c33/pkg/labels/matcher.go#L69 - [MatcherOperator.regex]: (lv, mv) => { - const valueWithFlagsParsed = parseFlags(`^(?:${mv})$`); - const re = new RegExp(valueWithFlagsParsed.cleaned, valueWithFlagsParsed.flags); - return re.test(lv); - }, - [MatcherOperator.notRegex]: (lv, mv) => { - const valueWithFlagsParsed = parseFlags(`^(?:${mv})$`); - const re = new RegExp(valueWithFlagsParsed.cleaned, valueWithFlagsParsed.flags); - return !re.test(lv); - }, -}; - -function isLabelMatchInSet(matcher: ObjectMatcher, labels: Label[]): boolean { - const [matcherKey, operator, matcherValue] = matcher; - - let labelValue = ''; // matchers that have no labels are treated as empty string label values - const labelForMatcher = Object.fromEntries(labels)[matcherKey]; - if (labelForMatcher) { - labelValue = labelForMatcher; - } - - const matchFunction = OperatorFunctions[operator]; - if (!matchFunction) { - throw new Error(`no such operator: ${operator}`); - } - - try { - // This can throw because the regex operators use the JavaScript regex engine - // and "new RegExp()" throws on invalid regular expressions. - // - // This is usually a user-error (because matcher values are taken from user input) - // but we're still logging this as a warning because it _might_ be a programmer error. - return matchFunction(labelValue, matcherValue); - } catch (err) { - console.warn(err); - return false; - } -} - -// ⚠️ DO NOT USE THIS FUNCTION FOR ROUTE SELECTION ALGORITHM -// for route selection algorithm, always compare a single matcher to the entire label set -// see "matchLabelsSet" -export function isLabelMatch(matcher: ObjectMatcher, label: Label): boolean { - const [labelKey, labelValue] = label; - const [matcherKey, operator, matcherValue] = matcher; - - if (labelKey !== matcherKey) { - return false; - } - - const matchFunction = OperatorFunctions[operator]; - if (!matchFunction) { - throw new Error(`no such operator: ${operator}`); - } - - return matchFunction(labelValue, matcherValue); -} - -export type MatcherFormatter = keyof typeof matcherFormatter; - -export type Label = [string, string]; diff --git a/public/app/features/alerting/unified/utils/navigation.ts b/public/app/features/alerting/unified/utils/navigation.ts index 259207f41b7..a77828af2c6 100644 --- a/public/app/features/alerting/unified/utils/navigation.ts +++ b/public/app/features/alerting/unified/utils/navigation.ts @@ -1,3 +1,4 @@ +import { ObjectMatcher } from 'app/plugins/datasource/alertmanager/types'; import { RuleGroupIdentifierV2, RuleIdentifier } from 'app/types/unified-alerting'; import { createReturnTo } from '../hooks/useReturnTo'; @@ -89,3 +90,12 @@ export const rulesNav = { { skipSubPath: options?.skipSubPath } ), }; + +export const notificationPolicies = { + viewLink: (matchers: ObjectMatcher[], alertmanagerSourceName?: string) => { + return createRelativeUrl('/alerting/routes', { + queryString: matchers.map((matcher) => matcher.join('')).join(','), + alertmanager: alertmanagerSourceName ?? 'grafana', + }); + }, +}; diff --git a/public/app/features/alerting/unified/utils/notification-policies.test.ts b/public/app/features/alerting/unified/utils/notification-policies.test.ts index d6b83406edb..05e19092012 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.test.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.test.ts @@ -1,399 +1,6 @@ -import { MatcherOperator, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { MatcherOperator, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; -import { - InheritableProperties, - computeInheritedTree, - findMatchingRoutes, - getInheritedProperties, - matchLabels, - normalizeRoute, - unquoteRouteMatchers, -} from './notification-policies'; - -const CATCH_ALL_ROUTE: Route = { - receiver: 'ALL', - object_matchers: [], -}; - -describe('findMatchingRoutes', () => { - const policies: Route = { - receiver: 'ROOT', - group_by: ['grafana_folder'], - object_matchers: [], - routes: [ - { - receiver: 'A', - object_matchers: [['team', MatcherOperator.equal, 'operations']], - routes: [ - { - receiver: 'B1', - object_matchers: [['region', MatcherOperator.equal, 'europe']], - }, - { - receiver: 'B2', - object_matchers: [['region', MatcherOperator.equal, 'nasa']], - }, - ], - }, - { - receiver: 'C', - object_matchers: [['foo', MatcherOperator.equal, 'bar']], - }, - ], - group_wait: '10s', - group_interval: '1m', - }; - - it('should match root route with no matching labels', () => { - const matches = findMatchingRoutes(policies, []); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'ROOT'); - }); - - it('should match parent route with no matching children', () => { - const matches = findMatchingRoutes(policies, [['team', 'operations']]); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'A'); - }); - - it('should match route with negative matchers', () => { - const policiesWithNegative = { - ...policies, - routes: policies.routes?.concat({ - receiver: 'D', - object_matchers: [['name', MatcherOperator.notEqual, 'gilles']], - }), - }; - const matches = findMatchingRoutes(policiesWithNegative, [['name', 'konrad']]); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'D'); - }); - - it('should match child route of matching parent', () => { - const matches = findMatchingRoutes(policies, [ - ['team', 'operations'], - ['region', 'europe'], - ]); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'B1'); - }); - - it('should match simple policy', () => { - const matches = findMatchingRoutes(policies, [['foo', 'bar']]); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'C'); - }); - - it('should match catch-all route', () => { - const policiesWithAll: Route = { - ...policies, - routes: [CATCH_ALL_ROUTE, ...(policies.routes ?? [])], - }; - - const matches = findMatchingRoutes(policiesWithAll, []); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'ALL'); - }); - - it('should match multiple routes with continue', () => { - const policiesWithAll: Route = { - ...policies, - routes: [ - { - ...CATCH_ALL_ROUTE, - continue: true, - }, - ...(policies.routes ?? []), - ], - }; - - const matches = findMatchingRoutes(policiesWithAll, [['foo', 'bar']]); - expect(matches).toHaveLength(2); - expect(matches[0].route).toHaveProperty('receiver', 'ALL'); - expect(matches[1].route).toHaveProperty('receiver', 'C'); - }); - - it('should not match grandchild routes with same labels as parent', () => { - const policies: Route = { - receiver: 'PARENT', - group_by: ['grafana_folder'], - object_matchers: [['foo', MatcherOperator.equal, 'bar']], - routes: [ - { - receiver: 'CHILD', - object_matchers: [['baz', MatcherOperator.equal, 'qux']], - routes: [ - { - receiver: 'GRANDCHILD', - object_matchers: [['foo', MatcherOperator.equal, 'bar']], - }, - ], - }, - ], - group_wait: '10s', - group_interval: '1m', - }; - - const matches = findMatchingRoutes(policies, [['foo', 'bar']]); - expect(matches).toHaveLength(1); - expect(matches[0].route).toHaveProperty('receiver', 'PARENT'); - }); -}); - -describe('getInheritedProperties()', () => { - describe('group_by: []', () => { - it('should get group_by: [] from parent', () => { - const parent: Route = { - receiver: 'PARENT', - group_by: ['label'], - }; - - const child: Route = { - receiver: 'CHILD', - group_by: [], - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('group_by', ['label']); - }); - - it('should get group_by: [] from parent inherited properties', () => { - const parent: Route = { - receiver: 'PARENT', - group_by: [], - }; - - const child: Route = { - receiver: 'CHILD', - group_by: [], - }; - - const parentInherited = { group_by: ['label'] }; - - const childInherited = getInheritedProperties(parent, child, parentInherited); - expect(childInherited).toHaveProperty('group_by', ['label']); - }); - - it('should not inherit if the child overrides an inheritable value (group_by)', () => { - const parent: Route = { - receiver: 'PARENT', - group_by: ['parentLabel'], - }; - - const child: Route = { - receiver: 'CHILD', - group_by: ['childLabel'], - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).not.toHaveProperty('group_by'); - }); - - it('should inherit if group_by is undefined', () => { - const parent: Route = { - receiver: 'PARENT', - group_by: ['label'], - }; - - const child: Route = { - receiver: 'CHILD', - group_by: undefined, - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('group_by', ['label']); - }); - - it('should inherit from grandparent when parent is inheriting', () => { - const parentInheritedProperties: InheritableProperties = { receiver: 'grandparent' }; - const parent: Route = { receiver: null, group_by: ['foo'] }; - const child: Route = { receiver: null }; - - const childInherited = getInheritedProperties(parent, child, parentInheritedProperties); - expect(childInherited).toHaveProperty('receiver', 'grandparent'); - expect(childInherited.group_by).toEqual(['foo']); - }); - }); - - describe('regular "undefined" or "null" values', () => { - it('should compute inherited properties being undefined', () => { - const parent: Route = { - receiver: 'PARENT', - group_wait: '10s', - }; - - const child: Route = { - receiver: 'CHILD', - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('group_wait', '10s'); - }); - - it('should compute inherited properties being null', () => { - const parent: Route = { - receiver: 'PARENT', - group_wait: '10s', - }; - - const child: Route = { - receiver: null, - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('receiver', 'PARENT'); - }); - - it('should compute inherited properties being undefined from parent inherited properties', () => { - const parent: Route = { - receiver: 'PARENT', - }; - - const child: Route = { - receiver: 'CHILD', - }; - - const childInherited = getInheritedProperties(parent, child, { group_wait: '10s' }); - expect(childInherited).toHaveProperty('group_wait', '10s'); - }); - - it('should not inherit if the child overrides an inheritable value', () => { - const parent: Route = { - receiver: 'PARENT', - group_wait: '10s', - }; - - const child: Route = { - receiver: 'CHILD', - group_wait: '30s', - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).not.toHaveProperty('group_wait'); - }); - - it('should not inherit if the child overrides an inheritable value and the parent inherits', () => { - const parent: Route = { - receiver: 'PARENT', - }; - - const child: Route = { - receiver: 'CHILD', - group_wait: '30s', - }; - - const childInherited = getInheritedProperties(parent, child, { group_wait: '60s' }); - expect(childInherited).not.toHaveProperty('group_wait'); - }); - - it('should inherit if the child property is an empty string', () => { - const parent: Route = { - receiver: 'PARENT', - }; - - const child: Route = { - receiver: '', - group_wait: '30s', - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('receiver', 'PARENT'); - }); - }); - - describe('timing options', () => { - it('should inherit timing options', () => { - const parent: Route = { - receiver: 'PARENT', - group_wait: '1m', - group_interval: '2m', - }; - - const child: Route = { - repeat_interval: '999s', - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).toHaveProperty('group_wait', '1m'); - expect(childInherited).toHaveProperty('group_interval', '2m'); - }); - }); - it('should not inherit mute timings from parent route', () => { - const parent: Route = { - receiver: 'PARENT', - group_by: ['parentLabel'], - mute_time_intervals: ['Mon-Fri 09:00-17:00'], - }; - - const child: Route = { - receiver: 'CHILD', - group_by: ['childLabel'], - }; - - const childInherited = getInheritedProperties(parent, child); - expect(childInherited).not.toHaveProperty('mute_time_intervals'); - }); -}); - -describe('computeInheritedTree', () => { - it('should merge properties from parent', () => { - const parent: Route = { - receiver: 'PARENT', - group_wait: '1m', - group_interval: '2m', - repeat_interval: '3m', - routes: [ - { - repeat_interval: '999s', - }, - ], - }; - - const treeRoot = computeInheritedTree(parent); - expect(treeRoot).toHaveProperty('group_wait', '1m'); - expect(treeRoot).toHaveProperty('group_interval', '2m'); - expect(treeRoot).toHaveProperty('repeat_interval', '3m'); - - expect(treeRoot).toHaveProperty('routes.0.group_wait', '1m'); - expect(treeRoot).toHaveProperty('routes.0.group_interval', '2m'); - expect(treeRoot).toHaveProperty('routes.0.repeat_interval', '999s'); - }); - - it('should not regress #73573', () => { - const parent: Route = { - routes: [ - { - group_wait: '1m', - group_interval: '2m', - repeat_interval: '3m', - routes: [ - { - group_wait: '10m', - group_interval: '20m', - repeat_interval: '30m', - }, - { - repeat_interval: '999m', - }, - ], - }, - ], - }; - - const treeRoot = computeInheritedTree(parent); - expect(treeRoot).toHaveProperty('routes.0.group_wait', '1m'); - expect(treeRoot).toHaveProperty('routes.0.group_interval', '2m'); - expect(treeRoot).toHaveProperty('routes.0.repeat_interval', '3m'); - - expect(treeRoot).toHaveProperty('routes.0.routes.0.group_wait', '10m'); - expect(treeRoot).toHaveProperty('routes.0.routes.0.group_interval', '20m'); - expect(treeRoot).toHaveProperty('routes.0.routes.0.repeat_interval', '30m'); - - expect(treeRoot).toHaveProperty('routes.0.routes.1.group_wait', '1m'); - expect(treeRoot).toHaveProperty('routes.0.routes.1.group_interval', '2m'); - expect(treeRoot).toHaveProperty('routes.0.routes.1.repeat_interval', '999m'); - }); -}); +import { normalizeRoute, unquoteRouteMatchers } from './notification-policies'; describe('normalizeRoute', () => { it('should map matchers property to object_matchers', function () { @@ -432,66 +39,6 @@ describe('normalizeRoute', () => { }); }); -describe('matchLabels', () => { - it('should match with non-matching matchers', () => { - const result = matchLabels( - [ - ['foo', MatcherOperator.equal, ''], - ['team', MatcherOperator.equal, 'operations'], - ], - [['team', 'operations']] - ); - - expect(result).toHaveProperty('matches', true); - expect(result.labelsMatch).toMatchSnapshot(); - }); - - it('should match with non-equal matchers', () => { - const result = matchLabels( - [ - ['foo', MatcherOperator.notEqual, 'bar'], - ['team', MatcherOperator.equal, 'operations'], - ], - [['team', 'operations']] - ); - - expect(result).toHaveProperty('matches', true); - expect(result.labelsMatch).toMatchSnapshot(); - }); - - it('should not match with a set of matchers', () => { - const result = matchLabels( - [ - ['foo', MatcherOperator.notEqual, 'bar'], - ['team', MatcherOperator.equal, 'operations'], - ], - [ - ['team', 'operations'], - ['foo', 'bar'], - ] - ); - - expect(result).toHaveProperty('matches', false); - expect(result.labelsMatch).toMatchSnapshot(); - }); - - it('does not match unanchored regular expressions', () => { - const result = matchLabels([['foo', MatcherOperator.regex, 'bar']], [['foo', 'barbarbar']]); - // This may seem unintuitive, but this is how Alertmanager matches, as it anchors the regex - expect(result.matches).toEqual(false); - }); - - it('matches regular expressions with wildcards', () => { - const result = matchLabels([['foo', MatcherOperator.regex, '.*bar.*']], [['foo', 'barbarbar']]); - expect(result.matches).toEqual(true); - }); - - it('does match regular expressions with flags', () => { - const result = matchLabels([['foo', MatcherOperator.regex, '(?i).*BAr.*']], [['foo', 'barbarbar']]); - expect(result.matches).toEqual(true); - }); -}); - describe('unquoteRouteMatchers', () => { it('should unquote and unescape matchers values', () => { const route: RouteWithID = { diff --git a/public/app/features/alerting/unified/utils/notification-policies.ts b/public/app/features/alerting/unified/utils/notification-policies.ts index dc7f4923ed8..7f2e0437b3b 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -1,100 +1,12 @@ -import { isArray, pick, reduce } from 'lodash'; +import { findMatchingRoutes } from '@grafana/alerting/unstable'; +import { AlertmanagerGroup, Route } from 'app/plugins/datasource/alertmanager/types'; -import { AlertmanagerGroup, ObjectMatcher, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; -import { Labels } from 'app/types/unified-alerting-dto'; - -import { Label, isLabelMatch, matchLabelsSet, normalizeMatchers, unquoteWithUnescape } from './matchers'; - -// If a policy has no matchers it still can be a match, hence matchers can be empty and match can be true -// So we cannot use null as an indicator of no match -interface LabelMatchResult { - match: boolean; - matcher: ObjectMatcher | null; -} - -export const INHERITABLE_KEYS = ['receiver', 'group_by', 'group_wait', 'group_interval', 'repeat_interval'] as const; -export type InheritableKeys = typeof INHERITABLE_KEYS; -export type InheritableProperties = Pick; - -type LabelsMatch = Map; - -interface MatchingResult { - matches: boolean; - labelsMatch: LabelsMatch; -} - -// returns a match results for given set of matchers (from a policy for instance) and a set of labels -export function matchLabels(matchers: ObjectMatcher[], labels: Label[]): MatchingResult { - const matches = matchLabelsSet(matchers, labels); - - // create initial map of label => match result - const labelsMatch: LabelsMatch = new Map(labels.map((label) => [label, { match: false, matcher: null }])); - - // for each matcher, check which label it matched for - matchers.forEach((matcher) => { - const matchingLabel = labels.find((label) => isLabelMatch(matcher, label)); - - // record that matcher for the label - if (matchingLabel) { - labelsMatch.set(matchingLabel, { - match: true, - matcher, - }); - } - }); - - return { matches, labelsMatch }; -} - -export interface AlertInstanceMatch { - instance: Labels; - labelsMatch: LabelsMatch; -} - -export interface RouteMatchResult { - route: T; - labelsMatch: LabelsMatch; -} - -// Match does a depth-first left-to-right search through the route tree -// and returns the matching routing nodes. - -// If the current node is not a match, return nothing -// Normalization should have happened earlier in the code -function findMatchingRoutes(route: T, labels: Label[]): Array> { - let childMatches: Array> = []; - - // If the current node is not a match, return nothing - const matchResult = matchLabels(route.object_matchers ?? [], labels); - if (!matchResult.matches) { - return []; - } - - // If the current node matches, recurse through child nodes - if (route.routes) { - for (const child of route.routes) { - const matchingChildren = findMatchingRoutes(child, labels); - // TODO how do I solve this typescript thingy? It looks correct to me /shrug - // @ts-ignore - childMatches = childMatches.concat(matchingChildren); - // we have matching children and we don't want to continue, so break here - if (matchingChildren.length && !child.continue) { - break; - } - } - } - - // If no child nodes were matches, the current node itself is a match. - if (childMatches.length === 0) { - childMatches.push({ route, labelsMatch: matchResult.labelsMatch }); - } - - return childMatches; -} +import { normalizeMatchers, unquoteWithUnescape } from './matchers'; +import { routeAdapter } from './routeAdapter'; // This is a performance improvement to normalize matchers only once and use the normalized version later on -export function normalizeRoute(rootRoute: RouteWithID): RouteWithID { - function normalizeRoute(route: RouteWithID) { +export function normalizeRoute(rootRoute: T): T { + function normalizeRoute(route: T) { route.object_matchers = normalizeMatchers(route); delete route.matchers; delete route.match; @@ -108,8 +20,8 @@ export function normalizeRoute(rootRoute: RouteWithID): RouteWithID { return normalizedRootRoute; } -export function unquoteRouteMatchers(route: RouteWithID): RouteWithID { - function unquoteRoute(route: RouteWithID) { +export function unquoteRouteMatchers(route: T): T { + function unquoteRoute(route: Route) { route.object_matchers = route.object_matchers?.map(([name, operator, value]) => { return [unquoteWithUnescape(name), operator, unquoteWithUnescape(value)]; }); @@ -137,7 +49,11 @@ function findMatchingAlertGroups( // find matching alerts in the current group const matchingAlerts = group.alerts.filter((alert) => { const labels = Object.entries(alert.labels); - return findMatchingRoutes(routeTree, labels).some((matchingRoute) => matchingRoute.route === route); + const alertingRouteTree = routeAdapter.toPackage(routeTree); + const alertingRoute = routeAdapter.toPackage(route); + return findMatchingRoutes(alertingRouteTree, labels).some( + (matchingRoute) => matchingRoute.route === alertingRoute + ); }); // if the groups has any alerts left after matching, add it to the results @@ -152,66 +68,6 @@ function findMatchingAlertGroups( }, matchingGroups); } -// inherited properties are config properties that exist on the parent route (or its inherited properties) but not on the child route -function getInheritedProperties( - parentRoute: Route, - childRoute: Route, - propertiesParentInherited?: InheritableProperties -): InheritableProperties { - const propsFromParent: InheritableProperties = pick(parentRoute, INHERITABLE_KEYS); - const inheritableProperties: InheritableProperties = { - ...propsFromParent, - ...propertiesParentInherited, - }; - - const inherited = reduce( - inheritableProperties, - (inheritedProperties: InheritableProperties, parentValue, property) => { - const parentHasValue = parentValue != null; - - const inheritableValues = [undefined, '', null]; - // @ts-ignore - const childIsInheriting = inheritableValues.some((value) => childRoute[property] === value); - const inheritFromValue = childIsInheriting && parentHasValue; - - const inheritEmptyGroupByFromParent = - property === 'group_by' && - parentHasValue && - isArray(childRoute[property]) && - childRoute[property]?.length === 0; - - const inheritFromParent = inheritFromValue || inheritEmptyGroupByFromParent; - - if (inheritFromParent) { - // @ts-ignore - inheritedProperties[property] = parentValue; - } - - return inheritedProperties; - }, - {} - ); - - return inherited; -} - -/** - * This function will compute the full tree with inherited properties – this is mostly used for search and filtering - */ -export function computeInheritedTree(parent: T): T { - return { - ...parent, - routes: parent.routes?.map((child) => { - const inheritedProperties = getInheritedProperties(parent, child); - - return computeInheritedTree({ - ...child, - ...inheritedProperties, - }); - }), - }; -} - // recursive function to rename receivers in all routes (notification policies) function renameReceiverInRoute(route: Route, oldName: string, newName: string) { const updated: Route = { @@ -229,4 +85,4 @@ function renameReceiverInRoute(route: Route, oldName: string, newName: string) { return updated; } -export { findMatchingAlertGroups, findMatchingRoutes, getInheritedProperties, renameReceiverInRoute }; +export { findMatchingAlertGroups, renameReceiverInRoute }; diff --git a/public/app/features/alerting/unified/utils/routeAdapter.test.ts b/public/app/features/alerting/unified/utils/routeAdapter.test.ts new file mode 100644 index 00000000000..7f0239ca621 --- /dev/null +++ b/public/app/features/alerting/unified/utils/routeAdapter.test.ts @@ -0,0 +1,267 @@ +import { Factory } from 'fishery'; + +import { RouteFactory } from '@grafana/alerting/testing'; +import { RouteWithID as AlertingRouteWithID } from '@grafana/alerting/unstable'; +import { MatcherOperator, ObjectMatcher, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; + +import { routeAdapter } from './routeAdapter'; + +describe('routeAdapter', () => { + // Create RouteWithID factory that extends the base RouteFactory + const RouteWithIDFactory = Factory.define(({ sequence }) => ({ + ...RouteFactory.build(), + id: `route-${sequence}`, + routes: [], + })); + + const mockObjectMatchers: ObjectMatcher[] = [['severity', MatcherOperator.equal, 'critical']]; + const mockLabelMatchers = [ + { + label: 'severity', + type: '=' as const, + value: 'critical', + }, + ]; + + describe('toPackage', () => { + it('should convert basic Route to AlertingRoute', () => { + const route: Route = { + receiver: 'test-receiver', + continue: true, + group_by: ['alertname'], + object_matchers: mockObjectMatchers, + routes: [], + }; + + const result = routeAdapter.toPackage(route); + + expect(result).toEqual({ + receiver: 'test-receiver', + continue: true, + group_by: ['alertname'], + matchers: mockLabelMatchers, + routes: [], + group_wait: undefined, + group_interval: undefined, + repeat_interval: undefined, + mute_time_intervals: undefined, + active_time_intervals: undefined, + }); + }); + + it('should convert RouteWithID to AlertingRouteWithID', () => { + const routeWithId: RouteWithID = { + id: 'test-id', + receiver: 'test-receiver', + continue: false, + object_matchers: mockObjectMatchers, + routes: [], + }; + + const result = routeAdapter.toPackage(routeWithId); + + expect(result).toEqual({ + id: 'test-id', + receiver: 'test-receiver', + continue: false, + matchers: mockLabelMatchers, + routes: [], + group_by: undefined, + group_wait: undefined, + group_interval: undefined, + repeat_interval: undefined, + mute_time_intervals: undefined, + active_time_intervals: undefined, + }); + }); + + it('should handle undefined continue as false', () => { + const route: Route = { + receiver: 'test-receiver', + object_matchers: mockObjectMatchers, + routes: [], + }; + + const result = routeAdapter.toPackage(route); + + expect(result.continue).toBe(false); + }); + + it('should handle null receiver as undefined', () => { + const route: Route = { + receiver: null, + object_matchers: mockObjectMatchers, + routes: [], + }; + + const result = routeAdapter.toPackage(route); + + expect(result.receiver).toBeUndefined(); + }); + + it('should recursively convert child routes', () => { + const route: RouteWithID = { + id: 'parent', + receiver: 'parent-receiver', + routes: [ + { + id: 'child', + receiver: 'child-receiver', + object_matchers: mockObjectMatchers, + routes: [], + }, + ], + }; + + const result = routeAdapter.toPackage(route); + + expect(result.routes).toHaveLength(1); + expect(result.routes[0]).toEqual({ + id: 'child', + receiver: 'child-receiver', + continue: false, + matchers: mockLabelMatchers, + routes: [], + group_by: undefined, + group_wait: undefined, + group_interval: undefined, + repeat_interval: undefined, + mute_time_intervals: undefined, + active_time_intervals: undefined, + }); + }); + }); + + describe('fromPackage', () => { + it('should convert basic AlertingRoute to Route', () => { + const alertingRoute = RouteFactory.build({ + receiver: 'test-receiver', + continue: true, + group_by: ['alertname'], + matchers: mockLabelMatchers, + routes: [], + }); + + const result = routeAdapter.fromPackage(alertingRoute); + + expect(result.receiver).toBe('test-receiver'); + expect(result.continue).toBe(true); + expect(result.group_by).toEqual(['alertname']); + expect(result.object_matchers).toEqual(mockObjectMatchers); + expect(result.routes).toBeUndefined(); + }); + + it('should convert AlertingRouteWithID to RouteWithID', () => { + const alertingRouteWithId = RouteWithIDFactory.build({ + id: 'test-id', + receiver: 'test-receiver', + continue: false, + matchers: mockLabelMatchers, + routes: [], + }); + + const result = routeAdapter.fromPackage(alertingRouteWithId); + + expect(result.id).toBe('test-id'); + expect(result.receiver).toBe('test-receiver'); + expect(result.continue).toBe(false); + expect(result.object_matchers).toEqual(mockObjectMatchers); + expect(result.routes).toBeUndefined(); + }); + + it('should handle undefined receiver as null', () => { + const alertingRoute = RouteFactory.build(); + // Override receiver to undefined after building + const alertingRouteWithUndefinedReceiver = { + ...alertingRoute, + receiver: undefined, + }; + + const result = routeAdapter.fromPackage(alertingRouteWithUndefinedReceiver); + + expect(result.receiver).toBeNull(); + }); + + it('should recursively convert child routes', () => { + const childRoute = RouteWithIDFactory.build({ + id: 'child', + receiver: 'child-receiver', + continue: true, + matchers: mockLabelMatchers, + routes: [], + }); + + const alertingRoute = RouteWithIDFactory.build({ + id: 'parent', + receiver: 'parent-receiver', + continue: false, + routes: [childRoute], + }); + + const result = routeAdapter.fromPackage(alertingRoute); + + expect(result.routes).toHaveLength(1); + expect(result.routes![0].id).toBe('child'); + expect(result.routes![0].receiver).toBe('child-receiver'); + expect(result.routes![0].continue).toBe(true); + expect(result.routes![0].object_matchers).toEqual(mockObjectMatchers); + expect(result.routes![0].routes).toBeUndefined(); + }); + + it('should handle routes without matchers', () => { + const alertingRoute = RouteFactory.build(); + // Override matchers to undefined after building + const alertingRouteWithoutMatchers = { + ...alertingRoute, + matchers: undefined, + }; + + const result = routeAdapter.fromPackage(alertingRouteWithoutMatchers); + + expect(result.object_matchers).toBeUndefined(); + }); + }); + + describe('round-trip conversion', () => { + it('should maintain data integrity through round-trip conversion', () => { + const childRoute: RouteWithID = { + id: 'child-route', + receiver: 'child-receiver', + continue: false, + object_matchers: [['environment', MatcherOperator.equal, 'prod']], + routes: [], + }; + + const originalRoute: RouteWithID = { + id: 'test-route', + receiver: 'test-receiver', + continue: true, + group_by: ['alertname', 'severity'], + group_wait: '10s', + group_interval: '5m', + repeat_interval: '1h', + object_matchers: [ + ['severity', MatcherOperator.equal, 'critical'], + ['team', MatcherOperator.regex, 'frontend|backend'], + ], + mute_time_intervals: ['maintenance'], + active_time_intervals: ['business-hours'], + routes: [childRoute], + }; + + // Convert to package format and back + const packageRoute = routeAdapter.toPackage(originalRoute); + const backToOriginal = routeAdapter.fromPackage(packageRoute); + + expect(backToOriginal).toEqual({ + ...originalRoute, + routes: [ + { + ...childRoute, + routes: undefined, // fromPackage doesn't add routes array if empty + }, + ], + }); + }); + }); +}); diff --git a/public/app/features/alerting/unified/utils/routeAdapter.ts b/public/app/features/alerting/unified/utils/routeAdapter.ts new file mode 100644 index 00000000000..e08c2fb7486 --- /dev/null +++ b/public/app/features/alerting/unified/utils/routeAdapter.ts @@ -0,0 +1,159 @@ +import { + Route as AlertingRoute, + RouteWithID as AlertingRouteWithID, + type LabelMatcher, +} from '@grafana/alerting/unstable'; +import { + MatcherOperator, + type ObjectMatcher, + type Route, + type RouteWithID, +} from 'app/plugins/datasource/alertmanager/types'; + +import { convertObjectMatcherToAlertingPackageMatcher, matcherToObjectMatcher, parseMatcherToArray } from './matchers'; + +/** + * Enhanced type guards using infer-based utility types + */ +function hasId(route: T): route is T & RouteWithID { + return 'id' in route && typeof route.id === 'string'; +} + +function hasAlertingId(route: T): route is T & AlertingRouteWithID { + return 'id' in route && typeof route.id === 'string'; +} + +/** + * Converts from package route format to alertmanager route format + */ +function fromPackageRoute(route: AlertingRouteWithID): RouteWithID; +function fromPackageRoute(route: AlertingRoute): Route; +function fromPackageRoute(route: AlertingRoute | AlertingRouteWithID): Route | RouteWithID { + // Convert matchers from LabelMatcher[] to ObjectMatcher[] + const object_matchers = route.matchers?.map(labelMatcherToObjectMatcher); + + // Recursively convert child routes + const routes = route.routes?.length ? route.routes.map(fromPackageRoute) : undefined; + + const baseRoute = { + receiver: route.receiver || null, + group_by: route.group_by, + continue: route.continue, + group_wait: route.group_wait, + group_interval: route.group_interval, + repeat_interval: route.repeat_interval, + mute_time_intervals: route.mute_time_intervals, + active_time_intervals: route.active_time_intervals, + }; + + const convertedRoute = { + ...baseRoute, + object_matchers, + routes, + }; + + // If the input route has an ID, include it in the output + if (hasAlertingId(route)) { + return { + ...convertedRoute, + id: route.id, + }; + } + + return convertedRoute; +} + +/** + * Converts from alertmanager route format to package route format + */ +function toPackageRoute(route: RouteWithID): AlertingRouteWithID; +function toPackageRoute(route: Route): AlertingRoute; +function toPackageRoute(route: Route | RouteWithID): AlertingRoute | AlertingRouteWithID { + // Convert matchers + let matchers: LabelMatcher[] = []; + + if (route.object_matchers) { + matchers = route.object_matchers.map(convertObjectMatcherToAlertingPackageMatcher); + } else if (route.matchers) { + matchers = []; + route.matchers.forEach((matcher) => { + const parsedMatchers = parseMatcherToArray(matcher) + .map(matcherToObjectMatcher) + .map(convertObjectMatcherToAlertingPackageMatcher); + matchers.push(...parsedMatchers); + }); + } + + // Recursively convert child routes + const routes = route.routes?.length ? route.routes.map(toPackageRoute) : []; + + const baseRoute = { + receiver: route.receiver ?? undefined, + group_by: route.group_by, + continue: route.continue ?? false, + group_wait: route.group_wait, + group_interval: route.group_interval, + repeat_interval: route.repeat_interval, + mute_time_intervals: route.mute_time_intervals, + active_time_intervals: route.active_time_intervals, + }; + + const convertedRoute = { + ...baseRoute, + matchers, + routes, + }; + + // If the input route has an ID, include it in the output + if (hasId(route)) { + return { + ...convertedRoute, + id: route.id, + }; + } + + return convertedRoute; +} + +/** + * Converts routes between alertmanager and package formats + */ +export const routeAdapter = { + /** + * Converts from package route format to alertmanager route format + * Handles both Route and RouteWithID variants + */ + fromPackage: fromPackageRoute, + + /** + * Converts from alertmanager route format to package route format + * Handles both Route and RouteWithID variants + */ + toPackage: toPackageRoute, +}; + +/** + * Safely converts a LabelMatcher type to MatcherOperator + */ +function convertToMatcherOperator(type: LabelMatcher['type']): MatcherOperator { + switch (type) { + case '=': + return MatcherOperator.equal; + case '!=': + return MatcherOperator.notEqual; + case '=~': + return MatcherOperator.regex; + case '!~': + return MatcherOperator.notRegex; + default: + const exhaustiveCheck: never = type; + throw new Error(`Unknown matcher type: ${exhaustiveCheck}`); + } +} + +/** + * Converts a LabelMatcher from the alerting package format to an ObjectMatcher for alertmanager format + */ +export function labelMatcherToObjectMatcher(matcher: LabelMatcher): ObjectMatcher { + return [matcher.label, convertToMatcherOperator(matcher.type), matcher.value]; +} diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 298ac1a1e84..752658e887d 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1671,6 +1671,10 @@ "inspector-yaml-tab": { "apply": "Apply" }, + "instance-match": { + "non-matching-labels": "Non-matching labels", + "notification-policy": "View route" + }, "irm-integration": { "connection-method": "How to connect to IRM", "disabled-description": "Enable IRM through a Webhook integration", @@ -1813,6 +1817,10 @@ } } }, + "match-details": { + "matched": "matched", + "no-matchers-matched": "Policy matches all labels" + }, "matcher-filter": { "filter-alerts-using-label-querying-without-braces": "Filter alerts using label querying without braces, ex:", "filter-alerts-using-label-querying-without-spaces": "Filter alerts using label querying without spaces, ex:", @@ -1975,8 +1983,10 @@ "label-notification-policies": "Notification Policies", "label-time-intervals": "Time intervals" }, + "notification-policy-drawer": { + "view-notification-policy-tree": "View notification policy tree" + }, "notification-policy-matchers": { - "default-policy": "Default policy", "no-matchers": "No matchers" }, "notification-preview": { @@ -1993,22 +2003,12 @@ }, "notification-route": { "no-labels": "No labels", - "no-matching-labels": "No matching labels" - }, - "notification-route-details-modal": { - "alert-instances-routed-follows": "Your alert instances are routed as follows.", - "close": "Close", - "contact-point": "Contact point", - "default-policy": "Default policy", - "notification-policy-path": "Notification policy path", - "see-details-link": "See details", - "title-routing-details": "Routing details" + "notification-policy": "Notification policy" }, "notification-route-header": { "aria-label-expand-policy-route": "Expand policy route", - "delivered-to": "@ Delivered to", - "notification-policy": "Notification policy", - "see-details": "See details" + "delivered-to": "Delivered to", + "instances": "instances" }, "notification-templates": { "duplicate": { @@ -2137,10 +2137,6 @@ "label-new-sibling-above": "New sibling above", "label-new-sibling-below": "New sibling below" }, - "policy-path": { - "default-policy": "Default policy", - "no-matchers": "No matchers" - }, "preview-rule": { "body-preview-is-not-available": "Cannot display the query preview. Some of the data sources used in the queries are not available.", "preview-alerts": "Preview alerts", From 237b718b95f3acdaa0d641a4f16b59f7d1f5b5d3 Mon Sep 17 00:00:00 2001 From: beejeebus Date: Wed, 24 Sep 2025 15:01:44 -0400 Subject: [PATCH 04/33] Bump the version of grafana-google-sdk (#111562) There was a [fix](https://github.com/grafana/grafana-google-sdk-go/pull/17) for lock in grafana-google-sdk-go v0.4.2. It has not been incorporated in grafana core yet, and is causing incidents: #incident-2025-09-24-prod_us_central_0_google_monitoring_errors --- go.mod | 2 +- go.sum | 4 ++-- go.work.sum | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index d0269ade929..5a66ee070ad 100644 --- a/go.mod +++ b/go.mod @@ -101,7 +101,7 @@ require ( github.com/grafana/grafana-aws-sdk v1.2.0 // @grafana/aws-datasources github.com/grafana/grafana-azure-sdk-go/v2 v2.2.0 // @grafana/partner-datasources github.com/grafana/grafana-cloud-migration-snapshot v1.9.0 // @grafana/grafana-operator-experience-squad - github.com/grafana/grafana-google-sdk-go v0.4.1 // @grafana/partner-datasources + github.com/grafana/grafana-google-sdk-go v0.4.2 // @grafana/partner-datasources github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 // @grafana/grafana-backend-group github.com/grafana/grafana-plugin-sdk-go v0.279.0 // @grafana/plugins-platform-backend github.com/grafana/loki/v3 v3.2.1 // @grafana/observability-logs diff --git a/go.sum b/go.sum index 5e77d1adea6..53c44fc95a0 100644 --- a/go.sum +++ b/go.sum @@ -1615,8 +1615,8 @@ github.com/grafana/grafana-azure-sdk-go/v2 v2.2.0 h1:0TYrkzAc3u0HX+9GK86cGrLTUAc github.com/grafana/grafana-azure-sdk-go/v2 v2.2.0/go.mod h1:H9sVh9A4yg5egMGZeh0mifxT1Q/uqwKe1LBjBJU6pN8= github.com/grafana/grafana-cloud-migration-snapshot v1.9.0 h1:JOzchPgptwJdruYoed7x28lFDwhzs7kssResYsnC0iI= github.com/grafana/grafana-cloud-migration-snapshot v1.9.0/go.mod h1:nOHgq4Oa829qmBKA5KIXw5Ipo3rhLs0d6A8UI9Nw8Zk= -github.com/grafana/grafana-google-sdk-go v0.4.1 h1:QdHmgDzlV3RzBTvIxd+WuxER1+afnFzUmEivbwDo27E= -github.com/grafana/grafana-google-sdk-go v0.4.1/go.mod h1:U73+w9DlbEtUonhQUzERwlXnzWTtfRoyrtKH8d3VY40= +github.com/grafana/grafana-google-sdk-go v0.4.2 h1:F44hQF1y6UVJhlJPi+Mz+GCJsioVgezEgPMMEQbUZRo= +github.com/grafana/grafana-google-sdk-go v0.4.2/go.mod h1:U73+w9DlbEtUonhQUzERwlXnzWTtfRoyrtKH8d3VY40= github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 h1:r+mU5bGMzcXCRVAuOrTn54S80qbfVkvTdUJZfSfTNbs= github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79/go.mod h1:wc6Hbh3K2TgCUSfBC/BOzabItujtHMESZeFk5ZhdxhQ= github.com/grafana/grafana-plugin-sdk-go v0.279.0 h1:/KCrsZkj9pEGwIGovqAz1A8rjI2A2YT+ZpvgfZN0LAA= diff --git a/go.work.sum b/go.work.sum index 918fc645382..d9a860898d1 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1074,6 +1074,8 @@ github.com/grafana/grafana-aws-sdk v0.38.2/go.mod h1:j3vi+cXYHEFqjhBGrI6/lw1TNM+ github.com/grafana/grafana-aws-sdk v1.0.2 h1:98eBuHYFmgvH0xO9kKf4RBsEsgQRp8EOA/9yhDIpkss= github.com/grafana/grafana-aws-sdk v1.0.2/go.mod h1:hO7q7yWV+t6dmiyJjMa3IbuYnYkBua+G/IAlOPVIYKE= github.com/grafana/grafana-azure-sdk-go/v2 v2.1.6/go.mod h1:V7y2BmsWxS3A9Ohebwn4OiSfJJqi//4JQydQ8fHTduo= +github.com/grafana/grafana-google-sdk-go v0.4.2 h1:F44hQF1y6UVJhlJPi+Mz+GCJsioVgezEgPMMEQbUZRo= +github.com/grafana/grafana-google-sdk-go v0.4.2/go.mod h1:U73+w9DlbEtUonhQUzERwlXnzWTtfRoyrtKH8d3VY40= github.com/grafana/grafana-plugin-sdk-go v0.263.0/go.mod h1:U43Cnrj/9DNYyvFcNdeUWNjMXTKNB0jcTcQGpWKd2gw= github.com/grafana/grafana-plugin-sdk-go v0.267.0/go.mod h1:OuwS4c/JYgn0rr/w5zhJBpLo4gKm/vw15RsfpYAvK9Q= github.com/grafana/grafana-plugin-sdk-go v0.269.1/go.mod h1:yv2KbO4mlr9WuDK2f+2gHAMTwwLmLuqaEnrPXTRU+OI= From 49c4dcfd132c96dcdf0bff221c4e689789716912 Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Wed, 24 Sep 2025 16:50:24 -0400 Subject: [PATCH 05/33] FileStatusPage: Disable save button if repo is read only (#111561) --- .../provisioning/File/FileStatusPage.tsx | 19 +++++++++++++++---- public/locales/en-US/grafana.json | 1 + 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/public/app/features/provisioning/File/FileStatusPage.tsx b/public/app/features/provisioning/File/FileStatusPage.tsx index 7b122e3b666..f910d8160ba 100644 --- a/public/app/features/provisioning/File/FileStatusPage.tsx +++ b/public/app/features/provisioning/File/FileStatusPage.tsx @@ -17,6 +17,7 @@ import { Page } from 'app/core/components/Page/Page'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { PROVISIONING_URL } from '../constants'; +import { useGetResourceRepositoryView } from '../hooks/useGetResourceRepositoryView'; export default function FileStatusPage() { const params = useParams(); @@ -26,6 +27,8 @@ export default function FileStatusPage() { const name = params['name'] ?? ''; const path = params['*'] ?? ''; const file = useGetRepositoryFilesWithPathQuery({ name, path, ref }); + const { isReadOnlyRepo } = useGetResourceRepositoryView({ name }); + console.log('isReadOnlyRepo', isReadOnlyRepo); return ( )} - {file.isSuccess && file.data && } + {file.isSuccess && file.data && ( + + )} @@ -59,9 +64,10 @@ interface Props { repo: string; repoRef?: string; tab: TabSelection; + isReadOnlyRepo: boolean; } -function ResourceView({ wrap, repo, repoRef, tab }: Props) { +function ResourceView({ wrap, repo, repoRef, tab, isReadOnlyRepo }: Props) { const isDashboard = wrap.resource?.type?.kind === 'Dashboard'; const existingName = wrap.resource?.existing?.metadata?.name; const location = useLocation(); @@ -159,8 +165,13 @@ function ResourceView({ wrap, repo, repoRef, tab }: Props) {
{ deleteFile({ name: repo, diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 752658e887d..c82c89e0a68 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11637,6 +11637,7 @@ "removed-from-repo-test-ui": "removed from repo test UI", "updated-from-repo-test-ui": "updated from repo test UI" }, + "read-only-repo-tooltip": "This is a read-only repository", "repository": "Repository", "tab-info": { "label": { From 020b87e91b55daeb8612462527b87a3d82fb5f29 Mon Sep 17 00:00:00 2001 From: Kevin Minehart <5140827+kminehart@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:53:37 -0500 Subject: [PATCH 06/33] CI: Add cleanup branches cronjob (with dry-run: true) (#111428) * CI: Add cleanup branches cronjob (with dry-run: true) * update codeowners --- .github/CODEOWNERS | 1 + .github/workflows/cleanup-branches.yml | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 .github/workflows/cleanup-branches.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c76673efd98..85db1f8d4c7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1262,6 +1262,7 @@ embed.go @grafana/grafana-as-code /.github/workflows/changelog.yml @zserge /.github/workflows/shellcheck.yml @grafana/grafana-developer-enablement-squad /.github/workflows/release-build.yml @grafana/grafana-developer-enablement-squad +/.github/workflows/cleanup-branches.yml @grafana/grafana-developer-enablement-squad /.github/workflows/publish-artifact.yml @grafana/grafana-developer-enablement-squad /.github/actions/changelog @zserge /.github/workflows/swagger-gen.yml @grafana/grafana-backend-group diff --git a/.github/workflows/cleanup-branches.yml b/.github/workflows/cleanup-branches.yml new file mode 100644 index 00000000000..00cb4f26541 --- /dev/null +++ b/.github/workflows/cleanup-branches.yml @@ -0,0 +1,18 @@ +name: Clean up orphaned branches +on: + workflow_dispatch: + schedule: + - cron: "0 9 * * 1" + +jobs: + cleanup-branches: + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: read + steps: + - uses: actions/checkout@v5 + - uses: grafana/shared-workflows/actions/cleanup-branches@cleanup-branches/v1.0.0 + with: + dry-run: true + max-date: "1 month ago" From 0c0554da5e68d0e204791f1ae77af6182029c0db Mon Sep 17 00:00:00 2001 From: Costa Alexoglou Date: Wed, 24 Sep 2025 23:35:06 +0200 Subject: [PATCH 07/33] fix: avoid child paths in repositories (#111573) * fix: avoid child paths in repositories * add another unit test; fix linter * Update pkg/registry/apis/provisioning/register.go * skip itself * fix: failing tests --------- Co-authored-by: Stephanie Hingtgen --- .../pkg/apis/provisioning/v0alpha1/types.go | 60 ++++++++++++++++ pkg/registry/apis/provisioning/register.go | 68 +++++++++++++++--- .../apis/provisioning/repository_test.go | 70 ++++++++++++++++++- 3 files changed, 187 insertions(+), 11 deletions(-) diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go index 9ddfcd3897f..fc24ac84869 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go @@ -159,6 +159,66 @@ func (r *Repository) Branch() string { return "" } +// URL returns the URL for git-based repositories +// or an empty string for local repositories +func (r *Repository) URL() string { + if !r.Spec.Type.IsGit() { + return "" + } + + switch r.Spec.Type { + case GitHubRepositoryType: + if r.Spec.GitHub != nil { + return r.Spec.GitHub.URL + } + case GitRepositoryType: + if r.Spec.Git != nil { + return r.Spec.Git.URL + } + case BitbucketRepositoryType: + if r.Spec.Bitbucket != nil { + return r.Spec.Bitbucket.URL + } + case GitLabRepositoryType: + if r.Spec.GitLab != nil { + return r.Spec.GitLab.URL + } + default: + return "" + } + + return "" +} + +func (r *Repository) Path() string { + switch r.Spec.Type { + case GitHubRepositoryType: + if r.Spec.GitHub != nil { + return r.Spec.GitHub.Path + } + case GitRepositoryType: + if r.Spec.Git != nil { + return r.Spec.Git.Path + } + case BitbucketRepositoryType: + if r.Spec.Bitbucket != nil { + return r.Spec.Bitbucket.Path + } + case GitLabRepositoryType: + if r.Spec.GitLab != nil { + return r.Spec.GitLab.Path + } + case LocalRepositoryType: + if r.Spec.Local != nil { + return r.Spec.Local.Path + } + default: + return "" + } + + return "" +} + type RepositorySpec struct { // The repository display name (shown in the UI) Title string `json:"title"` diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 920c8680b21..ffe315f63b6 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -2,9 +2,11 @@ package provisioning import ( "context" + "errors" "fmt" "net/http" "net/url" + "path/filepath" "slices" "strings" "time" @@ -75,6 +77,11 @@ var ( _ builder.OpenAPIPostProcessor = (*APIBuilder)(nil) ) +var ( + ErrRepositoryParentFolderConflict = errors.New("parent folder conflict") + ErrRepositoryDuplicatePath = errors.New("duplicate repository path") +) + // JobHistoryConfig holds configuration for job history backends type JobHistoryConfig struct { Loki *loki.Config `json:"loki,omitempty"` @@ -626,18 +633,32 @@ func invalidRepositoryError(name string, list field.ErrorList) error { } func (b *APIBuilder) getRepositoriesInNamespace(ctx context.Context) ([]provisioning.Repository, error) { - obj, err := b.store.List(ctx, &internalversion.ListOptions{ - Limit: 100, - }) - if err != nil { - return nil, err + var allRepositories []provisioning.Repository + continueToken := "" + + for { + obj, err := b.store.List(ctx, &internalversion.ListOptions{ + Limit: 100, + Continue: continueToken, + }) + if err != nil { + return nil, err + } + + repositoryList, ok := obj.(*provisioning.RepositoryList) + if !ok { + return nil, fmt.Errorf("expected repository list") + } + + allRepositories = append(allRepositories, repositoryList.Items...) + + continueToken = repositoryList.GetContinue() + if continueToken == "" { + break + } } - all, ok := obj.(*provisioning.RepositoryList) - if !ok { - return nil, fmt.Errorf("expected repository list") - } - return all.Items, nil + return allRepositories, nil } // TODO: move this to a more appropriate place. Probably controller/validation.go @@ -670,6 +691,33 @@ func (b *APIBuilder) verifyAgainstExistingRepositories(cfg *provisioning.Reposit } } + // If repo is git, ensure no other repository is defined with a child path + if cfg.Spec.Type.IsGit() { + for _, v := range all { + // skip itself + if cfg.Name == v.Name { + continue + } + if v.URL() == cfg.URL() { + if v.Path() == cfg.Path() { + return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"), + fmt.Sprintf("%s: %s", ErrRepositoryDuplicatePath.Error(), v.Name)) + } + + relPath, err := filepath.Rel(v.Path(), cfg.Path()) + if err != nil { + return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"), "failed to evaluate path: "+err.Error()) + } + // https://pkg.go.dev/path/filepath#Rel + // Rel will return "../" if the relative paths are not related + if !strings.HasPrefix(relPath, "../") { + return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"), + fmt.Sprintf("%s: %s", ErrRepositoryParentFolderConflict.Error(), v.Name)) + } + } + } + } + // Count repositories excluding the current one being created/updated count := 0 for _, v := range all { diff --git a/pkg/tests/apis/provisioning/repository_test.go b/pkg/tests/apis/provisioning/repository_test.go index 8d3e7daa089..4c49ba76814 100644 --- a/pkg/tests/apis/provisioning/repository_test.go +++ b/pkg/tests/apis/provisioning/repository_test.go @@ -3,12 +3,14 @@ package provisioning import ( "context" "encoding/json" + "errors" "fmt" "net/http" "strings" "testing" "time" + provisioningAPIServer "github.com/grafana/grafana/pkg/registry/apis/provisioning" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -213,6 +215,71 @@ func TestIntegrationProvisioning_RepositoryValidation(t *testing.T) { } }) } + + // Test Git repository path validation - ensure child paths are rejected + t.Run("Git repository path validation", func(t *testing.T) { + baseURL := "https://github.com/grafana/test-repo-path-validation" + + pathTests := []struct { + name string + path string + expectError error + }{ + { + name: "first repo with path 'demo/nested' should succeed", + path: "demo/nested", + expectError: nil, + }, + { + name: "second repo with child path 'demo/nested/again' should fail", + path: "demo/nested/again", + expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict, + }, + { + name: "third repo with parent path 'demo' should fail", + path: "demo", + expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict, + }, + { + name: "fourth repo with nested child path 'demo/nested/nested-second' should fail", + path: "demo/nested/again/two", + expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict, + }, + { + name: "fifth repo with duplicate path 'demo/nested' should fail", + path: "demo/nested", + expectError: provisioningAPIServer.ErrRepositoryDuplicatePath, + }, + } + + for i, test := range pathTests { + t.Run(test.name, func(t *testing.T) { + repoName := fmt.Sprintf("git-path-test-%d", i+1) + gitRepo := helper.RenderObject(t, "testdata/github-readonly.json.tmpl", map[string]any{ + "Name": repoName, + "URL": baseURL, + "Path": test.path, + "SyncEnabled": false, // Disable sync to avoid external dependencies + "SyncTarget": "folder", + }) + + _, err := helper.Repositories.Resource.Create(ctx, gitRepo, metav1.CreateOptions{FieldValidation: "Strict"}) + + if test.expectError != nil { + require.Error(t, err, "Expected error for repository with path: %s", test.path) + + require.ErrorContains(t, err, test.expectError.Error(), "Error should contain expected message for path: %s", test.path) + var statusError *apierrors.StatusError + if errors.As(err, &statusError) { + require.Equal(t, metav1.StatusReasonInvalid, statusError.ErrStatus.Reason, "Should be a validation error") + require.Equal(t, http.StatusUnprocessableEntity, int(statusError.ErrStatus.Code), "Should return 422 status code") + } + } else { + require.NoError(t, err, "Expected success for repository with path: %s", test.path) + } + }) + } + }) } func TestIntegrationProvisioning_FailInvalidSchema(t *testing.T) { @@ -364,7 +431,8 @@ func TestIntegrationProvisioning_CreatingGitHubRepository(t *testing.T) { "Name": test.name, "URL": test.input, "SyncTarget": "folder", - "SyncEnabled": false, // Disable sync since we're just testing URL cleanup + "SyncEnabled": false, // Disable sync since we're just testing URL cleanup, + "Path": fmt.Sprintf("grafana-%s/", test.name), }) _, err := helper.Repositories.Resource.Create(ctx, input, metav1.CreateOptions{}) From a51d1b76b435a7a33011fd3a7cc065af2d30ad91 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 25 Sep 2025 00:38:27 +0300 Subject: [PATCH 08/33] Dashboards: dependency cleanup (#111572) --- pkg/registry/apis/dashboard/authorizer.go | 3 +- pkg/registry/apis/dashboard/register.go | 41 +++++-------------- .../apis/dashboard/schema_validation.go | 3 +- pkg/registry/apis/dashboard/sub_dto.go | 7 +--- pkg/server/wire_gen.go | 4 +- .../integration/api_validation_test.go | 12 ++---- pkg/tests/testinfra/testinfra.go | 5 +++ 7 files changed, 25 insertions(+), 50 deletions(-) diff --git a/pkg/registry/apis/dashboard/authorizer.go b/pkg/registry/apis/dashboard/authorizer.go index 05dfc6a7e61..07ad6bfda19 100644 --- a/pkg/registry/apis/dashboard/authorizer.go +++ b/pkg/registry/apis/dashboard/authorizer.go @@ -8,13 +8,12 @@ import ( "github.com/grafana/authlib/types" dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/libraryelements" ) -func newLegacyAuthorizer(ac accesscontrol.AccessControl, l log.Logger) authorizer.Authorizer { +func newLegacyAuthorizer(ac accesscontrol.AccessControl) authorizer.Authorizer { return authorizer.AuthorizerFunc( func(ctx context.Context, attr authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) { // Note that we will return Allow more than expected. diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index 0bd80b9c4b7..1f295b29d21 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -20,6 +20,7 @@ import ( "k8s.io/kube-openapi/pkg/validation/spec" authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana-app-sdk/logging" internal "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard" dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" dashv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" @@ -33,7 +34,6 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacysearcher" @@ -48,7 +48,6 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/librarypanels" - "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/search/sort" @@ -103,12 +102,10 @@ type DashboardsAPIBuilder struct { dashStore dashboards.Store QuotaService quota.Service ProvisioningService provisioning.ProvisioningService - cfg *setting.Cfg + minRefreshInterval string dualWriter dualwrite.Service folderClientProvider client.K8sHandlerProvider - log log.Logger - reg prometheus.Registerer isStandalone bool // skips any handling including anything to do with legacy storage } @@ -118,7 +115,6 @@ func RegisterAPIService( apiregistration builder.APIRegistrar, dashboardService dashboards.DashboardService, provisioningDashboardService dashboards.DashboardProvisioningService, - pluginStore pluginstore.Store, datasourceService datasources.DataSourceService, dashboardPermissions dashboards.PermissionsRegistrationService, dashboardPermissionsSvc accesscontrol.DashboardPermissionsService, @@ -142,10 +138,8 @@ func RegisterAPIService( legacyDashboardSearcher := legacysearcher.NewDashboardSearchClient(dashStore, sorter) folderClient := client.NewK8sHandler(dual, request.GetNamespaceMapper(cfg), folders.FolderResourceInfo.GroupVersionResource(), restConfigProvider.GetRestConfig, dashStore, userService, unified, sorter, features) - dashLog := log.New("grafana-apiserver.dashboards") builder := &DashboardsAPIBuilder{ - log: dashLog, - authorizer: newLegacyAuthorizer(accessControl, dashLog), + authorizer: newLegacyAuthorizer(accessControl), dashboardService: dashboardService, dashboardPermissions: dashboardPermissions, dashboardPermissionsSvc: dashboardPermissionsSvc, @@ -158,7 +152,7 @@ func RegisterAPIService( dashStore: dashStore, QuotaService: quotaService, ProvisioningService: provisioning, - cfg: cfg, + minRefreshInterval: cfg.MinRefreshInterval, dualWriter: dual, folderClientProvider: newSimpleFolderClientProvider(folderClient), @@ -166,7 +160,6 @@ func RegisterAPIService( Access: legacy.NewDashboardAccess(dbp, namespacer, dashStore, provisioning, libraryPanelSvc, sorter, dashboardPermissionsSvc, accessControl, features), DashboardService: dashboardService, }, - reg: reg, } migration.RegisterMetrics(reg) @@ -177,24 +170,10 @@ func RegisterAPIService( return builder } -func NewAPIService(ac authlib.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider, pluginStore *pluginstore.Service) *DashboardsAPIBuilder { - // TODO: Plugin store will soon be removed, - // as the cases for plugin fetching is not needed. Keeping it now to not break implementation - if pluginStore == nil { - panic("pluginStore is nil") - } - - logger := log.New("grafana-apiserver.dashboards") - +func NewAPIService(ac authlib.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider) *DashboardsAPIBuilder { migration.Initialize(datasourceProvider) - return &DashboardsAPIBuilder{ - log: logger, - reg: prometheus.NewRegistry(), - - cfg: &setting.Cfg{ - MinRefreshInterval: "10s", - }, + minRefreshInterval: "10s", accessClient: ac, authorizer: authsvc.NewResourceAuthorizer(ac), features: features, @@ -336,7 +315,7 @@ func (b *DashboardsAPIBuilder) validateCreate(ctx context.Context, a admission.A } // Validate refresh interval - if err := b.dashboardService.ValidateDashboardRefreshInterval(b.cfg.MinRefreshInterval, refresh); err != nil { + if err := b.dashboardService.ValidateDashboardRefreshInterval(b.minRefreshInterval, refresh); err != nil { return apierrors.NewBadRequest(err.Error()) } @@ -422,7 +401,7 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A } // Validate refresh interval - if err := b.dashboardService.ValidateDashboardRefreshInterval(b.cfg.MinRefreshInterval, refresh); err != nil { + if err := b.dashboardService.ValidateDashboardRefreshInterval(b.minRefreshInterval, refresh); err != nil { return apierrors.NewBadRequest(err.Error()) } @@ -592,7 +571,7 @@ func (b *DashboardsAPIBuilder) storageForVersion( return nil } - legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, b.reg, b.dashboardPermissions, b.accessClient) + legacyStore, err := b.legacy.NewStore(dashboards, opts.Scheme, opts.OptsGetter, opts.MetricsRegister, b.dashboardPermissions, b.accessClient) if err != nil { return err } @@ -692,7 +671,7 @@ func (b *DashboardsAPIBuilder) verifyFolderAccessPermissions(ctx context.Context var accessInfo folders.FolderAccessInfo err = runtime.DefaultUnstructuredConverter.FromUnstructured(resp.Object, &accessInfo) if err != nil { - b.log.Error("Failed to convert folder access response", "error", err) + logging.FromContext(ctx).Error("Failed to convert folder access response", "error", err) return dashboards.ErrFolderAccessDenied } diff --git a/pkg/registry/apis/dashboard/schema_validation.go b/pkg/registry/apis/dashboard/schema_validation.go index 4263d2af388..c53ed82481a 100644 --- a/pkg/registry/apis/dashboard/schema_validation.go +++ b/pkg/registry/apis/dashboard/schema_validation.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/grafana/grafana-app-sdk/logging" v0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" v1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" v2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" @@ -62,7 +63,7 @@ func (b *DashboardsAPIBuilder) ValidateDashboardSpec(ctx context.Context, obj ru } if alwaysLogSchemaValidationErrors && len(errors) > 0 { - b.log.Info("Schema validation errors during dashboard validation", "group_version", obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), "name", accessor.GetName(), "errors", errors.ToAggregate().Error(), "schema_version_mismatch", schemaVersionError != nil) + logging.FromContext(ctx).Info("Schema validation errors during dashboard validation", "group_version", obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), "name", accessor.GetName(), "errors", errors.ToAggregate().Error(), "schema_version_mismatch", schemaVersionError != nil) } if errorOnSchemaMismatches { diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index 80ec5c8f12c..d3023664f65 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -9,9 +9,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" - claims "github.com/grafana/authlib/types" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" - "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" @@ -145,14 +144,12 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob access.CanAdmin, _ = r.accessControl.Evaluate(ctx, user, adminEvaluator) deleteEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsDelete, dashScope) access.CanDelete, _ = r.accessControl.Evaluate(ctx, user, deleteEvaluator) - access.CanStar = user.IsIdentityType(claims.TypeUser) + access.CanStar = user.IsIdentityType(authlib.TypeUser) access.AnnotationsPermissions = &dashboard.AnnotationPermission{} r.getAnnotationPermissionsByScope(ctx, user, &access.AnnotationsPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard) r.getAnnotationPermissionsByScope(ctx, user, &access.AnnotationsPermissions.Organization, accesscontrol.ScopeAnnotationsTypeOrganization) - // FIXME!!!! does not get the title! - // The title property next to unstructured and not found in this model title := obj.FindTitle("") access.Slug = slugify.Slugify(title) access.Url = dashboards.GetDashboardFolderURL(false, name, access.Slug) diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 11a6de3b807..40e49ead04c 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -809,7 +809,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api identitySynchronizer := authnimpl.ProvideIdentitySynchronizer(authnimplService) ldapImpl := service12.ProvideService(cfg, featureToggles, ssosettingsimplService) apiService := api4.ProvideService(cfg, routeRegisterImpl, accessControl, userService, authinfoimplService, ossGroups, identitySynchronizer, orgService, ldapImpl, userAuthTokenService, bundleregistryService) - dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, pluginstoreService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) + dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) snapshotsAPIBuilder := dashboardsnapshot.RegisterAPIService(serviceImpl, apiserverService, cfg, featureToggles, sqlStore, registerer) featureFlagAPIBuilder := featuretoggle.RegisterAPIService(featureManager, accessControl, apiserverService, cfg, registerer) dataSourceAPIBuilder, err := datasource.RegisterAPIService(featureToggles, apiserverService, middlewareHandler, scopedPluginDatasourceProvider, plugincontextProvider, pluginstoreService, accessControl, registerer) @@ -1417,7 +1417,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac identitySynchronizer := authnimpl.ProvideIdentitySynchronizer(authnimplService) ldapImpl := service12.ProvideService(cfg, featureToggles, ssosettingsimplService) apiService := api4.ProvideService(cfg, routeRegisterImpl, accessControl, userService, authinfoimplService, ossGroups, identitySynchronizer, orgService, ldapImpl, userAuthTokenService, bundleregistryService) - dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, pluginstoreService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) + dashboardsAPIBuilder := dashboard.RegisterAPIService(cfg, featureToggles, apiserverService, dashboardService, dashboardProvisioningService, service15, dashboardServiceImpl, dashboardPermissionsService, accessControl, accessClient, provisioningServiceImpl, dashboardsStore, registerer, sqlStore, tracingService, resourceClient, dualwriteService, sortService, quotaService, libraryPanelService, eventualRestConfigProvider, userService) snapshotsAPIBuilder := dashboardsnapshot.RegisterAPIService(serviceImpl, apiserverService, cfg, featureToggles, sqlStore, registerer) featureFlagAPIBuilder := featuretoggle.RegisterAPIService(featureManager, accessControl, apiserverService, cfg, registerer) dataSourceAPIBuilder, err := datasource.RegisterAPIService(featureToggles, apiserverService, middlewareHandler, scopedPluginDatasourceProvider, plugincontextProvider, pluginstoreService, accessControl, registerer) diff --git a/pkg/tests/apis/dashboard/integration/api_validation_test.go b/pkg/tests/apis/dashboard/integration/api_validation_test.go index 9239ce78309..6423f8edf56 100644 --- a/pkg/tests/apis/dashboard/integration/api_validation_test.go +++ b/pkg/tests/apis/dashboard/integration/api_validation_test.go @@ -647,12 +647,9 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) { }) t.Run("Dashboard refresh interval validations", func(t *testing.T) { - // Store original settings to restore after test - origCfg := ctx.Helper.GetEnv().Cfg - origMinRefreshInterval := origCfg.MinRefreshInterval - - // Set a fixed min_refresh_interval for all tests to make them predictable - ctx.Helper.GetEnv().Cfg.MinRefreshInterval = "10s" + // Test infrastructure is configured with + // [dashboards] + // min_refresh_interval = 10s testCases := []struct { name string @@ -720,9 +717,6 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) { } }) } - - // Restore original settings - ctx.Helper.GetEnv().Cfg.MinRefreshInterval = origMinRefreshInterval }) t.Run("Dashboard size limit validations", func(t *testing.T) { diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 0b026b7e185..bac5f5208f7 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -545,6 +545,11 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) } + dashboardsSection, err := getOrCreateSection("dashboards") + require.NoError(t, err) + _, err = dashboardsSection.NewKey("min_refresh_interval", "10s") + require.NoError(t, err) + if opts.APIServerRuntimeConfig != "" { section, err := getOrCreateSection("grafana-apiserver") require.NoError(t, err) From 64c700e5638e3f769f6cccaa4cb18d6ea2bab402 Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:25:41 -0700 Subject: [PATCH 09/33] Provisioning: kind name should be singular (#111570) --- apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go index 000ff7f22ff..777484828f6 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go @@ -18,7 +18,7 @@ const ( ) var RepositoryResourceInfo = utils.NewResourceInfo(GROUP, VERSION, - "repositories", "repository", "Repositories", + "repositories", "repository", "Repository", func() runtime.Object { return &Repository{} }, // newObj func() runtime.Object { return &RepositoryList{} }, // newList utils.TableColumns{ // Returned by `kubectl get`. Doesn't affect disk storage. From 9172daa3dd4cbf5e91fa397eadd58ce3ff46b29b Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 24 Sep 2025 16:37:55 -0600 Subject: [PATCH 10/33] Provisioning: Allow viewers to view /status subresource and wait on the frontend before sync (#111576) --- pkg/registry/apis/provisioning/register.go | 6 ++++- .../Wizard/ProvisioningWizard.test.tsx | 22 +++++++++++++++++++ .../provisioning/Wizard/SynchronizeStep.tsx | 19 +++++++++++++--- public/locales/en-US/grafana.json | 2 ++ 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index ffe315f63b6..472f0a088ec 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -355,7 +355,11 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { } else { return authorizer.DecisionDeny, "editor role is required", nil } - + case "status": + if id.GetOrgRole().Includes(identity.RoleViewer) && a.GetVerb() == apiutils.VerbGet { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "users cannot update the status of a repository", nil default: if id.GetIsGrafanaAdmin() { return authorizer.DecisionAllow, "", nil diff --git a/public/app/features/provisioning/Wizard/ProvisioningWizard.test.tsx b/public/app/features/provisioning/Wizard/ProvisioningWizard.test.tsx index 3fe7eeb57dc..e6d46be3471 100644 --- a/public/app/features/provisioning/Wizard/ProvisioningWizard.test.tsx +++ b/public/app/features/provisioning/Wizard/ProvisioningWizard.test.tsx @@ -7,6 +7,7 @@ import { useCreateRepositoryJobsMutation, useGetFrontendSettingsQuery, useGetRepositoryFilesQuery, + useGetRepositoryStatusQuery, useGetResourceStatsQuery, } from 'app/api/clients/provisioning/v0alpha1'; @@ -29,6 +30,7 @@ jest.mock('app/api/clients/provisioning/v0alpha1', () => ({ ...jest.requireActual('app/api/clients/provisioning/v0alpha1'), useGetFrontendSettingsQuery: jest.fn(), useGetRepositoryFilesQuery: jest.fn(), + useGetRepositoryStatusQuery: jest.fn(), useGetResourceStatsQuery: jest.fn(), useCreateRepositoryJobsMutation: jest.fn(), })); @@ -43,6 +45,9 @@ const mockUseGetFrontendSettingsQuery = useGetFrontendSettingsQuery as jest.Mock const mockUseGetRepositoryFilesQuery = useGetRepositoryFilesQuery as jest.MockedFunction< typeof useGetRepositoryFilesQuery >; +const mockUseGetRepositoryStatusQuery = useGetRepositoryStatusQuery as jest.MockedFunction< + typeof useGetRepositoryStatusQuery +>; const mockUseGetResourceStatsQuery = useGetResourceStatsQuery as jest.MockedFunction; const mockUseCreateRepositoryJobsMutation = useCreateRepositoryJobsMutation as jest.MockedFunction< typeof useCreateRepositoryJobsMutation @@ -131,6 +136,23 @@ describe('ProvisioningWizard', () => { refetch: jest.fn(), }); + mockUseGetRepositoryStatusQuery.mockReturnValue({ + data: { + status: { + health: { + healthy: true, + checked: true, + message: '', + }, + }, + }, + isLoading: false, + isFetching: false, + isError: false, + error: null, + refetch: jest.fn(), + }); + mockUseGetResourceStatsQuery.mockReturnValue({ data: { instance: [], diff --git a/public/app/features/provisioning/Wizard/SynchronizeStep.tsx b/public/app/features/provisioning/Wizard/SynchronizeStep.tsx index 09ab5ff7a24..3e7a70292e6 100644 --- a/public/app/features/provisioning/Wizard/SynchronizeStep.tsx +++ b/public/app/features/provisioning/Wizard/SynchronizeStep.tsx @@ -40,7 +40,9 @@ export function SynchronizeStep({ isLegacyStorage, onCancel, isCancelling }: Syn message: repositoryHealthMessages, checked, } = repositoryStatusQuery?.data?.status?.health || {}; - const isButtonDisabled = checked !== undefined && isRepositoryHealthy === false; + + const hasError = repositoryStatusQuery.isError; + const isButtonDisabled = hasError || (checked !== undefined && isRepositoryHealthy === false); const startSynchronization = async () => { const [history] = getValues(['migrate.history']); @@ -65,7 +67,18 @@ export function SynchronizeStep({ isLegacyStorage, onCancel, isCancelling }: Syn to the repository and provisioned back into the instance. - {repositoryHealthMessages && !isRepositoryHealthy && ( + {hasError && ( + + )} + {repositoryHealthMessages && !isRepositoryHealthy && !hasError && ( - {isRepositoryHealthy === false ? ( + {hasError || isRepositoryHealthy === false ? (