Before I get into the nitty-gritty, a brief recap of how things are working before any of the changes described in this article …
A Linaro static website consists of one or more git repositories, with potentially one being hosted as a private repository on Linaro’s BitBucket server and the others being hosted on GitHub as public repositories. Bamboo, the CI/CD tool chosen by Linaro’s IT Services to build the sites, monitors these repositories for changes and, when a change is identified, it runs the build plan for the web site associated with the changed repositories. If the build plan is successful, the staging or production web site gets updated, depending on which branch of the repository has been updated (develop or master, respectively).
All well and good but it does mean that if someone commits to a repository a breaking change (e.g. a broken link or some malformed YAML) then no other updates can be made to that website until that specific problem has been resolved.
To solve this required several changes being made that, together, helped to ensure that breaking changes couldn’t end up in the develop or master branches unless someone broke the rules by bypassing the protection. The changes we made were:
Using pull requests to carry out peer reviews of changes before they got committed into the develop or master branch.
Getting GitHub to trigger a custom build in Bamboo so that the proposed changes were used to drive a “test” build in Bamboo, thereby assisting the peer review by showing whether or not the test build would actually be successful.
Using branch protection rules in GitHub to enforce requirements such as needing the tests to succeed and needing code reviews.
Pull requests are not a native part of the git toolset but they have been implemented by a number of the git hosting platforms like GitHub, GitLab, BitBucket and others. They may vary in the approach taken but, essentially, one or more people are asked to look at the differences between the incoming changes and the existing files to see if anything wrong can be identified.
That, in itself, can be a laborious and not always successful process at spotting problems which is why there is an increasing use of automation to assist. GitHub’s approach is to have webhooks or apps trigger an external activity that might perform some testing and then report back on the results.
We opted to use webhooks to get GitHub to trigger the custom builds in Bamboo. They are called custom builds because one or more Bamboo variables are explicitly defined in order to change the behaviour of the build plan. I’ll talk more about them in a subsequent article.
The final piece of the puzzle was implementing branch protection rules. I’ve linked to the GitHub documentation above but I’ll pick out the key rules we’ve used:
Require pull request reviews before merging. When enabled, all commits must be made to a non-protected branch and submitted via a pull request with the required number of approving reviews.
Require status checks to pass before merging. Choose which status checks must pass before branches can be merged into a branch that matches this rule.
There is a further option that has been tried in the past which is “Include administrators”. This enforces all configured restrictions for administrators. Unfortunately, too many of the administrators have pushed back against this (normally because of the pull request review requirement) so we tend to leave it turned off now. That isn’t to say, though, that administrators get a “free ride”. If a pull request requires a review, an administrator can merge the pull request but GitHub doesn’t make it too easy:
Clicking on Merge pull request, highlighted in “warning red”, results in the expected merge dialog but with extra red bits:
So an administrator does have to tick the box to say they are aware they are using their admin privilege, after which step they can then complete the merge:
If an administrator pushes through a pull request that doesn’t build then they are in what I describe as the “you broke it, you fix it” scenario. After all, the protections are there for a good reason 😊.