Plugins: Remove plugin load failure logging for unauth users (#112940)

This commit is contained in:
Hugo Häggmark
2025-10-27 05:48:31 +01:00
committed by GitHub
parent cc4a6cff64
commit 6a15f40a85
4 changed files with 110 additions and 25 deletions
@@ -1,27 +1,21 @@
import {
PluginLoadingStrategy,
PluginType,
type AppPluginConfig,
type PluginMeta,
type PluginMetaInfo,
type AngularMeta,
type PluginDependencies,
type PluginExtensions,
AppPlugin,
OrgRole,
} from '@grafana/data';
import type { AppPluginConfig } from '@grafana/runtime';
import { ContextSrv, setContextSrv } from 'app/core/services/context_srv';
import { getPluginSettings } from 'app/features/plugins/pluginSettings';
import { pluginImporter } from './importer/pluginImporter';
import { clearPreloadedPluginsCache, preloadPlugins } from './pluginPreloader';
jest.mock('app/core/services/context_srv', () => ({
contextSrv: {
user: {
orgRole: 'Admin',
},
},
}));
jest.mock('app/features/plugins/pluginSettings', () => ({
getPluginSettings: jest.fn(),
}));
@@ -81,6 +75,9 @@ const createMockPluginMeta = (overrides: Partial<PluginMeta> = {}): PluginMeta =
describe('pluginPreloader', () => {
beforeEach(() => {
const contextSrv = new ContextSrv();
contextSrv.user.orgRole = OrgRole.Admin;
setContextSrv(contextSrv);
jest.clearAllMocks();
jest.resetModules();
clearPreloadedPluginsCache();
@@ -272,5 +269,71 @@ describe('pluginPreloader', () => {
expect(getPluginSettingsMock).toHaveBeenCalledTimes(2);
expect(importAppPluginMock).toHaveBeenCalledTimes(2);
});
it('should have showErrorAlert set to false for user with no role', async () => {
const contextSrv = new ContextSrv();
contextSrv.user.orgRole = '';
setContextSrv(contextSrv);
const appConfig = createMockAppPluginConfig({
id: 'test-plugin',
path: '/path/to/plugin',
version: '1.0.0',
});
const mockPluginMeta = createMockPluginMeta({
id: 'test-plugin',
name: 'Test Plugin',
type: PluginType.app,
});
getPluginSettingsMock.mockResolvedValue(mockPluginMeta);
importAppPluginMock.mockResolvedValue(new AppPlugin());
await preloadPlugins([appConfig]);
expect(getPluginSettingsMock).toHaveBeenCalledWith('test-plugin', {
showErrorAlert: false,
});
expect(importAppPluginMock).toHaveBeenCalledWith(mockPluginMeta);
});
it('should log all errors for user with role', async () => {
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const appConfig = createMockAppPluginConfig({
id: 'test-plugin',
path: '/path/to/plugin',
version: '1.0.0',
});
const error = { status: 401, message: 'Unauthorized' };
getPluginSettingsMock.mockRejectedValue(error);
await preloadPlugins([appConfig]);
expect(consoleSpy).toHaveBeenCalledWith(
`[Plugins] Failed to preload plugin: /path/to/plugin (version: 1.0.0)`,
error
);
});
it('should not log any errors for user without role', async () => {
const contextSrv = new ContextSrv();
contextSrv.user.orgRole = '';
setContextSrv(contextSrv);
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const appConfig = createMockAppPluginConfig({
id: 'test-plugin',
path: '/path/to/plugin',
version: '1.0.0',
});
const error = { status: 401, message: 'Unauthorized' };
getPluginSettingsMock.mockRejectedValue(error);
await preloadPlugins([appConfig]);
expect(consoleSpy).not.toHaveBeenCalled();
});
});
});
@@ -1,9 +1,9 @@
import type {
AppPluginConfig,
PluginExtensionAddedLinkConfig,
PluginExtensionExposedComponentConfig,
PluginExtensionAddedComponentConfig,
} from '@grafana/data';
import type { AppPluginConfig } from '@grafana/runtime';
import { contextSrv } from 'app/core/services/context_srv';
import { getPluginSettings } from 'app/features/plugins/pluginSettings';
@@ -36,13 +36,16 @@ export async function preloadPlugins(apps: AppPluginConfig[] = []) {
}
async function preload(config: AppPluginConfig): Promise<void> {
try {
const meta = await getPluginSettings(config.id, {
showErrorAlert: contextSrv.user.orgRole !== '',
});
const showErrorAlert = contextSrv.user.orgRole !== '';
try {
const meta = await getPluginSettings(config.id, { showErrorAlert });
await pluginImporter.importApp(meta);
} catch (error) {
if (!showErrorAlert) {
return;
}
console.error(`[Plugins] Failed to preload plugin: ${config.path} (version: ${config.version})`, error);
}
}
@@ -22,8 +22,8 @@ describe('PluginSettings', () => {
id: 'test-plugin',
enabled: true,
};
getBackendSrv().get = jest.fn().mockResolvedValue(testPluginResponse);
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get');
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get').mockResolvedValue(testPluginResponse);
// act
const response = await getPluginSettings('test');
// assert
@@ -42,8 +42,7 @@ describe('PluginSettings', () => {
id: 'test-plugin',
enabled: true,
};
getBackendSrv().get = jest.fn().mockResolvedValue(testPluginResponse);
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get');
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get').mockResolvedValue(testPluginResponse);
// act
const response1 = await getPluginSettings('test');
const response2 = await getPluginSettings('test');
@@ -62,8 +61,8 @@ describe('PluginSettings', () => {
id: 'test-plugin',
enabled: true,
};
getBackendSrv().get = jest.fn().mockResolvedValue(testPluginResponse);
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get');
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get').mockResolvedValue(testPluginResponse);
// act
const response1 = await getPluginSettings('test');
@@ -83,8 +82,7 @@ describe('PluginSettings', () => {
id: 'test-plugin',
enabled: true,
};
getBackendSrv().get = jest.fn().mockResolvedValue(testPluginResponse);
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get');
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get').mockResolvedValue(testPluginResponse);
// act
const response1 = await getPluginSettings('test');
await clearPluginSettingsCache('another-test');
@@ -103,8 +101,8 @@ describe('PluginSettings', () => {
id: 'test-plugin',
enabled: true,
};
getBackendSrv().get = jest.fn().mockResolvedValue(testPluginResponse);
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get');
const getRequestSpy = jest.spyOn(getBackendSrv(), 'get').mockResolvedValue(testPluginResponse);
// act
const response1 = await getPluginSettings('test');
@@ -116,4 +114,25 @@ describe('PluginSettings', () => {
expect(response2).toEqual(testPluginResponse);
expect(getRequestSpy).toHaveBeenCalledTimes(2);
});
it('should reject with Unknown Plugin message if error status is not 403 or 401', async () => {
const error = { status: 404, message: 'Not found' };
jest.spyOn(getBackendSrv(), 'get').mockRejectedValue(error);
await expect(getPluginSettings('test')).rejects.toEqual(new Error('Unknown Plugin'));
});
it('should reject thrown error if error status is 403', async () => {
const error = { status: 403, message: 'Forbidden' };
jest.spyOn(getBackendSrv(), 'get').mockRejectedValue(error);
await expect(getPluginSettings('test')).rejects.toEqual({ ...error, isHandled: true });
});
it('should reject thrown error if error status is 401', async () => {
const error = { status: 401, message: 'Unauthorized' };
jest.spyOn(getBackendSrv(), 'get').mockRejectedValue(error);
await expect(getPluginSettings('test')).rejects.toEqual({ ...error, isHandled: true });
});
});
@@ -20,7 +20,7 @@ export function getPluginSettings(pluginId: string, options?: Partial<BackendSrv
})
.catch((e) => {
// User does not have access to plugin
if (typeof e === 'object' && e !== null && 'status' in e && e.status === 403) {
if (typeof e === 'object' && e !== null && 'status' in e && (e.status === 403 || e.status === 401)) {
e.isHandled = true;
return Promise.reject(e);
}