diff --git a/dashboards/src/components/Panel/Panel.tsx b/dashboards/src/components/Panel/Panel.tsx index c0370001..436c72c7 100644 --- a/dashboards/src/components/Panel/Panel.tsx +++ b/dashboards/src/components/Panel/Panel.tsx @@ -129,7 +129,7 @@ export const Panel = memo(function Panel(props: PanelProps) { } try { - const plugin = await getPlugin('Panel', panelPluginKind); + const plugin = await getPlugin({ kind: 'Panel', name: panelPluginKind }); // More defensive checking for plugin and actions if ( diff --git a/dashboards/src/context/DatasourceStoreProvider.tsx b/dashboards/src/context/DatasourceStoreProvider.tsx index 2da70280..d036c802 100644 --- a/dashboards/src/context/DatasourceStoreProvider.tsx +++ b/dashboards/src/context/DatasourceStoreProvider.tsx @@ -106,7 +106,10 @@ export function DatasourceStoreProvider(props: DatasourceStoreProviderProps): Re const getDatasourceClient = useCallback( async function getClient(selector: DatasourceSelector): Promise { const { kind } = selector; - const [{ spec, proxyUrl }, plugin] = await Promise.all([findDatasource(selector), getPlugin('Datasource', kind)]); + const [{ spec, proxyUrl }, plugin] = await Promise.all([ + findDatasource(selector), + getPlugin({ kind: 'Datasource', name: kind }), + ]); // allows extending client const client = plugin.createClient(spec.plugin.spec, { proxyUrl }) as Client; diff --git a/package-lock.json b/package-lock.json index 34c8ef15..31f82198 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14430,10 +14430,9 @@ } }, "node_modules/semver": { - "version": "7.7.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.2.tgz", - "integrity": "sha512-RF0Fw+rO5AMf9MAyaRXI4AV0Ulj5lMHqVxxdSgiVbixSCXoEmmX/jk0CuJw4+3SqroYO9VoUh+HcuJivvtJemA==", - "dev": true, + "version": "7.8.0", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.8.0.tgz", + "integrity": "sha512-AcM7dV/5ul4EekoQ29Agm5vri8JNqRyj39o0qpX6vDF2GZrtutZl5RwgD1XnZjiTAfncsJhMI48QQH3sN87YNA==", "license": "ISC", "bin": { "semver": "bin/semver.js" @@ -16679,6 +16678,7 @@ "date-fns-tz": "^3.2.0", "immer": "^10.1.1", "react-hook-form": "^7.46.1", + "semver": "^7.8.0", "use-query-params": "^2.2.1", "zod": "^3.25.76" }, diff --git a/plugin-system/package.json b/plugin-system/package.json index 9c6c0e25..9f91c355 100644 --- a/plugin-system/package.json +++ b/plugin-system/package.json @@ -37,7 +37,8 @@ "immer": "^10.1.1", "react-hook-form": "^7.46.1", "use-query-params": "^2.2.1", - "zod": "^3.25.76" + "zod": "^3.25.76", + "semver": "^7.8.0" }, "peerDependencies": { "@emotion/react": "^11.14.0", diff --git a/plugin-system/src/components/PluginRegistry/PluginRegistry.tsx b/plugin-system/src/components/PluginRegistry/PluginRegistry.tsx index 8b7b7736..0f679f6a 100644 --- a/plugin-system/src/components/PluginRegistry/PluginRegistry.tsx +++ b/plugin-system/src/components/PluginRegistry/PluginRegistry.tsx @@ -23,7 +23,8 @@ import { } from '../../model'; import { PluginRegistryContext } from '../../runtime'; import { useEvent } from '../../utils'; -import { usePluginIndexes, getTypeAndKindKey } from './plugin-indexes'; +import { usePluginIndexes, PluginCompoundKey } from './plugin-indexes'; +import { lookUpDefaultPluginKey } from './getPluginSearchHelper'; export interface PluginRegistryProps { pluginLoader: PluginLoader; @@ -62,28 +63,68 @@ export function PluginRegistry(props: PluginRegistryProps): ReactElement { }); const getPlugin = useCallback( - async (kind: T, name: string): Promise> => { - // Get the indexes of the installed plugins + async (compoundKeyObj: PluginCompoundKey): Promise> => { const pluginIndexes = await getPluginIndexes(); + let resource: PluginModuleResource | undefined = undefined; + let pluginModule: Record> | undefined; + const { kind, name, version, registry } = compoundKeyObj; - // Figure out what module the plugin is in by looking in the index - const typeAndKindKey = getTypeAndKindKey(kind, name); - const resource = pluginIndexes.pluginResourcesByNameAndKind.get(typeAndKindKey); - if (resource === undefined) { + /** + * By default both version and registry are undefined, + * If one or both are passed, the registry will check if the plugin with the specific version and registry is available, + * falling back to the current behavior which is returning the default. + */ + + if (registry || version) { + let compoundKey = ''; + /** + * This branch tries to look up a resource deterministically, using a compound_key which consists of kind, name, registry, and version + * Based on the user input, the likely keys are + * 1- kind:name:registry:version (This is the complete compound key. It is very likely the key is looked up) + * 2- kind:name::version (This is very likely, because registry is an optional field of the resource object) + * 3- kind:name:registry: (It is impossible to find any combination, because version is a mandatory field of the resource. So, this will be handled by the fallback) + * Note: It is likely that the key is not found. However, the search should NOT give up easily! + * Instead it should continue with the fallback mechanism + */ + compoundKey = `${kind}:${name}:${registry}:${version}`; + resource = pluginIndexes.pluginResourcesByNameKindRegistryVersion.get(compoundKey); + if (resource) { + pluginModule = (await loadPluginModule(resource)) as Record>; + const plugin = pluginModule?.[`${name}:${registry ?? ''}:${version ?? ''}`]; + if (!plugin) { + throw new Error( + `The ${name} plugin for kind '${kind}' is missing from the ${resource.metadata.name} plugin module` + ); + } + return plugin as PluginImplementation; + } + } + /** + * This is the fallback mechanism branch. + * It performs a minimal search using the mandatory inputs from user and returns the default resource + * More information can be found in the searchHelper.ts + */ + const resourceKey = lookUpDefaultPluginKey( + Array.from(pluginIndexes.pluginResourcesByNameKindRegistryVersion.keys()), + { + kind, + name, + } + ); + + if (!resourceKey) { throw new Error(`A ${name} plugin for kind '${kind}' is not installed`); } - // Treat the plugin module as a bunch of named exports that have plugins - const pluginModule = (await loadPluginModule(resource)) as Record>; + resource = pluginIndexes.pluginResourcesByNameKindRegistryVersion.get(resourceKey)!; + pluginModule = (await loadPluginModule(resource)) as Record>; - // We currently assume that plugin modules will have named exports that match the kinds they handle - const plugin = pluginModule[name]; - if (plugin === undefined) { + const plugin = pluginModule?.[resourceKey]; + if (!plugin) { throw new Error( `The ${name} plugin for kind '${kind}' is missing from the ${resource.metadata.name} plugin module` ); } - return plugin as PluginImplementation; }, [getPluginIndexes, loadPluginModule] diff --git a/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.test.ts b/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.test.ts new file mode 100644 index 00000000..a07758a8 --- /dev/null +++ b/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.test.ts @@ -0,0 +1,59 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { lookUpDefaultPluginKey } from './getPluginSearchHelper'; + +describe('getPluginSearchHelper', () => { + describe('same registers but different versions', () => { + const keys: string[] = ['Panel:TimeSeriesChart:dev:1.0.0', 'Panel:TimeSeriesChart:dev:2.0.0']; + it('should take the higher version', () => { + expect(lookUpDefaultPluginKey(keys, { kind: 'Panel', name: 'TimeSeriesChart' })).toBe( + 'Panel:TimeSeriesChart:dev:2.0.0' + ); + }); + }); + + describe('with and without registry, versions race', () => { + const keys: string[] = [ + 'Panel:TimeSeriesChart:dev:2.0.0', + 'Panel:TimeSeriesChart::1.0.0', + 'Panel:TimeSeriesChartX:dev:1.0.0', + 'Panel:TimeSeriesChartX::2.0.0', + ]; + + it('should take the higher version', () => { + expect(lookUpDefaultPluginKey(keys, { kind: 'Panel', name: 'TimeSeriesChart' })).toBe( + 'Panel:TimeSeriesChart:dev:2.0.0' + ); + + expect(lookUpDefaultPluginKey(keys, { kind: 'Panel', name: 'TimeSeriesChartX' })).toBe( + 'Panel:TimeSeriesChartX::2.0.0' + ); + }); + }); + + describe('with and without registry - same versions - check policy', () => { + const keys: string[] = ['Panel:TimeSeriesChart:dev:2.0.0', 'Panel:TimeSeriesChart::2.0.0']; + it('should return the one without the registry by default', () => { + expect(lookUpDefaultPluginKey(keys, { kind: 'Panel', name: 'TimeSeriesChart' })).toBe( + 'Panel:TimeSeriesChart::2.0.0' + ); + }); + + it('should return the one with the registry', () => { + expect( + lookUpDefaultPluginKey(keys, { kind: 'Panel', name: 'TimeSeriesChart' }, { registryOverVersion: true }) + ).toBe('Panel:TimeSeriesChart:dev:2.0.0'); + }); + }); +}); diff --git a/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.ts b/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.ts new file mode 100644 index 00000000..39ddd45f --- /dev/null +++ b/plugin-system/src/components/PluginRegistry/getPluginSearchHelper.ts @@ -0,0 +1,108 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { gt } from 'semver'; +import { PluginType } from '../../model'; +import { PluginCompoundKey } from './plugin-indexes'; + +/** + * ____ LOOK UP DEFAULT PLUGIN KEYS WITH PLUGIN TYPE (KIND) AND NAME ___ + * This is the fallback mechanism to look up the default plugin using the plugin type (kind) and the name + * When the version and registry are not available, the function shortlists all plugins which have the kind and name combination + * If multiple plugins are nominated, the function will follow a precedence policy + * ___ PLUGIN LOOKUP PRECEDENCE POLICY ___ + * The search finds the latest versions available for plugins with and without registry by keeping them in two separate buckets + * 1- If nothing found, simply return undefined + * 2- If only one of the buckets have value, there will be no comparison. Return the one with the value + * 3- If both have the value, check the Precedence Logic input and act accordingly + * 3.1- If the one WITH the registry has a greater version, return it. + * 3.2- If the one WITHOUT the registry has a greater version, return it + * 3.2.1- If we have a draw consider the policy flag + */ + +export interface PluginLookupPrecedenceLogic { + registryOverVersion: boolean; +} + +const PLUGIN_LOOKUP_PRECEDENCE_LOGIC: PluginLookupPrecedenceLogic = { registryOverVersion: false }; + +export const lookUpDefaultPluginKey = ( + pluginModuleResourceMapKeys: string[], + query: Pick, 'kind' | 'name'>, + precedenceLogic: PluginLookupPrecedenceLogic = PLUGIN_LOOKUP_PRECEDENCE_LOGIC +): string | undefined => { + type PluginBucket = { key: string; version: string }; + const latestFoundVersionWithRegistry: PluginBucket = { key: '', version: '' }; + const latestFoundVersionWithoutRegistry: PluginBucket = { key: '', version: '' }; + + for (const np of pluginModuleResourceMapKeys) { + if (!np.startsWith(`${query.kind}:${query.name}:`)) continue; + const split = np.split(':'); + + /** + * This is not a valid key. A valid key has 4 sections. The registry is optional. It might be empty or holds a value + */ + if (split.length !== 4) { + console.warn(`An invalid Plugin Resource key detected during default plugin lookup: ${np}`); + continue; + } + + const [kind, name, registry, version] = split; + /** + * Such a case is representing a wrong key and it is not technically possible (Just to be precautious) + * A resource MUST have kind, name, and version according to its definition and interface + * So skip this record + */ + if (!kind || !name || !version) { + console.warn(`An invalid Plugin Resource key detected during default plugin lookup: ${np}`); + continue; + } + + if (registry) { + if (!latestFoundVersionWithRegistry.key || gt(version, latestFoundVersionWithRegistry.version!)) { + latestFoundVersionWithRegistry.key = np; + latestFoundVersionWithRegistry.version = version; + } + continue; + } + + if (!latestFoundVersionWithoutRegistry.key || gt(version, latestFoundVersionWithoutRegistry.version!)) { + latestFoundVersionWithoutRegistry.key = np; + latestFoundVersionWithoutRegistry.version = version; + } + } + + /* Before it proceeds with the precedence logic, it checks whether so far anything has been found, if not return undefined */ + if ([latestFoundVersionWithRegistry, latestFoundVersionWithoutRegistry].every((p) => !p.key)) { + return undefined; + } + + if (latestFoundVersionWithRegistry.key && latestFoundVersionWithoutRegistry.key) { + const { registryOverVersion } = precedenceLogic; + + // Registry version is strictly higher + if (gt(latestFoundVersionWithRegistry.version, latestFoundVersionWithoutRegistry.version)) { + return latestFoundVersionWithRegistry.key; + } + + // None-registry version is strictly higher + if (gt(latestFoundVersionWithoutRegistry.version, latestFoundVersionWithRegistry.version)) { + return latestFoundVersionWithoutRegistry.key; + } + + // Versions are equal - use the tie-breaker + return registryOverVersion ? latestFoundVersionWithRegistry.key : latestFoundVersionWithoutRegistry.key; + } + + return latestFoundVersionWithRegistry.key || latestFoundVersionWithoutRegistry?.key; +}; diff --git a/plugin-system/src/components/PluginRegistry/plugin-indexes.ts b/plugin-system/src/components/PluginRegistry/plugin-indexes.ts index e83e4dd6..64c9a03e 100644 --- a/plugin-system/src/components/PluginRegistry/plugin-indexes.ts +++ b/plugin-system/src/components/PluginRegistry/plugin-indexes.ts @@ -16,8 +16,8 @@ import { PluginLoader, PluginMetadataWithModule, PluginModuleResource, PluginTyp import { useEvent } from '../../utils'; export interface PluginIndexes { - // Plugin resources by plugin type and kind (i.e. look up what module a plugin type and kind is in) - pluginResourcesByNameAndKind: Map; + // Plugin resources by plugin type, kind, registry, and version + pluginResourcesByNameKindRegistryVersion: Map; // Plugin metadata by plugin type pluginMetadataByKind: Map; } @@ -34,22 +34,26 @@ export function usePluginIndexes( const installedPlugins = await getInstalledPlugins(); // Create the two indexes from the installed plugins - const pluginResourcesByNameAndKind = new Map(); + const pluginResourcesByNameKindRegistryVersion = new Map(); const pluginMetadataByKind = new Map(); for (const resource of installedPlugins) { + const { + metadata: { version, registry }, + } = resource; for (const pluginMetadata of resource.spec.plugins) { const { kind, spec: { name }, } = pluginMetadata; - // Index the plugin by type and kind to point at the module that contains it - const key = getTypeAndKindKey(kind, name); - if (pluginResourcesByNameAndKind.has(key)) { - console.warn(`Got more than one ${kind} plugin for kind ${name}`); + const key = getPluginModuleCompoundKey({ kind, name, registry, version }); + if (pluginResourcesByNameKindRegistryVersion.has(key)) { + console.warn( + `Got more than one ${kind} plugin for kind ${name}, registry '${registry || 'undefined'}', and version '${version || 'undefined'}'` + ); } - pluginResourcesByNameAndKind.set(key, resource); + pluginResourcesByNameKindRegistryVersion.set(key, resource); // Index the metadata by plugin type let list = pluginMetadataByKind.get(kind); @@ -62,7 +66,7 @@ export function usePluginIndexes( } return { - pluginResourcesByNameAndKind, + pluginResourcesByNameKindRegistryVersion, pluginMetadataByKind, }; }); @@ -84,9 +88,17 @@ export function usePluginIndexes( return getPluginIndexes; } +export type PluginCompoundKey = { + kind: T; + name: string; + registry?: string; + version?: string; +}; + /** - * Gets a unique key for a plugin type/kind that can be used as a cache key. + * Gets a unique key for a plugin type/kind/version/registry that can be used as a cache key. */ -export function getTypeAndKindKey(kind: PluginType, name: string): string { - return `${kind}:${name}`; +export function getPluginModuleCompoundKey(compoundKey: PluginCompoundKey): string { + const { kind, name, registry, version } = compoundKey; + return `${kind}:${name}:${registry ?? ''}:${version ?? ''}`; } diff --git a/plugin-system/src/components/PluginSpecEditor/PluginSpecEditor.tsx b/plugin-system/src/components/PluginSpecEditor/PluginSpecEditor.tsx index 01501b27..7f2a9551 100644 --- a/plugin-system/src/components/PluginSpecEditor/PluginSpecEditor.tsx +++ b/plugin-system/src/components/PluginSpecEditor/PluginSpecEditor.tsx @@ -30,7 +30,6 @@ export function PluginSpecEditor(props: PluginSpecEditorProps): ReactElement | n ...others } = props; const { data: plugin, isLoading, error } = usePlugin(pluginType, pluginKind); - if (error) { return ; } diff --git a/plugin-system/src/model/plugin-loading.ts b/plugin-system/src/model/plugin-loading.ts index 7142c86b..48016d3f 100644 --- a/plugin-system/src/model/plugin-loading.ts +++ b/plugin-system/src/model/plugin-loading.ts @@ -35,20 +35,33 @@ export interface DynamicImportPlugin { * the plugin itself via a dynamic `import()` statement. */ export function dynamicImportPluginLoader(plugins: DynamicImportPlugin[]): PluginLoader { - const importMap: Map = new Map( - plugins.map((plugin) => [plugin.resource, plugin.importPlugin]) - ); - + const importMap: Map = + new Map(); + for (const p of plugins) { + const { + resource, + resource: { + kind, + metadata: { name, registry, version }, + }, + importPlugin, + } = p; + importMap.set(`${kind}:${name}:${registry ?? ''}:${version ?? ''}`, { resource, importPlugin }); + } return { async getInstalledPlugins(): Promise { - return Promise.resolve(Array.from(importMap.keys())); + return Promise.resolve(Array.from(importMap.values()).map((v) => v.resource)); }, importPluginModule(resource): Promise { - const importFn = importMap.get(resource); - if (importFn === undefined) { + const { + kind, + metadata: { name, version, registry }, + } = resource; + const { importPlugin } = importMap.get(`${kind}:${name}:${registry ?? ''}:${version ?? ''}`) || {}; + if (importPlugin === undefined) { throw new Error('Plugin not found'); } - return importFn(); + return importPlugin(); }, }; } diff --git a/plugin-system/src/model/plugins.ts b/plugin-system/src/model/plugins.ts index 4c5a76c3..044a1f54 100644 --- a/plugin-system/src/model/plugins.ts +++ b/plugin-system/src/model/plugins.ts @@ -37,6 +37,8 @@ export interface PluginMetadata { kind: PluginType; spec: { name: string; + version?: string; + registry?: string; display: { name: string; description?: string; @@ -50,6 +52,7 @@ export interface PluginMetadata { export interface PluginModuleMetadata { name: string; version: string; + registry?: string; } /** diff --git a/plugin-system/src/remote/PersesPlugin.types.ts b/plugin-system/src/remote/PersesPlugin.types.ts index 05b14d09..84b6575c 100644 --- a/plugin-system/src/remote/PersesPlugin.types.ts +++ b/plugin-system/src/remote/PersesPlugin.types.ts @@ -13,6 +13,8 @@ export interface PersesPlugin { name: string; + version?: string; + registry?: string; moduleName: string; baseURL?: string; } diff --git a/plugin-system/src/remote/PluginRuntime.tsx b/plugin-system/src/remote/PluginRuntime.tsx index b6409280..accec308 100644 --- a/plugin-system/src/remote/PluginRuntime.tsx +++ b/plugin-system/src/remote/PluginRuntime.tsx @@ -214,31 +214,40 @@ const getPluginRuntime = (): ModuleFederation => { return instance; }; -const registerRemote = (name: string, baseURL?: string): void => { +const registerRemote = (name: string, registry?: string, version?: string, baseURL?: string): void => { const pluginRuntime = getPluginRuntime(); - const existingRemote = pluginRuntime.options.remotes.find((remote) => remote.name === name); + const registryName = `${name}:${registry ?? ''}:${version ?? ''}`; + + const existingRemote = pluginRuntime.options.remotes.find((remote) => remote.name === registryName); if (!existingRemote) { - const remoteEntryURL = baseURL ? `${baseURL}/${name}/mf-manifest.json` : `/plugins/${name}/mf-manifest.json`; + const segments = [registry, version].filter(Boolean); + const suffix = segments.length ? `/${segments.join('/')}` : ''; + const prefix = baseURL || '/plugins'; + const remoteEntryURL = `${prefix}/${name}${suffix}/mf-manifest.json`.replace(/\/+/g, '/'); + pluginRuntime.registerRemotes([ { - name, + name: registryName, entry: remoteEntryURL, - alias: name, + alias: registryName, }, ]); } }; -export const loadPlugin = async ( - moduleName: string, - pluginName: string, - baseURL?: string -): Promise => { - registerRemote(moduleName, baseURL); +export const loadPlugin = async (target: { + moduleName: string; + pluginName: string; + registry?: string; + version?: string; + baseURL?: string; +}): Promise => { + const { moduleName, pluginName, registry, version, baseURL } = target; + registerRemote(moduleName, registry, version, baseURL); const pluginRuntime = getPluginRuntime(); - - return pluginRuntime.loadRemote(`${moduleName}/${pluginName}`); + const registryName = `${moduleName}:${registry ?? ''}:${version ?? ''}`; + return pluginRuntime.loadRemote(`${registryName}/${pluginName}`); }; export function usePluginRuntime({ plugin }: { plugin: PersesPlugin }): { @@ -247,6 +256,9 @@ export function usePluginRuntime({ plugin }: { plugin: PersesPlugin }): { } { return { pluginRuntime: getPluginRuntime(), - loadPlugin: () => loadPlugin(plugin.moduleName, plugin.name, plugin.baseURL), + loadPlugin: (): Promise => { + const { moduleName, name: pluginName, registry, version, baseURL } = plugin; + return loadPlugin({ moduleName, pluginName, registry, version, baseURL }); + }, }; } diff --git a/plugin-system/src/remote/remotePluginLoader.test.ts b/plugin-system/src/remote/remotePluginLoader.test.ts index b60a251a..44950209 100644 --- a/plugin-system/src/remote/remotePluginLoader.test.ts +++ b/plugin-system/src/remote/remotePluginLoader.test.ts @@ -128,8 +128,17 @@ describe('remotePluginLoader', () => { const loader = remotePluginLoader(); const result = await loader.importPluginModule(MOCK_VALID_PLUGIN_MODULE_RESOURCE); - expect(mockLoadPlugin).toHaveBeenCalledWith('test-module', 'testPlugin', '/plugins'); - expect(result).toEqual(MOCK_REMOTE_PLUGIN_MODULE); + expect(mockLoadPlugin).toHaveBeenCalledWith({ + moduleName: 'test-module', + pluginName: 'testPlugin', + baseURL: '/plugins', + version: '1.0.0', + }); + expect(result).toEqual({ + 'testPlugin::1.0.0': { + ...MOCK_REMOTE_PLUGIN_MODULE.testPlugin, + }, + }); expect(mockConsoleError).not.toHaveBeenCalled(); }); @@ -154,8 +163,17 @@ describe('remotePluginLoader', () => { const loader = remotePluginLoader(testCase.options); const result = await loader.importPluginModule(MOCK_VALID_PLUGIN_MODULE_RESOURCE); - expect(mockLoadPlugin).toHaveBeenCalledWith('test-module', 'testPlugin', testCase.expected); - expect(result).toEqual(MOCK_REMOTE_PLUGIN_MODULE); + expect(mockLoadPlugin).toHaveBeenCalledWith({ + moduleName: 'test-module', + pluginName: 'testPlugin', + baseURL: testCase.expected, + version: '1.0.0', + }); + expect(result).toEqual({ + 'testPlugin::1.0.0': { + ...MOCK_REMOTE_PLUGIN_MODULE.testPlugin, + }, + }); expect(mockConsoleError).not.toHaveBeenCalled(); } @@ -174,11 +192,22 @@ describe('remotePluginLoader', () => { const result = await loader.importPluginModule(multiPluginModule); expect(mockLoadPlugin).toHaveBeenCalledTimes(2); - expect(mockLoadPlugin).toHaveBeenNthCalledWith(1, 'multi-plugin-module', 'plugin1', '/plugins'); - expect(mockLoadPlugin).toHaveBeenNthCalledWith(2, 'multi-plugin-module', 'plugin2', '/plugins'); + expect(mockLoadPlugin).toHaveBeenNthCalledWith(1, { + moduleName: 'multi-plugin-module', + pluginName: 'plugin1', + baseURL: '/plugins', + version: '1.0.0', + }); + expect(mockLoadPlugin).toHaveBeenNthCalledWith(2, { + moduleName: 'multi-plugin-module', + pluginName: 'plugin2', + baseURL: '/plugins', + version: '1.0.0', + }); + expect(result).toEqual({ - plugin1: { component: expect.any(Function) }, - plugin2: { component: expect.any(Function) }, + 'plugin1::1.0.0': { component: expect.any(Function) }, + 'plugin2::1.0.0': { component: expect.any(Function) }, }); }); @@ -193,7 +222,7 @@ describe('remotePluginLoader', () => { const result = await loader.importPluginModule(multiPluginModule); expect(result).toEqual({ - workingPlugin: { component: expect.any(Function) }, + 'workingPlugin::1.0.0': { component: expect.any(Function) }, }); expect(mockConsoleError).toHaveBeenCalledWith('RemotePluginLoader: Error loading plugin failingPlugin'); }); @@ -286,5 +315,34 @@ describe('remotePluginLoader', () => { expect(result).toHaveLength(1); expect(result[0]).toEqual(MOCK_VALID_PLUGIN_MODULE_RESOURCE); }); + + it('should call loadPlugin with specific version and registry', async () => { + mockLoadPlugin.mockResolvedValue({}); + const loader = remotePluginLoader(); + await loader.importPluginModule({ + kind: 'PluginModule', + metadata: { + name: 'custom-module', + version: '0.1.0', + }, + spec: { + plugins: [ + { + kind: 'Panel', + spec: { + display: { name: 'p1', description: 'p1 plugin' }, + name: 'p1', + }, + }, + ], + }, + }); + expect(mockLoadPlugin).toHaveBeenCalledWith({ + moduleName: 'custom-module', + pluginName: 'p1', + version: '0.1.0', + baseURL: '/plugins', + }); + }); }); }); diff --git a/plugin-system/src/remote/remotePluginLoader.ts b/plugin-system/src/remote/remotePluginLoader.ts index 88c61ba4..4d98afc6 100644 --- a/plugin-system/src/remote/remotePluginLoader.ts +++ b/plugin-system/src/remote/remotePluginLoader.ts @@ -88,7 +88,6 @@ export function remotePluginLoader(options?: RemotePluginLoaderOptions): PluginL const pluginsResponse = await fetch(pluginsApiPath); const plugins = await pluginsResponse.json(); - let pluginModules: PluginModuleResource[] = []; if (Array.isArray(plugins)) { @@ -104,20 +103,35 @@ export function remotePluginLoader(options?: RemotePluginLoaderOptions): PluginL return pluginModules; }, importPluginModule: async (resource): Promise => { - const pluginModuleName = resource.metadata.name; + const { name: pluginModuleName, version, registry } = resource.metadata; const pluginModule: RemotePluginModule = {}; - for (const plugin of resource.spec.plugins) { - const remotePluginModule = await loadPlugin(pluginModuleName, plugin.spec.name, pluginsAssetsPath); + const loadPromises = resource.spec.plugins.map(async (plugin) => { + const remotePluginModule = await loadPlugin({ + moduleName: pluginModuleName, + pluginName: plugin.spec.name, + registry, + version, + baseURL: pluginsAssetsPath, + }); const remotePlugin = remotePluginModule?.[plugin.spec.name]; if (remotePlugin) { - pluginModule[plugin.spec.name] = remotePlugin; + return { name: plugin.spec.name, remotePlugin }; } else { console.error(`RemotePluginLoader: Error loading plugin ${plugin.spec.name}`); + return null; } - } + }); + + const loadedPlugins = await Promise.all(loadPromises); + + loadedPlugins.forEach((item) => { + if (item?.remotePlugin) { + pluginModule[`${item.name}:${registry ?? ''}:${version ?? ''}`] = item.remotePlugin; + } + }); return pluginModule; }, diff --git a/plugin-system/src/runtime/log-queries.ts b/plugin-system/src/runtime/log-queries.ts index f3b208c9..f3e69128 100644 --- a/plugin-system/src/runtime/log-queries.ts +++ b/plugin-system/src/runtime/log-queries.ts @@ -46,7 +46,7 @@ export function useLogQueries(definitions: LogQueryDefinition[]): Array => { - const plugin = await getPlugin(LOG_QUERY_KEY, logQueryKind); + const plugin = await getPlugin({ kind: LOG_QUERY_KEY, name: logQueryKind }); const data = await plugin.getLogData(definition.spec.plugin.spec, context, signal); return data; }, diff --git a/plugin-system/src/runtime/plugin-registry.ts b/plugin-system/src/runtime/plugin-registry.ts index f8d4882c..4b40257f 100644 --- a/plugin-system/src/runtime/plugin-registry.ts +++ b/plugin-system/src/runtime/plugin-registry.ts @@ -15,9 +15,10 @@ import { BuiltinVariableDefinition } from '@perses-dev/spec'; import { useQueries, useQuery, UseQueryOptions, UseQueryResult } from '@tanstack/react-query'; import { createContext, useContext } from 'react'; import { DefaultPluginKinds, PluginImplementation, PluginMetadataWithModule, PluginType } from '../model'; +import { PluginCompoundKey } from '../components/PluginRegistry/plugin-indexes'; export interface PluginRegistryContextType { - getPlugin(kind: T, name: string): Promise>; + getPlugin(compoundKey: PluginCompoundKey): Promise>; listPluginMetadata(pluginTypes: PluginType[]): Promise; defaultPluginKinds?: DefaultPluginKinds; } @@ -58,7 +59,7 @@ export function usePlugin( const { getPlugin } = usePluginRegistry(); return useQuery({ queryKey: ['getPlugin', pluginType, kind], - queryFn: () => getPlugin(pluginType!, kind), + queryFn: () => getPlugin({ kind: pluginType!, name: kind }), ...options, }); } @@ -82,7 +83,7 @@ export function usePlugins( queries: kinds.map((kind) => { return { queryKey: ['getPlugin', pluginType, kind], - queryFn: () => getPlugin(pluginType, kind), + queryFn: () => getPlugin({ kind: pluginType, name: kind }), }; }), }); @@ -122,7 +123,7 @@ export function usePluginBuiltinVariableDefinitions(): UseQueryResult datasource.spec.name)); const result: BuiltinVariableDefinition[] = []; for (const name of datasourceNames) { - const plugin = await getPlugin('Datasource', name); + const plugin = await getPlugin({ kind: 'Datasource', name }); if (plugin.getBuiltinVariableDefinitions) { plugin.getBuiltinVariableDefinitions().forEach((definition) => result.push(definition)); } diff --git a/plugin-system/src/runtime/profile-queries.ts b/plugin-system/src/runtime/profile-queries.ts index f94e3c7c..3629b475 100644 --- a/plugin-system/src/runtime/profile-queries.ts +++ b/plugin-system/src/runtime/profile-queries.ts @@ -46,7 +46,7 @@ export function useProfileQueries(definitions: ProfileQueryDefinition[]): Array< refetchOnReconnect: false, staleTime: Infinity, queryFn: async ({ signal }: { signal?: AbortSignal }): Promise => { - const plugin = await getPlugin(PROFILE_QUERY_KEY, profileQueryKind); + const plugin = await getPlugin({ kind: PROFILE_QUERY_KEY, name: profileQueryKind }); const data = await plugin.getProfileData(definition.spec.plugin.spec, context, signal); return data; }, diff --git a/plugin-system/src/runtime/time-series-queries.ts b/plugin-system/src/runtime/time-series-queries.ts index 2c7b54ea..0b9d977e 100644 --- a/plugin-system/src/runtime/time-series-queries.ts +++ b/plugin-system/src/runtime/time-series-queries.ts @@ -139,7 +139,7 @@ export function useTimeSeriesQueries( staleTime: Infinity, queryKey: queryKey, queryFn: async ({ signal }: { signal: AbortSignal }): Promise => { - const plugin = await getPlugin(TIME_SERIES_QUERY_KEY, definition.spec.plugin.kind); + const plugin = await getPlugin({ kind: TIME_SERIES_QUERY_KEY, name: definition.spec.plugin.kind }); const data = await plugin.getTimeSeriesData(definition.spec.plugin.spec, context, signal); return data; }, diff --git a/plugin-system/src/runtime/trace-queries.ts b/plugin-system/src/runtime/trace-queries.ts index 2835fd9d..8e3e7670 100644 --- a/plugin-system/src/runtime/trace-queries.ts +++ b/plugin-system/src/runtime/trace-queries.ts @@ -51,7 +51,7 @@ export function useTraceQueries(definitions: TraceQueryDefinition[]): Array => { - const plugin = await getPlugin(TRACE_QUERY_KEY, traceQueryKind); + const plugin = await getPlugin({ kind: TRACE_QUERY_KEY, name: traceQueryKind }); const data = await plugin.getTraceData(definition.spec.plugin.spec, context, signal); return data; }, diff --git a/plugin-system/src/test-utils/mock-plugin-registry.tsx b/plugin-system/src/test-utils/mock-plugin-registry.tsx index 6a213df3..b7bca147 100644 --- a/plugin-system/src/test-utils/mock-plugin-registry.tsx +++ b/plugin-system/src/test-utils/mock-plugin-registry.tsx @@ -32,7 +32,7 @@ export function mockPluginRegistry(...mockPlugins: MockPlugin[]): Omit> = {}; for (const mockPlugin of mockPlugins) { // "Export" on the module under the same name as the kind the plugin handles - mockPluginModule[mockPlugin.spec.name] = mockPlugin.plugin; + mockPluginModule[ + `${mockPlugin.kind}:${mockPlugin.spec.name}:${mockPluginResource.metadata.registry ?? ''}:${mockPluginResource.metadata.version}` + ] = mockPlugin.plugin; } const pluginLoader: PluginLoader = { diff --git a/plugin-system/src/test/test-plugins/bert/index.tsx b/plugin-system/src/test/test-plugins/bert/index.tsx index 2912c2c1..609d8ef8 100644 --- a/plugin-system/src/test/test-plugins/bert/index.tsx +++ b/plugin-system/src/test/test-plugins/bert/index.tsx @@ -29,7 +29,7 @@ function BertPanel1Editor({ value, onChange }: OptionsEditorProps<{ option1: str } // Dummy plugins to test loading -export const BertPanel1: PanelPlugin<{ option1: string }> = { +const BertPanel1: PanelPlugin<{ option1: string }> = { PanelComponent: () => null, panelOptionsEditorComponents: [ { @@ -54,7 +54,7 @@ function BertPanel2Editor({ value, onChange }: OptionsEditorProps<{ option2: str ); } -export const BertPanel2: PanelPlugin<{ option2: string }> = { +const BertPanel2: PanelPlugin<{ option2: string }> = { PanelComponent: () => null, panelOptionsEditorComponents: [ { @@ -71,3 +71,8 @@ export const BertPanel2: PanelPlugin<{ option2: string }> = { createInitialOptions: () => ({ option2: '' }), hideQueryEditor: true, }; + +export const plugins = { + 'Panel:BertPanel1::1.0.0': BertPanel1, + 'Panel:BertPanel2::1.0.0': BertPanel2, +}; diff --git a/plugin-system/src/test/test-plugins/bert/plugin.json b/plugin-system/src/test/test-plugins/bert/plugin.json index 4183596b..b613f2fd 100644 --- a/plugin-system/src/test/test-plugins/bert/plugin.json +++ b/plugin-system/src/test/test-plugins/bert/plugin.json @@ -1,6 +1,6 @@ { "kind": "PluginModule", - "metadata": { "name": "Bert" }, + "metadata": { "name": "Bert", "version": "1.0.0" }, "spec": { "plugins": [ { diff --git a/plugin-system/src/test/test-plugins/ernie/index.tsx b/plugin-system/src/test/test-plugins/ernie/index.tsx index ed8cea85..1912d3d4 100644 --- a/plugin-system/src/test/test-plugins/ernie/index.tsx +++ b/plugin-system/src/test/test-plugins/ernie/index.tsx @@ -19,7 +19,7 @@ const data: VariableOption[] = [ ]; // Dummy plugin to test loading -export const ErnieVariable1: VariablePlugin<{ variableOption: string }> = { +const ErnieVariable1: VariablePlugin<{ variableOption: string }> = { getVariableOptions: async () => ({ data }), OptionsEditorComponent: function ErnieVariableEditor({ value, onChange }) { return ( @@ -37,7 +37,7 @@ export const ErnieVariable1: VariablePlugin<{ variableOption: string }> = { createInitialOptions: () => ({ variableOption: '' }), }; -export const ErnieVariable2: VariablePlugin<{ variableOption2: string }> = { +const ErnieVariable2: VariablePlugin<{ variableOption2: string }> = { getVariableOptions: async () => ({ data }), OptionsEditorComponent: function ErnieVariableEditor({ value, onChange }) { return ( @@ -54,3 +54,8 @@ export const ErnieVariable2: VariablePlugin<{ variableOption2: string }> = { }, createInitialOptions: () => ({ variableOption2: '' }), }; + +export const plugins = { + 'Variable:ErnieVariable1::1.0.0': ErnieVariable1, + 'Variable:ErnieVariable2::1.0.0': ErnieVariable2, +}; diff --git a/plugin-system/src/test/test-plugins/ernie/plugin.json b/plugin-system/src/test/test-plugins/ernie/plugin.json index 300e9d86..95f09d3e 100644 --- a/plugin-system/src/test/test-plugins/ernie/plugin.json +++ b/plugin-system/src/test/test-plugins/ernie/plugin.json @@ -1,6 +1,6 @@ { "kind": "PluginModule", - "metadata": { "name": "Ernie" }, + "metadata": { "name": "Ernie", "version": "1.0.0" }, "spec": { "plugins": [ { diff --git a/plugin-system/src/test/test-plugins/index.ts b/plugin-system/src/test/test-plugins/index.ts index 881a5a04..def200f9 100644 --- a/plugin-system/src/test/test-plugins/index.ts +++ b/plugin-system/src/test/test-plugins/index.ts @@ -19,6 +19,6 @@ import ernieResource from './ernie/plugin.json'; * A PluginLoader for tests that will dynamically load the plugins in this folder. */ export const testPluginLoader: PluginLoader = dynamicImportPluginLoader([ - { resource: bertResource as PluginModuleResource, importPlugin: () => import('./bert') }, - { resource: ernieResource as PluginModuleResource, importPlugin: () => import('./ernie') }, + { resource: bertResource as PluginModuleResource, importPlugin: () => import('./bert').then((m) => m.plugins) }, + { resource: ernieResource as PluginModuleResource, importPlugin: () => import('./ernie').then((m) => m.plugins) }, ]);