From f0c57622f86c88bc9c8fe556fd150cde45c4cd77 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Wed, 13 Nov 2024 11:46:10 +0100 Subject: [PATCH] Alerting: useProduceNewAlertmanagerConfiguration for notification templates (#95290) --- .../useNotificationTemplates.ts | 117 +++++------------ .../components/receivers/TemplateForm.tsx | 16 +-- .../components/receivers/TemplatesTable.tsx | 4 +- .../hooks/useProduceNewAlertmanagerConfig.ts | 8 +- .../notificationTemplates.test.ts.snap | 55 ++++++++ .../notificationTemplates.test.ts | 119 ++++++++++++++++++ .../alertmanager/notificationTemplates.ts | 83 ++++++++++++ 7 files changed, 304 insertions(+), 98 deletions(-) create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/notificationTemplates.test.ts.snap create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.test.ts create mode 100644 public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.ts diff --git a/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts b/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts index a640e936dcf..160a28001a6 100644 --- a/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts +++ b/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts @@ -1,17 +1,20 @@ -import { produce } from 'immer'; import { useEffect } from 'react'; import { Validate } from 'react-hook-form'; -import { useDispatch } from 'app/types'; - import { AlertManagerCortexConfig } from '../../../../../plugins/datasource/alertmanager/types'; import { alertmanagerApi } from '../../api/alertmanagerApi'; import { templatesApi } from '../../api/templateApi'; +import { useAsync } from '../../hooks/useAsync'; +import { useProduceNewAlertmanagerConfiguration } from '../../hooks/useProduceNewAlertmanagerConfig'; import { ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TemplateGroup, ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TemplateGroupList, } from '../../openapi/templatesApi.gen'; -import { updateAlertManagerConfigAction } from '../../state/actions'; +import { + addNotificationTemplateAction, + deleteNotificationTemplateAction, + updateNotificationTemplateAction, +} from '../../reducers/alertmanager/notificationTemplates'; import { K8sAnnotations, PROVENANCE_NONE } from '../../utils/k8s/constants'; import { getAnnotation, getK8sNamespace, shouldUseK8sApi } from '../../utils/k8s/utils'; import { ensureDefine } from '../../utils/templates'; @@ -30,6 +33,7 @@ export interface NotificationTemplate { } const { useGetAlertmanagerConfigurationQuery, useLazyGetAlertmanagerConfigurationQuery } = alertmanagerApi; + const { useListNamespacedTemplateGroupQuery, useLazyReadNamespacedTemplateGroupQuery, @@ -146,39 +150,17 @@ interface CreateTemplateParams { } export function useCreateNotificationTemplate({ alertmanager }: BaseAlertmanagerArgs) { - const dispatch = useDispatch(); - - const [fetchAmConfig] = useLazyGetAlertmanagerConfigurationQuery(); const [createNamespacedTemplateGroup] = useCreateNamespacedTemplateGroupMutation(); + const [updateAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); const k8sApiSupported = shouldUseK8sApi(alertmanager); - async function createUsingConfigFileApi({ templateValues }: CreateTemplateParams) { - const amConfig = await fetchAmConfig(alertmanager).unwrap(); - // wrap content in "define" if it's not already wrapped, in case user did not do it/ - // it's not obvious that this is needed for template to work - const content = ensureDefine(templateValues.title, templateValues.content); + const createUsingConfigFileApi = useAsync(({ templateValues }: CreateTemplateParams) => { + const action = addNotificationTemplateAction({ template: templateValues }); + return updateAlertmanagerConfiguration(action); + }); - const targetTemplateExists = amConfig.template_files?.[templateValues.title] !== undefined; - if (targetTemplateExists) { - throw new Error('target template already exists'); - } - - const updatedConfig = produce(amConfig, (draft) => { - draft.template_files[templateValues.title] = content; - draft.alertmanager_config.templates = [...(draft.alertmanager_config.templates ?? []), templateValues.title]; - }); - - return dispatch( - updateAlertManagerConfigAction({ - alertManagerSourceName: alertmanager, - newConfig: updatedConfig, - oldConfig: amConfig, - }) - ).unwrap(); - } - - async function createUsingK8sApi({ templateValues }: CreateTemplateParams) { + const createUsingK8sApi = useAsync(({ templateValues }: CreateTemplateParams) => { const content = ensureDefine(templateValues.title, templateValues.content); return createNamespacedTemplateGroup({ @@ -188,7 +170,7 @@ export function useCreateNotificationTemplate({ alertmanager }: BaseAlertmanager metadata: {}, }, }).unwrap(); - } + }); return k8sApiSupported ? createUsingK8sApi : createUsingConfigFileApi; } @@ -199,43 +181,17 @@ interface UpdateTemplateParams { } export function useUpdateNotificationTemplate({ alertmanager }: BaseAlertmanagerArgs) { - const dispatch = useDispatch(); - - const [fetchAmConfig] = useLazyGetAlertmanagerConfigurationQuery(); const [replaceNamespacedTemplateGroup] = useReplaceNamespacedTemplateGroupMutation(); + const [updateAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); const k8sApiSupported = shouldUseK8sApi(alertmanager); - async function updateUsingConfigFileApi({ template, patch }: UpdateTemplateParams) { - const amConfig = await fetchAmConfig(alertmanager).unwrap(); - // wrap content in "define" if it's not already wrapped, in case user did not do it/ - // it's not obvious that this is needed for template to work - const content = ensureDefine(patch.title, patch.content); + const updateUsingConfigFileApi = useAsync(({ template, patch }: UpdateTemplateParams) => { + const action = updateNotificationTemplateAction({ name: template.title, template: patch }); + return updateAlertmanagerConfiguration(action); + }); - const originalName = template.title; // For ConfigFile API name is the same as uid - const nameChanged = originalName !== patch.title; - - // TODO Maybe we could simplify or extract this logic - const updatedConfig = produce(amConfig, (draft) => { - if (nameChanged) { - delete draft.template_files[originalName]; - draft.alertmanager_config.templates = draft.alertmanager_config.templates?.filter((t) => t !== originalName); - } - - draft.template_files[patch.title] = content; - draft.alertmanager_config.templates = [...(draft.alertmanager_config.templates ?? []), patch.title]; - }); - - return dispatch( - updateAlertManagerConfigAction({ - alertManagerSourceName: alertmanager, - newConfig: updatedConfig, - oldConfig: amConfig, - }) - ).unwrap(); - } - - async function updateUsingK8sApi({ template, patch }: UpdateTemplateParams) { + const updateUsingK8sApi = useAsync(({ template, patch }: UpdateTemplateParams) => { const content = ensureDefine(patch.title, patch.content); return replaceNamespacedTemplateGroup({ @@ -246,43 +202,30 @@ export function useUpdateNotificationTemplate({ alertmanager }: BaseAlertmanager metadata: { name: template.uid }, }, }).unwrap(); - } + }); return k8sApiSupported ? updateUsingK8sApi : updateUsingConfigFileApi; } export function useDeleteNotificationTemplate({ alertmanager }: BaseAlertmanagerArgs) { - const dispatch = useDispatch(); - const [fetchAmConfig] = useLazyGetAlertmanagerConfigurationQuery(); const [deleteNamespacedTemplateGroup] = useDeleteNamespacedTemplateGroupMutation(); + const [updateAlertmanagerConfiguration] = useProduceNewAlertmanagerConfiguration(); - async function deleteUsingConfigFileApi({ uid }: { uid: string }) { - const amConfig = await fetchAmConfig(alertmanager).unwrap(); + const deleteUsingConfigAPI = useAsync(async ({ uid }: { uid: string }) => { + const action = deleteNotificationTemplateAction({ name: uid }); + return updateAlertmanagerConfiguration(action); + }); - const updatedConfig = produce(amConfig, (draft) => { - delete draft.template_files[uid]; - draft.alertmanager_config.templates = draft.alertmanager_config.templates?.filter((t) => t !== uid); - }); - - return dispatch( - updateAlertManagerConfigAction({ - alertManagerSourceName: alertmanager, - newConfig: updatedConfig, - oldConfig: amConfig, - }) - ).unwrap(); - } - - async function deleteUsingK8sApi({ uid }: { uid: string }) { + const deleteUsingK8sApi = useAsync(({ uid }: { uid: string }) => { return deleteNamespacedTemplateGroup({ namespace: getK8sNamespace(), name: uid, ioK8SApimachineryPkgApisMetaV1DeleteOptions: {}, }).unwrap(); - } + }); const k8sApiSupported = shouldUseK8sApi(alertmanager); - return k8sApiSupported ? deleteUsingK8sApi : deleteUsingConfigFileApi; + return k8sApiSupported ? deleteUsingK8sApi : deleteUsingConfigAPI; } interface ValidateNotificationTemplateParams { diff --git a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx index 205c49fcc5f..ad2c8bd2f71 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx @@ -10,18 +10,18 @@ import { GrafanaTheme2 } from '@grafana/data'; import { isFetchError, locationService } from '@grafana/runtime'; import { Alert, + Box, Button, + Drawer, Dropdown, FieldSet, + InlineField, Input, LinkButton, Menu, - useStyles2, Stack, useSplitter, - Drawer, - InlineField, - Box, + useStyles2, } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; import { useCleanup } from 'app/core/hooks/useCleanup'; @@ -93,8 +93,8 @@ export const TemplateForm = ({ originalTemplate, prefill, alertmanager }: Props) const appNotification = useAppNotification(); - const createNewTemplate = useCreateNotificationTemplate({ alertmanager }); - const updateTemplate = useUpdateNotificationTemplate({ alertmanager }); + const [createNewTemplate] = useCreateNotificationTemplate({ alertmanager }); + const [updateTemplate] = useUpdateNotificationTemplate({ alertmanager }); const { titleIsUnique } = useValidateNotificationTemplate({ alertmanager, originalTemplate }); useCleanup((state) => (state.unifiedAlerting.saveAMConfig = initialAsyncRequestState)); @@ -149,9 +149,9 @@ export const TemplateForm = ({ originalTemplate, prefill, alertmanager }: Props) try { if (!originalTemplate) { - await createNewTemplate({ templateValues: values }); + await createNewTemplate.execute({ templateValues: values }); } else { - await updateTemplate({ template: originalTemplate, patch: values }); + await updateTemplate.execute({ template: originalTemplate, patch: values }); } appNotification.success('Template saved', `Template ${values.title} has been saved`); locationService.push(returnLink); diff --git a/public/app/features/alerting/unified/components/receivers/TemplatesTable.tsx b/public/app/features/alerting/unified/components/receivers/TemplatesTable.tsx index 7debc6c7f16..62087390139 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplatesTable.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplatesTable.tsx @@ -30,7 +30,7 @@ interface Props { export const TemplatesTable = ({ alertManagerName, templates }: Props) => { const appNotification = useAppNotification(); - const deleteTemplate = useDeleteNotificationTemplate({ alertmanager: alertManagerName }); + const [deleteTemplate] = useDeleteNotificationTemplate({ alertmanager: alertManagerName }); const tableStyles = useStyles2(getAlertTableStyles); @@ -39,7 +39,7 @@ export const TemplatesTable = ({ alertManagerName, templates }: Props) => { const onDeleteTemplate = async () => { if (templateToDelete) { try { - await deleteTemplate({ uid: templateToDelete.uid }); + await deleteTemplate.execute({ uid: templateToDelete.uid }); appNotification.success('Template deleted', `Template ${templateToDelete.title} has been deleted`); } catch (error) { appNotification.error('Error deleting template', `Error deleting template ${templateToDelete.title}`); diff --git a/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts b/public/app/features/alerting/unified/hooks/useProduceNewAlertmanagerConfig.ts index d5a7ceb9562..b91e809b512 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 { notificationTemplatesReducer } from '../reducers/alertmanager/notificationTemplates'; import { receiversReducer } from '../reducers/alertmanager/receivers'; import { useAlertmanager } from '../state/AlertmanagerContext'; @@ -26,7 +27,12 @@ export const initialAlertmanagerConfiguration: AlertManagerCortexConfig = { template_files: {}, }; -const configurationReducer = reduceReducers(initialAlertmanagerConfiguration, muteTimingsReducer, receiversReducer); +const configurationReducer = reduceReducers( + initialAlertmanagerConfiguration, + muteTimingsReducer, + receiversReducer, + notificationTemplatesReducer +); /** * This hook will make sure we are always applying actions that mutate the Alertmanager configuration diff --git a/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/notificationTemplates.test.ts.snap b/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/notificationTemplates.test.ts.snap new file mode 100644 index 00000000000..370761c3fe6 --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/__snapshots__/notificationTemplates.test.ts.snap @@ -0,0 +1,55 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`notification templates should add a new notification template 1`] = ` +{ + "alertmanager_config": { + "templates": [ + "foo", + ], + }, + "template_files": { + "foo": "{{ define "foo" }} + foo +{{ end }}", + }, +} +`; + +exports[`notification templates should allow renaming a notification template 1`] = ` +{ + "alertmanager_config": { + "templates": [ + "rename-me-copy", + ], + }, + "template_files": { + "rename-me-copy": "{{ define "rename-me-copy" }} + rename me, please +{{ end }}", + }, +} +`; + +exports[`notification templates should remove a notification template 1`] = ` +{ + "alertmanager_config": { + "templates": [], + }, + "template_files": {}, +} +`; + +exports[`notification templates should update a notification template without renaming 1`] = ` +{ + "alertmanager_config": { + "templates": [ + "update-me", + ], + }, + "template_files": { + "update-me": "{{ define "update-me" }} + update me, please +{{ end }}", + }, +} +`; diff --git a/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.test.ts b/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.test.ts new file mode 100644 index 00000000000..c7cacdf2e5f --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.test.ts @@ -0,0 +1,119 @@ +import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types'; + +import { TemplateFormValues } from '../../components/receivers/TemplateForm'; + +import { + addNotificationTemplateAction, + deleteNotificationTemplateAction, + notificationTemplatesReducer, + updateNotificationTemplateAction, +} from './notificationTemplates'; + +describe('notification templates', () => { + it('should add a new notification template', () => { + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: {}, + template_files: {}, + }; + const newNotificationTemplate: TemplateFormValues = { + title: 'foo', + content: 'foo', + }; + + const action = addNotificationTemplateAction({ template: newNotificationTemplate }); + expect(notificationTemplatesReducer(initialConfig, action)).toMatchSnapshot(); + }); + + it('should not add a new notification template if the name already exists', () => { + const name = 'existing'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: { templates: [name] }, + template_files: { + [name]: 'foo', + }, + }; + const newNotificationTemplate: TemplateFormValues = { + title: name, + content: 'foo', + }; + + const action = addNotificationTemplateAction({ template: newNotificationTemplate }); + expect(() => notificationTemplatesReducer(initialConfig, action)).toThrow(/already exists/); + }); + + it('should update a notification template without renaming', () => { + const name = 'update-me'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: { + templates: [name], + }, + template_files: { + [name]: 'update me', + }, + }; + + const action = updateNotificationTemplateAction({ name, template: { title: name, content: 'update me, please' } }); + expect(notificationTemplatesReducer(initialConfig, action)).toMatchSnapshot(); + }); + + it('should not update if target does not exist', () => { + const name = 'update-me'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: {}, + template_files: {}, + }; + + const action = updateNotificationTemplateAction({ name, template: { title: name, content: 'update me, please' } }); + expect(() => notificationTemplatesReducer(initialConfig, action)).toThrow(/did not find it/); + }); + + it('should not update if renaming and new template name exist', () => { + const name = 'rename-me'; + const name2 = 'rename-me-2'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: { + templates: [name, name2], + }, + template_files: { + [name]: 'foo', + [name2]: 'bar', + }, + }; + + const action = updateNotificationTemplateAction({ name, template: { title: name2, content: 'foo' } }); + expect(() => notificationTemplatesReducer(initialConfig, action)).toThrow(/duplicate/i); + }); + + it('should allow renaming a notification template', () => { + const name = 'rename-me'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: { + templates: [name], + }, + template_files: { + [name]: 'rename me', + }, + }; + + const action = updateNotificationTemplateAction({ + name, + template: { title: 'rename-me-copy', content: 'rename me, please' }, + }); + expect(notificationTemplatesReducer(initialConfig, action)).toMatchSnapshot(); + }); + + it('should remove a notification template', () => { + const name = 'delete-me'; + const initialConfig: AlertManagerCortexConfig = { + alertmanager_config: { + templates: [name], + }, + template_files: { + [name]: 'delete me please', + }, + }; + + const action = deleteNotificationTemplateAction({ name }); + expect(notificationTemplatesReducer(initialConfig, action)).toMatchSnapshot(); + }); +}); diff --git a/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.ts b/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.ts new file mode 100644 index 00000000000..47bc215e719 --- /dev/null +++ b/public/app/features/alerting/unified/reducers/alertmanager/notificationTemplates.ts @@ -0,0 +1,83 @@ +import { createAction, createReducer } from '@reduxjs/toolkit'; +import { remove, toArray, unset } from 'lodash'; + +import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types'; + +import { TemplateFormValues } from '../../components/receivers/TemplateForm'; +import { ensureDefine } from '../../utils/templates'; + +export const addNotificationTemplateAction = createAction<{ template: TemplateFormValues }>('notificationTemplate/add'); +export const updateNotificationTemplateAction = createAction<{ + name: string; + template: TemplateFormValues; +}>('notificationTemplate/update'); +export const deleteNotificationTemplateAction = createAction<{ name: string }>('notificationTemplate/delete'); + +const initialState: AlertManagerCortexConfig = { + alertmanager_config: {}, + template_files: {}, +}; + +/** + * This reducer will manage action related to notification templates and make sure all operations on the alertmanager + * configuration happen immutably and only mutate what they need. + */ +export const notificationTemplatesReducer = createReducer(initialState, (builder) => { + builder + .addCase(addNotificationTemplateAction, (draft, { payload }) => { + const { alertmanager_config = {}, template_files = {} } = draft; + const { template } = payload; + + const targetTemplateExists = template_files[template.title] !== undefined; + if (targetTemplateExists) { + throw new Error('target template already exists'); + } + + // wrap content in "define" if it's not already wrapped, in case user did not do it/ + // it's not obvious that this is needed for template to work + const content = ensureDefine(template.title, template.content); + + // add the template to the list of template files + template_files[template.title] = content; + + // add the template to the alertmanager_config + alertmanager_config.templates = toArray(alertmanager_config.templates).concat(template.title); + }) + .addCase(updateNotificationTemplateAction, (draft, { payload }) => { + const { alertmanager_config = {}, template_files = {} } = draft; + const { name, template } = payload; + const renaming = name !== template.title; + + const targetExists = template_files[name] !== undefined; + if (!targetExists) { + throw new Error(`Expected notification template ${name} to exist, but did not find it in the config`); + } + + // wrap content in "define" if it's not already wrapped, in case user did not do it/ + // it's not obvious that this is needed for template to work + const content = ensureDefine(template.title, template.content); + + if (renaming) { + const oldName = name; + const newName = template.title; + + const targetExists = template_files[newName] !== undefined; + if (targetExists) { + throw new Error(`Duplicate template name ${newName}`); + } + + unset(template_files, oldName); + remove(alertmanager_config.templates ?? [], (templateName) => templateName === oldName); + alertmanager_config.templates = toArray(alertmanager_config.templates).concat(template.title); + } + + template_files[template.title] = content; + }) + .addCase(deleteNotificationTemplateAction, (draft, { payload }) => { + const { name } = payload; + const { alertmanager_config = {}, template_files = {} } = draft; + + unset(template_files, name); + remove(alertmanager_config.templates ?? [], (templateName) => templateName === name); + }); +});