From bbf3f1fb071d9093d96c869344dd085957d6998c Mon Sep 17 00:00:00 2001 From: Benedikt Seidl Date: Thu, 18 Jan 2024 09:41:48 +0100 Subject: [PATCH 1/2] Align defaults for datasource settings Previously we had two different defaults for the edition setting: * one default was set to CEE but was only applied if the datasource settings page was opened and was only applied to the backend after "save" was pressed. * the other default setting was set to RAW and was applied in the backend If you use a provisioned datasource and did not specify the edition explicitly, you saw "CEE" chosen in the UI, but the backend actually used the "RAW" edition. In order to align this, we chose to set both defaults to CEE. **If you use checkmk raw edition and a provisioned datasource or created the datasource with an very old version of this plugin** you have to take manual action: * if you use a provisioned datasource: specify the edition explicitly * if you used a old version of this plugin to create the datasource: open the datasource settings, make sure the RAW edition is chosen, and save the settings. You can use the following python script to check if you are affected: (You can only be affected if you use the plugin to fetch matrices from checkmk raw edition.) ``` import requests GRAFANA_URL = "http://127.0.0.1:3000/" # with closing slash URL = f"{GRAFANA_URL}api/datasources" GRAFANA_USER = "admin" GRAFANA_PASSWORD = "password" data_sources = requests.get(URL, auth=(GRAFANA_USER, GRAFANA_PASSWORD)) for data_source in data_sources.json(): if data_source["type"] in {"tribe-29-checkmk-datasource", "checkmk-cloud-datasource"}: json_data = data_source["jsonData"] if "edition" not in json_data: print("found affected data_source:") print(f' name: {data_source["name"]}') print(f' id: {data_source["id"]}') print(f' uid: {data_source["uid"]}') ``` --- src/DataSource.ts | 15 ++++++--------- src/settings.ts | 27 +++++++++++++++++++++++++++ src/ui/ConfigEditor.tsx | 18 ++++++------------ tests/unit/CloudEdition.test.tsx | 5 ----- 4 files changed, 39 insertions(+), 26 deletions(-) create mode 100644 src/settings.ts diff --git a/src/DataSource.ts b/src/DataSource.ts index a43ab9ad..84c421fd 100644 --- a/src/DataSource.ts +++ b/src/DataSource.ts @@ -12,6 +12,7 @@ import { MetricFindQuery, RequestSpec } from './RequestSpec'; import RestApiBackend from './backend/rest'; import { Backend } from './backend/types'; import WebApiBackend from './backend/web'; +import { Settings } from './settings'; import { Backend as BackendType, CmkQuery, DataSourceOptions, Edition, ResponseDataAutocomplete } from './types'; import { AutoCompleteParams } from './ui/autocomplete'; import { createCmkContext } from './utils'; @@ -20,11 +21,13 @@ import { WebApiResponse } from './webapi'; export class DataSource extends DataSourceApi { webBackend: WebApiBackend; restBackend: RestApiBackend; + settings: Settings; constructor(private instanceSettings: DataSourceInstanceSettings) { super(instanceSettings); this.webBackend = new WebApiBackend(this); this.restBackend = new RestApiBackend(this); + this.settings = new Settings(instanceSettings.jsonData); } async query(dataQueryRequest: DataQueryRequest): Promise { @@ -94,14 +97,12 @@ export class DataSource extends DataSourceApi { return this.instanceSettings.url; } - // TODO: Move config default values to a central place instead of scattering it in getEdition and getBackendType - getEdition(): Edition { - return this.instanceSettings.jsonData.edition ?? 'RAW'; + return this.settings.edition; } getBackendType(): BackendType { - return this.instanceSettings.jsonData.backend ?? 'rest'; + return this.settings.backend; } getBackend(): Backend { @@ -112,10 +113,6 @@ export class DataSource extends DataSourceApi { } getUsername(): string { - const username = this.instanceSettings.jsonData.username; - if (typeof username === 'string') { - return username; - } - throw Error('Impossible'); + return this.settings.username; } } diff --git a/src/settings.ts b/src/settings.ts new file mode 100644 index 00000000..9ee9e3d5 --- /dev/null +++ b/src/settings.ts @@ -0,0 +1,27 @@ +import { Backend as BackendType, DataSourceOptions, Edition } from './types'; + +export class Settings { + protected settings: DataSourceOptions; + + constructor(settings: DataSourceOptions) { + this.settings = settings; + } + + get edition(): Edition { + // cloud instances of this plugin don't save the edition and use this default + return this.settings.edition ?? 'CEE'; + } + + get backend(): BackendType { + // cloud instances of this plugin don't save the backend and use this default + return this.settings.backend ?? 'rest'; + } + + get url(): string | undefined { + return this.settings.url; + } + + get username(): string { + return this.settings.username ?? ''; + } +} diff --git a/src/ui/ConfigEditor.tsx b/src/ui/ConfigEditor.tsx index 89fad84d..ce5625d6 100644 --- a/src/ui/ConfigEditor.tsx +++ b/src/ui/ConfigEditor.tsx @@ -2,6 +2,7 @@ import { DataSourcePluginOptionsEditorProps, SelectableValue } from '@grafana/da import { Alert, FieldSet, InlineField, LegacyForms, Select } from '@grafana/ui'; import React, { ChangeEvent, useCallback } from 'react'; +import { Settings } from '../settings'; import { Backend, DataSourceOptions, Edition, SecureJsonData } from '../types'; const { SecretFormField, FormField } = LegacyForms; @@ -107,14 +108,7 @@ export const ConfigEditor = (props: Props) => { const { options } = props; const { jsonData, secureJsonFields } = options; const secureJsonData = options.secureJsonData || {}; - - if (!jsonData.edition) { - onEditionChange(cmkEditions[0]); - } - - if (!jsonData.backend) { - onBackendChange(cmkBackends[0]); - } + const settings = new Settings(jsonData); return ( <> @@ -125,7 +119,7 @@ export const ConfigEditor = (props: Props) => { labelWidth={6} inputWidth={20} onChange={onUrlChange} - value={jsonData.url || ''} + value={settings.url || ''} tooltip="Which Checkmk Server to connect to. (Example: https://checkmk.server/site)" data-test-id="checkmk-url" /> @@ -137,7 +131,7 @@ export const ConfigEditor = (props: Props) => { width={32} options={cmkEditions} onChange={onEditionChange} - value={jsonData.edition} + value={settings.edition} placeholder="Select your checkmk edition" inputId="checkmk-edition" /> @@ -151,7 +145,7 @@ export const ConfigEditor = (props: Props) => { width={32} options={cmkBackends} onChange={onBackendChange} - value={jsonData.backend} + value={settings.backend} placeholder="Select your checkmk version" inputId="checkmk-version" /> @@ -177,7 +171,7 @@ export const ConfigEditor = (props: Props) => { labelWidth={6} inputWidth={20} onChange={onUsernameChange} - value={jsonData.username || ''} + value={settings.username} tooltip="A checkmk monitoring user. Don't use 'automation' user, because it has admin rights." data-test-id="checkmk-username" /> diff --git a/tests/unit/CloudEdition.test.tsx b/tests/unit/CloudEdition.test.tsx index b7aafe47..5349579c 100644 --- a/tests/unit/CloudEdition.test.tsx +++ b/tests/unit/CloudEdition.test.tsx @@ -44,11 +44,6 @@ describe('Cloud Edition Restrictions', () => { expect(screen.queryByLabelText('Edition')).toBeNull(); expect(screen.queryByLabelText('Version')).toBeNull(); }); - - it('sets the non configurable values to the defaults', () => { - expect(onOptionsChange).toHaveBeenCalledWith(expect.objectContaining({ jsonData: { backend: 'rest' } })); - expect(onOptionsChange).toHaveBeenCalledWith(expect.objectContaining({ jsonData: { edition: 'CEE' } })); - }); }); describe('RestApiBackend', () => { From 2a5320ed21049ca47ce7bb112e558a76801a2114 Mon Sep 17 00:00:00 2001 From: Benedikt Seidl Date: Tue, 23 Jan 2024 11:45:26 +0100 Subject: [PATCH 2/2] Clarify error message on 404 on REST-API request We assumed that this can only happen if you query a Checkmk version that has no graph query endpoints at all. But the enterprise REST-API endpoint will return a 404 error in Checkmk raw edition, so we adapted the hint for the user. Unrelated fix: Checkmk should start with capital letter. --- src/backend/rest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/rest.ts b/src/backend/rest.ts index af030316..97160c6f 100644 --- a/src/backend/rest.ts +++ b/src/backend/rest.ts @@ -195,7 +195,7 @@ export default class RestApiBackend implements Backend { } return { status: 'success', - message: `Data source is working, reached version ${checkMkVersion} of checkmk`, + message: `Data source is working, reached version ${checkMkVersion} of Checkmk`, title: 'Success', }; } @@ -224,7 +224,7 @@ export default class RestApiBackend implements Backend { // but we may have a more detailed error message if (error.status === 404) { throw new Error( - 'REST API graph endpoints are unavailable. Choose correct checkmk version in data source settings.' + 'REST API graph endpoint is unavailable. Choose correct Checkmk edition and version in data source settings.' ); }