From ebfcc1be4bdc136c842a89c495cede8bdd57ebd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Arabaolaza=20Barquin?= Date: Fri, 25 Nov 2016 18:34:32 +0100 Subject: [PATCH] [Jenkins-39822] GitHub plugin functional tests broken against 1.651+ (#157) * [JENKINS-39822] Fix SECURITY-170 issues * [JENKINS-39822] Make sure there is always BuildData on testNoBuildRevision * [JENKINS-39822] Use conditional on plugin version * This way we change behaviour only if needed * [JENKINS-39822] Conditionally handle SECURITY-170 * [JENKINS-39822] Invoke build getting into account git plugin version * Git plugin 2.4.1+ does not include BuildData if checkout fails, resulting in testNoBuildRevision failing * [JENKINS-39822] Fix style * [JENKINS-39822] Added javadoc and more clarifying comments * [JENKINS-39822] Fix codacy warning --- .../jenkins/GitHubCommitNotifierTest.java | 15 ++++- .../github/common/ExpandableMessageTest.java | 18 ++++++ .../github/common/ParametersActionHelper.java | 61 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/jenkinsci/plugins/github/common/ParametersActionHelper.java diff --git a/src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java b/src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java index e3b8756d0..50f167f6b 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java @@ -6,11 +6,13 @@ import hudson.model.AbstractBuild; import hudson.model.Build; import hudson.model.BuildListener; +import hudson.model.Cause; import hudson.model.FreeStyleProject; import hudson.model.Result; import hudson.plugins.git.GitSCM; import hudson.plugins.git.Revision; import hudson.plugins.git.util.BuildData; +import hudson.util.VersionNumber; import org.eclipse.jgit.lib.ObjectId; import org.jenkinsci.plugins.github.config.GitHubPluginConfig; import org.jenkinsci.plugins.github.test.GHMockRule; @@ -96,7 +98,8 @@ public void testNoBuildRevision() throws Exception { FreeStyleProject prj = jRule.createFreeStyleProject(); prj.setScm(new GitSCM("http://non.existent.git.repo.nowhere/repo.git")); prj.getPublishersList().add(new GitHubCommitNotifier()); - Build b = prj.scheduleBuild2(0).get(); + //Git plugin 2.4.1 + does not include BuildData if checkout fails, so we add it if needed + Build b = safelyGenerateBuild(prj); jRule.assertBuildStatus(Result.FAILURE, b); jRule.assertLogContains(BuildDataHelper_NoLastRevisionError(), b); } @@ -139,6 +142,16 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen github.service().verify(1, postRequestedFor(urlPathMatching(".*/" + SOME_SHA))); } + private Build safelyGenerateBuild(FreeStyleProject prj) throws InterruptedException, java.util.concurrent.ExecutionException { + Build b; + if (jRule.getPluginManager().getPlugin("git").getVersionNumber().isNewerThan(new VersionNumber("2.4.0"))) { + b = prj.scheduleBuild2(0, new Cause.UserIdCause(), new BuildData()).get(); + } else { + b = prj.scheduleBuild2(0).get(); + } + return b; + } + @TestExtension public static final FixedGHRepoNameTestContributor CONTRIBUTOR = new FixedGHRepoNameTestContributor(); diff --git a/src/test/java/org/jenkinsci/plugins/github/common/ExpandableMessageTest.java b/src/test/java/org/jenkinsci/plugins/github/common/ExpandableMessageTest.java index b99f7b2dd..bac327f22 100644 --- a/src/test/java/org/jenkinsci/plugins/github/common/ExpandableMessageTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/common/ExpandableMessageTest.java @@ -4,7 +4,10 @@ import hudson.model.AbstractBuild; import hudson.model.BuildListener; import hudson.model.FreeStyleProject; +import hudson.model.ParameterDefinition; import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.StringParameterDefinition; import hudson.model.StringParameterValue; import org.junit.Rule; import org.junit.Test; @@ -43,6 +46,11 @@ public void shouldExpandEnvAndBuildVars() throws Exception { )); FreeStyleProject job = jRule.createFreeStyleProject(); + //Due to SECURITY-170 (jenkins versions 1.651.2+ and 2.3+) only build parameters that have been + //explicitly defined in a job's configuration will be available by default at build time. So if + //the test is running on such environment the appropriate parameter definitions must be added to + // the job + handleSecurity170(job); job.getBuildersList().add(expander); job.scheduleBuild2(0, new ParametersAction(new StringParameterValue(CUSTOM_BUILD_PARAM, CUSTOM_PARAM_VAL))) @@ -52,6 +60,7 @@ public void shouldExpandEnvAndBuildVars() throws Exception { startsWith(format(MSG_FORMAT, job.getFullName(), CUSTOM_PARAM_VAL, job.getFullName()))); } + public static String asVar(String name) { return format("${%s}", name); } @@ -60,6 +69,15 @@ public static String asTokenVar(String name) { return format(DEFAULT_TOKEN_TEMPLATE, name); } + private static void handleSecurity170(FreeStyleProject job) throws IOException { + ParametersActionHelper parametersActionHelper = new ParametersActionHelper(); + if (parametersActionHelper.getAbletoInspect() && parametersActionHelper.getHasSafeParameterConfig()) { + ParameterDefinition paramDef = new StringParameterDefinition(CUSTOM_BUILD_PARAM, "", ""); + ParametersDefinitionProperty paramsDef = new ParametersDefinitionProperty(paramDef); + job.addProperty(paramsDef); + } + } + private static class MessageExpander extends TestBuilder { private ExpandableMessage message; private String result; diff --git a/src/test/java/org/jenkinsci/plugins/github/common/ParametersActionHelper.java b/src/test/java/org/jenkinsci/plugins/github/common/ParametersActionHelper.java new file mode 100644 index 000000000..61d75d1ac --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/github/common/ParametersActionHelper.java @@ -0,0 +1,61 @@ +package org.jenkinsci.plugins.github.common; + +import hudson.model.ParametersAction; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; + +/** + * Helper class to check if the environment includes SECURITY-170 fix + * + * @see + */ +public class ParametersActionHelper { + + private static final Class actionClass = ParametersAction.class; + + private boolean hasSafeParameterConfig = false; + private boolean abletoInspect = true; + private static final String UNDEFINED_PARAMETERS_FIELD_NAME = "KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME"; + private static final String SAFE_PARAMETERS_FIELD_NAME = "SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME"; + + public ParametersActionHelper() { + try { + for (Field field : actionClass.getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) && isSafeParamsField(field)) { + this.hasSafeParameterConfig = true; + break; + } + } + } catch (Exception e) { + this.abletoInspect = false; + } + } + + /** + * Method to check if the fix for SECURITY-170 is present + * + * @return true if the SECURITY-170 fix is present, false otherwise + */ + public boolean getHasSafeParameterConfig() { + return hasSafeParameterConfig; + } + + /** + * Method to check if this class has been able to determine the existence of SECURITY-170 fix + * + * @return true if the check for SECURITY-170 has been executed (whatever the result) false otherwise + */ + public boolean getAbletoInspect() { + return abletoInspect; + } + + private boolean isSafeParamsField(Field field) { + String fieldName = field.getName(); + return UNDEFINED_PARAMETERS_FIELD_NAME.equals(fieldName) + || SAFE_PARAMETERS_FIELD_NAME.equals(fieldName); + } + + + +}