From 69b2aade1bc77c9d2ba81342564ecb816c16bf9a Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Mon, 20 Feb 2023 08:47:50 +0100 Subject: [PATCH] Alerting: Make the folder field read-only on the eval group modal (#62935) * Make the folder field read-only on the eval group modal * Code cleanup * Use useCombinedRuleNamespace to simplify groups fetching * Fix groups filtering and label * Revert go test changes, add folder button title * Revert go changes * Remove folder link when no url provided, fix messages * Fix tests * Fix lint --- .../alerting/unified/RuleList.test.tsx | 25 +- .../components/rule-editor/FolderAndGroup.tsx | 53 ++-- .../rule-editor/GrafanaEvaluationBehavior.tsx | 67 +++-- .../rules/EditRuleGroupModal.test.tsx | 253 ++++++++++-------- .../components/rules/EditRuleGroupModal.tsx | 183 +++++-------- .../unified/components/rules/RulesGroup.tsx | 10 +- .../alerting/unified/state/actions.ts | 30 +-- .../features/alerting/unified/utils/misc.ts | 6 + 8 files changed, 303 insertions(+), 324 deletions(-) diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index 2002599fb2c..81ba90f7178 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -1,5 +1,5 @@ import { SerializedError } from '@reduxjs/toolkit'; -import { render, screen, waitFor } from '@testing-library/react'; +import { prettyDOM, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { TestProvider } from 'test/helpers/TestProvider'; @@ -119,8 +119,9 @@ const ui = { newRuleButton: byRole('link', { name: 'Create alert rule' }), exportButton: byRole('button', { name: /export/i }), editGroupModal: { - namespaceInput: byRole('textbox', { hidden: true, name: /namespace/i }), - ruleGroupInput: byRole('textbox', { name: 'Evaluation group', exact: true }), + dialog: byRole('dialog'), + namespaceInput: byRole('textbox', { name: /^Namespace/ }), + ruleGroupInput: byRole('textbox', { name: /Evaluation group/ }), intervalInput: byRole('textbox', { name: /Rule group evaluation interval Evaluation interval should be smaller or equal to 'For' values for existing rules in this group./i, }), @@ -556,17 +557,19 @@ describe('RuleList', () => { expect(await ui.rulesFilterInput.find()).toHaveValue(''); + await waitFor(() => expect(ui.ruleGroup.queryAll()).toHaveLength(3)); + const groups = await ui.ruleGroup.findAll(); expect(groups).toHaveLength(3); // open edit dialog await userEvent.click(ui.editCloudGroupIcon.get(groups[0])); - await expect(screen.getByRole('textbox', { hidden: true, name: /namespace/i })).toHaveDisplayValue( - 'namespace1' - ); - await expect(screen.getByRole('textbox', { name: 'Evaluation group', exact: true })).toHaveDisplayValue( - 'group1' - ); + + await waitFor(() => expect(ui.editGroupModal.dialog.get()).toBeInTheDocument()); + prettyDOM(ui.editGroupModal.dialog.get()); + + expect(ui.editGroupModal.namespaceInput.get()).toHaveDisplayValue('namespace1'); + expect(ui.editGroupModal.ruleGroupInput.get()).toHaveDisplayValue('group1'); await fn(); }); } @@ -614,8 +617,8 @@ describe('RuleList', () => { testCase('rename just the lotex group', async () => { // make changes to form - await userEvent.clear(screen.getByRole('textbox', { name: 'Evaluation group', exact: true })); - await userEvent.type(screen.getByRole('textbox', { name: 'Evaluation group', exact: true }), 'super group'); + await userEvent.clear(ui.editGroupModal.ruleGroupInput.get()); + await userEvent.type(ui.editGroupModal.ruleGroupInput.get(), 'super group'); await userEvent.type( screen.getByRole('textbox', { name: /rule group evaluation interval evaluation interval should be smaller or equal to 'for' values for existing rules in this group\./i, diff --git a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx index bb36dad7fb4..2aabb21e8b1 100644 --- a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx @@ -10,8 +10,8 @@ import { FolderPickerFilter } from 'app/core/components/Select/FolderPicker'; import { contextSrv } from 'app/core/core'; import { DashboardSearchHit } from 'app/features/search/types'; import { AccessControlAction, useDispatch } from 'app/types'; -import { RulerRuleDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { useCombinedRuleNamespaces } from '../../hooks/useCombinedRuleNamespaces'; import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; import { fetchRulerRulesIfNotFetchedYet } from '../../state/actions'; import { RuleForm, RuleFormValues } from '../../types/rule-form'; @@ -19,56 +19,35 @@ import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { isGrafanaRulerRule } from '../../utils/rules'; import { InfoIcon } from '../InfoIcon'; -import { getIntervalForGroup } from './GrafanaEvaluationBehavior'; +import { MINUTE } from './AlertRuleForm'; import { containsSlashes, Folder, RuleFolderPicker } from './RuleFolderPicker'; import { checkForPathSeparator } from './util'; export const SLICE_GROUP_RESULTS_TO = 1000; -const useGetGroups = (groupfoldersForGrafana: RulerRulesConfigDTO | null | undefined, folderName: string) => { - const groupOptions = useMemo(() => { - const groupsForFolderResult: Array> = groupfoldersForGrafana - ? groupfoldersForGrafana[folderName] ?? [] - : []; - - const folderGroups = groupsForFolderResult.map((group) => ({ - name: group.name, - provisioned: group.rules.some((rule) => isGrafanaRulerRule(rule) && Boolean(rule.grafana_alert.provenance)), - })); - - return folderGroups.filter((group) => !group.provisioned).map((group) => group.name); - }, [groupfoldersForGrafana, folderName]); - - return groupOptions; -}; - -function mapGroupsToOptions( - groupsForFolder: RulerRulesConfigDTO | null | undefined, - groups: string[], - folderTitle: string -): Array> { - return groups.map((group) => ({ - label: group, - value: group, - description: `${getIntervalForGroup(groupsForFolder, group, folderTitle)}`, - })); -} interface FolderAndGroupProps { initialFolder: RuleForm | null; } export const useGetGroupOptionsFromFolder = (folderTitle: string) => { const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); - const groupfoldersForGrafana = rulerRuleRequests[GRAFANA_RULES_SOURCE_NAME]; - const groupsForFolder = groupfoldersForGrafana?.result; + const grafanaFolders = useCombinedRuleNamespaces(GRAFANA_RULES_SOURCE_NAME); + const folderGroups = grafanaFolders.find((f) => f.name === folderTitle)?.groups ?? []; + + const nonProvisionedGroups = folderGroups.filter((g) => { + return g.rules.every( + (r) => isGrafanaRulerRule(r.rulerRule) && Boolean(r.rulerRule.grafana_alert.provenance) === false + ); + }); + + const groupOptions = nonProvisionedGroups.map>((group) => ({ + label: group.name, + value: group.name, + description: group.interval ?? MINUTE, + })); - const groupOptions: Array> = mapGroupsToOptions( - groupsForFolder, - useGetGroups(groupfoldersForGrafana?.result, folderTitle), - folderTitle - ); return { groupOptions, loading: groupfoldersForGrafana?.loading }; }; diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index 5dfae11cf77..a1d5feab2b2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -5,9 +5,11 @@ import { RegisterOptions, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Stack } from '@grafana/experimental'; import { Button, Field, InlineLabel, Input, InputControl, useStyles2, Switch, Tooltip, Icon } from '@grafana/ui'; -import { RulerRuleDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { CombinedRuleGroup, CombinedRuleNamespace } from '../../../../../types/unified-alerting'; import { logInfo, LogMessages } from '../../Analytics'; +import { useCombinedRuleNamespaces } from '../../hooks/useCombinedRuleNamespaces'; import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; import { RuleForm, RuleFormValues } from '../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; @@ -22,18 +24,6 @@ import { RuleEditorSection } from './RuleEditorSection'; export const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds -export const getIntervalForGroup = ( - rulerRules: RulerRulesConfigDTO | null | undefined, - group: string, - folder: string -) => { - const folderObj: Array> = rulerRules ? rulerRules[folder] : []; - const groupObj = folderObj?.find((rule) => rule.name === group); - - const interval = groupObj?.interval ?? MINUTE; - return interval; -}; - const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({ required: { value: true, @@ -87,6 +77,10 @@ export const EvaluateEveryNewGroup = ({ rules }: { rules: RulerRulesConfigDTO | } = useFormContext(); const styles = useStyles2(getStyles); const evaluateEveryId = 'eval-every-input'; + const [groupName, folderName] = watch(['group', 'folder.title']); + + const groupRules = (rules && rules[folderName]?.find((g) => g.name === groupName)?.rules) ?? []; + return ( @@ -135,24 +126,25 @@ function FolderGroupAndEvaluationInterval({ const { watch, setValue } = useFormContext(); const [isEditingGroup, setIsEditingGroup] = useState(false); - const group = watch('group'); - const folder = watch('folder'); + const [groupName, folderName] = watch(['group', 'folder.title']); const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); const groupfoldersForGrafana = rulerRuleRequests[GRAFANA_RULES_SOURCE_NAME]; - const isNewGroup = useIsNewGroup(folder?.title ?? '', group); + const grafanaNamespaces = useCombinedRuleNamespaces(GRAFANA_RULES_SOURCE_NAME); + const existingNamespace = grafanaNamespaces.find((ns) => ns.name === folderName); + const existingGroup = existingNamespace?.groups.find((g) => g.name === groupName); + + const isNewGroup = useIsNewGroup(folderName ?? '', groupName); useEffect(() => { - if (!isNewGroup) { - group && - folder && - setEvaluateEvery(getIntervalForGroup(groupfoldersForGrafana?.result, group, folder?.title ?? '')); + if (!isNewGroup && existingGroup?.interval) { + setEvaluateEvery(existingGroup.interval); } else { setEvaluateEvery(MINUTE); setValue('evaluateEvery', MINUTE); } - }, [group, folder, groupfoldersForGrafana?.result, setEvaluateEvery, isNewGroup, setValue]); + }, [setEvaluateEvery, isNewGroup, setValue, existingGroup]); const closeEditGroupModal = (saved = false) => { if (!saved) { @@ -163,30 +155,36 @@ function FolderGroupAndEvaluationInterval({ const onOpenEditGroupModal = () => setIsEditingGroup(true); - const editGroupDisabled = groupfoldersForGrafana?.loading || isNewGroup || !folder || !group; + const editGroupDisabled = groupfoldersForGrafana?.loading || isNewGroup || !folderName || !groupName; + + const emptyNamespace: CombinedRuleNamespace = { + name: folderName, + rulesSource: GRAFANA_RULES_SOURCE_NAME, + groups: [], + }; + const emptyGroup: CombinedRuleGroup = { name: groupName, interval: evaluateEvery, rules: [] }; return (
- {isEditingGroup && ( + {folderName && isEditingGroup && ( closeEditGroupModal()} - folderAndGroupReadOnly + intervalEditOnly /> )} - {folder && group && ( + {folderName && groupName && (
- {isNewGroup && group ? ( + {isNewGroup && groupName ? ( ) : (
- {`Alert rules in the `} {group} group are evaluated every{' '} + {`Alert rules in the `} {groupName} group are evaluated every{' '} {evaluateEvery}.
{!isNewGroup && ( @@ -355,7 +353,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ margin: ${theme.spacing(2, 0, 2, -1)}; `, evaluateLabel: css` - align-self: left; margin-right: ${theme.spacing(1)}; `, evaluationContainer: css` diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx index f46fdb629a2..79c54e4023f 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx @@ -1,151 +1,192 @@ -import { render, screen } from '@testing-library/react'; +import { render } from '@testing-library/react'; import React from 'react'; import { Provider } from 'react-redux'; +import { byLabelText, byTestId, byText, byTitle } from 'testing-library-selector'; -import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting'; -import { RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { CombinedRuleNamespace } from 'app/types/unified-alerting'; import { mockCombinedRule, + mockCombinedRuleNamespace, mockDataSource, mockPromAlertingRule, + mockPromRecordingRule, mockRulerAlertingRule, mockRulerRecordingRule, mockRulerRuleGroup, mockStore, - someRulerRules, } from '../../mocks'; -import { GRAFANA_DATASOURCE_NAME } from '../../utils/datasource'; +import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; -import { CombinedGroupAndNameSpace, EditCloudGroupModal, ModalProps } from './EditRuleGroupModal'; +import { EditCloudGroupModal } from './EditRuleGroupModal'; -const dsSettings = mockDataSource({ - name: 'Prometheus-1', - uid: 'Prometheus-1', -}); - -export const someCloudRulerRules: RulerRulesConfigDTO = { - namespace1: [ - mockRulerRuleGroup({ - name: 'group1', - rules: [ - mockRulerRecordingRule({ - record: 'instance:node_num_cpu:sum', - expr: 'count without (cpu) (count without (mode) (node_cpu_seconds_total{job="integrations/node_exporter"}))', - labels: { type: 'cpu' }, - }), - mockRulerAlertingRule({ alert: 'nonRecordingRule' }), - ], - }), - ], +const ui = { + input: { + namespace: byLabelText(/^Folder|^Namespace/, { exact: true }), + group: byLabelText(/Evaluation group/), + interval: byLabelText(/Rule group evaluation interval/), + }, + folderLink: byTitle(/Go to folder/), // without a href has the generic role + table: byTestId('dynamic-table'), + tableRows: byTestId('row'), + noRulesText: byText('This group does not contain alert rules.'), }; - -export const onlyRecordingRulerRules: RulerRulesConfigDTO = { - namespace1: [ - mockRulerRuleGroup({ - name: 'group1', - rules: [ - mockRulerRecordingRule({ - record: 'instance:node_num_cpu:sum', - expr: 'count without (cpu) (count without (mode) (node_cpu_seconds_total{job="integrations/node_exporter"}))', - labels: { type: 'cpu' }, - }), - ], - }), - ], -}; - -const grafanaNamespace: CombinedRuleNamespace = { - name: 'namespace1', - rulesSource: dsSettings, - groups: [ - { - name: 'group1', - rules: [ - mockCombinedRule({ - namespace: { - groups: [], - name: 'namespace1', - rulesSource: mockDataSource(), - }, - promRule: mockPromAlertingRule(), - rulerRule: mockRulerAlertingRule(), - }), - ], - }, - ], -}; - -const group1: CombinedRuleGroup = { +mockRulerRuleGroup({ name: 'group1', rules: [ - mockCombinedRule({ - namespace: { - groups: [], - name: 'namespace1', - rulesSource: mockDataSource({ name: 'Prometheus-1' }), - }, - promRule: mockPromAlertingRule({ name: 'nonRecordingRule' }), - rulerRule: mockRulerAlertingRule({ alert: 'recordingRule' }), + mockRulerRecordingRule({ + record: 'instance:node_num_cpu:sum', + expr: 'count without (cpu) (count without (mode) (node_cpu_seconds_total{job="integrations/node_exporter"}))', + labels: { type: 'cpu' }, }), + mockRulerAlertingRule({ alert: 'nonRecordingRule' }), ], -}; - -const nameSpaceAndGroup: CombinedGroupAndNameSpace = { - namespace: grafanaNamespace, - group: group1, -}; -const defaultProps: ModalProps = { - nameSpaceAndGroup: nameSpaceAndGroup, - sourceName: 'Prometheus-1', - groupInterval: '1m', - onClose: jest.fn(), -}; +}); jest.mock('app/types', () => ({ ...jest.requireActual('app/types'), useDispatch: () => jest.fn(), })); -function getProvidersWrapper(cloudRules?: RulerRulesConfigDTO) { +function getProvidersWrapper() { return function Wrapper({ children }: React.PropsWithChildren<{}>) { - const store = mockStore((store) => { - store.unifiedAlerting.rulerRules[GRAFANA_DATASOURCE_NAME] = { - loading: false, - dispatched: true, - result: someRulerRules, - }; - store.unifiedAlerting.rulerRules['Prometheus-1'] = { - loading: false, - dispatched: true, - result: cloudRules ?? someCloudRulerRules, - }; - }); - + const store = mockStore(() => null); return {children}; }; } +describe('EditGroupModal', () => { + it('Should disable all inputs but interval when intervalEditOnly is set', () => { + const namespace = mockCombinedRuleNamespace({ + name: 'my-alerts', + rulesSource: mockDataSource(), + groups: [{ name: 'default-group', interval: '90s', rules: [] }], + }); + + const group = namespace.groups[0]; + + render( jest.fn()} />, { + wrapper: getProvidersWrapper(), + }); + + expect(ui.input.namespace.get()).toHaveAttribute('readonly'); + expect(ui.input.group.get()).toHaveAttribute('readonly'); + expect(ui.input.interval.get()).not.toHaveAttribute('readonly'); + }); +}); + describe('EditGroupModal component on cloud alert rules', () => { + const promDsSettings = mockDataSource({ name: 'Prometheus-1', uid: 'Prometheus-1' }); + + const alertingRule = mockCombinedRule({ + namespace: undefined, + promRule: mockPromAlertingRule({ name: 'alerting-rule-cpu' }), + rulerRule: mockRulerAlertingRule({ alert: 'alerting-rule-cpu' }), + }); + + const recordingRule1 = mockCombinedRule({ + namespace: undefined, + promRule: mockPromRecordingRule({ name: 'recording-rule-memory' }), + rulerRule: mockRulerRecordingRule({ record: 'recording-rule-memory' }), + }); + + const recordingRule2 = mockCombinedRule({ + namespace: undefined, + promRule: mockPromRecordingRule({ name: 'recording-rule-cpu' }), + rulerRule: mockRulerRecordingRule({ record: 'recording-rule-cpu' }), + }); + it('Should show alert table in case of having some non-recording rules in the group', () => { - render(, { + const promNs = mockCombinedRuleNamespace({ + name: 'prometheus-ns', + rulesSource: promDsSettings, + groups: [{ name: 'default-group', interval: '90s', rules: [alertingRule, recordingRule1, recordingRule2] }], + }); + + const group = promNs.groups[0]; + + render( jest.fn()} />, { wrapper: getProvidersWrapper(), }); - expect(screen.getByText(/nonRecordingRule/i)).toBeInTheDocument(); + + expect(ui.input.namespace.get()).toHaveValue('prometheus-ns'); + expect(ui.input.namespace.get()).not.toHaveAttribute('readonly'); + expect(ui.input.group.get()).toHaveValue('default-group'); + + expect(ui.tableRows.getAll()).toHaveLength(1); // Only one rule is non-recording + expect(ui.tableRows.getAll()[0]).toHaveTextContent('alerting-rule-cpu'); }); - it('Should not show alert table in case of not having some non-recording rules in the group', () => { - render(, { - wrapper: getProvidersWrapper(onlyRecordingRulerRules), + + it('Should not show alert table in case of having exclusively recording rules in the group', () => { + const promNs = mockCombinedRuleNamespace({ + name: 'prometheus-ns', + rulesSource: promDsSettings, + groups: [{ name: 'default-group', interval: '90s', rules: [recordingRule1, recordingRule2] }], }); - expect(screen.queryByText(/nonRecordingRule/i)).not.toBeInTheDocument(); - expect(screen.getByText(/this group does not contain alert rules\./i)); + + const group = promNs.groups[0]; + + render(, { + wrapper: getProvidersWrapper(), + }); + expect(ui.table.query()).not.toBeInTheDocument(); + expect(ui.noRulesText.get()).toBeInTheDocument(); }); }); + describe('EditGroupModal component on grafana-managed alert rules', () => { + const grafanaNamespace: CombinedRuleNamespace = { + name: 'namespace1', + rulesSource: GRAFANA_RULES_SOURCE_NAME, + groups: [ + { + name: 'grafanaGroup1', + interval: '30s', + rules: [ + mockCombinedRule({ + namespace: undefined, + promRule: mockPromAlertingRule({ name: 'high-cpu-1' }), + rulerRule: mockRulerAlertingRule({ alert: 'high-cpu-1' }), + }), + mockCombinedRule({ + namespace: undefined, + promRule: mockPromAlertingRule({ name: 'high-memory' }), + rulerRule: mockRulerAlertingRule({ alert: 'high-memory' }), + }), + ], + }, + ], + }; + + const grafanaGroup1 = grafanaNamespace.groups[0]; + it('Should show alert table', () => { - render(, { + render(, { wrapper: getProvidersWrapper(), }); - expect(screen.getByText(/alert1/i)).toBeInTheDocument(); + + expect(ui.input.namespace.get()).toHaveValue('namespace1'); + expect(ui.input.group.get()).toHaveValue('grafanaGroup1'); + expect(ui.input.interval.get()).toHaveValue('30s'); + + expect(ui.tableRows.getAll()).toHaveLength(2); + expect(ui.tableRows.getAll()[0]).toHaveTextContent('high-cpu-1'); + expect(ui.tableRows.getAll()[1]).toHaveTextContent('high-memory'); + }); + + it('Should have folder input in readonly mode', () => { + render(, { + wrapper: getProvidersWrapper(), + }); + + expect(ui.input.namespace.get()).toHaveAttribute('readonly'); + }); + + it('Should not display folder link if no folderUrl provided', () => { + render(, { + wrapper: getProvidersWrapper(), + }); + + expect(ui.folderLink.query()).not.toBeInTheDocument(); }); }); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index d7fd81e8e40..27b6d77b3aa 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -1,10 +1,11 @@ import { css } from '@emotion/css'; +import { compact } from 'lodash'; import React, { useEffect, useMemo } from 'react'; import { FormProvider, RegisterOptions, useForm, useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { Badge, Button, Field, Input, Label, Modal, useStyles2 } from '@grafana/ui'; +import { Badge, Button, Field, Input, Label, LinkButton, Modal, useStyles2 } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; import { useCleanup } from 'app/core/hooks/useCleanup'; import { useDispatch } from 'app/types'; @@ -14,7 +15,7 @@ import { RulerRuleDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/ import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; import { rulesInSameGroupHaveInvalidFor, updateLotexNamespaceAndGroupAction } from '../../state/actions'; import { checkEvaluationIntervalGlobalLimit } from '../../utils/config'; -import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; +import { getRulesSourceName, GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { initialAsyncRequestState } from '../../utils/redux'; import { isAlertingRulerRule, isGrafanaRulerRule, isRecordingRulerRule } from '../../utils/rules'; import { parsePrometheusDuration } from '../../utils/time'; @@ -23,13 +24,14 @@ import { InfoIcon } from '../InfoIcon'; import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior'; -const MINUTE = '1m'; const ITEMS_PER_PAGE = 10; + interface AlertInfo { alertName: string; forDuration: string; evaluationsToFire: number; } + function ForBadge({ message, error }: { message: string; error?: boolean }) { if (error) { return ; @@ -100,17 +102,6 @@ export const getGroupFromRuler = ( const folderObj: Array> = rulerRules ? rulerRules[folderName] : []; return folderObj?.find((rulerRuleGroup) => rulerRuleGroup.name === groupName); }; - -export const getIntervalForGroup = ( - rulerRules: RulerRulesConfigDTO | null | undefined, - groupName: string, - folderName: string -) => { - const group = getGroupFromRuler(rulerRules, groupName, folderName); - const interval = group?.interval ?? MINUTE; - return interval; -}; - export const safeParseDurationstr = (duration: string): number => { try { return parsePrometheusDuration(duration); @@ -188,40 +179,20 @@ export const RulesForGroupTable = ({ rulesWithoutRecordingRules }: { rulesWithou ); }; -export interface CombinedGroupAndNameSpace { - namespace: CombinedRuleNamespace; - group: CombinedRuleGroup; -} -interface GroupAndNameSpaceNames { - namespace: string; - group: string; -} -export interface ModalProps { - nameSpaceAndGroup: CombinedGroupAndNameSpace | GroupAndNameSpaceNames; - sourceName: string; - groupInterval: string; - onClose: (saved?: boolean) => void; - folderAndGroupReadOnly?: boolean; -} - interface FormValues { namespaceName: string; groupName: string; groupInterval: string; } -export const evaluateEveryValidationOptions = ( - rules: RulerRulesConfigDTO | null | undefined, - groupName: string, - nameSpaceName: string -): RegisterOptions => ({ +export const evaluateEveryValidationOptions = (rules: RulerRuleDTO[]): RegisterOptions => ({ required: { value: true, message: 'Required.', }, - validate: (value: string) => { + validate: (evaluateEvery: string) => { try { - const duration = parsePrometheusDuration(value); + const duration = parsePrometheusDuration(evaluateEvery); if (duration < MIN_TIME_RANGE_STEP_S * 1000) { return `Cannot be less than ${MIN_TIME_RANGE_STEP_S} seconds.`; @@ -230,7 +201,7 @@ export const evaluateEveryValidationOptions = ( if (duration % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) { return `Must be a multiple of ${MIN_TIME_RANGE_STEP_S} seconds.`; } - if (rulesInSameGroupHaveInvalidFor(rules, groupName, nameSpaceName, value).length === 0) { + if (rulesInSameGroupHaveInvalidFor(rules, evaluateEvery).length === 0) { return true; } else { return `Invalid evaluation interval. Evaluation interval should be smaller or equal to 'For' values for existing rules in this group.`; @@ -241,43 +212,37 @@ export const evaluateEveryValidationOptions = ( }, }); +export interface ModalProps { + namespace: CombinedRuleNamespace; + group: CombinedRuleGroup; + onClose: (saved?: boolean) => void; + intervalEditOnly?: boolean; + folderUrl?: string; +} + export function EditCloudGroupModal(props: ModalProps): React.ReactElement { - const { - nameSpaceAndGroup: { namespace, group }, - onClose, - groupInterval, - sourceName, - folderAndGroupReadOnly, - } = props; + const { namespace, group, onClose, intervalEditOnly } = props; const styles = useStyles2(getStyles); const dispatch = useDispatch(); const { loading, error, dispatched } = useUnifiedAlertingSelector((state) => state.updateLotexNamespaceAndGroup) ?? initialAsyncRequestState; const notifyApp = useAppNotification(); - const nameSpaceName = typeof namespace === 'string' ? namespace : namespace.name; - const groupName = typeof group === 'string' ? group : group.name; + const defaultValues = useMemo( (): FormValues => ({ - namespaceName: nameSpaceName, - groupName: groupName, - groupInterval: groupInterval ?? '', + namespaceName: namespace.name, + groupName: group.name, + groupInterval: group.interval ?? '', }), - [nameSpaceName, groupName, groupInterval] + [namespace, group] ); - const isGrafanaManagedGroup = sourceName === GRAFANA_RULES_SOURCE_NAME; - const nameSpaceLabel = isGrafanaManagedGroup ? 'Folder' : 'Namespace'; - const nameSpaceInfoIconLabelEditable = isGrafanaManagedGroup - ? 'Folder name can be updated to a non-existing folder name' - : 'Name space can be updated to a non-existing name space'; - const nameSpaceInfoIconLabelNonEditable = isGrafanaManagedGroup - ? 'Folder name can be updated in folder view' - : 'Name space can be updated folder view'; + const rulesSourceName = getRulesSourceName(namespace.rulesSource); + const isGrafanaManagedGroup = rulesSourceName === GRAFANA_RULES_SOURCE_NAME; + + const nameSpaceLabel = isGrafanaManagedGroup ? 'Folder' : 'Namespace'; - const spaceNameInfoIconLabel = folderAndGroupReadOnly - ? nameSpaceInfoIconLabelNonEditable - : nameSpaceInfoIconLabelEditable; // close modal if successfully saved useEffect(() => { if (dispatched && !loading && !error) { @@ -289,10 +254,10 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { const onSubmit = (values: FormValues) => { dispatch( updateLotexNamespaceAndGroupAction({ - rulesSourceName: sourceName, - groupName: groupName, + rulesSourceName: rulesSourceName, + groupName: group.name, newGroupName: values.groupName, - namespaceName: nameSpaceName, + namespaceName: namespace.name, newNamespaceName: values.namespaceName, groupInterval: values.groupInterval || undefined, }) @@ -315,56 +280,60 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { notifyApp.error('There are errors in the form. Correct the errors and retry.'); }; - const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); - const groupfoldersForSource = rulerRuleRequests[sourceName]; - - const groupWithRules = getGroupFromRuler(groupfoldersForSource?.result, groupName, nameSpaceName); - const rulesWithoutRecordingRules: RulerRuleDTO[] = - groupWithRules?.rules.filter((rule: RulerRuleDTO) => !isRecordingRulerRule(rule)) ?? []; + const rulesWithoutRecordingRules = compact( + group.rules.map((r) => r.rulerRule).filter((rule) => !isRecordingRulerRule(rule)) + ); const hasSomeNoRecordingRules = rulesWithoutRecordingRules.length > 0; + const modalTitle = + intervalEditOnly || isGrafanaManagedGroup ? 'Edit evaluation group' : 'Edit namespace or evaluation group'; return ( - +
e.preventDefault()} key={JSON.stringify(defaultValues)}> <> - - {nameSpaceLabel} - - + } invalid={!!errors.namespaceName} error={errors.namespaceName?.message} > - + + + {isGrafanaManagedGroup && props.folderUrl && ( + + )} + - - Evaluation group - {isGrafanaManagedGroup ? ( - - ) : ( - - )} - + } invalid={!!errors.groupName} @@ -372,7 +341,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { > @@ -426,8 +392,8 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
- {rulerRuleRequests && !hasSomeNoRecordingRules &&
This group does not contain alert rules.
} - {rulerRuleRequests && hasSomeNoRecordingRules && ( + {!hasSomeNoRecordingRules &&
This group does not contain alert rules.
} + {hasSomeNoRecordingRules && ( <>
List of rules that belong to this group
@@ -452,10 +418,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ position: relative; `, formInput: css` - width: 275px; - & + & { - margin-left: ${theme.spacing(3)}; - } + flex: 1; `, tableWrapper: css` margin-top: ${theme.spacing(2)}; diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx index 9924f6fbe73..a955e7fcf4c 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx @@ -14,8 +14,8 @@ import { useFolder } from '../../hooks/useFolder'; import { useHasRuler } from '../../hooks/useHasRuler'; import { deleteRulesGroupAction } from '../../state/actions'; import { useRulesAccess } from '../../utils/accessControlHooks'; -import { getRulesSourceName, GRAFANA_RULES_SOURCE_NAME, isCloudRulesSource } from '../../utils/datasource'; -import { makeFolderLink } from '../../utils/misc'; +import { GRAFANA_RULES_SOURCE_NAME, isCloudRulesSource } from '../../utils/datasource'; +import { makeFolderLink, makeFolderSettingsLink } from '../../utils/misc'; import { isFederatedRuleGroup, isGrafanaRulerRule } from '../../utils/rules'; import { CollapseToggle } from '../CollapseToggle'; import { RuleLocation } from '../RuleLocation'; @@ -238,10 +238,10 @@ export const RulesGroup: FC = React.memo(({ group, namespace, expandAll, )} {isEditingGroup && ( closeEditModal()} + folderUrl={folder?.canEdit ? makeFolderSettingsLink(folder) : undefined} /> )} {isReorderingGroup && ( diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index c9efd56ddab..771cc97d83e 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -61,7 +61,7 @@ import { FetchRulerRulesFilter, setRulerRuleGroup, } from '../api/ruler'; -import { getAlertInfo, safeParseDurationstr, getGroupFromRuler } from '../components/rules/EditRuleGroupModal'; +import { getAlertInfo, safeParseDurationstr } from '../components/rules/EditRuleGroupModal'; import { RuleFormType, RuleFormValues } from '../types/rule-form'; import { addDefaultsToAlertmanagerConfig, removeMuteTimingFromRoute } from '../utils/alertmanager'; import { @@ -770,20 +770,12 @@ interface UpdateNamespaceAndGroupOptions { groupInterval?: string; } -export const rulesInSameGroupHaveInvalidFor = ( - rulerRules: RulerRulesConfigDTO | null | undefined, - groupName: string, - folderName: string, - everyDuration: string -) => { - const group = getGroupFromRuler(rulerRules, groupName, folderName); - - const rulesSameGroup: RulerRuleDTO[] = group?.rules ?? []; - - return rulesSameGroup.filter((rule: RulerRuleDTO) => { +export const rulesInSameGroupHaveInvalidFor = (rules: RulerRuleDTO[], everyDuration: string) => { + return rules.filter((rule: RulerRuleDTO) => { const { forDuration } = getAlertInfo(rule, everyDuration); const forNumber = safeParseDurationstr(forDuration); const everyNumber = safeParseDurationstr(everyDuration); + return forNumber !== 0 && forNumber < everyNumber; }); }; @@ -809,16 +801,20 @@ export const updateLotexNamespaceAndGroupAction: AsyncThunk< if (!existingNamespace) { throw new Error(`Namespace "${namespaceName}" not found.`); } + const existingGroup = rulesResult[namespaceName].find((group) => group.name === groupName); if (!existingGroup) { throw new Error(`Group "${groupName}" not found.`); } + const newGroupAlreadyExists = Boolean( rulesResult[namespaceName].find((group) => group.name === newGroupName) ); + if (newGroupName !== groupName && newGroupAlreadyExists) { throw new Error(`Group "${newGroupName}" already exists in namespace "${namespaceName}".`); } + const newNamespaceAlreadyExists = Boolean(rulesResult[newNamespaceName]); if (newNamespaceName !== namespaceName && newNamespaceAlreadyExists) { throw new Error(`Namespace "${newNamespaceName}" already exists.`); @@ -830,16 +826,10 @@ export const updateLotexNamespaceAndGroupAction: AsyncThunk< ) { throw new Error('Nothing changed.'); } + // validation for new groupInterval if (groupInterval !== existingGroup.interval) { - const storeState = thunkAPI.getState(); - const groupfoldersForSource = storeState?.unifiedAlerting.rulerRules[rulesSourceName]; - const notValidRules = rulesInSameGroupHaveInvalidFor( - groupfoldersForSource?.result, - groupName, - namespaceName, - groupInterval ?? '1m' - ); + const notValidRules = rulesInSameGroupHaveInvalidFor(existingGroup.rules, groupInterval ?? '1m'); if (notValidRules.length > 0) { throw new Error( `These alerts belonging to this group will have an invalid 'For' value: ${notValidRules diff --git a/public/app/features/alerting/unified/utils/misc.ts b/public/app/features/alerting/unified/utils/misc.ts index 181ccd33dfe..966b6cde76a 100644 --- a/public/app/features/alerting/unified/utils/misc.ts +++ b/public/app/features/alerting/unified/utils/misc.ts @@ -10,6 +10,8 @@ import { mapStateWithReasonToBaseState, } from 'app/types/unified-alerting-dto'; +import { FolderDTO } from '../../../../types'; + import { ALERTMANAGER_NAME_QUERY_KEY } from './constants'; import { getRulesSourceName } from './datasource'; import { getMatcherQueryParams } from './matchers'; @@ -104,6 +106,10 @@ export function makeFolderLink(folderUID: string): string { return createUrl(`/dashboards/f/${folderUID}`); } +export function makeFolderSettingsLink(folder: FolderDTO): string { + return createUrl(`/dashboards/f/${folder.uid}/${folder.title}/settings`); +} + // keep retrying fn if it's error passes shouldRetry(error) and timeout has not elapsed yet export function retryWhile( fn: () => Promise,