Alerting: Allow deleting contact points referenced only by auto-generated policies (#86800)

This commit is contained in:
Gilles De Mey 2024-04-29 18:48:54 +02:00 committed by GitHub
parent 36a0499128
commit b679a32fad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 109 additions and 37 deletions

View File

@ -21,6 +21,7 @@ import setupMimirFlavoredServer, { MIMIR_DATASOURCE_UID } from './__mocks__/mimi
import setupVanillaAlertmanagerFlavoredServer, {
VANILLA_ALERTMANAGER_DATASOURCE_UID,
} from './__mocks__/vanillaAlertmanagerServer';
import { RouteReference } from './utils';
/**
* There are lots of ways in which we test our pages and components. Here's my opinionated approach to testing them.
@ -224,13 +225,19 @@ describe('contact points', () => {
expect(deleteButton).toBeDisabled();
});
it('should disable delete when contact point is linked to at least one notification policy', async () => {
render(
<ContactPoint name={'my-contact-point'} provisioned={true} receivers={[]} policies={1} onDelete={noop} />,
it('should disable delete when contact point is linked to at least one normal notification policy', async () => {
const policies: RouteReference[] = [
{
wrapper,
}
);
receiver: 'my-contact-point',
route: {
type: 'normal',
},
},
];
render(<ContactPoint name={'my-contact-point'} receivers={[]} policies={policies} onDelete={noop} />, {
wrapper,
});
expect(screen.getByRole('link', { name: 'is used by 1 notification policy' })).toBeInTheDocument();
@ -241,6 +248,27 @@ describe('contact points', () => {
expect(deleteButton).toBeDisabled();
});
it('should not disable delete when contact point is linked only to auto-generated notification policy', async () => {
const policies: RouteReference[] = [
{
receiver: 'my-contact-point',
route: {
type: 'auto-generated',
},
},
];
render(<ContactPoint name={'my-contact-point'} receivers={[]} policies={policies} onDelete={noop} />, {
wrapper,
});
const moreActions = screen.getByRole('button', { name: 'more-actions' });
await userEvent.click(moreActions);
const deleteButton = screen.getByRole('menuitem', { name: /delete/i });
expect(deleteButton).not.toBeDisabled();
});
it('should be able to search', async () => {
renderWithProvider();

View File

@ -60,7 +60,13 @@ import {
useContactPointsWithStatus,
useDeleteContactPoint,
} from './useContactPoints';
import { ContactPointWithMetadata, getReceiverDescription, isProvisioned, ReceiverConfigWithMetadata } from './utils';
import {
ContactPointWithMetadata,
getReceiverDescription,
isProvisioned,
ReceiverConfigWithMetadata,
RouteReference,
} from './utils';
export enum ActiveTab {
ContactPoints = 'contact_points',
@ -243,7 +249,7 @@ const ContactPointsList = ({
<>
{pageItems.map((contactPoint, index) => {
const provisioned = isProvisioned(contactPoint);
const policies = contactPoint.numberOfPolicies;
const policies = contactPoint.policies ?? [];
const key = `${contactPoint.name}-${index}`;
return (
@ -304,7 +310,7 @@ interface ContactPointProps {
disabled?: boolean;
provisioned?: boolean;
receivers: ReceiverConfigWithMetadata[];
policies?: number;
policies?: RouteReference[];
onDelete: (name: string) => void;
}
@ -313,7 +319,7 @@ export const ContactPoint = ({
disabled = false,
provisioned = false,
receivers,
policies = 0,
policies = [],
onDelete,
}: ContactPointProps) => {
const styles = useStyles2(getStyles);
@ -367,12 +373,12 @@ interface ContactPointHeaderProps {
name: string;
disabled?: boolean;
provisioned?: boolean;
policies?: number;
policies?: RouteReference[];
onDelete: (name: string) => void;
}
const ContactPointHeader = (props: ContactPointHeaderProps) => {
const { name, disabled = false, provisioned = false, policies = 0, onDelete } = props;
const { name, disabled = false, provisioned = false, policies = [], onDelete } = props;
const styles = useStyles2(getStyles);
const [exportSupported, exportAllowed] = useAlertmanagerAbility(AlertmanagerAction.ExportContactPoint);
@ -381,9 +387,12 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {
const [ExportDrawer, openExportDrawer] = useExportContactPoint();
const isReferencedByPolicies = policies > 0;
const numberOfPolicies = policies.length;
const isReferencedByAnyPolicy = numberOfPolicies > 0;
const isReferencedByRegularPolicies = policies.some((ref) => ref.route.type !== 'auto-generated');
const canEdit = editSupported && editAllowed && !provisioned;
const canDelete = deleteSupported && deleteAllowed && !provisioned && policies === 0;
const canDelete = deleteSupported && deleteAllowed && !provisioned && !isReferencedByRegularPolicies;
const menuActions: JSX.Element[] = [];
@ -407,7 +416,7 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {
menuActions.push(
<ConditionalWrap
key="delete-contact-point"
shouldWrap={isReferencedByPolicies}
shouldWrap={!canDelete}
wrap={(children) => (
<Tooltip content="Contact point is currently in use by one or more notification policies" placement="top">
<span>{children}</span>
@ -434,15 +443,15 @@ const ContactPointHeader = (props: ContactPointHeaderProps) => {
{name}
</Text>
</Stack>
{isReferencedByPolicies && (
{isReferencedByAnyPolicy && (
<MetaText>
<Link to={createUrl('/alerting/routes', { contactPoint: name })}>
is used by <Strong>{policies}</Strong> {pluralize('notification policy', policies)}
is used by <Strong>{numberOfPolicies}</Strong> {pluralize('notification policy', numberOfPolicies)}
</Link>
</MetaText>
)}
{provisioned && <ProvisioningBadge />}
{!isReferencedByPolicies && <UnusedContactPointBadge />}
{!isReferencedByAnyPolicy && <UnusedContactPointBadge />}
<Spacer />
<LinkButton
tooltipPlacement="top"

View File

@ -30,7 +30,14 @@ exports[`useContactPoints should return contact points with status 1`] = `
},
],
"name": "grafana-default-email",
"numberOfPolicies": 1,
"policies": [
{
"receiver": "grafana-default-email",
"route": {
"type": "normal",
},
},
],
},
{
"grafana_managed_receiver_configs": [
@ -58,7 +65,7 @@ exports[`useContactPoints should return contact points with status 1`] = `
},
],
"name": "lotsa-emails",
"numberOfPolicies": 0,
"policies": [],
},
{
"grafana_managed_receiver_configs": [
@ -84,7 +91,7 @@ exports[`useContactPoints should return contact points with status 1`] = `
},
],
"name": "OnCall Conctact point",
"numberOfPolicies": 0,
"policies": [],
},
{
"grafana_managed_receiver_configs": [
@ -113,7 +120,14 @@ exports[`useContactPoints should return contact points with status 1`] = `
},
],
"name": "provisioned-contact-point",
"numberOfPolicies": 1,
"policies": [
{
"receiver": "provisioned-contact-point",
"route": {
"type": "normal",
},
},
],
},
{
"grafana_managed_receiver_configs": [
@ -165,7 +179,7 @@ exports[`useContactPoints should return contact points with status 1`] = `
},
],
"name": "Slack with multiple channels",
"numberOfPolicies": 0,
"policies": [],
},
],
"error": undefined,
@ -204,7 +218,7 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag
},
],
"name": "grafana-default-email",
"numberOfPolicies": undefined,
"policies": undefined,
},
{
"grafana_managed_receiver_configs": [
@ -232,7 +246,7 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag
},
],
"name": "lotsa-emails",
"numberOfPolicies": undefined,
"policies": undefined,
},
{
"grafana_managed_receiver_configs": [
@ -255,7 +269,7 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag
},
],
"name": "OnCall Conctact point",
"numberOfPolicies": undefined,
"policies": undefined,
},
{
"grafana_managed_receiver_configs": [
@ -284,7 +298,7 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag
},
],
"name": "provisioned-contact-point",
"numberOfPolicies": undefined,
"policies": undefined,
},
{
"grafana_managed_receiver_configs": [
@ -336,7 +350,7 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag
},
],
"name": "Slack with multiple channels",
"numberOfPolicies": undefined,
"policies": undefined,
},
],
"error": undefined,

View File

@ -2,6 +2,8 @@ import React, { useCallback, useMemo, useState } from 'react';
import { Button, Modal, ModalProps } from '@grafana/ui';
import { stringifyErrorLike } from '../../../utils/misc';
type ModalHook<T = undefined> = [JSX.Element, (item: T) => void, () => void];
/**
@ -83,7 +85,9 @@ const ErrorModal = ({ isOpen, onDismiss, error }: ErrorModalProps) => (
>
<p>Failed to update your configuration:</p>
<p>
<code>{String(error)}</code>
<pre>
<code>{stringifyErrorLike(error)}</code>
</pre>
</p>
</Modal>
);

View File

@ -1,4 +1,4 @@
import { countBy, difference, take, trim, upperFirst } from 'lodash';
import { difference, groupBy, take, trim, upperFirst } from 'lodash';
import { ReactNode } from 'react';
import { config } from '@grafana/runtime';
@ -99,7 +99,7 @@ export interface ReceiverConfigWithMetadata extends GrafanaManagedReceiverConfig
}
export interface ContactPointWithMetadata extends GrafanaManagedContactPoint {
numberOfPolicies?: number; // now is optional as we don't have the data from the read-only endpoint
policies?: RouteReference[]; // now is optional as we don't have the data from the read-only endpoint
grafana_managed_receiver_configs: ReceiverConfigWithMetadata[];
}
@ -121,7 +121,7 @@ export function enhanceContactPointsWithMetadata(
// compute the entire inherited tree before finding what notification policies are using a particular contact point
const fullyInheritedTree = computeInheritedTree(alertmanagerConfiguration?.alertmanager_config?.route ?? {});
const usedContactPoints = getUsedContactPoints(fullyInheritedTree);
const usedContactPointsByName = countBy(usedContactPoints);
const usedContactPointsByName = groupBy(usedContactPoints, 'receiver');
const contactPointsList = alertmanagerConfiguration
? alertmanagerConfiguration?.alertmanager_config.receivers ?? []
@ -133,8 +133,8 @@ export function enhanceContactPointsWithMetadata(
return {
...contactPoint,
numberOfPolicies:
alertmanagerConfiguration && usedContactPointsByName && (usedContactPointsByName[contactPoint.name] ?? 0),
policies:
alertmanagerConfiguration && usedContactPointsByName && (usedContactPointsByName[contactPoint.name] ?? []),
grafana_managed_receiver_configs: receivers.map((receiver, index) => {
const isOnCallReceiver = receiver.type === ReceiverTypes.OnCall;
// if we don't have alertmanagerConfiguration we can't get the metadata for oncall receivers,
@ -171,10 +171,26 @@ export function isAutoGeneratedPolicy(route: Route) {
);
}
export function getUsedContactPoints(route: Route): string[] {
export interface RouteReference {
receiver: string;
route: {
type: 'auto-generated' | 'normal';
};
}
export function getUsedContactPoints(route: Route): RouteReference[] {
const childrenContactPoints = route.routes?.flatMap((route) => getUsedContactPoints(route)) ?? [];
if (route.receiver) {
return [route.receiver, ...childrenContactPoints];
return [
{
receiver: route.receiver,
route: {
type: isAutoGeneratedPolicy(route) ? 'auto-generated' : 'normal',
},
},
...childrenContactPoints,
];
}
return childrenContactPoints;

View File

@ -263,6 +263,7 @@ describe('Can create a new grafana managed alert unsing simplified routing', ()
expect(mocks.api.setRulerRuleGroup).not.toHaveBeenCalled();
});
});
it('can create new grafana managed alert when using simplified routing and selecting a contact point', async () => {
const contactPointsAvailable: ContactPointWithMetadata[] = [
{
@ -279,7 +280,7 @@ describe('Can create a new grafana managed alert unsing simplified routing', ()
settings: {},
},
],
numberOfPolicies: 0,
policies: [],
},
];
mocks.useContactPointsWithStatus.mockReturnValue({