From 53d268e7fcd43acb4222b9757c58defb4384bab3 Mon Sep 17 00:00:00 2001 From: mblevins4 Date: Thu, 30 Mar 2017 16:18:10 -0400 Subject: [PATCH 1/3] Add push event information as arguments to triggered jobs. --- .../cloudbees/jenkins/GitHubPushTrigger.java | 57 ++++++++++++++++++- .../com/cloudbees/jenkins/GitHubTrigger.java | 2 +- .../cloudbees/jenkins/GitHubTriggerEvent.java | 51 ++++++++++++++++- .../DefaultPushGHEventSubscriber.java | 4 ++ .../jenkins/GitHubPushTriggerTest.java | 26 ++++++++- .../DefaultPushGHEventListenerTest.java | 6 ++ 6 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java index 53033c12d..0d9991c73 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java @@ -8,9 +8,16 @@ import hudson.console.AnnotatedLargeText; import hudson.model.AbstractProject; import hudson.model.Action; +import hudson.model.CauseAction; import hudson.model.Item; import hudson.model.Job; +import hudson.model.ParameterDefinition; +import hudson.model.ParameterValue; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; import hudson.model.Project; +import hudson.model.StringParameterValue; +import hudson.model.queue.QueueTaskFuture; import hudson.triggers.SCMTrigger; import hudson.triggers.Trigger; import hudson.triggers.TriggerDescriptor; @@ -44,9 +51,11 @@ import java.net.MalformedURLException; import java.net.URL; import java.text.DateFormat; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.concurrent.Executors; @@ -79,10 +88,12 @@ public void onPost() { /** * Called when a POST is made. */ - public void onPost(String triggeredByUser) { + public void onPost(String triggeredByUser, String ref, String head) { onPost(GitHubTriggerEvent.create() .withOrigin(SCMEvent.originOf(Stapler.getCurrentRequest())) .withTriggeredByUser(triggeredByUser) + .withRef(ref) + .withHead(head) .build() ); } @@ -92,6 +103,8 @@ public void onPost(String triggeredByUser) { */ public void onPost(final GitHubTriggerEvent event) { final String pushBy = event.getTriggeredByUser(); + final String ref = event.getRef(); + final String head = event.getHead(); DescriptorImpl d = getDescriptor(); d.checkThreadPoolSizeAndUpdateIfNecessary(); d.queue.execute(new Runnable() { @@ -140,7 +153,23 @@ public void run() { LOGGER.warn("Failed to parse the polling log", e); cause = new GitHubPushCause(pushBy); } - if (asParameterizedJobMixIn(job).scheduleBuild(cause)) { + + // Get the parameters defined by the job, as we will be overriding certain values + List values = getDefaultParameters(); + Iterator it = values.iterator(); + while (it.hasNext()) { + ParameterValue pv = it.next(); + if (parameterIsOverridable(pv.getName())) { + it.remove(); + } + } + values.add(new StringParameterValue("ref", ref)); + values.add(new StringParameterValue("head", head)); + + // Schedule the job with the parameters and cause + QueueTaskFuture build = asParameterizedJobMixIn(job) + .scheduleBuild2(0, new ParametersAction(values), new CauseAction(cause)); + if (build != null) { LOGGER.info("SCM changes detected in " + job.getFullName() + ". Triggering #" + job.getNextBuildNumber()); } else { @@ -151,6 +180,30 @@ public void run() { }); } + /** + * Returns the default parameter values for the parameters of this job + */ + private List getDefaultParameters() { + ArrayList values = new ArrayList(); + ParametersDefinitionProperty pdp = this.job.getProperty(ParametersDefinitionProperty.class); + if (pdp != null) { + for (ParameterDefinition pd : pdp.getParameterDefinitions()) { + if (pd.getName().equals("sha1")) { + continue; + } + values.add(pd.getDefaultParameterValue()); + } + } + return values; + } + + /** + * Returns whether or not a given parameter WILL be overridden + */ + private static boolean parameterIsOverridable(String paramName) { + return "ref".equalsIgnoreCase(paramName) || "head".equalsIgnoreCase(paramName); + } + /** * Returns the file that records the last/current polling activity. */ diff --git a/src/main/java/com/cloudbees/jenkins/GitHubTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubTrigger.java index 9d44eb838..cc3372bfd 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubTrigger.java @@ -23,7 +23,7 @@ public interface GitHubTrigger { void onPost(); // TODO: document me - void onPost(String triggeredByUser); + void onPost(String triggeredByUser, String ref, String head); /** * Obtains the list of the repositories that this trigger is looking at. diff --git a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java index 25afa2f14..9cb434d04 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java @@ -22,11 +22,21 @@ public class GitHubTriggerEvent { * The user that the event was provided by. */ private final String triggeredByUser; + /** + * The user reference that the event affected + */ + private final String ref; + /** + * The head of the repo after the event + */ + private final String head; - private GitHubTriggerEvent(long timestamp, String origin, String triggeredByUser) { + private GitHubTriggerEvent(long timestamp, String origin, String triggeredByUser, String ref, String head) { this.timestamp = timestamp; this.origin = origin; this.triggeredByUser = triggeredByUser; + this.ref = ref; + this.head = head; } public static Builder create() { @@ -45,6 +55,14 @@ public String getTriggeredByUser() { return triggeredByUser; } + public String getRef() { + return ref; + } + + public String getHead() { + return head; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -62,7 +80,16 @@ public boolean equals(Object o) { if (origin != null ? !origin.equals(that.origin) : that.origin != null) { return false; } - return triggeredByUser != null ? triggeredByUser.equals(that.triggeredByUser) : that.triggeredByUser == null; + if (triggeredByUser != null ? !triggeredByUser.equals(that.triggeredByUser) : that.triggeredByUser != null) { + return false; + } + if (ref != null ? !ref.equals(that.ref) : that.ref != null) { + return false; + } + if (head != null ? !head.equals(that.head) : that.head != null) { + return false; + } + return true; } @Override @@ -70,6 +97,8 @@ public int hashCode() { int result = (int) (timestamp ^ (timestamp >>> 32)); result = 31 * result + (origin != null ? origin.hashCode() : 0); result = 31 * result + (triggeredByUser != null ? triggeredByUser.hashCode() : 0); + result = 31 * result + (ref != null ? ref.hashCode() : 0); + result = 31 * result + (head != null ? head.hashCode() : 0); return result; } @@ -79,6 +108,8 @@ public String toString() { + "timestamp=" + timestamp + ", origin='" + origin + '\'' + ", triggeredByUser='" + triggeredByUser + '\'' + + ", ref='" + ref + '\'' + + ", head='" + head + '\'' + '}'; } @@ -89,6 +120,8 @@ public static class Builder { private long timestamp; private String origin; private String triggeredByUser; + private String ref; + private String head; private Builder() { timestamp = System.currentTimeMillis(); @@ -109,8 +142,18 @@ public Builder withTriggeredByUser(String triggeredByUser) { return this; } + public Builder withRef(String ref) { + this.ref = ref; + return this; + } + + public Builder withHead(String head) { + this.head = head; + return this; + } + public GitHubTriggerEvent build() { - return new GitHubTriggerEvent(timestamp, origin, triggeredByUser); + return new GitHubTriggerEvent(timestamp, origin, triggeredByUser, ref, head); } @Override @@ -119,6 +162,8 @@ public String toString() { + "timestamp=" + timestamp + ", origin='" + origin + '\'' + ", triggeredByUser='" + triggeredByUser + '\'' + + ", ref='" + ref + '\'' + + ", head='" + head + '\'' + '}'; } } diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java b/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java index 203744bbb..5be879324 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java @@ -75,6 +75,8 @@ protected void onEvent(final GHSubscriberEvent event) { } URL repoUrl = push.getRepository().getUrl(); final String pusherName = push.getPusher().getName(); + final String ref = push.getRef(); + final String head = push.getHead(); LOGGER.info("Received PushEvent for {} from {}", repoUrl, event.getOrigin()); final GitHubRepositoryName changedRepository = GitHubRepositoryName.create(repoUrl.toExternalForm()); @@ -97,6 +99,8 @@ public void run() { .withTimestamp(event.getTimestamp()) .withOrigin(event.getOrigin()) .withTriggeredByUser(pusherName) + .withHead(head) + .withRef(ref) .build() ); } else { diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index 00a529c28..469b6ac1d 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -1,6 +1,9 @@ package com.cloudbees.jenkins; +import hudson.model.Action; +import hudson.model.CauseAction; import hudson.model.FreeStyleProject; +import hudson.model.ParametersAction; import hudson.plugins.git.GitSCM; import hudson.plugins.git.util.Build; import hudson.plugins.git.util.BuildData; @@ -20,11 +23,16 @@ import javax.inject.Inject; import java.io.IOException; import java.util.HashMap; +import java.util.List; import java.util.concurrent.TimeUnit; import static com.cloudbees.jenkins.GitHubWebHookFullTest.classpath; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.HEAD_COMMIT; +import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.MASTER_BRANCH_REF; import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.TRIGGERED_BY_USER_FROM_RESOURCE; /** @@ -69,12 +77,28 @@ public void shouldStartWorkflowByTrigger() throws Exception { buildData.buildsByBranchName = new HashMap(); buildData.getLastBuiltRevision().setSha1(ObjectId.zeroId()); - trigger.onPost(TRIGGERED_BY_USER_FROM_RESOURCE); + trigger.onPost(TRIGGERED_BY_USER_FROM_RESOURCE, MASTER_BRANCH_REF, HEAD_COMMIT); TimeUnit.SECONDS.sleep(job.getQuietPeriod()); jRule.waitUntilNoActivity(); assertThat("should be 2 build after hook", job.getLastBuild().getNumber(), is(2)); + + // Validate that the we have added 2 actions of the correct types + List jobActions = job.getLastBuild().getAllActions(); + assertThat("action list has at least two elements", jobActions.size(), greaterThanOrEqualTo(2)); + assertThat("first action is a ParametersAction", jobActions.get(0) instanceof ParametersAction); + assertThat("second action is a CauseAction", jobActions.get(1) instanceof CauseAction); + + // Validate that the parameters we have added are correct + ParametersAction parametersAction = ((ParametersAction)jobActions.get(0)); + assertThat("should be 2 parameters added", parametersAction.getParameters(), hasSize(2)); + assertThat("first parameter is ref with value " + MASTER_BRANCH_REF, + "ref".equalsIgnoreCase(parametersAction.getParameters().get(0).getName()) + && MASTER_BRANCH_REF.equals(parametersAction.getParameters().get(0).getValue())); + assertThat("second parameter is head with value " + HEAD_COMMIT, + "head".equalsIgnoreCase(parametersAction.getParameters().get(1).getName()) + && HEAD_COMMIT.equals(parametersAction.getParameters().get(1).getValue())); } @Test diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java b/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java index 78851d578..e2a47a601 100644 --- a/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java @@ -29,6 +29,8 @@ public class DefaultPushGHEventListenerTest { public static final GitSCM GIT_SCM_FROM_RESOURCE = new GitSCM("ssh://git@github.com/lanwen/test.git"); public static final String TRIGGERED_BY_USER_FROM_RESOURCE = "lanwen"; + public static final String MASTER_BRANCH_REF = "refs/heads/master"; + public static final String HEAD_COMMIT = "1eee2db8927ab3f7ec983b2e6052f351dd61a419"; @Rule public JenkinsRule jenkins = new JenkinsRule(); @@ -62,6 +64,8 @@ public void shouldParsePushPayload() throws Exception { .withTimestamp(subscriberEvent.getTimestamp()) .withOrigin("shouldParsePushPayload") .withTriggeredByUser(TRIGGERED_BY_USER_FROM_RESOURCE) + .withRef(MASTER_BRANCH_REF) + .withHead(HEAD_COMMIT) .build() )); } @@ -85,6 +89,8 @@ public void shouldReceivePushHookOnWorkflow() throws Exception { .withTimestamp(subscriberEvent.getTimestamp()) .withOrigin("shouldReceivePushHookOnWorkflow") .withTriggeredByUser(TRIGGERED_BY_USER_FROM_RESOURCE) + .withRef(MASTER_BRANCH_REF) + .withHead(HEAD_COMMIT) .build() )); } From c5df38f61372df1b46e33883393d67323bd5ce39 Mon Sep 17 00:00:00 2001 From: mblevins4 Date: Mon, 10 Apr 2017 00:19:18 -0400 Subject: [PATCH 2/3] Review comments, add parameters as environment variables as well, cleaned up unit test. --- .../jenkins/GitHubParametersAction.java | 55 +++++++++++++++++++ .../cloudbees/jenkins/GitHubPushTrigger.java | 6 +- .../cloudbees/jenkins/GitHubTriggerEvent.java | 5 +- .../jenkins/GitHubPushTriggerTest.java | 10 ++-- 4 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 src/main/java/com/cloudbees/jenkins/GitHubParametersAction.java diff --git a/src/main/java/com/cloudbees/jenkins/GitHubParametersAction.java b/src/main/java/com/cloudbees/jenkins/GitHubParametersAction.java new file mode 100644 index 000000000..1876e5de5 --- /dev/null +++ b/src/main/java/com/cloudbees/jenkins/GitHubParametersAction.java @@ -0,0 +1,55 @@ +package com.cloudbees.jenkins; + +import hudson.EnvVars; +import hudson.Extension; +import hudson.model.EnvironmentContributor; +import hudson.model.ParameterValue; +import hudson.model.ParametersAction; +import hudson.model.Run; +import hudson.model.TaskListener; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +public class GitHubParametersAction extends ParametersAction { + + private List parameters; + + public GitHubParametersAction(List parameters) { + super(parameters); + this.parameters = parameters; + } + + @Override + public List getParameters() { + return Collections.unmodifiableList(parameters); + } + + @Override + public ParameterValue getParameter(String name) { + for (ParameterValue parameter : parameters) { + if (parameter != null && parameter.getName().equals(name)) { + return parameter; + } + } + return null; + } + + @Extension + public static final class GitHubParameterEnvironmentContributor extends EnvironmentContributor { + + @Override + public void buildEnvironmentFor(Run run, + EnvVars envs, + TaskListener listener) throws IOException, InterruptedException { + GitHubParametersAction gpa = run.getAction(GitHubParametersAction.class); + if (gpa != null) { + for (ParameterValue p : gpa.getParameters()) { + envs.put(p.getName(), String.valueOf(p.getValue())); + } + } + super.buildEnvironmentFor(run, envs, listener); + } + } +} diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java index 0d9991c73..4bd7bca5e 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java @@ -13,7 +13,6 @@ import hudson.model.Job; import hudson.model.ParameterDefinition; import hudson.model.ParameterValue; -import hudson.model.ParametersAction; import hudson.model.ParametersDefinitionProperty; import hudson.model.Project; import hudson.model.StringParameterValue; @@ -168,7 +167,7 @@ public void run() { // Schedule the job with the parameters and cause QueueTaskFuture build = asParameterizedJobMixIn(job) - .scheduleBuild2(0, new ParametersAction(values), new CauseAction(cause)); + .scheduleBuild2(0, new GitHubParametersAction(values), new CauseAction(cause)); if (build != null) { LOGGER.info("SCM changes detected in " + job.getFullName() + ". Triggering #" + job.getNextBuildNumber()); @@ -188,9 +187,6 @@ private List getDefaultParameters() { ParametersDefinitionProperty pdp = this.job.getProperty(ParametersDefinitionProperty.class); if (pdp != null) { for (ParameterDefinition pd : pdp.getParameterDefinitions()) { - if (pd.getName().equals("sha1")) { - continue; - } values.add(pd.getDefaultParameterValue()); } } diff --git a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java index 9cb434d04..d8e664747 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java @@ -86,10 +86,7 @@ public boolean equals(Object o) { if (ref != null ? !ref.equals(that.ref) : that.ref != null) { return false; } - if (head != null ? !head.equals(that.head) : that.head != null) { - return false; - } - return true; + return head != null ? head.equals(that.head) : that.head == null; } @Override diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index 469b6ac1d..66f2831f1 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -85,13 +85,9 @@ public void shouldStartWorkflowByTrigger() throws Exception { assertThat("should be 2 build after hook", job.getLastBuild().getNumber(), is(2)); // Validate that the we have added 2 actions of the correct types - List jobActions = job.getLastBuild().getAllActions(); - assertThat("action list has at least two elements", jobActions.size(), greaterThanOrEqualTo(2)); - assertThat("first action is a ParametersAction", jobActions.get(0) instanceof ParametersAction); - assertThat("second action is a CauseAction", jobActions.get(1) instanceof CauseAction); - // Validate that the parameters we have added are correct - ParametersAction parametersAction = ((ParametersAction)jobActions.get(0)); + GitHubParametersAction parametersAction = job.getLastBuild().getAction(GitHubParametersAction.class); + assertThat("found GitHubParametersAction", parametersAction != null); assertThat("should be 2 parameters added", parametersAction.getParameters(), hasSize(2)); assertThat("first parameter is ref with value " + MASTER_BRANCH_REF, "ref".equalsIgnoreCase(parametersAction.getParameters().get(0).getName()) @@ -99,6 +95,8 @@ public void shouldStartWorkflowByTrigger() throws Exception { assertThat("second parameter is head with value " + HEAD_COMMIT, "head".equalsIgnoreCase(parametersAction.getParameters().get(1).getName()) && HEAD_COMMIT.equals(parametersAction.getParameters().get(1).getValue())); + CauseAction causeAction = job.getLastBuild().getAction(CauseAction.class); + assertThat("found CauseAction", causeAction != null); } @Test From 258be45c153733d266e3784305d7177e4eeae57d Mon Sep 17 00:00:00 2001 From: mblevins4 Date: Mon, 10 Apr 2017 10:19:23 -0400 Subject: [PATCH 3/3] codacy comments. --- .../java/com/cloudbees/jenkins/GitHubPushTriggerTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index 66f2831f1..045d94b27 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -1,9 +1,7 @@ package com.cloudbees.jenkins; -import hudson.model.Action; import hudson.model.CauseAction; import hudson.model.FreeStyleProject; -import hudson.model.ParametersAction; import hudson.plugins.git.GitSCM; import hudson.plugins.git.util.Build; import hudson.plugins.git.util.BuildData; @@ -23,12 +21,10 @@ import javax.inject.Inject; import java.io.IOException; import java.util.HashMap; -import java.util.List; import java.util.concurrent.TimeUnit; import static com.cloudbees.jenkins.GitHubWebHookFullTest.classpath; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.HEAD_COMMIT;