taskcluster / taskcluster-tools

Tools for debugging, inspecting and managing Taskcluster
https://tools.taskcluster.net/
Mozilla Public License 2.0
26 stars 91 forks source link

Add Heroku GitHub integration #589

Closed helfi92 closed 5 years ago

helfi92 commented 5 years ago

I added the following env vars in Heroku https://github.com/taskcluster/taskcluster-tools/blob/46b8ccb2f209f4dc7a5ea667be29d0233a9b82b5/.env. I did however notice there were 2 additional variables only defined in the Travis site, namely AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY. I don't see them being used in the repository. Do we need them?

Once this is merged and things look ok, we should enable automatic deploys from Heroku.

Relates to #587.

eliperelman commented 5 years ago

One reason this was moved from Heroku to Travis was to not pay the cost of 2 builds per deploy. Meaning from a merge to production could take upwards of 30 minutes.

helfi92 commented 5 years ago

@eliperelman Which 2 builds are you referring to? This PR removes the build from Travis.

eliperelman commented 5 years ago

The settings in Heroku, in the past, are to not deploy a master branch until the build passes in CI. This is to keep broken builds from being deployed to production. This means the master branch builds in both Travis and Heroku.

eliperelman commented 5 years ago

I guess I'm wondering why you would want to remove Travis and risk deploying broken builds.

helfi92 commented 5 years ago

This is to keep broken builds from being deployed to production

I thought deploys won't happen if yarn build exits with a non-zero code. I guess I was wrong. I'll add back Travis.

@eliperelman We are switching the Mozilla Heroku team to have SSO login and so there are extra security guidelines to follow to generate non-expiry access tokens (like regenerating the access token). Given that taskcluster-tools is going away soon, we thought we should just stop deploying from Travis even if there's a performance penalty to avoid having to generate a new access token. The new process at Mozilla requires creating a new service account to manage the app for non-expiry tokens.

djmitche commented 5 years ago

I'm for whatever is quickest here to keep the focus on tc-web. If that means we run a risk of deploying something with a broken build, that's OK by me -- I think it's rare to click "merge" on a red PR anyway. And, since we don't have any testing, there's not much to prevent shipping a "broken" master anyway, outside of "so broken that yarn build fails" :)

helfi92 commented 5 years ago

@djmitche I noticed there were 2 additional variables only defined in the Travis site, namely AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY. I don't see them being used in the repository. Do we need them? I wonder why they were there in the first place.

djmitche commented 5 years ago

We used to upload the built site from Travis to an S3 bucket, so those were probably in place to support that upload. If you look up the Access Key ID in the IAM console, you can probably find the relevant IAM user and delete it.