From e754c5a6c676fce47eb3cb02c3eb55769caf5e7f Mon Sep 17 00:00:00 2001 From: Hugo Kiyodi Oshiro Date: Fri, 10 Nov 2023 12:28:36 +0100 Subject: [PATCH] Plugins: Change managed plugins installation call (#77120) --- pkg/api/api.go | 2 +- pkg/api/plugins_test.go | 2 + .../pluginaccesscontrol/accesscontrol.go | 4 +- public/app/features/plugins/admin/api.ts | 12 +++++- .../InstallControls/InstallControlsButton.tsx | 12 ++++-- .../admin/components/PluginActions.tsx | 3 +- .../app/features/plugins/admin/constants.ts | 1 + .../features/plugins/admin/helpers.test.ts | 23 ++++++----- public/app/features/plugins/admin/helpers.ts | 38 +++++++++++++++---- .../plugins/admin/hooks/usePluginConfig.tsx | 2 +- .../features/plugins/admin/state/actions.ts | 22 ++++++++--- public/app/features/plugins/admin/types.ts | 7 ++++ 12 files changed, 95 insertions(+), 33 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index cd639f935b6..43d860080d6 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -392,7 +392,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Any("/plugin-proxy/:pluginId/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.ProxyPluginRequest) apiRoute.Any("/plugin-proxy/:pluginId", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), authorize(ac.EvalPermission(pluginaccesscontrol.ActionAppAccess, pluginIDScope)), hs.ProxyPluginRequest) - if hs.Cfg.PluginAdminEnabled && !hs.Cfg.PluginAdminExternalManageEnabled { + if hs.Cfg.PluginAdminEnabled && (hs.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagManagedPluginsInstall) || !hs.Cfg.PluginAdminExternalManageEnabled) { apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) { pluginRoute.Post("/:pluginId/install", authorize(ac.EvalPermission(pluginaccesscontrol.ActionInstall)), routing.Wrap(hs.InstallPlugin)) pluginRoute.Post("/:pluginId/uninstall", authorize(ac.EvalPermission(pluginaccesscontrol.ActionInstall)), routing.Wrap(hs.UninstallPlugin)) diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index e0f6a1457b2..9f5af9cb26c 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -67,6 +67,8 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { hs.Cfg = &setting.Cfg{ PluginAdminEnabled: tc.pluginAdminEnabled, PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled} + hs.Cfg.IsFeatureToggleEnabled = func(_ string) bool { return false } + hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} hs.pluginInstaller = NewFakePluginInstaller() hs.pluginFileStore = &fakes.FakePluginFileStore{} diff --git a/pkg/services/pluginsintegration/pluginaccesscontrol/accesscontrol.go b/pkg/services/pluginsintegration/pluginaccesscontrol/accesscontrol.go index bb0b0aa4a95..de00dcb5e27 100644 --- a/pkg/services/pluginsintegration/pluginaccesscontrol/accesscontrol.go +++ b/pkg/services/pluginsintegration/pluginaccesscontrol/accesscontrol.go @@ -3,6 +3,7 @@ package pluginaccesscontrol import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/setting" ) @@ -67,7 +68,8 @@ func DeclareRBACRoles(service ac.Service, cfg *setting.Cfg) error { Grants: []string{ac.RoleGrafanaAdmin}, } - if !cfg.PluginAdminEnabled || cfg.PluginAdminExternalManageEnabled { + if !cfg.PluginAdminEnabled || + (cfg.PluginAdminExternalManageEnabled && !cfg.IsFeatureToggleEnabled(featuremgmt.FlagManagedPluginsInstall)) { PluginsMaintainer.Grants = []string{} } diff --git a/public/app/features/plugins/admin/api.ts b/public/app/features/plugins/admin/api.ts index da44c8538fe..d1ea94be33a 100644 --- a/public/app/features/plugins/admin/api.ts +++ b/public/app/features/plugins/admin/api.ts @@ -2,9 +2,9 @@ import { PluginError, PluginMeta, renderMarkdown } from '@grafana/data'; import { getBackendSrv, isFetchError } from '@grafana/runtime'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; -import { API_ROOT, GCOM_API_ROOT } from './constants'; +import { API_ROOT, GCOM_API_ROOT, INSTANCE_API_ROOT } from './constants'; import { isLocalPluginVisibleByConfig, isRemotePluginVisibleByConfig } from './helpers'; -import { LocalPlugin, RemotePlugin, CatalogPluginDetails, Version, PluginVersion } from './types'; +import { LocalPlugin, RemotePlugin, CatalogPluginDetails, Version, PluginVersion, InstancePlugin } from './types'; export async function getPluginDetails(id: string): Promise { const remote = await getRemotePlugin(id); @@ -105,6 +105,14 @@ export async function getLocalPlugins(): Promise { return localPlugins.filter(isLocalPluginVisibleByConfig); } +export async function getInstancePlugins(): Promise { + const { items: instancePlugins }: { items: InstancePlugin[] } = await getBackendSrv().get( + `${INSTANCE_API_ROOT}/plugins` + ); + + return instancePlugins; +} + export async function installPlugin(id: string) { // This will install the latest compatible version based on the logic // on the backend. diff --git a/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx b/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx index 34595d30393..44edb733ab2 100644 --- a/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx +++ b/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx @@ -5,6 +5,7 @@ import { AppEvents } from '@grafana/data'; import { config, locationService } from '@grafana/runtime'; import { Button, HorizontalGroup, ConfirmModal } from '@grafana/ui'; import appEvents from 'app/core/app_events'; +import configCore from 'app/core/config'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { removePluginFromNavTree } from 'app/core/reducers/navBarTree'; import { useDispatch } from 'app/types'; @@ -65,13 +66,18 @@ export function InstallControlsButton({ const onInstall = async () => { trackPluginInstalled(trackingProps); const result = await install(plugin.id, latestCompatibleVersion?.version); - // refresh the store to have the new installed plugin - await fetchDetails(plugin.id); if (!errorInstalling && !('error' in result)) { - appEvents.emit(AppEvents.alertSuccess, [`Installed ${plugin.name}`]); + let successMessage = `Installed ${plugin.name}`; + if (config.pluginAdminExternalManageEnabled && configCore.featureToggles.managedPluginsInstall) { + successMessage = 'Install requested, this may take a few minutes.'; + } + + appEvents.emit(AppEvents.alertSuccess, [successMessage]); if (plugin.type === 'app') { setNeedReload?.(true); } + + await fetchDetails(plugin.id); } }; diff --git a/public/app/features/plugins/admin/components/PluginActions.tsx b/public/app/features/plugins/admin/components/PluginActions.tsx index 4ee30e3f0b6..e5df960c1f6 100644 --- a/public/app/features/plugins/admin/components/PluginActions.tsx +++ b/public/app/features/plugins/admin/components/PluginActions.tsx @@ -4,6 +4,7 @@ import React, { useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { config } from '@grafana/runtime'; import { HorizontalGroup, Icon, useStyles2, VerticalGroup } from '@grafana/ui'; +import configCore from 'app/core/config'; import { GetStartedWithPlugin } from '../components/GetStartedWithPlugin'; import { InstallControlsButton } from '../components/InstallControls'; @@ -40,7 +41,7 @@ export const PluginActions = ({ plugin }: Props) => { {!isInstallControlsDisabled && ( <> - {isExternallyManaged && !hasInstallWarning ? ( + {isExternallyManaged && !hasInstallWarning && !configCore.featureToggles.managedPluginsInstall ? ( { ]; test('adds all available plugins only once', () => { - const merged = mergeLocalsAndRemotes(localPlugins, remotePlugins); + const merged = mergeLocalsAndRemotes({ local: localPlugins, remote: remotePlugins }); const mergedIds = merged.map(({ id }) => id); expect(merged.length).toBe(4); @@ -48,7 +48,7 @@ describe('Plugins/Helpers', () => { }); test('merges all plugins with their counterpart (if available)', () => { - const merged = mergeLocalsAndRemotes(localPlugins, remotePlugins); + const merged = mergeLocalsAndRemotes({ local: localPlugins, remote: remotePlugins }); const findMerged = (mergedId: string) => merged.find(({ id }) => id === mergedId); // Both local & remote counterparts @@ -67,10 +67,10 @@ describe('Plugins/Helpers', () => { }); test('skips deprecated plugins unless they have a local - installed - counterpart', () => { - const merged = mergeLocalsAndRemotes(localPlugins, [ - ...remotePlugins, - getRemotePluginMock({ slug: 'plugin-5', status: RemotePluginStatus.Deprecated }), - ]); + const merged = mergeLocalsAndRemotes({ + local: localPlugins, + remote: [...remotePlugins, getRemotePluginMock({ slug: 'plugin-5', status: RemotePluginStatus.Deprecated })], + }); const findMerged = (mergedId: string) => merged.find(({ id }) => id === mergedId); expect(merged).toHaveLength(4); @@ -78,10 +78,10 @@ describe('Plugins/Helpers', () => { }); test('keeps deprecated plugins in case they have a local counterpart', () => { - const merged = mergeLocalsAndRemotes( - [...localPlugins, getLocalPluginMock({ id: 'plugin-5' })], - [...remotePlugins, getRemotePluginMock({ slug: 'plugin-5', status: RemotePluginStatus.Deprecated })] - ); + const merged = mergeLocalsAndRemotes({ + local: [...localPlugins, getLocalPluginMock({ id: 'plugin-5' })], + remote: [...remotePlugins, getRemotePluginMock({ slug: 'plugin-5', status: RemotePluginStatus.Deprecated })], + }); const findMerged = (mergedId: string) => merged.find(({ id }) => id === mergedId); expect(merged).toHaveLength(5); @@ -132,6 +132,7 @@ describe('Plugins/Helpers', () => { signature: 'valid', type: 'app', updatedAt: '2021-05-18T14:53:01.000Z', + isFullyInstalled: false, }); }); @@ -210,6 +211,7 @@ describe('Plugins/Helpers', () => { type: 'app', updatedAt: '2021-08-25', installedVersion: '4.2.2', + isFullyInstalled: true, }); }); @@ -259,6 +261,7 @@ describe('Plugins/Helpers', () => { type: 'app', updatedAt: '2021-05-18T14:53:01.000Z', installedVersion: '4.2.2', + isFullyInstalled: true, }); }); diff --git a/public/app/features/plugins/admin/helpers.ts b/public/app/features/plugins/admin/helpers.ts index 8b695b3f9bb..c870fa9fbe0 100644 --- a/public/app/features/plugins/admin/helpers.ts +++ b/public/app/features/plugins/admin/helpers.ts @@ -1,20 +1,31 @@ import { PluginSignatureStatus, dateTimeParse, PluginError, PluginType, PluginErrorCode } from '@grafana/data'; import { config, featureEnabled } from '@grafana/runtime'; -import { Settings } from 'app/core/config'; +import configCore, { Settings } from 'app/core/config'; import { contextSrv } from 'app/core/core'; import { getBackendSrv } from 'app/core/services/backend_srv'; import { AccessControlAction } from 'app/types'; -import { CatalogPlugin, LocalPlugin, RemotePlugin, RemotePluginStatus, Version } from './types'; +import { CatalogPlugin, InstancePlugin, LocalPlugin, RemotePlugin, RemotePluginStatus, Version } from './types'; -export function mergeLocalsAndRemotes( - local: LocalPlugin[] = [], - remote: RemotePlugin[] = [], - errors?: PluginError[] -): CatalogPlugin[] { +export function mergeLocalsAndRemotes({ + local = [], + remote = [], + instance = [], + pluginErrors: errors, +}: { + local: LocalPlugin[]; + remote?: RemotePlugin[]; + instance?: InstancePlugin[]; + pluginErrors?: PluginError[]; +}): CatalogPlugin[] { const catalogPlugins: CatalogPlugin[] = []; const errorByPluginId = groupErrorsByPluginId(errors); + const instancesSet = instance.reduce((set, instancePlugin) => { + set.add(instancePlugin.pluginSlug); + return set; + }, new Set()); + // add locals local.forEach((localPlugin) => { const remoteCounterpart = remote.find((r) => r.slug === localPlugin.id); @@ -32,7 +43,15 @@ export function mergeLocalsAndRemotes( const shouldSkip = remotePlugin.status === RemotePluginStatus.Deprecated && !localCounterpart; // We are only listing deprecated plugins in case they are installed. if (!shouldSkip) { - catalogPlugins.push(mergeLocalAndRemote(localCounterpart, remotePlugin, error)); + const catalogPlugin = mergeLocalAndRemote(localCounterpart, remotePlugin, error); + + // for managed instances, check if plugin is installed, but not yet present in the current instance + if (configCore.featureToggles.managedPluginsInstall && config.pluginAdminExternalManageEnabled) { + catalogPlugin.isFullyInstalled = instancesSet.has(remotePlugin.slug) && catalogPlugin.isInstalled; + catalogPlugin.isInstalled = instancesSet.has(remotePlugin.slug) || catalogPlugin.isInstalled; + } + + catalogPlugins.push(catalogPlugin); } }); @@ -95,6 +114,7 @@ export function mapRemoteToCatalog(plugin: RemotePlugin, error?: PluginError): C type: typeCode, error: error?.errorCode, angularDetected, + isFullyInstalled: isDisabled, }; } @@ -140,6 +160,7 @@ export function mapLocalToCatalog(plugin: LocalPlugin, error?: PluginError): Cat error: error?.errorCode, accessControl: accessControl, angularDetected, + isFullyInstalled: true, }; } @@ -196,6 +217,7 @@ export function mapToCatalogPlugin(local?: LocalPlugin, remote?: RemotePlugin, e // Only local plugins have access control metadata accessControl: local?.accessControl, angularDetected: local?.angularDetected || remote?.angularDetected, + isFullyInstalled: Boolean(local) || isDisabled, }; } diff --git a/public/app/features/plugins/admin/hooks/usePluginConfig.tsx b/public/app/features/plugins/admin/hooks/usePluginConfig.tsx index d515ba81594..4511b2dc396 100644 --- a/public/app/features/plugins/admin/hooks/usePluginConfig.tsx +++ b/public/app/features/plugins/admin/hooks/usePluginConfig.tsx @@ -9,7 +9,7 @@ export const usePluginConfig = (plugin?: CatalogPlugin) => { return null; } - if (plugin.isInstalled && !plugin.isDisabled) { + if (plugin.isFullyInstalled && !plugin.isDisabled) { return loadPlugin(plugin.id); } return null; diff --git a/public/app/features/plugins/admin/state/actions.ts b/public/app/features/plugins/admin/state/actions.ts index ad9b4e83118..7d3b3babdbf 100644 --- a/public/app/features/plugins/admin/state/actions.ts +++ b/public/app/features/plugins/admin/state/actions.ts @@ -1,8 +1,9 @@ import { createAction, createAsyncThunk, Update } from '@reduxjs/toolkit'; -import { from, forkJoin, timeout, lastValueFrom, catchError, throwError } from 'rxjs'; +import { from, forkJoin, timeout, lastValueFrom, catchError, throwError, of } from 'rxjs'; import { PanelPlugin, PluginError } from '@grafana/data'; -import { getBackendSrv, isFetchError } from '@grafana/runtime'; +import { config, getBackendSrv, isFetchError } from '@grafana/runtime'; +import configCore from 'app/core/config'; import { importPanelPlugin } from 'app/features/plugins/importPanelPlugin'; import { StoreState, ThunkResult } from 'app/types'; @@ -14,10 +15,11 @@ import { getPluginDetails, installPlugin, uninstallPlugin, + getInstancePlugins, } from '../api'; import { STATE_PREFIX } from '../constants'; import { mapLocalToCatalog, mergeLocalsAndRemotes, updatePanels } from '../helpers'; -import { CatalogPlugin, RemotePlugin, LocalPlugin } from '../types'; +import { CatalogPlugin, RemotePlugin, LocalPlugin, InstancePlugin } from '../types'; // Fetches export const fetchAll = createAsyncThunk(`${STATE_PREFIX}/fetchAll`, async (_, thunkApi) => { @@ -27,11 +29,16 @@ export const fetchAll = createAsyncThunk(`${STATE_PREFIX}/fetchAll`, async (_, t const local$ = from(getLocalPlugins()); const remote$ = from(getRemotePlugins()); + const instance$ = + config.pluginAdminExternalManageEnabled && configCore.featureToggles.managedPluginsInstall + ? from(getInstancePlugins()) + : of(undefined); const pluginErrors$ = from(getPluginErrors()); forkJoin({ local: local$, remote: remote$, + instance: instance$, pluginErrors: pluginErrors$, }) .pipe( @@ -55,9 +62,10 @@ export const fetchAll = createAsyncThunk(`${STATE_PREFIX}/fetchAll`, async (_, t if (remote.length > 0) { const local = await lastValueFrom(local$); + const instance = await lastValueFrom(instance$); const pluginErrors = await lastValueFrom(pluginErrors$); - thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes(local, remote, pluginErrors))); + thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes({ local, remote, instance, pluginErrors }))); } }); @@ -69,21 +77,23 @@ export const fetchAll = createAsyncThunk(`${STATE_PREFIX}/fetchAll`, async (_, t ({ local, remote, + instance, pluginErrors, }: { local: LocalPlugin[]; remote?: RemotePlugin[]; + instance?: InstancePlugin[]; pluginErrors: PluginError[]; }) => { thunkApi.dispatch({ type: `${STATE_PREFIX}/fetchLocal/fulfilled` }); // Both local and remote plugins are loaded if (local && remote) { - thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes(local, remote, pluginErrors))); + thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes({ local, remote, instance, pluginErrors }))); // Only remote plugins are loaded (remote timed out) } else if (local) { - thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes(local, [], pluginErrors))); + thunkApi.dispatch(addPlugins(mergeLocalsAndRemotes({ local, pluginErrors }))); } } ); diff --git a/public/app/features/plugins/admin/types.ts b/public/app/features/plugins/admin/types.ts index 576fefd75d3..6966ac07cf2 100644 --- a/public/app/features/plugins/admin/types.ts +++ b/public/app/features/plugins/admin/types.ts @@ -59,6 +59,9 @@ export interface CatalogPlugin extends WithAccessControlMetadata { details?: CatalogPluginDetails; error?: PluginErrorCode; angularDetected?: boolean; + // instance plugins may not be fully installed, which means a new instance + // running the plugin didn't started yet + isFullyInstalled?: boolean; } export interface CatalogPluginDetails { @@ -294,3 +297,7 @@ export type PluginVersion = { isCompatible: boolean; grafanaDependency: string | null; }; + +export type InstancePlugin = { + pluginSlug: string; +};