Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace PyKerberos dependency with python-gssapi #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svmhdvn
Copy link

@svmhdvn svmhdvn commented Apr 18, 2024

Replace PyKerberos dependency with python-gssapi

This patch comprises the following changes:

  • Replace 'kerberos' with 'gssapi'
  • Remove lingering python2 support
  • Use a real kerberos realm for running tests (i.e. remove mock calls)

@svmhdvn svmhdvn marked this pull request as ready for review April 18, 2024 16:52
@svmhdvn svmhdvn changed the title WIP: replace PyKerberos dependency with python-gssapi Replace PyKerberos dependency with python-gssapi Apr 18, 2024
@svmhdvn
Copy link
Author

svmhdvn commented Apr 18, 2024

Please test this PR and let me know of any feedback/fixes. All existing tests are passing with a real kerberos realm instead of mock calls now.

@mkomitee
Copy link
Collaborator

mkomitee commented May 7, 2024

I think in principle, we're on board with a change from the kerberos library to the gssapi library, but since you coupled that with a restructuring of the repository, it's quite difficult to review. Would you be willing to separate the changes related to the library replacement out into its own PR?

@svmhdvn
Copy link
Author

svmhdvn commented May 7, 2024

For sure! I'll do that and submit a couple separated PRs soon.

This patch comprises the following changes:
* Replace 'kerberos' with 'gssapi'
* Remove lingering python2 support
* Use a real kerberos realm for running tests (i.e. remove mock calls)
@svmhdvn
Copy link
Author

svmhdvn commented Jun 2, 2024

Updated the patch to only make the switch to the 'gssapi' python library. Let me know if you need clarification on any of the changes and I can explain in detail. I tried to make the diff as small as possible, but the test changes are still large since the testing methodology is changed in this patch.

import socket
import sys

__version__ = '1.0.2'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes setup.py fail

else:
LOG.debug('KerberosAuthMiddleware is identifying as %s', principal)
self.service = None
if hostname:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe default to socket.gethostname()?

Suggested change
if hostname:
if not hostname:
hostname = socket.gethostname()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants