From: Stephan Müller Date: Wed, 3 Apr 2019 14:13:54 +0000 (+0200) Subject: mgr/dashboard: Enable read only users to read again X-Git-Tag: v14.2.1~7^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f480de4dc24434427ec062e01e79210b68c2b475;p=ceph.git 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 (cherry picked from commit 35a75d088963043c77830865bc943e7d8c30f7e7) --- diff --git a/src/pybind/mgr/dashboard/controllers/prometheus.py b/src/pybind/mgr/dashboard/controllers/prometheus.py index d233361a5b1d..b80fc05c4bc0 100644 --- 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 0a0c49fc3be3..ed3edc9fe013 100644 --- 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 0fd288e9c01b..e17c5cf84f53 100644 --- 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(this.baseURL, { params }); } - getNotificationSince(notification): Observable { - return this.http.post( - `${this.baseURL}/get_notifications_since`, - notification - ); + getNotifications(notification?: PrometheusNotification): Observable { + const url = `${this.baseURL}/notifications?from=${ + notification && notification.id ? notification.id : 'last' + }`; + return this.http.get(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 204333110d5e..b45147623507 100644 --- 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 d395e84064be..b1f8b3b8b7b3 100644 --- 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 b931c26e09b9..b7fc85fbd6f9 100644 --- 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 2f3fb99271d9..1f9d79588b2d 100644 --- 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])