#1107 Modify MBS to use a separate Kerberos context per thread so both threads can use the thread keyring to store the Kerberos cache
Merged 4 months ago by mprahl. Opened 4 months ago by mprahl.

@@ -41,7 +41,6 @@ 

  """

  

  import pkg_resources

- import os

  from flask import Flask, has_app_context, url_for

  from flask_sqlalchemy import SQLAlchemy

  from sqlalchemy.pool import StaticPool

@@ -70,9 +69,6 @@ 

  

  conf = init_config(app)

  

- # We want to use a separate Kerberos cache per thread to avoid Kerberos cache corruption

- os.environ['KRB5CCNAME'] = 'KEYRING:thread:mbs'

- 

  

  class MBSSQLAlchemy(SQLAlchemy):

      """

@@ -49,6 +49,7 @@ 

  import module_build_service.scm

  import module_build_service.utils

  from module_build_service.builder.utils import execute_cmd

+ from module_build_service.builder.koji_backports import ClientSession as KojiClientSession

  from module_build_service.errors import ProgrammingError

  

  from module_build_service.builder.base import GenericBuilder

@@ -455,7 +456,7 @@ 

  

          address = koji_config.server

          log.info("Connecting to koji %r.", address)

-         koji_session = koji.ClientSession(address, opts=koji_config)

+         koji_session = KojiClientSession(address, opts=koji_config)

  

          if not login:

              return koji_session

@@ -463,13 +464,23 @@ 

          authtype = koji_config.authtype

          log.info("Authenticate session with %r.", authtype)

          if authtype == "kerberos":

+             try:

+                 import krbV

+             except ImportError:

+                 raise RuntimeError(

+                     "python-krbV must be installed to authenticate with Koji using Kerberos")

              keytab = getattr(config, "krb_keytab", None)

              principal = getattr(config, "krb_principal", None)

              if not keytab and principal:

                  raise ValueError(

                      "The Kerberos keytab and principal aren't set for Koji authentication")

              log.debug("  keytab: %r, principal: %r" % (keytab, principal))

-             koji_session.krb_login(principal=principal, keytab=keytab)

+             # We want to create a context per thread to avoid Kerberos cache corruption

+             ctx = krbV.Context()

+             # We want to use the thread keyring for the ccache to ensure we have one cache per

+             # thread to avoid Kerberos cache corruption

+             ccache = "KEYRING:thread:mbs"

+             koji_session.krb_login(principal=principal, keytab=keytab, ctx=ctx, ccache=ccache)

          elif authtype == "ssl":

              koji_session.ssl_login(

                  os.path.expanduser(koji_config.cert),

@@ -0,0 +1,98 @@ 

+ # flake8: noqa

+ import base64

+ import traceback

+ 

+ import koji

+ # Import krbV from here so we don't have to redo the whole try except that Koji does

+ from koji import krbV, PythonImportError, AuthError, AUTHTYPE_KERB

+ 

+ 

+ class ClientSession(koji.ClientSession):

+     """The koji.ClientSession class with patches from upstream."""

+ 

+     # This backport comes from https://pagure.io/koji/pull-request/1187

+     def krb_login(self, principal=None, keytab=None, ccache=None, proxyuser=None, ctx=None):

+         """Log in using Kerberos.  If principal is not None and keytab is

+         not None, then get credentials for the given principal from the given keytab.

+         If both are None, authenticate using existing local credentials (as obtained

+         from kinit).  ccache is the absolute path to use for the credential cache. If

+         not specified, the default ccache will be used.  If proxyuser is specified,

+         log in the given user instead of the user associated with the Kerberos

+         principal.  The principal must be in the "ProxyPrincipals" list on

+         the server side.  ctx is the Kerberos context to use, and should be unique

+         per thread.  If ctx is not specified, the default context is used."""

+ 

+         try:

+             # Silently try GSSAPI first

+             if self.gssapi_login(principal, keytab, ccache, proxyuser=proxyuser):

+                 return True

+         except Exception as e:

+             if krbV:

+                 e_str = ''.join(traceback.format_exception_only(type(e), e))

+                 self.logger.debug('gssapi auth failed: %s', e_str)

+                 pass

+             else:

+                 raise

+ 

+         if not krbV:

+             raise PythonImportError(

+                 "Please install python-krbV to use kerberos."

+             )

+ 

+         if not ctx:

+             ctx = krbV.default_context()

+ 

+         if ccache != None:

+             ccache = krbV.CCache(name=ccache, context=ctx)

+         else:

+             ccache = ctx.default_ccache()

+ 

+         if principal != None:

+             if keytab != None:

+                 cprinc = krbV.Principal(name=principal, context=ctx)

+                 keytab = krbV.Keytab(name=keytab, context=ctx)

+                 ccache.init(cprinc)

+                 ccache.init_creds_keytab(principal=cprinc, keytab=keytab)

+             else:

+                 raise AuthError('cannot specify a principal without a keytab')

+         else:

+             # We're trying to log ourself in.  Connect using existing credentials.

+             cprinc = ccache.principal()

+ 

+         self.logger.debug('Authenticating as: %s', cprinc.name)

+         sprinc = krbV.Principal(name=self._serverPrincipal(cprinc), context=ctx)

+ 

+         ac = krbV.AuthContext(context=ctx)

+         ac.flags = krbV.KRB5_AUTH_CONTEXT_DO_SEQUENCE | krbV.KRB5_AUTH_CONTEXT_DO_TIME

+         ac.rcache = ctx.default_rcache()

+ 

+         # create and encode the authentication request

+         (ac, req) = ctx.mk_req(server=sprinc, client=cprinc,

+                                auth_context=ac, ccache=ccache,

+                                options=krbV.AP_OPTS_MUTUAL_REQUIRED)

+         req_enc = base64.encodestring(req)

+ 

+         # ask the server to authenticate us

+         (rep_enc, sinfo_enc, addrinfo) = self.callMethod('krbLogin', req_enc, proxyuser)

+         # Set the addrinfo we received from the server

+         # (necessary before calling rd_priv())

+         # addrinfo is in (serveraddr, serverport, clientaddr, clientport)

+         # format, so swap the pairs because clientaddr is now the local addr

+         ac.addrs = tuple((addrinfo[2], addrinfo[3], addrinfo[0], addrinfo[1]))

+ 

+         # decode and read the reply from the server

+         rep = base64.decodestring(rep_enc)

+         ctx.rd_rep(rep, auth_context=ac)

+ 

+         # decode and decrypt the login info

+         sinfo_priv = base64.decodestring(sinfo_enc)

+         sinfo_str = ac.rd_priv(sinfo_priv)

+         sinfo = dict(zip(['session-id', 'session-key'], sinfo_str.split()))

+ 

+         if not sinfo:

+             self.logger.warn('No session info received')

+             return False

+         self.setSession(sinfo)

+ 

+         self.authtype = AUTHTYPE_KERB

+         return True

file modified
+10 -10

@@ -324,7 +324,7 @@ 

          expected_calls = [mock.call(1, 'foo'), mock.call(2, 'foo'), mock.call(1, 'bar')]

          assert mock_session.untagBuild.mock_calls == expected_calls

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights(self, ClientSession):

          session = ClientSession.return_value

          session.getLoggedInUser.return_value = {"id": 123}

@@ -347,7 +347,7 @@ 

          # getLoggedInUser requires to a logged-in session

          session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights_no_task_id(self, ClientSession):

          session = ClientSession.return_value

          session.getLoggedInUser.return_value = {"id": 123}

@@ -368,7 +368,7 @@ 

          assert session.getTaskDescendents.mock_calls == expected_calls

          session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights_no_build(self, ClientSession):

          session = ClientSession.return_value

          session.getLoggedInUser.return_value = {"id": 123}

@@ -389,7 +389,7 @@ 

          assert session.getTaskDescendents.mock_calls == expected_calls

          session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights_listBuilds_failed(self, ClientSession):

          session = ClientSession.return_value

          session.getLoggedInUser.return_value = {"id": 123}

@@ -406,7 +406,7 @@ 

          assert session.listBuilds.mock_calls == expected_calls

          session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights_getPackageID_failed(self, ClientSession):

          session = ClientSession.return_value

          session.getLoggedInUser.return_value = {"id": 123}

@@ -421,7 +421,7 @@ 

  

          session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_build_weights_getLoggedInUser_failed(self, ClientSession):

          session = ClientSession.return_value

          session.getAverageBuildDuration.return_value = None

@@ -546,7 +546,7 @@ 

              expected_calls = []

          assert session.packageListBlock.mock_calls == expected_calls

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_built_rpms_in_module_build(self, ClientSession):

          session = ClientSession.return_value

          session.listTaggedRPMS.return_value = ([

@@ -601,7 +601,7 @@ 

              []

          ),

      ))

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_filtered_rpms_on_self_dep(self, ClientSession, br_filtered_rpms, expected):

          session = ClientSession.return_value

          session.listTaggedRPMS.return_value = (

@@ -679,14 +679,14 @@ 

          else:

              mock_koji_cg.koji_import.assert_not_called()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_anonymous_session(self, ClientSession):

          mbs_config = mock.Mock(koji_profile='koji', koji_config='conf/koji.conf')

          session = KojiModuleBuilder.get_session(mbs_config, login=False)

          assert ClientSession.return_value == session

          assert ClientSession.return_value.krb_login.assert_not_called

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_ensure_builder_use_a_logged_in_koji_session(self, ClientSession):

          builder = KojiModuleBuilder('owner', MagicMock(), conf, 'module-tag', [])

          builder.koji_session.krb_login.assert_called_once()

@@ -85,7 +85,7 @@ 

          except OSError:

              pass

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      @patch("subprocess.Popen")

      @patch("subprocess.check_output", return_value='1.4')

      @patch("pkg_resources.get_distribution")

@@ -142,7 +142,7 @@ 

          # Ensure an anonymous Koji session works

          koji_session.krb_login.assert_not_called()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      @patch("subprocess.Popen")

      @patch("subprocess.check_output", return_value='1.4')

      @patch("pkg_resources.get_distribution")

@@ -206,7 +206,7 @@ 

          with open(path.join(dir_path, "modulemd.i686.txt")) as mmd:

              assert len(mmd.read()) == 255

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_tag_cg_build(self, ClientSession):

          """ Test that the CG build is tagged. """

          koji_session = ClientSession.return_value

@@ -221,7 +221,7 @@ 

          # tagBuild requires logging into a session in advance.

          koji_session.krb_login.assert_called_once()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_tag_cg_build_fallback_to_default_tag(self, ClientSession):

          """ Test that the CG build is tagged to default tag. """

          koji_session = ClientSession.return_value

@@ -238,7 +238,7 @@ 

          # tagBuild requires logging into a session in advance.

          koji_session.krb_login.assert_called_once()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_tag_cg_build_no_tag_set(self, ClientSession):

          """ Test that the CG build is not tagged when no tag set. """

          koji_session = ClientSession.return_value

@@ -252,7 +252,7 @@ 

          # tagBuild requires logging into a session in advance.

          koji_session.krb_login.assert_called_once()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_tag_cg_build_no_tag_available(self, ClientSession):

          """ Test that the CG build is not tagged when no tag available. """

          koji_session = ClientSession.return_value

@@ -332,7 +332,7 @@ 

              'type': 'file'

          }

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_koji_rpms_in_tag(self, ClientSession):

          koji_session = ClientSession.return_value

          koji_session.getUser.return_value = GET_USER_RV

@@ -420,7 +420,7 @@ 

          # Listing tagged RPMs does not require to log into a session

          koji_session.krb_login.assert_not_called()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_koji_rpms_in_tag_empty_tag(self, ClientSession):

          koji_session = ClientSession.return_value

          koji_session.getUser.return_value = GET_USER_RV

@@ -432,7 +432,7 @@ 

          assert rpms == []

          koji_session.multiCall.assert_not_called()

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_koji_rpms_in_tag_empty_headers(self, ClientSession):

          koji_session = ClientSession.return_value

          koji_session.getUser.return_value = GET_USER_RV

@@ -98,7 +98,7 @@ 

  

          assert len(start_build_component.mock_calls) == expected_build_calls

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_trigger_new_repo_when_failed(

              self, ClientSession, create_builder, global_consumer, dbg):

          """

@@ -131,7 +131,7 @@ 

          koji_session.newRepo.assert_called_once_with(

              "module-testmodule-master-20170219191323-c40c156c-build")

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_trigger_new_repo_when_succeeded(

              self, ClientSession, create_builder, global_consumer, dbg):

          """

@@ -203,7 +203,7 @@ 

          for component in components:

              assert component.state is None

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_old_build_targets_are_not_associated_with_any_module_builds(

              self, ClientSession, create_builder, global_consumer, dbg):

          consumer = mock.MagicMock()

@@ -223,7 +223,7 @@ 

  

          koji_session.deleteBuildTarget.assert_not_called()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_dont_delete_base_module_build_target(

              self, ClientSession, create_builder, global_consumer, dbg):

          module_build = models.ModuleBuild.query.filter_by(id=3).one()

@@ -248,7 +248,7 @@ 

  

              koji_session.deleteBuildTarget.assert_not_called()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_dont_delete_build_target_for_unfinished_module_builds(

              self, ClientSession, create_builder, global_consumer, dbg):

          module_build = models.ModuleBuild.query.filter_by(id=3).one()

@@ -275,7 +275,7 @@ 

  

              koji_session.deleteBuildTarget.assert_not_called()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_only_delete_build_target_with_allowed_koji_tag_prefix(

              self, ClientSession, create_builder, global_consumer, dbg):

          module_build_2 = models.ModuleBuild.query.filter_by(id=2).one()

@@ -320,7 +320,7 @@ 

              koji_session.deleteBuildTarget.assert_called_once_with(1)

              koji_session.krb_login.assert_called_once()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_cant_delete_build_target_if_not_reach_delete_time(

              self, ClientSession, create_builder, global_consumer, dbg):

          module_build_2 = models.ModuleBuild.query.filter_by(id=2).one()

@@ -121,7 +121,7 @@ 

      def teardown_method(self, test_method):

          clean_database()

  

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_return_empty_if_no_ursine_build_tag_is_found(self, ClientSession):

          session = ClientSession.return_value

  

@@ -141,7 +141,7 @@ 

          assert [] == modulemds

  

      @patch.object(conf, 'koji_tag_prefixes', new=['module'])

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_get_modulemds(self, ClientSession):

          session = ClientSession.return_value

  

@@ -246,7 +246,7 @@ 

      @patch.object(conf, 'base_module_names', new=['platform', 'project-platform'])

      @patch('module_build_service.utils.ursine.get_modulemds_from_ursine_content')

      @patch('module_build_service.resolver.GenericResolver.create')

-     @patch('koji.ClientSession')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

      def test_add_collision_modules(self, ClientSession, resolver_create,

                                     get_modulemds_from_ursine_content):

          xmd = {

@@ -417,7 +417,7 @@ 

                  for key, part in zip(nsvc_keys, nsvc_parts):

                      assert item[key] == part

  

-     @patch("koji.ClientSession")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession")

      def test_query_builds_with_binary_rpm(self, ClientSession):

          """

          Test for querying MBS with the binary rpm filename. MBS should return all the modules,

This commit includes the backport of the changes to krb_login in https://pagure.io/koji/pull-request/1187. This change is required for our separate threads to use a separate Kerberos context per thread.

For more context, please see the Koji PR linked above.

+1 Looks like a good temporary fix!

Pretty please pagure-ci rebuild

rebased onto b677beb3ccf35fae63caaae9804f9db9236540c3

4 months ago

Pretty please pagure-ci rebuild

rebased onto 25122cb

4 months ago

Pull-Request has been merged by mprahl

4 months ago