From f07b8b4da77022d963879f9381a6e0e5e9535b15 Mon Sep 17 00:00:00 2001 From: Andrei Paplauski Date: Apr 22 2020 15:17:56 +0000 Subject: Populate some fields for manual rebuilds Populate requester, requester_metadata and requested_rebuilds fields when consumers create new events in situation when they can't find them in db. Works for sync and async rebuilds RESOLVE: CLOUDWF-507 --- diff --git a/freshmaker/events.py b/freshmaker/events.py index 31ebbc0..c776b8a 100644 --- a/freshmaker/events.py +++ b/freshmaker/events.py @@ -314,7 +314,8 @@ class ManualRebuildWithAdvisoryEvent(ErrataAdvisoryRPMsSignedEvent): """ def __init__(self, msg_id, advisory, container_images, - requester_metadata_json=None, freshmaker_event_id=None, **kwargs): + requester_metadata_json=None, freshmaker_event_id=None, + requester=None, **kwargs): """ Creates new ManualRebuildWithAdvisoryEvent. @@ -322,14 +323,17 @@ class ManualRebuildWithAdvisoryEvent(ErrataAdvisoryRPMsSignedEvent): :param ErrataAdvisory advisory: Errata advisory associated with event. :param list container_images: List of NVRs of images to rebuild or empty list to rebuild all images affected by the advisory. + :param requester_metadata_json: JSON of additional information about rebuild :param freshmaker_event_id: Freshmaker event id on which this manual rebuild is based off on. + :param requester: name of requester of rebuild """ super(ManualRebuildWithAdvisoryEvent, self).__init__( msg_id, advisory, **kwargs) self.container_images = container_images self.requester_metadata_json = requester_metadata_json self.freshmaker_event_id = freshmaker_event_id + self.requester = requester class BrewSignRPMEvent(BaseEvent): @@ -416,7 +420,8 @@ class FreshmakerAsyncManualBuildEvent(BaseEvent): """Event triggered via API endpoint /async-builds""" def __init__(self, msg_id, dist_git_branch, container_images, - freshmaker_event_id=None, brew_target=None, dry_run=False): + freshmaker_event_id=None, brew_target=None, dry_run=False, + requester=None, requester_metadata_json=None): """Initialize this event :param str msg_id: the message id. @@ -434,6 +439,9 @@ class FreshmakerAsyncManualBuildEvent(BaseEvent): :param brew_target: the Brew target for the build. If not set, the previous ``buildContainer`` task build target will be used. :type brew_target: str or None + :param dry_run: True if the event should be handled in DRY_RUN mode. + :param requester: name of requester of rebuild + :param requester_metadata_json: JSON of additional information about rebuild """ super(FreshmakerAsyncManualBuildEvent, self).__init__( msg_id, manual=True, dry_run=dry_run) @@ -441,3 +449,5 @@ class FreshmakerAsyncManualBuildEvent(BaseEvent): self.container_images = container_images self.freshmaker_event_id = freshmaker_event_id self.brew_target = brew_target + self.requester = requester + self.requester_metadata_json = requester_metadata_json diff --git a/freshmaker/handlers/bob/rebuild_images_on_image_advisory_change.py b/freshmaker/handlers/bob/rebuild_images_on_image_advisory_change.py index 59d4cc2..89be665 100644 --- a/freshmaker/handlers/bob/rebuild_images_on_image_advisory_change.py +++ b/freshmaker/handlers/bob/rebuild_images_on_image_advisory_change.py @@ -27,8 +27,8 @@ from freshmaker import conf, db from freshmaker.models import Event from freshmaker.errata import Errata from freshmaker.pulp import Pulp -from freshmaker.events import (ErrataAdvisoryStateChangedEvent, - ManualRebuildWithAdvisoryEvent) +from freshmaker.events import ( + ErrataAdvisoryStateChangedEvent, ManualRebuildWithAdvisoryEvent) from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exception from freshmaker.types import EventState, ArtifactType, ArtifactBuildState @@ -53,6 +53,7 @@ class RebuildImagesOnImageAdvisoryChange(ContainerBuildHandler): self.force_dry_run() db_event = Event.get_or_create_from_event(db.session, event) + self.set_context(db_event) # Check if we are allowed to build this advisory. diff --git a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py index 53440e6..17324a4 100644 --- a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py +++ b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py @@ -27,8 +27,8 @@ import koji import kobo from freshmaker import conf, db -from freshmaker.events import ErrataAdvisoryRPMsSignedEvent -from freshmaker.events import ManualRebuildWithAdvisoryEvent +from freshmaker.events import ( + ErrataAdvisoryRPMsSignedEvent, ManualRebuildWithAdvisoryEvent) from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exception from freshmaker.lightblue import LightBlue from freshmaker.pulp import Pulp @@ -72,9 +72,8 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): # Generate the Database representation of `event`, it can be # triggered by user, we want to track what happened - db_event = Event.get_or_create( - db.session, event.msg_id, event.search_key, event.__class__, - released=False, manual=event.manual) + db_event = Event.get_or_create_from_event(db.session, event) + db.session.commit() self.set_context(db_event) @@ -210,8 +209,7 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): stored into database. :rtype: dict """ - db_event = Event.get_or_create( - db.session, event.msg_id, event.search_key, event.__class__) + db_event = Event.get_or_create_from_event(db.session, event) # Used as tmp dict with {brew_build_nvr: ArtifactBuild, ...} mapping. builds = builds or {} diff --git a/freshmaker/models.py b/freshmaker/models.py index 6ada479..d056388 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -172,7 +172,8 @@ class Event(FreshmakerBase): @classmethod def create(cls, session, message_id, search_key, event_type, released=True, - state=None, manual=False, dry_run=False, requester=None): + state=None, manual=False, dry_run=False, requester=None, + requested_rebuilds=None, requester_metadata=None): if event_type in EVENT_TYPES: event_type = EVENT_TYPES[event_type] now = datetime.utcnow() @@ -186,6 +187,8 @@ class Event(FreshmakerBase): manual_triggered=manual, dry_run=dry_run, requester=requester, + requested_rebuilds=requested_rebuilds, + requester_metadata=requester_metadata, ) session.add(event) return event @@ -206,22 +209,47 @@ class Event(FreshmakerBase): @classmethod def get_or_create(cls, session, message_id, search_key, event_type, - released=True, manual=False, dry_run=False): + released=True, manual=False, dry_run=False, + requester=None, requested_rebuilds=None, + requester_metadata=None): instance = cls.get(session, message_id) if instance: return instance instance = cls.create( session, message_id, search_key, event_type, - released=released, manual=manual, dry_run=dry_run) + released=released, manual=manual, dry_run=dry_run, + requester=requester, requested_rebuilds=requested_rebuilds, + requester_metadata=requester_metadata) session.commit() return instance @classmethod def get_or_create_from_event(cls, session, event, released=True): + # we must extract all needed arguments, + # because event might not have some of them so we will use defaults + requester = getattr(event, "requester", None) + requested_rebuilds_list = getattr(event, "container_images", None) + requested_rebuilds = None + # make sure 'container_images' field is a list and convert it to str + if requested_rebuilds_list is not None and \ + isinstance(requested_rebuilds_list, list): + requested_rebuilds = " ".join(requested_rebuilds_list) + requester_metadata = getattr(event, "requester_metadata_json", None) + if requester_metadata is not None: + # try to convert JSON into str, if it's invalid use None + try: + requester_metadata = json.dumps(requester_metadata) + except TypeError: + log.warning("requester_metadata_json field is ill-formatted: %s", + requester_metadata) + requester_metadata = None + return cls.get_or_create(session, event.msg_id, event.search_key, event.__class__, released=released, manual=event.manual, - dry_run=event.dry_run) + dry_run=event.dry_run, requester=requester, + requested_rebuilds=requested_rebuilds, + requester_metadata=requester_metadata) @classmethod def get_unreleased(cls, session, states=None): diff --git a/freshmaker/parsers/internal/async_manual_build.py b/freshmaker/parsers/internal/async_manual_build.py index 37b89ae..720b27c 100644 --- a/freshmaker/parsers/internal/async_manual_build.py +++ b/freshmaker/parsers/internal/async_manual_build.py @@ -48,7 +48,9 @@ class FreshmakerAsyncManualbuildParser(BaseParser): msg_id, data.get('dist_git_branch'), data.get('container_images', []), freshmaker_event_id=data.get('freshmaker_event_id'), brew_target=data.get('brew_target'), - dry_run=data.get('dry_run', False)) + dry_run=data.get('dry_run', False), + requester=data.get('requester', None), + requester_metadata_json=data.get("metadata", None)) return event diff --git a/freshmaker/parsers/internal/manual_rebuild.py b/freshmaker/parsers/internal/manual_rebuild.py index 51f0aa6..3872a36 100644 --- a/freshmaker/parsers/internal/manual_rebuild.py +++ b/freshmaker/parsers/internal/manual_rebuild.py @@ -52,7 +52,8 @@ class FreshmakerManualRebuildParser(BaseParser): event = ManualRebuildWithAdvisoryEvent( msg_id, advisory, data.get("container_images", []), data.get("metadata", None), - freshmaker_event_id=data.get('freshmaker_event_id'), manual=True, dry_run=dry_run) + freshmaker_event_id=data.get('freshmaker_event_id'), manual=True, + dry_run=dry_run, requester=data.get('requester', None)) return event diff --git a/freshmaker/views.py b/freshmaker/views.py index 7dafffc..54e9578 100644 --- a/freshmaker/views.py +++ b/freshmaker/views.py @@ -525,6 +525,10 @@ class BuildAPI(MethodView): # added to DB) to backend using UMB messaging. Backend will then # re-generate the event and start handling it. data["msg_id"] = db_event.message_id + + # add information about requester + data["requester"] = db_event.requester + messaging.publish("manual.rebuild", data) # Return back the JSON representation of Event to client. @@ -611,6 +615,10 @@ class AsyncBuildAPI(MethodView): # added to DB) to backend using UMB messaging. Backend will then # re-generate the event and start handling it. data["msg_id"] = db_event.message_id + + # add information about requester + data["requester"] = db_event.requester + messaging.publish("async.manual.build", data) # Return back the JSON representation of Event to client. diff --git a/tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py b/tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py index 9dd54fa..0e1cf4d 100644 --- a/tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py +++ b/tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py @@ -28,7 +28,8 @@ from freshmaker.config import all_ from freshmaker import db, events from freshmaker.events import ( ErrataAdvisoryRPMsSignedEvent, - ManualRebuildWithAdvisoryEvent) + ManualRebuildWithAdvisoryEvent, + BaseEvent) from freshmaker.handlers.koji import RebuildImagesOnRPMAdvisoryChange from freshmaker.lightblue import ContainerImage from freshmaker.models import Event, Compose, ArtifactBuild, EVENT_TYPES @@ -249,6 +250,23 @@ class TestRebuildImagesOnRPMAdvisoryChange(helpers.ModelsTestCase): ret = handler.can_handle(event) self.assertFalse(ret) + @patch.object(freshmaker.conf, 'dry_run', new=True) + def test_requester_on_manual_rebuild(self): + event = ManualRebuildWithAdvisoryEvent( + "123", + ErrataAdvisory(123, "RHBA-2017", "REL_PREP", ["rpm"], + security_impact="", + product_short_name="product"), + ["foo-container", "bar-container"], + requester="requester1") + handler = RebuildImagesOnRPMAdvisoryChange() + ret = handler.can_handle(event) + self.assertTrue(ret) + handler.handle(event) + + db_event = Event.get(db.session, message_id='123') + self.assertEqual(db_event.requester, 'requester1') + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'RebuildImagesOnRPMAdvisoryChange': { 'image': {'product_short_name': 'foo'} @@ -998,7 +1016,8 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): def setUp(self): super(TestRecordBatchesImages, self).setUp() - self.mock_event = Mock(msg_id='msg-id', search_key=12345) + self.mock_event = Mock(spec=BaseEvent, msg_id='msg-id', search_key=12345, + manual=False, dry_run=False) self.patcher = helpers.Patcher( 'freshmaker.handlers.koji.RebuildImagesOnRPMAdvisoryChange.') diff --git a/tests/test_views.py b/tests/test_views.py index 4e8e838..2a77bc1 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -677,7 +677,8 @@ class TestManualTriggerRebuild(ViewBaseTest): u'requester_metadata': {}}) publish.assert_called_once_with( 'manual.rebuild', - {'msg_id': 'manual_rebuild_123', u'errata_id': 1}) + {'msg_id': 'manual_rebuild_123', u'errata_id': 1, + 'requester': 'root'}) @patch('freshmaker.messaging.publish') @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' @@ -697,7 +698,8 @@ class TestManualTriggerRebuild(ViewBaseTest): self.assertEqual(data['dry_run'], True) publish.assert_called_once_with( 'manual.rebuild', - {'msg_id': 'manual_rebuild_123', u'errata_id': 1, 'dry_run': True}) + {'msg_id': 'manual_rebuild_123', u'errata_id': 1, 'dry_run': True, + 'requester': 'root'}) @patch('freshmaker.messaging.publish') @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' @@ -721,7 +723,7 @@ class TestManualTriggerRebuild(ViewBaseTest): publish.assert_called_once_with( 'manual.rebuild', {'msg_id': 'manual_rebuild_123', u'errata_id': 1, - 'container_images': ["foo-1-1", "bar-1-1"]}) + 'container_images': ["foo-1-1", "bar-1-1"], 'requester': 'root'}) @patch('freshmaker.messaging.publish') @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' @@ -745,7 +747,30 @@ class TestManualTriggerRebuild(ViewBaseTest): publish.assert_called_once_with( 'manual.rebuild', {'msg_id': 'manual_rebuild_123', u'errata_id': 1, - 'metadata': {"foo": ["bar"]}}) + 'metadata': {"foo": ["bar"]}, 'requester': 'root'}) + + @patch('freshmaker.messaging.publish') + @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' + 'from_advisory_id') + @patch('freshmaker.parsers.internal.manual_rebuild.time.time') + def test_manual_rebuild_requester(self, time, from_advisory_id, publish): + time.return_value = 123 + from_advisory_id.return_value = ErrataAdvisory( + 123, 'name', 'REL_PREP', ['rpm']) + + payload = { + 'errata_id': 1, + } + with self.test_request_context(user='root'): + resp = self.client.post('/api/1/builds/', json=payload, content_type='application/json') + data = resp.json + + # Other fields are predictible. + self.assertEqual(data['requester'], "root") + publish.assert_called_once_with( + 'manual.rebuild', + {'msg_id': 'manual_rebuild_123', u'errata_id': 1, + 'requester': 'root'}) @patch('freshmaker.messaging.publish') @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' @@ -778,7 +803,8 @@ class TestManualTriggerRebuild(ViewBaseTest): publish.assert_called_once_with( 'manual.rebuild', {'msg_id': 'manual_rebuild_123', u'errata_id': 103, - 'container_images': ["foo-1-1"], 'freshmaker_event_id': 1}) + 'container_images': ["foo-1-1"], 'freshmaker_event_id': 1, + 'requester': 'root'}) @patch('freshmaker.messaging.publish') @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.' @@ -952,7 +978,8 @@ class TestAsyncBuild(ViewBaseTest): { 'msg_id': 'async_build_123', 'dist_git_branch': 'master', - 'container_images': ['foo-1-1', 'bar-1-1'] + 'container_images': ['foo-1-1', 'bar-1-1'], + 'requester': 'root', }) @patch('freshmaker.messaging.publish') @@ -979,6 +1006,7 @@ class TestAsyncBuild(ViewBaseTest): 'dist_git_branch': 'master', 'container_images': ['foo-1-1', 'bar-1-1'], 'dry_run': True, + 'requester': 'root', }) def test_async_build_with_non_async_event(self):