From c0c2b844a7ac0edb0c7c937fbe9c8646d8a24efb Mon Sep 17 00:00:00 2001 From: Eoghan Glynn Date: Thu, 29 Aug 2013 09:42:42 +0000 Subject: [PATCH] Add query support to alarm history API Allow alarm history retrieval to be constrained by user, project, change type or timestamp. Partially implements bp alarm-audit-api Change-Id: Iba303b422a2893ed63375c79d9dc4b93711cf215 --- ceilometer/api/controllers/v2.py | 27 +++++-- ceilometer/storage/base.py | 12 ++- ceilometer/storage/impl_db2.py | 12 ++- ceilometer/storage/impl_hbase.py | 12 ++- ceilometer/storage/impl_log.py | 12 ++- ceilometer/storage/impl_mongodb.py | 24 +++++- ceilometer/storage/impl_sqlalchemy.py | 12 ++- tests/api/v2/test_alarm_scenarios.py | 109 +++++++++++++++++++++++--- 8 files changed, 193 insertions(+), 27 deletions(-) diff --git a/ceilometer/api/controllers/v2.py b/ceilometer/api/controllers/v2.py index 86790d222..4427b473d 100644 --- a/ceilometer/api/controllers/v2.py +++ b/ceilometer/api/controllers/v2.py @@ -210,7 +210,7 @@ class Query(_Base): return converted_value -def _sanitize_query(q): +def _sanitize_query(q, valid_keys): '''Check the query to see if: 1) the request is comming from admin - then allow full visibility 2) non-admin - make sure that the query includes the requester's @@ -230,7 +230,7 @@ def _sanitize_query(q): i.value) raise wsme.exc.ClientSideError(errstr) - if not proj_q: + if not proj_q and 'on_behalf_of' not in valid_keys: # The user is restricted, but they didn't specify a project # so add it for them. q.append(Query(field='project_id', @@ -239,12 +239,19 @@ def _sanitize_query(q): return q -def _query_to_kwargs(query, db_func): +def _exclude_from(keys, excluded): + if keys and excluded: + for key in excluded: + if key in keys: + keys.remove(key) + + +def _query_to_kwargs(query, db_func, internal_keys=[]): # TODO(dhellmann): This function needs tests of its own. - query = _sanitize_query(query) valid_keys = inspect.getargspec(db_func)[0] - if 'self' in valid_keys: - valid_keys.remove('self') + query = _sanitize_query(query, valid_keys) + internal_keys.append('self') + _exclude_from(valid_keys, internal_keys) translation = {'user_id': 'user', 'project_id': 'project', 'resource_id': 'resource'} @@ -297,7 +304,9 @@ def _query_to_kwargs(query, db_func): if trans: for k in trans: if k not in valid_keys: - raise wsme.exc.UnknownArgument(k, "unrecognized query field") + msg = ("unrecognized field in query: %s, valid keys: %s" % + (query, valid_keys)) + raise wsme.exc.UnknownArgument(k, msg) kwargs[k] = trans[k] return kwargs @@ -1045,8 +1054,10 @@ class AlarmController(rest.RestController): # avoid inappropriate cross-tenant visibility of alarm history auth_project = acl.get_limited_to_project(pecan.request.headers) conn = pecan.request.storage_conn + kwargs = _query_to_kwargs(q, conn.get_alarm_changes, ['on_behalf_of']) return [AlarmChange.from_db_model(ac) - for ac in conn.get_alarm_changes(self._id, auth_project)] + for ac in conn.get_alarm_changes(self._id, auth_project, + **kwargs)] class AlarmsController(rest.RestController): diff --git a/ceilometer/storage/base.py b/ceilometer/storage/base.py index e7ef7776b..8a27a9777 100644 --- a/ceilometer/storage/base.py +++ b/ceilometer/storage/base.py @@ -231,11 +231,21 @@ class Connection(object): """ @abc.abstractmethod - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ @abc.abstractmethod diff --git a/ceilometer/storage/impl_db2.py b/ceilometer/storage/impl_db2.py index 074748c3f..61142e27b 100644 --- a/ceilometer/storage/impl_db2.py +++ b/ceilometer/storage/impl_db2.py @@ -601,11 +601,21 @@ class Connection(base.Connection): """ self.db.alarm.remove({'alarm_id': alarm_id}) - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ raise NotImplementedError('Alarm history not implemented') diff --git a/ceilometer/storage/impl_hbase.py b/ceilometer/storage/impl_hbase.py index fac328c6c..75f6dc530 100644 --- a/ceilometer/storage/impl_hbase.py +++ b/ceilometer/storage/impl_hbase.py @@ -598,11 +598,21 @@ class Connection(base.Connection): """ raise NotImplementedError('Alarms not implemented') - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ raise NotImplementedError('Alarm history not implemented') diff --git a/ceilometer/storage/impl_log.py b/ceilometer/storage/impl_log.py index 5cdd1c547..d0a5826b2 100644 --- a/ceilometer/storage/impl_log.py +++ b/ceilometer/storage/impl_log.py @@ -177,11 +177,21 @@ class Connection(base.Connection): """Delete a alarm """ - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ raise NotImplementedError('Alarm history not implemented') diff --git a/ceilometer/storage/impl_mongodb.py b/ceilometer/storage/impl_mongodb.py index 6800f50fc..aa17006e0 100644 --- a/ceilometer/storage/impl_mongodb.py +++ b/ceilometer/storage/impl_mongodb.py @@ -834,15 +834,37 @@ class Connection(base.Connection): """ self.db.alarm.remove({'alarm_id': alarm_id}) - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ q = dict(alarm_id=alarm_id) if on_behalf_of is not None: q['on_behalf_of'] = on_behalf_of + if user is not None: + q['user_id'] = user + if project is not None: + q['project_id'] = project + if type is not None: + q['type'] = type + if start_timestamp or end_timestamp: + ts_range = make_timestamp_range(start_timestamp, end_timestamp, + start_timestamp_op, + end_timestamp_op) + if ts_range: + q['timestamp'] = ts_range sort = [("timestamp", pymongo.DESCENDING)] for alarm_change in self.db.alarm_history.find(q, sort=sort): diff --git a/ceilometer/storage/impl_sqlalchemy.py b/ceilometer/storage/impl_sqlalchemy.py index fef47def4..9998aa941 100644 --- a/ceilometer/storage/impl_sqlalchemy.py +++ b/ceilometer/storage/impl_sqlalchemy.py @@ -628,11 +628,21 @@ class Connection(base.Connection): session.query(Alarm).filter(Alarm.id == alarm_id).delete() session.flush() - def get_alarm_changes(self, alarm_id, on_behalf_of): + def get_alarm_changes(self, alarm_id, on_behalf_of, + user=None, project=None, type=None, + start_timestamp=None, start_timestamp_op=None, + end_timestamp=None, end_timestamp_op=None): """Yields list of AlarmChanges describing alarm history :param alarm_id: ID of alarm to return changes for :param on_behalf_of: ID of tenant to scope changes query (None for administrative user, indicating all projects) + :param user: Optional ID of user to return changes for + :param project: Optional ID of project to return changes for + :project type: Optional change type + :param start_timestamp: Optional modified timestamp start range + :param start_timestamp_op: Optional timestamp start range operation + :param end_timestamp: Optional modified timestamp end range + :param end_timestamp_op: Optional timestamp end range operation """ raise NotImplementedError('Alarm history not implemented') diff --git a/tests/api/v2/test_alarm_scenarios.py b/tests/api/v2/test_alarm_scenarios.py index d5501a352..0a9160fe4 100644 --- a/tests/api/v2/test_alarm_scenarios.py +++ b/tests/api/v2/test_alarm_scenarios.py @@ -189,9 +189,17 @@ class TestAlarms(FunctionalTest, self.assertEqual(1, len(match), 'alarm %s not found' % id) return match[0] - def _get_alarm_history(self, alarm, auth_headers=None): - return self.get_json('/alarms/%s/history' % alarm['alarm_id'], - headers=auth_headers or self.auth_headers) + def _get_alarm_history(self, alarm, auth_headers=None, query=None, + expect_errors=False, status=200): + url = '/alarms/%s/history' % alarm['alarm_id'] + if query: + url += '?q.op=%(op)s&q.value=%(value)s&q.field=%(field)s' % query + resp = self.get_json(url, + headers=auth_headers or self.auth_headers, + expect_errors=expect_errors) + if expect_errors: + self.assertEqual(resp.status_code, status) + return resp def _update_alarm(self, alarm, data, auth_headers=None): self.put_json('/alarms/%s' % alarm['alarm_id'], @@ -277,16 +285,58 @@ class TestAlarms(FunctionalTest, 'state transition', detail) - def test_get_recorded_alarm_history_rule_change_on_behalf_of(self): - data = dict(name='renamed') - detail = '{"name": "renamed"}' - auth = {'X-Roles': 'admin', - 'X-User-Id': str(uuid.uuid4()), - 'X-Project-Id': str(uuid.uuid4())} - self._do_test_get_recorded_alarm_history_on_update(data, - 'rule change', - detail, - auth) + def test_get_recorded_alarm_history_state_transition_on_behalf_of(self): + # credentials for new non-admin user, on who's behalf the alarm + # is created + member_user = str(uuid.uuid4()) + member_project = str(uuid.uuid4()) + member_auth = {'X-Roles': 'member', + 'X-User-Id': member_user, + 'X-Project-Id': member_project} + new_alarm = dict(name='new_alarm', + meter_name='other_meter', + comparison_operator='le', + threshold=42.0, + statistic='max') + self.post_json('/alarms', params=new_alarm, status=200, + headers=member_auth) + alarm = self.get_json('/alarms', headers=member_auth)[0] + + # effect a state transition as a new administrative user + admin_user = str(uuid.uuid4()) + admin_project = str(uuid.uuid4()) + admin_auth = {'X-Roles': 'admin', + 'X-User-Id': admin_user, + 'X-Project-Id': admin_project} + data = dict(state='alarm') + self._update_alarm(alarm, data, auth_headers=admin_auth) + + # ensure that both the creation event and state transition + # are visible to the non-admin alarm owner and admin user alike + for auth in [member_auth, admin_auth]: + history = self._get_alarm_history(alarm, auth_headers=auth) + self.assertEqual(2, len(history)) + self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], + detail='{"state": "alarm"}', + on_behalf_of=alarm['project_id'], + project_id=admin_project, + type='state transition', + user_id=admin_user), + history[0]) + self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], + on_behalf_of=alarm['project_id'], + project_id=member_project, + type='creation', + user_id=member_user), + history[1]) + self._assert_in_json(new_alarm, history[1]['detail']) + + # ensure on_behalf_of cannot be constrained in an API call + query = dict(field='on_behalf_of', + op='eq', + value=alarm['project_id']) + self._get_alarm_history(alarm, auth_headers=auth, query=query, + expect_errors=True, status=400) def test_get_recorded_alarm_history_segregation(self): data = dict(name='renamed') @@ -321,6 +371,39 @@ class TestAlarms(FunctionalTest, history[0]) self._assert_in_json(alarm, history[0]['detail']) + def test_get_constrained_alarm_history(self): + alarm = self._get_alarm('a') + self._update_alarm(alarm, dict(name='renamed')) + now = datetime.datetime.utcnow().isoformat() + query = dict(field='timestamp', op='gt', value=now) + history = self._get_alarm_history(alarm, query=query) + self.assertEqual(0, len(history)) + query['op'] = 'le' + history = self._get_alarm_history(alarm, query=query) + self.assertEqual(1, len(history)) + detail = '{"name": "renamed"}' + self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], + detail=detail, + on_behalf_of=alarm['project_id'], + project_id=alarm['project_id'], + type='rule change', + user_id=alarm['user_id']), + history[0]) + alarm = self._get_alarm('a') + self.delete('/alarms/%s' % alarm['alarm_id'], + headers=self.auth_headers, + status=200) + query = dict(field='type', op='eq', value='deletion') + history = self._get_alarm_history(alarm, query=query) + self.assertEqual(1, len(history)) + self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], + on_behalf_of=alarm['project_id'], + project_id=alarm['project_id'], + type='deletion', + user_id=alarm['user_id']), + history[0]) + self._assert_in_json(alarm, history[0]['detail']) + def test_get_nonexistent_alarm_history(self): # the existence of alarm history is independent of the # continued existence of the alarm itself