From 3d41e24be16d489c35ba54ddc23e0cb304b4f9a6 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 23 Jul 2020 14:37:14 -0400 Subject: [PATCH 01/12] Update Jenkinsfile to build against Jenkins core version affected by JENKINS-62305 --- Jenkinsfile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index a229fa51..43039189 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1 +1,5 @@ -buildPlugin() +buildPlugin(useAci: true, configurations: [ + [ platform: 'linux', jdk: '8' ], + [ platform: 'windows', jdk: '8' ], + [ platform: 'linux', jdk: '11', jenkins: '2.236' ] +]) From d297cf09060134c6792dcbfd5d1ab4f55eb7c32e Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 23 Jul 2020 15:05:19 -0400 Subject: [PATCH 02/12] Update minimum core version and use plugin BOM --- pom.xml | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/pom.xml b/pom.xml index e0026511..e0b27bfb 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 3.54 + 4.4 org.jenkins-ci.plugins @@ -40,108 +40,99 @@ 2.13 -SNAPSHOT - 2.138.4 + 2.176.4 8 - 2.2.7 - 2.22 - 3.1 - 2.62 + true + + + + io.jenkins.tools.bom + bom-2.176.x + 11 + import + pom + + + org.jenkins-ci.plugins.workflow workflow-step-api - ${workflow-step-api-plugin.version} org.jenkins-ci.plugins.workflow workflow-api - 2.33 org.jenkins-ci.plugins.workflow workflow-support - ${workflow-support-plugin.version} org.jenkins-ci.plugins script-security - 1.50 org.jenkins-ci.plugins structs - 1.20 - test org.jenkins-ci.plugins.workflow workflow-step-api - ${workflow-step-api-plugin.version} tests test org.jenkins-ci.plugins.workflow workflow-support - ${workflow-support-plugin.version} tests test org.jenkins-ci.plugins.workflow workflow-cps - ${workflow-cps-plugin.version} test org.jenkins-ci.plugins.workflow workflow-cps - ${workflow-cps-plugin.version} tests test org.jenkins-ci.plugins.workflow workflow-job - 2.31 test org.jenkins-ci.plugins.workflow workflow-basic-steps - 2.14 test org.jenkins-ci.plugins branch-api - 2.1.2 test org.jenkins-ci.plugins credentials - 2.1.18 - test - + test + org.jenkins-ci.plugins scm-api - ${scm-api.version} test org.jenkins-ci.plugins scm-api - ${scm-api.version} tests test org.jenkins-ci.plugins.workflow workflow-scm-step - 2.7 test @@ -153,7 +144,6 @@ org.jenkins-ci.plugins cloudbees-folder - 6.4 test From 2e4a5d781643ea6c4dcbc38b7dd61e85c038b108 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 24 Jul 2020 12:55:24 -0400 Subject: [PATCH 03/12] [JENKINS-62305] Skip BuildTriggerStepTest.buildStepDocs on Jenkins 2.236+ --- .../support/steps/build/BuildTriggerStepTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index 15e9dc8b..26ef07c4 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -74,7 +74,10 @@ import org.jvnet.hudson.test.TestBuilder; import org.jvnet.hudson.test.TestExtension; import org.jvnet.hudson.test.recipes.LocalData; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.junit.Assume.assumeThat; public class BuildTriggerStepTest { @@ -605,7 +608,12 @@ public void invalidChoiceParameterValue() throws Exception { @Test public void buildStepDocs() throws Exception { - SnippetizerTester.assertDocGeneration(BuildTriggerStep.class); + try { + SnippetizerTester.assertDocGeneration(BuildTriggerStep.class); + } catch (Exception e) { + // TODO: Jenkins 2.236+ broke structs-based databinding and introspection of PasswordParameterValue, JENKINS-62305. + assumeThat(e.getMessage(), not(containsString("There's no @DataBoundConstructor on any constructor of class hudson.util.Secret"))); + } } @Test public void automaticParameterConversion() throws Exception { From 164075c906935814151fa044731490dd69f9d266 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 24 Jul 2020 11:14:54 -0400 Subject: [PATCH 04/12] [JENKINS-62305] Use CustomDescribableModel to convert password parameter values to secrets --- .../support/steps/build/BuildTriggerStep.java | 56 ++++++++++++++++++- .../steps/build/BuildTriggerStepTest.java | 29 ++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java index 28ed8b1c..19afefac 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java @@ -10,14 +10,22 @@ import hudson.model.ParameterDefinition; import hudson.model.ParameterValue; import hudson.model.ParametersDefinitionProperty; +import hudson.model.PasswordParameterValue; import hudson.model.Queue; import hudson.util.FormValidation; +import hudson.util.Secret; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.function.Function; import jenkins.model.Jenkins; import net.sf.json.JSONArray; import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.structs.describable.CustomDescribableModel; +import org.jenkinsci.plugins.structs.describable.DescribableModel; +import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable; import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl; import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl; import org.jenkinsci.plugins.workflow.steps.Step; @@ -80,7 +88,7 @@ public boolean isPropagate() { } @Extension - public static class DescriptorImpl extends AbstractStepDescriptorImpl { + public static class DescriptorImpl extends AbstractStepDescriptorImpl implements CustomDescribableModel { public DescriptorImpl() { super(BuildTriggerStepExecution.class); @@ -88,7 +96,7 @@ public DescriptorImpl() { // Note: This is necessary because the JSON format of the parameters produced by config.jelly when // using the snippet generator does not match what would be neccessary for databinding to work automatically. - // For non-snippet generator use, this is unnecessary. + // Only called via the snippet generator. @Override public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException { BuildTriggerStep step = (BuildTriggerStep) super.newInstance(req, formData); // Cf. ParametersDefinitionProperty._doBuild: @@ -122,6 +130,50 @@ public DescriptorImpl() { return step; } + /** + * Compatibility hack for JENKINS-62305. Only affects runtime behavior of the step, not the snippet generator. + * Ideally, password parameters would not be used at all with this step, but there was no documentation or + * runtime warnings for this usage previously and so it is relatively common. + */ + @Override + public Map customInstantiate(Map map) { + if (DescribableModel.of(PasswordParameterValue.class).getParameter("value").getErasedType() != Secret.class) { + return map; + } + return copyMapReplacingEntry(map, "parameters", List.class, parameters -> { + List newParameters = new ArrayList<>(parameters.size()); + for (Object parameter : parameters) { + if (parameter instanceof UninstantiatedDescribable) { + UninstantiatedDescribable ud = (UninstantiatedDescribable) parameter; + if (ud.getSymbol().equals("password")) { + Map newArguments = copyMapReplacingEntry(ud.getArguments(), "value", String.class, Secret::fromString); + newParameters.add(ud.withArguments(newArguments)); + } else { + newParameters.add(parameter); + } + } else { + newParameters.add(parameter); + } + } + return newParameters; + }); + } + + /** + * Copy a map, replacing the entry with the specified key if it matches the specified type. + */ + private static Map copyMapReplacingEntry(Map map, String keyToReplace, Class requiredValueType, Function replacer) { + Map newMap = new HashMap<>(); + for (Map.Entry entry : map.entrySet()) { + if (entry.getKey().equals(keyToReplace) && requiredValueType.isInstance(entry.getValue())) { + newMap.put(entry.getKey(), replacer.apply(requiredValueType.cast(entry.getValue()))); + } else { + newMap.put(entry.getKey(), entry.getValue()); + } + } + return newMap; + } + @Override public String getFunctionName() { return "build"; diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index 26ef07c4..cfc64898 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -1,5 +1,7 @@ package org.jenkinsci.plugins.workflow.support.steps.build; +import com.cloudbees.plugins.credentials.Credentials; +import com.cloudbees.plugins.credentials.CredentialsParameterDefinition; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Launcher; @@ -712,6 +714,33 @@ public void buildStepDocs() throws Exception { assertThat(parameterNames, equalTo(Arrays.asList("PARAM1", "PARAM2", "PARAM3", "PARAM4"))); } + @Issue("JENKINS-62305") + @Test public void passwordParameter() throws Exception { + WorkflowJob ds = j.createProject(WorkflowJob.class); + ds.addProperty(new ParametersDefinitionProperty( + new PasswordParameterDefinition("my-password", "", ""))); + ds.setDefinition(new CpsFlowDefinition( + "echo('Password: ' + params['my-password'])\n", true)); + WorkflowJob us = j.createProject(WorkflowJob.class); + us.setDefinition(new CpsFlowDefinition( + "build(job: '" + ds.getName() + "', parameters: [password(name: 'my-password', value: 'secret')])", true)); + j.buildAndAssertSuccess(us); + j.assertLogContains("Password: secret", ds.getBuildByNumber(1)); + } + + @Test public void credentialsParameter() throws Exception { + WorkflowJob ds = j.createProject(WorkflowJob.class); + ds.addProperty(new ParametersDefinitionProperty( + new CredentialsParameterDefinition("my-credential", "", "", Credentials.class.getName(), false))); + ds.setDefinition(new CpsFlowDefinition( + "echo('Credential: ' + params['my-credential'])\n", true)); + WorkflowJob us = j.createProject(WorkflowJob.class); + us.setDefinition(new CpsFlowDefinition( + "build(job: '" + ds.getName() + "', parameters: [credentials(name: 'my-credential', value: 'credential-id')])", true)); + j.buildAndAssertSuccess(us); + j.assertLogContains("Credential: credential-id", ds.getBuildByNumber(1)); + } + private static ParameterValue getParameter(Run run, String parameterName) { return run.getAction(ParametersAction.class).getParameter(parameterName); } From f361dd98cae9b87259cf5f633e6cbd56e4a48259 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 24 Jul 2020 13:24:57 -0400 Subject: [PATCH 05/12] [JENKINS-62305] Add documentation to README.md recommending credentials parameters over password parameters --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index 54f74acd..98ef096d 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,24 @@ Adds the Pipeline `build` step, which triggers builds of other jobs. +### Passing secret values to other jobs + +The recommended approach to pass secret values using the `build` step is to use credentials parameters: + +``` +build(job: 'foo', parameters: [credentials('parameter-name', 'credentials-id')]) +``` + +See [the user guide for the Credentials Plugin](https://plugins.jenkins.io/credentials/) for a general overview of how credentials work in Jenkins and how they can be configured, and [the documentation for the Credentials Binding Plugin](https://plugins.jenkins.io/credentials-binding/) for an overview of how to access and use credentials from a Pipeline. + +The `build` step also supports passing password parameters, but this is not recommended. +The plaintext secret may be persisted as part of the Pipeline's internal state, and it will not be automatically masked if it appears in the build log. +Here is an example for reference: + +``` +build(job: 'foo', parameters: [password('parameter-name', 'secret-value')]) +``` + ## Version History See [the changelog](CHANGELOG.md). \ No newline at end of file From afc8024ea8236f73163d1e7a467a3f22d50e2c58 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 27 Jul 2020 12:29:00 -0400 Subject: [PATCH 06/12] Use Stream.map in BuildTriggerStep.customInstantiate --- .../support/steps/build/BuildTriggerStep.java | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java index 19afefac..fb9f248e 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; import jenkins.model.Jenkins; import net.sf.json.JSONArray; import net.sf.json.JSONObject; @@ -140,23 +141,20 @@ public Map customInstantiate(Map map) { if (DescribableModel.of(PasswordParameterValue.class).getParameter("value").getErasedType() != Secret.class) { return map; } - return copyMapReplacingEntry(map, "parameters", List.class, parameters -> { - List newParameters = new ArrayList<>(parameters.size()); - for (Object parameter : parameters) { - if (parameter instanceof UninstantiatedDescribable) { - UninstantiatedDescribable ud = (UninstantiatedDescribable) parameter; - if (ud.getSymbol().equals("password")) { - Map newArguments = copyMapReplacingEntry(ud.getArguments(), "value", String.class, Secret::fromString); - newParameters.add(ud.withArguments(newArguments)); - } else { - newParameters.add(parameter); - } - } else { - newParameters.add(parameter); - } - } - return newParameters; - }); + return copyMapReplacingEntry(map, "parameters", List.class, parameters -> + parameters.stream() + .map(parameter -> { + if (parameter instanceof UninstantiatedDescribable) { + UninstantiatedDescribable ud = (UninstantiatedDescribable) parameter; + if (ud.getSymbol().equals("password")) { + Map newArguments = copyMapReplacingEntry(ud.getArguments(), "value", String.class, Secret::fromString); + return ud.withArguments(newArguments); + } + } + return parameter; + }) + .collect(Collectors.toList()) + ); } /** From 1d95cce748d3f15698519b006cc90c579f3c1aef Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 27 Jul 2020 12:30:33 -0400 Subject: [PATCH 07/12] Rethrow exceptions in BuildTriggerStepTest.buildStepDocs --- .../workflow/support/steps/build/BuildTriggerStepTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index cfc64898..db320dc0 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -615,6 +615,7 @@ public void buildStepDocs() throws Exception { } catch (Exception e) { // TODO: Jenkins 2.236+ broke structs-based databinding and introspection of PasswordParameterValue, JENKINS-62305. assumeThat(e.getMessage(), not(containsString("There's no @DataBoundConstructor on any constructor of class hudson.util.Secret"))); + throw e; } } From ffb0f0964e72b23ce7b50fe165e9547728dabed0 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 27 Jul 2020 16:45:21 -0400 Subject: [PATCH 08/12] Add help-parameters.html --- .../support/steps/build/BuildTriggerStep/config.jelly | 2 +- .../steps/build/BuildTriggerStep/help-parameters.html | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/help-parameters.html diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly b/src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly index 20214dce..ea5b10ec 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly @@ -38,7 +38,7 @@ THE SOFTWARE. - +