#2274 Replace requests_kerberos with requests_gssapi when available
Closed 3 years ago by simo. Opened 3 years ago by simo.
https://pagure.io/forks/simo/koji.git use_request_gssapi  into  master

Use requests_gssapi instead of requests_kerberos
Simo Sorce • 3 years ago  
builder/kojid
file modified
+8 -4
@@ -75,10 +75,14 @@

      krbV = None

  

  try:

-     import requests_kerberos

-     Krb5Error = requests_kerberos.exceptions.RequestException

+     import requests_gssapi as reqgssapi

+     Krb5Error = reqgssapi.exceptions.RequestException

  except ImportError:  # pragma: no cover

-     requests_kerberos = None

+     try:

+         import requests_kerberos as reqgssapi

+         Krb5Error = reqgssapi.exceptions.RequestException

+     except ImportError:  # pragma: no cover

+         reqgssapi = None

  

  try:

      import librepo
@@ -6529,7 +6533,7 @@

              quit("Error: Unable to log in. Bad credentials?")

          except requests.exceptions.ConnectionError:

              quit("Error: Unable to connect to server %s" % (options.server))

-     elif krbV or requests_kerberos:

+     elif krbV or reqgssapi:

          krb_principal = options.krb_principal

          if krb_principal is None:

              krb_principal = options.host_principal_format % socket.getfqdn()

koji.spec
file modified
+8
@@ -125,11 +125,15 @@

  %endif

  Requires: pyOpenSSL

  Requires: python-requests

+ %if 0%{?fedora} >= 32

+ Requires: python-requests-gssapi

+ %else

  %if 0%{?fedora} || 0%{?rhel} >= 7

  Requires: python-requests-kerberos

  %else

  Requires: python-krbV >= 1.0.13

  %endif

+ %endif

  Requires: python-dateutil

  Requires: python-six

  
@@ -149,7 +153,11 @@

  %endif

  Requires: python%{python3_pkgversion}-pyOpenSSL

  Requires: python%{python3_pkgversion}-requests

+ %if 0%{?fedora} >= 32

+ Requires: python%{python3_pkgversion}-requests-gssapi

+ %else

  Requires: python%{python3_pkgversion}-requests-kerberos

+ %endif

  Requires: python%{python3_pkgversion}-dateutil

  Requires: python%{python3_pkgversion}-six

  

koji/__init__.py
file modified
+10 -7
@@ -80,9 +80,12 @@

      # we ignore it here

      pass

  try:

-     import requests_kerberos

+     import requests_gssapi as reqgssapi

  except ImportError:  # pragma: no cover

-     requests_kerberos = None

+     try:

+         import requests_kerberos as reqgssapi

+     except ImportError:  # pragma: no cover

+         reqgssapi = None

  try:

      import rpm

  except ImportError:
@@ -2524,9 +2527,9 @@

          return host

  

      def gssapi_login(self, principal=None, keytab=None, ccache=None, proxyuser=None):

-         if not requests_kerberos:

+         if not reqgssapi:

              raise PythonImportError(

-                 "Please install python-requests-kerberos to use GSSAPI."

+                 "Please install python-requests-gssapi to use GSSAPI."

              )

          # force https

          old_baseurl = self.baseurl
@@ -2553,14 +2556,14 @@

                  old_env['KRB5CCNAME'] = os.environ.get('KRB5CCNAME')

                  os.environ['KRB5CCNAME'] = ccache

              if principal:

-                 if re.match(r'0[.][1-8]\b', requests_kerberos.__version__):

+                 if re.match(r'0[.][1-8]\b', reqgssapi.__version__):

                      raise PythonImportError(

-                         'python-requests-kerberos >= 0.9.0 required for '

+                         'python-requests-gssapi >= 0.9.0 required for '

                          'keytab auth'

                      )

                  else:

                      kwargs['principal'] = principal

-             self.opts['auth'] = requests_kerberos.HTTPKerberosAuth(**kwargs)

+             self.opts['auth'] = reqgssapi.HTTPKerberosAuth(**kwargs)

              try:

                  # Depending on the server configuration, we might not be able to

                  # connect without client certificate, which means that the conn

setup.py
file modified
+1 -1
@@ -14,7 +14,7 @@

      requires = [

          'python-dateutil',

          'requests',

-         'requests-kerberos',

+         'requests-gssapi',

          'six',

          # 'libcomps',

          # 'rpm-py-installer', # it is optional feature

tests/test_lib/test_gssapi.py
file modified
+10 -10
@@ -21,7 +21,7 @@

  

      maxDiff = None

  

-     @mock.patch('koji.requests_kerberos', new=None)

+     @mock.patch('koji.reqgssapi', new=None)

      def test_gssapi_disabled(self):

          with self.assertRaises(ImportError):

              self.session.gssapi_login()
@@ -33,13 +33,13 @@

                                                           retry=False)

          self.assertEqual(old_environ, dict(**os.environ))

  

-     @mock.patch('requests_kerberos.HTTPKerberosAuth')

+     @mock.patch('reqgssapi.HTTPKerberosAuth')

Here it should be 'koji.reqgssapi.HTTPKerberosAuth'.

      def test_gssapi_login_keytab(self, HTTPKerberosAuth_mock):

          principal = 'user@EXAMPLE.COM'

          keytab = '/path/to/keytab'

          ccache = '/path/to/cache'

          old_environ = dict(**os.environ)

-         current_version = koji.requests_kerberos.__version__

+         current_version = koji.reqgssapi.__version__

          accepted_versions = ['0.12.0.beta1',

                               '0.12.0dev',

                               '0.12.0a1',
@@ -47,7 +47,7 @@

                               '0.10.0',

                               '0.9.0']

          for accepted_version in accepted_versions:

-             koji.requests_kerberos.__version__ = accepted_version

+             koji.reqgssapi.__version__ = accepted_version

              rv = self.session.gssapi_login(principal, keytab, ccache)

              self.session._callMethod.assert_called_once_with('sslLogin',

                                                               [None],
@@ -55,14 +55,14 @@

              self.assertEqual(old_environ, dict(**os.environ))

              self.assertTrue(rv)

              self.session._callMethod.reset_mock()

-         koji.requests_kerberos.__version__ = current_version

+         koji.reqgssapi.__version__ = current_version

  

-     def test_gssapi_login_keytab_unsupported_requests_kerberos_version(self):

+     def test_gssapi_login_keytab_unsupported_requests_kerberos(self):

          principal = 'user@EXAMPLE.COM'

          keytab = '/path/to/keytab'

          ccache = '/path/to/cache'

          old_environ = dict(**os.environ)

-         current_version = koji.requests_kerberos.__version__

+         current_version = koji.reqgssapi.__version__

          old_versions = ['0.8.0',

                          '0.7.0',

                          '0.6.1',
@@ -72,15 +72,15 @@

                          '0.2',

                          '0.1']

          for old_version in old_versions:

-             koji.requests_kerberos.__version__ = old_version

+             koji.reqgssapi.__version__ = old_version

              with self.assertRaises(koji.PythonImportError) as cm:

                  self.session.gssapi_login(principal, keytab, ccache)

              self.assertEqual(cm.exception.args[0],

-                              'python-requests-kerberos >= 0.9.0 required for '

+                              'python-requests-gssapi >= 0.9.0 required for '

                               'keytab auth')

              self.session._callMethod.assert_not_called()

              self.assertEqual(old_environ, dict(**os.environ))

-         koji.requests_kerberos.__version__ = current_version

+         koji.reqgssapi.__version__ = current_version

  

      def test_gssapi_login_error(self):

          old_environ = dict(**os.environ)

tests/test_lib/test_krbv.py
file modified
+2 -2
@@ -14,7 +14,7 @@

  

  class KrbVTestCase(unittest.TestCase):

      @mock.patch('koji.krbV', new=None)

-     @mock.patch('koji.requests_kerberos', new=None)

+     @mock.patch('koji.reqgssapi', new=None)

      def test_krbv_disabled(self):

          """Test that when krbV and gssapi are absent, we behave rationally"""

          self.assertEquals(koji.krbV, None)
@@ -25,7 +25,7 @@

      # this case should work on python3, but skipped still

      @unittest.skipIf(six.PY3, "skipped on python3 since missing of python-krbV")

      @mock.patch('koji.krbV', create=True)

-     @mock.patch('requests_kerberos.__version__', new='0.7.0')

+     @mock.patch('reqgssapi.__version__', new='0.7.0')

      @mock.patch('koji.ClientSession._serverPrincipal')

      def test_krbv_old_requests_kerberos(self, _serverPrincipal_mock, krbV_mock):

          self.assertIsNotNone(koji.krbV)

util/koji-gc
file modified
+1 -1
@@ -374,7 +374,7 @@

      elif options.user:

          # authenticate using user/password

          session.login()

-     elif koji.requests_kerberos:

+     elif koji.reqgssapi:

          session.gssapi_login(principal=options.principal, keytab=options.keytab,

                               proxyuser=options.runas)

      if not options.noauth and not session.logged_in:

util/kojira
file modified
+1 -1
@@ -1137,7 +1137,7 @@

      elif options.user:

          # authenticate using user/password

          session.login()

-     elif (koji.krbV or koji.requests_kerberos) and options.principal and options.keytab:

+     elif (koji.krbV or koji.reqgssapi) and options.principal and options.keytab:

          session.krb_login(options.principal, options.keytab, options.ccache)

      else:

          quit("No username/password/certificate supplied and Kerberos missing or not configured")

We want to retire python-kerberos which is used by requests_kerberos, so move koji off of using that old package to the replacement requests_gssapi which uses python-gssapi

pretty please pagure-ci rebuild

3 years ago

rebased onto 81cf9385f8b2ec6a0301929ca1d3ece6ac53187c

3 years ago

I'm a bit confused here, don't we already support gssapi?

ngompa, we "support" it currently by using a crummy old python-kerberos package that we want to remove since ages.
requests-kerberos -> python-kerberos
requests-gssapi -> python-gssapi

The latter one is what we want to support in the krb5/gssapi world.

pretty please pagure-ci rebuild

3 years ago

This should be fixed.

This PR makes python-requests-kerberos invalid as a dependency everywhere. Replace it instead of adding more conditionals.

I do not understand the CI failures, can someone exaplain to me?
I do not spot an actual failure anywhere

@ngompa unfortunately rquests-gssapi is not available on EL<=7 and not yet available in EL8 so I am using a soft approach...

Cool, we already have an issue for this: #882

tests says, that they are even not able to merge it to master without problems. You can try tests locally via make test3 && make flake8

I do not understand how it cannot merge to master given this PR is a single commit on top of master.

As for testing I am encountering some errors like the fact that requests_mock is not something available on my Fedora OS.

I also see an import issus in one of the tests I do not know how to handle.
I import request_gssapi or requests_kerberos as "reqgssapi", and the mock tests seem to try to import a module named "reqgssapi" directly ?
Guidance would be helpful.

And this is all I get with make flake8:

$ make flake8
error: line 93: Unknown tag:  <= 32BuildArch: noarch
error: query of specfile koji.spec failed, can't parse
error: line 93: Unknown tag:  <= 32BuildArch: noarch
error: query of specfile koji.spec failed, can't parse
flake8

rebased onto 457a71a45523f48abb443a46edb88a04369a8865

3 years ago

These lines are part of the problem. What it should have done? Something is missing there.

Oh ouch, I had a stray paste in there, sorry, fixed that, flake8 reports no errors now

rebased onto 3a51318

3 years ago

Here it should be 'koji.reqgssapi.HTTPKerberosAuth'.

I see this issue in Jenkins:

+ git remote rm proposed
fatal: No such remote: 'proposed'

I think this is due to the fact pagure was too slow to try to load my tree so I added the git tree manually to create the PR (as if it were a completely remote source).
Somehow I do not think Jenkins properly understand how to deal with that.

Should I try to create a new PR ?

https://pagure.io/koji/pull-request/2274#c-024067e50c7044b85564ebcb15974bf8d1914dbb-15 (you can click on "commented on line" title).

After fixing you can try to open new PR. Not sure, what is wrong here, but head looks differently compared to other PRs (here is full repo url, while e.g. in PR #2275 it only says tkopecek/koji in that place).

Ok I figured out the title thing and fixed the issue.

As for opening a new PR the problem is that whenever I open the "Open PR" menu, the spinners spin foever and never give me the ability to select my tree to open a PR, that's why I opened it manually last time.

np, I can test it + merge locally. Don't bother with that.

Opened #2280 to replace this PR

Pull-Request has been closed by simo

3 years ago