Skip to content

GitHub app access token lookup: allow to use PyJWT + cryptography instead of jwt #10664

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

weisheng-p
Copy link
Contributor

SUMMARY

Change community.general.github_app_access_token lookup to use PyJWT (https://pypi.org/project/PyJWT/) rather than jwt (https://github.com/GehirnInc/python-jwt).

Fixes #10299

this is a continuation of #10332

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

github_app_access_token

ADDITIONAL INFORMATION

Change the implementation to use pyjwt, rather than jwt that causes conflits elsewhere in the Ansible ecosystem.

thank you @blavoie for the intial commit and fixes. This pr would include both the compatibility issue and (hopefully) the required test cases

Im not too sure about how deprecation warning are usually shown in this collection, I've make used of display.deprecated function with a magic version number, any direction on this would be appreciated

@ansibullbot ansibullbot added bug This issue/PR relates to a bug lookup lookup plugin plugins plugin (any type) tests tests unit tests/unit labels Aug 13, 2025
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 13, 2025
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 13, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@felixfontein felixfontein added breaking_change This PR contains a breaking change that MUST NOT be backported check-before-release PR will be looked at again shortly before release and merged if possible. labels Aug 13, 2025
@felixfontein felixfontein added backport-11 Automatically create a backport for the stable-10 branch feature This issue/PR relates to a feature request and removed breaking_change This PR contains a breaking change that MUST NOT be backported labels Aug 14, 2025
@felixfontein felixfontein changed the title Fix lookup GitHub app access token GitHub app access token lookup: allow to use PyJWT + cryptography instead of jwt Aug 15, 2025
weisheng-p and others added 5 commits August 16, 2025 07:18
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@russoz
Copy link
Collaborator

russoz commented Aug 15, 2025

If I may suggest, give it a try to andebox by yours truly:

https://pypi.org/project/andebox/

You can run this sanity test easily in your local machine by simply running:

$ andebox test -- sanity --docker default --python 3.10 plugins/modules/your_module.py

@russoz
Copy link
Collaborator

russoz commented Aug 15, 2025

Though I suspect I am coming too late with that information :)

@weisheng-p
Copy link
Contributor Author

If I may suggest, give it a try to andebox by yours truly:

https://pypi.org/project/andebox/

You can run this sanity test easily in your local machine by simply running:

$ andebox test -- sanity --docker default --python 3.10 plugins/modules/your_module.py

I have a way to run it via ansible-test directly, I had a lot of difficuitly trying to get nox setup. I'll give this a go next time.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-11 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. feature This issue/PR relates to a feature request lookup lookup plugin plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple JWT libraries conflicts for github_app_access_token lookup and github_release module
5 participants