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

feat: opt-in gh app integration #1217

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

feat: opt-in gh app integration #1217

wants to merge 60 commits into from

Conversation

plyr4
Copy link
Contributor

@plyr4 plyr4 commented Oct 25, 2024

this PR adds opt-in GitHub App integrations to the server.

the main differences are:

  • integrate with an OAuth GitHub App by providing an app id and private key
  • use the GitHub App to generate the .netrc password for builds when the app is installed and it can access the repo
  • adds webhook handlers for GitHub App installation and installation_repositories events, and syncs existing repos when changes are made
  • adds Checks functionality, pulled from the original hackathon efforts feat: add github app #1070
    • pulled out this code, it will be a follow-up PR

New Flags

Key Description Default Value
VELA_SCM_APP_ID set to the App ID for the GitHub App N/A
VELA_SCM_APP_PRIVATE_KEY the string value for the GitHub App private key generated through GitHub N/A

Required GitHub App Configurations

Permissions

the GitHub App requires the following permissions at the very minimum:

  • contents:read
  • checks:write

builds would request write permissions through the git yaml block, see below.

Subscribed Events

  • Installation target

OAuth

the same configurations and oauth scopes should be assigned to the GitHub App, including:

  • oauth callback url set to /authenticate (like usual)
  • Webhook URL set to the base url (like usual)

New YAML block: git

integrating with a GitHub App allows the use of the git YAML block for customizing the permissions allocated to the netrc password embedded into Vela steps.

git:
  token:
    repositories:
      - foo/bar
      - helloworld
    permissions:
      contents: write
      checks: write

this lets users customize the list of repositories that the netrc password has access to, but that list is restricted to ONLY the repos that the GitHub App org installation has been given access to.

by default, the compiler will use the following configurations unless otherwise provided:

git:
  token:
    repositories:
      - VELA_BUILD_REPO
    permissions:
      contents: read
      checks: write

⚠️ Considerations

this WILL impact builds, check out the following list of things to consider when migrating to GitHub App

Cloning Private Resources

due to the new restrictive policies set on the netrc token, Vela builds might lose the ability to read/write from certain private repos that the repo owner may have had access to.

GitHub Apps do not support providing access to repos that are outside the installation org. meaning, for a Vela build to access private repos, Go modules, etc, that are outside of the repo's org then the build author must provide override the clone step and use an alternative authentication method like a PAT

@JordanSussman
Copy link
Collaborator

Can you expand on why the contents:write is required instead of contents:read?

@plyr4
Copy link
Contributor Author

plyr4 commented Oct 29, 2024

Can you expand on why the contents:write is required instead of contents:read?

whoops thats a typo in the description. the code is accurate though, the minimum is contents:read 👍 (fixed)

https://github.com/go-vela/server/pull/1217/files#diff-890f0035c0a081909504883358a94c8c2fe91934cbb145c8c53d7d56ea7aa3baR57-R60

EnvVars: []string{"VELA_SCM_APP_PRIVATE_KEY", "SCM_APP_PRIVATE_KEY"},
FilePath: "/vela/scm/app_private_key",
Name: "scm.app.private_key",
Usage: "set value of base64 encoded SCM App integration (GitHub App) private key",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we make this "path to private key" instead of the value? would make it easier to drop files instead of env vars

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 40.14870% with 483 lines in your changes missing coverage. Please review.

Project coverage is 56.40%. Comparing base (f463fc9) to head (3916cbe).

Files with missing lines Patch % Lines
scm/github/app_transport.go 25.56% 130 Missing and 1 partial ⚠️
scm/github/app_install.go 0.00% 97 Missing ⚠️
scm/github/github.go 2.56% 75 Missing and 1 partial ⚠️
scm/github/repo.go 76.00% 23 Missing and 7 partials ⚠️
scm/github/github_client.go 74.22% 19 Missing and 6 partials ⚠️
api/repo/repair.go 0.00% 23 Missing ⚠️
api/auth/get_token.go 0.00% 22 Missing ⚠️
scm/github/opts.go 0.00% 18 Missing ⚠️
api/webhook/post.go 0.00% 12 Missing ⚠️
compiler/native/native.go 0.00% 12 Missing ⚠️
... and 8 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1217      +/-   ##
==========================================
- Coverage   56.87%   56.40%   -0.47%     
==========================================
  Files         599      604       +5     
  Lines       32856    33577     +721     
==========================================
+ Hits        18687    18940     +253     
- Misses      13535    13988     +453     
- Partials      634      649      +15     
Files with missing lines Coverage Δ
compiler/native/environment.go 87.22% <100.00%> (+0.46%) ⬆️
compiler/registry/github/github.go 100.00% <100.00%> (ø)
compiler/registry/github/template.go 58.53% <100.00%> (ø)
compiler/types/yaml/build.go 91.04% <100.00%> (+0.27%) ⬆️
compiler/types/yaml/git.go 100.00% <100.00%> (ø)
database/repo/table.go 100.00% <ø> (ø)
database/types/repo.go 96.73% <100.00%> (+0.03%) ⬆️
internal/webhook.go 100.00% <ø> (ø)
mock/server/repo.go 0.00% <ø> (ø)
scm/github/access.go 85.00% <100.00%> (ø)
... and 24 more

@JordanSussman
Copy link
Collaborator

but that list is restricted to ONLY the repos that the GitHub App org installation has been given access to.

I think I know the answer, but just to confirm: if I install the GitHub app on both the foo and bar organizations and run a build within a repo in the foo org, does that mean I won’t be able to clone a repo from the bar org, even if I specify bar/<repo> in the token section?

@plyr4
Copy link
Contributor Author

plyr4 commented Oct 30, 2024

but that list is restricted to ONLY the repos that the GitHub App org installation has been given access to.

I think I know the answer, but just to confirm: if I install the GitHub app on both the foo and bar organizations and run a build within a repo in the foo org, does that mean I won’t be able to clone a repo from the bar org, even if I specify bar/<repo> in the token section?

that's correct, org installations are scoped to repos in that org, unfortunately. its extremely similar to fine-grained access tokens (the beta PATs).

the workaround is to supply a github classic PAT created by a user with access to all the things

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.

2 participants