From 23ddf20d7005af4e834f61698cbe32cbc99bb997 Mon Sep 17 00:00:00 2001 From: Jaromir Wysoglad Date: Wed, 30 Jul 2025 02:34:05 -0400 Subject: [PATCH] Rbac restrictions to prometheus alarms For nonadmin users, restrict prometheus alarms to the current project. For admins, restrict the alarms to the specified project (when explicitly specified) or leave them unrestricted. This is very similar to what we already do with Gnocchi alarms in https://opendev.org/openstack/aodh/src/branch/master/aodh/api/controllers/v2/alarm_rules/gnocchi.py#L181 Change-Id: I6f1b0a1bc67376dca3e00585f04bec697781113c Signed-off-by: Jaromir Wysoglad --- .../controllers/v2/alarm_rules/prometheus.py | 16 +- aodh/evaluator/prometheus.py | 25 ++- aodh/opts.py | 1 + .../functional/api/v2/test_alarm_scenarios.py | 68 +++++++++ aodh/tests/unit/evaluator/test_prometheus.py | 142 ++++++++++++++++++ ...o-prometheus-queries-eeb2b14b2a2710a0.yaml | 11 ++ requirements.txt | 2 +- 7 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 aodh/tests/unit/evaluator/test_prometheus.py create mode 100644 releasenotes/notes/add-rbac-to-prometheus-queries-eeb2b14b2a2710a0.yaml 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