From 94bbe4796cf6ee7f2f261a4b75a0d725dffdc6a9 Mon Sep 17 00:00:00 2001 From: Chris Rebert Date: Mon, 19 Oct 2015 13:56:34 -0700 Subject: Fix #25 --- README.md | 2 + src/main/resources/application.conf | 1 + .../getbootstrap/savage/github/util/package.scala | 1 + .../savage/server/PullRequestEventHandler.scala | 68 ++++++++++++---------- .../com/getbootstrap/savage/server/Settings.scala | 2 + 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") -- cgit v1.2.3