Merge pull request #27 from twbs/fix-25

Fix #25

5 files changed, 42 insertions, 32 deletions

diff --git a/README.md b/README.md
index d058664..7573094 100644
--- a/README.md
+++ b/README.md
@@ -82,6 +82,8 @@ savage {
     github-test-repo = "twbs/bootstrap-tests"
     // Ignore pull requests whose branch is from the watched repo (and is thus from a project team member)
     ignore-branches-from-watched-repo = true
+    // Pull requests must target one of these branches in the watched repo
+    allowed-base-branches = [ "master" ]
     // List of GitHub organization names whose users Savage should trust to authorize retries of builds
     trusted-orgs = [ "twbs" ]
     // List of Unix file globs constituting the whitelist of safely editable files
diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf
index c05219e..9af89ec 100644
--- a/src/main/resources/application.conf
+++ b/src/main/resources/application.conf
@@ -29,6 +29,7 @@ savage {
     github-repo-to-watch = "twbs/bootstrap"
     github-test-repo = "twbs-savage/bootstrap"
     ignore-branches-from-watched-repo = true
+    allowed-base-branches = [ "master", "v4-dev", "v3-dev" ]
     trusted-orgs = [ "twbs" ]
     whitelist = [
         "**.md",
diff --git a/src/main/scala/com/getbootstrap/savage/github/util/package.scala b/src/main/scala/com/getbootstrap/savage/github/util/package.scala
index fea3cdb..c62f5e8 100644
--- a/src/main/scala/com/getbootstrap/savage/github/util/package.scala
+++ b/src/main/scala/com/getbootstrap/savage/github/util/package.scala
@@ -18,6 +18,7 @@ package object util {
   }
   implicit class RichPullRequestMarker(marker: PullRequestMarker) {
     def commitSha: CommitSha = CommitSha(marker.getSha).getOrElse{ throw new IllegalStateException(s"Invalid commit SHA: ${marker.getSha}") }
+    def branch: Option[Branch] = Branch(marker.getRef)
   }
   implicit class RichCommitFile(file: CommitFile) {
 
diff --git a/src/main/scala/com/getbootstrap/savage/server/PullRequestEventHandler.scala b/src/main/scala/com/getbootstrap/savage/server/PullRequestEventHandler.scala
index 746a8fa..904e7a2 100644
--- a/src/main/scala/com/getbootstrap/savage/server/PullRequestEventHandler.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/PullRequestEventHandler.scala
@@ -89,48 +89,52 @@ class PullRequestEventHandler(
       destinationRepo match {
         case None => log.error(s"Received event from GitHub about irrelevant repository with unsafe name")
         case Some(settings.MainRepoId) => {
-          val destBranch = bsBase.getRef
-          destBranch match {
-            case "master" => {
-              prHead.getRepo.repositoryId match {
-                case None => log.error(s"Received event from GitHub about repository with unsafe name")
-                case Some(settings.MainRepoId) if settings.IgnoreBranchesFromMainRepo => log.info("Ignoring PR whose branch is from the main repo, per settings.")
-                case Some(foreignRepo) => {
-                  val baseSha = bsBase.commitSha
-                  val headSha = prHead.commitSha
+          bsBase.branch match {
+            case None => logPrInfo(s"Ignoring since PR targets the unsafely-named ${bsBase.getRef} branch")
+            case Some(destBranch) => {
+              if (settings.AllowedBaseBranches.contains(destBranch)) {
+                prHead.getRepo.repositoryId match {
+                  case None => log.error(s"Received event from GitHub about repository with unsafe name")
+                  case Some(settings.MainRepoId) if settings.IgnoreBranchesFromMainRepo => log.info("Ignoring PR whose branch is from the main repo, per settings.")
+                  case Some(foreignRepo) => {
+                    val baseSha = bsBase.commitSha
+                    val headSha = prHead.commitSha
 
-                  affectedFilesFor(foreignRepo, baseSha, headSha) match {
-                    case Failure(exc) => {
-                      log.error(exc, s"Could not get affected files for commits ${baseSha}...${headSha} for ${foreignRepo}")
-                    }
-                    case Success(affectedFiles) => {
-                      log.debug("Files affected by {}: {}", prNum, affectedFiles)
-                      if (areSafe(affectedFiles)) {
-                        if (areInteresting(affectedFiles)) {
-                          logPrInfo(s"Requesting build for safe & interesting PR")
-                          pusher ! PullRequestPushRequest(
-                            origin = foreignRepo,
-                            number = pr.number,
-                            commitSha = headSha
-                          )
-                          statusSetter ! StatusForCommit(
-                            status = commit_status.Pending("Savage has initiated its special separate Travis CI build"),
-                            commit = headSha
-                          )
+                    affectedFilesFor(foreignRepo, baseSha, headSha) match {
+                      case Failure(exc) => {
+                        log.error(exc, s"Could not get affected files for commits ${baseSha}...${headSha} for ${foreignRepo}")
+                      }
+                      case Success(affectedFiles) => {
+                        log.debug("Files affected by {}: {}", prNum, affectedFiles)
+                        if (areSafe(affectedFiles)) {
+                          if (areInteresting(affectedFiles)) {
+                            logPrInfo(s"Requesting build for safe & interesting PR")
+                            pusher ! PullRequestPushRequest(
+                              origin = foreignRepo,
+                              number = pr.number,
+                              commitSha = headSha
+                            )
+                            statusSetter ! StatusForCommit(
+                              status = commit_status.Pending("Savage has initiated its special separate Travis CI build"),
+                              commit = headSha
+                            )
+                          }
+                          else {
+                            logPrInfo(s"Ignoring PR with no interesting file changes")
+                          }
                         }
                         else {
-                          logPrInfo(s"Ignoring PR with no interesting file changes")
+                          logPrInfo(s"Ignoring PR with unsafe file changes")
                         }
                       }
-                      else {
-                        logPrInfo(s"Ignoring PR with unsafe file changes")
-                      }
                     }
                   }
                 }
               }
+              else {
+                logPrInfo(s"Ignoring since PR targets the ${destBranch} branch")
+              }
             }
-            case _ => logPrInfo(s"Ignoring since PR targets the ${destBranch} branch")
           }
         }
         case Some(otherRepo) => log.error(s"Received event from GitHub about irrelevant repository: ${otherRepo}")
diff --git a/src/main/scala/com/getbootstrap/savage/server/Settings.scala b/src/main/scala/com/getbootstrap/savage/server/Settings.scala
index d01c62d..a5e2020 100644
--- a/src/main/scala/com/getbootstrap/savage/server/Settings.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/Settings.scala
@@ -10,6 +10,7 @@ import akka.actor.ExtensionIdProvider
 import akka.actor.ExtendedActorSystem
 import akka.util.ByteString
 import org.eclipse.egit.github.core.RepositoryId
+import com.getbootstrap.savage.github.Branch
 import com.getbootstrap.savage.util.{FilePathWhitelist,FilePathWatchlist,Utf8String,RichConfig}
 
 class SettingsImpl(config: Config) extends Extension {
@@ -26,6 +27,7 @@ class SettingsImpl(config: Config) extends Extension {
   val Watchlist: FilePathWatchlist = new FilePathWatchlist(config.getStringList("savage.file-watchlist").asScala)
   val BranchPrefix: String = config.getString("savage.branch-prefix")
   val IgnoreBranchesFromMainRepo: Boolean = config.getBoolean("savage.ignore-branches-from-watched-repo")
+  val AllowedBaseBranches: Set[Branch] = config.getStringList("savage.allowed-base-branches").asScala.flatMap{ Branch(_) }.toSet
   val TrustedOrganizations: Set[String] = config.getStringList("savage.trusted-orgs").asScala.toSet
   val SetCommitStatus: Boolean = config.getBoolean("savage.set-commit-status")
   val TravisTimeout: FiniteDuration = config.getFiniteDuration("savage.travis-timeout")