tektoncd / pipeline

A cloud-native Pipeline resource.
https://tekton.dev
Apache License 2.0
8.48k stars 1.78k forks source link

Add GitHub App Support #999

Open wlynch opened 5 years ago

wlynch commented 5 years ago

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:

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.

wlynch commented 4 years ago

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

wlynch commented 4 years ago

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 (https://github.com/tektoncd/triggers/issues/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 commented 4 years ago

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 commented 4 years ago

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

wlynch commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

bobcatfish commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

FYI maybe that would be useful for others, we have this task in the catalog https://github.com/tektoncd/catalog/tree/master/task/github-app-token/0.1 to help the user getting a token from a github app private key. The user would usually plug this in its pipeline and use the resulted token for subsequent github api request.

tekton-robot commented 3 years ago

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.

dibyom commented 3 years ago

/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!

vdemeester commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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...