From 746e2eeee6af891700dfd628cd877f6ac6b69bd7 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Mon, 29 Jul 2024 11:59:15 +0200 Subject: [PATCH] Alerting: Add validation for path separators in the rule group edit modal (#90887) Co-authored-by: Tom Ratcliffe --- .../rules/EditRuleGroupModal.test.tsx | 64 ++++++------------- .../components/rules/EditRuleGroupModal.tsx | 9 +++ 2 files changed, 28 insertions(+), 45 deletions(-) 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 99facc65e27..7569cb33d18 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx @@ -1,6 +1,4 @@ -import { render } from '@testing-library/react'; -import * as React from 'react'; -import { Provider } from 'react-redux'; +import { render, screen, userEvent } from 'test/test-utils'; import { byLabelText, byTestId, byText, byTitle } from 'testing-library-selector'; import { CombinedRuleNamespace } from 'app/types/unified-alerting'; @@ -13,8 +11,6 @@ import { mockPromRecordingRule, mockRulerAlertingRule, mockRulerRecordingRule, - mockRulerRuleGroup, - mockStore, } from '../../mocks'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; @@ -31,29 +27,8 @@ const ui = { tableRows: byTestId('row'), noRulesText: byText('This group does not contain alert rules.'), }; -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' }), - ], -}); -jest.mock('app/types', () => ({ - ...jest.requireActual('app/types'), - useDispatch: () => jest.fn(), -})); - -function getProvidersWrapper() { - return function Wrapper({ children }: React.PropsWithChildren<{}>) { - const store = mockStore(() => null); - return {children}; - }; -} +const noop = () => jest.fn(); describe('EditGroupModal', () => { it('Should disable all inputs but interval when intervalEditOnly is set', async () => { @@ -65,9 +40,7 @@ describe('EditGroupModal', () => { const group = namespace.groups[0]; - render( jest.fn()} />, { - wrapper: getProvidersWrapper(), - }); + render(); expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); expect(ui.input.group.get()).toHaveAttribute('readonly'); @@ -107,9 +80,7 @@ describe('EditGroupModal component on cloud alert rules', () => { const group = promNs.groups[0]; - render( jest.fn()} />, { - wrapper: getProvidersWrapper(), - }); + render(); expect(await ui.input.namespace.find()).toHaveValue('prometheus-ns'); expect(ui.input.namespace.get()).not.toHaveAttribute('readonly'); @@ -128,9 +99,7 @@ describe('EditGroupModal component on cloud alert rules', () => { const group = promNs.groups[0]; - render(, { - wrapper: getProvidersWrapper(), - }); + render(); expect(ui.table.query()).not.toBeInTheDocument(); expect(await ui.noRulesText.find()).toBeInTheDocument(); }); @@ -163,10 +132,11 @@ describe('EditGroupModal component on grafana-managed alert rules', () => { const grafanaGroup1 = grafanaNamespace.groups[0]; + const renderWithGrafanaGroup = () => + render(); + it('Should show alert table', async () => { - render(, { - wrapper: getProvidersWrapper(), - }); + renderWithGrafanaGroup(); expect(await ui.input.namespace.find()).toHaveValue('namespace1'); expect(ui.input.group.get()).toHaveValue('grafanaGroup1'); @@ -178,18 +148,22 @@ describe('EditGroupModal component on grafana-managed alert rules', () => { }); it('Should have folder input in readonly mode', async () => { - render(, { - wrapper: getProvidersWrapper(), - }); + renderWithGrafanaGroup(); expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); }); it('Should not display folder link if no folderUrl provided', async () => { - render(, { - wrapper: getProvidersWrapper(), - }); + renderWithGrafanaGroup(); expect(await ui.input.namespace.find()).toHaveValue('namespace1'); expect(ui.folderLink.query()).not.toBeInTheDocument(); }); + + it('does not allow slashes in the group name', async () => { + const user = userEvent.setup(); + renderWithGrafanaGroup(); + await user.type(await ui.input.group.find(), 'group/with/slashes'); + await user.click(ui.input.interval.get()); + expect(await screen.findByText(/cannot contain \"\/\"/i)).toBeInTheDocument(); + }); }); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index d119d68b47a..73015c380ce 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -28,6 +28,7 @@ import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; import { decodeGrafanaNamespace, encodeGrafanaNamespace } from '../expressions/util'; import { EvaluationGroupQuickPick } from '../rule-editor/EvaluationGroupQuickPick'; import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior'; +import { checkForPathSeparator } from '../rule-editor/util'; const ITEMS_PER_PAGE = 10; @@ -299,6 +300,11 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly || isGrafanaManagedGroup} {...register('namespaceName', { required: 'Namespace name is required.', + validate: { + // for Grafana-managed we do not validate the name of the folder because we use the UID anyway + pathSeparator: (namespaceName) => + isGrafanaManagedGroup ? true : checkForPathSeparator(namespaceName), + }, })} /> @@ -331,6 +337,9 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly} {...register('groupName', { required: 'Evaluation group name is required.', + validate: { + pathSeparator: (namespace) => checkForPathSeparator(namespace), + }, })} />