From 819c0b5d4e7f741461d859129f4098d285fc8d36 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Thu, 17 Mar 2016 10:26:51 +0100 Subject: [PATCH] api: stop relying on side-effect of _alarm() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, self.conn is set in _alarm, meaning that the global attribute is copied to the instance and then used in random methods. This works until you stop calling the RBAC _alarm() method… This is useless, naive and very fragile. Fix that. Change-Id: I407503f1d0758f32d95ad17108daadeb03000feb --- .../controllers/v2/alarm_rules/combination.py | 2 +- aodh/api/controllers/v2/alarms.py | 26 +++++++++---------- aodh/api/controllers/v2/capabilities.py | 2 +- aodh/api/controllers/v2/query.py | 4 +-- aodh/api/hooks.py | 4 +-- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/aodh/api/controllers/v2/alarm_rules/combination.py b/aodh/api/controllers/v2/alarm_rules/combination.py index e72be43dc..a128e5070 100644 --- a/aodh/api/controllers/v2/alarm_rules/combination.py +++ b/aodh/api/controllers/v2/alarm_rules/combination.py @@ -55,7 +55,7 @@ class AlarmCombinationRule(base.AlarmRule): project = v2_utils.get_auth_project( alarm.project_id if alarm.project_id != wtypes.Unset else None) for id in alarm.combination_rule.alarm_ids: - alarms = list(pecan.request.alarm_storage_conn.get_alarms( + alarms = list(pecan.request.storage.get_alarms( alarm_id=id, project=project)) if not alarms: raise base.AlarmNotFound(id, project) diff --git a/aodh/api/controllers/v2/alarms.py b/aodh/api/controllers/v2/alarms.py index 63276259e..da48c690b 100644 --- a/aodh/api/controllers/v2/alarms.py +++ b/aodh/api/controllers/v2/alarms.py @@ -511,14 +511,12 @@ class AlarmController(rest.RestController): self._id = alarm_id def _alarm(self, rbac_directive): - self.conn = pecan.request.alarm_storage_conn - # TODO(sileht): We should be able to relax this since we # pass the alarm object to the enforcer. auth_project = rbac.get_limited_to_project(pecan.request.headers, pecan.request.enforcer) - alarms = list(self.conn.get_alarms(alarm_id=self._id, - project=auth_project)) + alarms = list(pecan.request.storage.get_alarms(alarm_id=self._id, + project=auth_project)) if not alarms: raise base.AlarmNotFound(alarm=self._id, auth_project=auth_project) alarm = alarms[0] @@ -551,7 +549,7 @@ class AlarmController(rest.RestController): severity=severity) try: - self.conn.record_alarm_change(payload) + pecan.request.storage.record_alarm_change(payload) except aodh.NotImplementedError: pass @@ -596,8 +594,8 @@ class AlarmController(rest.RestController): # make sure alarms are unique by name per project. if alarm_in.name != data.name: - alarms = list(self.conn.get_alarms(name=data.name, - project=data.project_id)) + alarms = list(pecan.request.storage.get_alarms( + name=data.name, project=data.project_id)) if alarms: raise base.ClientSideError( _("Alarm with name=%s exists") % data.name, @@ -615,7 +613,7 @@ class AlarmController(rest.RestController): LOG.exception(_("Error while putting alarm: %s") % updated_alarm) raise base.ClientSideError(_("Alarm incorrect")) - alarm = self.conn.update_alarm(alarm_in) + alarm = pecan.request.storage.update_alarm(alarm_in) change = dict((k, v) for k, v in updated_alarm.items() if v != old_alarm[k] and k not in @@ -629,7 +627,7 @@ class AlarmController(rest.RestController): # ensure alarm exists before deleting alarm = self._alarm('delete_alarm') - self.conn.delete_alarm(alarm.alarm_id) + pecan.request.storage.delete_alarm(alarm.alarm_id) alarm_object = Alarm.from_db_model(alarm) alarm_object.delete_actions() @@ -649,7 +647,7 @@ class AlarmController(rest.RestController): # avoid inappropriate cross-tenant visibility of alarm history auth_project = rbac.get_limited_to_project(pecan.request.headers, pecan.request.enforcer) - conn = pecan.request.alarm_storage_conn + conn = pecan.request.storage kwargs = v2_utils.query_to_kwargs( q, conn.get_alarm_changes, ['on_behalf_of', 'alarm_id']) return [AlarmChange.from_db_model(ac) @@ -673,7 +671,7 @@ class AlarmController(rest.RestController): now = timeutils.utcnow() alarm.state = state alarm.state_timestamp = now - alarm = self.conn.update_alarm(alarm) + alarm = pecan.request.storage.update_alarm(alarm) change = {'state': alarm.state} self._record_change(change, now, on_behalf_of=alarm.project_id, type=models.AlarmChange.STATE_TRANSITION) @@ -730,7 +728,7 @@ class AlarmsController(rest.RestController): rbac.enforce('create_alarm', pecan.request.headers, pecan.request.enforcer, {}) - conn = pecan.request.alarm_storage_conn + conn = pecan.request.storage now = timeutils.utcnow() data.alarm_id = str(uuid.uuid4()) @@ -798,7 +796,7 @@ class AlarmsController(rest.RestController): q = q or [] # Timestamp is not supported field for Simple Alarm queries kwargs = v2_utils.query_to_kwargs( - q, pecan.request.alarm_storage_conn.get_alarms, + q, pecan.request.storage.get_alarms, allow_timestamps=False) return [Alarm.from_db_model(m) - for m in pecan.request.alarm_storage_conn.get_alarms(**kwargs)] + for m in pecan.request.storage.get_alarms(**kwargs)] diff --git a/aodh/api/controllers/v2/capabilities.py b/aodh/api/controllers/v2/capabilities.py index 60cd3b0b2..d6a192231 100644 --- a/aodh/api/controllers/v2/capabilities.py +++ b/aodh/api/controllers/v2/capabilities.py @@ -101,7 +101,7 @@ class CapabilitiesController(rest.RestController): """ # variation in API capabilities is effectively determined by # the lack of strict feature parity across storage drivers - alarm_conn = pecan.request.alarm_storage_conn + alarm_conn = pecan.request.storage driver_capabilities = { 'alarms': alarm_conn.get_capabilities()['alarms'], } diff --git a/aodh/api/controllers/v2/query.py b/aodh/api/controllers/v2/query.py index 958f86af1..b37cc009b 100644 --- a/aodh/api/controllers/v2/query.py +++ b/aodh/api/controllers/v2/query.py @@ -357,7 +357,7 @@ class QueryAlarmHistoryController(rest.RestController): query = ValidatedComplexQuery(body, models.AlarmChange) query.validate(visibility_field="on_behalf_of") - conn = pecan.request.alarm_storage_conn + conn = pecan.request.storage return [alarms.AlarmChange.from_db_model(s) for s in conn.query_alarm_history(query.filter_expr, query.orderby, @@ -383,7 +383,7 @@ class QueryAlarmsController(rest.RestController): query = ValidatedComplexQuery(body, models.Alarm) query.validate(visibility_field="project_id") - conn = pecan.request.alarm_storage_conn + conn = pecan.request.storage return [alarms.Alarm.from_db_model(s) for s in conn.query_alarms(query.filter_expr, query.orderby, diff --git a/aodh/api/hooks.py b/aodh/api/hooks.py index dbae594ab..3d3e26d2c 100644 --- a/aodh/api/hooks.py +++ b/aodh/api/hooks.py @@ -35,10 +35,10 @@ class ConfigHook(hooks.PecanHook): class DBHook(hooks.PecanHook): def __init__(self, alarm_conn): - self.alarm_storage_connection = alarm_conn + self.storage = alarm_conn def before(self, state): - state.request.alarm_storage_conn = self.alarm_storage_connection + state.request.storage = self.storage class TranslationHook(hooks.PecanHook):