diff --git a/packages/grafana-data/src/types/pluginExtensions.ts b/packages/grafana-data/src/types/pluginExtensions.ts index a04b9b7470f..83523e4b595 100644 --- a/packages/grafana-data/src/types/pluginExtensions.ts +++ b/packages/grafana-data/src/types/pluginExtensions.ts @@ -195,6 +195,7 @@ export enum PluginExtensionPoints { TraceViewResourceAttributes = 'grafana/traceview/resource-attributes', LogsViewResourceAttributes = 'grafana/logsview/resource-attributes', AppChrome = 'grafana/app/chrome/v1', + ExtensionSidebar = 'grafana/extension-sidebar/v0-alpha', } export type PluginExtensionPanelContext = { diff --git a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebar.tsx b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebar.tsx index 89cb4b3db7c..328ff2aa469 100644 --- a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebar.tsx +++ b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebar.tsx @@ -1,14 +1,10 @@ import { css } from '@emotion/css'; -import { GrafanaTheme2 } from '@grafana/data'; +import { GrafanaTheme2, PluginExtensionPoints } from '@grafana/data'; import { usePluginComponents } from '@grafana/runtime'; import { useTheme2 } from '@grafana/ui'; -import { - EXTENSION_SIDEBAR_EXTENSION_POINT_ID, - getComponentMetaFromComponentId, - useExtensionSidebarContext, -} from './ExtensionSidebarProvider'; +import { getComponentMetaFromComponentId, useExtensionSidebarContext } from './ExtensionSidebarProvider'; export const DEFAULT_EXTENSION_SIDEBAR_WIDTH = 300; export const MIN_EXTENSION_SIDEBAR_WIDTH = 100; @@ -22,7 +18,7 @@ export function ExtensionSidebar() { const styles = getStyles(useTheme2()); const { dockedComponentId, isEnabled, props = {} } = useExtensionSidebarContext(); const { components, isLoading } = usePluginComponents({ - extensionPointId: EXTENSION_SIDEBAR_EXTENSION_POINT_ID, + extensionPointId: PluginExtensionPoints.ExtensionSidebar, }); if (isLoading || !dockedComponentId || !isEnabled) { diff --git a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx index 8693b71ed04..e1618f113cb 100644 --- a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx +++ b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx @@ -1,14 +1,13 @@ import { createContext, ReactNode, useCallback, useContext, useEffect, useState, useMemo } from 'react'; import { useLocalStorage } from 'react-use'; -import { store, type ExtensionInfo } from '@grafana/data'; +import { PluginExtensionPoints, store, type ExtensionInfo } from '@grafana/data'; import { config, getAppEvents, reportInteraction, usePluginLinks, locationService } from '@grafana/runtime'; import { ExtensionPointPluginMeta, getExtensionPointPluginMeta } from 'app/features/plugins/extensions/utils'; import { OpenExtensionSidebarEvent } from 'app/types/events'; import { DEFAULT_EXTENSION_SIDEBAR_WIDTH } from './ExtensionSidebar'; -export const EXTENSION_SIDEBAR_EXTENSION_POINT_ID = 'grafana/extension-sidebar/v0-alpha'; export const EXTENSION_SIDEBAR_DOCKED_LOCAL_STORAGE_KEY = 'grafana.navigation.extensionSidebarDocked'; export const EXTENSION_SIDEBAR_WIDTH_LOCAL_STORAGE_KEY = 'grafana.navigation.extensionSidebarWidth'; const PERMITTED_EXTENSION_SIDEBAR_PLUGINS = [ @@ -94,7 +93,7 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo // `grafana/extension-sidebar/v0-alpha` and the link's `configure` method would control // whether the component is rendered or not const { links, isLoading } = usePluginLinks({ - extensionPointId: EXTENSION_SIDEBAR_EXTENSION_POINT_ID, + extensionPointId: PluginExtensionPoints.ExtensionSidebar, context: { path: currentPath, }, @@ -107,7 +106,7 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo () => isEnabled ? new Map( - Array.from(getExtensionPointPluginMeta(EXTENSION_SIDEBAR_EXTENSION_POINT_ID).entries()).filter( + Array.from(getExtensionPointPluginMeta(PluginExtensionPoints.ExtensionSidebar).entries()).filter( ([pluginId, pluginMeta]) => PERMITTED_EXTENSION_SIDEBAR_PLUGINS.includes(pluginId) && links.some( diff --git a/public/app/features/plugins/extensions/errors.ts b/public/app/features/plugins/extensions/errors.ts index cb9d22e7b46..3ef7a93a638 100644 --- a/public/app/features/plugins/extensions/errors.ts +++ b/public/app/features/plugins/extensions/errors.ts @@ -1,5 +1,13 @@ export const INVALID_EXTENSION_POINT_ID = - 'Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "myorg-foo-app/toolbar/v1".'; + 'Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "myorg-foo-app/toolbar/v1". Returning an empty array of extensions.'; + +export const INVALID_EXTENSION_POINT_ID_PLUGIN = (pluginId: string, extensionPointId: string) => + `Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "${pluginId}/${extensionPointId}".`; + +export const INVALID_EXTENSION_POINT_ID_GRAFANA_PREFIX = (extensionPointId: string) => + `Invalid usage of extension point. Reason: Core Grafana extension point id should be prefixed with "grafana/", e.g "grafana/${extensionPointId}".`; + +export const INVALID_EXTENSION_POINT_ID_GRAFANA_EXPOSED = `Invalid usage of extension point. Reason: Core Grafana extension point id should be exposed to plugins via the "PluginExtensionPoints" enum in the "grafana-data" package (/packages/grafana-data/src/types/pluginExtensions.ts).`; export const EXTENSION_POINT_META_INFO_MISSING = 'Invalid usage of extension point. Reason: The extension point is not recorded in the "plugin.json" file. Extension points must be listed in the section "extensions.extensionPoints[]". Returning an empty array of extensions.'; diff --git a/public/app/features/plugins/extensions/usePluginComponents.test.tsx b/public/app/features/plugins/extensions/usePluginComponents.test.tsx index dc8b84a7287..7b55daa2800 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.test.tsx @@ -1,7 +1,7 @@ import { act, render, renderHook, screen } from '@testing-library/react'; import React from 'react'; -import { PluginContextProvider, PluginMeta, PluginType } from '@grafana/data'; +import { PluginContextProvider, PluginExtensionPoints, PluginMeta, PluginType } from '@grafana/data'; import { config } from '@grafana/runtime'; import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; @@ -498,7 +498,7 @@ describe('usePluginComponents()', () => { pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context configs: [ { - targets: 'grafana/extension-point/v1', + targets: PluginExtensionPoints.DashboardPanelMenu, title: '1', description: '1', component: () =>
Component
, @@ -506,14 +506,17 @@ describe('usePluginComponents()', () => { ], }); - let { result } = renderHook(() => usePluginComponents({ extensionPointId: 'grafana/extension-point/v1' }), { - wrapper, - }); + let { result } = renderHook( + () => usePluginComponents({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), + { + wrapper, + } + ); expect(result.current.components.length).toBe(1); expect(log.error).not.toHaveBeenCalled(); }); - it('should not validate the extension point id if used in Grafana core (no plugin context)', () => { + it('should not allow to create an extension point in core Grafana that is not exposed to plugins', () => { // Imitate running in dev mode jest.mocked(isGrafanaDevMode).mockReturnValue(true); @@ -522,11 +525,26 @@ describe('usePluginComponents()', () => { {children} ); - let { result } = renderHook(() => usePluginComponents({ extensionPointId: 'invalid-extension-point-id' }), { + const extensionPointId = 'grafana/not-exposed-extension-point/v1'; + + // Adding an extension to the extension point + registries.addedComponentsRegistry.register({ + pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + component: () =>
Component
, + }, + ], + }); + + let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper, }); expect(result.current.components.length).toBe(0); - expect(log.error).not.toHaveBeenCalled(); + expect(log.error).toHaveBeenCalled(); }); it('should validate if the extension point meta-info is correct if in dev-mode and used by a plugin', () => { diff --git a/public/app/features/plugins/extensions/usePluginComponents.tsx b/public/app/features/plugins/extensions/usePluginComponents.tsx index 58d95289c15..6f37f3bdbba 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.tsx @@ -28,8 +28,7 @@ export function usePluginComponents({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); return useMemo(() => { - // For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana. - const enableRestrictions = isGrafanaDevMode() && pluginContext; + const isInsidePlugin = Boolean(pluginContext); const components: Array> = []; const extensionsByPlugin: Record = {}; const pluginId = pluginContext?.meta.id ?? ''; @@ -38,13 +37,16 @@ export function usePluginComponents({ extensionPointId, }); - // Only log error for an invalid `extensionPointId` in DEV mode - if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) { - pointLog.error(errors.INVALID_EXTENSION_POINT_ID); + // Don't show extensions if the extension-point id is invalid in DEV mode + if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) { + return { + isLoading: false, + components: [], + }; } // Don't show extensions if the extension-point misses meta info (plugin.json) in DEV mode - if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { + if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); return { isLoading: false, diff --git a/public/app/features/plugins/extensions/usePluginFunctions.tsx b/public/app/features/plugins/extensions/usePluginFunctions.tsx index 68acee76221..18a8abd7aef 100644 --- a/public/app/features/plugins/extensions/usePluginFunctions.tsx +++ b/public/app/features/plugins/extensions/usePluginFunctions.tsx @@ -23,8 +23,7 @@ export function usePluginFunctions({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(deps); return useMemo(() => { - // For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana. - const enableRestrictions = isGrafanaDevMode() && pluginContext; + const isInsidePlugin = Boolean(pluginContext); const results: Array> = []; const extensionsByPlugin: Record = {}; const pluginId = pluginContext?.meta.id ?? ''; @@ -32,11 +31,15 @@ export function usePluginFunctions({ pluginId, extensionPointId, }); - if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) { - pointLog.error(errors.INVALID_EXTENSION_POINT_ID); + + if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) { + return { + isLoading: false, + functions: [], + }; } - if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { + if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); return { isLoading: false, diff --git a/public/app/features/plugins/extensions/usePluginLinks.test.tsx b/public/app/features/plugins/extensions/usePluginLinks.test.tsx index f9fb623f42a..b5d984b165c 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.test.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.test.tsx @@ -1,6 +1,6 @@ import { act, renderHook } from '@testing-library/react'; -import { PluginContextProvider, PluginMeta, PluginType } from '@grafana/data'; +import { PluginContextProvider, PluginExtensionPoints, PluginMeta, PluginType } from '@grafana/data'; import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; import { log } from './logs/log'; @@ -256,7 +256,7 @@ describe('usePluginLinks()', () => { pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context configs: [ { - targets: 'grafana/extension-point/v1', + targets: PluginExtensionPoints.DashboardPanelMenu, title: '1', description: '1', path: `/a/grafana/${pluginId}/2`, @@ -264,11 +264,42 @@ describe('usePluginLinks()', () => { ], }); - let { result } = renderHook(() => usePluginLinks({ extensionPointId: 'grafana/extension-point/v1' }), { wrapper }); + let { result } = renderHook(() => usePluginLinks({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), { + wrapper, + }); expect(result.current.links.length).toBe(1); expect(log.warning).not.toHaveBeenCalled(); }); + it('should not allow to create an extension point in core Grafana that is not exposed to plugins', () => { + // Imitate running in dev mode + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + // No plugin context -> used in Grafana core + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const extensionPointId = 'grafana/not-exposed-extension-point/v1'; + + // Adding an extension to the extension point + registries.addedLinksRegistry.register({ + pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + path: `/a/grafana/${pluginId}/2`, + }, + ], + }); + + let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + expect(result.current.links.length).toBe(0); + expect(log.error).toHaveBeenCalled(); + }); + it('should not validate the extension point id if used in Grafana core (no plugin context)', () => { // Imitate running in dev mode jest.mocked(isGrafanaDevMode).mockReturnValue(true); diff --git a/public/app/features/plugins/extensions/usePluginLinks.tsx b/public/app/features/plugins/extensions/usePluginLinks.tsx index 6ecaf6b71b9..adec8114582 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.tsx @@ -32,23 +32,21 @@ export function usePluginLinks({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); return useMemo(() => { - // For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana. - const enableRestrictions = isGrafanaDevMode() && pluginContext !== null; + const isInsidePlugin = Boolean(pluginContext); const pluginId = pluginContext?.meta.id ?? ''; const pointLog = log.child({ pluginId, extensionPointId, }); - if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) { - pointLog.error(errors.INVALID_EXTENSION_POINT_ID); + if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) { return { isLoading: false, links: [], }; } - if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { + if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) { pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); return { isLoading: false, diff --git a/public/app/features/plugins/extensions/validators.test.tsx b/public/app/features/plugins/extensions/validators.test.tsx index 8e3350494d5..46069467c44 100644 --- a/public/app/features/plugins/extensions/validators.test.tsx +++ b/public/app/features/plugins/extensions/validators.test.tsx @@ -198,9 +198,8 @@ describe('Plugin Extension Validators', () => { describe('isExtensionPointIdValid()', () => { test.each([ - // We (for now allow core Grafana extension points to run without a version) - ['grafana/extension-point', ''], - ['grafana/extension-point', 'grafana'], + [PluginExtensionPoints.DashboardPanelMenu, ''], + [PluginExtensionPoints.DashboardPanelMenu, 'grafana'], ['myorg-extensions-app/extension-point', 'myorg-extensions-app'], ['myorg-extensions-app/extension-point/v1', 'myorg-extensions-app'], ['plugins/myorg-extensions-app/extension-point/v1', 'myorg-extensions-app'], @@ -217,6 +216,8 @@ describe('Plugin Extension Validators', () => { isExtensionPointIdValid({ extensionPointId, pluginId, + isInsidePlugin: pluginId !== 'grafana' && pluginId !== '', + log: createLogMock(), }) ).toBe(true); }); @@ -232,11 +233,18 @@ describe('Plugin Extension Validators', () => { 'extension-point/v1', 'myorgs-extensions-app', ], + [ + // Not exposed to plugins + 'grafana/not-exposed-extension-point/v1', + 'grafana', + ], ])('should return FALSE if the extension point id is invalid ("%s", "%s")', (extensionPointId, pluginId) => { expect( isExtensionPointIdValid({ extensionPointId, pluginId, + isInsidePlugin: pluginId !== 'grafana' && pluginId !== '', + log: createLogMock(), }) ).toBe(false); }); diff --git a/public/app/features/plugins/extensions/validators.ts b/public/app/features/plugins/extensions/validators.ts index 4106544567f..6a6a7b51748 100644 --- a/public/app/features/plugins/extensions/validators.ts +++ b/public/app/features/plugins/extensions/validators.ts @@ -68,15 +68,33 @@ export function isLinkPathValid(pluginId: string, path: string) { export function isExtensionPointIdValid({ extensionPointId, pluginId, + isInsidePlugin, + log, }: { extensionPointId: string; pluginId: string; + isInsidePlugin: boolean; + log: ExtensionsLog; }) { - if (extensionPointId.startsWith('grafana/')) { - return true; + const startsWithPluginId = + extensionPointId.startsWith(`${pluginId}/`) || extensionPointId.startsWith(`plugins/${pluginId}/`); + + if (isInsidePlugin && !startsWithPluginId) { + log.error(errors.INVALID_EXTENSION_POINT_ID_PLUGIN(pluginId, extensionPointId)); + return false; } - return Boolean(extensionPointId.startsWith(`plugins/${pluginId}/`) || extensionPointId.startsWith(`${pluginId}/`)); + if (!isInsidePlugin && !extensionPointId.startsWith('grafana/')) { + log.error(errors.INVALID_EXTENSION_POINT_ID_GRAFANA_PREFIX(extensionPointId)); + return false; + } + + if (!isInsidePlugin && !Object.values(PluginExtensionPoints).includes(extensionPointId)) { + log.error(errors.INVALID_EXTENSION_POINT_ID_GRAFANA_EXPOSED); + return false; + } + + return true; } export function extensionPointEndsWithVersion(extensionPointId: string) {