Skip to content

Commit

Permalink
Change permissions used for finishing plugin setup (#2242)
Browse files Browse the repository at this point in the history 
Fixes issue where user not having `plugins:install`permission were
unable to complete setup of OnCall.

- Check multiple Grafana permissions to complete OnCall setup instead of
`plugins:install` since the plugin is already installed at this point
- Use the following permissions
  - `plugins:write` - Plugin setup will write to plugin config
- `users:read` - Grafana API key being granted to OnCall will be used to
read users from Grafana
- `teams:read` - Grafana API key being granted to OnCall will be used to
read teams from Grafana
- `apikeys:create` - If Grafana API key does not exist it will be
created
- `apikeys:delete` - If existing Grafana API key does not work it will
be deleted and recreated

Closes grafana/oncall-private#1925

TODO:
- [x] Fix tests
  • Loading branch information
@mderynck
mderynck authored Jun 26, 2023
1 parent 894593c commit de7ab97
Show file tree
Hide file tree
Showing 10 changed files with 412 additions and 178 deletions.
4 changes: 1 addition & 3 deletions src/containers/PluginConfigPage/PluginConfigPage.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down
50 changes: 23 additions & 27 deletions src/containers/PluginConfigPage/PluginConfigPage.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,12 +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 logo from 'img/logo.svg';
import PluginState, { PluginStatusResponseBase } from 'state/plugin';
import { 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';
Expand Down Expand Up @@ -75,13 +74,13 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
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 licenseType = pluginIsConnected?.license || FALLBACK_LICENSE;

const resetQueryParams = useCallback(() => removePluginConfiguredQueryParams(pluginIsEnabled), [pluginIsEnabled]);

const triggerDataSyncWithOnCall = useCallback(async () => {
resetMessages();
setSyncingPlugin(true);
setSyncError(null);

const syncDataResponse = await PluginState.syncDataWithOnCall(onCallApiUrl);

Expand Down Expand Up @@ -144,35 +143,25 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
}
}, [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.');
Expand All @@ -186,6 +175,15 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
[resettingPlugin, triggerPluginReset]
);

const ReconfigurePluginButtons = () => (
<HorizontalGroup>
<Button variant="primary" onClick={triggerDataSyncWithOnCall} size="md">
Retry Sync
</Button>
{licenseType === GRAFANA_LICENSE_OSS ? <RemoveConfigButton /> : null}
</HorizontalGroup>
);

let content: React.ReactNode;

if (checkingIfPluginIsConnected) {
Expand All @@ -196,16 +194,14 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
content = (
<>
<StatusMessageBlock text={pluginConnectionCheckError || pluginResetError} />
<RemoveConfigButton />
<ReconfigurePluginButtons />
</>
);
} else if (syncError) {
content = (
<>
<StatusMessageBlock text={syncError} />
<Button variant="primary" onClick={triggerDataSyncWithOnCall} size="md">
Retry Sync
</Button>
<ReconfigurePluginButtons />
</>
);
} else if (!pluginIsConnected) {
Expand All @@ -228,8 +224,8 @@ const PluginConfigPage: FC<OnCallPluginConfigPageProps> = ({
{pluginIsConnected ? (
<>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the{' '}
<img alt="Grafana OnCall Logo" src={logo} width={18} /> icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over
there 👈
</p>
<StatusMessageBlock
text={`Connected to OnCall (${pluginIsConnected.version}, ${pluginIsConnected.license})`}
Expand Down
183 changes: 127 additions & 56 deletions src/containers/PluginConfigPage/__snapshots__/PluginConfigPage.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand Down Expand Up @@ -152,16 +175,39 @@ exports[`PluginConfigPage If onCallApiUrl is set, and checkIfPluginIsConnected r
ohhh nooo a plugin connection error
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand All @@ -173,14 +219,7 @@ exports[`PluginConfigPage It doesn't make any network calls if the plugin config
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the
<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -212,14 +251,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the
<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -251,14 +283,7 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall does not r
Configure Grafana OnCall
</legend>
<p>
Plugin is connected! Continue to Grafana OnCall by clicking the
<img
alt="Grafana OnCall Logo"
src="[object Object]"
width="18"
/>
icon over there 👈
Plugin is connected! Continue to Grafana OnCall by clicking OnCall under Alerts & IRM in the navigation over there 👈
</p>
<pre
data-testid="status-message-block"
Expand Down Expand Up @@ -302,16 +327,39 @@ exports[`PluginConfigPage OnCallApiUrl is set, and syncDataWithOnCall returns an
ohhh noooo a sync issue
</span>
</pre>
<button
class="css-z53gi5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Retry Sync
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand All @@ -334,16 +382,39 @@ exports[`PluginConfigPage Plugin reset: successful - false 1`] = `
There was an error resetting your plugin, try again.
</span>
</pre>
<button
class="css-1ed0qk5-button"
type="button"
<div
class="css-ve64a7-horizontal-group"
style="width: 100%; height: 100%;"
>
<span
class="css-1mhnkuh"
<div
class="css-cvef6c-layoutChildrenWrapper"
>
Remove current configuration
</span>
</button>
<button
class="css-z53gi5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Retry Sync
</span>
</button>
</div>
<div
class="css-cvef6c-layoutChildrenWrapper"
>
<button
class="css-1ed0qk5-button"
type="button"
>
<span
class="css-1mhnkuh"
>
Remove current configuration
</span>
</button>
</div>
</div>
</div>
`;

Expand Down
Loading

0 comments on commit de7ab97

Please sign in to comment.