From 41b175e7ae0bdaa95038c7aa203e4a00cff66c7c Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 2 Oct 2024 16:56:35 +0200 Subject: [PATCH] Alerting: Use useProduceNewAlertmanagerConfiguration for contact points (#88456) --- .../contact-points/ContactPoint.tsx | 9 +- .../contact-points/ContactPointHeader.tsx | 6 +- .../contact-points/ContactPoints.test.tsx | 14 +- .../contact-points/ContactPoints.tsx | 10 +- ...eContactPoints.tsx => useContactPoints.ts} | 160 ++++++++---------- .../receivers/form/CloudReceiverForm.tsx | 13 +- .../form/GrafanaReceiverForm.test.tsx | 22 ++- .../receivers/form/GrafanaReceiverForm.tsx | 31 +++- .../unified/hooks/mergeRequestStates.tsx | 19 --- .../hooks/useProduceNewAlertmanagerConfig.ts | 3 +- .../app/features/alerting/unified/mockApi.ts | 2 +- .../__snapshots__/receivers.test.ts.snap | 99 +++++++++++ .../reducers/alertmanager/muteTimings.test.ts | 10 -- .../reducers/alertmanager/muteTimings.ts | 3 - .../reducers/alertmanager/receivers.test.ts | 138 +++++++++++++++ .../reducers/alertmanager/receivers.ts | 74 ++++++++ .../alerting/unified/state/actions.ts | 30 ---- .../unified/utils/notification-policies.ts | 25 ++- .../alerting/unified/utils/receiver-form.ts | 59 ------- 19 files changed, 461 insertions(+), 266 deletions(-) rename public/app/features/alerting/unified/components/contact-points/{useContactPoints.tsx => useContactPoints.ts} (80%) delete mode 100644 public/app/features/alerting/unified/hooks/mergeRequestStates.tsx create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/receivers.test.ts.snap create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/receivers.test.ts create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/receivers.ts diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx index b53b0ee7032..2c249b30171 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx @@ -22,16 +22,15 @@ import { RECEIVER_META_KEY, RECEIVER_PLUGIN_META_KEY, RECEIVER_STATUS_KEY } from import { ContactPointWithMetadata, getReceiverDescription, ReceiverConfigWithMetadata } from './utils'; interface ContactPointProps { - disabled?: boolean; contactPoint: ContactPointWithMetadata; } -export const ContactPoint = ({ disabled = false, contactPoint }: ContactPointProps) => { +export const ContactPoint = ({ contactPoint }: ContactPointProps) => { const { grafana_managed_receiver_configs: receivers } = contactPoint; const styles = useStyles2(getStyles); const { selectedAlertmanager } = useAlertmanager(); - const handleDelete = useDeleteContactPoint({ alertmanager: selectedAlertmanager! }); - const [DeleteModal, showDeleteModal] = useDeleteContactPointModal(handleDelete); + const [deleteTrigger] = useDeleteContactPoint({ alertmanager: selectedAlertmanager! }); + const [DeleteModal, showDeleteModal] = useDeleteContactPointModal(deleteTrigger.execute); // TODO probably not the best way to figure out if we want to show either only the summary or full metadata for the receivers? const showFullMetadata = receivers.some((receiver) => Boolean(receiver[RECEIVER_META_KEY])); @@ -41,7 +40,6 @@ export const ContactPoint = ({ disabled = false, contactPoint }: ContactPointPro showDeleteModal({ name: contactPointToDelete.id || contactPointToDelete.name, @@ -64,6 +62,7 @@ export const ContactPoint = ({ disabled = false, contactPoint }: ContactPointPro const sendingResolved = !Boolean(receiver.disableResolveMessage); const pluginMetadata = receiver[RECEIVER_PLUGIN_META_KEY]; const key = metadata.name + index; + return ( void; } -export const ContactPointHeader = ({ contactPoint, disabled = false, onDelete }: ContactPointHeaderProps) => { +export const ContactPointHeader = ({ contactPoint, onDelete }: ContactPointHeaderProps) => { const { name, id, provisioned, policies = [] } = contactPoint; const styles = useStyles2(getStyles); const [showPermissionsDrawer, setShowPermissionsDrawer] = useState(false); @@ -160,7 +159,7 @@ export const ContactPointHeader = ({ contactPoint, disabled = false, onDelete }: ariaLabel="delete" icon="trash-alt" destructive - disabled={disabled || !canBeDeleted} + disabled={!canBeDeleted} onClick={() => onDelete(contactPoint)} /> @@ -219,7 +218,6 @@ export const ContactPointHeader = ({ contactPoint, disabled = false, onDelete }: size="sm" icon={canEdit ? 'pen' : 'eye'} type="button" - disabled={disabled} aria-label={`${canEdit ? 'edit' : 'view'}-action`} data-testid={`${canEdit ? 'edit' : 'view'}-action`} href={`/alerting/notifications/receivers/${encodeURIComponent(urlId)}/edit`} diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx index ddfb8a8cf18..cf5417fc4ac 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx @@ -4,9 +4,9 @@ import { render, screen, userEvent, waitFor, waitForElementToBeRemoved, within } import { selectors } from '@grafana/e2e-selectors'; import { + flushMicrotasks, testWithFeatureToggles, testWithLicenseFeatures, - flushMicrotasks, } from 'app/features/alerting/unified/test/test-utils'; import { K8sAnnotations } from 'app/features/alerting/unified/utils/k8s/constants'; import { AlertManagerDataSourceJsonData, AlertManagerImplementation } from 'app/plugins/datasource/alertmanager/types'; @@ -45,7 +45,7 @@ import { ContactPointWithMetadata, ReceiverConfigWithMetadata, RouteReference } */ const server = setupMswServer(); -const renderWithProvider = ( +export const renderWithProvider = ( children: ReactNode, historyOptions?: MemoryHistoryBuildOptions, providerProps?: Partial> @@ -213,16 +213,6 @@ describe('contact points', () => { expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); - it('should disable edit button', async () => { - renderWithProvider(); - - const moreActions = screen.getByRole('button', { name: /More/ }); - expect(moreActions).toBeEnabled(); - - const editAction = screen.getByTestId('edit-action'); - expect(editAction).toHaveAttribute('aria-disabled', 'true'); - }); - it('should show warning when no receivers are configured', async () => { renderWithProvider(); diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx index 2e8a57588aa..17c4f58cd9b 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoints.tsx @@ -224,16 +224,10 @@ const ContactPointsPageContents = () => { interface ContactPointsListProps { contactPoints: ContactPointWithMetadata[]; search?: string | null; - disabled?: boolean; pageSize?: number; } -const ContactPointsList = ({ - contactPoints, - disabled = false, - search, - pageSize = DEFAULT_PAGE_SIZE, -}: ContactPointsListProps) => { +const ContactPointsList = ({ contactPoints, search, pageSize = DEFAULT_PAGE_SIZE }: ContactPointsListProps) => { const searchResults = useContactPointsSearch(contactPoints, search); const { page, pageItems, numberOfPages, onPageChange } = usePagination(searchResults, 1, pageSize); @@ -241,7 +235,7 @@ const ContactPointsList = ({ <> {pageItems.map((contactPoint, index) => { const key = `${contactPoint.name}-${index}`; - return ; + return ; })} diff --git a/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx b/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts similarity index 80% rename from public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx rename to public/app/features/alerting/unified/components/contact-points/useContactPoints.ts index 82e68c3324d..96e4b9462a2 100644 --- a/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx +++ b/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts @@ -3,18 +3,12 @@ * and (if available) it will also fetch the status from the Grafana Managed status endpoint */ -import { produce } from 'immer'; -import { merge, remove, set } from 'lodash'; +import { merge, set } from 'lodash'; import { useMemo } from 'react'; -import { alertingApi } from 'app/features/alerting/unified/api/alertingApi'; import { receiversApi } from 'app/features/alerting/unified/api/receiversK8sApi'; import { useOnCallIntegration } from 'app/features/alerting/unified/components/receivers/grafanaAppReceivers/onCall/useOnCallIntegration'; -import { - ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1Receiver, - generatedReceiversApi, -} from 'app/features/alerting/unified/openapi/receiversApi.gen'; -import { updateAlertManagerConfigAction } from 'app/features/alerting/unified/state/actions'; +import { ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1Receiver } from 'app/features/alerting/unified/openapi/receiversApi.gen'; import { BaseAlertmanagerArgs, Skippable } from 'app/features/alerting/unified/types/hooks'; import { cloudNotifierTypes } from 'app/features/alerting/unified/utils/cloud-alertmanager-notifier-types'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; @@ -23,17 +17,18 @@ import { isK8sEntityProvisioned, shouldUseK8sApi, } from 'app/features/alerting/unified/utils/k8s/utils'; -import { updateConfigWithReceiver } from 'app/features/alerting/unified/utils/receiver-form'; import { GrafanaManagedContactPoint, GrafanaManagedReceiverConfig, Receiver, } from 'app/plugins/datasource/alertmanager/types'; -import { useDispatch } from 'app/types'; import { alertmanagerApi } from '../../api/alertmanagerApi'; import { onCallApi } from '../../api/onCallApi'; +import { useAsync } from '../../hooks/useAsync'; import { usePluginBridge } from '../../hooks/usePluginBridge'; +import { useProduceNewAlertmanagerConfiguration } from '../../hooks/useProduceNewAlertmanagerConfig'; +import { addReceiverAction, deleteReceiverAction, updateReceiverAction } from '../../reducers/alertmanager/receivers'; import { SupportedPlugin } from '../../types/pluginBridges'; import { enhanceContactPointsWithMetadata } from './utils'; @@ -54,7 +49,6 @@ const { useGetContactPointsStatusQuery, useGrafanaNotifiersQuery, useLazyGetAlertmanagerConfigurationQuery, - useUpdateAlertmanagerConfigurationMutation, } = alertmanagerApi; const { useGrafanaOnCallIntegrationsQuery } = onCallApi; const { @@ -311,34 +305,28 @@ export function useContactPointsWithStatus({ return isGrafanaAlertmanager ? grafanaResponse : alertmanagerConfigResponse; } +type DeleteContactPointArgs = { name: string; resourceVersion?: string }; export function useDeleteContactPoint({ alertmanager }: BaseAlertmanagerArgs) { - const [fetchAlertmanagerConfig] = useLazyGetAlertmanagerConfigurationQuery(); - const [updateAlertManager] = useUpdateAlertmanagerConfigurationMutation(); - const [deleteReceiver] = useDeleteNamespacedReceiverMutation(); - const useK8sApi = shouldUseK8sApi(alertmanager); - return async ({ name, resourceVersion }: { name: string; resourceVersion?: string }) => { - if (useK8sApi) { - const namespace = getK8sNamespace(); - return deleteReceiver({ - name, - namespace, - ioK8SApimachineryPkgApisMetaV1DeleteOptions: { preconditions: { resourceVersion } }, - }).unwrap(); - } - const config = await fetchAlertmanagerConfig(alertmanager).unwrap(); + const [produceNewAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); + const [deleteReceiver] = useDeleteNamespacedReceiverMutation(); - const newConfig = produce(config, (draft) => { - remove(draft?.alertmanager_config?.receivers ?? [], (receiver) => receiver.name === name); - return draft; - }); - - return updateAlertManager({ - selectedAlertmanager: alertmanager, - config: newConfig, + const deleteFromK8sAPI = useAsync(async ({ name, resourceVersion }: DeleteContactPointArgs) => { + const namespace = getK8sNamespace(); + await deleteReceiver({ + name, + namespace, + ioK8SApimachineryPkgApisMetaV1DeleteOptions: { preconditions: { resourceVersion } }, }).unwrap(); - }; + }); + + const deleteFromAlertmanagerConfiguration = useAsync(async ({ name }: DeleteContactPointArgs) => { + const action = deleteReceiverAction(name); + return produceNewAlertmanagerConfiguration(action); + }); + + return useK8sApi ? deleteFromK8sAPI : deleteFromAlertmanagerConfiguration; } /** @@ -404,75 +392,69 @@ type ContactPointOperationArgs = { }; type CreateContactPointArgs = ContactPointOperationArgs; -type UpdateContactPointArgs = ContactPointOperationArgs & { - /** ID of existing contact point to update - used when updating via k8s API */ - id?: string; - resourceVersion?: string; - /** Name of the existing contact point - used for checking uniqueness of name when not using k8s API*/ - originalName?: string; -}; export const useCreateContactPoint = ({ alertmanager }: BaseAlertmanagerArgs) => { const isGrafanaAlertmanager = alertmanager === GRAFANA_RULES_SOURCE_NAME; const useK8sApi = shouldUseK8sApi(alertmanager); const { createOnCallIntegrations } = useOnCallIntegration(); - const [getAlertmanagerConfig] = useLazyGetAlertmanagerConfigurationQuery(); const [createGrafanaContactPoint] = useCreateNamespacedReceiverMutation(); - const dispatch = useDispatch(); + const [produceNewAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); - return async ({ contactPoint }: CreateContactPointArgs) => { - const receiverWithPotentialOnCall = isGrafanaAlertmanager + const updateK8sAPI = useAsync(async ({ contactPoint }: CreateContactPointArgs) => { + const contactPointWithMaybeOnCall = isGrafanaAlertmanager ? await createOnCallIntegrations(contactPoint) : contactPoint; - if (useK8sApi) { - const namespace = getK8sNamespace(); + const namespace = getK8sNamespace(); + const contactPointToUse = grafanaContactPointToK8sReceiver(contactPointWithMaybeOnCall); - const contactPointToUse = grafanaContactPointToK8sReceiver(receiverWithPotentialOnCall); + return createGrafanaContactPoint({ + namespace, + comGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1Receiver: contactPointToUse, + }).unwrap(); + }); - return createGrafanaContactPoint({ - namespace, - comGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1Receiver: contactPointToUse, - }).unwrap(); - } + const updateAlertmanagerConfiguration = useAsync(async ({ contactPoint }: CreateContactPointArgs) => { + const contactPointWithMaybeOnCall = isGrafanaAlertmanager + ? await createOnCallIntegrations(contactPoint) + : contactPoint; - const config = await getAlertmanagerConfig(alertmanager).unwrap(); - const newConfig = updateConfigWithReceiver(config, receiverWithPotentialOnCall); + const action = addReceiverAction(contactPointWithMaybeOnCall); + return produceNewAlertmanagerConfiguration(action); + }); - return await dispatch( - updateAlertManagerConfigAction({ - newConfig: newConfig, - oldConfig: config, - alertManagerSourceName: alertmanager, - }) - ) - .unwrap() - .then(() => { - dispatch(alertingApi.util.invalidateTags(['AlertmanagerConfiguration', 'ContactPoint', 'ContactPointsStatus'])); - dispatch(generatedReceiversApi.util.invalidateTags(['Receiver'])); - }); - }; + return useK8sApi ? updateK8sAPI : updateAlertmanagerConfiguration; }; +type UpdateContactPointArgsK8s = ContactPointOperationArgs & { + /** ID of existing contact point to update - used when updating via k8s API */ + id: string; + resourceVersion?: string; +}; +type UpdateContactPointArgsConfig = ContactPointOperationArgs & { + /** Name of the existing contact point - used for checking uniqueness of name when not using k8s API*/ + originalName: string; +}; +type UpdateContactpointArgs = UpdateContactPointArgsK8s | UpdateContactPointArgsConfig; + export const useUpdateContactPoint = ({ alertmanager }: BaseAlertmanagerArgs) => { const isGrafanaAlertmanager = alertmanager === GRAFANA_RULES_SOURCE_NAME; const useK8sApi = shouldUseK8sApi(alertmanager); const { createOnCallIntegrations } = useOnCallIntegration(); - const [getAlertmanagerConfig] = useLazyGetAlertmanagerConfigurationQuery(); const [replaceGrafanaContactPoint] = useReplaceNamespacedReceiverMutation(); + const [produceNewAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); - const dispatch = useDispatch(); + const updateContactPoint = useAsync(async (args: UpdateContactpointArgs) => { + if ('resourceVersion' in args && useK8sApi) { + const { contactPoint, id, resourceVersion } = args; - return async ({ contactPoint, id, originalName, resourceVersion }: UpdateContactPointArgs) => { - const receiverWithPotentialOnCall = isGrafanaAlertmanager - ? await createOnCallIntegrations(contactPoint) - : contactPoint; + const receiverWithPotentialOnCall = isGrafanaAlertmanager + ? await createOnCallIntegrations(contactPoint) + : contactPoint; - if (useK8sApi && id) { const namespace = getK8sNamespace(); - const contactPointToUse = grafanaContactPointToK8sReceiver(receiverWithPotentialOnCall, id, resourceVersion); return replaceGrafanaContactPoint({ @@ -480,24 +462,18 @@ export const useUpdateContactPoint = ({ alertmanager }: BaseAlertmanagerArgs) => namespace, comGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1Receiver: contactPointToUse, }).unwrap(); + } else if ('originalName' in args) { + const { contactPoint, originalName } = args; + const receiverWithPotentialOnCall = isGrafanaAlertmanager + ? await createOnCallIntegrations(contactPoint) + : contactPoint; + + const action = updateReceiverAction({ name: originalName, receiver: receiverWithPotentialOnCall }); + return produceNewAlertmanagerConfiguration(action); } + }); - const config = await getAlertmanagerConfig(alertmanager).unwrap(); - const newConfig = updateConfigWithReceiver(config, receiverWithPotentialOnCall, originalName); - - return dispatch( - updateAlertManagerConfigAction({ - newConfig: newConfig, - oldConfig: config, - alertManagerSourceName: alertmanager, - }) - ) - .unwrap() - .then(() => { - dispatch(alertingApi.util.invalidateTags(['AlertmanagerConfiguration', 'ContactPoint', 'ContactPointsStatus'])); - dispatch(generatedReceiversApi.util.invalidateTags(['Receiver'])); - }); - }; + return updateContactPoint; }; export const useValidateContactPoint = ({ alertmanager }: BaseAlertmanagerArgs) => { diff --git a/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx index 2809617aac3..88e1442f94b 100644 --- a/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/CloudReceiverForm.tsx @@ -9,7 +9,7 @@ import { } from 'app/features/alerting/unified/components/contact-points/useContactPoints'; import { Receiver } from 'app/plugins/datasource/alertmanager/types'; -import { CloudChannelValues, ReceiverFormValues, CloudChannelMap } from '../../../types/receiver-form'; +import { CloudChannelMap, CloudChannelValues, ReceiverFormValues } from '../../../types/receiver-form'; import { cloudNotifierTypes } from '../../../utils/cloud-alertmanager-notifier-types'; import { isVanillaPrometheusAlertManagerDataSource } from '../../../utils/datasource'; import { cloudReceiverToFormValues, formValuesToCloudReceiver } from '../../../utils/receiver-form'; @@ -41,8 +41,8 @@ export const CloudReceiverForm = ({ contactPoint, alertManagerSourceName, readOn const { isLoading, data: config } = useGetAlertmanagerConfigurationQuery(alertManagerSourceName); const isVanillaAM = isVanillaPrometheusAlertManagerDataSource(alertManagerSourceName); - const createContactPoint = useCreateContactPoint({ alertmanager: alertManagerSourceName }); - const updateContactPoint = useUpdateContactPoint({ alertmanager: alertManagerSourceName }); + const [createContactPoint] = useCreateContactPoint({ alertmanager: alertManagerSourceName }); + const [updateContactPoint] = useUpdateContactPoint({ alertmanager: alertManagerSourceName }); // transform receiver DTO to form values const [existingValue] = useMemo((): [ReceiverFormValues | undefined, CloudChannelMap] => { @@ -54,11 +54,12 @@ export const CloudReceiverForm = ({ contactPoint, alertManagerSourceName, readOn const onSubmit = async (values: ReceiverFormValues) => { const newReceiver = formValuesToCloudReceiver(values, defaultChannelValues); + try { - if (editMode) { - await updateContactPoint({ contactPoint: newReceiver, originalName: contactPoint!.name }); + if (editMode && contactPoint) { + await updateContactPoint.execute({ contactPoint: newReceiver, originalName: contactPoint.name }); } else { - await createContactPoint({ contactPoint: newReceiver }); + await createContactPoint.execute({ contactPoint: newReceiver }); } locationService.push('/alerting/notifications'); } catch (error) { diff --git a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx index 23e2d50466f..b7c6c9517be 100644 --- a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.test.tsx @@ -1,18 +1,21 @@ import { clickSelectOption } from 'test/helpers/selectOptionInTest'; -import { render, waitFor, screen } from 'test/test-utils'; +import { screen, waitFor } from 'test/test-utils'; import { byLabelText, byRole, byTestId, byText } from 'testing-library-selector'; import { config } from '@grafana/runtime'; import { disablePlugin } from 'app/features/alerting/unified/mocks/server/configure'; -import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { setOnCallFeatures, setOnCallIntegrations, } from 'app/features/alerting/unified/mocks/server/handlers/plugins/configure-plugins'; import { SupportedPlugin } from 'app/features/alerting/unified/types/pluginBridges'; import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types'; +import { AccessControlAction } from 'app/types'; import { AlertmanagerConfigBuilder, setupMswServer } from '../../../mockApi'; +import { grantUserPermissions } from '../../../mocks'; +import { captureRequests } from '../../../mocks/server/events'; +import { renderWithProvider } from '../../contact-points/ContactPoints.test'; import { GrafanaReceiverForm } from './GrafanaReceiverForm'; @@ -35,6 +38,13 @@ const ui = { }; describe('GrafanaReceiverForm', () => { + beforeEach(() => { + grantUserPermissions([ + AccessControlAction.AlertingNotificationsRead, + AccessControlAction.AlertingNotificationsWrite, + ]); + }); + describe('alertingApiServer', () => { beforeEach(() => { config.featureToggles.alertingApiServer = true; @@ -47,7 +57,7 @@ describe('GrafanaReceiverForm', () => { const capturedRequests = captureRequests( (req) => req.url.includes('/v0alpha1/namespaces/default/receivers') && req.method === 'POST' ); - const { user } = render(); + const { user } = renderWithProvider(); const { type, click } = user; await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); @@ -84,7 +94,7 @@ describe('GrafanaReceiverForm', () => { it('OnCall contact point should be disabled if OnCall integration is not enabled', async () => { disablePlugin(SupportedPlugin.OnCall); - render(); + renderWithProvider(); await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); @@ -104,7 +114,7 @@ describe('GrafanaReceiverForm', () => { { display_name: 'apac-oncall', value: 'apac-oncall', integration_url: 'https://apac.oncall.example.com' }, ]); - const { user } = render(); + const { user } = renderWithProvider(); await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); @@ -156,7 +166,7 @@ describe('GrafanaReceiverForm', () => { ) ); - render(); + renderWithProvider(); await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); diff --git a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx index 00ef22ca8fc..108d188e629 100644 --- a/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/GrafanaReceiverForm.tsx @@ -19,6 +19,7 @@ import { useDispatch } from 'app/types'; import { alertmanagerApi } from '../../../api/alertmanagerApi'; import { testReceiversAction } from '../../../state/actions'; import { GrafanaChannelValues, ReceiverFormValues } from '../../../types/receiver-form'; +import { shouldUseK8sApi } from '../../../utils/k8s/utils'; import { formChannelValuesToGrafanaChannelConfig, formValuesToGrafanaReceiver, @@ -52,8 +53,13 @@ const { useGrafanaNotifiersQuery } = alertmanagerApi; export const GrafanaReceiverForm = ({ contactPoint, readOnly = false, editMode }: Props) => { const dispatch = useDispatch(); - const createContactPoint = useCreateContactPoint({ alertmanager: GRAFANA_RULES_SOURCE_NAME }); - const updateContactPoint = useUpdateContactPoint({ alertmanager: GRAFANA_RULES_SOURCE_NAME }); + const useK8sAPI = shouldUseK8sApi(GRAFANA_RULES_SOURCE_NAME); + const [createContactPoint] = useCreateContactPoint({ + alertmanager: GRAFANA_RULES_SOURCE_NAME, + }); + const [updateContactPoint] = useUpdateContactPoint({ + alertmanager: GRAFANA_RULES_SOURCE_NAME, + }); const { onCallNotifierMeta, @@ -82,16 +88,23 @@ export const GrafanaReceiverForm = ({ contactPoint, readOnly = false, editMode } const onSubmit = async (values: ReceiverFormValues) => { const newReceiver = formValuesToGrafanaReceiver(values, id2original, defaultChannelValues, grafanaNotifiers); + try { if (editMode) { - await updateContactPoint({ - contactPoint: newReceiver, - id: contactPoint!.id, - resourceVersion: contactPoint?.metadata?.resourceVersion, - originalName: contactPoint?.name, - }); + if (useK8sAPI && contactPoint && contactPoint.id) { + await updateContactPoint.execute({ + contactPoint: newReceiver, + id: contactPoint.id, + resourceVersion: contactPoint?.metadata?.resourceVersion, + }); + } else if (contactPoint) { + await updateContactPoint.execute({ + contactPoint: newReceiver, + originalName: contactPoint.name, + }); + } } else { - await createContactPoint({ contactPoint: newReceiver }); + await createContactPoint.execute({ contactPoint: newReceiver }); } locationService.push('/alerting/notifications'); } catch (error) { diff --git a/public/app/features/alerting/unified/hooks/mergeRequestStates.tsx b/public/app/features/alerting/unified/hooks/mergeRequestStates.tsx deleted file mode 100644 index f57f6b1d415..00000000000 --- a/public/app/features/alerting/unified/hooks/mergeRequestStates.tsx +++ /dev/null @@ -1,19 +0,0 @@ -interface RequestState { - error?: unknown; - - isUninitialized: boolean; - isSuccess: boolean; - isError: boolean; - isLoading: boolean; -} - -// @TODO what to do with the other props that we get from RTKQ's state such as originalArgs, etc? -export function mergeRequestStates(...states: RequestState[]): RequestState { - return { - error: states.find((s) => s.error), - isUninitialized: states.every((s) => s.isUninitialized), - isSuccess: states.every((s) => s.isSuccess), - isError: states.some((s) => s.isError), - isLoading: states.some((s) => s.isLoading), - }; -} diff --git a/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts b/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts index 9a9147473d1..d5a7ceb9562 100644 --- a/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts +++ b/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts @@ -5,6 +5,7 @@ import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/ty import { alertmanagerApi } from '../api/alertmanagerApi'; import { muteTimingsReducer } from '../reducers/alertmanager/muteTimings'; +import { receiversReducer } from '../reducers/alertmanager/receivers'; import { useAlertmanager } from '../state/AlertmanagerContext'; import { mergeRequestStates } from './mergeRequestStates'; @@ -25,7 +26,7 @@ export const initialAlertmanagerConfiguration: AlertManagerCortexConfig = { template_files: {}, }; -const configurationReducer = reduceReducers(initialAlertmanagerConfiguration, muteTimingsReducer); +const configurationReducer = reduceReducers(initialAlertmanagerConfiguration, muteTimingsReducer, receiversReducer); /** * This hook will make sure we are always applying actions that mutate the Alertmanager configuration diff --git a/public/app/features/alerting/unified/mockApi.ts b/public/app/features/alerting/unified/mockApi.ts index 00b76cddd80..6fb410301d3 100644 --- a/public/app/features/alerting/unified/mockApi.ts +++ b/public/app/features/alerting/unified/mockApi.ts @@ -129,7 +129,7 @@ class GrafanaReceiverConfigBuilder { } } -class AlertmanagerReceiverBuilder { +export class AlertmanagerReceiverBuilder { private receiver: AlertmanagerReceiver = { name: '', email_configs: [], grafana_managed_receiver_configs: [] }; withName(name: string): AlertmanagerReceiverBuilder { diff --git a/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/receivers.test.ts.snap b/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/receivers.test.ts.snap new file mode 100644 index 00000000000..d181614c0f2 --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/receivers.test.ts.snap @@ -0,0 +1,99 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`receivers adding receivers should be able to add a new Alertmanager receiver 1`] = ` +{ + "alertmanager_config": { + "receivers": [ + { + "email_configs": [ + { + "to": "address@domain.com", + }, + ], + "grafana_managed_receiver_configs": [], + "name": "new contact point", + }, + ], + }, + "template_files": {}, +} +`; + +exports[`receivers adding receivers should be able to add a new Grafana Alertmanager receiver 1`] = ` +{ + "alertmanager_config": { + "receivers": [ + { + "email_configs": [], + "grafana_managed_receiver_configs": [ + { + "disableResolveMessage": false, + "name": "emea-oncall", + "settings": { + "url": "https://oncall.example.com", + }, + "type": "oncall", + }, + ], + "name": "another contact point", + }, + ], + }, + "template_files": {}, +} +`; + +exports[`receivers should delete a receiver 1`] = ` +{ + "alertmanager_config": { + "receivers": [ + { + "name": "another receiver", + }, + ], + }, + "template_files": {}, +} +`; + +exports[`receivers updating receivers should allow renaming a receiver and update routes 1`] = ` +{ + "alertmanager_config": { + "receivers": [ + { + "email_configs": [], + "grafana_managed_receiver_configs": [], + "name": "receiver 2", + }, + ], + "route": { + "receiver": "receiver 2", + "routes": [ + { + "receiver": "receiver 2", + }, + ], + }, + }, + "template_files": {}, +} +`; + +exports[`receivers updating receivers should allow updating an existing receiver 1`] = ` +{ + "alertmanager_config": { + "receivers": [ + { + "email_configs": [ + { + "to": "address+1@domain.com", + }, + ], + "grafana_managed_receiver_configs": [], + "name": "existing receiver", + }, + ], + }, + "template_files": {}, +} +`; diff --git a/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.test.ts b/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.test.ts index d5e7d4a0d0d..679af78cd32 100644 --- a/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.test.ts +++ b/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.test.ts @@ -1,5 +1,3 @@ -import { UnknownAction } from 'redux'; - import { AlertManagerCortexConfig, MuteTimeInterval } from 'app/plugins/datasource/alertmanager/types'; import { addMuteTimingAction, deleteMuteTimingAction, muteTimingsReducer, updateMuteTimingAction } from './muteTimings'; @@ -54,14 +52,6 @@ describe('mute timings', () => { const updateMuteTiming = updateMuteTimingAction({ originalName: muteTimingName, interval: newMuteTiming }); expect(muteTimingsReducer(initialConfig, updateMuteTiming)).toMatchSnapshot(); }); - - it('should throw for unknown action', () => { - const action: UnknownAction = { type: 'unknown' }; - - expect(() => { - muteTimingsReducer(initialConfig, action); - }).toThrow('unknown'); - }); }); function mockTimeInterval(overrides: Partial = {}): MuteTimeInterval { diff --git a/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.ts b/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.ts index 1427452bdde..14c90b52116 100644 --- a/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.ts +++ b/public/app/features/alerting/unified/reducers/alertmanager/muteTimings.ts @@ -68,8 +68,5 @@ export const muteTimingsReducer = createReducer(initialState, (builder) => { // remove the mute timing from all routes alertmanager_config.route = removeTimeIntervalFromRoute(name, alertmanager_config.route ?? {}); - }) - .addDefaultCase((_state, action) => { - throw new Error(`Unknown action for mute timing reducer: ${action.type}`); }); }); diff --git a/public/app/features/alerting/unified/reducers/alertmanager/receivers.test.ts b/public/app/features/alerting/unified/reducers/alertmanager/receivers.test.ts new file mode 100644 index 00000000000..a7991660e38 --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/receivers.test.ts @@ -0,0 +1,138 @@ +import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types'; + +import { AlertmanagerReceiverBuilder } from '../../mockApi'; + +import { addReceiverAction, deleteReceiverAction, receiversReducer, updateReceiverAction } from './receivers'; + +describe('receivers', () => { + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: {}, + template_files: {}, + }; + + it('should delete a receiver', () => { + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'my receiver' }, { name: 'another receiver' }], + }, + template_files: {}, + }; + + const action = deleteReceiverAction('my receiver'); + expect(receiversReducer(config, action)).toMatchSnapshot(); + }); + + describe('adding receivers', () => { + it('should be able to add a new Alertmanager receiver', () => { + const newReceiver = new AlertmanagerReceiverBuilder() + .withName('new contact point') + .addEmailConfig((b) => b.withTo('address@domain.com')) + .build(); + + const action = addReceiverAction(newReceiver); + expect(receiversReducer(initialConfig, action)).toMatchSnapshot(); + }); + + it('should be able to add a new Grafana Alertmanager receiver', () => { + const newReceiver = new AlertmanagerReceiverBuilder() + .withName('another contact point') + .addGrafanaReceiverConfig((receiver) => + receiver.withType('oncall').withName('emea-oncall').addSetting('url', 'https://oncall.example.com') + ) + .build(); + + const action = addReceiverAction(newReceiver); + expect(receiversReducer(initialConfig, action)).toMatchSnapshot(); + }); + + it('should throw if we add a receiver with duplicate name', () => { + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'my receiver' }], + }, + template_files: {}, + }; + + const newReceiver = new AlertmanagerReceiverBuilder().withName('my receiver').build(); + const action = addReceiverAction(newReceiver); + + expect(() => { + receiversReducer(config, action); + }).toThrow(/duplicate receiver/i); + }); + }); + + describe('updating receivers', () => { + it('should throw if updating a receiver that does not exist', () => { + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'my receiver' }], + }, + template_files: {}, + }; + + const updatedReceiver = new AlertmanagerReceiverBuilder().withName('my receiver').build(); + const action = updateReceiverAction({ name: 'does not exist', receiver: updatedReceiver }); + + expect(() => { + receiversReducer(config, action); + }).toThrow(/expected receiver .+ to exist/i); + }); + + it('should throw if renaming a receiver to an existing name', () => { + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'receiver 1' }, { name: 'receiver 2' }], + }, + template_files: {}, + }; + + const updatedReceiver = new AlertmanagerReceiverBuilder().withName('receiver 1').build(); + const action = updateReceiverAction({ name: 'receiver 2', receiver: updatedReceiver }); + + expect(() => { + receiversReducer(config, action); + }).toThrow(/duplicate receiver name/i); + }); + + it('should allow renaming a receiver and update routes', () => { + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'receiver 1' }], + route: { + receiver: 'receiver 1', + routes: [{ receiver: 'receiver 1' }], + }, + }, + template_files: {}, + }; + + const updatedReceiver = new AlertmanagerReceiverBuilder().withName('receiver 2').build(); + const action = updateReceiverAction({ name: 'receiver 1', receiver: updatedReceiver }); + + expect(receiversReducer(config, action)).toMatchSnapshot(); + }); + + it('should allow updating an existing receiver', () => { + const existingReceiver = new AlertmanagerReceiverBuilder() + .withName('existing receiver') + .addEmailConfig((build) => build.withTo('address@domain.com')) + .build(); + + const config: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [existingReceiver], + }, + template_files: {}, + }; + + const updatedReceiver = new AlertmanagerReceiverBuilder() + .withName('existing receiver') + .addEmailConfig((build) => build.withTo('address+1@domain.com')) + .build(); + const action = updateReceiverAction({ name: 'existing receiver', receiver: updatedReceiver }); + + expect(receiversReducer(config, action)).toMatchSnapshot(); + }); + }); +}); diff --git a/public/app/features/alerting/unified/reducers/alertmanager/receivers.ts b/public/app/features/alerting/unified/reducers/alertmanager/receivers.ts new file mode 100644 index 00000000000..308cc909a60 --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/receivers.ts @@ -0,0 +1,74 @@ +import { createAction, createReducer } from '@reduxjs/toolkit'; +import { remove } from 'lodash'; + +import { AlertManagerCortexConfig, Receiver } from 'app/plugins/datasource/alertmanager/types'; + +import { renameReceiverInRoute } from '../../utils/notification-policies'; + +export const addReceiverAction = createAction('receiver/add'); +export const updateReceiverAction = createAction<{ name: string; receiver: Receiver }>('receiver/update'); +export const deleteReceiverAction = createAction('receiver/delete'); + +const initialState: AlertManagerCortexConfig = { + alertmanager_config: {}, + template_files: {}, +}; + +/** + * This reducer will manage action related to receiver (Contact points) and make sure all operations on the alertmanager + * configuration happen immutably and only mutate what they need. + */ +export const receiversReducer = createReducer(initialState, (builder) => { + builder + // add a new receiver + .addCase(addReceiverAction, (draft, { payload: newReceiver }) => { + // ensure the receivers are always an array + const currentReceivers = (draft.alertmanager_config.receivers = draft.alertmanager_config.receivers ?? []); + + // check if the name doesn't already exist + const nameExists = currentReceivers.some((receiver) => receiver.name === newReceiver.name); + if (nameExists) { + throw new Error(`Duplicate receiver name ${newReceiver.name}`); + } + + // add the receiver + currentReceivers.push(newReceiver); + }) + // upate an existing receiver + .addCase(updateReceiverAction, (draft, { payload }) => { + const { name, receiver } = payload; + const renaming = name !== receiver.name; + + const receivers = draft.alertmanager_config.receivers ?? []; + + const targetIndex = receivers.findIndex((receiver) => receiver.name === name); + const targetExists = targetIndex > -1; + + // check if the receiver we want to update exists + if (!targetExists) { + throw new Error(`Expected receiver ${name} to exist, but did not find it in the config`); + } + + // check if the new name doesn't already exist + if (renaming) { + const nameExists = receivers.some((oldReceiver) => oldReceiver.name === receiver.name); + if (nameExists) { + throw new Error(`Duplicate receiver name ${receiver.name}`); + } + } + + // overwrite the receiver with the new one + receivers[targetIndex] = receiver; + + // check if we need to update routes if the contact point was renamed + const routeTree = draft.alertmanager_config.route; + + if (routeTree && renaming) { + draft.alertmanager_config.route = renameReceiverInRoute(routeTree, name, receiver.name); + } + }) + // delete a receiver from the alertmanager configuration + .addCase(deleteReceiverAction, (draft, { payload: name }) => { + remove(draft.alertmanager_config.receivers ?? [], (receiver) => receiver.name === name); + }); +}); diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index 77565cfa437..78ad5a54e2f 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -342,36 +342,6 @@ export const updateAlertManagerConfigAction = createAsyncThunk => { - return async (dispatch) => { - const config = await dispatch( - alertmanagerApi.endpoints.getAlertmanagerConfiguration.initiate(alertManagerSourceName) - ).unwrap(); - - if (!config) { - throw new Error(`Config for ${alertManagerSourceName} not found`); - } - if (!config.alertmanager_config.receivers?.find((receiver) => receiver.name === receiverName)) { - throw new Error(`Cannot delete receiver ${receiverName}: not found in config.`); - } - const newConfig: AlertManagerCortexConfig = { - ...config, - alertmanager_config: { - ...config.alertmanager_config, - receivers: config.alertmanager_config.receivers.filter((receiver) => receiver.name !== receiverName), - }, - }; - return dispatch( - updateAlertManagerConfigAction({ - newConfig, - oldConfig: config, - alertManagerSourceName, - successMessage: 'Contact point deleted.', - }) - ); - }; -}; - export const fetchFolderAction = createAsyncThunk( 'unifiedalerting/fetchFolder', (uid: string): Promise => withSerializedError(backendSrv.getFolderByUid(uid, { withAccessControl: true })) diff --git a/public/app/features/alerting/unified/utils/notification-policies.ts b/public/app/features/alerting/unified/utils/notification-policies.ts index 685172a7b7e..6f2d6969215 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -281,4 +281,27 @@ function isLabelMatch(matcher: ObjectMatcher, label: Label): boolean { return matchFunction(labelValue, matcherValue); } -export { findMatchingAlertGroups, findMatchingRoutes, getInheritedProperties, isLabelMatchInSet }; +// recursive function to rename receivers in all routes (notification policies) +function renameReceiverInRoute(route: Route, oldName: string, newName: string) { + const updated: Route = { + ...route, + }; + + if (updated.receiver === oldName) { + updated.receiver = newName; + } + + if (updated.routes) { + updated.routes = updated.routes.map((route) => renameReceiverInRoute(route, oldName, newName)); + } + + return updated; +} + +export { + findMatchingAlertGroups, + findMatchingRoutes, + getInheritedProperties, + isLabelMatchInSet, + renameReceiverInRoute, +}; diff --git a/public/app/features/alerting/unified/utils/receiver-form.ts b/public/app/features/alerting/unified/utils/receiver-form.ts index 0f0a0a9b2d4..4ecfcd05144 100644 --- a/public/app/features/alerting/unified/utils/receiver-form.ts +++ b/public/app/features/alerting/unified/utils/receiver-form.ts @@ -1,12 +1,10 @@ import { get, has, isArray, isNil, omit, omitBy, reduce } from 'lodash'; import { - AlertManagerCortexConfig, AlertmanagerReceiver, GrafanaManagedContactPoint, GrafanaManagedReceiverConfig, Receiver, - Route, } from 'app/plugins/datasource/alertmanager/types'; import { CloudNotifierType, NotificationChannelOption, NotifierDTO, NotifierType } from 'app/types'; @@ -121,63 +119,6 @@ export function formValuesToCloudReceiver( return recv; } -// will add new receiver, or replace exisitng one -export function updateConfigWithReceiver( - config: AlertManagerCortexConfig, - receiver: Receiver, - existingReceiverName?: string -): AlertManagerCortexConfig { - const existingReceivers = config.alertmanager_config.receivers ?? []; - - const receiverWasRenamed = existingReceiverName && receiver.name !== existingReceiverName; - - // sanity check that name is not duplicated - if (!existingReceiverName && !!existingReceivers.find(({ name }) => name === receiver.name)) { - throw new Error(`Duplicate receiver name ${receiver.name}`); - } - - // sanity check that existing receiver exists - if (existingReceiverName && !existingReceivers.find(({ name }) => name === existingReceiverName)) { - throw new Error(`Expected receiver ${existingReceiverName} to exist, but did not find it in the config`); - } - - const updated: AlertManagerCortexConfig = { - ...config, - alertmanager_config: { - ...config.alertmanager_config, - receivers: existingReceiverName - ? existingReceivers.map((existingReceiver) => - existingReceiver.name === existingReceiverName ? receiver : existingReceiver - ) - : [...existingReceivers, receiver], - }, - }; - - // if receiver was renamed, rename it in routes as well - if (updated.alertmanager_config.route && receiverWasRenamed) { - updated.alertmanager_config.route = renameReceiverInRoute( - updated.alertmanager_config.route, - existingReceiverName, - receiver.name - ); - } - - return updated; -} - -function renameReceiverInRoute(route: Route, oldName: string, newName: string) { - const updated: Route = { - ...route, - }; - if (updated.receiver === oldName) { - updated.receiver = newName; - } - if (updated.routes) { - updated.routes = updated.routes.map((route) => renameReceiverInRoute(route, oldName, newName)); - } - return updated; -} - function cloudChannelConfigToFormChannelValues( id: string, type: CloudNotifierType,