From 937a043fabbbe326efdb405abba5dc487f357deb Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 30 Jul 2024 12:05:02 +0200 Subject: [PATCH] [v11.0.x] Alerting: Add validation for path separators in the rule group edit modal (#91179) --- .../rules/EditRuleGroupModal.test.tsx | 89 ++++++++----------- .../components/rules/EditRuleGroupModal.tsx | 9 ++ 2 files changed, 44 insertions(+), 54 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 5d7137c0935..cb029eeca3a 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,7 @@ -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; -import { Provider } from 'react-redux'; +import { TestProvider } from 'test/helpers/TestProvider'; import { byLabelText, byTestId, byText, byTitle } from 'testing-library-selector'; import { CombinedRuleNamespace } from 'app/types/unified-alerting'; @@ -13,8 +14,6 @@ import { mockPromRecordingRule, mockRulerAlertingRule, mockRulerRecordingRule, - mockRulerRuleGroup, - mockStore, } from '../../mocks'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; @@ -31,32 +30,11 @@ 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', () => { + it('Should disable all inputs but interval when intervalEditOnly is set', async () => { const namespace = mockCombinedRuleNamespace({ name: 'my-alerts', rulesSource: mockDataSource(), @@ -65,11 +43,11 @@ describe('EditGroupModal', () => { const group = namespace.groups[0]; - render( jest.fn()} />, { - wrapper: getProvidersWrapper(), + render(, { + wrapper: TestProvider, }); - expect(ui.input.namespace.get()).toHaveAttribute('readonly'); + expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); expect(ui.input.group.get()).toHaveAttribute('readonly'); expect(ui.input.interval.get()).not.toHaveAttribute('readonly'); }); @@ -96,7 +74,7 @@ describe('EditGroupModal component on cloud alert rules', () => { rulerRule: mockRulerRecordingRule({ record: 'recording-rule-cpu' }), }); - it('Should show alert table in case of having some non-recording rules in the group', () => { + it('Should show alert table in case of having some non-recording rules in the group', async () => { const promNs = mockCombinedRuleNamespace({ name: 'prometheus-ns', rulesSource: promDsSettings, @@ -107,11 +85,9 @@ describe('EditGroupModal component on cloud alert rules', () => { const group = promNs.groups[0]; - render( jest.fn()} />, { - wrapper: getProvidersWrapper(), - }); + render(, { wrapper: TestProvider }); - expect(ui.input.namespace.get()).toHaveValue('prometheus-ns'); + expect(await ui.input.namespace.find()).toHaveValue('prometheus-ns'); expect(ui.input.namespace.get()).not.toHaveAttribute('readonly'); expect(ui.input.group.get()).toHaveValue('default-group'); @@ -119,7 +95,7 @@ describe('EditGroupModal component on cloud alert rules', () => { expect(ui.tableRows.getAll()[0]).toHaveTextContent('alerting-rule-cpu'); }); - it('Should not show alert table in case of having exclusively recording rules in the group', () => { + it('Should not show alert table in case of having exclusively recording rules in the group', async () => { const promNs = mockCombinedRuleNamespace({ name: 'prometheus-ns', rulesSource: promDsSettings, @@ -128,11 +104,9 @@ describe('EditGroupModal component on cloud alert rules', () => { const group = promNs.groups[0]; - render(, { - wrapper: getProvidersWrapper(), - }); + render(, { wrapper: TestProvider }); expect(ui.table.query()).not.toBeInTheDocument(); - expect(ui.noRulesText.get()).toBeInTheDocument(); + expect(await ui.noRulesText.find()).toBeInTheDocument(); }); }); @@ -163,12 +137,15 @@ describe('EditGroupModal component on grafana-managed alert rules', () => { const grafanaGroup1 = grafanaNamespace.groups[0]; - it('Should show alert table', () => { - render(, { - wrapper: getProvidersWrapper(), + const renderWithGrafanaGroup = () => + render(, { + wrapper: TestProvider, }); - expect(ui.input.namespace.get()).toHaveValue('namespace1'); + it('Should show alert table', async () => { + renderWithGrafanaGroup(); + + expect(await ui.input.namespace.find()).toHaveValue('namespace1'); expect(ui.input.group.get()).toHaveValue('grafanaGroup1'); expect(ui.input.interval.get()).toHaveValue('30s'); @@ -177,19 +154,23 @@ describe('EditGroupModal component on grafana-managed alert rules', () => { expect(ui.tableRows.getAll()[1]).toHaveTextContent('high-memory'); }); - it('Should have folder input in readonly mode', () => { - render(, { - wrapper: getProvidersWrapper(), - }); + it('Should have folder input in readonly mode', async () => { + renderWithGrafanaGroup(); - expect(ui.input.namespace.get()).toHaveAttribute('readonly'); + expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); }); - it('Should not display folder link if no folderUrl provided', () => { - render(, { - wrapper: getProvidersWrapper(), - }); - + it('Should not display folder link if no folderUrl provided', async () => { + 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 4864237c17c..6d4d50f642c 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -22,6 +22,7 @@ import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '.. import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; import { decodeGrafanaNamespace, encodeGrafanaNamespace } from '../expressions/util'; import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior'; +import { checkForPathSeparator } from '../rule-editor/util'; const ITEMS_PER_PAGE = 10; @@ -268,6 +269,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), + }, })} /> @@ -293,6 +299,9 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly} {...register('groupName', { required: 'Evaluation group name is required.', + validate: { + pathSeparator: (namespace) => checkForPathSeparator(namespace), + }, })} />