-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add support for testing against Heimdal realms #13
Conversation
0ed7c05
to
a60c1c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
The k5test codebase was of course pulled out of the krb5 test suite back when python2 support was still important to python-gssapi. These days the minimum python version I'm interested in is 3.6. (If there were a case for something older, I'd listen, but that's the minimum in python-gssapi right now.) So the codebase has the support for old things, and that's fine to leave, but doesn't need to be preserved - it can be removed as needed or in one giant "remove it" commit/series.
Also, between Heimdal and abc, most likely you understand this code better than I do - that's fine but you will be called upon if it breaks :)
k5test/realm.py
Outdated
path = subprocess.check_output(['which', name], | ||
stderr=stderr_out).strip() | ||
path = path.decode(sys.getfilesystemencoding() or | ||
sys.getdefaultencoding()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation here is wonky. Also, is there a reason not to assume utf-8 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just copied from what it was before, will fix the indentation https://github.com/pythongssapi/k5test/blob/main/k5test/realm.py#L172-L173 but will leave the encoding as to what it was unless you want me to change it.
Sounds good, I wasn't sure if Python 2.7 support was desired for testing with MIT so I just kept it was it was. Because it's not I've pushed a commit that bumps the python_requires value in setup.py and removed some of the compatibility code. More could be done (like remove the
I only know Heimdal to the extent of getting it to run but that's fine with me :) |
Signed-off-by: Jordan Borean <jborean93@gmail.com> [rharwood@redhat.com: style changes, copyright, changelist]
9ee5d3a
to
4da5e32
Compare
(I've also updated the README and repo description - please let me know if I've missed something.) |
This adds support for using
k5test
against a Heimdal install. It extracts provider specific logic into their own classes and implements the basic methods for Heimdal. The realm type is chosen based on the value ofkrb5-config --version
.