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

Add a credential plugin that uses GitHub Apps to get tokens #87

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Feb 19, 2025

This patch uses PyGitHub to implement it. It currently only supports
GitHub App IDs due to limitations in PyGitHub [1]. It also
takes care of passing it as a string into the library because PyGitHub
does not perform type conversion, which sometimes leads to producing
invalid JWTs [2] as the underlying library PyJWT lacks validation
[3].

This change temporarily excludes mentions of client ID and marks it as
not allowed.

The plugin accepts both GitHub App and installation IDs as integers
and normalizes them into strings.

The tests use ephemerally-produced in-memory RSA keys for JWT
verification. As an optimization, they are 1024-bit sized because it
is cheaper and makes the tests more performant compared to bigger
values.

As a temporary measure, the patch includes a suppression of WPS202
in flake8 for github_app.py.

A few type-ignores were added around PyGitHub exception handling due
to them using Any [4].

The patch also includes a change to the flake8-eradicate config
letting distinguish comments with URL lists.

Supersedes #84.

Co-authored-by: Sviatoslav Sydorenko webknjaz@redhat.com

@arrestle arrestle requested a review from fosterseth February 19, 2025 21:42
@webknjaz webknjaz changed the title Aap 38589 GitHub app cred Add a credential plugin that uses GitHub Apps to get tokens Feb 19, 2025
@webknjaz webknjaz force-pushed the aap-38589-github-app-cred branch 2 times, most recently from 95e3489 to cf39bb6 Compare February 19, 2025 22:30
@webknjaz webknjaz enabled auto-merge February 19, 2025 22:42
@webknjaz webknjaz force-pushed the aap-38589-github-app-cred branch 2 times, most recently from fc8da9a to e3f207e Compare February 19, 2025 22:47
arrestle and others added 2 commits February 19, 2025 23:54
This patch uses PyGitHub to implement it. It currently only supports
GitHub App IDs due to limitations in PyGitHub [[1]]. It also
takes care of passing it as a string into the library because PyGitHub
does not perform type conversion, which sometimes leads to producing
invalid JWTs [[2]] as the underlying library PyJWT lacks validation
[[3]].

The plugin accepts both GitHub App and installation IDs as integers
and normalizes them into strings.

The tests use ephemerally-produced in-memory RSA keys for JWT
verification. As an optimization, they are 1024-bit sized because it
is cheaper and makes the tests more performant compared to bigger
values.

As a temporary measure, the patch includes a suppression of WPS202
in flake8 for `github_app.py`.

A few type-ignores were added around PyGitHub exception handling due
to them using `Any` [[4]].

The patch also includes a change to the flake8-eradicate config
letting distinguish comments with URL lists.

[1]: PyGithub/PyGithub#3213
[2]: PyGithub/PyGithub#3214
[3]: jpadilla/pyjwt#1039
[4]: PyGithub/PyGithub#3218

Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>
This change temporarily removes mentions of client ID and marks it as
not allowed. PyGitHub seems to not fully support it [[1]].

[1]: PyGithub/PyGithub#3213
@webknjaz webknjaz force-pushed the aap-38589-github-app-cred branch from 681e75d to fdf7f09 Compare February 19, 2025 22:54
@arrestle arrestle requested a review from fosterseth February 19, 2025 22:57
@webknjaz webknjaz added this pull request to the merge queue Feb 19, 2025
Merged via the queue into ansible:devel with commit 719e3e3 Feb 19, 2025
27 checks passed
@webknjaz webknjaz deleted the aap-38589-github-app-cred branch February 19, 2025 23:03
webknjaz pushed a commit that referenced this pull request Feb 19, 2025
…lockfiles-updating-PyGitHub

This patch adds PyGitHub introduced in #87 to lock files.
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.

4 participants