Alerting: Fix issue with Slack contact point validation (#47559)

* secureFields and secureSettings

* revert channelIndex

* readd lost code

* use specific return

* register secure fields and use not hard coded index

* fix for determineReadOnly

* fix lint error

* fix test suite

Co-authored-by: gillesdemey <gilles.de.mey@gmail.com>
This commit is contained in:
Peter Holmberg
2022-04-20 09:40:57 +02:00
committed by GitHub
co-authored by gillesdemey
parent e58a015baf
commit 39d3c8afd7
4 changed files with 53 additions and 26 deletions
@@ -383,7 +383,7 @@ func GetAvailableNotifiers() []*alerting.NotifierPlugin {
Description: "Specify channel, private group, or IM channel (can be an encoded ID or a name) - required unless you provide a webhook", Description: "Specify channel, private group, or IM channel (can be an encoded ID or a name) - required unless you provide a webhook",
PropertyName: "recipient", PropertyName: "recipient",
Required: true, Required: true,
DependsOn: "secureSettings.url", DependsOn: "url",
}, },
// Logically, this field should be required when not using a webhook, since the Slack API needs a token. // Logically, this field should be required when not using a webhook, since the Slack API needs a token.
// However, since the UI doesn't allow to say that a field is required or not depending on another field, // However, since the UI doesn't allow to say that a field is required or not depending on another field,
@@ -397,7 +397,7 @@ func GetAvailableNotifiers() []*alerting.NotifierPlugin {
PropertyName: "token", PropertyName: "token",
Secure: true, Secure: true,
Required: true, Required: true,
DependsOn: "secureSettings.url", DependsOn: "url",
}, },
{ {
Label: "Username", Label: "Username",
@@ -463,7 +463,7 @@ func GetAvailableNotifiers() []*alerting.NotifierPlugin {
PropertyName: "url", PropertyName: "url",
Secure: true, Secure: true,
Required: true, Required: true,
DependsOn: "secureSettings.token", DependsOn: "token",
}, },
{ // New in 8.4. { // New in 8.4.
Label: "Endpoint URL", Label: "Endpoint URL",
@@ -47,12 +47,7 @@ export function ChannelOptions<R extends ChannelValues>({
value="Configured" value="Configured"
suffix={ suffix={
readOnly ? null : ( readOnly ? null : (
<Button <Button onClick={() => onResetSecureField(option.propertyName)} fill="text" type="button" size="sm">
onClick={() => onResetSecureField(option.propertyName)}
variant="link"
type="button"
size="sm"
>
Clear Clear
</Button> </Button>
) )
@@ -74,7 +69,8 @@ export function ChannelOptions<R extends ChannelValues>({
readOnly={readOnly} readOnly={readOnly}
key={key} key={key}
error={error} error={error}
pathPrefix={option.secure ? `${pathPrefix}secureSettings.` : `${pathPrefix}settings.`} pathPrefix={pathPrefix}
pathSuffix={option.secure ? 'secureSettings.' : 'settings.'}
option={option} option={option}
/> />
); );
@@ -37,12 +37,15 @@ export function ChannelSubForm<R extends ChannelValues>({
}: Props<R>): JSX.Element { }: Props<R>): JSX.Element {
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const name = (fieldName: string) => `${pathPrefix}${fieldName}`; const name = (fieldName: string) => `${pathPrefix}${fieldName}`;
const { control, watch, register, trigger, formState } = useFormContext(); const { control, watch, register, trigger, formState, setValue } = useFormContext();
const selectedType = watch(name('type')) ?? defaultValues.type; // nope, setting "default" does not work at all. const selectedType = watch(name('type')) ?? defaultValues.type; // nope, setting "default" does not work at all.
const { loading: testingReceiver } = useUnifiedAlertingSelector((state) => state.testReceivers); const { loading: testingReceiver } = useUnifiedAlertingSelector((state) => state.testReceivers);
useEffect(() => { useEffect(() => {
register(`${pathPrefix}.__id`); register(`${pathPrefix}.__id`);
/* Need to manually register secureFields or else they'll
be lost when testing a contact point */
register(`${pathPrefix}.secureFields`);
}, [register, pathPrefix]); }, [register, pathPrefix]);
const [_secureFields, setSecureFields] = useState(secureFields ?? {}); const [_secureFields, setSecureFields] = useState(secureFields ?? {});
@@ -52,6 +55,7 @@ export function ChannelSubForm<R extends ChannelValues>({
const updatedSecureFields = { ...secureFields }; const updatedSecureFields = { ...secureFields };
delete updatedSecureFields[key]; delete updatedSecureFields[key];
setSecureFields(updatedSecureFields); setSecureFields(updatedSecureFields);
setValue(`${pathPrefix}.secureFields`, updatedSecureFields);
} }
}; };
@@ -1,5 +1,6 @@
import React, { FC, useEffect } from 'react'; import React, { FC, useEffect } from 'react';
import { Checkbox, Field, Input, InputControl, Select, TextArea } from '@grafana/ui'; import { Checkbox, Field, Input, InputControl, Select, TextArea } from '@grafana/ui';
import { isEmpty } from 'lodash';
import { NotificationChannelOption } from 'app/types'; import { NotificationChannelOption } from 'app/types';
import { useFormContext, FieldError, DeepMap } from 'react-hook-form'; import { useFormContext, FieldError, DeepMap } from 'react-hook-form';
import { SubformField } from './SubformField'; import { SubformField } from './SubformField';
@@ -13,11 +14,22 @@ interface Props {
option: NotificationChannelOption; option: NotificationChannelOption;
invalid?: boolean; invalid?: boolean;
pathPrefix: string; pathPrefix: string;
pathSuffix?: string;
error?: FieldError | DeepMap<any, FieldError>; error?: FieldError | DeepMap<any, FieldError>;
readOnly?: boolean; readOnly?: boolean;
} }
export const OptionField: FC<Props> = ({ option, invalid, pathPrefix, error, defaultValue, readOnly = false }) => { export const OptionField: FC<Props> = ({
option,
invalid,
pathPrefix,
pathSuffix = '',
error,
defaultValue,
readOnly = false,
}) => {
const optionPath = `${pathPrefix}${pathSuffix}`;
if (option.element === 'subform') { if (option.element === 'subform') {
return ( return (
<SubformField <SubformField
@@ -25,7 +37,7 @@ export const OptionField: FC<Props> = ({ option, invalid, pathPrefix, error, def
defaultValue={defaultValue} defaultValue={defaultValue}
option={option} option={option}
errors={error as DeepMap<any, FieldError> | undefined} errors={error as DeepMap<any, FieldError> | undefined}
pathPrefix={pathPrefix} pathPrefix={optionPath}
/> />
); );
} }
@@ -35,7 +47,7 @@ export const OptionField: FC<Props> = ({ option, invalid, pathPrefix, error, def
readOnly={readOnly} readOnly={readOnly}
defaultValues={defaultValue} defaultValues={defaultValue}
option={option} option={option}
pathPrefix={pathPrefix} pathPrefix={optionPath}
errors={error as Array<DeepMap<any, FieldError>> | undefined} errors={error as Array<DeepMap<any, FieldError>> | undefined}
/> />
); );
@@ -48,18 +60,26 @@ export const OptionField: FC<Props> = ({ option, invalid, pathPrefix, error, def
error={error?.message} error={error?.message}
> >
<OptionInput <OptionInput
id={`${pathPrefix}${option.propertyName}`} id={`${optionPath}${option.propertyName}`}
defaultValue={defaultValue} defaultValue={defaultValue}
option={option} option={option}
invalid={invalid} invalid={invalid}
pathPrefix={pathPrefix} pathPrefix={optionPath}
readOnly={readOnly} readOnly={readOnly}
pathIndex={pathPrefix}
/> />
</Field> </Field>
); );
}; };
const OptionInput: FC<Props & { id: string }> = ({ option, invalid, id, pathPrefix = '', readOnly = false }) => { const OptionInput: FC<Props & { id: string; pathIndex?: string }> = ({
option,
invalid,
id,
pathPrefix = '',
pathIndex = '',
readOnly = false,
}) => {
const { control, register, unregister, getValues } = useFormContext(); const { control, register, unregister, getValues } = useFormContext();
const name = `${pathPrefix}${option.propertyName}`; const name = `${pathPrefix}${option.propertyName}`;
@@ -87,11 +107,11 @@ const OptionInput: FC<Props & { id: string }> = ({ option, invalid, id, pathPref
return ( return (
<Input <Input
id={id} id={id}
readOnly={readOnly || determineReadOnly(option, getValues)} readOnly={readOnly || determineReadOnly(option, getValues, pathIndex)}
invalid={invalid} invalid={invalid}
type={option.inputType} type={option.inputType}
{...register(name, { {...register(name, {
required: determineRequired(option, getValues), required: determineRequired(option, getValues, pathIndex),
validate: (v) => (option.validationRule !== '' ? validateOption(v, option.validationRule) : true), validate: (v) => (option.validationRule !== '' ? validateOption(v, option.validationRule) : true),
})} })}
placeholder={option.placeholder} placeholder={option.placeholder}
@@ -165,19 +185,26 @@ const validateOption = (value: string, validationRule: string) => {
return RegExp(validationRule).test(value) ? true : 'Invalid format'; return RegExp(validationRule).test(value) ? true : 'Invalid format';
}; };
const determineRequired = (option: NotificationChannelOption, getValues: any) => { const determineRequired = (option: NotificationChannelOption, getValues: any, pathIndex: string) => {
if (!option.dependsOn) { if (!option.dependsOn) {
return option.required ? 'Required' : false; return option.required ? 'Required' : false;
} }
if (isEmpty(getValues(`${pathIndex}secureFields`))) {
const dependentOn = getValues(`items[0].${option.dependsOn}`); const dependentOn = getValues(`${pathIndex}secureSettings.${option.dependsOn}`);
return !dependentOn && option.required ? 'Required' : false; return !Boolean(dependentOn) && option.required ? 'Required' : false;
} else {
const dependentOn: boolean = getValues(`${pathIndex}secureFields.${option.dependsOn}`);
return !dependentOn && option.required ? 'Required' : false;
}
}; };
const determineReadOnly = (option: NotificationChannelOption, getValues: any) => { const determineReadOnly = (option: NotificationChannelOption, getValues: any, pathIndex: string) => {
if (!option.dependsOn) { if (!option.dependsOn) {
return false; return false;
} }
if (isEmpty(getValues(`${pathIndex}secureFields`))) {
return getValues(`items[0].${option.dependsOn}`); return getValues(`${pathIndex}secureSettings.${option.dependsOn}`);
} else {
return getValues(`${pathIndex}secureFields.${option.dependsOn}`);
}
}; };