From 16328917c2174bead1c361ba89006e0072fbe7c6 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Nov 02 2017 13:22:03 +0000 Subject: [PATCH 1/2] Trigger container rebuild manually New endpoint /api/1/builds/ is added for triggering container rebuild that contains signed RPMs related to given Errata advisory ID. POST to this endpoint. Currently, only errata id is required to be passed and post data must be in JSON data with content type application/json. And finding containers synchronously. Signed-off-by: Chenxiong Qi --- diff --git a/conf/configrh.py b/conf/configrh.py index 82980ba..015a35d 100644 --- a/conf/configrh.py +++ b/conf/configrh.py @@ -56,6 +56,11 @@ class BaseConfiguration(config.BaseConfiguration): PULP_USERNAME = '' PULP_PASSWORD = '' + AUTH_BACKEND = 'kerberos' + # Replace with real ldap server URL + AUTH_LDAP_SERVER = '' + AUTH_LDAP_GROUP_BASE = 'ou=groups,dc=redhat,dc=com' + class DevConfiguration(BaseConfiguration): DEBUG = True diff --git a/freshmaker/__init__.py b/freshmaker/__init__.py index 80882e6..d56746a 100644 --- a/freshmaker/__init__.py +++ b/freshmaker/__init__.py @@ -43,10 +43,10 @@ db = SQLAlchemy(app) init_logging(conf) log = getLogger(__name__) -from freshmaker import views # noqa - login_manager = LoginManager() login_manager.init_app(app) from freshmaker.auth import init_auth # noqa init_auth(login_manager, conf.auth_backend) + +from freshmaker import views # noqa diff --git a/freshmaker/errata.py b/freshmaker/errata.py index 940b002..cc7cd75 100644 --- a/freshmaker/errata.py +++ b/freshmaker/errata.py @@ -61,14 +61,17 @@ class Errata(object): product_region = dogpile.cache.make_region().configure( conf.dogpile_cache_backend, expiration_time=24 * 3600) - def __init__(self, server_url): + def __init__(self, server_url=None): """ Initializes the Errata instance. :param str server_url: Base URL of Errata server. """ self._rest_api_ver = 'api/v1' - self.server_url = server_url.rstrip('/') + if server_url is not None: + self.server_url = server_url.rstrip('/') + else: + self.server_url = conf.errata_tool_server_url.rstrip('/') def _errata_rest_get(self, endpoint): """Request REST-style API @@ -92,6 +95,9 @@ class Errata(object): r.raise_for_status() return r.json() + def get_advisory(self, errata_id): + return self._errata_http_get('advisory/{0}.json'.format(errata_id)) + @region.cache_on_arguments() def _advisories_from_nvr(self, nvr): """ @@ -128,8 +134,7 @@ class Errata(object): if isinstance(event, BrewSignRPMEvent): return self._advisories_from_nvr(event.nvr) elif isinstance(event, ErrataAdvisoryStateChangedEvent): - data = self._errata_http_get( - "advisory/%s.json" % str(event.errata_id)) + data = self.get_advisory(event.errata_id) advisory = ErrataAdvisory( data["id"], data["advisory_name"], data["status"], data["security_impact"]) diff --git a/freshmaker/errors.py b/freshmaker/errors.py index b162a4c..9c7b89f 100644 --- a/freshmaker/errors.py +++ b/freshmaker/errors.py @@ -49,3 +49,7 @@ class Unauthorized(ValueError): class Forbidden(ValueError): pass + + +class BuildNotAllowed(Exception): + """Not allow to build docker images under particular circumstances""" diff --git a/freshmaker/handlers/brew/sign_rpm.py b/freshmaker/handlers/brew/sign_rpm.py index 8b192a7..9fc7f7a 100644 --- a/freshmaker/handlers/brew/sign_rpm.py +++ b/freshmaker/handlers/brew/sign_rpm.py @@ -21,7 +21,6 @@ # # Written by Chenxiong Qi -from freshmaker import conf from freshmaker import log from freshmaker import db from freshmaker.events import BrewSignRPMEvent, ErrataAdvisoryRPMsSignedEvent @@ -68,7 +67,7 @@ class BrewSignRPMHandler(BaseHandler): # When get a signed RPM, first step is to find out advisories # containing that RPM and ensure all builds are signed. - errata = Errata(conf.errata_tool_server_url) + errata = Errata() advisories = errata.advisories_from_event(event) # Filter out advisories which are not allowed by configuration. diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index b4acea1..a3f2adf 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -38,6 +38,7 @@ from freshmaker.types import ArtifactType, ArtifactBuildState from freshmaker.models import Event from freshmaker.consumer import work_queue_put from freshmaker.utils import krb_context, retry, get_rebuilt_nvr +from freshmaker.errors import BuildNotAllowed from odcs.client.odcs import ODCS from odcs.client.odcs import AuthMech @@ -58,7 +59,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): def can_handle(self, event): return isinstance(event, ErrataAdvisoryRPMsSignedEvent) - def handle(self, event): + def handle(self, event, manual=False): """ Rebuilds all Docker images which contain packages from the Errata advisory. @@ -68,14 +69,18 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): if not self.allow_build( ArtifactType.IMAGE, advisory_name=event.errata_name, advisory_security_impact=event.security_impact): - log.info("Errata advisory %s not allowed to trigger rebuilds.", - event.errata_name) + msg = 'Errata advisory {0} not allowed to trigger ' \ + 'rebuilds.'.format(event.errata_id) + if manual: + raise BuildNotAllowed(msg) + else: + log.info(msg) return [] # Generate the Database representation of `event`. db_event = Event.get_or_create( db.session, event.msg_id, event.search_key, event.__class__, - released=False) + released=False, manual=manual) db.session.commit() # Get and record all images to rebuild based on the current @@ -175,7 +180,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): errata_id = int(db_event.search_key) packages = [] - errata = Errata(conf.errata_tool_server_url) + errata = Errata() builds = errata.get_builds(errata_id) compose_source = None for nvr in builds: @@ -510,7 +515,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): :param int errata_id: Errata ID. """ - errata = Errata(conf.errata_tool_server_url) + errata = Errata() errata_id = int(errata_id) # Use the errata_id to find out Pulp repository IDs from Errata Tool diff --git a/freshmaker/handlers/errata/errata_advisory_state_changed.py b/freshmaker/handlers/errata/errata_advisory_state_changed.py index 880dad0..cf6dd8f 100644 --- a/freshmaker/handlers/errata/errata_advisory_state_changed.py +++ b/freshmaker/handlers/errata/errata_advisory_state_changed.py @@ -19,7 +19,7 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -from freshmaker import db, conf, log +from freshmaker import db, log from freshmaker.events import ( ErrataAdvisoryStateChangedEvent, ErrataAdvisoryRPMsSignedEvent) from freshmaker.models import Event, EVENT_TYPES @@ -79,7 +79,7 @@ class ErrataAdvisoryStateChangedHandler(BaseHandler): return [] # Get additional info from Errata to fill in the needed data. - errata = Errata(conf.errata_tool_server_url) + errata = Errata() advisories = errata.advisories_from_event(event) if not advisories: log.error("Unknown Errata advisory %d" % errata_id) diff --git a/freshmaker/migrations/versions/2f5a2f4385a0_add_manual_triggered_to_event_model.py b/freshmaker/migrations/versions/2f5a2f4385a0_add_manual_triggered_to_event_model.py new file mode 100644 index 0000000..1a3ff15 --- /dev/null +++ b/freshmaker/migrations/versions/2f5a2f4385a0_add_manual_triggered_to_event_model.py @@ -0,0 +1,26 @@ +"""Add manual_triggered to Event model + +Revision ID: 2f5a2f4385a0 +Revises: 2acc88805404 +Create Date: 2017-11-01 14:03:02.555397 + +""" + +# revision identifiers, used by Alembic. +revision = '2f5a2f4385a0' +down_revision = '2acc88805404' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('events', sa.Column('manual_triggered', sa.Boolean())) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_column('events', 'manual_triggered') + ### end Alembic commands ### diff --git a/freshmaker/models.py b/freshmaker/models.py index e8e5a86..dbc9387 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -127,8 +127,14 @@ class Event(FreshmakerBase): default=None, doc='Used to include new version packages to rebuild docker images') + manual_triggered = db.Column( + db.Boolean, + default=False, + doc='Whether this event is triggered manually') + @classmethod - def create(cls, session, message_id, search_key, event_type, released=True): + def create(cls, session, message_id, search_key, event_type, + released=True, manual=False): if event_type in EVENT_TYPES: event_type = EVENT_TYPES[event_type] event = cls( @@ -136,6 +142,7 @@ class Event(FreshmakerBase): search_key=search_key, event_type_id=event_type, released=released, + manual_triggered=manual, ) session.add(event) return event @@ -145,11 +152,13 @@ class Event(FreshmakerBase): return session.query(cls).filter_by(message_id=message_id).first() @classmethod - def get_or_create(cls, session, message_id, search_key, event_type, released=True): + def get_or_create(cls, session, message_id, search_key, event_type, + released=True, manual=False): instance = cls.get(session, message_id) if instance: return instance - return cls.create(session, message_id, search_key, event_type, released) + return cls.create(session, message_id, search_key, event_type, + released=released, manual=manual) @classmethod def get_unreleased(cls, session): diff --git a/freshmaker/views.py b/freshmaker/views.py index 8b33737..36b172d 100644 --- a/freshmaker/views.py +++ b/freshmaker/views.py @@ -21,6 +21,9 @@ # # Written by Jan Kaluza +from datetime import datetime +from uuid import uuid4 + import six from flask import request, jsonify from flask.views import MethodView @@ -32,6 +35,11 @@ from freshmaker.api_utils import pagination_metadata from freshmaker.api_utils import filter_artifact_builds from freshmaker.api_utils import filter_events from freshmaker.api_utils import json_error +from freshmaker.errata import Errata +from freshmaker.errors import BuildNotAllowed +from freshmaker.events import ErrataAdvisoryRPMsSignedEvent +from freshmaker.handlers.errata.errata_advisory_rpms_signed import ErrataAdvisoryRPMsSignedHandler +from freshmaker.auth import login_required, requires_role api_v1 = { 'event_types': { @@ -108,6 +116,12 @@ api_v1 = { 'methods': ['GET'], } }, + 'manual_trigger': { + 'url': '/api/1/builds/', + 'options': { + 'methods': ['POST'], + } + }, }, } @@ -214,6 +228,56 @@ class BuildAPI(MethodView): else: return json_error(404, "Not Found", "No such build found.") + @login_required + @requires_role('admins') + def post(self): + """Trigger image rebuild""" + if 'errata_id' not in request.json: + return json_error( + 403, 'Bad Request', 'Missing errata_id in request') + + errata_id = request.json['errata_id'] + errata = Errata() + advisory = errata.get_advisory(errata_id) + + if advisory['type'] != 'RHSA': + return json_error( + 403, 'Bad Request', + 'Errata {0} is not a RHSA advisory'.format(errata_id)) + + allowed_adv_status = ( + 'QA', 'REL_PREP', 'PUSH_READY', 'IN_PUSH', 'SHIPPED_LIVE' + ) + if advisory['status'] not in allowed_adv_status: + return json_error( + 403, 'Bad Request', + 'Errata {0} status {1} is not in {0}'.format( + errata_id, advisory['status'], + ', '.join(allowed_adv_status))) + + if not errata.builds_signed(advisory['id']): + return json_error( + 403, 'Bad Request', + 'Not all RPMs within advisory {0} are signed'.format( + advisory['id'])) + + event = ErrataAdvisoryRPMsSignedEvent( + '{0}-{1}.{2}'.format(datetime.today().year, + str(uuid4()), + advisory['advisory_name']), + advisory['advisory_name'], + advisory['id'], + advisory['security_impact']) + + handler = ErrataAdvisoryRPMsSignedHandler() + # FIXME: this should happen asynchronously + try: + handler.handle(event, manual=True) + except BuildNotAllowed as e: + return json_error(403, 'Bad Request', str(e)) + + return jsonify({'errata_id': errata_id}), 200 + API_V1_MAPPING = { 'events': EventAPI, diff --git a/tests/test_views.py b/tests/test_views.py index 2b95ec7..76e3955 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -23,7 +23,9 @@ import unittest import json import six -from freshmaker import app, db, events, models +from mock import patch + +from freshmaker import app, conf, db, events, models from freshmaker.types import ArtifactType, ArtifactBuildState @@ -252,5 +254,109 @@ class TestViews(unittest.TestCase): self.assertEqual(data['message'], 'No such build state found.') +class TestManualTriggerRebuild(unittest.TestCase): + def setUp(self): + db.session.remove() + db.drop_all() + db.create_all() + db.session.commit() + + self.client = app.test_client() + + self.errata_patcher = patch('freshmaker.views.Errata') + self.mock_errata = self.errata_patcher.start() + + self.et_server_url_patcher = patch.object( + conf, 'errata_tool_server_url', new='http://localhost/') + self.mock_et_server_url = self.et_server_url_patcher.start() + + def tearDown(self): + self.et_server_url_patcher.stop() + self.errata_patcher.stop() + + db.session.remove() + db.drop_all() + db.session.commit() + + def test_not_allow_if_not_RHSA(self): + errata = self.mock_errata.return_value + errata.get_advisory.return_value = {'type': 'RHBA'} + + resp = self.client.post('/api/1/builds/', + data=json.dumps({'errata_id': 1}), + content_type='application/json') + data = json.loads(resp.data.decode('utf-8')) + + self.assertEqual(403, data['status']) + self.assertEqual('Bad Request', data['error']) + self.assertEqual('Errata 1 is not a RHSA advisory', data['message']) + + def test_not_allow_if_advisory_status_is_valid(self): + errata = self.mock_errata.return_value + errata.get_advisory.return_value = { + 'type': 'RHSA', + 'status': 'NEW_FILES', + } + + resp = self.client.post('/api/1/builds/', + data=json.dumps({'errata_id': 1}), + content_type='application/json') + data = json.loads(resp.data.decode('utf-8')) + + self.assertEqual(403, data['status']) + self.assertEqual('Bad Request', data['error']) + self.assertIn('Errata 1 status NEW_FILES is not in', data['message']) + + def test_now_allow_if_not_all_rpms_signed(self): + errata = self.mock_errata.return_value + errata.get_advisory.return_value = { + 'type': 'RHSA', + 'status': 'QA', + 'id': 1, + } + errata.builds_signed.return_value = False + + resp = self.client.post('/api/1/builds/', + data=json.dumps({'errata_id': 1}), + content_type='application/json') + data = json.loads(resp.data.decode('utf-8')) + + errata.builds_signed.assert_called_once_with(1) + + self.assertEqual(403, data['status']) + self.assertEqual('Bad Request', data['error']) + self.assertEqual('Not all RPMs within advisory 1 are signed', + data['message']) + + @patch('freshmaker.views.ErrataAdvisoryRPMsSignedHandler.allow_build') + def test_bad_request_if_not_allow_build_from_advisory(self, allow_build): + allow_build.return_value = False + errata = self.mock_errata.return_value + fake_advisory = { + 'type': 'RHSA', + 'status': 'QA', + 'id': 1, + 'security_impact': 'important', + 'advisory_name': 'RHSA-2017:1' + } + errata.get_advisory.return_value = fake_advisory + errata.builds_signed.return_value = True + + resp = self.client.post('/api/1/builds/', + data=json.dumps({'errata_id': 1}), + content_type='application/json') + data = json.loads(resp.data.decode('utf-8')) + + allow_build.assert_called_once_with( + ArtifactType.IMAGE, + advisory_name=fake_advisory['advisory_name'], + advisory_security_impact=fake_advisory['security_impact']) + + self.assertEqual(403, data['status']) + self.assertEqual('Bad Request', data['error']) + self.assertEqual('Errata advisory 1 not allowed to trigger rebuilds.', + data['message']) + + if __name__ == '__main__': unittest.main() From 29d1ce422e5bbed03e290f09355fa531f75ad00f Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Nov 06 2017 07:33:50 +0000 Subject: [PATCH 2/2] Return 404 if errata_id does not exist Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/views.py b/freshmaker/views.py index 36b172d..b155156 100644 --- a/freshmaker/views.py +++ b/freshmaker/views.py @@ -25,6 +25,7 @@ from datetime import datetime from uuid import uuid4 import six +import requests from flask import request, jsonify from flask.views import MethodView @@ -234,15 +235,25 @@ class BuildAPI(MethodView): """Trigger image rebuild""" if 'errata_id' not in request.json: return json_error( - 403, 'Bad Request', 'Missing errata_id in request') + 400, 'Bad Request', 'Missing errata_id in request') errata_id = request.json['errata_id'] errata = Errata() - advisory = errata.get_advisory(errata_id) + try: + advisory = errata.get_advisory(errata_id) + except requests.HTTPError as e: + if e.response.status_code == 404: + return json_error( + 404, 'Not Found', + 'Advisory {0} is not found.'.format(errata_id)) + else: + raise + except Exception: + raise if advisory['type'] != 'RHSA': return json_error( - 403, 'Bad Request', + 400, 'Bad Request', 'Errata {0} is not a RHSA advisory'.format(errata_id)) allowed_adv_status = ( @@ -250,14 +261,14 @@ class BuildAPI(MethodView): ) if advisory['status'] not in allowed_adv_status: return json_error( - 403, 'Bad Request', + 400, 'Bad Request', 'Errata {0} status {1} is not in {0}'.format( errata_id, advisory['status'], ', '.join(allowed_adv_status))) if not errata.builds_signed(advisory['id']): return json_error( - 403, 'Bad Request', + 400, 'Bad Request', 'Not all RPMs within advisory {0} are signed'.format( advisory['id'])) @@ -274,7 +285,7 @@ class BuildAPI(MethodView): try: handler.handle(event, manual=True) except BuildNotAllowed as e: - return json_error(403, 'Bad Request', str(e)) + return json_error(400, 'Bad Request', str(e)) return jsonify({'errata_id': errata_id}), 200 diff --git a/tests/test_views.py b/tests/test_views.py index 76e3955..7032a6a 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -23,7 +23,8 @@ import unittest import json import six -from mock import patch +from mock import patch, Mock +from requests import HTTPError from freshmaker import app, conf, db, events, models from freshmaker.types import ArtifactType, ArtifactBuildState @@ -287,7 +288,7 @@ class TestManualTriggerRebuild(unittest.TestCase): content_type='application/json') data = json.loads(resp.data.decode('utf-8')) - self.assertEqual(403, data['status']) + self.assertEqual(400, data['status']) self.assertEqual('Bad Request', data['error']) self.assertEqual('Errata 1 is not a RHSA advisory', data['message']) @@ -303,7 +304,7 @@ class TestManualTriggerRebuild(unittest.TestCase): content_type='application/json') data = json.loads(resp.data.decode('utf-8')) - self.assertEqual(403, data['status']) + self.assertEqual(400, data['status']) self.assertEqual('Bad Request', data['error']) self.assertIn('Errata 1 status NEW_FILES is not in', data['message']) @@ -323,7 +324,7 @@ class TestManualTriggerRebuild(unittest.TestCase): errata.builds_signed.assert_called_once_with(1) - self.assertEqual(403, data['status']) + self.assertEqual(400, data['status']) self.assertEqual('Bad Request', data['error']) self.assertEqual('Not all RPMs within advisory 1 are signed', data['message']) @@ -352,11 +353,24 @@ class TestManualTriggerRebuild(unittest.TestCase): advisory_name=fake_advisory['advisory_name'], advisory_security_impact=fake_advisory['security_impact']) - self.assertEqual(403, data['status']) + self.assertEqual(400, data['status']) self.assertEqual('Bad Request', data['error']) self.assertEqual('Errata advisory 1 not allowed to trigger rebuilds.', data['message']) + def test_404_if_errata_id_not_exist(self): + errata = self.mock_errata.return_value + errata.get_advisory.side_effect = HTTPError(response=Mock(status_code=404)) + + resp = self.client.post('/api/1/builds/', + data=json.dumps({'errata_id': 1}), + content_type='application/json') + data = json.loads(resp.data.decode('utf-8')) + + self.assertEqual(404, data['status']) + self.assertEqual('Not Found', data['error']) + self.assertEqual('Advisory 1 is not found.', data['message']) + if __name__ == '__main__': unittest.main()