Rewrite server deploy script #1793

paulmelnikow · 2018-07-22T13:39:36Z

~~I haven't run this yet but I will do that when I run the deploy later today. Would appreciate an "eyeballs" review first!~~

To run this requires renaming private/secret.json to private/secret-production.json in the working tree used for deployment.

Goals:

Ensure production secrets are not used in development
Avoid modifying the current working tree
Avoid branch switching: make sure the current ref gets deployed
- If something other than master is deployed, leave HEAD alone; don't reset to master
Ensure the build runs before server deploy (Accessing the website on a specific server has link breakage #1941)

This makes use of Git working trees, which is a relatively new but stable feature in Git. I was initially reluctant to use git worktree, mostly because I don't like adding new tooling that isn't necessary. The other alternative I experimented with was copying or re-cloning to an entirely separate working copy. This was messier and more brittle than using git worktree.

cc @espadrine

This requires renaming `private/secret.json` to `private/secret-production.json` in the working tree used for deployment.

Makefile

espadrine · 2018-07-22T14:34:26Z

Makefile

+	cp private/secret-production.json ${SERVER_TMP}/private/secret.json
+	cp Verdana.ttf ${SERVER_TMP}
+	git -C ${SERVER_TMP} add private/secret.json Verdana.ttf
+	git -C ${SERVER_TMP} commit -m '[DEPLOY] MUST NOT BE ON GITHUB'


That is a significant change in behavior. It feels like it does more than just use secret-production.json instead of secret.json. Could you explain this change further please?

Without a working tree, the solution as far as I can see is to:

Back up the existing private/secret.json, if it exists

Copy private/secret-production.json to private/secret.json

Check it in

Restore the original private/secret.json afterward

That has more moving parts than I'd like, seems brittle, and also seems like it could result in accidental data loss.

By moving the branch manipulation into a separate working tree, private/secret.json can be copied for deployment without needing to preserve the original.

The other change is how the branching is handled, which I'll respond to below.

espadrine · 2018-07-22T14:38:46Z

Makefile

-	git checkout master
+	rm -rf ${SERVER_TMP}
+	git worktree prune
+	git worktree add -B production ${SERVER_TMP}


The use of git worktree feels a bit like tempting the devil. The use of -B in particular risks losing information without realizing.

In general, I prefer having deployment procedures so dumb that it takes a lot for them to go wrong, and when they do, they are dumb enough that it is simple to fix them.

Yes, I agree that deployment procedures should be as easy to reason about as possible. If they fail, they should fail in very safe ways.

To that end, I'd like to avoid modifying the existing working tree during deployment. I'd also like to avoid branch switching to make sure the current ref gets deployed. In the current process, if two servers are deployed at once, I believe the second one will get master. I'd also like to deploy the exact same front-end build to each server. There's no need to build it again and it's one less thing to reason about.

When I was working on #1774 I was initially reluctant to use git worktree, mostly because I don't like adding new tooling that isn't necessary. The other alternative I experimented with was copying or re-cloning to an entirely separate working copy. This was messier and more brittle than using git worktree.

I'm not thrilled with using -B. I took it from the github pages deploy, which starts with a git checkout -B gh-pages.

Before my change, the deploy commits happen on a detached head. Nothing is deleted in the process, however it's deleted right afterward. Without a branch, it's difficult to inspect what was pushed to the server. I don't like deleting data. However, it is not surprising that a deploy process would clobber a branch named production. I'd rather reserve a branch for the production deploy work than do it all on a detached head that gets tossed right after it runs.

Would you be more comfortable using a name that was really explicit like server-deploy-working-branch?

#1941 is another issue with the current scripts. git worktree has been working more reliably for deploy-gh-pages than the current deploy is for the servers.

paulmelnikow · 2018-07-22T15:33:20Z

The use of git worktree feels a bit like tempting the devil. The use of -B in particular risks losing information without realizing.

In general, I prefer having deployment procedures so dumb that it takes a lot for them to go wrong, and when they do, they are dumb enough that it is simple to fix them.

Yes, the deploy process should be safe, simple, and reliable. Using worktree makes it safer, by not doing the work inside a tree, and in a way throws away less because the deploy branch sticks around after the deploy is done. It also lets you push the same build to each machine rather than running a new build for each one.

Let's not rush this discussion; I'll circle back.

shields-ci · 2018-07-22T21:29:58Z

	Messages
📖	✨ Thanks for your contribution to Shields, @paulmelnikow!

Generated by 🚫 dangerJS

paulmelnikow · 2018-08-07T03:37:05Z

I can confirm that the current scripts, when aborted in the middle, leave a deploy commit hanging on the working copy. When I reset to last commit from master, Verdana.ttf, the server config, and the build are removed from the working copy.

Any thoughts about my comments?

paulmelnikow · 2018-10-27T16:53:06Z

I continue to encounter issues where I accidentally run things using production credentials while deploying. I feel it's necessary to move forward with this. I will update it in light of #2178 and then it would be great if someone would please give a cursory 👍to merge it.

# Conflicts: # .prettierignore

chris48s

Accepting that I can't test this and haven't run it, I've had a read through the code and I can't see anything that looks obviously wrong or buggy. If this is the process you are actually using to deploy the site, I think it makes sense for the repo to reflect reality in this respect.

As an additional risk mitigation, do you think we should set up any branch names used in deployment with a branch protection rule in https://github.com/badges/shields/settings/branches to prevent us accidentally pushing it to GH?

chris48s · 2018-10-27T21:33:36Z

This shouldn't be a blocker for this PR, but for the front-end part of the build/deploy, have you ever looked at this package: https://github.com/tschaub/gh-pages ?

paulmelnikow · 2018-10-29T14:42:23Z

As an additional risk mitigation, do you think we should set up any branch names used in deployment with a branch protection rule in https://github.com/badges/shields/settings/branches to prevent us accidentally pushing it to GH?

It's not a bad idea. I just added branch protection for server-deploy-working-branch – see #1793 (comment) – and I'll update this to use the same.

paulmelnikow · 2018-10-29T14:47:08Z

Okay, this is updated.

I'll re-test it on a real deploy before merging.

Added: Done!

shields-deployment · 2018-10-30T21:08:56Z

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

Ensure production secrets are not used in development

7ab6b5d

This requires renaming `private/secret.json` to `private/secret-production.json` in the working tree used for deployment.

paulmelnikow added the operations Hosting, monitoring, and reliability for the production badge servers label Jul 22, 2018

paulmelnikow requested review from espadrine and chris48s July 22, 2018 13:39

paulmelnikow had a problem deploying to shields-staging-pr-1793 July 22, 2018 13:39 Failure

espadrine reviewed Jul 22, 2018

View reviewed changes

Merge branch 'master' into deploy-server

3f865d2

paulmelnikow temporarily deployed to shields-staging-pr-1793 July 22, 2018 21:28 Inactive

Add new targets to phony

dd98aa4

paulmelnikow temporarily deployed to shields-staging-pr-1793 July 22, 2018 21:34 Inactive

Merge branch 'master' into deploy-server

2fd5b85

paulmelnikow deployed to shields-staging-pr-1793 July 25, 2018 21:37 View deployment

Add to prettierignore

617ba94

dependabot-preview bot mentioned this pull request Aug 2, 2018

Bump eslint-plugin-react from 7.7.0 to 7.10.0 #1837

Merged

paulmelnikow mentioned this pull request Aug 22, 2018

Accessing the website on a specific server has link breakage #1941

Closed

tooomm mentioned this pull request Aug 29, 2018

Clean up some alerts detected by LGTM [pypi] #2010

Merged

paulmelnikow added 2 commits October 27, 2018 12:54

Merge branch 'master' into deploy-server

6eba3e2

# Conflicts: # .prettierignore

Disable pre-commit hooks during deploy

91919c5

paulmelnikow changed the title ~~Ensure production secrets are not used in development~~ Rewrite server deploy script Oct 27, 2018

chris48s previously approved these changes Oct 27, 2018

View reviewed changes

Merge branch 'master' into deploy-server

f51f37f

Use a protected branch

07dc2b5

paulmelnikow dismissed chris48s’s stale review via 07dc2b5 October 29, 2018 14:46

A couple fixes

5556ccf

chris48s approved these changes Oct 30, 2018

View reviewed changes

Merge branch 'master' into deploy-server

046d69e

paulmelnikow merged commit 94611fb into master Oct 30, 2018

paulmelnikow deleted the deploy-server branch October 30, 2018 21:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite server deploy script #1793

Rewrite server deploy script #1793

paulmelnikow commented Jul 22, 2018 •

edited

Loading

espadrine Jul 22, 2018

paulmelnikow Jul 22, 2018

espadrine Jul 22, 2018

paulmelnikow Jul 22, 2018

paulmelnikow Aug 22, 2018

paulmelnikow commented Jul 22, 2018

shields-ci commented Jul 22, 2018 •

edited

Loading

paulmelnikow commented Aug 7, 2018

paulmelnikow commented Oct 27, 2018

chris48s left a comment

chris48s commented Oct 27, 2018

paulmelnikow commented Oct 29, 2018

paulmelnikow commented Oct 29, 2018 •

edited

Loading

shields-deployment bot commented Oct 30, 2018

Rewrite server deploy script #1793

Rewrite server deploy script #1793

Conversation

paulmelnikow commented Jul 22, 2018 • edited Loading

espadrine Jul 22, 2018

Choose a reason for hiding this comment

paulmelnikow Jul 22, 2018

Choose a reason for hiding this comment

espadrine Jul 22, 2018

Choose a reason for hiding this comment

paulmelnikow Jul 22, 2018

Choose a reason for hiding this comment

paulmelnikow Aug 22, 2018

Choose a reason for hiding this comment

paulmelnikow commented Jul 22, 2018

shields-ci commented Jul 22, 2018 • edited Loading

paulmelnikow commented Aug 7, 2018

paulmelnikow commented Oct 27, 2018

chris48s left a comment

Choose a reason for hiding this comment

chris48s commented Oct 27, 2018

paulmelnikow commented Oct 29, 2018

paulmelnikow commented Oct 29, 2018 • edited Loading

shields-deployment bot commented Oct 30, 2018

paulmelnikow commented Jul 22, 2018 •

edited

Loading

shields-ci commented Jul 22, 2018 •

edited

Loading

paulmelnikow commented Oct 29, 2018 •

edited

Loading