From f4e7402e3ff70be79063df16412c3387ba49711c Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 30 Jul 2024 12:05:17 +0200 Subject: [PATCH] [v11.1.x] Alerting: Add validation for path separators in the rule group edit modal (#91180) --- .../rules/EditRuleGroupModal.test.tsx | 63 ++++++------------- .../components/rules/EditRuleGroupModal.tsx | 9 +++ 2 files changed, 28 insertions(+), 44 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 5e916d86781..2820bc3b2e8 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,5 @@ -import { render } from '@testing-library/react'; import 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 +12,6 @@ import { mockPromRecordingRule, mockRulerAlertingRule, mockRulerRecordingRule, - mockRulerRuleGroup, - mockStore, } from '../../mocks'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; @@ -31,29 +28,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 +41,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 +81,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 +100,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 +133,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 +149,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 662c582d1cd..13b0d8c46bc 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -24,6 +24,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; @@ -278,6 +279,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), + }, })} /> @@ -310,6 +316,9 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly} {...register('groupName', { required: 'Evaluation group name is required.', + validate: { + pathSeparator: (namespace) => checkForPathSeparator(namespace), + }, })} />