From f30795b0884dd3485423a12d7d7d0860453967a7 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Wed, 17 Aug 2022 14:37:29 +0200 Subject: [PATCH] Prometheus: Throw error on direct access (#50162) * Disable direct access * Hide access select if Server mode is already selected * Update docs * Add more tests Co-authored-by: Beto Muniz --- .betterer.results | 13 +++-- docs/sources/datasources/prometheus.md | 2 +- .../prometheus/configuration/ConfigEditor.tsx | 10 ++-- .../datasource/prometheus/datasource.test.ts | 55 ++++++++++++++++++- .../datasource/prometheus/datasource.tsx | 19 ++++++- 5 files changed, 84 insertions(+), 15 deletions(-) diff --git a/.betterer.results b/.betterer.results index 0d0c2935017..dfcd2fb49dd 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7857,7 +7857,10 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "102"], [0, 0, 0, "Unexpected any. Specify a different type.", "103"], [0, 0, 0, "Unexpected any. Specify a different type.", "104"], - [0, 0, 0, "Unexpected any. Specify a different type.", "105"] + [0, 0, 0, "Unexpected any. Specify a different type.", "105"], + [0, 0, 0, "Unexpected any. Specify a different type.", "106"], + [0, 0, 0, "Unexpected any. Specify a different type.", "107"], + [0, 0, 0, "Unexpected any. Specify a different type.", "108"] ], "public/app/plugins/datasource/prometheus/datasource.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -7882,9 +7885,9 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "19"], [0, 0, 0, "Unexpected any. Specify a different type.", "20"], [0, 0, 0, "Unexpected any. Specify a different type.", "21"], - [0, 0, 0, "Unexpected any. Specify a different type.", "22"], + [0, 0, 0, "Do not use any type assertions.", "22"], [0, 0, 0, "Unexpected any. Specify a different type.", "23"], - [0, 0, 0, "Do not use any type assertions.", "24"], + [0, 0, 0, "Unexpected any. Specify a different type.", "24"], [0, 0, 0, "Unexpected any. Specify a different type.", "25"], [0, 0, 0, "Unexpected any. Specify a different type.", "26"], [0, 0, 0, "Unexpected any. Specify a different type.", "27"], @@ -7893,9 +7896,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "30"], [0, 0, 0, "Unexpected any. Specify a different type.", "31"], [0, 0, 0, "Unexpected any. Specify a different type.", "32"], - [0, 0, 0, "Unexpected any. Specify a different type.", "33"], - [0, 0, 0, "Unexpected any. Specify a different type.", "34"], - [0, 0, 0, "Unexpected any. Specify a different type.", "35"] + [0, 0, 0, "Unexpected any. Specify a different type.", "33"] ], "public/app/plugins/datasource/prometheus/language_provider.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/docs/sources/datasources/prometheus.md b/docs/sources/datasources/prometheus.md index 5e498b8c20b..21a6d3f3675 100644 --- a/docs/sources/datasources/prometheus.md +++ b/docs/sources/datasources/prometheus.md @@ -26,7 +26,7 @@ To access Prometheus settings, hover your mouse over the **Configuration** (gear | `Name` | The data source name. This is how you refer to the data source in panels and queries. | | `Default` | Default data source that is pre-selected for new panels. | | `Url` | The URL of your Prometheus server, for example, `http://prometheus.example.org:9090`. | -| `Access` | Server (default) = URL needs to be accessible from the Grafana backend/server, Browser = URL needs to be accessible from the browser. **Note**: Browser (direct) access is deprecated and will be removed in a future release. | +| `Access` | Only Server access mode is functional. If Server mode is already selected this option is hidden. Otherwise change to Server mode to prevent errors. | | `Basic Auth` | Enable basic authentication to the Prometheus data source. | | `User` | User name for basic authentication. | | `Password` | Password for basic authentication. | diff --git a/public/app/plugins/datasource/prometheus/configuration/ConfigEditor.tsx b/public/app/plugins/datasource/prometheus/configuration/ConfigEditor.tsx index f2f90f14e54..cec3f13b871 100644 --- a/public/app/plugins/datasource/prometheus/configuration/ConfigEditor.tsx +++ b/public/app/plugins/datasource/prometheus/configuration/ConfigEditor.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useRef } from 'react'; import { SIGV4ConnectionConfig } from '@grafana/aws-sdk'; import { DataSourcePluginOptionsEditorProps, DataSourceSettings } from '@grafana/data'; @@ -16,6 +16,8 @@ export type Props = DataSourcePluginOptionsEditorProps; export const ConfigEditor = (props: Props) => { const { options, onOptionsChange } = props; const alertmanagers = getAllAlertmanagerDataSources(); + // use ref so this is evaluated only first time it renders and the select does not disappear suddenly. + const showAccessOptions = useRef(props.options.access === 'direct'); const azureAuthSettings = { azureAuthSupported: config.azureAuthEnabled, @@ -28,15 +30,15 @@ export const ConfigEditor = (props: Props) => { return ( <> {options.access === 'direct' && ( - - Browser access mode in the Prometheus datasource is deprecated and will be removed in a future release. + + Browser access mode in the Prometheus datasource is no longer available. Switch to server access mode. )} { expect(response[0].state).toBe(LoadingState.Done); }); }); + + it('throws if using direct access', async () => { + const instanceSettings = { + url: 'proxied', + directUrl: 'direct', + user: 'test', + password: 'mupp', + access: 'direct', + jsonData: { + customQueryParameters: '', + } as any, + } as unknown as DataSourceInstanceSettings; + const range = { from: time({ seconds: 63 }), to: time({ seconds: 183 }) }; + const directDs = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); + + await expect( + lastValueFrom(directDs.query(createDataRequest([{}, {}], { app: CoreApp.Dashboard }))) + ).rejects.toMatchObject({ message: expect.stringMatching('Browser access') }); + + // Cannot test because some other tests need "./metric_find_query" to be mocked and that prevents this to be + // tested. Checked manually that this ends up with throwing + // await expect(directDs.metricFindQuery('label_names(foo)')).rejects.toBeDefined(); + + jest.spyOn(console, 'error').mockImplementation(() => {}); + await expect(directDs.testDatasource()).resolves.toMatchObject({ + message: expect.stringMatching('Browser access'), + status: 'error', + }); + await expect( + directDs.annotationQuery({ + range: { ...range, raw: range }, + rangeRaw: range, + // Should be DataModel but cannot import that here from the main app. Needs to be moved to package first. + dashboard: {}, + annotation: { + expr: 'metric', + name: 'test', + enable: true, + iconColor: '', + }, + }) + ).rejects.toMatchObject({ + message: expect.stringMatching('Browser access'), + }); + await expect(directDs.getTagKeys()).rejects.toMatchObject({ + message: expect.stringMatching('Browser access'), + }); + await expect(directDs.getTagValues()).rejects.toMatchObject({ + message: expect.stringMatching('Browser access'), + }); + }); }); describe('Datasource metadata requests', () => { @@ -683,7 +734,7 @@ const HOUR = 60 * MINUTE; const time = ({ hours = 0, seconds = 0, minutes = 0 }) => dateTime(hours * HOUR + minutes * MINUTE + seconds * SECOND); -describe('PrometheusDatasource', () => { +describe('PrometheusDatasource2', () => { const instanceSettings = { url: 'proxied', directUrl: 'direct', diff --git a/public/app/plugins/datasource/prometheus/datasource.tsx b/public/app/plugins/datasource/prometheus/datasource.tsx index 473771061d2..729d1aea0b8 100644 --- a/public/app/plugins/datasource/prometheus/datasource.tsx +++ b/public/app/plugins/datasource/prometheus/datasource.tsx @@ -22,6 +22,7 @@ import { TimeRange, DataFrame, dateTime, + AnnotationQueryRequest, QueryFixAction, } from '@grafana/data'; import { @@ -161,6 +162,13 @@ export class PrometheusDatasource data: Record | null, overrides: Partial = {} ): Observable> { + if (this.access === 'direct') { + const error = new Error( + 'Browser access mode in the Prometheus datasource is no longer available. Switch to server access mode.' + ); + return throwError(() => error); + } + data = data || {}; for (const [key, value] of this.customQueryParameters) { if (data[key] == null) { @@ -698,7 +706,14 @@ export class PrometheusDatasource }; } - async annotationQuery(options: any): Promise { + async annotationQuery(options: AnnotationQueryRequest): Promise { + if (this.access === 'direct') { + const error = new Error( + 'Browser access mode in the Prometheus datasource is no longer available. Switch to server access mode.' + ); + return Promise.reject(error); + } + const annotation = options.annotation; const { expr = '' } = annotation; @@ -738,7 +753,7 @@ export class PrometheusDatasource ); } - processAnnotationResponse = (options: any, data: BackendDataSourceResponse) => { + processAnnotationResponse = (options: AnnotationQueryRequest, data: BackendDataSourceResponse) => { const frames: DataFrame[] = toDataQueryResponse({ data: data }).data; if (!frames || !frames.length) { return [];