Merge pull request #93 from tomaslin/git-trigger-filter-branch

allow git triggers to be filtered by branch
spinnaker · Jun 24, 2016 · 7538d78 · 7538d78
2 parents da850be + d055bb5
commit 7538d78
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 31 deletions.
diff --git a/echo-model/src/main/groovy/com/netflix/spinnaker/echo/model/trigger/GitEvent.groovy b/echo-model/src/main/groovy/com/netflix/spinnaker/echo/model/trigger/GitEvent.groovy
@@ -40,5 +40,10 @@ class GitEvent extends TriggerEvent {
   String getHash() {
     return content.hash
   }
+
+  @JsonIgnore
+  String getBranch() {
+    return content.branch
+  }
 }
 
diff --git a/echo-model/src/main/java/com/netflix/spinnaker/echo/model/Trigger.java b/echo-model/src/main/java/com/netflix/spinnaker/echo/model/Trigger.java
@@ -27,7 +27,7 @@
 @JsonDeserialize(builder = Trigger.TriggerBuilder.class)
 @Builder
 @Wither
-@ToString(of = {"type", "master", "job", "cronExpression", "source", "project", "slug", "account", "repository", "tag", "constraints"}, includeFieldNames = false)
+@ToString(of = {"type", "master", "job", "cronExpression", "source", "project", "slug", "account", "repository", "tag", "constraints", "branch"}, includeFieldNames = false)
 @Value
 public class Trigger {
   public enum Type {
@@ -66,22 +66,26 @@ public String toString() {
   String tag;
   String digest;
   Map constraints;
-
+  String branch;
 
   public Trigger atBuildNumber(final int buildNumber) {
-    return new Trigger(enabled, id, type, master, job, buildNumber, propertyFile, cronExpression, source, project, slug, null, account, repository, null, digest, null);
+    return new Trigger(enabled, id, type, master, job, buildNumber, propertyFile, cronExpression, source, project, slug, null, account, repository, null, digest, null, branch);
   }
 
   public Trigger atHash(final String hash) {
-    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, hash, account, repository, null, digest, null);
+    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, hash, account, repository, null, digest, null, branch);
+  }
+
+  public Trigger atBranch(final String branch) {
+    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, hash, account, repository, null, digest, null, branch);
   }
 
   public Trigger atTag(final String tag) {
-    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, null, account, repository, tag, digest, null);
+    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, null, account, repository, tag, digest, null, branch);
   }
 
   public Trigger atConstraints(final Map constraints) {
-    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, null, account, repository, tag, null, constraints);
+    return new Trigger(enabled, id, type, master, job, null, propertyFile, cronExpression, source, project, slug, null, account, repository, tag, null, constraints, branch);
   }
 
   @JsonPOJOBuilder(withPrefix = "")

diff --git a/...rs/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/monitor/GitEventMonitor.java b/...rs/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/monitor/GitEventMonitor.java
@@ -89,13 +89,18 @@ protected Predicate<Trigger> matchTriggerFor(final TriggerEvent event) {
     String source = gitEvent.getDetails().getSource();
     String project = gitEvent.getContent().getRepoProject();
     String slug = gitEvent.getContent().getSlug();
-    return trigger -> trigger.getType().equals(GIT_TRIGGER_TYPE) && trigger.getSource().equals(source) && trigger.getProject().equals(project) && trigger.getSlug().equals(slug);
+    String branch = gitEvent.getContent().getBranch();
+    return trigger -> trigger.getType().equals(GIT_TRIGGER_TYPE)
+      && trigger.getSource().equals(source)
+      && trigger.getProject().equals(project)
+      && trigger.getSlug().equals(slug)
+      && (trigger.getBranch() == null || trigger.getBranch().equals("") || matchesPattern(branch, trigger.getBranch()));
   }
 
   @Override
   protected Function<Trigger, Pipeline> buildTrigger(Pipeline pipeline, TriggerEvent event) {
     GitEvent gitEvent = (GitEvent) event;
-    return trigger -> pipeline.withTrigger(trigger.atHash(gitEvent.getHash()));
+    return trigger -> pipeline.withTrigger(trigger.atHash(gitEvent.getHash()).atBranch(gitEvent.getBranch()));
   }
 
   @Override

diff --git a/...ers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/monitor/TriggerMonitor.java b/...ers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/monitor/TriggerMonitor.java
@@ -34,6 +34,8 @@
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Triggers pipelines on _Orca_ when a trigger-enabled build completes successfully.
@@ -59,6 +61,12 @@ public TriggerMonitor(@NonNull Action1<Pipeline> subscriber,
     this.registry = registry;
   }
 
+  protected boolean matchesPattern(String s, String pattern) {
+    Pattern p = Pattern.compile(pattern);
+    Matcher m = p.matcher(s);
+    return m.matches();
+  }
+
   protected void onEchoResponse(final TriggerEvent event) {
     registry.gauge("echo.events.per.poll", 1);
   }

diff --git a/...rs/src/test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/GitEventMonitorSpec.groovy b/...rs/src/test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/GitEventMonitorSpec.groovy
@@ -57,8 +57,8 @@ class GitEventMonitorSpec extends Specification implements RetrofitStubs {
     })
 
     where:
-    event              | trigger             | triggerType
-    createGitEvent()   | enabledStashTrigger | 'stash'
+    event            | trigger
+    createGitEvent() | enabledStashTrigger
   }
 
   def "attaches stash trigger to the pipeline"() {
@@ -115,9 +115,9 @@ class GitEventMonitorSpec extends Specification implements RetrofitStubs {
     0 * subscriber._
 
     where:
-    trigger                                 | description
-    disabledStashTrigger                    | "disabled stash trigger"
-    nonJenkinsTrigger                       | "non-Jenkins"
+    trigger              | description
+    disabledStashTrigger | "disabled stash trigger"
+    nonJenkinsTrigger    | "non-Jenkins"
 
     pipeline = createPipelineWith(trigger)
     event = createGitEvent()
@@ -140,6 +140,7 @@ class GitEventMonitorSpec extends Specification implements RetrofitStubs {
     enabledStashTrigger.withSlug("notSlug")       | "different slug"
     enabledStashTrigger.withSource("github")      | "different source"
     enabledStashTrigger.withProject("notProject") | "different project"
+    enabledStashTrigger.withBranch("notMaster")   | "different branch"
 
     pipeline = createPipelineWith(trigger)
     event = createGitEvent()
@@ -149,7 +150,6 @@ class GitEventMonitorSpec extends Specification implements RetrofitStubs {
   def "does not trigger a pipeline that has an enabled stash trigger with missing #field"() {
     given:
     pipelineCache.getPipelines() >> [badPipeline, goodPipeline]
-    println objectMapper.writeValueAsString(createGitEvent())
 
     when:
     monitor.processEvent(objectMapper.convertValue(event, Event))
@@ -167,4 +167,51 @@ class GitEventMonitorSpec extends Specification implements RetrofitStubs {
     goodPipeline = createPipelineWith(enabledStashTrigger)
     badPipeline = createPipelineWith(trigger)
   }
+
+  @Unroll
+  def "triggers events on branch when #description"() {
+    given:
+    def gitEvent = createGitEvent()
+    gitEvent.content.branch = eventBranch
+    def trigger = enabledStashTrigger.atBranch(triggerBranch)
+    def pipeline = createPipelineWith(trigger)
+    pipelineCache.getPipelines() >> [pipeline]
+
+    when:
+    monitor.processEvent(objectMapper.convertValue(gitEvent, Event))
+
+    then:
+    1 * subscriber.call({
+      it.application == pipeline.application && it.name == pipeline.name
+    })
+
+    where:
+    eventBranch         | triggerBranch       | description
+    'whatever'          | null                | 'no branch set in trigger'
+    'whatever'          | ""                  | 'empty string in trigger'
+    'master'            | 'master'            | 'branches are identical'
+    'ref/origin/master' | 'ref/origin/master' | 'branches have slashes'
+    'regex12345'        | 'regex.*'           | 'branches match pattern'
+  }
+
+  @Unroll
+  def "does not triggers events on branch on mistmatch branch"() {
+    given:
+    def gitEvent = createGitEvent()
+    gitEvent.content.branch = eventBranch
+    def trigger = enabledStashTrigger.atBranch(triggerBranch)
+    def pipeline = createPipelineWith(trigger)
+    pipelineCache.getPipelines() >> [pipeline]
+
+    when:
+    monitor.processEvent(objectMapper.convertValue(gitEvent, Event))
+
+    then:
+    0 * subscriber._
+
+    where:
+    eventBranch  | triggerBranch
+    'master'     | 'featureBranch'
+    'regex12345' | 'not regex.*'
+  }
 }
diff --git a/echo-pipelinetriggers/src/test/groovy/com/netflix/spinnaker/echo/test/RetrofitStubs.groovy b/echo-pipelinetriggers/src/test/groovy/com/netflix/spinnaker/echo/test/RetrofitStubs.groovy
@@ -20,15 +20,15 @@ import static retrofit.RetrofitError.httpError
 trait RetrofitStubs {
 
   final String url = "http://echo"
-  final Trigger enabledJenkinsTrigger = new Trigger(true, null, 'jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null)
-  final Trigger disabledJenkinsTrigger = new Trigger(false, null, 'jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null)
-  final Trigger nonJenkinsTrigger = new Trigger(true, null, 'not jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null)
-  final Trigger enabledStashTrigger = new Trigger(true, null, 'git', null, null, null, null, null, 'stash', 'project', 'slug', null, null, null, null, null, null)
-  final Trigger disabledStashTrigger = new Trigger(false, null, 'git', 'master', 'job', null, null, null, 'stash', 'project', 'slug', null, null, null, null, null, null)
-  final Trigger enabledDockerTrigger = new Trigger(true, null, 'docker', null, null, null, null, null, null, null, null, null, 'registry', 'repository', 'tag', null, null)
-  final Trigger disabledDockerTrigger = new Trigger(false, null, 'git', null, null, null, null, null, null, null, null, null, 'registry', 'repository', 'tag', null, null)
-  final Trigger enabledWebhookTrigger = new Trigger(true, null, 'webhook', null, null, null, null, null, null, null, null, null, null, null, null, null, null)
-  final Trigger disabledWebhookTrigger = new Trigger(false, null, 'webhook', null, null, null, null, null, null, null, null, null, null, null, null, null, null)
+  final Trigger enabledJenkinsTrigger = new Trigger(true, null, 'jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null, null)
+  final Trigger disabledJenkinsTrigger = new Trigger(false, null, 'jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null,null)
+  final Trigger nonJenkinsTrigger = new Trigger(true, null, 'not jenkins', 'master', 'job', null, null, null, null, null, null, null, null, null, null, null, null,null)
+  final Trigger enabledStashTrigger = new Trigger(true, null, 'git', null, null, null, null, null, 'stash', 'project', 'slug', null, null, null, null, null, null,null)
+  final Trigger disabledStashTrigger = new Trigger(false, null, 'git', 'master', 'job', null, null, null, 'stash', 'project', 'slug', null, null, null, null, null, null,null)
+  final Trigger enabledDockerTrigger = new Trigger(true, null, 'docker', null, null, null, null, null, null, null, null, null, 'registry', 'repository', 'tag', null, null,null)
+  final Trigger disabledDockerTrigger = new Trigger(false, null, 'git', null, null, null, null, null, null, null, null, null, 'registry', 'repository', 'tag', null, null,null)
+  final Trigger enabledWebhookTrigger = new Trigger(true, null, 'webhook', null, null, null, null, null, null, null, null, null, null, null, null, null, null,null)
+  final Trigger disabledWebhookTrigger = new Trigger(false, null, 'webhook', null, null, null, null, null, null, null, null, null, null, null, null, null, null,null)
   final Trigger nonWebhookTrigger = Trigger.builder().enabled(true).type('not webhook').build()
   final Trigger webhookTriggerWithConstraints = Trigger.builder().enabled(true).type('webhook').constraints([ "application": "myApplicationName", "pipeline": "myPipeLineName" ]).build()
   final Trigger webhookTriggerWithoutConstraints = Trigger.builder().enabled(true).type('webhook').constraints().build()

diff --git a/.../netflix/spinnaker/echo/scheduler/actions/pipeline/PipelineConfigsPollingAgentSpec.groovy b/.../netflix/spinnaker/echo/scheduler/actions/pipeline/PipelineConfigsPollingAgentSpec.groovy
@@ -36,7 +36,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'when a new pipeline trigger is added, a scheduled action instance is registered with an id same as the trigger id'() {
         given:
-        Trigger trigger = new Trigger(true, null, 'cron', null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, null, 'cron', null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         pipelineCache.getPipelines() >> [pipeline]
         actionsOperator.getActionInstances() >> []
@@ -54,7 +54,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'when an existing pipeline trigger is disabled, corresponding scheduled action is also disabled'() {
         given:
-        Trigger trigger = new Trigger(false, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(false, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         ActionInstance actionInstance = buildScheduledAction(trigger.id, '* 0/30 * * * ? *', true)
         pipelineCache.getPipelines() >> [pipeline]
@@ -74,7 +74,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'when an existing disabled pipeline trigger is enabled, corresponding scheduled action is also enabled'() {
         given:
-        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         ActionInstance actionInstance = buildScheduledAction(trigger.id, '* 0/30 * * * ? *', false)
         pipelineCache.getPipelines() >> [pipeline]
@@ -111,7 +111,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'when an existing pipeline trigger is updated, corresponding scheduled action is also updated'() {
         given:
-        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/45 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/45 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         ActionInstance actionInstance = buildScheduledAction('t1', '* 0/30 * * * ? *', true)
         pipelineCache.getPipelines() >> [pipeline]
@@ -130,7 +130,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'when an existing pipeline trigger is updated but is still disabled, corresponding scheduled action is NOT updated'() {
         given:
-        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/45 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/45 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         ActionInstance actionInstance = buildScheduledAction('t1', '* 0/30 * * * ? *', false)
         pipelineCache.getPipelines() >> [pipeline]
@@ -149,7 +149,7 @@ class PipelineConfigsPollingAgentSpec extends Specification {
 
     void 'with no changes to pipeline trigger, no scheduled actions are updated for that pipeline'() {
         given:
-        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, 't1', Trigger.Type.CRON.toString(), null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null, null)
         Pipeline pipeline = buildPipeline([trigger])
         ActionInstance actionInstance = buildScheduledAction('t1', '* 0/30 * * * ? *', true)
         pipelineCache.getPipelines() >> [pipeline]

diff --git a/...tflix/spinnaker/echo/scheduler/actions/pipeline/PipelineTriggerActionConverterSpec.groovy b/...tflix/spinnaker/echo/scheduler/actions/pipeline/PipelineTriggerActionConverterSpec.groovy
@@ -41,7 +41,7 @@ class PipelineTriggerActionConverterSpec extends Specification {
 
     void 'toParameters() should return an equivalent map of parameters'() {
         setup:
-        Trigger trigger = new Trigger(true, '123-456', 'cron', null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, '123-456', 'cron', null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null, null)
 
         when:
         Map parameters = PipelineTriggerConverter.toParameters(pipeline, trigger, 'America/New_York')
@@ -84,7 +84,7 @@ class PipelineTriggerActionConverterSpec extends Specification {
 
     void 'toScheduledAction() should return an equivalent valid ActionInstance'() {
         setup:
-        Trigger trigger = new Trigger(true, '123-456', 'cron', null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
+        Trigger trigger = new Trigger(true, '123-456', 'cron', null, null, null, null, null, '* 0/30 * * * ? *', null, null, null, null, null, null, null, null, null)
 
         when:
         ActionInstance actionInstance = PipelineTriggerConverter.toScheduledAction(pipeline, trigger, 'America/Los_Angeles')