From b0b11c313f777427f4fec4eaf77750b1f0601584 Mon Sep 17 00:00:00 2001 From: Jan Kaluža Date: Jan 08 2018 09:53:54 +0000 Subject: Merge #170 `Simplify initiate ODCS client` --- diff --git a/conf/config.py b/conf/config.py index 71c76db..a7e5a2e 100644 --- a/conf/config.py +++ b/conf/config.py @@ -121,7 +121,12 @@ class BaseConfiguration(object): # ODCS configs # URL to ODCS to call APIs - ODCS_SERVER_URL = '' + ODCS_SERVER_URL = 'https://odcs.localhost/' + ODCS_VERIFY_SSL = True + # Valid authentication method would be kerberos or openidc + ODCS_AUTH_MECH = 'kerberos' + # When use openidc authentcation, set the openidc token for accessing ODCS + ODCS_OPENIDC_TOKEN = '' # Kerberos authentication Settings used to authenticated freshmaker itself # by other services diff --git a/freshmaker/config.py b/freshmaker/config.py index 63e4660..5a7f4bf 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -234,10 +234,18 @@ class Config(object): 'type': str, 'default': '', 'desc': 'Server URL to ODCS'}, + 'odcs_auth_mech': { + 'type': str, + 'default': 'kerberos', + 'desc': 'ODCS authentication mechanism.'}, 'odcs_verify_ssl': { 'type': bool, 'default': True, 'desc': 'Whether to enable SSL verification over HTTP with ODCS.'}, + 'odcs_openidc_token': { + 'type': str, + 'default': '', + 'desc': 'OpenIDC token used to access ODCS.'}, 'odcs_sigkeys': { 'type': list, 'default': [], diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 763ad74..71ebd0e 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -36,9 +36,7 @@ from freshmaker.types import EventState from freshmaker.models import ArtifactBuild, Event from freshmaker.utils import krb_context, get_rebuilt_nvr from freshmaker.errors import UnprocessableEntity, ProgrammingError - -from freshmaker.odcsclient import ODCS -from freshmaker.odcsclient import AuthMech +from freshmaker.odcsclient import create_odcs_client from freshmaker.odcsclient import COMPOSE_STATES @@ -401,10 +399,8 @@ class ContainerBuildHandler(BaseHandler): compose['state'] = COMPOSE_STATES['done'] return compose - odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) with krb_context(): - return odcs.get_compose(compose_id) + return create_odcs_client().get_compose(compose_id) def get_repo_urls(self, build): """ diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 1b0f81f..7a9c4ff 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -41,9 +41,8 @@ from freshmaker.types import ArtifactType, ArtifactBuildState, EventState from freshmaker.models import Event, Compose from freshmaker.consumer import work_queue_put from freshmaker.utils import krb_context, retry, get_rebuilt_nvr +from freshmaker.odcsclient import create_odcs_client -from odcs.client.odcs import ODCS -from odcs.client.odcs import AuthMech from odcs.common.types import COMPOSE_STATES @@ -236,11 +235,9 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): 'source: %s, source type: %s, packages: %s', compose_source, 'tag', packages) - odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) if not conf.dry_run: with krb_context(): - new_compose = odcs.new_compose( + new_compose = create_odcs_client().new_compose( compose_source, 'tag', packages=packages, sigkeys=conf.odcs_sigkeys, flags=["no_deps"]) else: @@ -264,8 +261,7 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): self.log_info('Generating new PULP type compose for content_sets: %r', content_sets) - odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) + odcs = create_odcs_client() if not conf.dry_run: with krb_context(): new_compose = odcs.new_compose( @@ -345,15 +341,12 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): if not build_tag: return None - odcs = ODCS(conf.odcs_server_url, - auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) if conf.dry_run: new_compose = self._fake_odcs_new_compose( build_tag, 'tag', results=['boot.iso']) else: with krb_context(): - new_compose = odcs.new_compose( + new_compose = create_odcs_client().new_compose( build_tag, 'tag', results=['boot.iso']) return new_compose diff --git a/freshmaker/models.py b/freshmaker/models.py index dab40c0..964d3da 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -33,9 +33,9 @@ from sqlalchemy.sql.expression import false from flask_login import UserMixin -from freshmaker import conf, db, log +from freshmaker import db, log from freshmaker import messaging -from freshmaker.odcsclient import ODCS, AuthMech +from freshmaker.odcsclient import create_odcs_client from freshmaker.utils import get_url_for, krb_context from freshmaker.types import ArtifactType, ArtifactBuildState, EventState from freshmaker.events import ( @@ -560,11 +560,8 @@ class Compose(FreshmakerBase): @property def finished(self): - odcs = ODCS(conf.odcs_server_url, - auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) with krb_context(): - return 'done' == odcs.get_compose( + return 'done' == create_odcs_client().get_compose( self.odcs_compose_id)['state_name'] diff --git a/freshmaker/odcsclient.py b/freshmaker/odcsclient.py index a14cb18..01ee914 100644 --- a/freshmaker/odcsclient.py +++ b/freshmaker/odcsclient.py @@ -29,5 +29,29 @@ # in freshmaker.handlers __init__.py. We cannot "import odcs" there, because # it would import freshmaker.handlers.odcs, so instead, we import it here # and in freshmaker.handler do "from freshmaker.odcsclient import ODCS". -from odcs.client.odcs import * # noqa -from odcs.common.types import * # noqa + +from odcs.client.odcs import AuthMech, ODCS +from odcs.common.types import COMPOSE_STATES # noqa + +from freshmaker import conf + + +def create_odcs_client(): + """ + Create instance of ODCS according to configured authentication mechasnim + """ + if conf.odcs_auth_mech == 'kerberos': + return ODCS(conf.odcs_server_url, + auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + elif conf.odcs_auth_mech == 'openidc': + if not conf.odcs_openidc_token: + raise ValueError('Missing OpenIDC token in configuration.') + return ODCS(conf.odcs_server_url, + auth_mech=AuthMech.OpenIDC, + openidc_token=conf.odcs_openidc_token, + verify_ssl=conf.odcs_verify_ssl) + else: + raise ValueError( + 'Authentication mechanism {0} is not supported yet.'.format( + conf.odcs_auth_mech)) diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index b5583ea..24387d5 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -428,15 +428,16 @@ class TestRequestBootISOCompose(unittest.TestCase): self.handler = ErrataAdvisoryRPMsSignedHandler() @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.krb_context') - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'create_odcs_client') @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' '_get_base_image_build_target') @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' '_get_base_image_build_tag') def test_get_boot_iso_compose( self, get_base_image_build_tag, get_base_image_build_target, - ODCS, krb_context): - odcs = ODCS.return_value + create_odcs_client, krb_context): + odcs = create_odcs_client.return_value odcs.new_compose.return_value = {'id': 1} get_base_image_build_target.return_value = 'build-target' diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index ddc0235..dcd7e32 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -289,13 +289,13 @@ class TestBatches(unittest.TestCase): "content_sets": ["first-content-set"] }) - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.new_compose') - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.get_compose') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.create_odcs_client') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.krb_context') - def test_batches_records(self, krb_context, get_compose, new_compose): + def test_batches_records(self, krb_context, create_odcs_client): """ Tests that batches are properly recorded in DB. """ + odcs = create_odcs_client.return_value # There are 8 mock builds below and each of them requires one pulp # compose. composes = [{ @@ -303,8 +303,8 @@ class TestBatches(unittest.TestCase): 'result_repofile': 'http://localhost/{}.repo'.format(compose_id), 'state_name': 'done' } for compose_id in range(1, 9)] - new_compose.side_effect = composes - get_compose.side_effect = composes + odcs.new_compose.side_effect = composes + odcs.get_compose.side_effect = composes # Creates following tree: # shared_parent @@ -622,7 +622,8 @@ class TestPrepareYumRepo(unittest.TestCase): db.drop_all() db.session.commit() - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'create_odcs_client') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' 'ErrataAdvisoryRPMsSignedHandler._get_packages_for_compose') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' @@ -632,10 +633,11 @@ class TestPrepareYumRepo(unittest.TestCase): @patch('freshmaker.utils.krbContext') def test_get_repo_url_when_succeed_to_generate_compose( self, krb_context, errata, sleep, _get_compose_source, - _get_packages_for_compose, ODCS): + _get_packages_for_compose, create_odcs_client): + odcs = create_odcs_client.return_value _get_packages_for_compose.return_value = ['httpd', 'httpd-debuginfo'] _get_compose_source.return_value = 'rhel-7.2-candidate' - ODCS.return_value.new_compose.return_value = { + odcs.new_compose.return_value = { "id": 3, "result_repo": "http://localhost/composes/latest-odcs-3-1/compose/Temporary", "result_repofile": "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo", @@ -657,7 +659,7 @@ class TestPrepareYumRepo(unittest.TestCase): _get_packages_for_compose.assert_called_once_with("httpd-2.4.15-1.f27") # Ensure new_compose is called to request a new compose - ODCS.return_value.new_compose.assert_called_once_with( + odcs.new_compose.assert_called_once_with( 'rhel-7.2-candidate', 'tag', packages=['httpd', 'httpd-debuginfo'], sigkeys=[], flags=["no_deps"]) @@ -666,7 +668,8 @@ class TestPrepareYumRepo(unittest.TestCase): "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo", compose['result_repofile']) - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'create_odcs_client') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' 'ErrataAdvisoryRPMsSignedHandler._get_packages_for_compose') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' @@ -677,7 +680,7 @@ class TestPrepareYumRepo(unittest.TestCase): new_callable=PropertyMock) def test_get_repo_url_packages_in_multiple_tags( self, krb_context, errata, sleep, _get_compose_source, - _get_packages_for_compose, ODCS): + _get_packages_for_compose, create_odcs_client): _get_packages_for_compose.return_value = ['httpd', 'httpd-debuginfo'] _get_compose_source.side_effect = [ 'rhel-7.2-candidate', 'rhel-7.7-candidate'] @@ -688,7 +691,7 @@ class TestPrepareYumRepo(unittest.TestCase): handler = ErrataAdvisoryRPMsSignedHandler() repo_url = handler._prepare_yum_repo(self.ev) - ODCS.return_value.new_compose.assert_not_called() + create_odcs_client.return_value.new_compose.assert_not_called() self.assertEqual(repo_url, None) db.session.refresh(self.ev) @@ -697,7 +700,8 @@ class TestPrepareYumRepo(unittest.TestCase): self.assertEqual(build.state_reason, "Packages for errata " "advisory 123 found in multiple different tags.") - @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'create_odcs_client') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' 'ErrataAdvisoryRPMsSignedHandler._get_packages_for_compose') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' @@ -708,7 +712,7 @@ class TestPrepareYumRepo(unittest.TestCase): new_callable=PropertyMock) def test_get_repo_url_packages_not_found_in_tag( self, krb_context, errata, sleep, _get_compose_source, - _get_packages_for_compose, ODCS): + _get_packages_for_compose, create_odcs_client): _get_packages_for_compose.return_value = ['httpd', 'httpd-debuginfo'] _get_compose_source.return_value = None @@ -718,7 +722,7 @@ class TestPrepareYumRepo(unittest.TestCase): handler = ErrataAdvisoryRPMsSignedHandler() repo_url = handler._prepare_yum_repo(self.ev) - ODCS.return_value.new_compose.assert_not_called() + create_odcs_client.return_value.new_compose.assert_not_called() self.assertEqual(repo_url, None) db.session.refresh(self.ev) diff --git a/tests/test_odcsclient.py b/tests/test_odcsclient.py new file mode 100644 index 0000000..5293c10 --- /dev/null +++ b/tests/test_odcsclient.py @@ -0,0 +1,71 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2018 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Chenxiong Qi + +import six +import unittest + +from mock import patch +from odcs.client.odcs import AuthMech + +from freshmaker import conf +from freshmaker.odcsclient import create_odcs_client + + +class TestCreateODCSClient(unittest.TestCase): + """Test odcsclient.create_odcs_client""" + + @patch.object(conf, 'odcs_auth_mech', new='kerberos') + @patch('freshmaker.odcsclient.ODCS') + def test_create_with_kerberos_auth(self, ODCS): + odcs = create_odcs_client() + + self.assertEqual(ODCS.return_value, odcs) + ODCS.assert_called_once_with( + conf.odcs_server_url, + auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + + @patch.object(conf, 'odcs_auth_mech', new='fas') + def test_error_if_unsupported_auth_configured(self): + six.assertRaisesRegex( + self, ValueError, r'.*fas is not supported yet.', + create_odcs_client) + + @patch.object(conf, 'odcs_auth_mech', new='openidc') + @patch.object(conf, 'odcs_openidc_token', new='12345') + @patch('freshmaker.odcsclient.ODCS') + def test_create_with_openidc_auth(self, ODCS): + odcs = create_odcs_client() + + self.assertEqual(ODCS.return_value, odcs) + ODCS.assert_called_once_with( + conf.odcs_server_url, + auth_mech=AuthMech.OpenIDC, + openidc_token='12345', + verify_ssl=conf.odcs_verify_ssl) + + @patch.object(conf, 'odcs_auth_mech', new='openidc') + def test_error_if_missing_openidc_token(self): + six.assertRaisesRegex( + self, ValueError, r'Missing OpenIDC token.*', + create_odcs_client)