From e82da5520e41c44dbf47e6cdd1dabfecc83f8a04 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 14 Jun 2023 15:07:34 -0600 Subject: [PATCH 1/8] Change permissions used for finishing plugin setup --- .../src/state/rootBaseStore/index.ts | 55 +++++++++++++------ .../state/rootBaseStore/rootBaseStore.test.ts | 15 ++--- .../src/utils/authorization/index.ts | 5 +- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index b801999224..a4018f7a65 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -1,3 +1,5 @@ +import { OrgRole } from '@grafana/data'; +import { contextSrv } from 'grafana/app/core/core'; import { action, observable } from 'mobx'; import moment from 'moment-timezone'; import qs from 'query-string'; @@ -32,7 +34,6 @@ import { UserGroupStore } from 'models/user_group/user_group'; import { makeRequest } from 'network'; import { AppFeature } from 'state/features'; import PluginState from 'state/plugin'; -import { isUserActionAllowed, UserActions } from 'utils/authorization'; import { GRAFANA_LICENSE_OSS } from 'utils/consts'; // ------ Dashboard ------ // @@ -184,22 +185,42 @@ export class RootBaseStore { if (!allow_signup) { return this.setupPluginError('🚫 OnCall has temporarily disabled signup of new users. Please try again later.'); } - - if (!isUserActionAllowed(UserActions.PluginsInstall)) { - return this.setupPluginError( - '🚫 An Admin in your organization must sign on and setup OnCall before it can be used' - ); - } - - try { - /** - * this will install AND sync the necessary data - * the sync is done automatically by the /plugin/install OnCall API endpoint - * therefore there is no need to trigger an additional/separate sync, nor poll a status - */ - await PluginState.installPlugin(); - } catch (e) { - return this.setupPluginError(PluginState.getHumanReadableErrorFromOnCallError(e, this.onCallApiUrl, 'install')); + const fallback = contextSrv.user.orgRole === OrgRole.Admin && !contextSrv.accessControlEnabled(); + const setupRequiredPermissions = [ + 'plugins:write', + 'users:read', + 'teams:read', + 'apikeys:create', + 'apikeys:delete', + ]; + const missingPermissions = setupRequiredPermissions.filter(function (permission) { + return !contextSrv.hasAccess(permission, fallback); + }); + if (missingPermissions.length === 0) { + try { + /** + * this will install AND sync the necessary data + * the sync is done automatically by the /plugin/install OnCall API endpoint + * therefore there is no need to trigger an additional/separate sync, nor poll a status + */ + await PluginState.installPlugin(); + } catch (e) { + return this.setupPluginError( + PluginState.getHumanReadableErrorFromOnCallError(e, this.onCallApiUrl, 'install') + ); + } + } else { + if (contextSrv.accessControlEnabled()) { + return this.setupPluginError( + '🚫 User is missing permission(s) ' + + missingPermissions.join(', ') + + ' to setup OnCall before it can be used' + ); + } else { + return this.setupPluginError( + '🚫 User with Admin permissions in your organization must sign on and setup OnCall before it can be used' + ); + } } } else { const syncDataResponse = await PluginState.syncDataWithOnCall(this.onCallApiUrl); diff --git a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts index ee8a43518b..2afd96a5de 100644 --- a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts +++ b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts @@ -1,7 +1,7 @@ import { OnCallAppPluginMeta } from 'types'; import PluginState from 'state/plugin'; -import { UserActions, isUserActionAllowed as isUserActionAllowedOriginal } from 'utils/authorization'; +import { isUserActionAllowed as isUserActionAllowedOriginal } from 'utils/authorization'; import { RootBaseStore } from './'; @@ -10,8 +10,6 @@ jest.mock('utils/authorization'); const isUserActionAllowed = isUserActionAllowedOriginal as jest.Mock>; -const PluginInstallAction = UserActions.PluginsInstall; - const generatePluginData = ( onCallApiUrl: OnCallAppPluginMeta['jsonData']['onCallApiUrl'] = null ): OnCallAppPluginMeta => @@ -159,14 +157,13 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - expect(isUserActionAllowed).toHaveBeenCalledTimes(1); - expect(isUserActionAllowed).toHaveBeenCalledWith(PluginInstallAction); + // todo: add check for access expect(PluginState.installPlugin).toHaveBeenCalledTimes(0); expect(rootBaseStore.appLoading).toBe(false); expect(rootBaseStore.initializationError).toEqual( - '🚫 An Admin in your organization must sign on and setup OnCall before it can be used' + '🚫 User with Admin permissions in your organization must sign on and setup OnCall before it can be used' ); }); @@ -198,8 +195,7 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - expect(isUserActionAllowed).toHaveBeenCalledTimes(1); - expect(isUserActionAllowed).toHaveBeenCalledWith(PluginInstallAction); + // todo: add check for access expect(PluginState.installPlugin).toHaveBeenCalledTimes(1); expect(PluginState.installPlugin).toHaveBeenCalledWith(); @@ -238,8 +234,7 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - expect(isUserActionAllowed).toHaveBeenCalledTimes(1); - expect(isUserActionAllowed).toHaveBeenCalledWith(PluginInstallAction); + // todo: add check for access expect(PluginState.installPlugin).toHaveBeenCalledTimes(1); expect(PluginState.installPlugin).toHaveBeenCalledWith(); diff --git a/grafana-plugin/src/utils/authorization/index.ts b/grafana-plugin/src/utils/authorization/index.ts index 10574c47eb..1ab1b17344 100644 --- a/grafana-plugin/src/utils/authorization/index.ts +++ b/grafana-plugin/src/utils/authorization/index.ts @@ -35,7 +35,6 @@ export enum Action { TEST = 'test', EXPORT = 'export', UPDATE_SETTINGS = 'update-settings', - INSTALL = 'install', } type Actions = @@ -66,8 +65,7 @@ type Actions = | 'UserSettingsAdmin' | 'OtherSettingsRead' | 'OtherSettingsWrite' - | 'TeamsWrite' - | 'PluginsInstall'; + | 'TeamsWrite'; const roleMapping: Record = { [OrgRole.Admin]: 0, @@ -164,5 +162,4 @@ export const UserActions: { [action in Actions]: UserAction } = { // These are not oncall specific TeamsWrite: constructAction(Resource.TEAMS, Action.WRITE, OrgRole.Admin, false), - PluginsInstall: constructAction(Resource.PLUGINS, Action.INSTALL, OrgRole.Admin, false), }; From f3db6dcd961f582b83137235518a558ee6c4d559 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 16 Jun 2023 16:36:01 -0600 Subject: [PATCH 2/8] Add/fix tests --- .../src/state/rootBaseStore/index.ts | 20 ++--- .../state/rootBaseStore/rootBaseStore.test.ts | 88 ++++++++++++++++++- .../src/utils/authorization/index.ts | 1 - 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index a4018f7a65..8f4990ce57 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -185,17 +185,7 @@ export class RootBaseStore { if (!allow_signup) { return this.setupPluginError('🚫 OnCall has temporarily disabled signup of new users. Please try again later.'); } - const fallback = contextSrv.user.orgRole === OrgRole.Admin && !contextSrv.accessControlEnabled(); - const setupRequiredPermissions = [ - 'plugins:write', - 'users:read', - 'teams:read', - 'apikeys:create', - 'apikeys:delete', - ]; - const missingPermissions = setupRequiredPermissions.filter(function (permission) { - return !contextSrv.hasAccess(permission, fallback); - }); + const missingPermissions = this.checkMissingSetupPermissions(); if (missingPermissions.length === 0) { try { /** @@ -244,6 +234,14 @@ export class RootBaseStore { this.appLoading = false; } + checkMissingSetupPermissions() { + const fallback = contextSrv.user.orgRole === OrgRole.Admin && !contextSrv.accessControlEnabled(); + const setupRequiredPermissions = ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete']; + return setupRequiredPermissions.filter(function (permission) { + return !contextSrv.hasAccess(permission, fallback); + }); + } + hasFeature(feature: string | AppFeature) { // todo use AppFeature only return this.features?.[feature]; diff --git a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts index 2afd96a5de..a29426bba1 100644 --- a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts +++ b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts @@ -1,3 +1,5 @@ +import { OrgRole } from '@grafana/data'; +import { contextSrv } from 'grafana/app/core/core'; import { OnCallAppPluginMeta } from 'types'; import PluginState from 'state/plugin'; @@ -7,6 +9,13 @@ import { RootBaseStore } from './'; jest.mock('state/plugin'); jest.mock('utils/authorization'); +jest.mock('grafana/app/core/core', () => ({ + contextSrv: { + user: { + orgRole: null, + }, + }, +})); const isUserActionAllowed = isUserActionAllowedOriginal as jest.Mock>; @@ -138,6 +147,10 @@ describe('rootBaseStore', () => { const onCallApiUrl = 'http://asdfasdf.com'; const rootBaseStore = new RootBaseStore(); + contextSrv.user.orgRole = OrgRole.Viewer; + contextSrv.accessControlEnabled = jest.fn().mockReturnValue(false); + contextSrv.hasAccess = jest.fn().mockReturnValue(false); + PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, @@ -157,8 +170,6 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - // todo: add check for access - expect(PluginState.installPlugin).toHaveBeenCalledTimes(0); expect(rootBaseStore.appLoading).toBe(false); @@ -176,6 +187,10 @@ describe('rootBaseStore', () => { const rootBaseStore = new RootBaseStore(); const mockedLoadCurrentUser = jest.fn(); + contextSrv.user.orgRole = OrgRole.Admin; + contextSrv.accessControlEnabled = jest.fn().mockResolvedValueOnce(false); + contextSrv.hasAccess = jest.fn().mockReturnValue(true); + PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ ...scenario, @@ -195,8 +210,6 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - // todo: add check for access - expect(PluginState.installPlugin).toHaveBeenCalledTimes(1); expect(PluginState.installPlugin).toHaveBeenCalledWith(); @@ -207,6 +220,69 @@ describe('rootBaseStore', () => { expect(rootBaseStore.initializationError).toBeNull(); }); + test.each([ + { role: OrgRole.Admin, missing_permissions: [], expected_result: true }, + { role: OrgRole.Viewer, missing_permissions: [], expected_result: true }, + { + role: OrgRole.Admin, + missing_permissions: ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], + expected_result: false, + }, + { + role: OrgRole.Viewer, + missing_permissions: ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], + expected_result: false, + }, + ])('signup is allowed, accessControlEnabled, various roles and permissions', async (scenario) => { + // mocks/setup + const onCallApiUrl = 'http://asdfasdf.com'; + const rootBaseStore = new RootBaseStore(); + const mockedLoadCurrentUser = jest.fn(); + + contextSrv.user.orgRole = scenario.role; + contextSrv.accessControlEnabled = jest.fn().mockReturnValue(true); + rootBaseStore.checkMissingSetupPermissions = jest.fn().mockImplementation(() => scenario.missing_permissions); + + PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ + ...scenario, + is_user_anonymous: false, + allow_signup: true, + version: 'asdfasdf', + license: 'asdfasdf', + }); + isUserActionAllowed.mockReturnValueOnce(true); + PluginState.installPlugin = jest.fn().mockResolvedValueOnce(null); + rootBaseStore.userStore.loadCurrentUser = mockedLoadCurrentUser; + + // test + await rootBaseStore.setupPlugin(generatePluginData(onCallApiUrl)); + + // assertions + expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); + expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); + + expect(rootBaseStore.appLoading).toBe(false); + + if (scenario.expected_result) { + expect(PluginState.installPlugin).toHaveBeenCalledTimes(1); + expect(PluginState.installPlugin).toHaveBeenCalledWith(); + + expect(mockedLoadCurrentUser).toHaveBeenCalledTimes(1); + expect(mockedLoadCurrentUser).toHaveBeenCalledWith(); + + expect(rootBaseStore.initializationError).toBeNull(); + } else { + expect(PluginState.installPlugin).toHaveBeenCalledTimes(0); + + expect(rootBaseStore.initializationError).toEqual( + '🚫 User is missing permission(s) ' + + scenario.missing_permissions.join(', ') + + ' to setup OnCall before it can be used' + ); + } + }); + test('plugin is not installed, signup is allowed, the user is an admin, and plugin installation throws an error', async () => { // mocks/setup const onCallApiUrl = 'http://asdfasdf.com'; @@ -214,6 +290,10 @@ describe('rootBaseStore', () => { const installPluginError = new Error('asdasdfasdfasf'); const humanReadableErrorMsg = 'asdfasldkfjaksdjflk'; + contextSrv.user.orgRole = OrgRole.Admin; + contextSrv.accessControlEnabled = jest.fn().mockReturnValue(false); + contextSrv.hasAccess = jest.fn().mockReturnValue(true); + PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, diff --git a/grafana-plugin/src/utils/authorization/index.ts b/grafana-plugin/src/utils/authorization/index.ts index 1ab1b17344..0efc00560f 100644 --- a/grafana-plugin/src/utils/authorization/index.ts +++ b/grafana-plugin/src/utils/authorization/index.ts @@ -25,7 +25,6 @@ export enum Resource { OTHER_SETTINGS = 'other-settings', TEAMS = 'teams', - PLUGINS = 'plugins', } export enum Action { From 6bceaeb085894bd3104bc65bbf4af4547f57e018 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 16 Jun 2023 16:38:45 -0600 Subject: [PATCH 3/8] Update CHANGELOG.md, merge dev --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f80630ae5..e68d1a965d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Change .Values.externalRabbitmq.passwordKey from `password` to `""` (default value `rabbitmq-password`) ([#864](https://github.com/grafana/oncall/pull/864)) - Remove deprecated `permissions` string array from the internal API user serializer by @joeyorlando ([#2269](https://github.com/grafana/oncall/pull/2269)) +- Change permissions used during setup to better represent actions being taken by @mderynck ([#2242](https://github.com/grafana/oncall/pull/2242)) ### Added From c6727159bd0c9be9d54a1f2cd4302f599660cd40 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 16 Jun 2023 16:49:37 -0600 Subject: [PATCH 4/8] Remove todo --- grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts index a29426bba1..4dedc2f832 100644 --- a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts +++ b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts @@ -314,8 +314,6 @@ describe('rootBaseStore', () => { expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(onCallApiUrl); - // todo: add check for access - expect(PluginState.installPlugin).toHaveBeenCalledTimes(1); expect(PluginState.installPlugin).toHaveBeenCalledWith(); From fe5287de546af074fae0d26ea0e9c571ac228140 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 19 Jun 2023 17:29:03 -0600 Subject: [PATCH 5/8] Handle edge cases for plugin initialization when Grafana API key is broken and can be repaired or unable to communicate with backend to get maintenance status --- .../PluginConfigPage/PluginConfigPage.tsx | 5 +- .../src/plugin/PluginSetup/index.tsx | 6 ++- grafana-plugin/src/state/plugin/index.ts | 46 ++++++++++++++----- .../src/state/plugin/plugin.test.ts | 3 +- .../src/state/rootBaseStore/index.ts | 10 ++-- 5 files changed, 50 insertions(+), 20 deletions(-) diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx index 9b44999e56..b80fd54eaa 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx @@ -4,7 +4,6 @@ import { Button, Label, Legend, LoadingPlaceholder } from '@grafana/ui'; import { useLocation } from 'react-router-dom'; import { OnCallPluginConfigPageProps } from 'types'; -import logo from 'img/logo.svg'; import PluginState, { PluginStatusResponseBase } from 'state/plugin'; import { GRAFANA_LICENSE_OSS } from 'utils/consts'; @@ -228,8 +227,8 @@ const PluginConfigPage: FC = ({ {pluginIsConnected ? ( <>

- Plugin is connected! Continue to Grafana OnCall by clicking the{' '} - Grafana OnCall Logo icon over there 👈 + Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over + there 👈

= ({ text, children }) => const PluginSetup: FC = observer(({ InitializedComponent, ...props }) => { const store = useStore(); - const setupPlugin = useCallback(() => store.setupPlugin(props.meta), [props.meta]); + const setupPlugin = useCallback( + () => PluginState.getGrafanaPluginSettings().then((meta) => store.setupPlugin(meta)), + [] + ); useEffect(() => { setupPlugin(); diff --git a/grafana-plugin/src/state/plugin/index.ts b/grafana-plugin/src/state/plugin/index.ts index 26d0f7402a..e1600f6324 100644 --- a/grafana-plugin/src/state/plugin/index.ts +++ b/grafana-plugin/src/state/plugin/index.ts @@ -137,16 +137,20 @@ class PluginState { static updateGrafanaPluginSettings = async (data: UpdateGrafanaPluginSettingsProps, enabled = true) => this.grafanaBackend.post(this.GRAFANA_PLUGIN_SETTINGS_URL, { ...data, enabled, pinned: true }); - static createGrafanaToken = async () => { - const baseUrl = '/api/auth/keys'; - const keys = await this.grafanaBackend.get(baseUrl); - const existingKey = keys.find((key: { id: number; name: string; role: string }) => key.name === 'OnCall'); + static readonly KEYS_BASE_URL = '/api/auth/keys'; + + static getGrafanaToken = async () => { + const keys = await this.grafanaBackend.get(this.KEYS_BASE_URL); + return keys.find((key: { id: number; name: string; role: string }) => key.name === 'OnCall'); + }; + static createGrafanaToken = async () => { + const existingKey = await this.getGrafanaToken(); if (existingKey) { - await this.grafanaBackend.delete(`${baseUrl}/${existingKey.id}`); + await this.grafanaBackend.delete(`${this.KEYS_BASE_URL}/${existingKey.id}`); } - return await this.grafanaBackend.post(baseUrl, { + return await this.grafanaBackend.post(this.KEYS_BASE_URL, { name: 'OnCall', role: 'Admin', secondsToLive: null, @@ -205,6 +209,15 @@ class PluginState { onCallApiUrlIsConfiguredThroughEnvVar = false ): Promise => { try { + /** + * Allows the plugin config page to repair settings like the app initialization screen if a user deletes + * an API on accident but leaves the plugin settings intact. + */ + const existingKey = await this.getGrafanaToken(); + if (!existingKey) { + await this.installPlugin(); + } + const startSyncResponse = await makeRequest(`${this.ONCALL_BASE_URL}/sync`, { method: 'POST' }); if (typeof startSyncResponse === 'string') { // an error occured trying to initiate the sync @@ -300,11 +313,22 @@ class PluginState { return null; }; - static checkIfBackendIsInMaintenanceMode = async (): Promise => { - const response = await makeRequest('/maintenance-mode-status', { - method: 'GET', - }); - return response.currently_undergoing_maintenance_message; + static checkIfBackendIsInMaintenanceMode = async ( + onCallApiUrl: string, + onCallApiUrlIsConfiguredThroughEnvVar = false + ): Promise => { + try { + return await makeRequest('/maintenance-mode-status', { + method: 'GET', + }); + } catch (e) { + return this.getHumanReadableErrorFromOnCallError( + e, + onCallApiUrl, + 'install', + onCallApiUrlIsConfiguredThroughEnvVar + ); + } }; static checkIfPluginIsConnected = async ( diff --git a/grafana-plugin/src/state/plugin/plugin.test.ts b/grafana-plugin/src/state/plugin/plugin.test.ts index 7b67fad040..6080cf2a0b 100644 --- a/grafana-plugin/src/state/plugin/plugin.test.ts +++ b/grafana-plugin/src/state/plugin/plugin.test.ts @@ -663,10 +663,11 @@ describe('PluginState.checkIfBackendIsInMaintenanceMode', () => { // mocks const maintenanceModeMsg = 'asdfljkadsjlfkajsdf'; const mockedResp = { currently_undergoing_maintenance_message: maintenanceModeMsg }; + const onCallApiUrl = 'http://hello.com'; makeRequest.mockResolvedValueOnce(mockedResp); // test - const response = await PluginState.checkIfBackendIsInMaintenanceMode(); + const response = await PluginState.checkIfBackendIsInMaintenanceMode(onCallApiUrl); // assertions expect(response).toEqual(maintenanceModeMsg); diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index 8f4990ce57..c2796dcdd3 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -163,13 +163,15 @@ export class RootBaseStore { return this.setupPluginError('🚫 Plugin has not been initialized'); } - const isInMaintenanceMode = await PluginState.checkIfBackendIsInMaintenanceMode(); - if (isInMaintenanceMode !== null) { + const maintenanceMode = await PluginState.checkIfBackendIsInMaintenanceMode(this.onCallApiUrl); + if (typeof maintenanceMode === 'string') { + return this.setupPluginError(maintenanceMode); + } else if (maintenanceMode.currently_undergoing_maintenance_message) { this.currentlyUndergoingMaintenance = true; - return this.setupPluginError(`🚧 ${isInMaintenanceMode} 🚧`); + return this.setupPluginError(`🚧 ${maintenanceMode.currently_undergoing_maintenance_message} 🚧`); } - // at this point we know the plugin is provionsed + // at this point we know the plugin is provisioned const pluginConnectionStatus = await PluginState.checkIfPluginIsConnected(this.onCallApiUrl); if (typeof pluginConnectionStatus === 'string') { return this.setupPluginError(pluginConnectionStatus); From 1b2dfc98d21b2a8dab0352c7985ec301d1d1d0b9 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 21 Jun 2023 15:28:17 -0600 Subject: [PATCH 6/8] Change to reload when config is removed, change messages error messages to be less confusing when not self hosting, add fallback for if plugin should treat as cloud configured when unable to connect to backend, fix tests --- .../PluginConfigPage.test.tsx | 2 +- .../PluginConfigPage/PluginConfigPage.tsx | 47 ++- .../PluginConfigPage.test.tsx.snap | 291 ++++++++++-------- .../src/plugin/PluginSetup/index.tsx | 6 +- .../plugin/__snapshots__/plugin.test.ts.snap | 28 +- grafana-plugin/src/state/plugin/index.ts | 35 ++- .../src/state/plugin/plugin.test.ts | 5 +- .../src/state/rootBaseStore/index.ts | 16 +- .../state/rootBaseStore/rootBaseStore.test.ts | 44 ++- grafana-plugin/src/utils/consts.ts | 6 + 10 files changed, 273 insertions(+), 207 deletions(-) diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx index bb656de8a3..1885e26eeb 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx @@ -302,7 +302,7 @@ describe('PluginConfigPage', () => { // click the confirm button within the modal, which actually triggers the callback await userEvent.click(screen.getByText('Remove')); - await screen.findByTestId(successful ? PLUGIN_CONFIGURATION_FORM_DATA_ID : STATUS_MESSAGE_BLOCK_DATA_ID); + // await screen.findByTestId(successful ? PLUGIN_CONFIGURATION_FORM_DATA_ID : STATUS_MESSAGE_BLOCK_DATA_ID); // assertions expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx index b80fd54eaa..a2248ed3fe 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx @@ -1,11 +1,11 @@ import React, { FC, useCallback, useEffect, useState } from 'react'; -import { Button, Label, Legend, LoadingPlaceholder } from '@grafana/ui'; +import { Button, HorizontalGroup, Label, Legend, LoadingPlaceholder } from '@grafana/ui'; import { useLocation } from 'react-router-dom'; import { OnCallPluginConfigPageProps } from 'types'; import PluginState, { PluginStatusResponseBase } from 'state/plugin'; -import { GRAFANA_LICENSE_OSS } from 'utils/consts'; +import { APP_VERSION, CLOUD_VERSION_REGEX, GRAFANA_LICENSE_CLOUD, GRAFANA_LICENSE_OSS } from 'utils/consts'; import ConfigurationForm from './parts/ConfigurationForm'; import RemoveCurrentConfigurationButton from './parts/RemoveCurrentConfigurationButton'; @@ -74,13 +74,14 @@ const PluginConfigPage: FC = ({ const pluginMetaOnCallApiUrl = jsonData?.onCallApiUrl; const processEnvOnCallApiUrl = process.env.ONCALL_API_URL; // don't destructure this, will break how webpack supplies this const onCallApiUrl = pluginMetaOnCallApiUrl || processEnvOnCallApiUrl; - const licenseType = pluginIsConnected?.license; + const fallbackLicense = CLOUD_VERSION_REGEX.test(APP_VERSION) ? GRAFANA_LICENSE_CLOUD : GRAFANA_LICENSE_OSS; + const licenseType = pluginIsConnected?.license || fallbackLicense; const resetQueryParams = useCallback(() => removePluginConfiguredQueryParams(pluginIsEnabled), [pluginIsEnabled]); const triggerDataSyncWithOnCall = useCallback(async () => { + await resetMessages(); setSyncingPlugin(true); - setSyncError(null); const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl); @@ -143,35 +144,25 @@ const PluginConfigPage: FC = ({ } }, [pluginMetaOnCallApiUrl, processEnvOnCallApiUrl, onCallApiUrl, pluginConfiguredRedirect]); - const resetState = useCallback(() => { + const resetMessages = useCallback(() => { setPluginResetError(null); setPluginConnectionCheckError(null); setPluginIsConnected(null); setSyncError(null); + }, []); + + const resetState = useCallback(() => { + resetMessages(); resetQueryParams(); }, [resetQueryParams]); - /** - * NOTE: there is a possible edge case when resetting the plugin, that would lead to an error message being shown - * (which could be fixed by just reloading the page) - * This would happen if the user removes the plugin configuration, leaves the page, then comes back to the plugin - * configuration. - * - * This is because the props being passed into this component wouldn't reflect the actual plugin - * provisioning state. The props would still have onCallApiUrl set in the plugin jsonData, so when we make the API - * call to check the plugin state w/ OnCall API the plugin-proxy would return a 502 Bad Gateway because the actual - * provisioned plugin doesn't know about the onCallApiUrl. - * - * This could be fixed by instead of passing in the plugin provisioning information as props always fetching it - * when this component renders (via a useEffect). We probably don't need to worry about this because it should happen - * very rarely, if ever - */ const triggerPluginReset = useCallback(async () => { setResettingPlugin(true); resetState(); try { await PluginState.resetPlugin(); + window.location.reload(); } catch (e) { // this should rarely, if ever happen, but we should handle the case nevertheless setPluginResetError('There was an error resetting your plugin, try again.'); @@ -195,16 +186,24 @@ const PluginConfigPage: FC = ({ content = ( <> - + + + {licenseType === GRAFANA_LICENSE_OSS ? : null} + ); } else if (syncError) { content = ( <> - + + + {licenseType === GRAFANA_LICENSE_OSS ? : null} + ); } else if (!pluginIsConnected) { diff --git a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap index 41d4e6ab16..3c2d6ffd14 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap +++ b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap @@ -19,16 +19,39 @@ exports[`PluginConfigPage If onCallApiUrl is not set in the plugin's meta jsonDa ohhh nooo an error msg from self hosted install plugin - + + +
+ +
+ `; @@ -152,16 +175,39 @@ exports[`PluginConfigPage If onCallApiUrl is set, and checkIfPluginIsConnected r ohhh nooo a plugin connection error - + + +
+ +
+ `; @@ -173,14 +219,7 @@ exports[`PluginConfigPage It doesn't make any network calls if the plugin config Configure Grafana OnCall

- Plugin is connected! Continue to Grafana OnCall by clicking the - - Grafana OnCall Logo - icon over there 👈 + Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈

   

- Plugin is connected! Continue to Grafana OnCall by clicking the - - Grafana OnCall Logo - icon over there 👈 + Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈

   

- Plugin is connected! Continue to Grafana OnCall by clicking the - - Grafana OnCall Logo - icon over there 👈 + Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈

   
- + + +
+ +
+ `; @@ -334,16 +382,39 @@ exports[`PluginConfigPage Plugin reset: successful - false 1`] = ` There was an error resetting your plugin, try again.
- + + +
+ +
+ `; @@ -357,93 +428,47 @@ exports[`PluginConfigPage Plugin reset: successful - true 1`] = `

This page will help you configure the OnCall plugin 👋

-
-
-

- 1. Launch the OnCall backend -

- - Run hobby, dev or production backend. See - - - - here - - - - on how to get started. - -
+ There was an error resetting your plugin, try again. + +
+
-

- 2. Let us know the base URL of your OnCall API -

- - The OnCall backend must be reachable from your Grafana installation. Some examples are: -
- - http://host.docker.internal:8080 -
- - http://localhost:8080 -
+ + Retry Sync + +
-
- -
-
-
-
- -
-
-
+ Remove current configuration + +
- - +
`; diff --git a/grafana-plugin/src/plugin/PluginSetup/index.tsx b/grafana-plugin/src/plugin/PluginSetup/index.tsx index 673a53940e..85dfe16e2c 100644 --- a/grafana-plugin/src/plugin/PluginSetup/index.tsx +++ b/grafana-plugin/src/plugin/PluginSetup/index.tsx @@ -8,7 +8,6 @@ import { AppRootProps } from 'types'; import logo from 'img/logo.svg'; import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; -import PluginState from 'state/plugin'; import { useStore } from 'state/useStore'; export type PluginSetupProps = AppRootProps & { @@ -35,10 +34,7 @@ const PluginSetupWrapper: FC = ({ text, children }) => const PluginSetup: FC = observer(({ InitializedComponent, ...props }) => { const store = useStore(); - const setupPlugin = useCallback( - () => PluginState.getGrafanaPluginSettings().then((meta) => store.setupPlugin(meta)), - [] - ); + const setupPlugin = useCallback(() => store.setupPlugin(props.meta), [props.meta]); useEffect(() => { setupPlugin(); diff --git a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap index 1bb513e312..329fd8fce6 100644 --- a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap +++ b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap @@ -1,58 +1,58 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`PluginState.generateInvalidOnCallApiURLErrorMsg it returns the proper error message - configured through env var: false 1`] = ` -"Could not communicate with your OnCall API at http://hello.com. -Validate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance." +"Could not communicate with OnCall API at http://hello.com. +Validate that the URL is correct, OnCall API is running, and that it is accessible from your Grafana instance." `; exports[`PluginState.generateInvalidOnCallApiURLErrorMsg it returns the proper error message - configured through env var: true 1`] = ` -"Could not communicate with your OnCall API at http://hello.com (NOTE: your OnCall API URL is currently being taken from process.env of your UI). -Validate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance." +"Could not communicate with OnCall API at http://hello.com (NOTE: OnCall API URL is currently being taken from process.env of your UI). +Validate that the URL is correct, OnCall API is running, and that it is accessible from your Grafana instance." `; exports[`PluginState.generateOnCallApiUrlConfiguredThroughEnvVarMsg it returns the proper error message - configured through env var: false 1`] = `""`; -exports[`PluginState.generateOnCallApiUrlConfiguredThroughEnvVarMsg it returns the proper error message - configured through env var: true 1`] = `" (NOTE: your OnCall API URL is currently being taken from process.env of your UI)"`; +exports[`PluginState.generateOnCallApiUrlConfiguredThroughEnvVarMsg it returns the proper error message - configured through env var: true 1`] = `" (NOTE: OnCall API URL is currently being taken from process.env of your UI)"`; exports[`PluginState.generateUnknownErrorMsg it returns the proper error message - configured through env var: false 1`] = ` -"An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct? +"An unknown error occurred when trying to install the plugin. Verify OnCall API URL, http://hello.com, is correct? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.generateUnknownErrorMsg it returns the proper error message - configured through env var: false 2`] = ` -"An unknown error occured when trying to sync the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct? +"An unknown error occurred when trying to sync the plugin. Verify OnCall API URL, http://hello.com, is correct? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.generateUnknownErrorMsg it returns the proper error message - configured through env var: true 1`] = ` -"An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? +"An unknown error occurred when trying to install the plugin. Verify OnCall API URL, http://hello.com, is correct (NOTE: OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.generateUnknownErrorMsg it returns the proper error message - configured through env var: true 2`] = ` -"An unknown error occured when trying to sync the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? +"An unknown error occurred when trying to sync the plugin. Verify OnCall API URL, http://hello.com, is correct (NOTE: OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: false 1`] = ` -"An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? +"An unknown error occurred when trying to install the plugin. Verify OnCall API URL, http://hello.com, is correct (NOTE: OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: true 1`] = `"ohhhh nooo an error"`; exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 409 1`] = ` -"An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? +"An unknown error occurred when trying to install the plugin. Verify OnCall API URL, http://hello.com, is correct (NOTE: OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 502 1`] = ` -"Could not communicate with your OnCall API at http://hello.com (NOTE: your OnCall API URL is currently being taken from process.env of your UI). -Validate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance." +"Could not communicate with OnCall API at http://hello.com (NOTE: OnCall API URL is currently being taken from process.env of your UI). +Validate that the URL is correct, OnCall API is running, and that it is accessible from your Grafana instance." `; exports[`PluginState.getHumanReadableErrorFromOnCallError it handles an unknown error properly 1`] = ` -"An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? +"An unknown error occurred when trying to install the plugin. Verify OnCall API URL, http://hello.com, is correct (NOTE: OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; diff --git a/grafana-plugin/src/state/plugin/index.ts b/grafana-plugin/src/state/plugin/index.ts index e1600f6324..7a63925ce4 100644 --- a/grafana-plugin/src/state/plugin/index.ts +++ b/grafana-plugin/src/state/plugin/index.ts @@ -48,21 +48,19 @@ class PluginState { static grafanaBackend = getBackendSrv(); static generateOnCallApiUrlConfiguredThroughEnvVarMsg = (isConfiguredThroughEnvVar: boolean): string => - isConfiguredThroughEnvVar - ? ' (NOTE: your OnCall API URL is currently being taken from process.env of your UI)' - : ''; + isConfiguredThroughEnvVar ? ' (NOTE: OnCall API URL is currently being taken from process.env of your UI)' : ''; static generateInvalidOnCallApiURLErrorMsg = (onCallApiUrl: string, isConfiguredThroughEnvVar: boolean): string => - `Could not communicate with your OnCall API at ${onCallApiUrl}${this.generateOnCallApiUrlConfiguredThroughEnvVarMsg( + `Could not communicate with OnCall API at ${onCallApiUrl}${this.generateOnCallApiUrlConfiguredThroughEnvVarMsg( isConfiguredThroughEnvVar - )}.\nValidate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance.`; + )}.\nValidate that the URL is correct, OnCall API is running, and that it is accessible from your Grafana instance.`; static generateUnknownErrorMsg = ( onCallApiUrl: string, verb: InstallationVerb, isConfiguredThroughEnvVar: boolean ): string => - `An unknown error occured when trying to ${verb} the plugin. Are you sure that your OnCall API URL, ${onCallApiUrl}, is correct${this.generateOnCallApiUrlConfiguredThroughEnvVarMsg( + `An unknown error occurred when trying to ${verb} the plugin. Verify OnCall API URL, ${onCallApiUrl}, is correct${this.generateOnCallApiUrlConfiguredThroughEnvVarMsg( isConfiguredThroughEnvVar )}?\nRefresh your page and try again, or try removing your plugin configuration and reconfiguring.`; @@ -78,7 +76,7 @@ class PluginState { installationVerb, onCallApiUrlIsConfiguredThroughEnvVar ); - const consoleMsg = `occured while trying to ${installationVerb} the plugin w/ the OnCall backend`; + const consoleMsg = `occurred while trying to ${installationVerb} the plugin w/ the OnCall backend`; if (isNetworkError(e)) { const { status: statusCode } = e.response; @@ -104,7 +102,7 @@ class PluginState { errorMsg = unknownErrorMsg; } } else { - // a non-network related error occured.. this scenario shouldn't occur... + // a non-network related error occurred.. this scenario shouldn't occur... console.warn(`An unknown error ${consoleMsg}`, e); errorMsg = unknownErrorMsg; } @@ -121,11 +119,11 @@ class PluginState { if (isNetworkError(e)) { // The user likely put in a bogus URL for the OnCall API URL - console.warn('An HTTP related error occured while trying to provision the plugin w/ Grafana', e.response); + console.warn('An HTTP related error occurred while trying to provision the plugin w/ Grafana', e.response); errorMsg = this.generateInvalidOnCallApiURLErrorMsg(onCallApiUrl, onCallApiUrlIsConfiguredThroughEnvVar); } else { - // a non-network related error occured.. this scenario shouldn't occur... - console.warn('An unknown error occured while trying to provision the plugin w/ Grafana', e); + // a non-network related error occurred.. this scenario shouldn't occur... + console.warn('An unknown error occurred while trying to provision the plugin w/ Grafana', e); errorMsg = this.generateUnknownErrorMsg(onCallApiUrl, installationVerb, onCallApiUrlIsConfiguredThroughEnvVar); } return errorMsg; @@ -211,16 +209,25 @@ class PluginState { try { /** * Allows the plugin config page to repair settings like the app initialization screen if a user deletes - * an API on accident but leaves the plugin settings intact. + * an API key on accident but leaves the plugin settings intact. */ const existingKey = await this.getGrafanaToken(); if (!existingKey) { - await this.installPlugin(); + try { + await this.installPlugin(); + } catch (e) { + return this.getHumanReadableErrorFromOnCallError( + e, + onCallApiUrl, + 'install', + onCallApiUrlIsConfiguredThroughEnvVar + ); + } } const startSyncResponse = await makeRequest(`${this.ONCALL_BASE_URL}/sync`, { method: 'POST' }); if (typeof startSyncResponse === 'string') { - // an error occured trying to initiate the sync + // an error occurred trying to initiate the sync return startSyncResponse; } diff --git a/grafana-plugin/src/state/plugin/plugin.test.ts b/grafana-plugin/src/state/plugin/plugin.test.ts index 6080cf2a0b..24292a9dad 100644 --- a/grafana-plugin/src/state/plugin/plugin.test.ts +++ b/grafana-plugin/src/state/plugin/plugin.test.ts @@ -383,6 +383,7 @@ describe('PluginState.syncDataWithOnCall', () => { const errorMsg = 'asdfasdf'; makeRequest.mockResolvedValueOnce(errorMsg); + PluginState.getGrafanaToken = jest.fn().mockReturnValueOnce({ id: 1 }); PluginState.pollOnCallDataSyncStatus = jest.fn(); // test @@ -403,6 +404,7 @@ describe('PluginState.syncDataWithOnCall', () => { const mockedPollOnCallDataSyncStatusResponse = 'dfjkdfjdf'; makeRequest.mockResolvedValueOnce(mockedResponse); + PluginState.getGrafanaToken = jest.fn().mockReturnValueOnce({ id: 1 }); PluginState.pollOnCallDataSyncStatus = jest.fn().mockResolvedValueOnce(mockedPollOnCallDataSyncStatusResponse); // test @@ -427,6 +429,7 @@ describe('PluginState.syncDataWithOnCall', () => { const mockedHumanReadableError = 'asdfjkdfjkdfjk'; makeRequest.mockRejectedValueOnce(mockedError); + PluginState.getGrafanaToken = jest.fn().mockReturnValueOnce({ id: 1 }); PluginState.pollOnCallDataSyncStatus = jest.fn(); PluginState.getHumanReadableErrorFromOnCallError = jest.fn().mockReturnValueOnce(mockedHumanReadableError); @@ -670,7 +673,7 @@ describe('PluginState.checkIfBackendIsInMaintenanceMode', () => { const response = await PluginState.checkIfBackendIsInMaintenanceMode(onCallApiUrl); // assertions - expect(response).toEqual(maintenanceModeMsg); + expect(response).toEqual(mockedResp); expect(makeRequest).toHaveBeenCalledTimes(1); expect(makeRequest).toHaveBeenCalledWith('/maintenance-mode-status', { method: 'GET' }); }); diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index c2796dcdd3..e2c80f194b 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -34,7 +34,7 @@ import { UserGroupStore } from 'models/user_group/user_group'; import { makeRequest } from 'network'; import { AppFeature } from 'state/features'; import PluginState from 'state/plugin'; -import { GRAFANA_LICENSE_OSS } from 'utils/consts'; +import { APP_VERSION, CLOUD_VERSION_REGEX, GRAFANA_LICENSE_CLOUD, GRAFANA_LICENSE_OSS } from 'utils/consts'; // ------ Dashboard ------ // @@ -181,7 +181,7 @@ export class RootBaseStore { if (is_user_anonymous) { return this.setupPluginError( - '😞 Unfortunately Grafana OnCall is available for authorized users only, please sign in to proceed.' + '😞 Grafana OnCall is available for authorized users only, please sign in to proceed.' ); } else if (!is_installed || !token_ok) { if (!allow_signup) { @@ -249,8 +249,18 @@ export class RootBaseStore { return this.features?.[feature]; } + getLicense() { + if (this.backendLicense) { + return this.backendLicense; + } + if (CLOUD_VERSION_REGEX.test(APP_VERSION)) { + return GRAFANA_LICENSE_CLOUD; + } + return GRAFANA_LICENSE_OSS; + } + isOpenSource(): boolean { - return this.backendLicense === GRAFANA_LICENSE_OSS; + return this.getLicense() === GRAFANA_LICENSE_OSS; } @observable diff --git a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts index 4dedc2f832..f8f13c128d 100644 --- a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts +++ b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts @@ -49,7 +49,9 @@ describe('rootBaseStore', () => { const onCallApiUrl = 'http://asdfasdf.com'; const rootBaseStore = new RootBaseStore(); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(errorMsg); // test @@ -69,14 +71,16 @@ describe('rootBaseStore', () => { const rootBaseStore = new RootBaseStore(); const maintenanceMessage = 'mncvnmvcmnvkjdjkd'; - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(maintenanceMessage); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: maintenanceMessage }); // test await rootBaseStore.setupPlugin(generatePluginData(onCallApiUrl)); // assertions expect(PluginState.checkIfBackendIsInMaintenanceMode).toHaveBeenCalledTimes(1); - expect(PluginState.checkIfBackendIsInMaintenanceMode).toHaveBeenCalledWith(); + expect(PluginState.checkIfBackendIsInMaintenanceMode).toHaveBeenCalledWith(onCallApiUrl); expect(rootBaseStore.appLoading).toBe(false); expect(rootBaseStore.initializationError).toEqual(`🚧 ${maintenanceMessage} 🚧`); @@ -88,7 +92,9 @@ describe('rootBaseStore', () => { const onCallApiUrl = 'http://asdfasdf.com'; const rootBaseStore = new RootBaseStore(); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: true, is_installed: true, @@ -107,7 +113,7 @@ describe('rootBaseStore', () => { expect(rootBaseStore.appLoading).toBe(false); expect(rootBaseStore.initializationError).toEqual( - '😞 Unfortunately Grafana OnCall is available for authorized users only, please sign in to proceed.' + '😞 Grafana OnCall is available for authorized users only, please sign in to proceed.' ); }); @@ -116,7 +122,9 @@ describe('rootBaseStore', () => { const onCallApiUrl = 'http://asdfasdf.com'; const rootBaseStore = new RootBaseStore(); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, is_installed: false, @@ -151,7 +159,9 @@ describe('rootBaseStore', () => { contextSrv.accessControlEnabled = jest.fn().mockReturnValue(false); contextSrv.hasAccess = jest.fn().mockReturnValue(false); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, is_installed: false, @@ -191,7 +201,9 @@ describe('rootBaseStore', () => { contextSrv.accessControlEnabled = jest.fn().mockResolvedValueOnce(false); contextSrv.hasAccess = jest.fn().mockReturnValue(true); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ ...scenario, is_user_anonymous: false, @@ -243,7 +255,9 @@ describe('rootBaseStore', () => { contextSrv.accessControlEnabled = jest.fn().mockReturnValue(true); rootBaseStore.checkMissingSetupPermissions = jest.fn().mockImplementation(() => scenario.missing_permissions); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ ...scenario, is_user_anonymous: false, @@ -294,7 +308,9 @@ describe('rootBaseStore', () => { contextSrv.accessControlEnabled = jest.fn().mockReturnValue(false); contextSrv.hasAccess = jest.fn().mockReturnValue(true); - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, is_installed: false, @@ -336,7 +352,9 @@ describe('rootBaseStore', () => { const version = 'asdfalkjslkjdf'; const license = 'lkjdkjfdkjfdjkfd'; - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, is_installed: true, @@ -372,7 +390,9 @@ describe('rootBaseStore', () => { const mockedLoadCurrentUser = jest.fn(); const syncDataWithOnCallError = 'asdasdfasdfasf'; - PluginState.checkIfBackendIsInMaintenanceMode = jest.fn().mockResolvedValueOnce(null); + PluginState.checkIfBackendIsInMaintenanceMode = jest + .fn() + .mockResolvedValueOnce({ currently_undergoing_maintenance_message: null }); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce({ is_user_anonymous: false, is_installed: true, diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index a5951b5cb9..f34cd0fc08 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -4,9 +4,15 @@ import plugin from '../../package.json'; // eslint-disable-line export const APP_TITLE = 'Grafana OnCall'; export const APP_SUBTITLE = `Developer-friendly incident response (${plugin?.version})`; +export const APP_VERSION = `${plugin?.version}`; + +export const CLOUD_VERSION_REGEX = new RegExp('r[\\d]+-v[\\d]+.[\\d]+.[\\d]+'); + // License export const GRAFANA_LICENSE_OSS = 'OpenSource'; +export const GRAFANA_LICENSE_CLOUD = 'Cloud'; + // Reusable breakpoint sizes export const BREAKPOINT_TABS = 1024; From a8d9f3357e1b6122cff727ed4fb9aa547ee5e3b2 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 22 Jun 2023 13:58:13 -0600 Subject: [PATCH 7/8] Fixes for comments --- .../PluginConfigPage.test.tsx | 4 +- .../PluginConfigPage/PluginConfigPage.tsx | 30 +++-- .../PluginConfigPage.test.tsx.snap | 108 +++++++++++++----- .../src/state/rootBaseStore/index.ts | 4 +- grafana-plugin/src/utils/consts.ts | 2 + 5 files changed, 96 insertions(+), 52 deletions(-) diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx index 1885e26eeb..f186050aad 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.test.tsx @@ -283,7 +283,7 @@ describe('PluginConfigPage', () => { const metaJsonDataOnCallApiUrl = 'onCallApiUrlFromMetaJsonData'; process.env.ONCALL_API_URL = processEnvOnCallApiUrl; - + window.location.reload = jest.fn(); PluginState.checkIfPluginIsConnected = jest.fn().mockResolvedValueOnce(null); mockSyncDataWithOnCall(License.OSS); @@ -302,8 +302,6 @@ describe('PluginConfigPage', () => { // click the confirm button within the modal, which actually triggers the callback await userEvent.click(screen.getByText('Remove')); - // await screen.findByTestId(successful ? PLUGIN_CONFIGURATION_FORM_DATA_ID : STATUS_MESSAGE_BLOCK_DATA_ID); - // assertions expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledTimes(1); expect(PluginState.checkIfPluginIsConnected).toHaveBeenCalledWith(metaJsonDataOnCallApiUrl); diff --git a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx index a2248ed3fe..6b25db8722 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx +++ b/grafana-plugin/src/containers/PluginConfigPage/PluginConfigPage.tsx @@ -5,7 +5,7 @@ import { useLocation } from 'react-router-dom'; import { OnCallPluginConfigPageProps } from 'types'; import PluginState, { PluginStatusResponseBase } from 'state/plugin'; -import { APP_VERSION, CLOUD_VERSION_REGEX, GRAFANA_LICENSE_CLOUD, GRAFANA_LICENSE_OSS } from 'utils/consts'; +import { FALLBACK_LICENSE, GRAFANA_LICENSE_OSS } from 'utils/consts'; import ConfigurationForm from './parts/ConfigurationForm'; import RemoveCurrentConfigurationButton from './parts/RemoveCurrentConfigurationButton'; @@ -74,13 +74,12 @@ const PluginConfigPage: FC = ({ const pluginMetaOnCallApiUrl = jsonData?.onCallApiUrl; const processEnvOnCallApiUrl = process.env.ONCALL_API_URL; // don't destructure this, will break how webpack supplies this const onCallApiUrl = pluginMetaOnCallApiUrl || processEnvOnCallApiUrl; - const fallbackLicense = CLOUD_VERSION_REGEX.test(APP_VERSION) ? GRAFANA_LICENSE_CLOUD : GRAFANA_LICENSE_OSS; - const licenseType = pluginIsConnected?.license || fallbackLicense; + const licenseType = pluginIsConnected?.license || FALLBACK_LICENSE; const resetQueryParams = useCallback(() => removePluginConfiguredQueryParams(pluginIsEnabled), [pluginIsEnabled]); const triggerDataSyncWithOnCall = useCallback(async () => { - await resetMessages(); + resetMessages(); setSyncingPlugin(true); const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl); @@ -176,6 +175,15 @@ const PluginConfigPage: FC = ({ [resettingPlugin, triggerPluginReset] ); + const ReconfigurePluginButtons = () => ( + + + {licenseType === GRAFANA_LICENSE_OSS ? : null} + + ); + let content: React.ReactNode; if (checkingIfPluginIsConnected) { @@ -186,24 +194,14 @@ const PluginConfigPage: FC = ({ content = ( <> - - - {licenseType === GRAFANA_LICENSE_OSS ? : null} - + ); } else if (syncError) { content = ( <> - - - {licenseType === GRAFANA_LICENSE_OSS ? : null} - + ); } else if (!pluginIsConnected) { diff --git a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap index 3c2d6ffd14..cd7e024ff2 100644 --- a/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap +++ b/grafana-plugin/src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap @@ -428,47 +428,93 @@ exports[`PluginConfigPage Plugin reset: successful - true 1`] = `

This page will help you configure the OnCall plugin 👋

-
-    
-      There was an error resetting your plugin, try again.
-    
-  
-
- + + here + + + + on how to get started. +
-
+
+
+ +
+
+
- Remove current configuration - - +
+ +
+
+
-
+ + `; diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index e2c80f194b..262a56f5f8 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -249,7 +249,7 @@ export class RootBaseStore { return this.features?.[feature]; } - getLicense() { + get license() { if (this.backendLicense) { return this.backendLicense; } @@ -260,7 +260,7 @@ export class RootBaseStore { } isOpenSource(): boolean { - return this.getLicense() === GRAFANA_LICENSE_OSS; + return this.license === GRAFANA_LICENSE_OSS; } @observable diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index b308ada3fc..2ee3b7c067 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -13,6 +13,8 @@ export const GRAFANA_LICENSE_OSS = 'OpenSource'; export const GRAFANA_LICENSE_CLOUD = 'Cloud'; +export const FALLBACK_LICENSE = CLOUD_VERSION_REGEX.test(APP_VERSION) ? GRAFANA_LICENSE_CLOUD : GRAFANA_LICENSE_OSS; + // height of new Grafana sticky header with breadcrumbs export const GRAFANA_HEADER_HEIGTH = 80; From db9a4943faaef38b4949eb1324ffa64649b53ef5 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 23 Jun 2023 10:39:40 -0600 Subject: [PATCH 8/8] Change permission for user read --- grafana-plugin/src/state/rootBaseStore/index.ts | 8 +++++++- .../src/state/rootBaseStore/rootBaseStore.test.ts | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index 262a56f5f8..eaf8804acb 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -238,7 +238,13 @@ export class RootBaseStore { checkMissingSetupPermissions() { const fallback = contextSrv.user.orgRole === OrgRole.Admin && !contextSrv.accessControlEnabled(); - const setupRequiredPermissions = ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete']; + const setupRequiredPermissions = [ + 'plugins:write', + 'org.users:read', + 'teams:read', + 'apikeys:create', + 'apikeys:delete', + ]; return setupRequiredPermissions.filter(function (permission) { return !contextSrv.hasAccess(permission, fallback); }); diff --git a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts index f8f13c128d..e3229fc215 100644 --- a/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts +++ b/grafana-plugin/src/state/rootBaseStore/rootBaseStore.test.ts @@ -237,12 +237,12 @@ describe('rootBaseStore', () => { { role: OrgRole.Viewer, missing_permissions: [], expected_result: true }, { role: OrgRole.Admin, - missing_permissions: ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], + missing_permissions: ['plugins:write', 'org.users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], expected_result: false, }, { role: OrgRole.Viewer, - missing_permissions: ['plugins:write', 'users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], + missing_permissions: ['plugins:write', 'org.users:read', 'teams:read', 'apikeys:create', 'apikeys:delete'], expected_result: false, }, ])('signup is allowed, accessControlEnabled, various roles and permissions', async (scenario) => {