alexellis / derek

Reduce maintainer fatigue by automating GitHub
https://github.com/alexellis/derek/blob/master/USER_GUIDE.md
MIT License
806 stars 72 forks source link

Use-case: support private repos #116

Closed alexellis closed 5 years ago

alexellis commented 5 years ago

Expected Behaviour

I visited CompareTheMarket today who have a use-case for Derek for delegation of how people label issues without admin access to inner-source repos.

They want Derek to work on their private repos.

Current Behaviour

Derek reads from a CDN for all repos, but with the private ones there is no CDN.

Possible Solution

Find out if the event came from a private repo by using the metadata of the event. If private, grab .DEREK.yml from the GitHub API If not, then go ahead and fallback to the CDN

Steps to Reproduce (for bugs)

  1. Create private repo (this is free on GH now)
  2. Install your dev instance of Derek
  3. Push code and see nothing happen
alexellis commented 5 years ago

This is where we need to make a change: https://github.com/alexellis/derek/blob/9aadd63d106b92907107d5c6172140024d30f53d/handler/permissionsHandler.go#L89

matipan commented 5 years ago

I'll start by setting up Derek on a private repo and reproducing this issue :+1:

alexellis commented 5 years ago

I also saw:

No customer for ComparetheMarket in the logs, which means there needs to be a PR to this file: https://github.com/alexellis/derek/blob/master/.CUSTOMERS

Derek did the right thing by using the ACL, but I think we still need this PR.

Alex

alexellis commented 5 years ago

We need to start parsing the private repo status:

{
  "repository": {
    "id": 13379,
    "node_id": "xyz=",
    "name": "repo",
    "full_name": "owner/repo",
    "private": true,
   }
}
alexellis commented 5 years ago

I raised #117 to give a logging message and add the Private repo flag. The PR @matipan is working on is still needed.

matipan commented 5 years ago

I was able to reproduce the issue using a new derek installation on my OpenFaaS GKE cluster. I started to make a few changes and noticed what you mentioned previously. We will need to make adjustments for fetching .CUSTOMERS as well. I believe I have this working. If you want @alexellis I can give you access to the private repo I'm using to test this :+1:

matipan commented 5 years ago

A bit of evidence to show that it is working: derek-private-repos

matipan commented 5 years ago

Forgot to mention. I had to give Read-only access to the repository contents, we'll probably want to document that on the guides clarifying that the permission is only necessary for private files.

alexellis commented 5 years ago

My current SaaS version of Derek already has read-only on contents, so we should update the docs if that's not clear. Fortunately existing users won't have to update permissions.

screenshot 2019-02-21 at 09 21 13

Alex

alexellis commented 5 years ago

We will need to make adjustments for fetching .CUSTOMERS as well.

I'm not sure that this is the case, the CUSTOMERS file is configured once per Derek instance and the ACL could be in a separate public repo. I haven't seen a requirement for a private Derek ACL yet, but if we get it then it should be tracked as a new issue.

matipan commented 5 years ago

You are right :+1: I'm going through the contributing guide now to make sure I follow the correct steps to create a PR.

alexellis commented 5 years ago

PR raised: #119

rgee0 commented 5 years ago

Derek close: implemented in #119