mgr/dashboard: Enable read only users to read again

The dashboards Prometheus receiver API needed to receive a POST from the
frontend, but POSTs aren't allowed by default by any read only
users. As a result the receiver API call had thrown a 403 error which
redirected the user to the 403 error page. Now you can get the last
notifications via GET. This prevents the redirection for read only
users, as a result they can get the last notifications and also
see all other allowed pages again.

Fixes: https://tracker.ceph.com/issues/39086
Signed-off-by: Stephan Müller <smueller@suse.com>

diff --git a/src/pybind/mgr/dashboard/controllers/prometheus.py b/src/pybind/mgr/dashboard/controllers/prometheus.py
index d233361a5b1da1d53760b859aa10c8340714ae3d..b80fc05c4bc057d71f96d543ff3dfdca8f03938a 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/prometheus.py
+++ b/src/pybind/mgr/dashboard/controllers/prometheus.py
@@ -12,17 +12,19 @@ from ..settings import Settings
 
 @Controller('/api/prometheus_receiver', secure=False)
 class PrometheusReceiver(BaseController):
-    # The receiver is needed in order to receive alert notifications (reports)
+    ''' The receiver is needed in order to receive alert notifications (reports) '''
     notifications = []
 
     @Endpoint('POST', path='/')
     def fetch_alert(self, **notification):
         notification['notified'] = datetime.now().isoformat()
+        notification['id'] = str(len(self.notifications))
         self.notifications.append(notification)
 
 
 @ApiController('/prometheus', Scope.PROMETHEUS)
 class Prometheus(RESTController):
+
     def _get_api_url(self):
         return Settings.ALERTMANAGER_API_HOST.rstrip('/') + '/api/v1'
 
@@ -35,10 +37,14 @@ class Prometheus(RESTController):
     def list(self, **params):
         return self._api_request('/alerts', params)
 
-    @RESTController.Collection('POST')
-    def get_notifications_since(self, **last_notification):
-        notifications = PrometheusReceiver.notifications
-        if last_notification not in notifications:
-            return notifications[-1:]
-        index = notifications.index(last_notification)
-        return notifications[index + 1:]
+
+@ApiController('/prometheus/notifications', Scope.PROMETHEUS)
+class PrometheusNotifications(RESTController):
+
+    def list(self, **params):
+        if 'from' in params:
+            f = params['from']
+            if f == 'last':
+                return PrometheusReceiver.notifications[-1:]
+            return PrometheusReceiver.notifications[int(f) + 1:]
+        return PrometheusReceiver.notifications

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.spec.ts
index 0a0c49fc3be3cfe66bbc5ebe3881cce5fe21c0bd..ed3edc9fe013d4d21fa48741dec54b5f915b13eb 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.spec.ts
@@ -2,6 +2,7 @@ import { HttpClientTestingModule, HttpTestingController } from '@angular/common/
 import { TestBed } from '@angular/core/testing';
 
 import { configureTestBed } from '../../../testing/unit-test-helper';
+import { PrometheusNotification } from '../models/prometheus-alerts';
 import { PrometheusService } from './prometheus.service';
 import { SettingsService } from './settings.service';
 
@@ -33,10 +34,16 @@ describe('PrometheusService', () => {
     expect(req.request.method).toBe('GET');
   });
 
-  it('should call getNotificationSince', () => {
-    service.getNotificationSince({}).subscribe();
-    const req = httpTesting.expectOne('api/prometheus/get_notifications_since');
-    expect(req.request.method).toBe('POST');
+  it('should call getNotificationSince without a notification', () => {
+    service.getNotifications().subscribe();
+    const req = httpTesting.expectOne('api/prometheus/notifications?from=last');
+    expect(req.request.method).toBe('GET');
+  });
+
+  it('should call getNotificationSince with notification', () => {
+    service.getNotifications({ id: '42' } as PrometheusNotification).subscribe();
+    const req = httpTesting.expectOne('api/prometheus/notifications?from=42');
+    expect(req.request.method).toBe('GET');
   });
 
   describe('ifAlertmanagerConfigured', () => {

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.ts
index 0fd288e9c01bb6f1a5f43a10ba3e5fa19d91abfb..e17c5cf84f538bb12b06946bdc3c47de9b1f6567 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/prometheus.service.ts
@@ -23,10 +23,10 @@ export class PrometheusService {
     return this.http.get<PrometheusAlert[]>(this.baseURL, { params });
   }
 
-  getNotificationSince(notification): Observable<PrometheusNotification[]> {
-    return this.http.post<PrometheusNotification[]>(
-      `${this.baseURL}/get_notifications_since`,
-      notification
-    );
+  getNotifications(notification?: PrometheusNotification): Observable<PrometheusNotification[]> {
+    const url = `${this.baseURL}/notifications?from=${
+      notification && notification.id ? notification.id : 'last'
+    }`;
+    return this.http.get<PrometheusNotification[]>(url);
   }
 }

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts
index 204333110d5ee83be3f715b182fbbb27da2f244c..b45147623507be97115ab318307a1ec12e715ad5 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts
@@ -34,6 +34,7 @@ export class PrometheusNotification {
   commonAnnotations: object;
   groupKey: string;
   notified: string;
+  id: string;
   alerts: PrometheusNotificationAlert[];
   version: string;
   receiver: string;

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts
index d395e84064be8350db073d463a7b6ca7ed8f5040..b1f8b3b8b7b3cbf2e08ad5035a32189357168852 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts
@@ -45,7 +45,7 @@ describe('PrometheusNotificationService', () => {
     spyOn(window, 'setTimeout').and.callFake((fn: Function) => fn());
 
     prometheusService = TestBed.get(PrometheusService);
-    spyOn(prometheusService, 'getNotificationSince').and.callFake(() => of(notifications));
+    spyOn(prometheusService, 'getNotifications').and.callFake(() => of(notifications));
 
     notifications = [prometheus.createNotification()];
   });
@@ -57,7 +57,7 @@ describe('PrometheusNotificationService', () => {
   describe('getLastNotification', () => {
     it('returns an empty object on the first call', () => {
       service.refresh();
-      expect(prometheusService.getNotificationSince).toHaveBeenCalledWith({});
+      expect(prometheusService.getNotifications).toHaveBeenCalledWith(undefined);
       expect(service['notifications'].length).toBe(1);
     });
 
@@ -65,18 +65,14 @@ describe('PrometheusNotificationService', () => {
       service.refresh();
       notifications = [prometheus.createNotification(1, 'resolved')];
       service.refresh();
-      expect(prometheusService.getNotificationSince).toHaveBeenCalledWith(
-        service['notifications'][0]
-      );
+      expect(prometheusService.getNotifications).toHaveBeenCalledWith(service['notifications'][0]);
       expect(service['notifications'].length).toBe(2);
 
       notifications = [prometheus.createNotification(2)];
       service.refresh();
       notifications = [prometheus.createNotification(3, 'resolved')];
       service.refresh();
-      expect(prometheusService.getNotificationSince).toHaveBeenCalledWith(
-        service['notifications'][2]
-      );
+      expect(prometheusService.getNotifications).toHaveBeenCalledWith(service['notifications'][2]);
       expect(service['notifications'].length).toBe(4);
     });
   });

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.ts
index b931c26e09b9f6a269c949a88caf7bf89b968b0e..b7fc85fbd6f96b75633160b052e4fb581443190a 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.ts
@@ -22,16 +22,11 @@ export class PrometheusNotificationService {
   }
 
   refresh() {
-    const last = this.getLastNotification();
     this.prometheusService
-      .getNotificationSince(last)
+      .getNotifications(_.last(this.notifications))
       .subscribe((notifications) => this.handleNotifications(notifications));
   }
 
-  private getLastNotification() {
-    return _.last(this.notifications) || {};
-  }
-
   private handleNotifications(notifications: PrometheusNotification[]) {
     if (notifications.length === 0) {
       return;

diff --git a/src/pybind/mgr/dashboard/tests/test_prometheus.py b/src/pybind/mgr/dashboard/tests/test_prometheus.py
index 2f3fb99271d9528a5be718dc9279774c8de8a5cf..1f9d79588b2db010c278a7ab983cd4b8fc729a62 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_prometheus.py
+++ b/src/pybind/mgr/dashboard/tests/test_prometheus.py
@@ -1,8 +1,9 @@
 # -*- coding: utf-8 -*-
+# pylint: disable=protected-access
 from . import ControllerTestCase
 from .. import mgr
 from ..controllers import BaseController, Controller
-from ..controllers.prometheus import Prometheus, PrometheusReceiver
+from ..controllers.prometheus import Prometheus, PrometheusReceiver, PrometheusNotifications
 
 
 @Controller('alertmanager/mocked/api/v1/alerts', secure=False)
@@ -18,8 +19,10 @@ class PrometheusControllerTest(ControllerTestCase):
             'ALERTMANAGER_API_HOST': 'http://localhost:{}/alertmanager/mocked/'.format(54583)
         }
         mgr.get_module_option.side_effect = settings.get
-        Prometheus._cp_config['tools.authenticate.on'] = False  # pylint: disable=protected-access
-        cls.setup_controllers([AlertManagerMockInstance, Prometheus, PrometheusReceiver])
+        Prometheus._cp_config['tools.authenticate.on'] = False
+        PrometheusNotifications._cp_config['tools.authenticate.on'] = False
+        cls.setup_controllers([AlertManagerMockInstance, Prometheus,
+                               PrometheusNotifications, PrometheusReceiver])
 
     def test_list(self):
         self._get('/api/prometheus')
@@ -33,26 +36,47 @@ class PrometheusControllerTest(ControllerTestCase):
         self.assertEqual(notification['name'], 'foo')
         self.assertTrue(len(notification['notified']) > 20)
 
-    def test_get_last_notification_with_empty_notifications(self):
+    def test_get_empty_list_with_no_notifications(self):
+        PrometheusReceiver.notifications = []
+        self._get('/api/prometheus/notifications')
+        self.assertStatus(200)
+        self.assertJsonBody([])
+        self._get('/api/prometheus/notifications?from=last')
+        self.assertStatus(200)
+        self.assertJsonBody([])
+
+    def test_get_all_notification(self):
+        PrometheusReceiver.notifications = []
+        self._post('/api/prometheus_receiver', {'name': 'foo'})
+        self._post('/api/prometheus_receiver', {'name': 'bar'})
+        self._get('/api/prometheus/notifications')
+        self.assertStatus(200)
+        self.assertJsonBody(PrometheusReceiver.notifications)
+
+    def test_get_last_notification_with_use_of_last_keyword(self):
         PrometheusReceiver.notifications = []
         self._post('/api/prometheus_receiver', {'name': 'foo'})
         self._post('/api/prometheus_receiver', {'name': 'bar'})
-        self._post('/api/prometheus/get_notifications_since', {})
+        self._get('/api/prometheus/notifications?from=last')
         self.assertStatus(200)
         last = PrometheusReceiver.notifications[1]
-        self.assertEqual(self.jsonBody(), [last])
+        self.assertJsonBody([last])
 
-    def test_get_no_notification_since_with_last_notification(self):
+    def test_get_no_notification_with_unknown_id(self):
         PrometheusReceiver.notifications = []
         self._post('/api/prometheus_receiver', {'name': 'foo'})
-        notification = PrometheusReceiver.notifications[0]
-        self._post('/api/prometheus/get_notifications_since', notification)
-        self.assertBody('[]')
+        self._post('/api/prometheus_receiver', {'name': 'bar'})
+        self._get('/api/prometheus/notifications?from=42')
+        self.assertStatus(200)
+        self.assertJsonBody([])
 
-    def test_get_empty_list_with_no_notifications(self):
+    def test_get_no_notification_since_with_last_notification(self):
         PrometheusReceiver.notifications = []
-        self._post('/api/prometheus/get_notifications_since', {})
-        self.assertEqual(self.jsonBody(), [])
+        self._post('/api/prometheus_receiver', {'name': 'foo'})
+        notification = PrometheusReceiver.notifications[0]
+        self._get('/api/prometheus/notifications?from=' + notification['id'])
+        self.assertStatus(200)
+        self.assertJsonBody([])
 
     def test_get_notifications_since_last_notification(self):
         PrometheusReceiver.notifications = []
@@ -60,7 +84,7 @@ class PrometheusControllerTest(ControllerTestCase):
         next_to_last = PrometheusReceiver.notifications[0]
         self._post('/api/prometheus_receiver', {'name': 'foo'})
         self._post('/api/prometheus_receiver', {'name': 'bar'})
-        self._post('/api/prometheus/get_notifications_since', next_to_last)
+        self._get('/api/prometheus/notifications?from=' + next_to_last['id'])
         foreLast = PrometheusReceiver.notifications[1]
         last = PrometheusReceiver.notifications[2]
         self.assertEqual(self.jsonBody(), [foreLast, last])