From f4559a91a47ddbcaa068431eee09d980b5c77236 Mon Sep 17 00:00:00 2001 From: Romain Ziba Date: Thu, 11 Jun 2015 16:06:09 +0200 Subject: [PATCH] Cerberus creates an uuid for security reports Previously, Cerberus retrieved reports according to the report identifier the plugin provided. The goal of this US is for Cerberus to create its own uuid and keep the report identifier of the plugin. The changes include: * Migration of the db * an unique attribute UUID * Tuple (report_id, plugin_id) must be unique * Changes of unit & functional tests Change-Id: I9aedb88fd5a4f66ccd5e2e831790d9655f76bb09 --- .../api/v1/controllers/security_reports.py | 12 +++--- cerberus/api/v1/datamodels/security_report.py | 12 +++--- cerberus/common/exception.py | 3 +- cerberus/db/api.py | 12 +++--- cerberus/db/sqlalchemy/api.py | 39 ++++++++++--------- .../migrate_repo/versions/002_initial.py | 30 ++++++++++++++ cerberus/db/sqlalchemy/models.py | 17 ++++---- cerberus/manager.py | 6 +-- cerberus/tests/functional/api/v1/test_api.py | 20 ++++++++-- .../unit/api/v1/test_security_reports.py | 10 ++--- cerberus/tests/unit/db/utils.py | 4 +- 11 files changed, 105 insertions(+), 60 deletions(-) create mode 100644 cerberus/db/sqlalchemy/migrate_repo/versions/002_initial.py diff --git a/cerberus/api/v1/controllers/security_reports.py b/cerberus/api/v1/controllers/security_reports.py index ebee7aa..e12cede 100644 --- a/cerberus/api/v1/controllers/security_reports.py +++ b/cerberus/api/v1/controllers/security_reports.py @@ -82,18 +82,18 @@ class SecurityReportController(base.BaseController): 'tickets': ['PUT'] } - def __init__(self, report_id): + def __init__(self, uuid): super(SecurityReportController, self).__init__() - pecan.request.context['report_id'] = report_id - self._id = report_id + pecan.request.context['uuid'] = uuid + self._id = uuid - def get_security_report(self, report_id): + def get_security_report(self, uuid): try: - security_report = db.security_report_get(report_id) + security_report = db.security_report_get(uuid) except Exception as e: LOG.exception(e) raise errors.DbError( - "Security report %s could not be retrieved" % report_id + "Security report %s could not be retrieved" % uuid ) return security_report diff --git a/cerberus/api/v1/datamodels/security_report.py b/cerberus/api/v1/datamodels/security_report.py index 3027cee..f07e666 100644 --- a/cerberus/api/v1/datamodels/security_report.py +++ b/cerberus/api/v1/datamodels/security_report.py @@ -15,7 +15,7 @@ # import datetime -import decimal +import uuid from cerberus.api.v1.datamodels import base from wsme import types as wtypes @@ -25,14 +25,14 @@ class SecurityReportResource(base.Base): """ Representation of a security report. """ - id = wtypes.IntegerType() + uuid = wtypes.wsattr(wtypes.text) """Security report id.""" plugin_id = wtypes.wsattr(wtypes.text) """Associated plugin id.""" report_id = wtypes.wsattr(wtypes.text) - """Associated report id.""" + """Associated report id provided by plugin.""" component_id = wtypes.wsattr(wtypes.text) """Associated component id.""" @@ -56,7 +56,7 @@ class SecurityReportResource(base.Base): """Security rating.""" vulnerabilities = wtypes.wsattr(wtypes.text) - """Associated report id.""" + """Vulnerabilities.""" vulnerabilities_number = wtypes.IntegerType() """Total of Vulnerabilities.""" @@ -69,7 +69,7 @@ class SecurityReportResource(base.Base): def as_dict(self): return self.as_dict_from_keys( - ['id', 'plugin_id', 'report_id', 'component_id', + ['uuid', 'plugin_id', 'report_id', 'component_id', 'component_type', 'component_name', 'project_id', 'title', 'description', 'security_rating', 'vulnerabilities', 'vulnerabilities_number', @@ -85,7 +85,7 @@ class SecurityReportResource(base.Base): @classmethod def sample(cls): sample = cls(initial_data={ - 'id': decimal.Decimal(1), + 'uuid': str(uuid.uuid4()), 'security_rating': float(7.4), 'component_name': 'openstack-server', 'component_id': 'a1d869a1-6ab0-4f02-9e56-f83034bacfcb', diff --git a/cerberus/common/exception.py b/cerberus/common/exception.py index 716ccb8..8212312 100644 --- a/cerberus/common/exception.py +++ b/cerberus/common/exception.py @@ -147,7 +147,8 @@ class DBException(CerberusException): class ReportExists(DBException): - msg_fmt = _("Report %(report_id)s already exists.") + msg_fmt = _("Report %(report_id)s already exists for plugin " + "%(plugin_id)s.") class PluginInfoExists(DBException): diff --git a/cerberus/db/api.py b/cerberus/db/api.py index aa68e59..2ffc452 100644 --- a/cerberus/db/api.py +++ b/cerberus/db/api.py @@ -56,14 +56,14 @@ def security_report_create(values): return IMPL.security_report_create(values) -def security_report_update_last_report_date(id, date): +def security_report_update_last_report_date(uuid, date): """Create an instance from the values dictionary.""" - return IMPL.security_report_update_last_report_date(id, date) + return IMPL.security_report_update_last_report_date(uuid, date) -def security_report_update_ticket_id(id, ticket_id): +def security_report_update_ticket_id(uuid, ticket_id): """Create an instance from the values dictionary.""" - return IMPL.security_report_update_ticket_id(id, ticket_id) + return IMPL.security_report_update_ticket_id(uuid, ticket_id) def security_report_get_all(project_id=None): @@ -71,9 +71,9 @@ def security_report_get_all(project_id=None): return IMPL.security_report_get_all(project_id=project_id) -def security_report_get(id): +def security_report_get(uuid): """Get security report from its id in database""" - return IMPL.security_report_get(id) + return IMPL.security_report_get(uuid) def security_report_get_from_report_id(report_id): diff --git a/cerberus/db/sqlalchemy/api.py b/cerberus/db/sqlalchemy/api.py index 50203b4..a6bb143 100644 --- a/cerberus/db/sqlalchemy/api.py +++ b/cerberus/db/sqlalchemy/api.py @@ -90,7 +90,8 @@ def _security_report_create(values): security_report_ref.save() except db_exc.DBDuplicateEntry as e: LOG.exception(e) - raise exception.ReportExists(report_id=values['report_id']) + raise exception.ReportExists(report_id=values['report_id'], + plugin_id=values['plugin_id']) except Exception as e: LOG.exception(e) raise exception.DBException() @@ -101,12 +102,12 @@ def security_report_create(values): return _security_report_create(values) -def _security_report_update_last_report_date(report_id, date): +def _security_report_update_last_report_date(uuid, date): try: session = get_session() report = model_query(models.SecurityReport, read_deleted="no", - session=session).filter(models.SecurityReport.id - == report_id).first() + session=session).filter(models.SecurityReport.uuid + == uuid).first() report.last_report_date = date report.save(session) except Exception as e: @@ -114,16 +115,16 @@ def _security_report_update_last_report_date(report_id, date): raise exception.DBException() -def security_report_update_last_report_date(report_id, date): - _security_report_update_last_report_date(report_id, date) +def security_report_update_last_report_date(uuid, date): + _security_report_update_last_report_date(uuid, date) -def _security_report_update_ticket_id(report_id, ticket_id): +def _security_report_update_ticket_id(uuid, ticket_id): try: session = get_session() report = model_query(models.SecurityReport, read_deleted="no", - session=session).filter(models.SecurityReport.id - == report_id).first() + session=session).filter(models.SecurityReport.uuid + == uuid).first() report.ticket_id = ticket_id report.save(session) except Exception as e: @@ -131,8 +132,8 @@ def _security_report_update_ticket_id(report_id, ticket_id): raise exception.DBException() -def security_report_update_ticket_id(report_id, ticket_id): - _security_report_update_ticket_id(report_id, ticket_id) +def security_report_update_ticket_id(uuid, ticket_id): + _security_report_update_ticket_id(uuid, ticket_id) def _security_report_get_all(project_id=None): @@ -154,19 +155,19 @@ def security_report_get_all(project_id=None): return _security_report_get_all(project_id=project_id) -def _security_report_get(report_id): +def _security_report_get(uuid): try: session = get_session() return model_query( models.SecurityReport, read_deleted="no", session=session).filter( - models.SecurityReport.report_id == report_id).first() + models.SecurityReport.uuid == uuid).first() except Exception as e: LOG.exception(e) raise exception.DBException() -def security_report_get(id): - return _security_report_get(id) +def security_report_get(uuid): + return _security_report_get(uuid) def _security_report_get_from_report_id(report_id): @@ -185,19 +186,19 @@ def security_report_get_from_report_id(report_id): return _security_report_get_from_report_id(report_id) -def _security_report_delete(report_id): +def _security_report_delete(uuid): try: session = get_session() report = model_query(models.SecurityReport, read_deleted="no", - session=session).filter_by(report_id=report_id) + session=session).filter_by(uuid=uuid) report.delete() except Exception as e: LOG.exception(e) raise exception.DBException() -def security_report_delete(report_id): - return _security_report_delete(report_id) +def security_report_delete(uuid): + return _security_report_delete(uuid) def _plugin_info_create(values): diff --git a/cerberus/db/sqlalchemy/migrate_repo/versions/002_initial.py b/cerberus/db/sqlalchemy/migrate_repo/versions/002_initial.py new file mode 100644 index 0000000..657607a --- /dev/null +++ b/cerberus/db/sqlalchemy/migrate_repo/versions/002_initial.py @@ -0,0 +1,30 @@ +# +# 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. + + +def upgrade(migrate_engine): + + if migrate_engine.name == "mysql": + security_report = 'security_report' + sql = "ALTER TABLE %s ADD uuid VARCHAR(255)" \ + " NOT NULL AFTER id," \ + "ADD UNIQUE(uuid);" % security_report + sql += "ALTER TABLE %s DROP INDEX report_id;" % security_report + sql += "ALTER TABLE %s ADD UNIQUE (report_id, plugin_id);" \ + % security_report + migrate_engine.execute(sql) + + +def downgrade(migrate_engine): + raise NotImplementedError('Database downgrade not supported - ' + 'would drop all tables') diff --git a/cerberus/db/sqlalchemy/models.py b/cerberus/db/sqlalchemy/models.py index 64925b2..ecf1dd5 100644 --- a/cerberus/db/sqlalchemy/models.py +++ b/cerberus/db/sqlalchemy/models.py @@ -80,8 +80,9 @@ class SecurityReport(BASE, CerberusBase): __table_args__ = () id = Column(Integer, primary_key=True) + uuid = Column(String(255), unique=True) plugin_id = Column(String(255)) - report_id = Column(String(255), unique=True) + report_id = Column(String(255)) component_id = Column(String(255)) component_type = Column(String(255)) component_name = Column(String(255)) @@ -98,13 +99,13 @@ class SecurityReport(BASE, CerberusBase): class SecurityReportJsonSerializer(serialize.JsonSerializer): """Security report serializer""" - __attributes__ = ['id', 'title', 'description', 'plugin_id', 'report_id', - 'component_id', 'component_type', 'component_name', - 'project_id', 'security_rating', 'vulnerabilities', - 'vulnerabilities_number', 'last_report_date', - 'ticket_id', 'deleted', 'created_at', 'deleted_at', - 'updated_at'] - __required__ = ['id', 'title', 'component_id'] + __attributes__ = ['id', 'uuid', 'title', 'description', 'plugin_id', + 'report_id', 'component_id', 'component_type', + 'component_name', 'project_id', 'security_rating', + 'vulnerabilities', 'vulnerabilities_number', + 'last_report_date', 'ticket_id', 'deleted', 'created_at', + 'deleted_at', 'updated_at'] + __required__ = ['uuid', 'title', 'component_id'] __attribute_serializer__ = dict(created_at='date', deleted_at='date', acknowledged_at='date') __object_class__ = SecurityReport diff --git a/cerberus/manager.py b/cerberus/manager.py index c2b5370..4929885 100644 --- a/cerberus/manager.py +++ b/cerberus/manager.py @@ -51,8 +51,10 @@ def store_report_and_notify(title, plugin_id, report_id, component_id, component_name, component_type, project_id, description, security_rating, vulnerabilities, vulnerabilities_number, last_report_date): + report_uuid = uuid.uuid4() report = {'title': title, 'plugin_id': plugin_id, + 'uuid': str(report_uuid), 'report_id': report_id, 'component_id': component_id, 'component_type': component_type, @@ -64,10 +66,8 @@ def store_report_and_notify(title, plugin_id, report_id, component_id, 'vulnerabilities_number': vulnerabilities_number} try: db_api.security_report_create(report) - db_report_id = db_api.security_report_get_from_report_id( - report_id).id db_api.security_report_update_last_report_date( - db_report_id, last_report_date) + report_uuid, last_report_date) notifications.send_notification('store', 'security_report', report) except cerberus_exception.DBException: raise diff --git a/cerberus/tests/functional/api/v1/test_api.py b/cerberus/tests/functional/api/v1/test_api.py index 8001cdb..e3da68d 100644 --- a/cerberus/tests/functional/api/v1/test_api.py +++ b/cerberus/tests/functional/api/v1/test_api.py @@ -18,6 +18,8 @@ import json from cerberus.tests.functional import base +TEST_REPORT_ID = 'test_plugin_report_id' + class AlarmTestsV1(base.TestCase): @@ -67,10 +69,20 @@ class ReportTestsV1(base.TestCase): headers=headers) self.assertEqual(200, resp.status) - # Check if security report has been stored in db and delete it - report_id = 'test_plugin_report_id' + # Get uuid of the security report resp, body = self.security_client.get( - self.security_client._version + '/security_reports/' + report_id) + self.security_client._version + '/security_reports/') + + report_uuid = '' + security_reports = json.loads(body).get('security_reports', []) + for security_report in security_reports: + if security_report['report_id'] == TEST_REPORT_ID: + report_uuid = security_report['uuid'] + + # Check if security report has been stored in db and delete it + + resp, body = self.security_client.get( + self.security_client._version + '/security_reports/' + report_uuid) report = json.loads(body) self.assertEqual('a1d869a1-6ab0-4f02-9e56-f83034bacfcb', report['component_id']) @@ -78,7 +90,7 @@ class ReportTestsV1(base.TestCase): # Delete security report resp, body = self.security_client.delete( - self.security_client._version + '/security_reports/' + report_id) + self.security_client._version + '/security_reports/' + report_uuid) self.assertEqual(204, resp.status) diff --git a/cerberus/tests/unit/api/v1/test_security_reports.py b/cerberus/tests/unit/api/v1/test_security_reports.py index f2c4278..4fabd63 100644 --- a/cerberus/tests/unit/api/v1/test_security_reports.py +++ b/cerberus/tests/unit/api/v1/test_security_reports.py @@ -32,27 +32,27 @@ class TestSecurityReports(base.TestApiBase): def setUp(self): super(TestSecurityReports, self).setUp() self.fake_security_report = db_utils.get_test_security_report( - id=SECURITY_REPORT_ID + uuid=SECURITY_REPORT_ID ) self.fake_security_reports = [] self.fake_security_reports.append(self.fake_security_report) self.fake_security_reports.append(db_utils.get_test_security_report( - id=SECURITY_REPORT_ID_2 + uuid=SECURITY_REPORT_ID_2 )) self.fake_security_report_model = db_utils.get_security_report_model( - id=SECURITY_REPORT_ID + uuid=SECURITY_REPORT_ID ) self.fake_security_reports_model = [] self.fake_security_reports_model.append( self.fake_security_report_model) self.fake_security_reports_model.append( db_utils.get_security_report_model( - id=SECURITY_REPORT_ID_2 + uuid=SECURITY_REPORT_ID_2 ) ) self.security_reports_path = '/security_reports' self.security_report_path = '/security_reports/%s' \ - % self.fake_security_report['report_id'] + % self.fake_security_report['uuid'] def test_get(self): diff --git a/cerberus/tests/unit/db/utils.py b/cerberus/tests/unit/db/utils.py index bad2fba..571c0a4 100644 --- a/cerberus/tests/unit/db/utils.py +++ b/cerberus/tests/unit/db/utils.py @@ -26,7 +26,7 @@ def fake_function(): def get_test_security_report(**kwargs): return { - 'id': kwargs.get('id', 1), + 'uuid': kwargs.get('uuid', 1), 'plugin_id': kwargs.get('plugin_id', '228df8e8-d5f4-4eb9-a547-dfc649dd1017'), 'report_id': kwargs.get('report_id', '1234'), @@ -49,7 +49,7 @@ def get_test_security_report(**kwargs): def get_security_report_model(**kwargs): security_report = models.SecurityReport() - security_report.id = kwargs.get('id', 1) + security_report.uuid = kwargs.get('uuid', 1) security_report.plugin_id = kwargs.get( 'plugin_id', '228df8e8-d5f4-4eb9-a547-dfc649dd1017'