diff --git a/aodh/api/controllers/v2/alarm_rules/prometheus.py b/aodh/api/controllers/v2/alarm_rules/prometheus.py index 4e2d24539..8cb90caa0 100644 --- a/aodh/api/controllers/v2/alarm_rules/prometheus.py +++ b/aodh/api/controllers/v2/alarm_rules/prometheus.py @@ -18,6 +18,7 @@ import wsme from wsme import types as wtypes from aodh.api.controllers.v2 import base +from aodh.api.controllers.v2 import utils as v2_utils LOG = log.getLogger(__name__) @@ -35,12 +36,19 @@ class PrometheusRule(base.AlarmRule): query = wsme.wsattr(wtypes.text, mandatory=True) "The Prometheus query" - @staticmethod - def validate(rule): - # TO-DO(mmagr): validate Prometheus query maybe? + @classmethod + def validate_alarm(cls, alarm): + super().validate_alarm(alarm) + + rule = alarm.prometheus_rule + + auth_project = v2_utils.get_auth_project(alarm.project_id) + cls.scope_to_project = None + if auth_project: + cls.scope_to_project = auth_project return rule def as_dict(self): rule = self.as_dict_from_keys(['comparison_operator', 'threshold', - 'query']) + 'query', 'scope_to_project']) return rule diff --git a/aodh/evaluator/prometheus.py b/aodh/evaluator/prometheus.py index f9cc61e43..b2054a0a1 100644 --- a/aodh/evaluator/prometheus.py +++ b/aodh/evaluator/prometheus.py @@ -17,6 +17,7 @@ from oslo_config import cfg from oslo_log import log from observabilityclient import client +from observabilityclient import rbac as obsc_rbac from aodh.evaluator import threshold from aodh import keystone_client @@ -32,7 +33,13 @@ OPTS = [ "It's not possible to correctly use " "client-side rbac enforcement from within " "services. Using it can cause issues.", - deprecated_since="Flamingo") + deprecated_since="Flamingo"), + cfg.StrOpt('prometheus_project_label_name', + default="project", + help='Name of label used to identify metric project IDs ' + 'in Prometheus. This label name will be used to ' + 'restrict queries to the appropriate project.' + ) ] @@ -40,6 +47,7 @@ class PrometheusBase(threshold.ThresholdEvaluator): def __init__(self, conf): super().__init__(conf) self._set_obsclient(conf) + self.conf = conf def _set_obsclient(self, conf): session = keystone_client.get_session(conf) @@ -49,7 +57,7 @@ class PrometheusBase(threshold.ThresholdEvaluator): def _get_metric_data(self, query): LOG.debug(f'Querying Prometheus instance on: {query}') - return self._prom.query.query(query, disable_rbac=True) + return self._prom.query.query(query) class PrometheusEvaluator(PrometheusBase): @@ -66,10 +74,19 @@ class PrometheusEvaluator(PrometheusBase): :returns: state, trending state, statistics, number of samples outside threshold and reason """ - metrics = self._get_metric_data(alarm_rule['query']) + scope_to_project = alarm_rule.get('scope_to_project', False) + query = alarm_rule['query'] + if scope_to_project: + promQLRbac = obsc_rbac.PromQLRbac( + self._prom.prometheus_client, + scope_to_project, + project_label=self.conf.prometheus_project_label_name + ) + query = promQLRbac.modify_query(query) + metrics = self._get_metric_data(query) if not metrics: LOG.warning("Empty result fetched from Prometheus for query" - f" {alarm_rule['query']}") + f" {query}") statistics = self._sanitize(metrics) if not statistics: diff --git a/aodh/opts.py b/aodh/opts.py index b7d9b59e9..97ec59777 100644 --- a/aodh/opts.py +++ b/aodh/opts.py @@ -18,6 +18,7 @@ from oslo_config import cfg import aodh.api import aodh.api.controllers.v2.alarm_rules.gnocchi +import aodh.api.controllers.v2.alarm_rules.prometheus import aodh.api.controllers.v2.alarms import aodh.coordination import aodh.evaluator diff --git a/aodh/tests/functional/api/v2/test_alarm_scenarios.py b/aodh/tests/functional/api/v2/test_alarm_scenarios.py index 79bbc49f8..5e69bb073 100644 --- a/aodh/tests/functional/api/v2/test_alarm_scenarios.py +++ b/aodh/tests/functional/api/v2/test_alarm_scenarios.py @@ -1365,6 +1365,74 @@ class TestAlarmsRuleGnocchi(TestAlarmsBase): self._verify_alarm(json, alarms[0]) +class TestAlarmsRulePrometheus(TestAlarmsBase): + + def _do_post_alarm(self, headers={}, project=None, status=201): + json = { + 'name': 'added_prometheus_alarm', + 'enabled': True, + 'type': 'prometheus', + 'ok_actions': ['http://something/ok'], + 'prometheus_rule': { + 'query': 'ceilometer_cpu', + 'comparison_operator': 'le', + 'threshold': 50, + } + } + if project: + json['project_id'] = project + + response = self.post_json( + '/alarms', params=json, headers=headers, status=status + ) + return response + + def test_post_prometheus_alarm_as_admin(self): + auth_headers = self.auth_headers + auth_headers['X-Roles'] = 'admin' + resp = self._do_post_alarm(headers=auth_headers) + + alarms = list(self.alarm_conn.get_alarms( + alarm_id=resp.json['alarm_id'])) + + self.assertEqual(1, len(alarms)) + self.assertIsNone(alarms[0].rule['scope_to_project']) + + def test_post_prometheus_alarm_as_admin_on_behalf_of_another_project(self): + auth_headers = self.auth_headers + auth_headers['X-Roles'] = 'admin' + project_id = uuidutils.generate_uuid() + resp = self._do_post_alarm( + headers=auth_headers, project=project_id + ) + + alarms = list(self.alarm_conn.get_alarms( + alarm_id=resp.json['alarm_id'])) + self.assertEqual(1, len(alarms)) + self.assertEqual(project_id, alarms[0].rule['scope_to_project']) + + def test_post_prometheus_alarm_as_nonadmin(self): + auth_headers = self.auth_headers + auth_headers['X-Roles'] = 'nonadmin' + resp = self._do_post_alarm(headers=auth_headers) + + alarms = list(self.alarm_conn.get_alarms( + alarm_id=resp.json['alarm_id'])) + self.assertEqual(1, len(alarms)) + self.assertEqual(self.project_id, alarms[0].rule['scope_to_project']) + + def test_post_prometheus_alarm_as_nonadmin_on_behalf_of_another_project( + self + ): + auth_headers = self.auth_headers + auth_headers['X-Roles'] = 'nonadmin' + resp = self._do_post_alarm( + headers=auth_headers, project='another_project_id', status=401 + ) + self.assertEqual("Not Authorized to access project another_project_id", + resp.json['error_message']['faultstring']) + + class TestAlarmsCompositeRule(TestAlarmsBase): def setUp(self): diff --git a/aodh/tests/unit/evaluator/test_prometheus.py b/aodh/tests/unit/evaluator/test_prometheus.py new file mode 100644 index 000000000..88f879130 --- /dev/null +++ b/aodh/tests/unit/evaluator/test_prometheus.py @@ -0,0 +1,142 @@ +# +# Copyright 2025 Red Hat, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import fixtures +from unittest import mock + +from oslo_utils import uuidutils + +from aodh.evaluator import prometheus +from aodh.storage import models +from aodh.tests import constants +from aodh.tests.unit.evaluator import base + + +class TestPrometheusEvaluator(base.TestEvaluatorBase): + EVALUATOR = prometheus.PrometheusEvaluator + + def setUp(self): + self.client = self.useFixture(fixtures.MockPatch( + 'aodh.evaluator.prometheus.client' + )).mock.Client.return_value + self.prepared_alarms = [ + models.Alarm(name='instance_running_hot', + description='instance_running_hot', + type='prometheus', + enabled=True, + user_id='foobar', + project_id='123', + alarm_id=uuidutils.generate_uuid(), + state='insufficient data', + state_reason='insufficient data', + state_timestamp=constants.MIN_DATETIME, + timestamp=constants.MIN_DATETIME, + insufficient_data_actions=[], + ok_actions=[], + alarm_actions=[], + repeat_actions=False, + time_constraints=[], + severity='low', + rule=dict( + comparison_operator='gt', + threshold=80.0, + evaluation_periods=5, + query='ceilometer_cpu') + ), + models.Alarm(name='group_running_idle', + description='group_running_idle', + type='prometheus', + enabled=True, + user_id='foobar', + project_id='123', + state='insufficient data', + state_reason='insufficient data', + state_timestamp=constants.MIN_DATETIME, + timestamp=constants.MIN_DATETIME, + insufficient_data_actions=[], + ok_actions=[], + alarm_actions=[], + repeat_actions=False, + alarm_id=uuidutils.generate_uuid(), + time_constraints=[], + rule=dict( + comparison_operator='le', + threshold=10.0, + evaluation_periods=4, + query='ceilometer_memory'), + ), + + ] + super().setUp() + + def prepare_alarms(self): + self.alarms = self.prepared_alarms[0:1] + + def test_project_scoping_old_alarm(self): + # Alarm created before scope_to_project was introduced + self.client.query.query.side_effect = None + with ( + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.modify_query', + ) as mock_modify_query, + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.__init__', + return_value=None) as mock_rbac_init): + self._evaluate_all_alarms() + mock_rbac_init.assert_not_called() + mock_modify_query.assert_not_called() + + def test_project_scoping_not_scoped(self): + # Alarm likely created by admin. This shouldn't get scoped + # to a single project. + self.client.query.query.side_effect = None + self.alarms[0].rule['scope_to_project'] = None + with ( + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.modify_query', + ) as mock_modify_query, + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.__init__', + return_value=None) as mock_rbac_init): + self._evaluate_all_alarms() + mock_rbac_init.assert_not_called() + mock_modify_query.assert_not_called() + + def test_project_scoping_scoped(self): + # Alarm which needs to be scoped to a single project + project_id = 123 + project_label = 'custom_label' + self.conf.set_override('prometheus_project_label_name', project_label) + self.alarms[0].rule['scope_to_project'] = project_id + self.client.query.query.side_effect = None + + with ( + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.modify_query', + return_value='') as mock_modify_query, + mock.patch('aodh.evaluator.prometheus.' + 'obsc_rbac.PromQLRbac.__init__', + return_value=None) as mock_rbac_init): + self._evaluate_all_alarms() + + mock_rbac_init.assert_called_once() + self.assertEqual( + project_id, mock_rbac_init.call_args.args[1]) + self.assertEqual( + project_label, + mock_rbac_init.call_args.kwargs['project_label']) + + mock_modify_query.assert_called_once_with( + 'ceilometer_cpu') diff --git a/releasenotes/notes/add-rbac-to-prometheus-queries-eeb2b14b2a2710a0.yaml b/releasenotes/notes/add-rbac-to-prometheus-queries-eeb2b14b2a2710a0.yaml new file mode 100644 index 000000000..10ca66f0e --- /dev/null +++ b/releasenotes/notes/add-rbac-to-prometheus-queries-eeb2b14b2a2710a0.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Prometheus queries for nonadmin users or for admin users who + specify a particular project are now being restricted to that + project or to the nonadmin user's current project. Admins can + still create alarms for resources across multiple projects without + restrictions. This works similarly to how gnocchi alarms work for + some time already. Use the [DEFAULT].prometheus_project_label_name + configuration option to configure label name used for + distinguishing between projects. diff --git a/requirements.txt b/requirements.txt index e38cb50a8..23775544e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -33,7 +33,7 @@ WSME>=0.12.1 # MIT cachetools>=1.1.6 # MIT cotyledon>=1.7.3 # Apache-2.0 keystoneauth1>=2.1 # Apache-2.0 -python-observabilityclient>=0.0.4 # Apache-2.0 +python-observabilityclient>=1.1.0 # Apache-2.0 python-octaviaclient>=1.8.0 # Apache-2.0 python-dateutil>=2.5.3 # BSD python-heatclient>=1.17.0 # Apache-2.0