From 817f7879478dede2394c262501cb64364272cdfb Mon Sep 17 00:00:00 2001 From: Vanilla Date: Thu, 18 Apr 2024 21:46:04 +0800 Subject: [PATCH 01/21] Cli: Check missing plugin parameter of plugin update command (#86410) Co-authored-by: Marcus Efraimsson --- pkg/cmd/grafana-cli/commands/upgrade_command.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/grafana-cli/commands/upgrade_command.go b/pkg/cmd/grafana-cli/commands/upgrade_command.go index a122f55b2fc..a0a3ba4500e 100644 --- a/pkg/cmd/grafana-cli/commands/upgrade_command.go +++ b/pkg/cmd/grafana-cli/commands/upgrade_command.go @@ -2,6 +2,7 @@ package commands import ( "context" + "errors" "fmt" "github.com/fatih/color" @@ -15,6 +16,9 @@ func upgradeCommand(c utils.CommandLine) error { ctx := context.Background() pluginsDir := c.PluginDirectory() pluginID := c.Args().First() + if pluginID == "" { + return errors.New("please specify plugin to update") + } localPlugin, err := services.GetLocalPlugin(pluginsDir, pluginID) if err != nil { From 344cea1725eb413192b07f7beda2e4b3818e0435 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Thu, 18 Apr 2024 15:55:13 +0200 Subject: [PATCH 02/21] Alerting: Fix external Alertmanager settings payload (#86413) * Remove helper properties from the AM config object * Make the omitTemporaryIdentifiers a pure function * Add formValuesToCloudReceiver test * Remove immer produce to prevent object freezing --- .../unified/utils/receiver-form.test.ts | 145 +++++++++++++++++- .../alerting/unified/utils/receiver-form.ts | 20 ++- 2 files changed, 162 insertions(+), 3 deletions(-) diff --git a/public/app/features/alerting/unified/utils/receiver-form.test.ts b/public/app/features/alerting/unified/utils/receiver-form.test.ts index 7db175181df..06ee33629a2 100644 --- a/public/app/features/alerting/unified/utils/receiver-form.test.ts +++ b/public/app/features/alerting/unified/utils/receiver-form.test.ts @@ -1,8 +1,17 @@ +import 'core-js/stable/structured-clone'; + import { NotifierDTO } from 'app/types'; -import { GrafanaChannelValues, ReceiverFormValues } from '../types/receiver-form'; +import { Receiver } from '../../../../plugins/datasource/alertmanager/types'; +import { CloudChannelValues, GrafanaChannelValues, ReceiverFormValues } from '../types/receiver-form'; -import { formValuesToGrafanaReceiver, omitEmptyValues, omitEmptyUnlessExisting } from './receiver-form'; +import { + formValuesToGrafanaReceiver, + omitEmptyValues, + omitEmptyUnlessExisting, + omitTemporaryIdentifiers, + formValuesToCloudReceiver, +} from './receiver-form'; describe('Receiver form utils', () => { describe('omitEmptyStringValues', () => { @@ -67,6 +76,91 @@ describe('Receiver form utils', () => { expect(omitEmptyUnlessExisting(original, existing)).toEqual(expected); }); }); + + describe('omitTemporaryIdentifiers', () => { + it('should remove __id from the root object', () => { + const original = { + __id: '1', + foo: 'bar', + }; + + const expected = { + foo: 'bar', + }; + + expect(omitTemporaryIdentifiers(original)).toEqual(expected); + }); + + it('should remove __id from nested objects', () => { + const original = { + foo: 'bar', + nested: { + __id: '1', + baz: 'qux', + doubleNested: { __id: '2', url: 'example.com' }, + }, + }; + + const expected = { + foo: 'bar', + nested: { + baz: 'qux', + doubleNested: { url: 'example.com' }, + }, + }; + + expect(omitTemporaryIdentifiers(original)).toEqual(expected); + }); + + it('should remove __id from objects in an array', () => { + const original = { + foo: 'bar', + array: [ + { + __id: '1', + baz: 'qux', + actions: [ + { __id: '3', type: 'email' }, + { __id: '4', type: 'slack' }, + ], + }, + { __id: '2', quux: 'quuz' }, + ], + }; + + const expected = { + foo: 'bar', + array: [ + { + baz: 'qux', + actions: [{ type: 'email' }, { type: 'slack' }], + }, + { + quux: 'quuz', + }, + ], + }; + + expect(omitTemporaryIdentifiers(original)).toEqual(expected); + }); + + it('should return a new object and keep the original intact', () => { + const original = { + foo: 'bar', + nested: { + __id: '1', + baz: 'qux', + doubleNested: { __id: '2', url: 'example.com' }, + }, + }; + + const withOmitted = omitTemporaryIdentifiers(original); + + expect(withOmitted).not.toBe(original); + expect(original.nested.__id).toBe('1'); + expect(original.nested.doubleNested.__id).toBe('2'); + }); + }); }); describe('formValuesToGrafanaReceiver', () => { @@ -111,3 +205,50 @@ describe('formValuesToGrafanaReceiver', () => { expect(formValuesToGrafanaReceiver(formValues, channelMap, {}, notifiers)).toMatchSnapshot(); }); }); + +describe('formValuesToCloudReceiver', () => { + it('should remove temporary ids from receivers and settings', () => { + const formValues: ReceiverFormValues = { + name: 'my-receiver', + items: [ + { + __id: '1', + type: 'slack', + settings: { + url: 'https://slack.example.com/', + actions: [{ __id: '2', text: 'Acknowledge', type: 'button' }], + fields: [{ __id: '10', title: 'priority', value: '1' }], + }, + secureFields: {}, + secureSettings: {}, + sendResolved: true, + }, + ], + }; + + const defaults: CloudChannelValues = { + __id: '1', + type: 'slack', + settings: { + url: 'https://slack.example.com/', + }, + secureFields: {}, + secureSettings: {}, + sendResolved: true, + }; + + const expected: Receiver = { + name: 'my-receiver', + slack_configs: [ + { + url: 'https://slack.example.com/', + actions: [{ text: 'Acknowledge', type: 'button' }], + fields: [{ title: 'priority', value: '1' }], + send_resolved: true, + }, + ], + }; + + expect(formValuesToCloudReceiver(formValues, defaults)).toEqual(expected); + }); +}); diff --git a/public/app/features/alerting/unified/utils/receiver-form.ts b/public/app/features/alerting/unified/utils/receiver-form.ts index d779c9f2cb8..b7e4b35233b 100644 --- a/public/app/features/alerting/unified/utils/receiver-form.ts +++ b/public/app/features/alerting/unified/utils/receiver-form.ts @@ -108,7 +108,7 @@ export function formValuesToCloudReceiver( }; values.items.forEach(({ __id, type, settings, sendResolved }) => { const channel = omitEmptyValues({ - ...settings, + ...omitTemporaryIdentifiers(settings), send_resolved: sendResolved ?? defaults.sendResolved, }); @@ -292,3 +292,21 @@ export function omitEmptyValues(obj: T): T { export function omitEmptyUnlessExisting(settings = {}, existing = {}): Record { return omitBy(settings, (value, key) => isUnacceptableValue(value) && !(key in existing)); } + +export function omitTemporaryIdentifiers(object: Readonly): T { + function omitIdentifiers(obj: T) { + if (isArray(obj)) { + obj.forEach(omitIdentifiers); + } else if (typeof obj === 'object' && obj !== null) { + if ('__id' in obj) { + delete obj.__id; + } + Object.values(obj).forEach(omitIdentifiers); + } + } + + const objectCopy = structuredClone(object); + omitIdentifiers(objectCopy); + + return objectCopy; +} From bbf4281d8d51543f472745335ae289abd76932ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Thu, 18 Apr 2024 16:13:08 +0200 Subject: [PATCH 03/21] Grafana UI: `Dropdown.story.tsx` - Replace `VerticalGroup` with `Stack` (#86521) --- .betterer.results | 3 --- .../grafana-ui/src/components/Dropdown/Dropdown.story.tsx | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.betterer.results b/.betterer.results index 31648327a82..e861dbe1415 100644 --- a/.betterer.results +++ b/.betterer.results @@ -797,9 +797,6 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], - "packages/grafana-ui/src/components/Dropdown/Dropdown.story.tsx:5381": [ - [0, 0, 0, "\'VerticalGroup\' import from \'../Layout/Layout\' is restricted from being used by a pattern. Use Stack component instead.", "0"] - ], "packages/grafana-ui/src/components/Forms/Checkbox.story.tsx:5381": [ [0, 0, 0, "\'VerticalGroup\' import from \'../Layout/Layout\' is restricted from being used by a pattern. Use Stack component instead.", "0"] ], diff --git a/packages/grafana-ui/src/components/Dropdown/Dropdown.story.tsx b/packages/grafana-ui/src/components/Dropdown/Dropdown.story.tsx index d991195e5da..fbcef7c6c8d 100644 --- a/packages/grafana-ui/src/components/Dropdown/Dropdown.story.tsx +++ b/packages/grafana-ui/src/components/Dropdown/Dropdown.story.tsx @@ -4,7 +4,7 @@ import React from 'react'; import { StoryExample } from '../../utils/storybook/StoryExample'; import { Button } from '../Button'; import { IconButton } from '../IconButton/IconButton'; -import { VerticalGroup } from '../Layout/Layout'; +import { Stack } from '../Layout/Stack/Stack'; import { Menu } from '../Menu/Menu'; import { Dropdown } from './Dropdown'; @@ -34,7 +34,7 @@ export function Examples() { ); return ( - + @@ -46,7 +46,7 @@ export function Examples() { - + ); } From 99f34cb1ed0a5e59c682837c08a443184e05334e Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Thu, 18 Apr 2024 16:25:58 +0200 Subject: [PATCH 04/21] Common labels/displayed fields: Show label names with values (#86345) * LogLabels: create specialized component for arrays of labels * Logs: sort displayed fields when assigning to state * LogsMetaRow: fix types and use specialized components * LogLabels: show label and value * LogsPanel: update common labels * LogsMetaRow: use LogsLabelsList * Update unit tests * Formatting * Update betterer * Prettier * Logs panel: update test * LogLabels: add actual tooltip * Logs: remove sorting of displayed fields --- .betterer.results | 6 +- .../app/features/explore/Logs/LogsMetaRow.tsx | 18 +++-- .../logs/components/LogLabels.test.tsx | 22 ++++-- .../features/logs/components/LogLabels.tsx | 71 +++++++++++++++---- .../app/plugins/panel/logs/LogsPanel.test.tsx | 4 +- public/app/plugins/panel/logs/LogsPanel.tsx | 7 +- 6 files changed, 93 insertions(+), 35 deletions(-) diff --git a/.betterer.results b/.betterer.results index e861dbe1415..4018a113db2 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3276,8 +3276,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "public/app/features/explore/Logs/LogsMetaRow.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] + [0, 0, 0, "Styles should be written using objects.", "0"] ], "public/app/features/explore/Logs/LogsNavigation.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"], @@ -6019,9 +6018,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "6"], [0, 0, 0, "Styles should be written using objects.", "7"] ], - "public/app/plugins/panel/logs/LogsPanel.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] - ], "public/app/plugins/panel/logs/types.ts:5381": [ [0, 0, 0, "Do not re-export imported variable (\`./panelcfg.gen\`)", "0"] ], diff --git a/public/app/features/explore/Logs/LogsMetaRow.tsx b/public/app/features/explore/Logs/LogsMetaRow.tsx index 55f5bdacc02..b6608f69b20 100644 --- a/public/app/features/explore/Logs/LogsMetaRow.tsx +++ b/public/app/features/explore/Logs/LogsMetaRow.tsx @@ -13,13 +13,14 @@ import { transformDataFrame, DataTransformerConfig, CustomTransformOperator, + Labels, } from '@grafana/data'; import { DataFrame } from '@grafana/data/'; import { reportInteraction } from '@grafana/runtime'; import { Button, Dropdown, Menu, ToolbarButton, Tooltip, useStyles2 } from '@grafana/ui'; import { downloadDataFrameAsCsv, downloadLogsModelAsTxt } from '../../inspector/utils/download'; -import { LogLabels } from '../../logs/components/LogLabels'; +import { LogLabels, LogLabelsList } from '../../logs/components/LogLabels'; import { MAX_CHARACTERS } from '../../logs/components/LogRowMessage'; import { logRowsToReadableJson } from '../../logs/utils'; import { MetaInfoText, MetaItemProps } from '../MetaInfoText'; @@ -133,7 +134,7 @@ export const LogsMetaRow = React.memo( logsMetaItem.push( { label: 'Showing only selected fields', - value: renderMetaItem(displayedFields, LogsMetaKind.LabelsMap), + value: , }, { label: '', @@ -195,11 +196,16 @@ export const LogsMetaRow = React.memo( LogsMetaRow.displayName = 'LogsMetaRow'; -function renderMetaItem(value: any, kind: LogsMetaKind) { +function renderMetaItem(value: string | number | Labels, kind: LogsMetaKind) { + if (typeof value === 'string' || typeof value === 'number') { + return <>{value}; + } if (kind === LogsMetaKind.LabelsMap) { return ; - } else if (kind === LogsMetaKind.Error) { - return {value}; } - return value; + if (kind === LogsMetaKind.Error) { + return {value.toString()}; + } + console.error(`Meta type ${typeof value} ${value} not recognized.`); + return <>; } diff --git a/public/app/features/logs/components/LogLabels.test.tsx b/public/app/features/logs/components/LogLabels.test.tsx index e76459f0b65..9bfd070d9a7 100644 --- a/public/app/features/logs/components/LogLabels.test.tsx +++ b/public/app/features/logs/components/LogLabels.test.tsx @@ -1,27 +1,35 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; -import { LogLabels } from './LogLabels'; +import { LogLabels, LogLabelsList } from './LogLabels'; describe('', () => { it('renders notice when no labels are found', () => { - render(); + render(); expect(screen.queryByText('(no unique labels)')).toBeInTheDocument(); }); it('renders labels', () => { render(); - expect(screen.queryByText('bar')).toBeInTheDocument(); - expect(screen.queryByText('42')).toBeInTheDocument(); + expect(screen.queryByText('foo=bar')).toBeInTheDocument(); + expect(screen.queryByText('baz=42')).toBeInTheDocument(); }); it('excludes labels with certain names or labels starting with underscore', () => { render(); - expect(screen.queryByText('bar')).toBeInTheDocument(); - expect(screen.queryByText('42')).not.toBeInTheDocument(); + expect(screen.queryByText('foo=bar')).toBeInTheDocument(); + expect(screen.queryByText('level=42')).not.toBeInTheDocument(); expect(screen.queryByText('13')).not.toBeInTheDocument(); }); it('excludes labels with empty string values', () => { render(); + expect(screen.queryByText('foo=bar')).toBeInTheDocument(); + expect(screen.queryByText(/baz/)).not.toBeInTheDocument(); + }); +}); + +describe('', () => { + it('renders labels', () => { + render(); expect(screen.queryByText('bar')).toBeInTheDocument(); - expect(screen.queryByText('baz')).not.toBeInTheDocument(); + expect(screen.queryByText('42')).toBeInTheDocument(); }); }); diff --git a/public/app/features/logs/components/LogLabels.tsx b/public/app/features/logs/components/LogLabels.tsx index 3f76fda46dd..2305ad7e866 100644 --- a/public/app/features/logs/components/LogLabels.tsx +++ b/public/app/features/logs/components/LogLabels.tsx @@ -1,47 +1,90 @@ import { css, cx } from '@emotion/css'; -import React from 'react'; +import React, { useMemo } from 'react'; import { GrafanaTheme2, Labels } from '@grafana/data'; -import { useStyles2 } from '@grafana/ui'; +import { Tooltip, useStyles2 } from '@grafana/ui'; // Levels are already encoded in color, filename is a Loki-ism const HIDDEN_LABELS = ['level', 'lvl', 'filename']; interface Props { labels: Labels; + emptyMessage?: string; } -export const LogLabels = ({ labels }: Props) => { +export const LogLabels = React.memo(({ labels, emptyMessage }: Props) => { const styles = useStyles2(getStyles); - const displayLabels = Object.keys(labels).filter((label) => !label.startsWith('_') && !HIDDEN_LABELS.includes(label)); + const displayLabels = useMemo( + () => + Object.keys(labels) + .filter((label) => !label.startsWith('_') && !HIDDEN_LABELS.includes(label)) + .sort(), + [labels] + ); - if (displayLabels.length === 0) { + if (displayLabels.length === 0 && emptyMessage) { return ( - (no unique labels) + {emptyMessage} ); } return ( - {displayLabels.sort().map((label) => { + {displayLabels.map((label) => { const value = labels[label]; if (!value) { return; } - const tooltip = `${label}: ${value}`; + const labelValue = `${label}=${value}`; return ( - - - {value} - - + + {labelValue} + ); })} ); -}; +}); +LogLabels.displayName = 'LogLabels'; + +interface LogLabelsArrayProps { + labels: string[]; +} + +export const LogLabelsList = React.memo(({ labels }: LogLabelsArrayProps) => { + const styles = useStyles2(getStyles); + return ( + + {labels.map((label) => ( + + {label} + + ))} + + ); +}); +LogLabelsList.displayName = 'LogLabelsList'; + +interface LogLabelProps { + styles: Record; + tooltip?: string; + children: JSX.Element | string; +} + +const LogLabel = React.forwardRef( + ({ styles, tooltip, children }: LogLabelProps, ref) => { + return ( + + + {children} + + + ); + } +); +LogLabel.displayName = 'LogLabel'; const getStyles = (theme: GrafanaTheme2) => { return { diff --git a/public/app/plugins/panel/logs/LogsPanel.test.tsx b/public/app/plugins/panel/logs/LogsPanel.test.tsx index 7c3fdbf0726..2230f70dae1 100644 --- a/public/app/plugins/panel/logs/LogsPanel.test.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.test.tsx @@ -87,7 +87,7 @@ describe('LogsPanel', () => { options: { showCommonLabels: true, sortOrder: LogsSortOrder.Descending }, }); expect(await screen.findByText(/common labels:/i)).toBeInTheDocument(); - expect(container.firstChild?.childNodes[0].textContent).toMatch(/^Common labels:common_appcommon_job/); + expect(container.firstChild?.childNodes[0].textContent).toMatch(/^Common labels:app=common_appjob=common_job/); }); it('shows common labels on bottom when ascending sort order', async () => { const { container } = setup({ @@ -95,7 +95,7 @@ describe('LogsPanel', () => { options: { showCommonLabels: true, sortOrder: LogsSortOrder.Ascending }, }); expect(await screen.findByText(/common labels:/i)).toBeInTheDocument(); - expect(container.firstChild?.childNodes[0].textContent).toMatch(/Common labels:common_appcommon_job$/); + expect(container.firstChild?.childNodes[0].textContent).toMatch(/Common labels:app=common_appjob=common_job$/); }); it('does not show common labels when showCommonLabels is set to false', async () => { setup({ data: { series: seriesWithCommonLabels }, options: { showCommonLabels: false } }); diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx index 8069088e533..40aa8dab3f0 100644 --- a/public/app/plugins/panel/logs/LogsPanel.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.tsx @@ -39,6 +39,8 @@ interface LogsPermalinkUrlState { }; } +const noCommonLabels: Labels = {}; + export const LogsPanel = ({ data, timeZone, @@ -225,7 +227,10 @@ export const LogsPanel = ({ const renderCommonLabels = () => (
Common labels: - +
); From 28a683cf28c24bd88dd56d3427665d62a13f900f Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Thu, 18 Apr 2024 16:29:27 +0200 Subject: [PATCH 05/21] InfluxDB: Remove influxdbSqlSupport feature toggle (#86518) Remove influxdbSqlSupport feature toggle --- .../configure-grafana/feature-toggles/index.md | 1 - .../grafana-data/src/types/featureToggles.gen.ts | 1 - pkg/services/featuremgmt/registry.go | 10 ---------- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/services/featuremgmt/toggles_gen.json | 3 ++- .../components/editor/config/ConfigEditor.tsx | 13 ++++--------- 7 files changed, 6 insertions(+), 27 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index dadf12020cc..16f94bbdef4 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -43,7 +43,6 @@ Some features are enabled by default. You can disable these feature by setting t | `traceQLStreaming` | Enables response streaming of TraceQL queries of the Tempo data source | | | `awsAsyncQueryCaching` | Enable caching for async queries for Redshift and Athena. Requires that the datasource has caching and async query support enabled | Yes | | `prometheusConfigOverhaulAuth` | Update the Prometheus configuration page with the new auth component | Yes | -| `influxdbSqlSupport` | Enable InfluxDB SQL query language support with new querying UI | Yes | | `alertingNoDataErrorExecution` | Changes how Alerting state manager handles execution of NoData/Error | Yes | | `angularDeprecationUI` | Display Angular warnings in dashboards and panels | Yes | | `dashgpt` | Enable AI powered features in dashboards | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index a670448deb8..ee22f554a83 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -101,7 +101,6 @@ export interface FeatureToggles { permissionsFilterRemoveSubquery?: boolean; prometheusConfigOverhaulAuth?: boolean; configurableSchedulerTick?: boolean; - influxdbSqlSupport?: boolean; alertingNoDataErrorExecution?: boolean; angularDeprecationUI?: boolean; dashgpt?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index d4834ab39ce..9ecc6f8cf26 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -620,16 +620,6 @@ var ( RequiresRestart: true, HideFromDocs: true, }, - { - Name: "influxdbSqlSupport", - Description: "Enable InfluxDB SQL query language support with new querying UI", - Stage: FeatureStageGeneralAvailability, - FrontendOnly: false, - Owner: grafanaObservabilityMetricsSquad, - RequiresRestart: true, - AllowSelfServe: true, - Expression: "true", // enabled by default - }, { Name: "alertingNoDataErrorExecution", Description: "Changes how Alerting state manager handles execution of NoData/Error", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 79355f63171..6714b969fc1 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -82,7 +82,6 @@ awsAsyncQueryCaching,GA,@grafana/aws-datasources,false,false,false permissionsFilterRemoveSubquery,experimental,@grafana/backend-platform,false,false,false prometheusConfigOverhaulAuth,GA,@grafana/observability-metrics,false,false,false configurableSchedulerTick,experimental,@grafana/alerting-squad,false,true,false -influxdbSqlSupport,GA,@grafana/observability-metrics,false,true,false alertingNoDataErrorExecution,GA,@grafana/alerting-squad,false,true,false angularDeprecationUI,GA,@grafana/plugins-platform-backend,false,false,true dashgpt,GA,@grafana/dashboards-squad,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 389557e105a..0b387c29b96 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -339,10 +339,6 @@ const ( // Enable changing the scheduler base interval via configuration option unified_alerting.scheduler_tick_interval FlagConfigurableSchedulerTick = "configurableSchedulerTick" - // FlagInfluxdbSqlSupport - // Enable InfluxDB SQL query language support with new querying UI - FlagInfluxdbSqlSupport = "influxdbSqlSupport" - // FlagAlertingNoDataErrorExecution // Changes how Alerting state manager handles execution of NoData/Error FlagAlertingNoDataErrorExecution = "alertingNoDataErrorExecution" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 387c6e40226..0f30ede4d7b 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -977,7 +977,8 @@ "metadata": { "name": "influxdbSqlSupport", "resourceVersion": "1712639261786", - "creationTimestamp": "2024-04-09T05:07:41Z" + "creationTimestamp": "2024-04-09T05:07:41Z", + "deletionTimestamp": "2024-04-18T12:35:02Z" }, "spec": { "description": "Enable InfluxDB SQL query language support with new querying UI", diff --git a/public/app/plugins/datasource/influxdb/components/editor/config/ConfigEditor.tsx b/public/app/plugins/datasource/influxdb/components/editor/config/ConfigEditor.tsx index 70926e18d80..f93f5c8e2af 100644 --- a/public/app/plugins/datasource/influxdb/components/editor/config/ConfigEditor.tsx +++ b/public/app/plugins/datasource/influxdb/components/editor/config/ConfigEditor.tsx @@ -6,9 +6,9 @@ import { DataSourceSettings, SelectableValue, updateDatasourcePluginJsonDataOption, -} from '@grafana/data/src'; -import { Alert, DataSourceHttpSettings, InlineField, Select, Field, Input, FieldSet } from '@grafana/ui/src'; -import { config } from 'app/core/config'; +} from '@grafana/data'; +import { config } from '@grafana/runtime'; +import { Alert, DataSourceHttpSettings, InlineField, Select, Field, Input, FieldSet } from '@grafana/ui'; import { BROWSER_MODE_DISABLED_MESSAGE } from '../../../constants'; import { InfluxOptions, InfluxOptionsV1, InfluxVersion } from '../../../types'; @@ -36,11 +36,6 @@ const versionMap: Record> = { }; const versions: Array> = [ - versionMap[InfluxVersion.InfluxQL], - versionMap[InfluxVersion.Flux], -]; - -const versionsWithSQL: Array> = [ versionMap[InfluxVersion.InfluxQL], versionMap[InfluxVersion.SQL], versionMap[InfluxVersion.Flux], @@ -119,7 +114,7 @@ export class ConfigEditor extends PureComponent { aria-label="Query language" className="width-30" value={versionMap[options.jsonData.version ?? InfluxVersion.InfluxQL]} - options={config.featureToggles.influxdbSqlSupport ? versionsWithSQL : versions} + options={versions} defaultValue={versionMap[InfluxVersion.InfluxQL]} onChange={this.onVersionChanged} /> From 18bd11ca6a83e3e5b56cc97dbdeb19b464974882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Thu, 18 Apr 2024 16:37:48 +0200 Subject: [PATCH 06/21] Grafana UI: `TextLink.story.tsx` - Replace `VerticalGroup` with `Stack` (#86526) --- .betterer.results | 3 --- .../grafana-ui/src/components/Link/TextLink.story.tsx | 8 ++++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.betterer.results b/.betterer.results index 4018a113db2..ec6320fa7de 100644 --- a/.betterer.results +++ b/.betterer.results @@ -829,9 +829,6 @@ exports[`better eslint`] = { [0, 0, 0, "\'VerticalGroup\' import from \'@grafana/ui\' is restricted from being used by a pattern. Use Stack component instead.", "0"], [0, 0, 0, "\'HorizontalGroup\' import from \'@grafana/ui\' is restricted from being used by a pattern. Use Stack component instead.", "1"] ], - "packages/grafana-ui/src/components/Link/TextLink.story.tsx:5381": [ - [0, 0, 0, "\'VerticalGroup\' import from \'../Layout/Layout\' is restricted from being used by a pattern. Use Stack component instead.", "0"] - ], "packages/grafana-ui/src/components/MatchersUI/FieldValueMatcher.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] ], diff --git a/packages/grafana-ui/src/components/Link/TextLink.story.tsx b/packages/grafana-ui/src/components/Link/TextLink.story.tsx index deeed326145..65a0238f21e 100644 --- a/packages/grafana-ui/src/components/Link/TextLink.story.tsx +++ b/packages/grafana-ui/src/components/Link/TextLink.story.tsx @@ -2,7 +2,7 @@ import { Meta, StoryFn } from '@storybook/react'; import React from 'react'; import { StoryExample } from '../../utils/storybook/StoryExample'; -import { VerticalGroup } from '../Layout/Layout'; +import { Stack } from '../Layout/Stack/Stack'; import { Text } from '../Text/Text'; import { TextLink } from './TextLink'; @@ -41,7 +41,7 @@ const meta: Meta = { export const Example: StoryFn = (args) => { return ( - + To get started with a forever free Grafana Cloud account, sign up at   @@ -52,7 +52,7 @@ export const Example: StoryFn = (args) => { - + Learn how in the docs @@ -60,7 +60,7 @@ export const Example: StoryFn = (args) => { *The examples cannot contemplate an internal link due to conflicts between Storybook and React Router - + ); }; From 0d11f9b2f4bd3dbe01a2a08d139d6c13d6c3c3c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=B1or=20Performo=20-=20Leandro=20Melendez?= <54183040+srperf@users.noreply.github.com> Date: Thu, 18 Apr 2024 08:57:08 -0600 Subject: [PATCH 07/21] Docs: Add GeoMaps YouTube Video (#86472) * Update index.md on GeoMaps adding YouTube Video Added the GeoMap YouTube video to the documentation * Update docs/sources/panels-visualizations/visualizations/geomap/index.md Totally agree, I tend to use those words and not realize :P Co-authored-by: Isabel Matwawana <76437239+imatwawana@users.noreply.github.com> --------- Co-authored-by: Isabel Matwawana <76437239+imatwawana@users.noreply.github.com> --- .../panels-visualizations/visualizations/geomap/index.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/sources/panels-visualizations/visualizations/geomap/index.md b/docs/sources/panels-visualizations/visualizations/geomap/index.md index 8abc60912e7..2a1efcfda48 100644 --- a/docs/sources/panels-visualizations/visualizations/geomap/index.md +++ b/docs/sources/panels-visualizations/visualizations/geomap/index.md @@ -44,6 +44,10 @@ Geomaps allow you to view and customize the world map using geospatial data. You {{< figure src="/static/img/docs/geomap-panel/geomap-example-8-1-0.png" max-width="1200px" caption="Geomap panel" >}} +The following video provides beginner steps for creating geomap visualizations. You'll learn the data requirements and caveats, special customizations, preconfigured displays and much more: + +{{< youtube id="HwM8AFQ7EUs" >}} + ## Map View The map view controls the initial view of the map when the dashboard loads. From e65669bc0d98bcbd1d0c24f7256b882c57420da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Thu, 18 Apr 2024 17:05:19 +0200 Subject: [PATCH 08/21] Grafana UI: `Checkbox.story.tsx` - Replace `VerticalGroup`with `Stack` (#86524) --- .betterer.results | 3 --- .../src/components/Forms/Checkbox.story.tsx | 24 +++++++------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/.betterer.results b/.betterer.results index ec6320fa7de..650429be0be 100644 --- a/.betterer.results +++ b/.betterer.results @@ -797,9 +797,6 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], - "packages/grafana-ui/src/components/Forms/Checkbox.story.tsx:5381": [ - [0, 0, 0, "\'VerticalGroup\' import from \'../Layout/Layout\' is restricted from being used by a pattern. Use Stack component instead.", "0"] - ], "packages/grafana-ui/src/components/Forms/FieldArray.story.tsx:5381": [ [0, 0, 0, "\'HorizontalGroup\' import from \'@grafana/ui\' is restricted from being used by a pattern. Use Stack component instead.", "0"] ], diff --git a/packages/grafana-ui/src/components/Forms/Checkbox.story.tsx b/packages/grafana-ui/src/components/Forms/Checkbox.story.tsx index cd9e7a3ebd5..6ce2c1907bf 100644 --- a/packages/grafana-ui/src/components/Forms/Checkbox.story.tsx +++ b/packages/grafana-ui/src/components/Forms/Checkbox.story.tsx @@ -1,7 +1,7 @@ import { Meta, StoryFn } from '@storybook/react'; import React, { useState, useCallback } from 'react'; -import { VerticalGroup } from '../Layout/Layout'; +import { Stack } from '../Layout/Stack/Stack'; import { Checkbox } from './Checkbox'; import mdx from './Checkbox.mdx'; @@ -26,11 +26,7 @@ export const Basic: StoryFn = (args) => { (e: React.FormEvent) => setChecked(e.currentTarget.checked), [setChecked] ); - return ( -
- -
- ); + return ; }; Basic.args = { @@ -44,7 +40,7 @@ Basic.args = { export const StackedList = () => { return (
- + { label="Another checkbox times 2" description="Another long description that does not make any sense or does it?" /> - +
); }; export const InAField: StoryFn = (args) => { return ( -
- - - -
+ + + ); }; @@ -93,14 +87,14 @@ export const AllStates: StoryFn = (args) => { return (
- + - +
); }; From 7a147f2ce805c12fbeb388481782e401dbf37b92 Mon Sep 17 00:00:00 2001 From: Alexa V <239999+axelavargas@users.noreply.github.com> Date: Thu, 18 Apr 2024 17:15:18 +0200 Subject: [PATCH 09/21] Dashboard: DashboardPageProxy - Use chaining operators to prevent runtime error (#86507) * Use chaining operators to prevent runtime error * Fix tests --------- Co-authored-by: Ivan Ortega --- .../dashboard/containers/DashboardPageProxy.test.tsx | 11 +++++++++-- .../dashboard/containers/DashboardPageProxy.tsx | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx index 0972c777522..28df23c5a1c 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx @@ -147,7 +147,7 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Home, component: () => null, path: '/' }, - match: { params: {}, isExact: true, path: '/', url: '/' }, + match: { params: { uid: '' }, isExact: true, path: '/', url: '/' }, }); }); @@ -176,7 +176,14 @@ describe('DashboardPageProxy', () => { act(() => { setup({ route: { routeName: DashboardRoutes.Home, component: () => null, path: '/' }, - match: { params: {}, isExact: true, path: '/', url: '/' }, + match: { + params: { + uid: '', + }, + isExact: true, + path: '/', + url: '/', + }, }); }); diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.tsx index b7b70b12f16..b4182e9f7ca 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.tsx @@ -54,7 +54,7 @@ function DashboardPageProxy(props: DashboardPageProxyProps) { return null; } - if (dashboard.value && dashboard.value.dashboard.uid && dashboard.value.dashboard.uid !== props.match.params.uid) { + if (dashboard?.value?.dashboard?.uid !== props.match.params.uid) { return null; } From 272b2e139a3b38828a64aad472e02df929171c67 Mon Sep 17 00:00:00 2001 From: antonio <45235678+tonypowa@users.noreply.github.com> Date: Thu, 18 Apr 2024 17:24:22 +0200 Subject: [PATCH 10/21] email contact point -url fix (#86538) url fix --- .../manage-contact-points/integrations/configure-email.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/sources/alerting/configure-notifications/manage-contact-points/integrations/configure-email.md b/docs/sources/alerting/configure-notifications/manage-contact-points/integrations/configure-email.md index d1b3f89a015..b956d8eb2bd 100644 --- a/docs/sources/alerting/configure-notifications/manage-contact-points/integrations/configure-email.md +++ b/docs/sources/alerting/configure-notifications/manage-contact-points/integrations/configure-email.md @@ -95,6 +95,4 @@ If you have more than one contact point, add a new child notification policy rat [smtp-settings]: "/docs/grafana/ -> /docs/grafana/latest/setup-grafana/configure-grafana/#smtp" -[smtp-settings]: "/docs/grafana-cloud/ -> /docs/grafana-cloud/alerting-and-irm/alerting/configure-notifications/manage-contact-points/" - {{% /docs/reference %}} From fe24404432d1bbfb6e469d64599f41dcda8b7f24 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Thu, 18 Apr 2024 16:25:27 +0100 Subject: [PATCH 11/21] I18n: Support for Enterprise translations (#86215) * I18n: Support for Enterprise translations * don't attempt to link to enterprise in tests * move extract script to makefile to optionally support enterprise * update references to old extract script * update docs * thank god for unit tests --- .drone.yml | 10 +-- .prettierignore | 4 -- Makefile | 19 ++++++ contribute/internationalization.md | 7 ++- package.json | 3 - .../core/internationalization/constants.ts | 62 ++++++++++++++---- .../app/core/internationalization/index.tsx | 21 ++++--- .../internationalization/loadTranslations.ts | 11 +++- .../i18next-parser-enterprise.config.cjs | 8 +++ public/locales/i18next-parser.config.cjs | 10 ++- public/locales/pseudo.js | 24 ------- public/locales/pseudo.mjs | 63 +++++++++++++++++++ scripts/drone/steps/lib.star | 4 +- yarn.lock | 6 +- 14 files changed, 183 insertions(+), 69 deletions(-) create mode 100644 public/locales/i18next-parser-enterprise.config.cjs delete mode 100644 public/locales/pseudo.js create mode 100644 public/locales/pseudo.mjs diff --git a/.drone.yml b/.drone.yml index 6f896b5526f..9f6ba848ad2 100644 --- a/.drone.yml +++ b/.drone.yml @@ -243,11 +243,11 @@ steps: - commands: - apk add --update git - |- - yarn run i18n:extract || (echo " + make i18n-extract || (echo " Extraction failed. Make sure that you have no dynamic translation phrases, such as 't(\`preferences.theme.\$${themeID}\`, themeName)' and that no translation key is used twice. Search the output for '[warning]' to find the offending file." && false) - "\n file_diff=$(git diff --dirstat public/locales)\n if [ -n \"$file_diff\" ]; then\n echo $file_diff\n echo - \"\nTranslation extraction has not been committed. Please run 'yarn i18n:extract', + \"\nTranslation extraction has not been committed. Please run 'make i18n-extract', commit the changes and push again.\"\n exit 1\n fi\n \ " depends_on: @@ -1627,11 +1627,11 @@ steps: - commands: - apk add --update git - |- - yarn run i18n:extract || (echo " + make i18n-extract || (echo " Extraction failed. Make sure that you have no dynamic translation phrases, such as 't(\`preferences.theme.\$${themeID}\`, themeName)' and that no translation key is used twice. Search the output for '[warning]' to find the offending file." && false) - "\n file_diff=$(git diff --dirstat public/locales)\n if [ -n \"$file_diff\" ]; then\n echo $file_diff\n echo - \"\nTranslation extraction has not been committed. Please run 'yarn i18n:extract', + \"\nTranslation extraction has not been committed. Please run 'make i18n-extract', commit the changes and push again.\"\n exit 1\n fi\n \ " depends_on: @@ -4925,6 +4925,6 @@ kind: secret name: gcr_credentials --- kind: signature -hmac: fbd59890dac44eb6fb34562f2b2b2db0fc0a8a50d451f9887f815ee35757e0f6 +hmac: 958ed40ca0620498c01fa867b05a1d6c3bcd908067fbb34be691923e95cfc77b ... diff --git a/.prettierignore b/.prettierignore index 160926fd718..76bf2adf696 100644 --- a/.prettierignore +++ b/.prettierignore @@ -19,10 +19,6 @@ vendor # TS generate from cue by cuetsy **/*.gen.ts -# Auto-generated internationalization files -public/locales/_build/ -public/locales/**/*.js - # Auto-generated theme files theme.light.generated.json theme.dark.generated.json diff --git a/Makefile b/Makefile index bd6bc808bb4..0d47a3d0215 100644 --- a/Makefile +++ b/Makefile @@ -110,6 +110,25 @@ OAPI_SPEC_TARGET = public/openapi3.json openapi3-gen: swagger-gen ## Generates OpenApi 3 specs from the Swagger 2 already generated $(GO) run scripts/openapi3/openapi3conv.go $(MERGED_SPEC_TARGET) $(OAPI_SPEC_TARGET) +##@ Internationalisation +.PHONY: i18n-extract-enterprise +ENTERPRISE_FE_EXT_FILE = public/app/extensions/index.ts +ifeq ("$(wildcard $(ENTERPRISE_FE_EXT_FILE))","") ## if enterprise is not enabled +i18n-extract-enterprise: + @echo "Skipping i18n extract for Enterprise: not enabled" +else +i18n-extract-enterprise: $(SWAGGER) ## Generate API Swagger specification + @echo "Extracting i18n strings for Enterprise" + yarn run i18next --config public/locales/i18next-parser-enterprise.config.cjs + node ./public/locales/pseudo.mjs --mode enterprise +endif + +.PHONY: i18n-extract +i18n-extract: i18n-extract-enterprise + @echo "Extracting i18n strings for OSS" + yarn run i18next --config public/locales/i18next-parser.config.cjs + node ./public/locales/pseudo.mjs --mode oss + ##@ Building .PHONY: gen-cue gen-cue: ## Do all CUE/Thema code generation diff --git a/contribute/internationalization.md b/contribute/internationalization.md index 7a0ac2a249f..577d0188eb6 100644 --- a/contribute/internationalization.md +++ b/contribute/internationalization.md @@ -9,7 +9,7 @@ Grafana uses the [i18next](https://www.i18next.com/) framework for managing tran - Use `Go to {{ pageTitle }}` in code to add a translatable phrase - Translations are stored in JSON files in `public/locales/{locale}/grafana.json` - If a particular phrase is not available in the a language then it will fall back to English -- To update phrases in English, edit the default phrase in the component's source and then run `yarn i18n:extract`. +- To update phrases in English, edit the default phrase in the component's source and then run `make i18n-extract`. - The single source of truth for en-US (fallback language) is in grafana/grafana, the single source of truth for any translated language is Crowdin - To update phrases in any translated language, edit the phrase in Crowdin. Do not edit the `{locale}/grafana.json` @@ -41,7 +41,7 @@ const ErrorMessage = ({ id, message }) => There 2. Upon reload, the default English phrase will appear on the page. -3. Before submitting your PR, run the `yarn i18n:extract` command to extract the messages you added into the `public/locales/en-US/grafana.json` file and make them available for translation. +3. Before submitting your PR, run the `make i18n-extract` command to extract the messages you added into the `public/locales/en-US/grafana.json` file and make them available for translation. **Note:** All other languages will receive their translations when they are ready to be downloaded from Crowdin. ### Plain JS usage @@ -79,6 +79,7 @@ While the `t` function can technically be used outside of React functions (e.g, 1. Add a new constant for the new language 2. Add the new constant to the `LOCALES` array 3. Create a PR with the changes and merge when you are ready to release the new language (probably wait until we have translations for it) +4. In the Enterprise repo, update `src/public/locales/localeExtensions.ts` ## How translations work in Grafana @@ -186,7 +187,7 @@ import { t } from 'app/core/internationalization'; const translatedString = t('inbox.heading', 'You got {{count}} message', { count: messages.length }); ``` -Once extracted with `yarn i18n:extract` you will need to manually edit the [English grafana.json message catalogue](../public/locales/en-US/grafana.json) to correct the plural forms. See the [react-i18next docs](https://react.i18next.com/latest/trans-component#plural) for more details. +Once extracted with `make i18n-extract` you will need to manually edit the [English grafana.json message catalogue](../public/locales/en-US/grafana.json) to correct the plural forms. See the [react-i18next docs](https://react.i18next.com/latest/trans-component#plural) for more details. ```json { diff --git a/package.json b/package.json index 5a182619e3a..4c9bc730654 100644 --- a/package.json +++ b/package.json @@ -48,9 +48,6 @@ "plugins:build-bundled": "find plugins-bundled -name package.json -not -path '*/node_modules/*' -execdir yarn build \\;", "watch": "yarn start -d watch,start core:start --watchTheme", "ci:test-frontend": "yarn run test:ci", - "i18n:clean": "rimraf public/locales/en-US/grafana.json", - "i18n:extract": "yarn run i18next -c public/locales/i18next-parser.config.cjs 'public/**/*.{tsx,ts}' 'packages/grafana-ui/**/*.{tsx,ts}' && yarn i18n:pseudo", - "i18n:pseudo": "node ./public/locales/pseudo.js", "i18n:stats": "node ./scripts/cli/reportI18nStats.mjs", "betterer": "betterer", "betterer:json": "ts-node --transpile-only --project ./scripts/cli/tsconfig.json ./scripts/cli/bettererResultsToJson.ts", diff --git a/public/app/core/internationalization/constants.ts b/public/app/core/internationalization/constants.ts index b367bb57a44..a241519fed4 100644 --- a/public/app/core/internationalization/constants.ts +++ b/public/app/core/internationalization/constants.ts @@ -1,4 +1,5 @@ import { ResourceKey } from 'i18next'; +import { uniq } from 'lodash'; export const ENGLISH_US = 'en-US'; export const FRENCH_FRANCE = 'fr-FR'; @@ -10,7 +11,9 @@ export const PSEUDO_LOCALE = 'pseudo-LOCALE'; export const DEFAULT_LANGUAGE = ENGLISH_US; -interface LanguageDefinitions { +export type LocaleFileLoader = () => Promise; + +export interface LanguageDefinition { /** IETF language tag for the language e.g. en-US */ code: string; @@ -18,53 +21,90 @@ interface LanguageDefinitions { name: string; /** Function to load translations */ - loader: () => Promise; + loader: Record; } -export const LANGUAGES: LanguageDefinitions[] = [ +export const LANGUAGES: LanguageDefinition[] = [ { code: ENGLISH_US, name: 'English', - loader: () => import('../../../locales/en-US/grafana.json'), + loader: { + grafana: () => import('../../../locales/en-US/grafana.json'), + }, }, { code: FRENCH_FRANCE, name: 'Français', - loader: () => import('../../../locales/fr-FR/grafana.json'), + loader: { + grafana: () => import('../../../locales/fr-FR/grafana.json'), + }, }, { code: SPANISH_SPAIN, name: 'Español', - loader: () => import('../../../locales/es-ES/grafana.json'), + loader: { + grafana: () => import('../../../locales/es-ES/grafana.json'), + }, }, { code: GERMAN_GERMANY, name: 'Deutsch', - loader: () => import('../../../locales/de-DE/grafana.json'), + loader: { + grafana: () => import('../../../locales/de-DE/grafana.json'), + }, }, { code: CHINESE_SIMPLIFIED, name: '中文（简体）', - loader: () => import('../../../locales/zh-Hans/grafana.json'), + loader: { + grafana: () => import('../../../locales/zh-Hans/grafana.json'), + }, }, { code: BRAZILIAN_PORTUGUESE, name: 'Português Brasileiro', - loader: () => import('../../../locales/pt-BR/grafana.json'), + loader: { + grafana: () => import('../../../locales/pt-BR/grafana.json'), + }, }, -]; +] satisfies Array>; if (process.env.NODE_ENV === 'development') { LANGUAGES.push({ code: PSEUDO_LOCALE, name: 'Pseudo-locale', - loader: () => import('../../../locales/pseudo-LOCALE/grafana.json'), + loader: { + grafana: () => import('../../../locales/pseudo-LOCALE/grafana.json'), + }, }); } +// Optionally load enterprise locale extensions, if they are present. +// It is important that this happens before NAMESPACES is defined so it has the correct value +// +// require.context doesn't work in jest, so we don't even attempt to load enterprise translations... +if (process.env.NODE_ENV !== 'test') { + const extensionRequireContext = require.context('../../', true, /app\/extensions\/locales\/localeExtensions/); + if (extensionRequireContext.keys().includes('app/extensions/locales/localeExtensions')) { + const { LOCALE_EXTENSIONS, ENTERPRISE_I18N_NAMESPACE } = extensionRequireContext( + 'app/extensions/locales/localeExtensions' + ); + + for (const language of LANGUAGES) { + const localeLoader = LOCALE_EXTENSIONS[language.code]; + + if (localeLoader) { + language.loader[ENTERPRISE_I18N_NAMESPACE] = localeLoader; + } + } + } +} + export const VALID_LANGUAGES = LANGUAGES.map((v) => v.code); + +export const NAMESPACES = uniq(LANGUAGES.flatMap((v) => Object.keys(v.loader))); diff --git a/public/app/core/internationalization/index.tsx b/public/app/core/internationalization/index.tsx index 4d3e4bf047d..812fc4f83b7 100644 --- a/public/app/core/internationalization/index.tsx +++ b/public/app/core/internationalization/index.tsx @@ -3,12 +3,12 @@ import LanguageDetector, { DetectorOptions } from 'i18next-browser-languagedetec import React from 'react'; import { Trans as I18NextTrans, initReactI18next } from 'react-i18next'; // eslint-disable-line no-restricted-imports -import { DEFAULT_LANGUAGE, VALID_LANGUAGES } from './constants'; +import { DEFAULT_LANGUAGE, NAMESPACES, VALID_LANGUAGES } from './constants'; import { loadTranslations } from './loadTranslations'; let tFunc: TFunction | undefined; -export function initializeI18n(language: string): Promise<{ language: string | undefined }> { +export async function initializeI18n(language: string): Promise<{ language: string | undefined }> { // This is a placeholder so we can put a 'comment' in the message json files. // Starts with an underscore so it's sorted to the top of the file. Even though it is in a comment the following line is still extracted // t('_comment', 'The code is the source of truth for English phrases. They should be updated in the components directly, and additional plurals specified in this file.'); @@ -24,7 +24,10 @@ export function initializeI18n(language: string): Promise<{ language: string | u // Required to ensure that `resolvedLanguage` is set property when an invalid language is passed (such as through 'detect') supportedLngs: VALID_LANGUAGES, fallbackLng: DEFAULT_LANGUAGE, + + ns: NAMESPACES, }; + let i18nInstance = i18n; if (language === 'detect') { i18nInstance = i18nInstance.use(LanguageDetector); @@ -39,13 +42,13 @@ export function initializeI18n(language: string): Promise<{ language: string | u .use(initReactI18next) // passes i18n down to react-i18next .init(options); - tFunc = i18n.t; + await loadPromise; - return loadPromise.then(() => { - return { - language: i18nInstance.resolvedLanguage, - }; - }); + tFunc = i18n.getFixedT(null, NAMESPACES); + + return { + language: i18nInstance.resolvedLanguage, + }; } export function changeLanguage(locale: string) { @@ -54,7 +57,7 @@ export function changeLanguage(locale: string) { } export const Trans: typeof I18NextTrans = (props) => { - return ; + return ; }; // Wrap t() to provide default namespaces and enforce a consistent API diff --git a/public/app/core/internationalization/loadTranslations.ts b/public/app/core/internationalization/loadTranslations.ts index 3e618027ee4..301261fca6d 100644 --- a/public/app/core/internationalization/loadTranslations.ts +++ b/public/app/core/internationalization/loadTranslations.ts @@ -12,10 +12,17 @@ export const loadTranslations: BackendModule = { if (!localeDef) { localeDef = LANGUAGES.find((v) => getLanguagePartFromCode(v.code) === getLanguagePartFromCode(language)); } + if (!localeDef) { - return callback(new Error('No message loader available for ' + language), null); + return callback(new Error(`No message loader available for ${language}`), null); } - const messages = await localeDef.loader(); + + const namespaceLoader = localeDef.loader[namespace]; + if (!namespaceLoader) { + return callback(new Error(`No message loader available for ${language} with namespace ${namespace}`), null); + } + + const messages = await namespaceLoader(); callback(null, messages); }, }; diff --git a/public/locales/i18next-parser-enterprise.config.cjs b/public/locales/i18next-parser-enterprise.config.cjs new file mode 100644 index 00000000000..66e8cd9cca2 --- /dev/null +++ b/public/locales/i18next-parser-enterprise.config.cjs @@ -0,0 +1,8 @@ +const baseConfig = require('./i18next-parser.config.cjs'); + +module.exports = { + ...baseConfig, + defaultNamespace: 'grafana-enterprise', + input: ['../../public/app/extensions/**/*.{tsx,ts}'], + output: './public/app/extensions/locales/$LOCALE/$NAMESPACE.json', +}; diff --git a/public/locales/i18next-parser.config.cjs b/public/locales/i18next-parser.config.cjs index 2aeca9d3745..f6c47aa7cc1 100644 --- a/public/locales/i18next-parser.config.cjs +++ b/public/locales/i18next-parser.config.cjs @@ -1,6 +1,6 @@ module.exports = { - // Base config - locales: ['en-US'], // Only en-US is updated - Crowdin will PR with other languages + // Base config - same for both OSS and Enterprise + locales: ['en-US'], // Only en-US is updated - Crowdin will PR with other languages sort: true, createOldCatalogs: false, failOnWarnings: true, @@ -9,6 +9,10 @@ module.exports = { // OSS-specific config defaultNamespace: 'grafana', - input: ['../../public/**/*.{tsx,ts}', '!../../public/app/extensions/**/*', '../../packages/grafana-ui/**/*.{tsx,ts}'], + input: [ + '../../public/**/*.{tsx,ts}', + '!../../public/app/extensions/**/*', // Don't extract from Enterprise + '../../packages/grafana-ui/**/*.{tsx,ts}', + ], output: './public/locales/$LOCALE/$NAMESPACE.json', }; diff --git a/public/locales/pseudo.js b/public/locales/pseudo.js deleted file mode 100644 index 4bcfef5df05..00000000000 --- a/public/locales/pseudo.js +++ /dev/null @@ -1,24 +0,0 @@ -const fs = require('fs/promises'); -const pseudoizer = require('pseudoizer'); -const prettier = require('prettier'); - -function pseudoizeJsonReplacer(key, value) { - if (typeof value === 'string') { - // Split string on brace-enclosed segments. Odd indices will be {{variables}} - const phraseParts = value.split(/(\{\{[^}]+}\})/g); - const translatedParts = phraseParts.map((str, index) => index % 2 ? str : pseudoizer.pseudoize(str)) - return translatedParts.join("") - } - - return value; -} - -fs.readFile('./public/locales/en-US/grafana.json').then(async (enJson) => { - const enMessages = JSON.parse(enJson); - // Add newline to make prettier happy - const pseudoJson = await prettier.format(JSON.stringify(enMessages, pseudoizeJsonReplacer, 2), { - parser: 'json', - }); - - return fs.writeFile('./public/locales/pseudo-LOCALE/grafana.json', pseudoJson); -}); diff --git a/public/locales/pseudo.mjs b/public/locales/pseudo.mjs new file mode 100644 index 00000000000..05f3f89626d --- /dev/null +++ b/public/locales/pseudo.mjs @@ -0,0 +1,63 @@ +// @ts-check +import { readFile, writeFile } from 'fs/promises'; +import { format } from 'prettier'; +import { pseudoize } from 'pseudoizer'; +import { hideBin } from 'yargs/helpers'; +import yargs from 'yargs/yargs'; + +const argv = await yargs(hideBin(process.argv)) + .option('mode', { + demandOption: true, + describe: 'Path to a template to use for each issue. See source bettererIssueTemplate.md for an example', + type: 'string', + choices: ['oss', 'enterprise', 'both'], + }) + .version(false).argv; + +const extractOSS = ['oss', 'both'].includes(argv.mode); +const extractEnterprise = ['enterprise', 'both'].includes(argv.mode); + +/** + * @param {string} key + * @param {unknown} value + */ +function pseudoizeJsonReplacer(key, value) { + if (typeof value === 'string') { + // Split string on brace-enclosed segments. Odd indices will be {{variables}} + const phraseParts = value.split(/(\{\{[^}]+}\})/g); + const translatedParts = phraseParts.map((str, index) => (index % 2 ? str : pseudoize(str))); + return translatedParts.join(''); + } + + return value; +} +/** + * @param {string} inputPath + * @param {string} outputPath + */ +async function pseudoizeJson(inputPath, outputPath) { + const baseJson = await readFile(inputPath, 'utf-8'); + const enMessages = JSON.parse(baseJson); + const pseudoJson = JSON.stringify(enMessages, pseudoizeJsonReplacer, 2); + const prettyPseudoJson = await format(pseudoJson, { + parser: 'json', + }); + + await writeFile(outputPath, prettyPseudoJson); + console.log('Wrote', outputPath); +} + +// +// OSS translations +if (extractOSS) { + await pseudoizeJson('./public/locales/en-US/grafana.json', './public/locales/pseudo-LOCALE/grafana.json'); +} + +// +// Enterprise translations +if (extractEnterprise) { + await pseudoizeJson( + './public/app/extensions/locales/en-US/grafana-enterprise.json', + './public/app/extensions/locales/pseudo-LOCALE/grafana-enterprise.json' + ); +} diff --git a/scripts/drone/steps/lib.star b/scripts/drone/steps/lib.star index 85d435f7f7c..26070962065 100644 --- a/scripts/drone/steps/lib.star +++ b/scripts/drone/steps/lib.star @@ -641,7 +641,7 @@ def lint_frontend_step(): def verify_i18n_step(): extract_error_message = "\nExtraction failed. Make sure that you have no dynamic translation phrases, such as 't(\\`preferences.theme.\\$${themeID}\\`, themeName)' and that no translation key is used twice. Search the output for '[warning]' to find the offending file." - uncommited_error_message = "\nTranslation extraction has not been committed. Please run 'yarn i18n:extract', commit the changes and push again." + uncommited_error_message = "\nTranslation extraction has not been committed. Please run 'make i18n-extract', commit the changes and push again." return { "name": "verify-i18n", "image": images["node"], @@ -651,7 +651,7 @@ def verify_i18n_step(): "failure": "ignore", "commands": [ "apk add --update git", - "yarn run i18n:extract || (echo \"{}\" && false)".format(extract_error_message), + "make i18n-extract || (echo \"{}\" && false)".format(extract_error_message), # Verify that translation extraction has been committed ''' file_diff=$(git diff --dirstat public/locales) diff --git a/yarn.lock b/yarn.lock index 700b3a5c9bb..39e99715e8d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18365,8 +18365,8 @@ __metadata: linkType: hard "glob-stream@npm:^8.0.0": - version: 8.0.0 - resolution: "glob-stream@npm:8.0.0" + version: 8.0.2 + resolution: "glob-stream@npm:8.0.2" dependencies: "@gulpjs/to-absolute-glob": "npm:^4.0.0" anymatch: "npm:^3.1.3" @@ -18376,7 +18376,7 @@ __metadata: is-negated-glob: "npm:^1.0.0" normalize-path: "npm:^3.0.0" streamx: "npm:^2.12.5" - checksum: 10/b1d18b6fd49086ff02e031f03e3debac747047d304b349a6dced3b7944c665344ef63496363f483acc7c6afcd6ebfb11af1652824f2c370d83c0f3905d5c67e0 + checksum: 10/cda46c02b6313d4a5cd0a3e67c7a2bd477d5f708904dc761c0d6364611f188a303051ec4e0cd405597522c7f7ffbba530f147754b4bf5af9f18e970c024734d8 languageName: node linkType: hard From bfb79d20ffaf4ddef20aa22f24a4c79e8b00b74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Thu, 18 Apr 2024 17:35:36 +0200 Subject: [PATCH 12/21] Grafana UI: `Menu.story.tsx` - Replace `VerticalGroup` with `Stack` (#86529) --- .betterer.results | 3 --- packages/grafana-ui/src/components/Menu/Menu.story.tsx | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.betterer.results b/.betterer.results index 650429be0be..54e1a496e23 100644 --- a/.betterer.results +++ b/.betterer.results @@ -832,9 +832,6 @@ exports[`better eslint`] = { "packages/grafana-ui/src/components/MatchersUI/fieldMatchersUI.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], - "packages/grafana-ui/src/components/Menu/Menu.story.tsx:5381": [ - [0, 0, 0, "\'VerticalGroup\' import from \'../Layout/Layout\' is restricted from being used by a pattern. Use Stack component instead.", "0"] - ], "packages/grafana-ui/src/components/Modal/ModalsContext.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/packages/grafana-ui/src/components/Menu/Menu.story.tsx b/packages/grafana-ui/src/components/Menu/Menu.story.tsx index 4a547fb84ae..edae46c59ad 100644 --- a/packages/grafana-ui/src/components/Menu/Menu.story.tsx +++ b/packages/grafana-ui/src/components/Menu/Menu.story.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { GraphContextMenuHeader } from '..'; import { StoryExample } from '../../utils/storybook/StoryExample'; -import { VerticalGroup } from '../Layout/Layout'; +import { Stack } from '../Layout/Stack/Stack'; import { Menu } from './Menu'; import mdx from './Menu.mdx'; @@ -30,7 +30,7 @@ const meta: Meta = { export function Examples() { return ( - + @@ -167,7 +167,7 @@ export function Examples() { /> - + ); } From 3ab361abfc163a82bb229e903dc61e71402e4321 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Thu, 18 Apr 2024 18:35:38 +0300 Subject: [PATCH 13/21] Chore: Update swagger (#86523) * Chore: Update swagger --- public/api-enterprise-spec.json | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/public/api-enterprise-spec.json b/public/api-enterprise-spec.json index 3f7b9bb95f0..e506c1d1ca6 100644 --- a/public/api-enterprise-spec.json +++ b/public/api-enterprise-spec.json @@ -48,7 +48,7 @@ }, "/access-control/roles": { "get": { - "description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.", + "description": "Gets all existing roles. The response contains all global and organization local roles, for the organization which user is signed in.\n\nYou need to have a permission with action `roles:read` and scope `roles:*`.\n\nThe `delegatable` flag reduces the set of roles to only those for which the signed-in user has permissions to assign.", "tags": [ "access_control", "enterprise" @@ -60,6 +60,11 @@ "type": "boolean", "name": "delegatable", "in": "query" + }, + { + "type": "boolean", + "name": "includeHidden", + "in": "query" } ], "responses": { From 1d1ebee36b2bdafaa14c60ff3b494a64b7b601d1 Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Thu, 18 Apr 2024 09:53:34 -0600 Subject: [PATCH 14/21] RolePicker: Adjust dynamic position (#86424) * Adjust dynamic position --- .../core/components/RolePicker/RolePicker.tsx | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/public/app/core/components/RolePicker/RolePicker.tsx b/public/app/core/components/RolePicker/RolePicker.tsx index 24675c92ce0..fe958e408c6 100644 --- a/public/app/core/components/RolePicker/RolePicker.tsx +++ b/public/app/core/components/RolePicker/RolePicker.tsx @@ -80,15 +80,24 @@ export const RolePicker = ({ } const { bottom, top, left, right } = dimensions; let horizontal = left; - let vertical = bottom + 10; // Add extra 10px to offset to account for border and outline let menuToLeft = false; const distance = window.innerHeight - bottom; - vertical = bottom; + let vertical = bottom + 5; + let ToTheSide = false; if (distance < MENU_MAX_HEIGHT + 20) { - // Limit the top position to 80px to avoid the menu going off the screen - vertical = top < 80 ? 80 : top; - horizontal += right - left + 8; + // move the menu above the input if there is not enough space below + vertical = top - MENU_MAX_HEIGHT - 50; + } + + // check if there is enough space above the input field + if (top < MENU_MAX_HEIGHT + 50) { + // if not, reset the vertical position + vertical = top; + // move the menu to the right edge of the input field + horizontal = right + 5; + // flag to align the menu to the right or left edge of the input field + ToTheSide = true; } /* @@ -98,8 +107,8 @@ export const RolePicker = ({ * both (the role picker menu and its sub menu) aligned to the left edge of the input. * Otherwise, it aligns the role picker menu to the right. */ - if (left + ROLE_PICKER_MAX_MENU_WIDTH > window.innerWidth) { - horizontal = window.innerWidth - right; + if (horizontal + ROLE_PICKER_MAX_MENU_WIDTH > window.innerWidth) { + horizontal = window.innerWidth - (ToTheSide ? left : right); menuToLeft = true; } From f3fcfad2c89ea06ab73dc34aeae0be7d1b55a28d Mon Sep 17 00:00:00 2001 From: timo Date: Thu, 18 Apr 2024 18:39:16 +0200 Subject: [PATCH 15/21] NodeGraph: Fix invisible arrow tips in Editor (#86517) NodeGraphPanel: Namespace marker IDs to fix invisible arrow tips in editor Co-authored-by: Andrej Ocenas --- public/app/plugins/panel/nodeGraph/Edge.tsx | 7 ++++--- public/app/plugins/panel/nodeGraph/NodeGraph.tsx | 13 ++++++++++++- .../app/plugins/panel/nodeGraph/NodeGraphPanel.tsx | 10 ++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/public/app/plugins/panel/nodeGraph/Edge.tsx b/public/app/plugins/panel/nodeGraph/Edge.tsx index 9a9459e9c19..b80a22d71d7 100644 --- a/public/app/plugins/panel/nodeGraph/Edge.tsx +++ b/public/app/plugins/panel/nodeGraph/Edge.tsx @@ -11,13 +11,14 @@ export const defaultEdgeColor = '#999'; interface Props { edge: EdgeDatum; hovering: boolean; + svgIdNamespace: string; onClick: (event: MouseEvent, link: EdgeDatum) => void; onMouseEnter: (id: string) => void; onMouseLeave: (id: string) => void; } export const Edge = memo(function Edge(props: Props) { - const { edge, onClick, onMouseEnter, onMouseLeave, hovering } = props; + const { edge, onClick, onMouseEnter, onMouseLeave, hovering, svgIdNamespace } = props; // Not great typing but after we do layout these properties are full objects not just references const { source, target, sourceNodeRadius, targetNodeRadius } = edge as { @@ -47,8 +48,8 @@ export const Edge = memo(function Edge(props: Props) { // in case both are provided const highlightedEdgeColor = edge.color || defaultHighlightedEdgeColor; - const markerId = `triangle-${edge.id}`; - const coloredMarkerId = `triangle-colored-${edge.id}`; + const markerId = `triangle-${svgIdNamespace}-${edge.id}`; + const coloredMarkerId = `triangle-colored-${svgIdNamespace}-${edge.id}`; return ( <> diff --git a/public/app/plugins/panel/nodeGraph/NodeGraph.tsx b/public/app/plugins/panel/nodeGraph/NodeGraph.tsx index f0053aada0f..d49e9edb8eb 100644 --- a/public/app/plugins/panel/nodeGraph/NodeGraph.tsx +++ b/public/app/plugins/panel/nodeGraph/NodeGraph.tsx @@ -111,8 +111,9 @@ interface Props { dataFrames: DataFrame[]; getLinks: (dataFrame: DataFrame, rowIndex: number) => LinkModel[]; nodeLimit?: number; + panelId?: string; } -export function NodeGraph({ getLinks, dataFrames, nodeLimit }: Props) { +export function NodeGraph({ getLinks, dataFrames, nodeLimit, panelId }: Props) { const nodeCountLimit = nodeLimit || defaultNodeCountLimit; const { edges: edgesDataFrames, nodes: nodesDataFrames } = useCategorizeFrames(dataFrames); @@ -122,6 +123,13 @@ export function NodeGraph({ getLinks, dataFrames, nodeLimit }: Props) { const firstNodesDataFrame = nodesDataFrames[0]; const firstEdgesDataFrame = edgesDataFrames[0]; + // Ensure we use unique IDs for the marker tip elements, since IDs are global + // in the entire HTML document. This prevents hidden tips when an earlier + // occurence is hidden (editor is open in front of an existing node graph + // panel) or when the earlier tips have different properties (color, size, or + // shape for example). + const svgIdNamespace = panelId || 'nodegraphpanel'; + // TODO we should be able to allow multiple dataframes for both edges and nodes, could be issue with node ids which in // that case should be unique or figure a way to link edges and nodes dataframes together. const processed = useMemo( @@ -215,6 +223,7 @@ export function NodeGraph({ getLinks, dataFrames, nodeLimit }: Props) { onClick={onEdgeOpen} onMouseEnter={setEdgeHover} onMouseLeave={clearEdgeHover} + svgIdNamespace={svgIdNamespace} /> )} , link: EdgeDatum) => void; onMouseEnter: (id: string) => void; onMouseLeave: (id: string) => void; @@ -351,6 +361,7 @@ const Edges = memo(function Edges(props: EdgesProps) { onClick={props.onClick} onMouseEnter={props.onMouseEnter} onMouseLeave={props.onMouseLeave} + svgIdNamespace={props.svgIdNamespace} /> ))} diff --git a/public/app/plugins/panel/nodeGraph/NodeGraphPanel.tsx b/public/app/plugins/panel/nodeGraph/NodeGraphPanel.tsx index c1f256dda52..feb7e18e160 100644 --- a/public/app/plugins/panel/nodeGraph/NodeGraphPanel.tsx +++ b/public/app/plugins/panel/nodeGraph/NodeGraphPanel.tsx @@ -1,5 +1,5 @@ import memoizeOne from 'memoize-one'; -import React from 'react'; +import React, { useId } from 'react'; import { PanelProps } from '@grafana/data'; @@ -11,6 +11,8 @@ import { getNodeGraphDataFrames } from './utils'; export const NodeGraphPanel = ({ width, height, data, options }: PanelProps) => { const getLinks = useLinks(data.timeRange); + const panelId = useId(); + if (!data || !data.series.length) { return (
@@ -22,7 +24,11 @@ export const NodeGraphPanel = ({ width, height, data, options }: PanelProps - +
); }; From b311612cf2637f23862417ccc339274aa4381c4c Mon Sep 17 00:00:00 2001 From: brendamuir <100768211+brendamuir@users.noreply.github.com> Date: Thu, 18 Apr 2024 20:32:04 +0200 Subject: [PATCH 16/21] Alerting docs: RBAC for enterprise and cloud (#86506) * Alerting docs: RBAC for enterprise and cloud * rbac structure * ran prettier * updates to data source permissions * adds tables for roles * ran prettier * adds examples for custom role * ran prettier * updates table * typo fix * ran prettier --- .../alerting/set-up/configure-rbac/_index.md | 55 ++++++ .../configure-rbac/access-folders/index.md | 62 +++++++ .../configure-rbac/access-roles/index.md | 156 ++++++++++++++++++ .../alerting/set-up/configure-roles/index.md | 14 +- 4 files changed, 276 insertions(+), 11 deletions(-) create mode 100644 docs/sources/alerting/set-up/configure-rbac/_index.md create mode 100644 docs/sources/alerting/set-up/configure-rbac/access-folders/index.md create mode 100644 docs/sources/alerting/set-up/configure-rbac/access-roles/index.md diff --git a/docs/sources/alerting/set-up/configure-rbac/_index.md b/docs/sources/alerting/set-up/configure-rbac/_index.md new file mode 100644 index 00000000000..113381bdc9d --- /dev/null +++ b/docs/sources/alerting/set-up/configure-rbac/_index.md @@ -0,0 +1,55 @@ +--- +canonical: https://grafana.com/docs/grafana/latest/alerting/set-up/configure-rbac/ +description: Configure RBAC for Grafana Alerting +keywords: + - grafana + - alerting + - set up + - configure + - RBAC +labels: + products: + - enterprise + - cloud +title: Configure RBAC +weight: 155 +--- + +# Configure RBAC + +Role-based access control (RBAC) for Grafana Enterprise and Grafana Cloud provides a standardized way of granting, changing, and revoking access, so that users can view and modify Grafana resources. + +A user is any individual who can log in to Grafana. Each user is associated with a role that includes permissions. Permissions determine the tasks a user can perform in the system. + +Each permission contains one or more actions and a scope. + +## Permissions + +Grafana Alerting has the following permissions. + +| Action | Applicable scope | Description | +| ------------------------------------- | -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `alert.instances.external:read` | `datasources:*`
`datasources:uid:*` | Read alerts and silences in data sources that support alerting. | +| `alert.instances.external:write` | `datasources:*`
`datasources:uid:*` | Manage alerts and silences in data sources that support alerting. | +| `alert.instances:create` | n/a | Create silences in the current organization. | +| `alert.instances:read` | n/a | Read alerts and silences in the current organization. | +| `alert.instances:write` | n/a | Update and expire silences in the current organization. | +| `alert.notifications.external:read` | `datasources:*`
`datasources:uid:*` | Read templates, contact points, notification policies, and mute timings in data sources that support alerting. | +| `alert.notifications.external:write` | `datasources:*`
`datasources:uid:*` | Manage templates, contact points, notification policies, and mute timings in data sources that support alerting. | +| `alert.notifications:write` | n/a | Manage templates, contact points, notification policies, and mute timings in the current organization. | +| `alert.notifications:read` | n/a | Read all templates, contact points, notification policies, and mute timings in the current organization. | +| `alert.rules.external:read` | `datasources:*`
`datasources:uid:*` | Read alert rules in data sources that support alerting (Prometheus, Mimir, and Loki) | +| `alert.rules.external:write` | `datasources:*`
`datasources:uid:*` | Create, update, and delete alert rules in data sources that support alerting (Mimir and Loki). | +| `alert.rules:create` | `folders:*`
`folders:uid:*` | Create Grafana alert rules in a folder and its subfolders. Combine this permission with `folders:read` in a scope that includes the folder and `datasources:query` in the scope of data sources the user can query. | +| `alert.rules:delete` | `folders:*`
`folders:uid:*` | Delete Grafana alert rules in a folder and its subfolders. Combine this permission with `folders:read` in a scope that includes the folder and `datasources:query` in the scope of data sources the user can query. | +| `alert.rules:read` | `folders:*`
`folders:uid:*` | Read Grafana alert rules in a folder and its subfolders. Combine this permission with `folders:read` in a scope that includes the folder and `datasources:query` in the scope of data sources the user can query. | +| `alert.rules:write` | `folders:*`
`folders:uid:*` | Update Grafana alert rules in a folder and its subfolders. Combine this permission with `folders:read` in a scope that includes the folder and `datasources:query` in the scope of data sources the user can query. | +| `alert.silences:create` | `folders:*`
`folders:uid:*` | Create rule-specific silences in a folder and its subfolders. | +| `alert.silences:read` | `folders:*`
`folders:uid:*` | Read general and rule-specific silences in a folder and its subfolders. | +| `alert.silences:write` | `folders:*`
`folders:uid:*` | Update and expire rule-specific silences in a folder and its subfolders. | +| `alert.provisioning:read` | n/a | Read all Grafana alert rules, notification policies, etc via provisioning API. Permissions to folders and datasource are not required. | +| `alert.provisioning.secrets:read` | n/a | Same as `alert.provisioning:read` plus ability to export resources with decrypted secrets. | +| `alert.provisioning:write` | n/a | Update all Grafana alert rules, notification policies, etc via provisioning API. Permissions to folders and datasource are not required. | +| `alert.provisioning.provenance:write` | n/a | Set provisioning status for alerting resources. Cannot be used alone. Requires user to have permissions to access resources | + +To help plan your RBAC rollout strategy, refer to [Plan your RBAC rollout strategy](https://grafana.com/docs/grafana/next/administration/roles-and-permissions/access-control/plan-rbac-rollout-strategy/). diff --git a/docs/sources/alerting/set-up/configure-rbac/access-folders/index.md b/docs/sources/alerting/set-up/configure-rbac/access-folders/index.md new file mode 100644 index 00000000000..e16dfabf9eb --- /dev/null +++ b/docs/sources/alerting/set-up/configure-rbac/access-folders/index.md @@ -0,0 +1,62 @@ +--- +canonical: https://grafana.com/docs/grafana/latest/alerting/set-up/configure-rbac/access-folders/ +description: Manage access using folders +keywords: + - grafana + - alerting + - set up + - configure + - RBAC + - folder access +labels: + products: + - enterprise + - cloud +title: Manage access using folders or data sources +weight: 200 +--- + +## Manage access using folders or data sources + +You can further customize access for alert rules by assigning permissions to individual folders or data sources, regardless of role assigned. + +{{< admonition type="note" >}} +You can't use folders to customize access to notification resources. +{{< /admonition >}} + +Details of how role access can combine with folder permissions for Grafana Alerting are below. + +| Role | Folder | Access | +| ------ | ------ | ---------------------------------------------------------------------------------------- | +| Admin | - | Write access to alert rules in all folders. | +| Editor | - | Write access to alert rules in all folders. | +| Viewer | Admin | Write access to alert rules **only** in the folders where the Admin permission is added. | +| Viewer | Edit | Write access to alert rules **only** in the folders where the Edit permission is added. | +| Viewer | View | Read access to alert rules in all folders. | + +## Folder permissions + +To manage folder permissions, complete the following steps. + +1. In the left-side menu, click **Dashboards**. +1. Choose the folder you want to add permissions for. + +{{< admonition type="note" >}}It doesn’t matter which tab you’re on (Dashboards, Panels, or Alert rules); the folder permission you set applies to all.{{< /admonition >}} + +2. Click **Manage permissions** from the Folder actions menu. +3. Update or add permissions as required. + +## Data source permissions + +By default, users with the basic roles Admin, Editor, and Viewer roles have query access to data sources for Grafana Alerting. + +If you used fixed roles or custom roles, you need to update data source permissions. + +Alternatively, an admin can assign the role **Datasource Reader**, which grants the user access to all data sources. + +To manage data source permissions, complete the following steps. + +1. In the left-side menu, click **Connections** > **Data sources**. +1. Click the data source you want to change the permissions for. +1. Click the **Permissions** tab. +1. In the **Permission column**, update the permission or remove it by clicking **X**. diff --git a/docs/sources/alerting/set-up/configure-rbac/access-roles/index.md b/docs/sources/alerting/set-up/configure-rbac/access-roles/index.md new file mode 100644 index 00000000000..5e82b38534d --- /dev/null +++ b/docs/sources/alerting/set-up/configure-rbac/access-roles/index.md @@ -0,0 +1,156 @@ +--- +canonical: https://grafana.com/docs/grafana/latest/alerting/set-up/configure-rbac/access-roles +description: Manage access using roles +keywords: + - grafana + - alerting + - set up + - configure + - RBAC + - role access +labels: + products: + - enterprise + - cloud +title: Manage access using roles +weight: 100 +--- + +# Manage access using roles + +In Grafana Enterprise and Grafana Cloud, there are Basic, Fixed, and Custom roles. + +## Basic roles + +There are four basic roles: Admin, Editor, Viewer, and No basic role. Each basic role contains a number of fixed roles. + +The No basic role allows you to further customize access by assigning fixed roles to users, which you can also modify. You can also create and assign custom roles to a user with No basic role. + +Details of the basic roles and the access they provide for Grafana Alerting are below. + +| Role | Access | +| ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Admin | Write access to alert rules, notification resources (notification API, contact points, templates, time intervals, notification policies, and silences), and provisioning. | +| Editor | Write access to alert rules, notification resources (notification API, contact points, templates, time intervals, notification policies, and silences), and provisioning. | +| Viewer | Read access to alert rules, notification resources (notification API, contact points, templates, time intervals, notification policies, and silences). | +| No basic role | A blank canvas to assign fixed or custom roles and craft permissions more precisely. For example, if you want to give a user the ability to see alert rules, but not notification settings, add No basic role and then the fixed role Rules reader. | + +## Fixed roles + +A fixed role is a group of multiple permissions. + +Fixed roles provide users more granular access to create, view, and update Alerting resources than you would have with basic roles alone. + +Details of the fixed roles and the access they provide for Grafana Alerting are below. + +| Fixed role | Permissions | Description | +| -------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | +| `fixed:alerting.instances:writer` | All permissions from `fixed:alerting.instances:reader` and
`alert.instances:create`
`alert.instances:write` for organization scope
`alert.instances.external:write` for scope `datasources:*` | Create, update and expire all silences. | +| `fixed:alerting.instances:reader` | `alert.instances:read` for organization scope
`alert.instances.external:read` for scope `datasources:*` | Read all alerts and silences. | +| `fixed:alerting.notifications:writer` | All permissions from `fixed:alerting.notifications:reader` and
`alert.notifications:write`for organization scope
`alert.notifications.external:read` for scope `datasources:*` | Create, update, and delete contact points, templates, mute timings and notification policies for Grafana and external Alertmanager. | +| `fixed:alerting.notifications:reader` | `alert.notifications:read` for organization scope
`alert.notifications.external:read` for scope `datasources:*` | Read all Grafana and Alertmanager contact points, templates, and notification policies. | +| `fixed:alerting.rules:writer` | All permissions from `fixed:alerting.rules:reader` and
`alert.rule:create`
`alert.rule:write`
`alert.rule:delete`
`alert.silences:create`
`alert.silences:write` for scope `folders:*`
`alert.rules.external:write` for scope `datasources:*` | Create, update, and delete all alert rules and manage rule-specific silences. | +| `fixed:alerting.rules:reader` | `alert.rule:read`, `alert.silences:read` for scope `folders:*`
`alert.rules.external:read` for scope `datasources:*`
`alert.notifications.time-intervals:read`
`alert.notifications.receivers:list` | Read all alert rules and read rule-specific silences. | +| `fixed:alerting:writer` | All permissions from `fixed:alerting.rules:writer`
`fixed:alerting.instances:writer`
`fixed:alerting.notifications:writer` | Create, update, and delete all alert rules, silences, contact points, templates, mute timings, and notification policies. | +| `fixed:alerting:reader` | All permissions from `fixed:alerting.rules:reader`
`fixed:alerting.instances:reader`
`fixed:alerting.notifications:reader` | Read-only permissions for all alert rules, alerts, contact points, and notification policies. | +| `fixed:alerting.provisioning.secrets:reader` | `alert.provisioning:read` and `alert.provisioning.secrets:read` | Read-only permissions for Provisioning API and let export resources with decrypted secrets. | +| `fixed:alerting.provisioning:writer` | `alert.provisioning:read` and `alert.provisioning:write` | Create, update and delete Grafana alert rules, notification policies, contact points, templates, etc via provisioning API. | +| `fixed:alerting.provisioning.status:writer` | `alert.provisioning.provenance:write` | Set provenance status to alert rules, notification policies, contact points, etc. Should be used together with regular writer roles. | + +## Create custom roles + +Create custom roles of your own to manage permissions. Custom roles contain unique combinations of permissions, actions and scopes. Create a custom role when basic roles and fixed roles do not meet your permissions requirements. + +For more information on creating custom roles, refer to [Create custom roles](https://grafana.com/docs/grafana/latest/administration/roles-and-permissions/access-control/manage-rbac-roles/#create-custom-roles). + +### Examples + +The following examples give you an idea of how you can combine permissions for Grafana Alerting. + +A custom role for read access to alert rules that uses data source DS1 and DS2 in folder F: + + +``` +PUT access-control/roles +{ + "name": "custom:alert_rules_reader", + "displayName": "Alert rule reader in folder F", + "description": "Read access to rules in folder F that use DS1 and DS2", + "permissions": [ + { + "action": "datasources:query", + "scope": "datasources:uid:UID_DS1" + }, + { + "action": "datasources:query", + "scope": "datasources:uid:UID_DS2" + }, + { + "action": "alert.rules:read", + "scope": "folders:uid:UID_F" + }, + { + "action": "folders:read", + "scope": "folders:uid:UID_F" + } + ] +} +``` + + +A custom role for write access to alert rules that uses simplified routing: + + +``` +PUT access-control/roles +{ + "name": "custom:alert_rules_updater", + "displayName": "Alert rules editor in folder F", + "description": "Edit access to rules in folder F that use DS1 and DS2", + "permissions": [ + { + "action": "datasources:query", + "scope": "datasources:uid:UID_DS1" + }, + { + "action": "datasources:query", + "scope": "datasources:uid:UID_DS2" + }, + { + "action": "alert.rules:read", + "scope": "folders:uid:UID_F" + }, + { + "action": "alert.rules:read", + "scope": "folders:uid:UID_F" + }, + { + "action": "alert.rules:write", + "scope": "folders:uid:UID_F" + }, + { + "action": "alert.rules:create", + "scope": "folders:uid:UID_F" + }, + { + "action": "alert.notifications.receivers:list", + }, +{ + "action": "alert.notifications.time-intervals:read", + }, + ] +} +``` + + +{{< admonition type="note" >}} +Delete the last two permissions if you aren’t using simplified notification routing. +{{< /admonition >}} + +## Assign roles + +To assign roles, complete the following steps. + +1. Navigate to Administration > Users and access > Users, Teams, or Service Accounts. +1. Search for the user, team or service account you want to add a role for. +1. Select the role you want to assign. diff --git a/docs/sources/alerting/set-up/configure-roles/index.md b/docs/sources/alerting/set-up/configure-roles/index.md index f682c99c47f..5bff40250ea 100644 --- a/docs/sources/alerting/set-up/configure-roles/index.md +++ b/docs/sources/alerting/set-up/configure-roles/index.md @@ -42,15 +42,10 @@ To assign roles, admins need to complete the following steps. ## Manage access using folder permissions -You can further customize access for alert rules, simplified alert routing, and provisioning by assigning permissions to individual folders. +You can further customize access for alert rules by assigning permissions to individual folders. This prevents every user from having access to modify all alert rules and gives them access to the folders with the alert rules they're working on. -For example, if you are using simplified alert routing and adding contact points to your alert rules, it also helps you avoid the scenario where someone from another team accidentally removes the wrong notification policy or adds a competing one, and all of a sudden you stop getting your notifications. - -In this case, you would assign the **Viewer** role and then add **Editor** permission to the folder. -Adding the **Editor** permission to the folder doesn't overwrite the **Viewer** role. - Details on the adding folder permissions as well as roles and the access that provides for Grafana Alerting is below. | Role | Folder permission | Access | @@ -69,8 +64,5 @@ To manage folder permissions, complete the following steps. 1. In the left-side menu, click **Dashboards**. 1. Hover your mouse cursor over a folder and click **Go to folder**. -1. Click the **Permissions** tab. -1. Click **Add a permission**. -1. Select the user, service account, team, or role. -1. Select **Viewer, Editor or Admin**. -1. Click **Save**. +1. Click **Manage permissions** from the Folder actions menu. +1. Update or add permissions as required. From 842c8dd20667f7646b0989f7d7ba6f854b840c7b Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Fri, 19 Apr 2024 01:14:29 +0200 Subject: [PATCH 17/21] InfluxDB: Fix interpolating field keys in influxql (#86401) * interpolate field keys * use scopedVars --- .../plugins/datasource/influxdb/datasource.ts | 2 +- .../influxdb/datasource_backend_mode.test.ts | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/influxdb/datasource.ts b/public/app/plugins/datasource/influxdb/datasource.ts index 9395dafb313..e54bde8734d 100644 --- a/public/app/plugins/datasource/influxdb/datasource.ts +++ b/public/app/plugins/datasource/influxdb/datasource.ts @@ -254,7 +254,7 @@ export default class InfluxDatasource extends DataSourceWithBackend { return { ...select, - params: select.params?.map((param) => this.templateSrv.replace(param.toString(), undefined)), + params: select.params?.map((param) => this.templateSrv.replace(param.toString(), scopedVars)), }; }); }); diff --git a/public/app/plugins/datasource/influxdb/datasource_backend_mode.test.ts b/public/app/plugins/datasource/influxdb/datasource_backend_mode.test.ts index 4ce1dff8e2f..691677ce07d 100644 --- a/public/app/plugins/datasource/influxdb/datasource_backend_mode.test.ts +++ b/public/app/plugins/datasource/influxdb/datasource_backend_mode.test.ts @@ -221,6 +221,26 @@ describe('InfluxDataSource Backend Mode', () => { const variablesMock = [ queryBuilder().withId('var1').withName('var1').withCurrent('var1').build(), queryBuilder().withId('path').withName('path').withCurrent('/etc/hosts').build(), + queryBuilder() + .withId('field_var') + .withName('field_var') + .withMulti(true) + .withOptions( + { + text: `field_1`, + value: `field_1`, + }, + { + text: `field_2`, + value: `field_2`, + }, + { + text: `field_3`, + value: `field_3`, + } + ) + .withCurrent(['field_1', 'field_3']) + .build(), ]; const mockTemplateService = new TemplateSrv({ getVariables: () => variablesMock, @@ -328,6 +348,34 @@ describe('InfluxDataSource Backend Mode', () => { const expected = `/etc/hosts`; expect(res.tags?.[0].value).toEqual(expected); }); + + it('should interpolate field keys with given scopedVars', () => { + const query: InfluxQuery = { + refId: 'A', + tags: [ + { + key: 'key', + operator: '=', + value: 'value', + }, + ], + select: [ + [ + { + type: 'field', + params: ['$field_var'], + }, + { + type: 'mean', + params: [], + }, + ], + ], + }; + const res = ds.applyVariables(query, { field_var: { text: 'field_3', value: 'field_3' } }); + const expected = `field_3`; + expect(res.select?.[0][0].params?.[0]).toEqual(expected); + }); }); describe('metric find query', () => { From 71445002b7061244cd08b4c180481248e262c309 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 18 Apr 2024 21:08:14 -0400 Subject: [PATCH 18/21] Alerting: Fix simplified routing group by override (#86552) * Alerting: Fix simplified routing custom group by override Custom group by overrides for simplified routing were missing required fields GroupBy and GroupByAll normally set during upstream Route validation. This fix ensures those missing fields are applied to the generated routes. * Inline GroupBy and GroupByAll initialization instead of normalize after --- .../ngalert/notifier/autogen_alertmanager.go | 26 +++++++++++++++++-- .../notifier/autogen_alertmanager_test.go | 3 +++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager.go b/pkg/services/ngalert/notifier/autogen_alertmanager.go index 003ff617e33..80df43c9a0f 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/common/model" "golang.org/x/exp/maps" "github.com/grafana/grafana/pkg/infra/log" @@ -115,12 +116,16 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp if err != nil { return autogeneratedRoute{}, err } + groupByStr := append([]string{}, models.DefaultNotificationSettingsGroupBy...) + groupByAll, groupBy := toGroupBy(groupByStr...) receiverRoute = &definitions.Route{ Receiver: s.Receiver, ObjectMatchers: definitions.ObjectMatchers{contactMatcher}, Continue: false, // Since we'll have many rules from different folders using this policy, we ensure it has these necessary groupings. - GroupByStr: append([]string{}, models.DefaultNotificationSettingsGroupBy...), + GroupByStr: groupByStr, + GroupBy: groupBy, + GroupByAll: groupByAll, } receiverRoutes[s.Receiver] = receiverRoute autoGenRoot.Routes = append(autoGenRoot.Routes, receiverRoute) @@ -134,12 +139,16 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp if err != nil { return autogeneratedRoute{}, err } + normalized := s.NormalizedGroupBy() + groupByAll, groupBy := toGroupBy(normalized...) receiverRoute.Routes = append(receiverRoute.Routes, &definitions.Route{ Receiver: s.Receiver, ObjectMatchers: definitions.ObjectMatchers{settingMatcher}, Continue: false, // Only a single setting-specific route should match. - GroupByStr: s.NormalizedGroupBy(), + GroupByStr: normalized, + GroupBy: groupBy, + GroupByAll: groupByAll, MuteTimeIntervals: s.MuteTimeIntervals, GroupWait: s.GroupWait, GroupInterval: s.GroupInterval, @@ -152,6 +161,19 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp }, nil } +// toGroupBy converts the given label strings to (groupByAll, []model.LabelName) where groupByAll is true if the input +// contains models.GroupByAll. This logic is in accordance with upstream Route.ValidateChild(). +func toGroupBy(groupByStr ...string) (groupByAll bool, groupBy []model.LabelName) { + for _, l := range groupByStr { + if l == models.GroupByAll { + return true, nil + } else { + groupBy = append(groupBy, model.LabelName(l)) + } + } + return false, groupBy +} + // addToRoute adds this autogenerated route to the given route as the first top-level route under the root. func (ar *autogeneratedRoute) addToRoute(route *definitions.Route) error { if route == nil { diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go index 14e50e844ff..bc962728eac 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go @@ -292,6 +292,9 @@ func TestAddAutogenConfig(t *testing.T) { require.NoError(t, err) } + // We compare against the upstream normalized route. + require.NoError(t, tt.expRoute.Validate()) + cOpt := []cmp.Option{ cmpopts.IgnoreUnexported(definitions.Route{}, labels.Matcher{}), } From a20197229e833ea8c75877d8f5bf9a46cc2a4755 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Thu, 18 Apr 2024 21:08:38 -0400 Subject: [PATCH 19/21] Alerting: Prevent simplified routing zero duration GroupInterval and RepeatInterval (#86561) Prevent zero duration GroupInterval and RepeatInterval --- pkg/services/ngalert/models/notifications.go | 8 ++++---- pkg/services/ngalert/models/notifications_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/services/ngalert/models/notifications.go b/pkg/services/ngalert/models/notifications.go index 8d40c5d88b1..895a70da0a6 100644 --- a/pkg/services/ngalert/models/notifications.go +++ b/pkg/services/ngalert/models/notifications.go @@ -81,11 +81,11 @@ func (s *NotificationSettings) Validate() error { if s.GroupWait != nil && *s.GroupWait < 0 { return errors.New("group wait must be a positive duration") } - if s.GroupInterval != nil && *s.GroupInterval < 0 { - return errors.New("group interval must be a positive duration") + if s.GroupInterval != nil && *s.GroupInterval <= 0 { + return errors.New("group interval must be greater than zero") } - if s.RepeatInterval != nil && *s.RepeatInterval < 0 { - return errors.New("repeat interval must be a positive duration") + if s.RepeatInterval != nil && *s.RepeatInterval <= 0 { + return errors.New("repeat interval must be greater than zero") } return nil } diff --git a/pkg/services/ngalert/models/notifications_test.go b/pkg/services/ngalert/models/notifications_test.go index 14e3a3e89bb..4cdf589614c 100644 --- a/pkg/services/ngalert/models/notifications_test.go +++ b/pkg/services/ngalert/models/notifications_test.go @@ -74,6 +74,11 @@ func TestValidate(t *testing.T) { notificationSettings: CopyNotificationSettings(validNotificationSettings(), NSMuts.WithGroupInterval(util.Pointer(-1*time.Second))), expErrorContains: "group interval", }, + { + name: "group interval zero is invalid", + notificationSettings: CopyNotificationSettings(validNotificationSettings(), NSMuts.WithGroupInterval(util.Pointer(0*time.Second))), + expErrorContains: "group interval", + }, { name: "repeat interval empty is valid", notificationSettings: CopyNotificationSettings(validNotificationSettings(), NSMuts.WithRepeatInterval(nil)), @@ -87,6 +92,11 @@ func TestValidate(t *testing.T) { notificationSettings: CopyNotificationSettings(validNotificationSettings(), NSMuts.WithRepeatInterval(util.Pointer(-1*time.Second))), expErrorContains: "repeat interval", }, + { + name: "repeat interval zero is invalid", + notificationSettings: CopyNotificationSettings(validNotificationSettings(), NSMuts.WithRepeatInterval(util.Pointer(0*time.Second))), + expErrorContains: "repeat interval", + }, } for _, tt := range testCases { From aa825f5deef97985ab181866e6be8222cc462d27 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Fri, 19 Apr 2024 09:16:38 +0300 Subject: [PATCH 20/21] Chore: Fix Swagger/OpenAPI instructions (#86541) Update README.md --- pkg/api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/README.md b/pkg/api/README.md index 1449c6077b6..08358933378 100644 --- a/pkg/api/README.md +++ b/pkg/api/README.md @@ -82,4 +82,4 @@ Finally, they can browser and try out both the OpenAPI v2 and v3 via the Swagger If there are any issues generating the specifications (e.g., diff containing unrelated changes to your PR or unusually large diff), please run the following two commands to ensure your Swagger version is up to date, then re-run the make commands. - `go install github.com/bwplotka/bingo@latest` -- `bingo get swagger` \ No newline at end of file +- `bingo get github.com/go-swagger/go-swagger/cmd/swagger@v0.30.2` From 8a5c0cfdc00dc3ed2e59edf4dddb94bcbea3279a Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 19 Apr 2024 08:22:14 +0200 Subject: [PATCH 21/21] Plugins: Pass cancellable context during API server creation (#86545) --- pkg/cmd/grafana/apiserver/cmd.go | 9 ++++++++- pkg/cmd/grafana/apiserver/server.go | 5 +++-- pkg/services/apiserver/standalone/factory.go | 4 ++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/grafana/apiserver/cmd.go b/pkg/cmd/grafana/apiserver/cmd.go index cb3530b7c2c..9adab683f42 100644 --- a/pkg/cmd/grafana/apiserver/cmd.go +++ b/pkg/cmd/grafana/apiserver/cmd.go @@ -1,6 +1,7 @@ package apiserver import ( + "context" "os" "github.com/spf13/cobra" @@ -54,8 +55,14 @@ func newCommandStartExampleAPIServer(o *APIServerOptions, stopCh <-chan struct{} // TODO: Fix so that TracingOptions.ApplyTo happens before or during loadAPIGroupBuilders. tracer := newLateInitializedTracingService() + ctx, cancel := context.WithCancel(c.Context()) + go func() { + <-stopCh + cancel() + }() + // Load each group from the args - if err := o.loadAPIGroupBuilders(tracer, apis); err != nil { + if err := o.loadAPIGroupBuilders(ctx, tracer, apis); err != nil { return err } diff --git a/pkg/cmd/grafana/apiserver/server.go b/pkg/cmd/grafana/apiserver/server.go index ed064d1cef1..1ead6da20e5 100644 --- a/pkg/cmd/grafana/apiserver/server.go +++ b/pkg/cmd/grafana/apiserver/server.go @@ -1,6 +1,7 @@ package apiserver import ( + "context" "fmt" "io" "net" @@ -50,10 +51,10 @@ func newAPIServerOptions(out, errOut io.Writer) *APIServerOptions { } } -func (o *APIServerOptions) loadAPIGroupBuilders(tracer tracing.Tracer, apis []schema.GroupVersion) error { +func (o *APIServerOptions) loadAPIGroupBuilders(ctx context.Context, tracer tracing.Tracer, apis []schema.GroupVersion) error { o.builders = []builder.APIGroupBuilder{} for _, gv := range apis { - api, err := o.factory.MakeAPIServer(tracer, gv) + api, err := o.factory.MakeAPIServer(ctx, tracer, gv) if err != nil { return err } diff --git a/pkg/services/apiserver/standalone/factory.go b/pkg/services/apiserver/standalone/factory.go index 32009a224e0..0af7bf0132c 100644 --- a/pkg/services/apiserver/standalone/factory.go +++ b/pkg/services/apiserver/standalone/factory.go @@ -35,7 +35,7 @@ type APIServerFactory interface { GetEnabled(runtime []RuntimeConfig) ([]schema.GroupVersion, error) // Make an API server for a given group+version - MakeAPIServer(tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error) + MakeAPIServer(ctx context.Context, tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error) } // Zero dependency provider for testing @@ -67,7 +67,7 @@ func (p *DummyAPIFactory) ApplyTo(config *genericapiserver.RecommendedConfig) er return nil } -func (p *DummyAPIFactory) MakeAPIServer(tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error) { +func (p *DummyAPIFactory) MakeAPIServer(_ context.Context, tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error) { if gv.Version != "v0alpha1" { return nil, fmt.Errorf("only alpha supported now") }