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.

Sign up for GitHub

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

Jump to bottom

[RFC-007] Implement GitHub app authentication for git repositories. #818

Merged
merged 1 commit into from
Dec 5, 2024
Merged

[RFC-007] Implement GitHub app authentication for git repositories. #818

merged 1 commit into from
Dec 5, 2024

Conversation

dipti-pai
Copy link
Member

This PR does the following:

  • Adds GitHub App authentication to git repositories in auth package that provides new APIs to fetch an app installation token using GitHub API.
  • Adds unit tests to test the github app authentication APIs.
  • Adds GitHub provider options in git package.
  • Adds unit tests to test fetching git credentials in git package.
  • Uses the GitHub provider to clone a git repository in go-git package.
  • Adds unit tests to validate auth options specified while cloning a git repository and fetching the authentication token in go-git package.
  • Add e2e tests to test pull from and push to git repositories using github app authentication APIs.
  • Updates the github workflow to run e2etests from CI.

@dipti-pai dipti-pai mentioned this pull request Oct 22, 2024
Open
15 tasks
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 22, 2024
View reviewed changes
git/credentials.go Dismissed Show dismissed Hide dismissed
git/credentials.go Dismissed Show dismissed Hide dismissed
@dipti-pai dipti-pai marked this pull request as draft October 22, 2024 20:09
@dipti-pai dipti-pai marked this pull request as ready for review October 28, 2024 17:30
darkowlzz
darkowlzz reviewed Nov 8, 2024
View reviewed changes
git/gogit/client.go Outdated Show resolved Hide resolved
git/gogit/client.go Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
git/gogit/client.go Outdated Show resolved Hide resolved
@darkowlzz darkowlzz added the area/git Git and SSH related issues and pull requests label Nov 8, 2024
@dipti-pai dipti-pai force-pushed the github-app-auth branch 2 times, most recently from 09c2862 to 005baa1 Compare November 12, 2024 22:36
darkowlzz
darkowlzz reviewed Nov 19, 2024
View reviewed changes
auth/github/client.go Outdated Show resolved Hide resolved
auth/github/test_helper.go Outdated Show resolved Hide resolved
auth/github/client_test.go Outdated Show resolved Hide resolved
auth/github/client_test.go Outdated Show resolved Hide resolved
git/gogit/client.go Outdated Show resolved Hide resolved
git/gogit/client.go Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
darkowlzz
darkowlzz reviewed Nov 19, 2024
View reviewed changes
auth/github/test_helper.go Outdated Show resolved Hide resolved
git/internal/e2e/github_test.go Outdated Show resolved Hide resolved
git/internal/e2e/github_test.go Outdated Show resolved Hide resolved
git/internal/e2e/github_test.go Outdated Show resolved Hide resolved
stefanprodan
stefanprodan reviewed Dec 3, 2024
View reviewed changes
auth/github/client.go Outdated Show resolved Hide resolved
darkowlzz
darkowlzz reviewed Dec 3, 2024
View reviewed changes
git/internal/e2e/README.md Outdated Show resolved Hide resolved
git/internal/e2e/README.md Show resolved Hide resolved
git/internal/e2e/github_test.go Outdated Show resolved Hide resolved
@dipti-pai dipti-pai force-pushed the github-app-auth branch 2 times, most recently from b8b60dd to 76cb6b7 Compare December 3, 2024 19:15
darkowlzz
darkowlzz reviewed Dec 3, 2024
View reviewed changes
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Tried this with source-controller and it seems to be working well.
Left last few minor comments.

git/internal/e2e/github_test.go Outdated Show resolved Hide resolved
git/internal/e2e/README.md Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
auth/github/client.go Outdated Show resolved Hide resolved
stefanprodan
stefanprodan approved these changes Dec 4, 2024
View reviewed changes
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @dipti-pai 🏅

darkowlzz
darkowlzz reviewed Dec 4, 2024
View reviewed changes
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Again, last few comments 😄 mostly about the tests.
Other than that, everything else looks good to me.

auth/github/client_test.go Outdated Show resolved Hide resolved
auth/github/client_test.go Outdated Show resolved Hide resolved
git/credentials_test.go Outdated Show resolved Hide resolved
git/gogit/client_test.go Outdated Show resolved Hide resolved
git/gogit/client_test.go Show resolved Hide resolved
darkowlzz
darkowlzz approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@darkowlzz darkowlzz 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 addressing all the comments. I have done all the testing and it LGTM!

I left a minor comment about the e2e test README. The extra commit can be squashed while fixing that before merge.

git/internal/e2e/README.md Outdated Show resolved Hide resolved
@dipti-pai
[RFC-007] Implement GitHub app authentication for git repositories.
0431fec 
- Add github app based authentication method to fetch installation token in auth package.
- Add unit tests to test the github app authentication
- Add github provider options in git package.
- Use the github provider to clone from go-git package.
- Add unit tests to fetch git credentials and cloning the repository using github app authentication.
- Add e2e tests to test pull/push to git repositories using github app authentication.
- Update the github workflow to run e2etests from CI.

Signed-off-by: Dipti Pai <diptipai89@outlook.com>
@stefanprodan stefanprodan merged commit 8262fd9 into fluxcd:main Dec 5, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanprodan stefanprodan stefanprodan approved these changes

@darkowlzz darkowlzz darkowlzz approved these changes

@aryan9600 aryan9600 Awaiting requested review from aryan9600 aryan9600 is a code owner

@hiddeco hiddeco Awaiting requested review from hiddeco hiddeco is a code owner

Assignees
No one assigned
Labels
area/git Git and SSH related issues and pull requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dipti-pai @darkowlzz @stefanprodan