Skip to content
This repository was archived by the owner on Sep 25, 2020. It is now read-only.

Commit ebfcc1b

Browse files
Raúl Arabaolaza BarquinKostyaSha
Raúl Arabaolaza Barquin
authored and
KostyaSha
committed
[Jenkins-39822] GitHub plugin functional tests broken against 1.651+ (jenkinsci#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
1 parent 3e7eef3 commit ebfcc1b

3 files changed

Lines changed: 93 additions & 1 deletion

File tree

‎src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
import hudson.model.AbstractBuild;
77
import hudson.model.Build;
88
import hudson.model.BuildListener;
9+
import hudson.model.Cause;
910
import hudson.model.FreeStyleProject;
1011
import hudson.model.Result;
1112
import hudson.plugins.git.GitSCM;
1213
import hudson.plugins.git.Revision;
1314
import hudson.plugins.git.util.BuildData;
15+
import hudson.util.VersionNumber;
1416
import org.eclipse.jgit.lib.ObjectId;
1517
import org.jenkinsci.plugins.github.config.GitHubPluginConfig;
1618
import org.jenkinsci.plugins.github.test.GHMockRule;
@@ -96,7 +98,8 @@ public void testNoBuildRevision() throws Exception {
9698
FreeStyleProject prj = jRule.createFreeStyleProject();
9799
prj.setScm(new GitSCM("http://non.existent.git.repo.nowhere/repo.git"));
98100
prj.getPublishersList().add(new GitHubCommitNotifier());
99-
Build b = prj.scheduleBuild2(0).get();
101+
//Git plugin 2.4.1 + does not include BuildData if checkout fails, so we add it if needed
102+
Build b = safelyGenerateBuild(prj);
100103
jRule.assertBuildStatus(Result.FAILURE, b);
101104
jRule.assertLogContains(BuildDataHelper_NoLastRevisionError(), b);
102105
}
@@ -139,6 +142,16 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
139142
github.service().verify(1, postRequestedFor(urlPathMatching(".*/" + SOME_SHA)));
140143
}
141144

145+
private Build safelyGenerateBuild(FreeStyleProject prj) throws InterruptedException, java.util.concurrent.ExecutionException {
146+
Build b;
147+
if (jRule.getPluginManager().getPlugin("git").getVersionNumber().isNewerThan(new VersionNumber("2.4.0"))) {
148+
b = prj.scheduleBuild2(0, new Cause.UserIdCause(), new BuildData()).get();
149+
} else {
150+
b = prj.scheduleBuild2(0).get();
151+
}
152+
return b;
153+
}
154+
142155
@TestExtension
143156
public static final FixedGHRepoNameTestContributor CONTRIBUTOR = new FixedGHRepoNameTestContributor();
144157

‎src/test/java/org/jenkinsci/plugins/github/common/ExpandableMessageTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
import hudson.model.AbstractBuild;
55
import hudson.model.BuildListener;
66
import hudson.model.FreeStyleProject;
7+
import hudson.model.ParameterDefinition;
78
import hudson.model.ParametersAction;
9+
import hudson.model.ParametersDefinitionProperty;
10+
import hudson.model.StringParameterDefinition;
811
import hudson.model.StringParameterValue;
912
import org.junit.Rule;
1013
import org.junit.Test;
@@ -43,6 +46,11 @@ public void shouldExpandEnvAndBuildVars() throws Exception {
4346
));
4447

4548
FreeStyleProject job = jRule.createFreeStyleProject();
49+
//Due to SECURITY-170 (jenkins versions 1.651.2+ and 2.3+) only build parameters that have been
50+
//explicitly defined in a job's configuration will be available by default at build time. So if
51+
//the test is running on such environment the appropriate parameter definitions must be added to
52+
// the job
53+
handleSecurity170(job);
4654
job.getBuildersList().add(expander);
4755

4856
job.scheduleBuild2(0, new ParametersAction(new StringParameterValue(CUSTOM_BUILD_PARAM, CUSTOM_PARAM_VAL)))
@@ -52,6 +60,7 @@ public void shouldExpandEnvAndBuildVars() throws Exception {
5260
startsWith(format(MSG_FORMAT, job.getFullName(), CUSTOM_PARAM_VAL, job.getFullName())));
5361
}
5462

63+
5564
public static String asVar(String name) {
5665
return format("${%s}", name);
5766
}
@@ -60,6 +69,15 @@ public static String asTokenVar(String name) {
6069
return format(DEFAULT_TOKEN_TEMPLATE, name);
6170
}
6271

72+
private static void handleSecurity170(FreeStyleProject job) throws IOException {
73+
ParametersActionHelper parametersActionHelper = new ParametersActionHelper();
74+
if (parametersActionHelper.getAbletoInspect() && parametersActionHelper.getHasSafeParameterConfig()) {
75+
ParameterDefinition paramDef = new StringParameterDefinition(CUSTOM_BUILD_PARAM, "", "");
76+
ParametersDefinitionProperty paramsDef = new ParametersDefinitionProperty(paramDef);
77+
job.addProperty(paramsDef);
78+
}
79+
}
80+
6381
private static class MessageExpander extends TestBuilder {
6482
private ExpandableMessage message;
6583
private String result;

‎src/test/java/org/jenkinsci/plugins/github/common/ParametersActionHelper.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package org.jenkinsci.plugins.github.common;
2+
3+
import hudson.model.ParametersAction;
4+
5+
import java.lang.reflect.Field;
6+
import java.lang.reflect.Modifier;
7+
8+
/**
9+
* Helper class to check if the environment includes SECURITY-170 fix
10+
*
11+
* @see <a href=https://wiki.jenkins-ci.org/display/JENKINS/Plugins+affected+by+fix+for+SECURITY-170</a>
12+
*/
13+
public class ParametersActionHelper {
14+
15+
private static final Class<ParametersAction> actionClass = ParametersAction.class;
16+
17+
private boolean hasSafeParameterConfig = false;
18+
private boolean abletoInspect = true;
19+
private static final String UNDEFINED_PARAMETERS_FIELD_NAME = "KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME";
20+
private static final String SAFE_PARAMETERS_FIELD_NAME = "SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME";
21+
22+
public ParametersActionHelper() {
23+
try {
24+
for (Field field : actionClass.getDeclaredFields()) {
25+
if (Modifier.isStatic(field.getModifiers()) && isSafeParamsField(field)) {
26+
this.hasSafeParameterConfig = true;
27+
break;
28+
}
29+
}
30+
} catch (Exception e) {
31+
this.abletoInspect = false;
32+
}
33+
}
34+
35+
/**
36+
* Method to check if the fix for SECURITY-170 is present
37+
*
38+
* @return true if the SECURITY-170 fix is present, false otherwise
39+
*/
40+
public boolean getHasSafeParameterConfig() {
41+
return hasSafeParameterConfig;
42+
}
43+
44+
/**
45+
* Method to check if this class has been able to determine the existence of SECURITY-170 fix
46+
*
47+
* @return true if the check for SECURITY-170 has been executed (whatever the result) false otherwise
48+
*/
49+
public boolean getAbletoInspect() {
50+
return abletoInspect;
51+
}
52+
53+
private boolean isSafeParamsField(Field field) {
54+
String fieldName = field.getName();
55+
return UNDEFINED_PARAMETERS_FIELD_NAME.equals(fieldName)
56+
|| SAFE_PARAMETERS_FIELD_NAME.equals(fieldName);
57+
}
58+
59+
60+
61+
}

0 commit comments

Comments
 (0)