-
Notifications
You must be signed in to change notification settings - Fork 547
feat(github): Add support for app-based authentication and filtering by repository #8556
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
Conversation
erezrokah
left a comment
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.
Looks great, but can you link to the GitHub docs for the APIs that only work on GitHub apps?
I added repo traffic a while back and tested in with a personal access token
| for _, repo := range s.Repos { | ||
| if err := validateRepo(repo); err != nil { | ||
| return err | ||
| } | ||
| } |
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.
Do we want to validate that the repo is actually part of the Org?
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.
This would require one or more API calls, which will be done during initialization in any case (for multiplexing). I think some basic validation is fine here, we can't catch everything
Co-authored-by: bbernays <ben@cloudquery.io>
Co-authored-by: candiduslynx <candiduslynx@users.noreply.github.com>
@erezrokah Hmm, interesting. I think I must have misunderstood what they mean in the docs here: It seems like they in fact mean that it works with both PATs and Apps? Not sure why our workflow failed with a Personal Access Token then though. I suppose the app authentication is useful either way, but I will test a bit more and update the docs accordingly. |
I think it means both, see https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-organization-repositories Maybe it was a permissions issue? I was using the old personal access token (not the new fined grained one) |
|
@erezrokah Yeah exactly, that's what I just realized as well. I was able to make it work just now using a classic PAT, but it depends on what your organization allows as well. |
🤖 I have created a release *beep* *boop* --- ## [4.1.0](plugins-source-github-v4.0.1...plugins-source-github-v4.1.0) (2023-03-07) ### Features * **docs:** Render tables as a part of the Website and add a [tables search box](https://www.cloudquery.io/tables). The equivalent of the GitHub README.md file is now under each plugin's docs section, for example https://www.cloudquery.io/docs/plugins/sources/aws/tables. The Website HTML page is built from the GitHub markdown file located under each plugin's path in our Website code, for example https://github.com/cloudquery/cloudquery/blob/main/website/pages/docs/plugins/sources/aws/tables.md. For the list of all plugins table files as they are stored on GitHub see https://github.com/cloudquery/cloudquery/tree/main/website/tables ([342b0c5](342b0c5)) * **github:** Add support for app-based authentication and filtering by repository ([#8556](#8556)) ([d5d5ddd](d5d5ddd)) ### Bug Fixes * **deps:** Update golang.org/x/exp digest to c95f2b4 ([#8560](#8560)) ([9c3bd5b](9c3bd5b)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v1.41.0 ([#8682](#8682)) ([ea9d065](ea9d065)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v1.42.0 ([#8725](#8725)) ([b83b277](b83b277)) * **deps:** Update module github.com/stretchr/testify to v1.8.2 ([#8599](#8599)) ([2ec8086](2ec8086)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).


This implementation uses the appropriate app authentication for the given org, but if no org-specific authentication mechanism is available it falls back to either the personal access token or the first listed app auth.