[JENKINS-40765] - Apply some fixes after the manual testing (#24)

* Update Parent POM to the latest version * [JENKINS-40765] - Restore the broken SafeParameters action * [JENKINS-40765] - ReleaseStep#parameters.groovy should return proper message when specifying a Pipeline job * [JENKINS-40765] - Cleanup support of folders in the release() step * [JENKINS-40765] - Restrict the newly introduced classes Just in case we need to change them
jenkinsci · Apr 8, 2017 · 82a1b5d · 82a1b5d
1 parent 1f9b17f
commit 82a1b5d
Show file tree

Hide file tree

Showing 10 changed files with 112 additions and 41 deletions.
diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
   <parent>
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>plugin</artifactId>
-    <version>2.15</version>
+    <version>2.24</version>
   </parent>
   <artifactId>release</artifactId>
   <packaging>hpi</packaging>
@@ -16,6 +16,7 @@
   <properties>
     <!-- https://issues.jenkins-ci.org/browse/JENKINS-29692  https://issues.jenkins-ci.org/browse/JENKINS-28781 -->
     <jenkins.version>1.642.4</jenkins.version>
+    <java.level>7</java.level>
   </properties>
 
   <scm>

diff --git a/src/main/java/hudson/plugins/release/SafeParametersAction.java b/src/main/java/hudson/plugins/release/SafeParametersAction.java
@@ -1,6 +1,7 @@
 package hudson.plugins.release;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.List;
 
 import org.kohsuke.accmod.Restricted;
@@ -13,19 +14,45 @@
 import hudson.model.ParametersAction;
 import hudson.model.Run;
 import hudson.model.TaskListener;
+import javax.annotation.Nonnull;
 
 @Restricted(NoExternalUse.class)
 public class SafeParametersAction extends ParametersAction {
 
+    @Nonnull
+    private final List<ParameterValue> parameters;
+
     /**
      * At this point the list of parameter values is guaranteed to be safe, which is
      * parameter defined either at top level or release wrapper level.
-     *
-     * @param parameters parameters allowed by the job and parameters allowed by release-specific parameters definition
-     * @since 2.7 - public constructor
+     * @param parameters Parameters to be passed. All of them will be considered as safe
+     */
+    public SafeParametersAction(@Nonnull List<ParameterValue> parameters) {
+        this.parameters = parameters;
+    }
+
+    /**
+     * Returns all parameters allowed by the job (defined as regular job parameters) and
+     * the parameters allowed by release-specific parameters definition.
      */
-    public SafeParametersAction(List<ParameterValue> parameters) {
-        super(parameters);
+    @Override
+    public List<ParameterValue> getParameters() {
+        return Collections.unmodifiableList(parameters);
+    }
+
+    /**
+     * Returns the parameter if defined as a regular parameters or it is a release-specific parameter defined
+     * by the release wrapper.
+     */
+    @Override
+    public ParameterValue getParameter(String name) {
+        for (ParameterValue p : parameters) {
+            if (p == null) continue;
+            if (p.getName().equals(name)) {
+                return p;
+            }
+        }
+        return null;
     }
 
     @Extension
@@ -35,7 +62,7 @@ public static final class SafeParametersActionEnvironmentContributor extends Env
         public void buildEnvironmentFor(Run r, EnvVars envs, TaskListener listener) throws IOException, InterruptedException {
             SafeParametersAction action = r.getAction(SafeParametersAction.class);
             if (action != null) {
-                for (ParameterValue p : action.getParameters()) {
+                for(ParameterValue p : action.getParameters()) {
                     envs.put(p.getName(), String.valueOf(p.getValue()));
                 }
             }

diff --git a/src/main/java/hudson/plugins/release/pipeline/ReleaseStep.java b/src/main/java/hudson/plugins/release/pipeline/ReleaseStep.java
@@ -33,36 +33,53 @@
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.plugins.release.ReleaseWrapper;
+import javax.annotation.CheckForNull;
 import jenkins.model.Jenkins;
 import jenkins.model.ParameterizedJobMixIn;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
+import org.apache.commons.lang.StringUtils;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
- * Pipeline step implementation for Release plugin.
+ * Pipeline step implementation for the Release plugin.
  *
  * @author Alexey Merezhin
  * @since 2.7
  */
+@Restricted(NoExternalUse.class)
 public class ReleaseStep extends Step {
     private static final Logger LOGGER = Logger.getLogger(ReleaseStep.class.getName());
 
+    /**
+     * Full name of the job.
+     * If {@code null}, the release step will fail
+     */
+    @CheckForNull
     private String job;
     @Nonnull
     private List<ParameterValue> parameters;
 
     @DataBoundConstructor
     public ReleaseStep(String job) {
-        this.job = job;
+        this.job = StringUtils.trimToNull(job);
         parameters = new ArrayList<>();
     }
 
+    /**
+     * Gets full name of the job to be released
+     * @return Full name of the job.
+     *         {@code null} if it is not specified in the Pipeline definition.
+     *         {@code null} will cause the build failure.
+     */
+    @CheckForNull
     public String getJob() {
         return job;
     }
 
+    @Restricted(NoExternalUse.class)
     public void setJob(String job) {
-        this.job = job;
+        this.job = StringUtils.trimToNull(job);
     }
 
     public List<ParameterValue> getParameters() {
@@ -104,24 +121,27 @@ public Set<Class<?>> getRequiredContext() {
         @Override
         public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException {
             ReleaseStep step = (ReleaseStep) super.newInstance(req, formData);
+
+            final String jobFullName = step.getJob();
+            if (jobFullName == null) {
+                throw new FormException("Job name is not specified or blank", "job");
+            }
+
             // Cf. ParametersDefinitionProperty._doBuild:
             Object parameter = formData.get("parameter");
             JSONArray params = parameter != null ? JSONArray.fromObject(parameter) : null;
             if (params != null) {
-                Jenkins jenkins = Jenkins.getInstance();
-                Job<?,?> context = StaplerReferer.findItemFromRequest(Job.class);
-                Job<?,?> job = jenkins != null ? jenkins.getItem(step.getJob(), context, Job.class) : null;
-
+                // TODO: Add support of relative paths like in the Promoted Builds plugin
                 BuildableItem project = Jenkins.getActiveInstance()
-                                               .getItem(step.getJob(), context, BuildableItem.class);
+                                               .getItemByFullName(jobFullName, BuildableItem.class);
 
                 if (project == null) {
-                    throw new IllegalArgumentException("Can't find project " + step.getJob());
+                    throw new FormException("Can't find buildable item " + jobFullName, "job");
                 } else if (project instanceof BuildableItemWithBuildWrappers) {
-                    ReleaseWrapper wrapper = ((BuildableItemWithBuildWrappers) project).getBuildWrappersList()
-                                                                       .get(ReleaseWrapper.class);
+                    ReleaseWrapper wrapper = ((BuildableItemWithBuildWrappers) project)
+                            .getBuildWrappersList().get(ReleaseWrapper.class);
                     if (wrapper == null) {
-                        throw new FormException("Job doesn't have release plugin configuration", "job");
+                        throw new FormException("Job doesn't have the Release plugin configuration", "job");
                     }
                     List<ParameterDefinition> parameterDefinitions = wrapper.getParameterDefinitions();
                     if (parameterDefinitions != null) {
@@ -141,7 +161,7 @@ public Step newInstance(StaplerRequest req, JSONObject formData) throws FormExce
                         step.setParameters(values);
                     }
                 } else {
-                    throw new FormException("Wrong job type: " + project.getClass().getName(), "job");
+                    throw new FormException("The job is not an instance of the BuildableItemWithBuildWrappers class: " + project.getClass().getName(), "job");
                 }
             }
             return step;

diff --git a/src/main/java/hudson/plugins/release/pipeline/ReleaseStepExecution.java b/src/main/java/hudson/plugins/release/pipeline/ReleaseStepExecution.java
@@ -18,6 +18,7 @@
 import hudson.model.BuildableItemWithBuildWrappers;
 import hudson.model.Cause;
 import hudson.model.CauseAction;
+import hudson.model.Item;
 import hudson.model.Job;
 import hudson.model.ParameterDefinition;
 import hudson.model.ParameterValue;
@@ -28,11 +29,14 @@
 import hudson.plugins.release.SafeParametersAction;
 import jenkins.model.Jenkins;
 import jenkins.model.ParameterizedJobMixIn;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * @author Alexey Merezhin
  * @since 2.7
  */
+@Restricted(NoExternalUse.class)
 public class ReleaseStepExecution extends StepExecution {
     private static final Logger LOGGER = Logger.getLogger(ReleaseStepExecution.class.getName());
 
@@ -63,39 +67,44 @@ private List<ParameterValue> updateParametersWithDefaults(ParameterizedJobMixIn.
                     }
                 }
             } else {
-                throw new AbortException("Job doesn't have release plugin configuration");
+                throw new AbortException("Job doesn't have the Release plugin configuration");
             }
         }
         return parameters;
     }
 
     @Override
     public boolean start() throws Exception {
-        if (step.getJob() == null) {
+        final String jobFullName = step.getJob();
+        if (jobFullName == null) {
             throw new AbortException("Job name is not defined.");
         }
 
-        final ParameterizedJobMixIn.ParameterizedJob project = Jenkins.getActiveInstance().
-                getItemByFullName(step.getJob(), ParameterizedJobMixIn.ParameterizedJob.class);;
-        if (project == null) {
-            throw new AbortException("No parametrized job named " + step.getJob() + " found");
-        }
-        println("Releasing project: " + ModelHyperlinkNote.encodeTo(project));
-
-        LOGGER.log(Level.FINER, "scheduling a release of {0} from {1}", new Object[] { project, getContext() });
+        final Item itemToRun = Jenkins.getActiveInstance().getItemByFullName(jobFullName);
+        if (itemToRun == null) {
+            throw new AbortException("No item found: " + jobFullName );
+        } else if (!(itemToRun instanceof Job<?, ?> && itemToRun instanceof ParameterizedJobMixIn.ParameterizedJob)) {
+            throw new AbortException("The specified item is not a parameterizable job: " + jobFullName );
+        } 
+
+        final ParameterizedJobMixIn.ParameterizedJob jobToRun = (ParameterizedJobMixIn.ParameterizedJob) itemToRun;
+        println("Releasing job: " + ModelHyperlinkNote.encodeTo(jobToRun));
+
+        LOGGER.log(Level.FINER, "Scheduling release of {0} from {1}", new Object[] { jobToRun, getContext() });
         Run run = getContext().get(Run.class);
         List<Action> actions = new ArrayList<>(3);
         actions.add(new ReleaseTriggerAction(getContext()));
         actions.add(new ReleaseWrapper.ReleaseBuildBadgeAction());
-        actions.add(new SafeParametersAction(updateParametersWithDefaults(project, step.getParameters())));
+        actions.add(new SafeParametersAction(updateParametersWithDefaults(jobToRun, step.getParameters())));
         if (run != null) {
             actions.add(new CauseAction(new Cause.UpstreamCause(run)));
         }
 
-        Queue.Item item = ParameterizedJobMixIn.scheduleBuild2((Job<?, ?>) project, 0, actions.toArray(new Action[0]));
+        Queue.Item item = ParameterizedJobMixIn.scheduleBuild2((Job<?, ?>)jobToRun, 0, 
+                actions.toArray(new Action[0]));
 
         if (item == null || item.getFuture() == null) {
-            throw new AbortException("Failed to trigger build of " + project.getFullName());
+            throw new AbortException("Failed to trigger build of " + jobToRun.getFullName());
         }
 
         return false;

diff --git a/src/main/java/hudson/plugins/release/pipeline/ReleaseTriggerAction.java b/src/main/java/hudson/plugins/release/pipeline/ReleaseTriggerAction.java
@@ -9,11 +9,14 @@
 
 import hudson.model.Actionable;
 import hudson.model.InvisibleAction;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * copied from org.jenkinsci.plugins.workflow.support.steps.build.BuildTriggerAction
  * @since 2.7
  */
+@Restricted(NoExternalUse.class)
 class ReleaseTriggerAction extends InvisibleAction {
     /** Record of one upstream build step. */
     static class Trigger {

diff --git a/src/main/java/hudson/plugins/release/pipeline/ReleaseTriggerListener.java b/src/main/java/hudson/plugins/release/pipeline/ReleaseTriggerListener.java
@@ -17,12 +17,15 @@
 
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 import org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * copied from org.jenkinsci.plugins.workflow.support.steps.build.BuildTriggerListener
  * @since 2.7
  */
 @Extension
+@Restricted(NoExternalUse.class)
 public class ReleaseTriggerListener extends RunListener<Run<?,?>>{
 
     private static final Logger LOGGER = Logger.getLogger(ReleaseTriggerListener.class.getName());

diff --git a/...in/resources/hudson/plugins/release/pipeline/ReleaseStep/DescriptorImpl/parameters.groovy b/...in/resources/hudson/plugins/release/pipeline/ReleaseStep/DescriptorImpl/parameters.groovy
@@ -1,17 +1,19 @@
 package hudson.plugins.release.pipeline.ReleaseStep.DescriptorImpl
 
-import hudson.model.AbstractProject
+import hudson.model.Item
 import hudson.model.BuildableItemWithBuildWrappers
 import hudson.model.StringParameterDefinition
 import hudson.plugins.release.ReleaseWrapper;
 def st = namespace('jelly:stapler')
 def l = namespace('/lib/layout')
+//TODO: add support of warnings
+//TODO: support of folders???
 l.ajax {
     def jobName = request.getParameter('job')
     if (jobName != null) {
         def contextName = request.getParameter('context')
         def context = contextName != null ? app.getItemByFullName(contextName) : null
-        def project = app.getItem(jobName, context, AbstractProject)
+        def project = app.getItem(jobName, context, Item)
 
         if (project != null) {
             if (project instanceof BuildableItemWithBuildWrappers) {
@@ -32,15 +34,16 @@ l.ajax {
                         }
                     }
                 } else {
-                    text("${project.fullDisplayName} doesn't have release plugin configuration")
+                    text("${project.fullDisplayName} does not have the release configuration")
                 }
             } else {
-                text("${project.fullDisplayName} is of wrong type and can't be released: ${project.class.simpleName}")
+                text("${project.fullDisplayName} is of wrong type and can't be released: " +
+                     "(${project.class.simpleName} does not implement BuildableItemWithBuildWrappers)")
             }
         } else {
-            text("no such job ${jobName}")
+            text("No such item ${jobName}")
         }
     } else {
-        text('no job specified')
+        text('No item specified')
     }
 }
diff --git a/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/config.jelly b/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/config.jelly
@@ -26,7 +26,7 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler">
     <j:set var="jobFieldId" value="${h.generateId()}"/>
-    <f:entry field="job" title="${%Project to Release}">
+    <f:entry field="job" title="${%Job to Release}">
         <f:textbox onblur="loadParams()" id="${jobFieldId}"/>
     </f:entry>
     <f:entry title="${%Parameters}">

diff --git a/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/config.properties b/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/config.properties
@@ -20,5 +20,5 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-Project\ to\ Release=Project to Release
+Job\ to\ Release=Job to Release
 Parameters=Parameters
diff --git a/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/help-job.html b/src/main/resources/hudson/plugins/release/pipeline/ReleaseStep/help-job.html
@@ -0,0 +1,5 @@
+<div>
+  Full name of the job to be released.
+  This job must be visible to the current user.
+  It should contain the Release configuration defined by the plugin.
+</div>