Skip to content

Add GitHub App Support #999

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
wlynch opened this issue Jun 20, 2019 · 17 comments
Open

Add GitHub App Support #999

wlynch opened this issue Jun 20, 2019 · 17 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@wlynch
Copy link
Member

wlynch commented Jun 20, 2019

Tracking issue for adding GitHub App support.

See https://developer.github.com/apps/differences-between-apps for the differences between GitHub Apps and OAuth.

Primary wins we get from Apps are:

  • Access to the Check Run API
  • Single webhook endpoint for the entire App config (no need to request admin permissions to create a webhook)
  • Fine grain access token permissions (can generate tokens scope by repo and permission, including read-only source)
  • Short lived tokens (last for 1 hour)

This primarily changes how handle authentication tokens for GitHub, since we will need to dynamic generate and attach secrets to Pipeline runs.

Current idea is to implement this with a Dynamic Admission Controller, generating an installation token, create a k8s secret containing that token, then attach it to the PipelineRun.

@bobcatfish bobcatfish added this to the Pipelines 1.1 / Post-beta 🐱 milestone Nov 6, 2019
@dibyom dibyom added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 3, 2019
@wlynch
Copy link
Member Author

wlynch commented Apr 2, 2020

Did some initial work on this awhile back - linking for reference https://docs.google.com/document/d/1dauJtzMWwxutwV38FwtSpauBhA9nKLNjZTIrYEjSaao

@afrittoli afrittoli removed this from the Pipelines Post-beta 🐱 milestone May 4, 2020
@wlynch
Copy link
Member Author

wlynch commented Jul 10, 2020

Was inspired by Tekton Results, and wrote a TaskRun -> GitHub Check PoC using a reconciler, largely based off: https://github.com/wlynch/tekton-demo/tree/master/github-app/cmd/watcher
Example CheckRun: https://github.com/wlynch/test/pull/1/checks?check_run_id=858655863

This relies on Object annotations for integration specific controllers to ID TaskRuns. I'm liking this as a flexible mechanism for supporting arbitrary integrations, and can be very easy to implement integration specific Triggers when paired with things like Trigger ObjectMeta templating (tektoncd/triggers#618). I'm planning to draft this as a larger TEP for how we can handle pipeline integrations.

Example TaskRun:

kind: TaskRun
metadata:
  name: echo-sleep
  annotations:
    github.integrations.tekton.dev/app-installation: "111795"
    github.integrations.tekton.dev/owner: "wlynch"
    github.integrations.tekton.dev/repo: "test"
    github.integrations.tekton.dev/commit: "db165c3a71dc45d096aebd0f49f07ec565ad1e08"
spec:
  taskSpec:
    steps:
      - name: hello
        image: ubuntu
        command: ["echo", "hello"]

@dibyom
Copy link
Member

dibyom commented Jul 10, 2020

Neat! Once we have Custom Tasks, it would be interesting to see if this can also evolve to a Custom Task i.e. the watcher becomes the GitHub App controller, the annotations become params for the GitHubApp/Check Run objects whose values can be passed in via Triggers. The use case would be for a more explicit pipeline that for example will have a finally GithubAction custom task for updates!

@vdemeester
Copy link
Member

@wlynch I guess it uses a different API than https://github.com/tektoncd/experimental/tree/master/commit-status-tracker right ? (cc @bigkevmcd )

@wlynch
Copy link
Member Author

wlynch commented Jul 13, 2020

Yes, but this differs from @bigkevmcd's Operator in a few ways:

  1. Uses the GitHub Checks API instead of the go-scm package. I think this is a good example of how integration specific APIs can provide more detail than a generic go-scm implementation.
  2. Doesn't require use of a Git pipeline resource. I think this is generally more flexible because:
    • It's less ambiguous in cases where multiple Git resources are used.
    • Allows for cases where no Git resources are used.

That said, I think the PoC follows much of the core of https://github.com/tektoncd/experimental/tree/master/commit-status-tracker. :)

@bigkevmcd
Copy link
Member

Yeah, that's the case.

One of the complications that I can see, is that each organisation would likely want to manage their own GitHub App for this, because the owner of the GitHub app can authenticate as any user that has authorised it to (within the scope of the token that was requested).

So, you'd effectively have to have your own private GitHub application for this (and of course, this only works on GitHub).

@wlynch
Copy link
Member Author

wlynch commented Jul 13, 2020

One of the complications that I can see, is that each organisation would likely want to manage their own GitHub App for this, because the owner of the GitHub app can authenticate as any user that has authorised it to (within the scope of the token that was requested).

I think this is actually one of the benefits of the reconciler model - as long as there is a mechanism to call into the cluster, you can store and handle integration creds outside of the user controlled cluster. This enables centralized integrations that rely on Tekton as an execution mechanism without exposing the App private key. App providers would just need to verify that the user cluster is authorized to update statuses for a cluster (e.g. to protect against instances where the user modifies the annotations).

@fbongiovanni29
Copy link

fbongiovanni29 commented Jul 29, 2020

This is awesome! Not to be a nudge but my developers labeled this a must have if we're to adopt tekton. Any idea when it'll be ready?

If there's anything I can do to contribute let me know, I'm not great with go but I can give it a shot

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2020
@bobcatfish
Copy link
Collaborator

I'm thinking we want to keep this open, since it covers an important use case. However I do wonder about where this feature belongs, esp. as we seem to be moving to relying more on the catalog for specific provider integrations 🤔 Triggers has GitHub awareness via interceptors but even that is being updated to be pluggable and not built in.

@wlynch
Copy link
Member Author

wlynch commented Oct 28, 2020

I can look into cleaning up my PoC and add it somewhere in experimental.

I think between my GitHub App PoC, commit-status-tracker, and the possible (? - not sure where this landed) cloud events notification controller, I think there's a pattern here of that might benefit from a common framework to help other integrations get bootstrapped for responding to Tekton events. 🤔 (I won't worry about this for my PoC, but something worth thinking about)

@chmouel
Copy link
Member

chmouel commented Oct 28, 2020 via email

@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 27, 2020
@dibyom
Copy link
Member

dibyom commented Dec 9, 2020

/remove-lifecycle-rotten
/lifecycle frozen

This is a pretty important use case to solve regardless of whether the actual solution is within the pipelines repo or not!

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Dec 9, 2020
@vdemeester
Copy link
Member

This is a pretty important use case to solve regardless of whether the actual solution is within the pipelines repo or not!

Agreed, but I really think this is probably something that make sense for triggers and other component that have a direct connection to GitHub, … But this can be discussed 😉

@mike-sirs
Copy link

Greetings
I've built a small app that uses GitHub application private key to generate and store an access token in K8S secrets.
https://github.com/mike-sirs/gha-get-token

@spstarr
Copy link

spstarr commented Apr 29, 2022

What I would like to be able to do is this:
https://tryexceptpass.org/article/pytest-github-integration/

Where by Kaniko builds --> the workspace is kept --> the github app runs pytest -> sends report to Github

I think if i create a task for a github app to it could be possible...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Todo
Development

No branches or pull requests