From cc68f1b673c668312cab11fe8a6eda1844c5c8b3 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Mon, 23 Sep 2024 14:02:58 +0100 Subject: [PATCH] Alerting: Fix sending secure settings when using K8S API for contact points (#93498) --- .../contact-points/useContactPoints.tsx | 22 +- .../form/GrafanaReceiverForm.test.tsx | 49 +++- .../GrafanaReceiverForm.test.tsx.snap | 31 ++ .../alerting/unified/mockGrafanaNotifiers.ts | 272 ++++++++++++++++++ .../unified/mocks/server/configure.ts | 2 + .../server/handlers/k8s/receivers.k8s.ts | 2 +- 6 files changed, 372 insertions(+), 6 deletions(-) create mode 100644 public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap diff --git a/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx b/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx index 3584be0bf62..10d56e2ac5f 100644 --- a/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx +++ b/public/app/features/alerting/unified/components/contact-points/useContactPoints.tsx @@ -4,7 +4,7 @@ */ import { produce } from 'immer'; -import { remove } from 'lodash'; +import { merge, remove, set } from 'lodash'; import { useMemo } from 'react'; import { alertingApi } from 'app/features/alerting/unified/api/alertingApi'; @@ -349,9 +349,9 @@ export function useDeleteContactPoint({ alertmanager }: BaseAlertmanagerArgs) { * so we should not tell the API that we want to preserve it. Those values will instead be sent within `settings` */ const mapIntegrationSettingsForK8s = (integration: GrafanaManagedReceiverConfig): GrafanaManagedReceiverConfig => { - const { secureSettings, ...restOfIntegration } = integration; - + const { secureSettings, settings, ...restOfIntegration } = integration; const secureFields = Object.entries(secureSettings || {}).reduce((acc, [key, value]) => { + // If a secure field has no (changed) value, then we tell the backend to persist it if (value === undefined) { return { ...acc, @@ -361,10 +361,24 @@ const mapIntegrationSettingsForK8s = (integration: GrafanaManagedReceiverConfig) return acc; }, {}); + const mappedSecureSettings = Object.entries(secureSettings || {}).reduce((acc, [key, value]) => { + // If the value is an empty string/falsy value, then we need to omit it from the payload + // so the backend knows to remove it + if (!value) { + return acc; + } + + // Otherwise, we send the value of the secure field + return set(acc, key, value); + }, {}); + + // Merge settings properly with lodash so we don't lose any information from nested keys/secure settings + const mergedSettings = merge({}, settings, mappedSecureSettings); + return { ...restOfIntegration, secureFields, - settings: { ...restOfIntegration.settings, ...secureSettings }, + settings: mergedSettings, }; }; const grafanaContactPointToK8sReceiver = ( 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 9cbf672221e..23e2d50466f 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,8 +1,10 @@ import { clickSelectOption } from 'test/helpers/selectOptionInTest'; -import { render, waitFor } from 'test/test-utils'; +import { render, waitFor, screen } 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, @@ -33,6 +35,51 @@ const ui = { }; describe('GrafanaReceiverForm', () => { + describe('alertingApiServer', () => { + beforeEach(() => { + config.featureToggles.alertingApiServer = true; + }); + afterEach(() => { + config.featureToggles.alertingApiServer = false; + }); + + it('handles nested secure fields correctly', async () => { + const capturedRequests = captureRequests( + (req) => req.url.includes('/v0alpha1/namespaces/default/receivers') && req.method === 'POST' + ); + const { user } = render(); + const { type, click } = user; + + await waitFor(() => expect(ui.loadingIndicator.query()).not.toBeInTheDocument()); + + // Select MQTT receiver and fill out basic required fields for contact point + await clickSelectOption(await byTestId('items.0.type').find(), 'MQTT'); + await type(screen.getByLabelText(/^name/i), 'mqtt contact point'); + await type(screen.getByLabelText(/broker url/i), 'broker url'); + await type(screen.getByLabelText(/topic/i), 'topic'); + + // Fill out fields that we know will be nested secure fields + await click(screen.getByText(/optional mqtt settings/i)); + await click(screen.getByRole('button', { name: /^Add$/i })); + await type(screen.getByLabelText(/ca certificate/i), 'some cert'); + + await click(screen.getByRole('button', { name: /save contact point/i })); + + const [request] = await capturedRequests; + const postRequestbody = await request.clone().json(); + + const integrationPayload = postRequestbody.spec.integrations[0]; + expect(integrationPayload.settings.tlsConfig).toEqual({ + // Expect the payload to have included the value of a secret field + caCertificate: 'some cert', + // And to not have removed other values (which would happen if we incorrectly merged settings together) + insecureSkipVerify: false, + }); + + expect(postRequestbody).toMatchSnapshot(); + }); + }); + describe('OnCall contact point', () => { it('OnCall contact point should be disabled if OnCall integration is not enabled', async () => { disablePlugin(SupportedPlugin.OnCall); diff --git a/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap new file mode 100644 index 00000000000..547720877c2 --- /dev/null +++ b/public/app/features/alerting/unified/components/receivers/form/__snapshots__/GrafanaReceiverForm.test.tsx.snap @@ -0,0 +1,31 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`GrafanaReceiverForm alertingApiServer handles nested secure fields correctly 1`] = ` +{ + "metadata": {}, + "spec": { + "integrations": [ + { + "disableResolveMessage": false, + "name": "mqtt contact point", + "secureFields": { + "password": true, + "tlsConfig.clientCertificate": true, + "tlsConfig.clientKey": true, + }, + "settings": { + "brokerUrl": "broker url", + "retain": false, + "tlsConfig": { + "caCertificate": "some cert", + "insecureSkipVerify": false, + }, + "topic": "topic", + }, + "type": "mqtt", + }, + ], + "title": "mqtt contact point", + }, +} +`; diff --git a/public/app/features/alerting/unified/mockGrafanaNotifiers.ts b/public/app/features/alerting/unified/mockGrafanaNotifiers.ts index 6efaa9d89f8..58c20a86262 100644 --- a/public/app/features/alerting/unified/mockGrafanaNotifiers.ts +++ b/public/app/features/alerting/unified/mockGrafanaNotifiers.ts @@ -2461,6 +2461,278 @@ export const grafanaAlertNotifiersMock: NotifierDTO[] = [ }, ], }, + { + type: 'mqtt', + name: 'MQTT', + heading: 'MQTT settings', + description: 'Sends notifications to an MQTT broker', + info: 'The MQTT notifier sends messages to an MQTT broker. The message is sent to the topic specified in the configuration. ', + options: [ + { + element: 'input', + inputType: 'text', + label: 'Broker URL', + description: 'The URL of the MQTT broker.', + placeholder: 'tcp://localhost:1883', + propertyName: 'brokerUrl', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: true, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'input', + inputType: 'text', + label: 'Topic', + description: 'The topic to which the message will be sent.', + placeholder: 'grafana/alerts', + propertyName: 'topic', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: true, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'select', + inputType: 'text', + label: 'Message format', + description: + "The format of the message to be sent. If set to 'json', the message will be sent as a JSON object. If set to 'text', the message will be sent as a plain text string. By default json is used.", + placeholder: 'json', + propertyName: 'messageFormat', + selectOptions: [ + { + value: 'json', + label: 'json', + }, + { + value: 'text', + label: 'text', + }, + ], + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'input', + inputType: 'text', + label: 'Client ID', + description: 'The client ID to use when connecting to the MQTT broker. If blank, a random client ID is used.', + placeholder: '', + propertyName: 'clientId', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'textarea', + inputType: '', + label: 'Message', + description: '', + placeholder: '{{ template "default.message" . }}', + propertyName: 'message', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'input', + inputType: 'text', + label: 'Username', + description: 'The username to use when connecting to the MQTT broker.', + placeholder: '', + propertyName: 'username', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'input', + inputType: 'text', + label: 'Password', + description: 'The password to use when connecting to the MQTT broker.', + placeholder: '', + propertyName: 'password', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: true, + dependsOn: '', + }, + { + element: 'select', + inputType: '', + label: 'QoS', + description: 'The quality of service to use when sending the message.', + placeholder: '', + propertyName: 'qos', + selectOptions: [ + { + value: '0', + label: 'At most once (0)', + }, + { + value: '1', + label: 'At least once (1)', + }, + { + value: '2', + label: 'Exactly once (2)', + }, + ], + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'checkbox', + inputType: '', + label: 'Retain', + description: 'If set to true, the message will be retained by the broker.', + placeholder: '', + propertyName: 'retain', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'subform', + inputType: '', + label: 'TLS', + description: 'TLS configuration options', + placeholder: '', + propertyName: 'tlsConfig', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + subformOptions: [ + { + element: 'checkbox', + inputType: '', + label: 'Disable certificate verification', + description: "Do not verify the broker's certificate chain and host name.", + placeholder: '', + propertyName: 'insecureSkipVerify', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: false, + dependsOn: '', + }, + { + element: 'textarea', + inputType: 'text', + label: 'CA Certificate', + description: "Certificate in PEM format to use when verifying the broker's certificate chain.", + placeholder: '', + propertyName: 'caCertificate', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: true, + dependsOn: '', + }, + { + element: 'textarea', + inputType: 'text', + label: 'Client Certificate', + description: 'Client certificate in PEM format to use when connecting to the broker.', + placeholder: '', + propertyName: 'clientCertificate', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: true, + dependsOn: '', + }, + { + element: 'textarea', + inputType: 'text', + label: 'Client Key', + description: 'Client key in PEM format to use when connecting to the broker.', + placeholder: '', + propertyName: 'clientKey', + selectOptions: null, + showWhen: { + field: '', + is: '', + }, + required: false, + validationRule: '', + secure: true, + dependsOn: '', + }, + ], + }, + ], + }, { type: 'opsgenie', name: 'OpsGenie', diff --git a/public/app/features/alerting/unified/mocks/server/configure.ts b/public/app/features/alerting/unified/mocks/server/configure.ts index db47b049265..bfd4ec5467a 100644 --- a/public/app/features/alerting/unified/mocks/server/configure.ts +++ b/public/app/features/alerting/unified/mocks/server/configure.ts @@ -17,6 +17,7 @@ import { getPluginMissingHandler, } from 'app/features/alerting/unified/mocks/server/handlers/plugins'; import { SupportedPlugin } from 'app/features/alerting/unified/types/pluginBridges'; +import { clearPluginSettingsCache } from 'app/features/plugins/pluginSettings'; import { AlertManagerCortexConfig, AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types'; import { FolderDTO } from 'app/types'; @@ -126,6 +127,7 @@ export const removePlugin = (pluginId: string) => { /** Make a plugin respond with `enabled: false`, as if its installed but disabled */ export const disablePlugin = (pluginId: SupportedPlugin) => { + clearPluginSettingsCache(pluginId); server.use(getDisabledPluginHandler(pluginId)); }; diff --git a/public/app/features/alerting/unified/mocks/server/handlers/k8s/receivers.k8s.ts b/public/app/features/alerting/unified/mocks/server/handlers/k8s/receivers.k8s.ts index 8983b0171b0..9963678011a 100644 --- a/public/app/features/alerting/unified/mocks/server/handlers/k8s/receivers.k8s.ts +++ b/public/app/features/alerting/unified/mocks/server/handlers/k8s/receivers.k8s.ts @@ -43,7 +43,7 @@ const createNamespacedReceiverHandler = () => http.post<{ namespace: string }>( `${ALERTING_API_SERVER_BASE_URL}/namespaces/:namespace/receivers`, async ({ request }) => { - const body = await request.json(); + const body = await request.clone().json(); return HttpResponse.json(body); } );