gr2m / github-app-slack-demo

A minimal demo that integrates GitHub with Slack
https://github-app-slack-demo.netlify.app/
MIT License
5 stars 0 forks source link

[Batch] Implement Netlify Best Practices #39

Open gr2m opened 3 months ago

gr2m commented 3 months ago

I talked to @krider2010 today πŸ‘‹ about a few things that I'd love input for from Netlify. Also I don't know what I don't know so if anyone things our setup is overly complicated, I'm more than happy to change it.

Our deployment setup is as follows:

screenshot: Build & Deploy - Branches and deploy contexts

I disabled preview deploys because the URLs are variable and both GitHub Apps and Slack Apps to have a configured URL to which they send their respective webhook requests. So I figured the simplest would be is to only enable branch deploys for staging, as it results in a predictable URL: https://staging--github-app-slack-demo.netlify.app/

The two handlers for the GitHub and Slack webhook requests are implemented as Netlify Functions, specifically

That being said, it would be possible to make preview deploys with variable deploy URLs possible.

  1. Both GitHub and Slack allows to programmatically update the webhook URLs. So we could register a pool of both GitHub Apps and Slack Apps for testing, and implement some kind of logic that takes one of each pool for each pull request that has a preview deployment.
  2. Slack Apps can even be created and deleted programmatically, we might not even need a pool here.

I'd love to hear what people think we should do here as a best practices for this kind of apps. The deployment through staging works, but

  1. I miss the comments by https://github.com/apps/netlify like this one: https://github.com/gr2m/github-app-slack-demo/pull/9#issuecomment-2175740626. For branch deployments, there are not comments or deployments in GitHub's UI that would indicate that anything was deployed.
  2. Deploying to staging is tedious. First, a staging branch based off latest main needs to be created and pushed to the GitHub repository. Then changes can be made. Once merged, the staging branch gets deleted and will need to be re-created when needed again.

Another thing I'd like to do is to add a "deploy to Netlify" button to the GitHub README.md file, and make the flow of deploying an own instance with their own GitHub App / Slack App as seamless as possible. I'm not sure what the best strategy would be, but it might make sense to implement a "setup" state of the app that would be served at the / root path if no GitHub App and/or Slack App is registered. This Setup UI can walk the user through the necessary steps and e.g. show all environment variables in an .env file format which could be directly copied and pasted into the respective Netlify site configuration, after which the app could be re-deployed. If Netlify has APIs for it that we could implement with an OAuth flow, that would be even better, and probably quite fun to build.

Tasks

gr2m commented 3 months ago

@krider2010 here is what I see at https://app.netlify.com/sites/github-app-slack-demo/configuration/notifications#deploy-notifications

Image

I don't think that turning the Deploy Previews settings back on will have the desired result. I assume it will add the comments to pull requsets from staging -> main, but it will also create preview deploys for any other pull request against either staging or main based on what it says in the form

Image

seancdavis commented 3 months ago

Hey @gr2m! Here are some brief thoughts β€” I've tinkered with a similar approach (using Netlify to support Slack apps).

Programmatic URL workflows

When I've needed to register a URL remotely, I typically stick with the branch deploys as you've mentioned. However, I don't disable deploy previews. But if there's a need to test a one-off condition, I might just add that specific URL temporarily to the Slack app (as an example).

That's been enough for me. Aside from some workflow annoyance, is there a need to be more dynamic in the app URLs? Or is it just to avoid the staging branch workflow?

Using a predictable branch

I miss the comments by https://github.com/apps/netlify like this one: https://github.com/gr2m/github-app-slack-demo/pull/9#issuecomment-2175740626. For branch deployments, there are not comments or deployments in GitHub's UI that would indicate that anything was deployed.

Not sure I'm following this one. The trouble here is that if you use the staging workflow, you deploy by pushing directly to staging, so you don't get PR comments? Is there more to this?

Deploying to staging is tedious. First, a staging branch based off latest main needs to be created and pushed to the GitHub repository. Then changes can be made. Once merged, the staging branch gets deleted and will need to be re-created when needed again.

I hear you on this β€” I see how it can take you out of your typical way of working. I'll wait to see your response to my previous question before suggesting anything here.

But, in the meantime, if you protect the staging branch, it shouldn't be automatically deleted when merging into main, correct? (I haven't tested this in awhile.)

Deploy to Netlify button

As you're working through this, one thing that may help is using templates. You can include a set of environment variables that someone would have to fill out when creating a new Netlify site.

Would that solve your challenges in a new site setup? Or are there still OAuth things that you wouldn't be able to solve with environment variables?

gr2m commented 3 months ago

Hi Sean, thank you so much for getting back to me!

I don't disable deploy previews. But if there's a need to test a one-off condition, I might just add that specific URL temporarily to the Slack app (as an example).

I find it confusing to get the notifications for deploys that are not usable because my staging app(s) are not hooked up to it. Ideally, I would like the notification or at least a deploy event, but on on pull requests from staging to master.

But the more I think about it, the more I like the idea of just using the standard preview deploys, and making sure they are usable with hooked up slack and github apps using automations on my side. I feel this could be a big rabbit whole that I don't have time for right now, but I've run into this before with past GitHub Apps I've built. I could imagine a custom automation like this

  1. Register a pool of GitHub apps for staging, e.g. 10. Store all credentials and repository secrets.
  2. Add a Slack workspace configuration access token to repository secrets.
  3. Use a Repository variable to store JSON in order to keep state
  4. When a new pull request is created, create a new slack app with the webhook URL pointing to the new deployment (based on branch) and update the webhook URL for one of the idle GitHub Apps to point to the new deployment as well. Update the state accordingly in the repository variable.
  5. Now the deployment can be fully used for testing.
  6. When the pull request is merged or closed, delete the slack up, reset the GitHub App, and update the state in the repository variable.

If done well, this could be a great setup for building and collaborating on GitHub and/or Slack apps hosted on GitHub.

I think I would implement this as two GitHub Actions, one for GitHub Apps, one for Slack. That way they would be useful independently of each other.

What do you think?

But, in the meantime, if you protect the staging branch, it shouldn't be automatically deleted when merging into main, correct? (I haven't tested this in awhile.)

Yes, a branch can be protected from being deleted. However, if staging is not deleted and re-created it becomes stale. We doing a new staging deployment, I want it to be based on the latest version of the repository's default branch.

As you're working through this, one thing that may help is using templates. You can include a set of environment variables that someone would have to fill out when creating a new Netlify site.

Thank you, that's a great start!

I also saw that there are APIs to set environment variables, so we could create a wizzard-like setup experience in future, if anyone would like to build it: https://open-api.netlify.com/#tag/environmentVariables. We can start out by asking users to create and pass in a Netlify personal access token, and if we want to improve on that, register a Netlify App in future?

seancdavis commented 2 months ago

Interesting! That could definitely work, although I agree it sounds like a big lift. Not sure there's a great workaround for what you're trying to accomplish.

We can start out by asking users to create and pass in a Netlify personal access token, and if we want to improve on that, register a Netlify App in future?

Great idea – definitely a possibility.


Sounds like you have a theoretical path forward, just need to find the time to build it out?

gr2m commented 2 months ago

Thank you for confirming from your side that my plans are sound

Sounds like you have a theoretical path forward, just need to find the time to build it out?

Yes. Not sure when that'd be right now. But if anyone from @netlify would be interested in collaborating I can prioritize it.

seancdavis commented 2 months ago

Sounds great, @gr2m! We'll be chatting internally over the next week or so (have some folks OOO right now) and will get back to you shortly.

seancdavis commented 2 months ago

Checking back in, @gr2m β€” we don't have the engineers to put on this right now. But we can certainly consult as you make progress. Ping me once you get some time to start digging in!