UI Extensions: Remove path validation from link extensions (#112259)
feat: remove validation for link extension paths
This commit is contained in:
@@ -22,8 +22,6 @@ export const INVALID_CONFIGURE_FUNCTION = 'The "configure" function is invalid.
|
||||
|
||||
export const INVALID_PATH_OR_ON_CLICK = 'Either "path" or "onClick" is required.';
|
||||
|
||||
export const INVALID_PATH = 'The "path" is required and should start with "/a/<pluginId>".';
|
||||
|
||||
export const INVALID_EXPOSED_COMPONENT_ID =
|
||||
"The component id does not match the id naming convention. Id should be prefixed with plugin id. e.g 'myorg-basic-app/my-component-id/v1'.";
|
||||
|
||||
|
||||
@@ -318,34 +318,6 @@ describe('getPluginExtensions()', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test('should skip the link extension if the configure() function returns with an invalid path', async () => {
|
||||
link1.configure = jest.fn().mockImplementation(() => ({
|
||||
path: '/a/another-plugin/page-a',
|
||||
}));
|
||||
link2.configure = jest.fn().mockImplementation(() => ({
|
||||
path: 'invalid-path',
|
||||
}));
|
||||
|
||||
const registries = await createRegistries([
|
||||
{ pluginId, addedLinkConfigs: [link1, link2], addedComponentConfigs: [] },
|
||||
]);
|
||||
const { extensions: extensionsAtPlacement1 } = getPluginExtensions({
|
||||
...registries,
|
||||
extensionPointId: extensionPoint1,
|
||||
});
|
||||
const { extensions: extensionsAtPlacement2 } = getPluginExtensions({
|
||||
...registries,
|
||||
extensionPointId: extensionPoint2,
|
||||
});
|
||||
|
||||
expect(extensionsAtPlacement1).toHaveLength(0);
|
||||
expect(extensionsAtPlacement2).toHaveLength(0);
|
||||
|
||||
expect(link1.configure).toHaveBeenCalledTimes(1);
|
||||
expect(link2.configure).toHaveBeenCalledTimes(1);
|
||||
expect(log.error).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test('should skip the extension if any of the updated props returned by the configure() function are invalid', async () => {
|
||||
const overrides = {
|
||||
title: '', // Invalid empty string for title - should be ignored
|
||||
|
||||
@@ -551,34 +551,6 @@ describe('AddedLinksRegistry', () => {
|
||||
expect(registry).toEqual({});
|
||||
});
|
||||
|
||||
it('should not register link extensions with invalid path configured', () => {
|
||||
const pluginId = 'grafana-basic-app';
|
||||
const reactiveRegistry = new AddedLinksRegistry();
|
||||
const observable = reactiveRegistry.asObservable();
|
||||
const subscribeCallback = jest.fn();
|
||||
|
||||
reactiveRegistry.register({
|
||||
pluginId: pluginId,
|
||||
configs: [
|
||||
{
|
||||
title: 'Title 1',
|
||||
description: 'Description 1',
|
||||
path: `/a/another-plugin/declare-incident`,
|
||||
targets: 'grafana/dashboard/panel/menu',
|
||||
configure: jest.fn().mockReturnValue({}),
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(log.error).toHaveBeenCalled();
|
||||
|
||||
observable.subscribe(subscribeCallback);
|
||||
expect(subscribeCallback).toHaveBeenCalledTimes(1);
|
||||
|
||||
const registry = subscribeCallback.mock.calls[0][0];
|
||||
expect(registry).toEqual({});
|
||||
});
|
||||
|
||||
it('should not be possible to register a link on a read-only registry', async () => {
|
||||
const pluginId = 'grafana-basic-app';
|
||||
const registry = new AddedLinksRegistry();
|
||||
|
||||
@@ -5,7 +5,7 @@ import { PluginAddedLinksConfigureFunc, PluginExtensionEventHelpers } from '@gra
|
||||
|
||||
import * as errors from '../errors';
|
||||
import { isGrafanaDevMode } from '../utils';
|
||||
import { isAddedLinkMetaInfoMissing, isConfigureFnValid, isLinkPathValid } from '../validators';
|
||||
import { isAddedLinkMetaInfoMissing, isConfigureFnValid } from '../validators';
|
||||
|
||||
import { PluginExtensionConfigs, Registry, RegistryType } from './Registry';
|
||||
|
||||
@@ -64,11 +64,6 @@ export class AddedLinksRegistry extends Registry<AddedLinkRegistryItem[], Plugin
|
||||
continue;
|
||||
}
|
||||
|
||||
if (path && !isLinkPathValid(pluginId, path)) {
|
||||
configLog.error(`${logPrefix} ${errors.INVALID_PATH}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (pluginId !== 'grafana' && isGrafanaDevMode() && isAddedLinkMetaInfoMissing(pluginId, config, configLog)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -27,7 +27,7 @@ import { RestrictedGrafanaApisProvider } from '../components/restrictedGrafanaAp
|
||||
import { ExtensionErrorBoundary } from './ExtensionErrorBoundary';
|
||||
import { ExtensionsLog, log as baseLog } from './logs/log';
|
||||
import { AddedLinkRegistryItem } from './registry/AddedLinksRegistry';
|
||||
import { assertIsNotPromise, assertLinkPathIsValid, assertStringProps, isPromise } from './validators';
|
||||
import { assertIsNotPromise, assertStringProps, isPromise } from './validators';
|
||||
|
||||
export function handleErrorsInFn(fn: Function, errorMessagePrefix = '') {
|
||||
return (...args: unknown[]) => {
|
||||
@@ -481,7 +481,6 @@ export function getLinkExtensionOverrides(
|
||||
`The configure() function for "${config.title}" returned a promise, skipping updates.`
|
||||
);
|
||||
|
||||
path && assertLinkPathIsValid(pluginId, path);
|
||||
assertStringProps({ title, description }, ['title', 'description']);
|
||||
|
||||
if (Object.keys(rest).length > 0) {
|
||||
|
||||
@@ -12,7 +12,6 @@ import { config } from '@grafana/runtime';
|
||||
import { createLogMock } from './logs/testUtils';
|
||||
import {
|
||||
assertConfigureIsValid,
|
||||
assertLinkPathIsValid,
|
||||
assertStringProps,
|
||||
isAddedComponentMetaInfoMissing,
|
||||
isAddedLinkMetaInfoMissing,
|
||||
@@ -25,48 +24,6 @@ import {
|
||||
} from './validators';
|
||||
|
||||
describe('Plugin Extension Validators', () => {
|
||||
describe('assertLinkPathIsValid()', () => {
|
||||
it('should not throw an error if the link path is valid', () => {
|
||||
expect(() => {
|
||||
const pluginId = 'myorg-b-app';
|
||||
const extension = {
|
||||
path: `/a/${pluginId}/overview`,
|
||||
title: 'My Plugin',
|
||||
description: 'My Plugin Description',
|
||||
extensionPointId: '...',
|
||||
};
|
||||
|
||||
assertLinkPathIsValid(pluginId, extension.path);
|
||||
}).not.toThrowError();
|
||||
});
|
||||
|
||||
it('should throw an error if the link path is pointing to a different plugin', () => {
|
||||
expect(() => {
|
||||
const extension = {
|
||||
path: `/a/myorg-b-app/overview`,
|
||||
title: 'My Plugin',
|
||||
description: 'My Plugin Description',
|
||||
extensionPointId: '...',
|
||||
};
|
||||
|
||||
assertLinkPathIsValid('another-plugin-app', extension.path);
|
||||
}).toThrowError();
|
||||
});
|
||||
|
||||
it('should throw an error if the link path is not prefixed with "/a/<PLUGIN_ID>"', () => {
|
||||
expect(() => {
|
||||
const extension = {
|
||||
path: `/some-bad-path`,
|
||||
title: 'My Plugin',
|
||||
description: 'My Plugin Description',
|
||||
extensionPointId: '...',
|
||||
};
|
||||
|
||||
assertLinkPathIsValid('myorg-b-app', extension.path);
|
||||
}).toThrowError();
|
||||
});
|
||||
});
|
||||
|
||||
describe('assertConfigureIsValid()', () => {
|
||||
it('should NOT throw an error if the configure() function is missing', () => {
|
||||
expect(() => {
|
||||
|
||||
@@ -24,14 +24,6 @@ export function assertPluginExtensionLink(
|
||||
}
|
||||
}
|
||||
|
||||
export function assertLinkPathIsValid(pluginId: string, path: string) {
|
||||
if (!isLinkPathValid(pluginId, path)) {
|
||||
throw new Error(
|
||||
`Invalid link extension. The "path" is required and should start with "/a/${pluginId}/" (currently: "${path}"). Skipping the extension.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export function assertIsReactComponent(component: React.ComponentType) {
|
||||
if (!isReactComponent(component)) {
|
||||
throw new Error(`Invalid component extension, the "component" property needs to be a valid React component.`);
|
||||
@@ -62,10 +54,6 @@ export function assertIsNotPromise(value: unknown, errorMessage = 'The provided
|
||||
}
|
||||
}
|
||||
|
||||
export function isLinkPathValid(pluginId: string, path: string) {
|
||||
return Boolean(typeof path === 'string' && path.length > 0 && path.startsWith(`/a/${pluginId}/`));
|
||||
}
|
||||
|
||||
export function isExtensionPointIdValid({
|
||||
extensionPointId,
|
||||
pluginId,
|
||||
|
||||
Reference in New Issue
Block a user