ewolfe / prlint

GitHub App for linting pull request meta data
https://github.com/apps/prlint
MIT License
126 stars 9 forks source link

Self hosted version #85

Open mrchief opened 5 years ago

mrchief commented 5 years ago

Is it possible to deploy this to a self-hosted service? The source code kind of assumes public GitHub for URLs and stuff so it doesn't seem possible at the moment but it'd be cool if it did support that.

ewolfe commented 5 years ago

Yeah this can definitely be self hosted. You'll just have to expose an ENV variable called PRIVATE_KEY_B64 and then go through the process of registering your own GitHub app.

ewolfe commented 5 years ago

Let me know if you want to pair on this at all. I think it would be great to add documentation to the README on how to self host.

mrchief commented 5 years ago

I think 2 things will be a roadblock:

I'd suggest adding GH_API_URL= (for custom API endpoint) and ERROR_REPORTING=true|false to turn off Raven might help get past those issues.

mrchief commented 5 years ago

And I'll love to pair on this! Let me know if you want me to send a PR for above.

ewolfe commented 5 years ago

Yeah those are good points. It seems fairly trivial to abstract logging/raven out, and expose a way to setup GitHub Enterprise URL's. A PR for those would be great :)

mrchief commented 5 years ago

Cool! I'll get going on this then...

mrchief commented 5 years ago

@ewolfe Pushed #87 that allows disabling Raven logging. This change is backward compatible (although we can flip it not to be if so desire), so existing users will not have to do anything. Anyone wishing to disable logs needs to set DISABLE_RAVEN_LOG to some truthy value.

ewolfe commented 5 years ago

hey @mrchief, I just merged in your pull requests and deployed them. Everything seems to be working great! I think the next step would just be documenting how to self host. A rough outline:

  1. Fork repo
  2. How to keep the fork up to date with this upstream source
  3. A quick blurb on deployment options. Could include a now.sh specific example
  4. A link to create your own GitHub app and screenshot of suggested values (https://github.com/settings/apps/new)
mrchief commented 5 years ago

Thanks and sure thing! I'm in the process of deploying it internally. Once I'm done with that, I'll send a doc update.

ewolfe commented 5 years ago

@all-contributors please add @mrchief for infrastructure, tests and code

allcontributors[bot] commented 5 years ago

@ewolfe

I've put up a pull request to add @mrchief! :tada:

mrchief commented 5 years ago

Hey @ewolfe! Update on deployment to a private repo - I think we need to segregate the part that does the validation from the part that handles the web server. Something like prlint-core which:

  1. Receives the prlint.json
  2. Does the validation
  3. Return the body payload

Step 1 this could be baked in as default with an optional way left to the caller to retrieve it however they want. This would go well with any internal infosec requirements.

This way, the user can host it however their org prefers (Dedicated servers, lambdas etc.) without having to fork the repo. It's easier to stay updated (and in sync with the root repo) as it's a matter of upgrading the package version as opposed to having to maintain a fork.

If this sounds good to you, I can send in a PR.

ewolfe commented 5 years ago

Yeah that sounds like a really good idea. I'll definitely accept a pull request for that!

mrchief commented 5 years ago

Hey @ewolfe - do you know why we have 2 different ways of figuring out the pull request's status URL?

https://github.com/ewolfe/prlint/blob/master/src/app.js#L116 and https://github.com/ewolfe/prlint/blob/master/src/app.js#L140

mrchief commented 5 years ago

In fact, it appears that #L116 shouldn't work since it'd look like https://api.github.com/repos/ewolfe/prlint/statuses/{sha} and we aren't doing anything to replace the {sha} part. Maybe there is some github magic here that makes it work? I can't seem to dig up the relevant documentation for this from the pile of API docs that github has.

The logic in #L140 will produce a similar URL with the {sha} actually replaced with the correct one so to me, I think sticking to #L140 has no harm and we can avoid L116 logic entirely. I'm asking this because it'd make abstracting that part easier and it'd mean we'd need to modify one of the tests slightly.

ewolfe commented 5 years ago

Oh right, I think that was to cover the case where the pull request was coming from a forked version of the repo. It's been a while though so I could be wrong.

moustafab commented 5 years ago

Hey @ewolfe and @mrchief ,

I've been playing with this on a couple repos in my org. I like the extensibility of using regex for inspecting things, but need to be able to inspect commits too. There are a couple of other tools which are doing similar things, but this seems the most straightforward to get our teams to adopt. I'd like to make this easier to self host in a docker/kubernetes environment. If that sounds interesting I can contribute back my changes. For now I'm going to hack in the changes I need and build an image, but when this PR merges I can try out the probot based version.

echernyavskiy commented 4 years ago

@moustafab have you worked out a way to deploy PRLint into ECS or as a Lambda?

mrchief commented 4 years ago

Sounds great @moustafab! I think we wanted to move it to its own org and that's where things fell in limbo.

I like the extensibility of using regex for inspecting things, but need to be able to inspect commits too.

I remember adding commit linting into it as well (internally if course). I can share more details later.

@echernyavskiy I'm using this as a lambda internally. Probot apps can be deployed as lambdas as stated in their docs. There are some quirks though and I can share my recipe if that helps.

Both I & @ewolfe have not been able to find time to move this forward so any help is much appreciated!

echernyavskiy commented 4 years ago

@mrchief,

I'm using this as a lambda internally. Probot apps can be deployed as lambdas as stated in their docs. There are some quirks though and I can share my recipe if that helps.

I'd appreciate it, thanks.