public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Fix selecting empty contact point value for notification policy inheritance (#81482)

Browse Source
This commit is contained in:
Gilles De Mey
2024-02-01 16:11:23 +01:00
committed by GitHub
parent 0726c7c3fa
commit 7f1138dfe6
5 changed files with 61 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/app/features/alerting/unified/components/notification-policies/Policy.tsx
+21 -18
View File
@@ -34,7 +34,7 @@ import { AlertmanagerAction, useAlertmanagerAbilities, useAlertmanagerAbility }
import { INTEGRATION_ICONS } from '../../types/contact-points';
import { normalizeMatchers } from '../../utils/matchers';
import { createContactPointLink, createMuteTimingLink } from '../../utils/misc';
import { InhertitableProperties, getInheritedProperties } from '../../utils/notification-policies';
import { InheritableProperties, getInheritedProperties } from '../../utils/notification-policies';
import { Authorize } from '../Authorize';
import { HoverCard } from '../HoverCard';
import { Label } from '../Label';
@@ -53,7 +53,7 @@ interface PolicyComponentProps {
contactPointsState?: ReceiversState;
readOnly?: boolean;
provisioned?: boolean;
inheritedProperties?: Partial<InhertitableProperties>;
inheritedProperties?: Partial<InheritableProperties>;
routesMatchingFilters?: RouteWithID[];
// routeAlertGroupsMap?: Map<string, AlertmanagerGroup[]>;
@@ -239,7 +239,7 @@ const Policy = (props: PolicyComponentProps) => {
<MetadataRow
matchingInstancesPreview={matchingInstancesPreview}
numberOfAlertInstances={numberOfAlertInstances}
contactPoint={contactPoint}
contactPoint={contactPoint ?? undefined}
groupBy={groupBy}
muteTimings={muteTimings}
timingOptions={timingOptions}
@@ -261,7 +261,7 @@ const Policy = (props: PolicyComponentProps) => {
const childInheritedProperties = getInheritedProperties(currentRoute, child, inheritedProperties);
// This child is autogenerated if it's the autogenerated root or if it's a child of an autogenerated policy.
const isThisChildAutoGenerated = isAutoGeneratedRootAndSimplifiedEnabled(child) || isAutoGenerated;
/* pass the "readOnly" prop from the parent, because for any child policy , if its parent it's not editable,
/* pass the "readOnly" prop from the parent, because for any child policy , if its parent it's not editable,
then the child policy should not be editable either */
const isThisChildReadOnly = readOnly || provisioned || isAutoGenerated;
@@ -319,7 +319,7 @@ interface MetadataRowProps {
groupBy?: string[];
muteTimings?: string[];
timingOptions?: TimingOptions;
inheritedProperties?: Partial<InhertitableProperties>;
inheritedProperties?: Partial<InheritableProperties>;
alertManagerSourceName: string;
receivers: Receiver[];
matchingAlertGroups?: AlertmanagerGroup[];
@@ -571,19 +571,25 @@ function AutogeneratedRootIndicator() {
return <strong> Auto-generated policies</strong>;
}
const InheritedProperties: FC<{ properties: InhertitableProperties }> = ({ properties }) => (
const InheritedProperties: FC<{ properties: InheritableProperties }> = ({ properties }) => (
<HoverCard
arrow
placement="top"
content={
<Stack direction="row" gap={0.5}>
{Object.entries(properties).map(([key, value]) => (
<Label
key={key}
label={routePropertyToLabel(key)}
value={<Strong>{routePropertyToValue(key, value)}</Strong>}
/>
))}
{Object.entries(properties).map(([key, value]) => {
if (!value) {
return null;
}
return (
<Label
key={key}
label={routePropertyToLabel(key)}
value={<Strong>{routePropertyToValue(key, value)}</Strong>}
/>
);
})}
</Stack>
}
>
@@ -750,7 +756,7 @@ function getContactPointErrors(contactPoint: string, contactPointsState: Receive
return contactPointErrors;
}
const routePropertyToLabel = (key: keyof InhertitableProperties | string): string => {
const routePropertyToLabel = (key: keyof InheritableProperties | string): string => {
switch (key) {
case 'receiver':
return 'Contact Point';
@@ -767,10 +773,7 @@ const routePropertyToLabel = (key: keyof InhertitableProperties | string): strin
}
};
const routePropertyToValue = (
key: keyof InhertitableProperties | string,
value: string | string[]
): React.ReactNode => {
const routePropertyToValue = (key: keyof InheritableProperties | string, value: string | string[]): React.ReactNode => {
const isNotGrouping = key === 'group_by' && Array.isArray(value) && value[0] === '...';
const isSingleGroup = key === 'group_by' && Array.isArray(value) && value.length === 0;
public/app/features/alerting/unified/utils/amroutes.ts
+2 -2
View File
@@ -198,10 +198,10 @@ export const stringToSelectableValue = (str: string): SelectableValue<string> =>
export const stringsToSelectableValues = (arr: string[] | undefined): Array<SelectableValue<string>> =>
(arr ?? []).map(stringToSelectableValue);
export const mapSelectValueToString = (selectableValue: SelectableValue<string>): string | undefined => {
export const mapSelectValueToString = (selectableValue: SelectableValue<string>): string | null => {
// this allows us to deal with cleared values
if (selectableValue === null) {
return undefined;
return null;
}
if (!selectableValue) {
public/app/features/alerting/unified/utils/notification-policies.test.ts
+22 -10
View File
@@ -1,7 +1,7 @@
import { MatcherOperator, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types';
import {
InhertitableProperties,
InheritableProperties,
computeInheritedTree,
findMatchingRoutes,
getInheritedProperties,
@@ -205,20 +205,18 @@ describe('getInheritedProperties()', () => {
expect(childInherited).toHaveProperty('group_by', ['label']);
});
// This scenario is technically impossible unless we have a bug in our code.
// A route cannot both specify a receiver and inherit it from its parent at the same time.
it('should inherit from parent instead of grandparent', () => {
const parent: Route = { receiver: 'parent' };
const parentInherited: InhertitableProperties = { receiver: 'grandparent', group_by: ['foo'] };
const child: Route = {};
it('should inherit from grandparent when parent is inheriting', () => {
const parentInheritedProperties: InheritableProperties = { receiver: 'grandparent' };
const parent: Route = { receiver: null, group_by: ['foo'] };
const child: Route = { receiver: null };
const childInherited = getInheritedProperties(parent, child, parentInherited);
expect(childInherited).toHaveProperty('receiver', 'parent');
const childInherited = getInheritedProperties(parent, child, parentInheritedProperties);
expect(childInherited).toHaveProperty('receiver', 'grandparent');
expect(childInherited.group_by).toEqual(['foo']);
});
});
describe('regular "undefined" values', () => {
describe('regular "undefined" or "null" values', () => {
it('should compute inherited properties being undefined', () => {
const parent: Route = {
receiver: 'PARENT',
@@ -233,6 +231,20 @@ describe('getInheritedProperties()', () => {
expect(childInherited).toHaveProperty('group_wait', '10s');
});
it('should compute inherited properties being null', () => {
const parent: Route = {
receiver: 'PARENT',
group_wait: '10s',
};
const child: Route = {
receiver: null,
};
const childInherited = getInheritedProperties(parent, child);
expect(childInherited).toHaveProperty('receiver', 'PARENT');
});
it('should compute inherited properties being undefined from parent inherited properties', () => {
const parent: Route = {
receiver: 'PARENT',
public/app/features/alerting/unified/utils/notification-policies.ts
+15 -14
View File
@@ -1,4 +1,4 @@
import { isArray, merge, pick, reduce } from 'lodash';
import { isArray, pick, reduce } from 'lodash';
import {
AlertmanagerGroup,
@@ -20,7 +20,7 @@ interface LabelMatchResult {
export const INHERITABLE_KEYS = ['receiver', 'group_by', 'group_wait', 'group_interval', 'repeat_interval'] as const;
export type InheritableKeys = typeof INHERITABLE_KEYS;
export type InhertitableProperties = Pick<Route, InheritableKeys[number]>;
export type InheritableProperties = Pick<Route, InheritableKeys[number]>;
type LabelsMatch = Map<Label, LabelMatchResult>;
@@ -158,21 +158,23 @@ function findMatchingAlertGroups(
function getInheritedProperties(
parentRoute: Route,
childRoute: Route,
propertiesParentInherited?: Partial<InhertitableProperties>
) {
const propsFromParent: InhertitableProperties = pick(parentRoute, INHERITABLE_KEYS);
const inheritableProperties: InhertitableProperties = merge({}, propertiesParentInherited, propsFromParent);
propertiesParentInherited?: InheritableProperties
): InheritableProperties {
const propsFromParent: InheritableProperties = pick(parentRoute, INHERITABLE_KEYS);
const inheritableProperties: InheritableProperties = {
...propsFromParent,
...propertiesParentInherited,
};
// TODO how to solve this TypeScript mystery?
const inherited = reduce(
inheritableProperties,
(inheritedProperties: Partial<Route> = {}, parentValue, property) => {
const parentHasValue = parentValue !== undefined;
(inheritedProperties: InheritableProperties, parentValue, property) => {
const parentHasValue = parentValue != null;
const inheritableValues = [undefined, '', null];
// @ts-ignore
const inheritFromParentUndefined = parentHasValue && childRoute[property] === undefined;
// @ts-ignore
const inheritFromParentEmptyString = parentHasValue && childRoute[property] === '';
const childIsInheriting = inheritableValues.some((value) => childRoute[property] === value);
const inheritFromValue = childIsInheriting && parentHasValue;
const inheritEmptyGroupByFromParent =
property === 'group_by' &&
@@ -180,8 +182,7 @@ function getInheritedProperties(
isArray(childRoute[property]) &&
childRoute[property]?.length === 0;
const inheritFromParent =
inheritFromParentUndefined || inheritFromParentEmptyString || inheritEmptyGroupByFromParent;
const inheritFromParent = inheritFromValue || inheritEmptyGroupByFromParent;
if (inheritFromParent) {
// @ts-ignore
public/app/plugins/datasource/alertmanager/types.ts
+1 -1
View File
@@ -99,7 +99,7 @@ export type Receiver = GrafanaManagedContactPoint | AlertmanagerReceiver;
export type ObjectMatcher = [name: string, operator: MatcherOperator, value: string];
export type Route = {
receiver?: string;
receiver?: string | null;
group_by?: string[];
continue?: boolean;
object_matchers?: ObjectMatcher[];