From a8bfaafeb5ef503a7764f2b390963ab8affd4e2b Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Wed, 13 Apr 2022 12:09:25 -0400 Subject: [PATCH] Alerting: FGAC for alert rule view and edit page (#47441) (#47717) * Add permission check to the find route, add query not accessible warning for the view page * Hide grafana or mimir rule buttons depending on user's permissions * Add grafana and cloud read rules checking on the alert rules list view * Improve missing data source handling, refactor edit and remove permissions handling * Add tests for rule edit permissions * PR feedback (cherry picked from commit a30ab515500644b1f9e314114386410971209073) Co-authored-by: Konrad Lalik --- public/app/features/alerting/routes.tsx | 22 +-- .../features/alerting/unified/RuleViewer.tsx | 59 ++++--- .../rule-editor/AlertTypeStep.test.tsx | 75 +++++++++ .../components/rule-editor/AlertTypeStep.tsx | 23 ++- .../components/rule-editor/PreviewRule.tsx | 39 +++-- .../rule-editor/rule-types/RuleTypePicker.tsx | 47 +++--- .../components/rules/RuleDetails.test.tsx | 26 +-- .../rules/RuleDetailsActionButtons.tsx | 51 +++--- .../rules/RuleListGroupView.test.tsx | 108 ++++++++++++ .../components/rules/RuleListGroupView.tsx | 10 +- .../unified/hooks/useAlertQueriesStatus.ts | 12 ++ .../unified/hooks/useIsRuleEditable.test.tsx | 156 ++++++++++++++++++ .../unified/hooks/useIsRuleEditable.ts | 15 +- public/app/features/alerting/unified/mocks.ts | 16 ++ 14 files changed, 541 insertions(+), 118 deletions(-) create mode 100644 public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx create mode 100644 public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx create mode 100644 public/app/features/alerting/unified/hooks/useAlertQueriesStatus.ts create mode 100644 public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx index ea93183aa40..d8eaa9cd338 100644 --- a/public/app/features/alerting/routes.tsx +++ b/public/app/features/alerting/routes.tsx @@ -1,12 +1,11 @@ -import React from 'react'; -import { Redirect } from 'react-router-dom'; +import { OrgRole } from '@grafana/data'; import { SafeDynamicImport } from 'app/core/components/DynamicImports/SafeDynamicImport'; import { config } from 'app/core/config'; import { RouteDescriptor } from 'app/core/navigation/types'; -import { uniq } from 'lodash'; -import { contextSrv } from 'app/core/core'; import { AccessControlAction } from 'app/types'; -import { OrgRole } from '@grafana/data'; +import { uniq } from 'lodash'; +import React from 'react'; +import { Redirect } from 'react-router-dom'; import { evaluateAccess } from './unified/utils/access-control'; const commonRoutes: RouteDescriptor[] = [ @@ -101,11 +100,10 @@ const unifiedRoutes: RouteDescriptor[] = [ }, { path: '/alerting/routes', - roles: () => - contextSrv.evaluatePermission(config.unifiedAlertingEnabled ? () => ['Editor', 'Admin'] : () => [], [ - AccessControlAction.AlertingNotificationsRead, - AccessControlAction.AlertingNotificationsExternalRead, - ]), + roles: evaluateAccess( + [AccessControlAction.AlertingNotificationsRead, AccessControlAction.AlertingNotificationsExternalRead], + [OrgRole.Editor, OrgRole.Admin] + ), component: SafeDynamicImport( () => import(/* webpackChunkName: "AlertAmRoutes" */ 'app/features/alerting/unified/AmRoutes') ), @@ -263,6 +261,10 @@ const unifiedRoutes: RouteDescriptor[] = [ { path: '/alerting/:sourceName/:name/find', pageClass: 'page-alerting', + roles: evaluateAccess( + [AccessControlAction.AlertingRuleRead, AccessControlAction.AlertingRuleExternalRead], + [OrgRole.Viewer, OrgRole.Editor, OrgRole.Admin] + ), component: SafeDynamicImport( () => import(/* webpackChunkName: "AlertingRedirectToRule"*/ 'app/features/alerting/unified/RedirectToRuleViewer') ), diff --git a/public/app/features/alerting/unified/RuleViewer.tsx b/public/app/features/alerting/unified/RuleViewer.tsx index 1b904541fdc..105e078bc4a 100644 --- a/public/app/features/alerting/unified/RuleViewer.tsx +++ b/public/app/features/alerting/unified/RuleViewer.tsx @@ -1,36 +1,37 @@ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { useObservable } from 'react-use'; import { css } from '@emotion/css'; import { GrafanaTheme2, LoadingState, PanelData } from '@grafana/data'; import { - withErrorBoundary, - useStyles2, Alert, + Button, + Icon, LoadingPlaceholder, PanelChromeLoadingIndicator, - Icon, - Button, + useStyles2, VerticalGroup, + withErrorBoundary, } from '@grafana/ui'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; -import { AlertingQueryRunner } from './state/AlertingQueryRunner'; -import { useCombinedRule } from './hooks/useCombinedRule'; -import { alertRuleToQueries } from './utils/query'; -import { RuleState } from './components/rules/RuleState'; -import { getRulesSourceByName } from './utils/datasource'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useObservable } from 'react-use'; +import { AlertQuery } from '../../../types/unified-alerting-dto'; +import { AlertLabels } from './components/AlertLabels'; import { DetailsField } from './components/DetailsField'; -import { RuleHealth } from './components/rules/RuleHealth'; +import { RuleViewerLayout, RuleViewerLayoutContent } from './components/rule-viewer/RuleViewerLayout'; import { RuleViewerVisualization } from './components/rule-viewer/RuleViewerVisualization'; import { RuleDetailsActionButtons } from './components/rules/RuleDetailsActionButtons'; -import { RuleDetailsMatchingInstances } from './components/rules/RuleDetailsMatchingInstances'; -import { RuleDetailsDataSources } from './components/rules/RuleDetailsDataSources'; -import { RuleViewerLayout, RuleViewerLayoutContent } from './components/rule-viewer/RuleViewerLayout'; -import { AlertLabels } from './components/AlertLabels'; -import { RuleDetailsExpression } from './components/rules/RuleDetailsExpression'; import { RuleDetailsAnnotations } from './components/rules/RuleDetailsAnnotations'; -import * as ruleId from './utils/rule-id'; -import { AlertQuery } from '../../../types/unified-alerting-dto'; +import { RuleDetailsDataSources } from './components/rules/RuleDetailsDataSources'; +import { RuleDetailsExpression } from './components/rules/RuleDetailsExpression'; import { RuleDetailsFederatedSources } from './components/rules/RuleDetailsFederatedSources'; +import { RuleDetailsMatchingInstances } from './components/rules/RuleDetailsMatchingInstances'; +import { RuleHealth } from './components/rules/RuleHealth'; +import { RuleState } from './components/rules/RuleState'; +import { useAlertQueriesStatus } from './hooks/useAlertQueriesStatus'; +import { useCombinedRule } from './hooks/useCombinedRule'; +import { AlertingQueryRunner } from './state/AlertingQueryRunner'; +import { getRulesSourceByName } from './utils/datasource'; +import { alertRuleToQueries } from './utils/query'; +import * as ruleId from './utils/rule-id'; import { isFederatedRuleGroup } from './utils/rules'; type RuleViewerProps = GrafanaRouteComponentProps<{ id?: string; sourceName?: string }>; @@ -49,19 +50,23 @@ export function RuleViewer({ match }: RuleViewerProps) { const queries2 = useMemo(() => alertRuleToQueries(rule), [rule]); const [queries, setQueries] = useState([]); + const { allDataSourcesAvailable } = useAlertQueriesStatus(queries2); + const onRunQueries = useCallback(() => { - if (queries.length > 0) { + if (queries.length > 0 && allDataSourcesAvailable) { runner.run(queries); } - }, [queries, runner]); + }, [queries, runner, allDataSourcesAvailable]); useEffect(() => { setQueries(queries2); }, [queries2]); useEffect(() => { - onRunQueries(); - }, [onRunQueries]); + if (allDataSourcesAvailable) { + onRunQueries(); + } + }, [onRunQueries, allDataSourcesAvailable]); useEffect(() => { return () => runner.destroy(); @@ -192,6 +197,11 @@ export function RuleViewer({ match }: RuleViewerProps) { )} + {!isFederatedRule && !allDataSourcesAvailable && ( + + Cannot display the query preview. Some of the data sources used in the queries are not available. + + )} ); } @@ -219,6 +229,9 @@ const getStyles = (theme: GrafanaTheme2) => { border-bottom: 1px solid ${theme.colors.border.medium}; padding: ${theme.spacing(2)}; `, + queryWarning: css` + margin: ${theme.spacing(4, 0)}; + `, details: css` display: flex; flex-direction: row; diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx new file mode 100644 index 00000000000..7eacd6a1fcb --- /dev/null +++ b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.test.tsx @@ -0,0 +1,75 @@ +import { render } from '@testing-library/react'; +import { contextSrv } from 'app/core/services/context_srv'; +import { configureStore } from 'app/store/configureStore'; +import { AccessControlAction } from 'app/types'; +import React from 'react'; +import { FormProvider, useForm } from 'react-hook-form'; +import { Provider } from 'react-redux'; +import { byText } from 'testing-library-selector'; +import { AlertTypeStep } from './AlertTypeStep'; + +const ui = { + ruleTypePicker: { + grafanaManagedButton: byText('Grafana managed alert'), + mimirOrLokiButton: byText('Mimir or Loki alert'), + mimirOrLokiRecordingButton: byText('Mimir or Loki recording rule'), + }, +}; + +const FormProviderWrapper: React.FC = ({ children }) => { + const methods = useForm({}); + return {children}; +}; + +function renderAlertTypeStep() { + const store = configureStore(); + + render( + + + , + { wrapper: FormProviderWrapper } + ); +} + +describe('RuleTypePicker', () => { + describe('FGAC', () => { + it('Should display grafana, mimir alert and mimir recording buttons when user has rule create and write permissions', async () => { + jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => { + return [AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite].includes( + action as AccessControlAction + ); + }); + + renderAlertTypeStep(); + + expect(ui.ruleTypePicker.grafanaManagedButton.get()).toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiButton.get()).toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.get()).toBeInTheDocument(); + }); + + it('Should hide grafana button when user does not have rule create permission', () => { + jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => { + return [AccessControlAction.AlertingRuleExternalWrite].includes(action as AccessControlAction); + }); + + renderAlertTypeStep(); + + expect(ui.ruleTypePicker.grafanaManagedButton.query()).not.toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiButton.get()).toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.get()).toBeInTheDocument(); + }); + + it('Should hide mimir alert and mimir recording when user does not have rule external write permission', () => { + jest.spyOn(contextSrv, 'hasPermission').mockImplementation((action) => { + return [AccessControlAction.AlertingRuleCreate].includes(action as AccessControlAction); + }); + + renderAlertTypeStep(); + + expect(ui.ruleTypePicker.grafanaManagedButton.get()).toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiButton.query()).not.toBeInTheDocument(); + expect(ui.ruleTypePicker.mimirOrLokiRecordingButton.query()).not.toBeInTheDocument(); + }); + }); +}); diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx index b06e7e2497c..c0ebdaa73bf 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx @@ -10,6 +10,8 @@ import { GroupAndNamespaceFields } from './GroupAndNamespaceFields'; import { CloudRulesSourcePicker } from './CloudRulesSourcePicker'; import { checkForPathSeparator } from './util'; import { RuleTypePicker } from './rule-types/RuleTypePicker'; +import { contextSrv } from 'app/core/services/context_srv'; +import { AccessControlAction } from 'app/types'; interface Props { editingExistingRule: boolean; @@ -24,6 +26,8 @@ const recordingRuleNameValidationPattern = { export const AlertTypeStep: FC = ({ editingExistingRule }) => { const styles = useStyles2(getStyles); + const { enabledRuleTypes, defaultRuleType } = getAvailableRuleTypes(); + const { register, control, @@ -44,8 +48,9 @@ export const AlertTypeStep: FC = ({ editingExistingRule }) => { render={({ field: { onChange } }) => ( )} name="type" @@ -140,6 +145,22 @@ export const AlertTypeStep: FC = ({ editingExistingRule }) => { ); }; +function getAvailableRuleTypes() { + const canCreateGrafanaRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleCreate); + const canCreateCloudRules = contextSrv.hasPermission(AccessControlAction.AlertingRuleExternalWrite); + const defaultRuleType = canCreateGrafanaRules ? RuleFormType.grafana : RuleFormType.cloudAlerting; + + const enabledRuleTypes: RuleFormType[] = []; + if (canCreateGrafanaRules) { + enabledRuleTypes.push(RuleFormType.grafana); + } + if (canCreateCloudRules) { + enabledRuleTypes.push(RuleFormType.cloudAlerting, RuleFormType.cloudRecording); + } + + return { enabledRuleTypes, defaultRuleType }; +} + const getStyles = (theme: GrafanaTheme2) => ({ formInput: css` width: 330px; diff --git a/public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx b/public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx index 9cac7d83d27..4b2bf6a9510 100644 --- a/public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx @@ -1,33 +1,44 @@ -import React, { useCallback, useState } from 'react'; import { css } from '@emotion/css'; -import { useFormContext } from 'react-hook-form'; -import { takeWhile } from 'rxjs/operators'; -import { useMountedState } from 'react-use'; -import { Button, HorizontalGroup, useStyles2 } from '@grafana/ui'; import { dateTimeFormatISO, GrafanaTheme2, LoadingState } from '@grafana/data'; -import { RuleFormType } from '../../types/rule-form'; -import { PreviewRuleRequest, PreviewRuleResponse } from '../../types/preview'; +import { Alert, Button, HorizontalGroup, useStyles2 } from '@grafana/ui'; +import React, { useCallback, useState } from 'react'; +import { useFormContext } from 'react-hook-form'; +import { useMountedState } from 'react-use'; +import { takeWhile } from 'rxjs/operators'; import { previewAlertRule } from '../../api/preview'; +import { useAlertQueriesStatus } from '../../hooks/useAlertQueriesStatus'; +import { PreviewRuleRequest, PreviewRuleResponse } from '../../types/preview'; +import { RuleFormType, RuleFormValues } from '../../types/rule-form'; import { PreviewRuleResult } from './PreviewRuleResult'; -const fields: string[] = ['type', 'dataSourceName', 'condition', 'queries', 'expression']; +const fields: Array = ['type', 'dataSourceName', 'condition', 'queries', 'expression']; export function PreviewRule(): React.ReactElement | null { const styles = useStyles2(getStyles); const [preview, onPreview] = usePreview(); - const { watch } = useFormContext(); - const [type, condition] = watch(['type', 'condition']); + const { watch } = useFormContext(); + const [type, condition, queries] = watch(['type', 'condition', 'queries']); + const { allDataSourcesAvailable } = useAlertQueriesStatus(queries); if (type === RuleFormType.cloudRecording || type === RuleFormType.cloudAlerting) { return null; } + const isPreviewAvailable = Boolean(condition) && allDataSourcesAvailable; + return (
- + {allDataSourcesAvailable && ( + + )} + {!allDataSourcesAvailable && ( + + Cannot display the query preview. Some of the data sources used in the queries are not available. + + )}
@@ -36,7 +47,7 @@ export function PreviewRule(): React.ReactElement | null { function usePreview(): [PreviewRuleResponse | undefined, () => void] { const [preview, setPreview] = useState(); - const { getValues } = useFormContext(); + const { getValues } = useFormContext(); const isMounted = useMountedState(); const onPreview = useCallback(() => { diff --git a/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx b/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx index 85e3365cd7f..e9dcc0c3450 100644 --- a/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/rule-types/RuleTypePicker.tsx @@ -1,3 +1,6 @@ +import { css } from '@emotion/css'; +import { GrafanaTheme2 } from '@grafana/data/src'; +import { Stack } from '@grafana/experimental'; import { useStyles2 } from '@grafana/ui'; import { isEmpty } from 'lodash'; import React, { FC } from 'react'; @@ -6,16 +9,14 @@ import { RuleFormType } from '../../../types/rule-form'; import { GrafanaManagedRuleType } from './GrafanaManagedAlert'; import { MimirFlavoredType } from './MimirOrLokiAlert'; import { RecordingRuleType } from './MimirOrLokiRecordingRule'; -import { css } from '@emotion/css'; -import { GrafanaTheme2 } from '@grafana/data/src'; -import { Stack } from '@grafana/experimental'; interface RuleTypePickerProps { onChange: (value: RuleFormType) => void; selected: RuleFormType; + enabledTypes: RuleFormType[]; } -const RuleTypePicker: FC = ({ selected, onChange }) => { +const RuleTypePicker: FC = ({ selected, onChange, enabledTypes }) => { const rulesSourcesWithRuler = useRulesSourcesWithRuler(); const hasLotexDatasources = !isEmpty(rulesSourcesWithRuler); @@ -24,22 +25,30 @@ const RuleTypePicker: FC = ({ selected, onChange }) => { return ( <> - - - + {enabledTypes.includes(RuleFormType.grafana) && ( + + )} + {enabledTypes.includes(RuleFormType.cloudAlerting) && ( + + )} + {enabledTypes.includes(RuleFormType.cloudRecording) && ( + + )} - - Select “Grafana managed” unless you have a Mimir, Loki or Cortex data source with the Ruler API - enabled. - + {enabledTypes.includes(RuleFormType.grafana) && ( + + Select “Grafana managed” unless you have a Mimir, Loki or Cortex data source with the Ruler API + enabled. + + )} ); }; diff --git a/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx b/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx index 4c78bd50443..6e5c6147bf0 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx @@ -28,13 +28,11 @@ const ui = { jest.spyOn(contextSrv, 'accessControlEnabled').mockReturnValue(true); describe('RuleDetails FGAC', () => { - mocks.useIsRuleEditable.mockReturnValue({ loading: false, isEditable: true }); - describe('Grafana rules action buttons', () => { const grafanaRule = getGrafanaRule({ name: 'Grafana' }); it('Should not render Edit button for users without the update permission', () => { // Arrange - jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isEditable: false }); // Act renderRuleDetails(grafanaRule); @@ -45,7 +43,7 @@ describe('RuleDetails FGAC', () => { it('Should not render Delete button for users without the delete permission', () => { // Arrange - jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isRemovable: false }); // Act renderRuleDetails(grafanaRule); @@ -56,9 +54,7 @@ describe('RuleDetails FGAC', () => { it('Should render Edit button for users with the update permission', () => { // Arrange - jest - .spyOn(contextSrv, 'hasPermission') - .mockImplementation((action) => action === AccessControlAction.AlertingRuleUpdate); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isEditable: true }); // Act renderRuleDetails(grafanaRule); @@ -69,9 +65,7 @@ describe('RuleDetails FGAC', () => { it('Should render Delete button for users with the delete permission', () => { // Arrange - jest - .spyOn(contextSrv, 'hasPermission') - .mockImplementation((action) => action === AccessControlAction.AlertingRuleDelete); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isRemovable: true }); // Act renderRuleDetails(grafanaRule); @@ -109,7 +103,7 @@ describe('RuleDetails FGAC', () => { const cloudRule = getCloudRule({ name: 'Cloud' }); it('Should not render Edit button for users without the update permission', () => { // Arrange - jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isEditable: false }); // Act renderRuleDetails(cloudRule); @@ -120,7 +114,7 @@ describe('RuleDetails FGAC', () => { it('Should not render Delete button for users without the delete permission', () => { // Arrange - jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isRemovable: false }); // Act renderRuleDetails(cloudRule); @@ -131,9 +125,7 @@ describe('RuleDetails FGAC', () => { it('Should render Edit button for users with the update permission', () => { // Arrange - jest - .spyOn(contextSrv, 'hasPermission') - .mockImplementation((action) => action === AccessControlAction.AlertingRuleExternalWrite); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isEditable: true }); // Act renderRuleDetails(cloudRule); @@ -144,9 +136,7 @@ describe('RuleDetails FGAC', () => { it('Should render Delete button for users with the delete permission', () => { // Arrange - jest - .spyOn(contextSrv, 'hasPermission') - .mockImplementation((action) => action === AccessControlAction.AlertingRuleExternalWrite); + mocks.useIsRuleEditable.mockReturnValue({ loading: false, isRemovable: true }); // Act renderRuleDetails(cloudRule); diff --git a/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx b/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx index 7f26d28ba87..43604a68733 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx @@ -4,7 +4,6 @@ import { config } from '@grafana/runtime'; import { Button, ClipboardButton, ConfirmModal, HorizontalGroup, LinkButton, useStyles2 } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; import { contextSrv } from 'app/core/services/context_srv'; -import { getRulesPermissions } from 'app/features/alerting/unified/utils/access-control'; import { AccessControlAction } from 'app/types'; import { CombinedRule, RulesSource } from 'app/types/unified-alerting'; import { RulerGrafanaRuleDTO, RulerRuleDTO } from 'app/types/unified-alerting-dto'; @@ -41,16 +40,13 @@ export const RuleDetailsActionButtons: FC = ({ rule, rulesSource }) => { : getAlertmanagerByUid(rulesSource.jsonData.alertmanagerUid)?.name; const rulesSourceName = getRulesSourceName(rulesSource); - const rulesPermissions = getRulesPermissions(rulesSourceName); - const hasEditPermission = contextSrv.hasPermission(rulesPermissions.update); - const hasDeletePermission = contextSrv.hasPermission(rulesPermissions.delete); const hasExplorePermission = contextSrv.hasPermission(AccessControlAction.DataSourcesExplore); const leftButtons: JSX.Element[] = []; const rightButtons: JSX.Element[] = []; const isFederated = isFederatedRuleGroup(group); - const { isEditable } = useIsRuleEditable(rulesSourceName, rulerRule); + const { isEditable, isRemovable } = useIsRuleEditable(rulesSourceName, rulerRule); const returnTo = location.pathname + location.search; const isViewMode = inViewMode(location.pathname); @@ -187,7 +183,6 @@ export const RuleDetailsActionButtons: FC = ({ rule, rulesSource }) => { ); } - // TODO Maybe there is a way to unify isEditable with FGAC permissions if (isEditable && rulerRule && !isFederated) { const sourceName = getRulesSourceName(rulesSource); const identifier = ruleId.fromRulerRule(sourceName, namespace.name, group.name, rulerRule); @@ -218,29 +213,29 @@ export const RuleDetailsActionButtons: FC = ({ rule, rulesSource }) => { ); } - if (hasEditPermission) { - rightButtons.push( - - Edit - - ); - } - if (hasDeletePermission) { - rightButtons.push( - - ); - } + rightButtons.push( + + Edit + + ); } + + if (isRemovable && rulerRule && !isFederated) { + rightButtons.push( + + ); + } + if (leftButtons.length || rightButtons.length) { return ( <> diff --git a/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx new file mode 100644 index 00000000000..e7d2192e5d8 --- /dev/null +++ b/public/app/features/alerting/unified/components/rules/RuleListGroupView.test.tsx @@ -0,0 +1,108 @@ +import { render } from '@testing-library/react'; +import { contextSrv } from 'app/core/services/context_srv'; +import { configureStore } from 'app/store/configureStore'; +import { AccessControlAction } from 'app/types'; +import { CombinedRuleNamespace } from 'app/types/unified-alerting'; +import React from 'react'; +import { Provider } from 'react-redux'; +import { byRole } from 'testing-library-selector'; +import { mockCombinedRule, mockDataSource } from '../../mocks'; +import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; +import { RuleListGroupView } from './RuleListGroupView'; + +const ui = { + grafanaRulesHeading: byRole('heading', { name: 'Grafana' }), + cloudRulesHeading: byRole('heading', { name: 'Mimir / Cortex / Loki' }), +}; + +describe('RuleListGroupView', () => { + describe('FGAC', () => { + jest.spyOn(contextSrv, 'accessControlEnabled').mockReturnValue(true); + + it('Should display Grafana rules when the user has the alert rule read permission', () => { + const grafanaNamespace = getGrafanaNamespace(); + const namespaces: CombinedRuleNamespace[] = [grafanaNamespace]; + + jest + .spyOn(contextSrv, 'hasPermission') + .mockImplementation((action) => action === AccessControlAction.AlertingRuleRead); + + renderRuleList(namespaces); + + expect(ui.grafanaRulesHeading.get()).toBeInTheDocument(); + }); + + it('Should display Cloud rules when the user has the external rules read permission', () => { + const cloudNamespace = getCloudNamespace(); + const namespaces: CombinedRuleNamespace[] = [cloudNamespace]; + + jest + .spyOn(contextSrv, 'hasPermission') + .mockImplementation((action) => action === AccessControlAction.AlertingRuleExternalRead); + + renderRuleList(namespaces); + + expect(ui.cloudRulesHeading.get()).toBeInTheDocument(); + }); + + it('Should not display Grafana rules when the user does not have alert rule read permission', () => { + const grafanaNamespace = getGrafanaNamespace(); + const namespaces: CombinedRuleNamespace[] = [grafanaNamespace]; + + jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + + renderRuleList(namespaces); + + expect(ui.grafanaRulesHeading.query()).not.toBeInTheDocument(); + }); + + it('Should not display Cloud rules when the user does not have the external rules read permission', () => { + const cloudNamespace = getCloudNamespace(); + + const namespaces: CombinedRuleNamespace[] = [cloudNamespace]; + renderRuleList(namespaces); + + jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + + renderRuleList(namespaces); + + expect(ui.cloudRulesHeading.query()).not.toBeInTheDocument(); + }); + }); +}); + +function renderRuleList(namespaces: CombinedRuleNamespace[]) { + const store = configureStore(); + + render( + + + + ); +} + +function getGrafanaNamespace(): CombinedRuleNamespace { + return { + name: 'Grafana Test Namespace', + rulesSource: GRAFANA_RULES_SOURCE_NAME, + groups: [ + { + name: 'default', + rules: [mockCombinedRule()], + }, + ], + }; +} + +function getCloudNamespace(): CombinedRuleNamespace { + return { + name: 'Cloud Test Namespace', + rulesSource: mockDataSource(), + groups: [ + { + name: 'Prom group', + rules: [mockCombinedRule()], + }, + ], + }; +} diff --git a/public/app/features/alerting/unified/components/rules/RuleListGroupView.tsx b/public/app/features/alerting/unified/components/rules/RuleListGroupView.tsx index e2a51b2b390..9bb64492da7 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListGroupView.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListGroupView.tsx @@ -1,6 +1,8 @@ +import { AccessControlAction } from 'app/types'; import { CombinedRuleNamespace } from 'app/types/unified-alerting'; import React, { FC, useMemo } from 'react'; import { isCloudRulesSource, isGrafanaRulesSource } from '../../utils/datasource'; +import { Authorize } from '../Authorize'; import { CloudRules } from './CloudRules'; import { GrafanaRules } from './GrafanaRules'; @@ -25,8 +27,12 @@ export const RuleListGroupView: FC = ({ namespaces, expandAll }) => { return ( <> - - + + + + + + ); }; diff --git a/public/app/features/alerting/unified/hooks/useAlertQueriesStatus.ts b/public/app/features/alerting/unified/hooks/useAlertQueriesStatus.ts new file mode 100644 index 00000000000..b5e3c61eb05 --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useAlertQueriesStatus.ts @@ -0,0 +1,12 @@ +import { getDataSourceSrv } from '@grafana/runtime'; +import { AlertQuery } from 'app/types/unified-alerting-dto'; +import { useMemo } from 'react'; + +export function useAlertQueriesStatus(queries: AlertQuery[]) { + const allDataSourcesAvailable = useMemo( + () => queries.every((query) => Boolean(getDataSourceSrv().getInstanceSettings(query.datasourceUid))), + [queries] + ); + + return { allDataSourcesAvailable }; +} diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx new file mode 100644 index 00000000000..7925df6f9d6 --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -0,0 +1,156 @@ +import React from 'react'; +import { renderHook } from '@testing-library/react-hooks'; +import { contextSrv } from 'app/core/services/context_srv'; +import { configureStore } from 'app/store/configureStore'; +import { AccessControlAction, FolderDTO, StoreState } from 'app/types'; +import { Provider } from 'react-redux'; +import { mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule } from '../mocks'; +import { useFolder } from './useFolder'; +import { useIsRuleEditable } from './useIsRuleEditable'; +import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector'; + +jest.mock('./useFolder'); + +const mocks = { + useFolder: jest.mocked(useFolder), + useUnifiedAlertingSelector: jest.mocked(useUnifiedAlertingSelector), +}; + +describe('useIsRuleEditable', () => { + describe('FGAC enabled', () => { + jest.spyOn(contextSrv, 'accessControlEnabled').mockReturnValue(true); + describe('Grafana rules', () => { + it('Should allow editing when the user has the alert rule update permission and folder permissions', () => { + mockPermissions([AccessControlAction.AlertingRuleUpdate]); + mockUseFolder({ canSave: true }); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(true); + }); + + it('Should allow deleting when the user has the alert rule delete permission and folder permissions', () => { + mockPermissions([AccessControlAction.AlertingRuleDelete]); + mockUseFolder({ canSave: true }); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isRemovable).toBe(true); + }); + + it('Should forbid editing when the user has no alert rule update permission and has folder permissions', () => { + mockPermissions([]); + mockUseFolder({ canSave: true }); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(false); + }); + + it('Should forbid deleting when the user has no alert rule delete permission and has folder permissions', () => { + mockPermissions([]); + mockUseFolder({ canSave: true }); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isRemovable).toBe(false); + }); + + it('Should forbid editing and deleting when the user has aler rule permissions but does not have folder permissions', () => { + mockPermissions([AccessControlAction.AlertingRuleUpdate, AccessControlAction.AlertingRuleDelete]); + mockUseFolder({ canSave: false }); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(false); + expect(result.current.isRemovable).toBe(false); + }); + }); + + describe('Cloud rules', () => { + it('Should allow editing and deleting when the user has alert rule external write permission', () => { + mockPermissions([AccessControlAction.AlertingRuleExternalWrite]); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('cortex', mockRulerAlertingRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(true); + expect(result.current.isRemovable).toBe(true); + }); + + it('Should forbid editing and deleting when the user has no alert rule external write permission', () => { + mockPermissions([]); + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('cortex', mockRulerAlertingRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(false); + expect(result.current.isRemovable).toBe(false); + }); + }); + }); +}); + +function mockUseFolder(partial?: Partial) { + mocks.useFolder.mockReturnValue({ loading: false, folder: mockFolder(partial) }); +} + +function mockPermissions(grantedPermissions: AccessControlAction[]) { + jest + .spyOn(contextSrv, 'hasPermission') + .mockImplementation((action) => grantedPermissions.includes(action as AccessControlAction)); +} + +function getProviderWrapper() { + const dataSources = getMockedDataSources(); + const store = mockStore({ dataSources }); + const wrapper: React.FC = ({ children }) => {children}; + return wrapper; +} + +function getMockedDataSources(): StoreState['unifiedAlerting']['dataSources'] { + return { + grafana: { + loading: false, + dispatched: false, + result: { + id: 'grafana', + name: 'grafana', + rulerConfig: { dataSourceName: 'grafana', apiVersion: 'legacy' }, + }, + }, + cortex: { + loading: false, + dispatched: false, + result: { + id: 'cortex', + name: 'Cortex', + rulerConfig: { dataSourceName: 'cortex', apiVersion: 'legacy' }, + }, + }, + }; +} + +function mockStore(unifiedAlerting?: Partial) { + const defaultState = configureStore().getState(); + + return configureStore({ + ...defaultState, + unifiedAlerting: { + ...defaultState.unifiedAlerting, + ...unifiedAlerting, + }, + }); +} diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index 97695319adf..5d42819b433 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -1,11 +1,13 @@ import { contextSrv } from 'app/core/services/context_srv'; import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; +import { getRulesPermissions } from '../utils/access-control'; import { isGrafanaRulerRule } from '../utils/rules'; import { useFolder } from './useFolder'; import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector'; interface ResultBag { isEditable?: boolean; + isRemovable?: boolean; loading: boolean; } @@ -13,10 +15,14 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): const dataSources = useUnifiedAlertingSelector((state) => state.dataSources); const folderUID = rule && isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; + const rulePermission = getRulesPermissions(rulesSourceName); + const hasEditPermission = contextSrv.hasAccess(rulePermission.update, contextSrv.isEditor); + const hasRemovePermission = contextSrv.hasAccess(rulePermission.delete, contextSrv.isEditor); + const { folder, loading } = useFolder(folderUID); if (!rule) { - return { isEditable: false, loading: false }; + return { isEditable: false, isRemovable: false, loading: false }; } // grafana rules can be edited if user can edit the folder they're in @@ -27,14 +33,17 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): ); } return { - isEditable: folder?.canSave, + isEditable: hasEditPermission && folder?.canSave, + isRemovable: hasRemovePermission && folder?.canSave, loading, }; } // prom rules are only editable by users with Editor role and only if rules source supports editing + const isRulerAvailable = Boolean(dataSources[rulesSourceName]?.result?.rulerConfig); return { - isEditable: contextSrv.isEditor && Boolean(dataSources[rulesSourceName]?.result?.rulerConfig), + isEditable: hasEditPermission && isRulerAvailable, + isRemovable: hasRemovePermission && isRulerAvailable, loading: dataSources[rulesSourceName]?.loading, }; } diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index eca52ac0931..952037e6f44 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -29,6 +29,7 @@ import { Silence, SilenceState, } from 'app/plugins/datasource/alertmanager/types'; +import { FolderDTO } from 'app/types'; let nextDataSourceId = 1; @@ -449,3 +450,18 @@ export const mockCombinedRule = (partial?: Partial): CombinedRule rulerRule: mockRulerAlertingRule(), ...partial, }); + +export const mockFolder = (partial?: Partial): FolderDTO => { + return { + id: 1, + uid: 'gdev-1', + title: 'Gdev', + version: 1, + url: '', + canAdmin: true, + canDelete: true, + canEdit: true, + canSave: true, + ...partial, + }; +};