Change permissions used for finishing plugin setup (#2242)

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
grafana · Jun 26, 2023 · dd713ba · dd713ba
1 parent d23e2f4
commit dd713ba
Show file tree

Hide file tree

Showing 11 changed files with 418 additions and 178 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changed
+
+- Change permissions used during setup to better represent actions being taken by @mderynck ([#2242](https://github.com/grafana/oncall/pull/2242))
+
 ## v1.3.1 (2023-06-26)
 
 ### Fixed

diff --git 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
@@ -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';
@@ -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);
 
@@ -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.');
@@ -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) {
@@ -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) {
@@ -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})`}

diff --git 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
     </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>
 `;
 
@@ -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>
 `;
 
@@ -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"
@@ -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"
@@ -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"
@@ -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>
 `;
 
@@ -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>
 `;