diff --git a/src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java b/src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java index fe60f5946..0fde684c1 100644 --- a/src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java +++ b/src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java @@ -23,6 +23,7 @@ import org.jenkins.plugins.lockableresources.queue.Utils; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript; import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -136,7 +137,8 @@ public RequiredResourcesProperty newInstance(StaplerRequest req, JSONObject form public FormValidation doCheckResourceNames(@QueryParameter String value, @QueryParameter String labelName, - @QueryParameter boolean script) { + @QueryParameter boolean script, + @AncestorInPath AbstractProject project) { String labelVal = Util.fixEmptyAndTrim(labelName); String names = Util.fixEmptyAndTrim(value); @@ -148,6 +150,7 @@ public FormValidation doCheckResourceNames(@QueryParameter String value, } else { List wrongNames = new ArrayList<>(); List varNames = new ArrayList<>(); + List unknownParams = new ArrayList<>(); for (String name : names.split("\\s+")) { boolean found = false; for (LockableResource r : LockableResourcesManager.get() @@ -158,86 +161,119 @@ public FormValidation doCheckResourceNames(@QueryParameter String value, } } if (!found) { - if (Utils.isVariable(name)) { - varNames.add(name); + if (Utils.containsParameter(name)) { + List badParams = Utils.checkParameters(name, project); + if (!badParams.isEmpty()) { + unknownParams.addAll(badParams); + } + varNames.add(name); } else { wrongNames.add(name); } } } - if (wrongNames.isEmpty() && varNames.isEmpty()) { - return FormValidation.ok(); - } else if (wrongNames.isEmpty()) { - return FormValidation - .warning("The following resources cannot be validated as they are the environment variables: " - + varNames); - } else { - return FormValidation - .error("The following resources do not exist: " - + wrongNames); - } - } - } - - public FormValidation doCheckLabelName( - @QueryParameter String value, - @QueryParameter String resourceNames, - @QueryParameter boolean script) { - String label = Util.fixEmptyAndTrim(value); - String names = Util.fixEmptyAndTrim(resourceNames); - - if (label == null) { - return FormValidation.ok(); - } else if (names != null || script) { - return FormValidation.error( - "Only label, groovy expression, or resources can be defined, not more than one."); - } else { - if (LockableResourcesManager.get().isValidLabel(label)) { - return FormValidation.ok(); - } else { - return FormValidation.error( - "The label does not exist: " + label); - } - } - } + if (wrongNames.isEmpty() && varNames.isEmpty() && unknownParams.isEmpty()) { + return FormValidation.ok(); + } else if (!wrongNames.isEmpty()) { + return FormValidation + .error("The following resources do not exist: " + + wrongNames); + } else if (!unknownParams.isEmpty()) { + return FormValidation + .error("The following parameters do not exist: " + + unknownParams); + } else { + return FormValidation + .warning("The following resources cannot be validated as they contain parameter values: " + + varNames); + } + } + } + + public FormValidation doCheckLabelName(@QueryParameter String value, + @QueryParameter String resourceNames, + @QueryParameter boolean script, + @AncestorInPath AbstractProject project) { + + String label = Util.fixEmptyAndTrim(value); + String names = Util.fixEmptyAndTrim(resourceNames); + + if (label == null) { + return FormValidation.ok(); + } else if (names != null || script) { + return FormValidation.error( + "Only label, groovy expression, or resources can be defined, not more than one."); + } else { + if (LockableResourcesManager.get().isValidLabel(label)) { + return FormValidation.ok(); + } else if (Utils.containsParameter(label)) { + List badParams = Utils.checkParameters(label, project); + if (!badParams.isEmpty()) { + return FormValidation + .error("The following parameters do not exist: " + + badParams); + } + return FormValidation + .warning("The label cannot be validated as it contains a parameter value: " + + label); + } else { + return FormValidation.error( + "The label does not exist: " + label); + } + } + } - public FormValidation doCheckResourceNumber(@QueryParameter String value, - @QueryParameter String resourceNames, - @QueryParameter String labelName, - @QueryParameter String resourceMatchScript) - { + public FormValidation doCheckResourceNumber(@QueryParameter String value, + @QueryParameter String resourceNames, + @QueryParameter String labelName, + @QueryParameter String resourceMatchScript, + @AncestorInPath AbstractProject project) { - String number = Util.fixEmptyAndTrim(value); - String names = Util.fixEmptyAndTrim(resourceNames); - String label = Util.fixEmptyAndTrim(labelName); + String number = Util.fixEmptyAndTrim(value); + String names = Util.fixEmptyAndTrim(resourceNames); + String label = Util.fixEmptyAndTrim(labelName); String script = Util.fixEmptyAndTrim(resourceMatchScript); - if (number == null || number.equals("") || number.trim().equals("0")) { - return FormValidation.ok(); - } + if (number == null || number.equals("0")) { + return FormValidation.ok(); + } - int numAsInt; - try { - numAsInt = Integer.parseInt(number); - } catch(NumberFormatException e) { - return FormValidation.error( - "Could not parse the given value as integer."); - } - int numResources = 0; - if (names != null) { - numResources = names.split("\\s+").length; - } else if (label != null || script != null) { - numResources = Integer.MAX_VALUE; + int numAsInt; + try { + numAsInt = Integer.parseInt(number); + } catch(NumberFormatException e) { + if (Utils.isParameter(number)) { + List badParams = Utils.checkParameters(number, project); + if (!badParams.isEmpty()) { + return FormValidation + .error("The following parameter does not exist: " + + badParams); + } + return FormValidation + .warning("The value cannot be validated as it is a parameter value: " + + number); + } + + return FormValidation.error( + "Could not parse the given value as integer."); + } + int numResources = 0; + if (names != null) { + numResources = names.split("\\s+").length; + } else if (label != null) { + numResources = LockableResourcesManager.get().getResourcesWithLabel(label, null).size(); + } else if (script != null) { + numResources = Integer.MAX_VALUE; } - if (numResources < numAsInt) { - return FormValidation.error(String.format( - "Given amount %d is greater than amount of resources: %d.", - numAsInt, - numResources)); - } - return FormValidation.ok(); - } + if (numResources < numAsInt) { + return FormValidation.error(String.format( + "Given amount %d is greater than amount of resources: %d.", + numAsInt, + numResources)); + } + return FormValidation.ok(); + } public AutoCompletionCandidates doAutoCompleteLabelName( @QueryParameter String value) { diff --git a/src/main/java/org/jenkins/plugins/lockableresources/queue/Utils.java b/src/main/java/org/jenkins/plugins/lockableresources/queue/Utils.java index 18a3a5c41..3e5d3a989 100644 --- a/src/main/java/org/jenkins/plugins/lockableresources/queue/Utils.java +++ b/src/main/java/org/jenkins/plugins/lockableresources/queue/Utils.java @@ -10,13 +10,23 @@ import hudson.EnvVars; import hudson.matrix.MatrixConfiguration; +import hudson.model.AbstractProject; import hudson.model.Job; +import hudson.model.ParametersDefinitionProperty; import hudson.model.Queue; import hudson.model.Run; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.regex.Matcher; import java.util.regex.Pattern; + import org.jenkins.plugins.lockableresources.RequiredResourcesProperty; +import javax.annotation.Nonnull; + public final class Utils { private Utils() { } @@ -26,6 +36,11 @@ private Utils() { */ private static final Pattern VARIABLE = Pattern.compile("\\$([A-Za-z0-9_]+|\\{[A-Za-z0-9_.]+\\})"); + /** + * Pattern for capturing parameters. ${xyz} but not $${xyz} + */ + private static final Pattern PARAMETER = Pattern.compile("(? getProject(Queue.Item item) { if (item.task instanceof Job) return (Job) item.task; @@ -53,7 +68,33 @@ public static LockableResourcesStruct requiredResources( return null; } - public static boolean isVariable(String name) { - return VARIABLE.matcher(name).matches(); + @Nonnull + public static List getProjectParameterNames(AbstractProject project) { + ParametersDefinitionProperty params = project.getProperty(ParametersDefinitionProperty.class); + if (params != null) + return params.getParameterDefinitionNames(); + return Collections.emptyList(); + } + + public static boolean isParameter(String s) { + return PARAMETER.matcher(s).matches(); + } + + public static boolean containsParameter(String s) { + return PARAMETER.matcher(s).find(); + } + + @Nonnull + public static List checkParameters(String s, AbstractProject project) { + List unknownParameters = new ArrayList<>(); + List paramNames = getProjectParameterNames(project); + Matcher m = PARAMETER.matcher(s); + while (m.find()) { + String param = m.group(1); + if (!paramNames.contains(param)) { + unknownParameters.add(param); + } + } + return unknownParameters; } } diff --git a/src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java b/src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java index 8163ebc60..a386cf159 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java @@ -165,6 +165,38 @@ public void configRoundTripWithParam() throws Exception { assertNull(resourcesProp.getResourceMatchScript()); } + @Test + public void configRoundTripWithLabelParam() throws Exception { + FreeStyleProject withLabel = j.createFreeStyleProject("withLabelParam"); + withLabel.addProperty(new RequiredResourcesProperty(null, null, null, "${labelParam}", null)); + FreeStyleProject withLabelRoundTrip = j.configRoundtrip(withLabel); + + RequiredResourcesProperty withLabelProp = + withLabelRoundTrip.getProperty(RequiredResourcesProperty.class); + assertNotNull(withLabelProp); + assertNull(withLabelProp.getResourceNames()); + assertNull(withLabelProp.getResourceNamesVar()); + assertNull(withLabelProp.getResourceNumber()); + assertEquals("${labelParam}", withLabelProp.getLabelName()); + assertNull(withLabelProp.getResourceMatchScript()); + } + + @Test + public void configRoundTripWithNumParam() throws Exception { + FreeStyleProject withNum = j.createFreeStyleProject("withNumParam"); + withNum.addProperty(new RequiredResourcesProperty(null, null, "${resNum}", "some-resources", null)); + FreeStyleProject withNumRoundTrip = j.configRoundtrip(withNum); + + RequiredResourcesProperty withNumProp = + withNumRoundTrip.getProperty(RequiredResourcesProperty.class); + assertNotNull(withNumProp); + assertNull(withNumProp.getResourceNames()); + assertNull(withNumProp.getResourceNamesVar()); + assertEquals("${resNum}", withNumProp.getResourceNumber()); + assertEquals("some-resources", withNumProp.getLabelName()); + assertNull(withNumProp.getResourceMatchScript()); + } + @Test public void configRoundTripWithScript() throws Exception { FreeStyleProject withScript = j.createFreeStyleProject("withScript"); diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java index 9a27be8db..5e2de1a88 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java @@ -927,6 +927,7 @@ public void lockWithInvalidLabel() throws Exception { WorkflowRun b1 = p.scheduleBuild2(0).waitForStart(); j.waitForCompletion(b1); j.assertBuildStatus(Result.FAILURE, b1); + j.waitUntilNoActivity(); j.assertLogContains("The label does not exist: invalidLabel", b1); isPaused(b1, 0, 0); } diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceManagerTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceManagerTest.java index 946a824ab..f13da2a16 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceManagerTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceManagerTest.java @@ -2,6 +2,9 @@ import static org.junit.Assert.assertEquals; +import hudson.model.FreeStyleProject; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.StringParameterDefinition; import hudson.util.FormValidation; import org.junit.Rule; import org.junit.Test; @@ -9,35 +12,86 @@ public class LockableResourceManagerTest { - @Rule public JenkinsRule j = new JenkinsRule(); - - @Test - public void validationFailure() { - RequiredResourcesProperty.DescriptorImpl d = new RequiredResourcesProperty.DescriptorImpl(); - LockableResourcesManager.get().createResource("resource1"); - LockableResource r = LockableResourcesManager.get().getResources().get(0); - r.setLabels("some-label"); - - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckResourceNames("resource1", null, true).getMessage()); - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckResourceNames("resource1", "some-label", false).getMessage()); - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckResourceNames("resource1", "some-label", true).getMessage()); - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckLabelName("some-label", "resource1", false).getMessage()); - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckLabelName("some-label", null, true).getMessage()); - assertEquals( - "Only label, groovy expression, or resources can be defined, not more than one.", - d.doCheckLabelName("some-label", "resource1", true).getMessage()); - - assertEquals(FormValidation.ok(), d.doCheckResourceNames("resource1", null, false)); - assertEquals(FormValidation.ok(), d.doCheckLabelName("some-label", null, false)); - } + @Rule public JenkinsRule j = new JenkinsRule(); + + @Test + public void validationFailure() throws Exception { + ParametersDefinitionProperty params = new ParametersDefinitionProperty( + new StringParameterDefinition("param1", "resource1", "parameter 1"), + new StringParameterDefinition("param2", "2", "parameter 2") + ); + FreeStyleProject p = j.createFreeStyleProject("p"); + p.addProperty(params); + + RequiredResourcesProperty.DescriptorImpl d = new RequiredResourcesProperty.DescriptorImpl(); + LockableResourcesManager.get().createResource("resource1"); + LockableResource r = LockableResourcesManager.get().getResources().get(0); + r.setLabels("some-label"); + + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckResourceNames("resource1", null, true, p).getMessage()); + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckResourceNames("resource1", "some-label", false, p).getMessage()); + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckResourceNames("resource1", "some-label", true, p).getMessage()); + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckLabelName("some-label", "resource1", false, p).getMessage()); + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckLabelName("some-label", null, true, p).getMessage()); + assertEquals( + "Only label, groovy expression, or resources can be defined, not more than one.", + d.doCheckLabelName("some-label", "resource1", true, p).getMessage()); + + assertEquals(FormValidation.ok(), d.doCheckResourceNames("resource1", null, false, p)); + assertEquals(FormValidation.ok(), d.doCheckLabelName("some-label", null, false, p)); + assertEquals(FormValidation.ok(), d.doCheckResourceNumber("1", "resource1", null,null, p)); + + assertEquals( + "The following resources do not exist: [resource3]", + d.doCheckResourceNames("${param5} resource3", null, false, p).getMessage()); + assertEquals( + "The following parameters do not exist: [param5, param4]", + d.doCheckResourceNames("${param5} ${param4} resource1", null, false, p).getMessage()); + assertEquals( + "The label does not exist: other-label", + d.doCheckLabelName("other-label", null, false, p).getMessage()); + + assertEquals( + "The following resources cannot be validated as they contain parameter values: [${param1}]", + d.doCheckResourceNames("${param1}", null, false, p).getMessage()); + assertEquals( + "The following resources cannot be validated as they contain parameter values: [xyz_${param1}]", + d.doCheckResourceNames("xyz_${param1}", null, false, p).getMessage()); + assertEquals( + "The label cannot be validated as it contains a parameter value: ${param1}", + d.doCheckLabelName("${param1}", null, false, p).getMessage()); + assertEquals( + "The label cannot be validated as it contains a parameter value: ${param1}${param2}", + d.doCheckLabelName("${param1}${param2}", null, false, p).getMessage()); + assertEquals( + "The label cannot be validated as it contains a parameter value: resource${param2}", + d.doCheckLabelName("resource${param2}", null, false, p).getMessage()); + assertEquals( + "The value cannot be validated as it is a parameter value: ${param1}", + d.doCheckResourceNumber("${param1}", null,null, null, p).getMessage()); + + assertEquals( + "Could not parse the given value as integer.", + d.doCheckResourceNumber("${param1}${param2}", null,null, null, p).getMessage()); + assertEquals( + "Could not parse the given value as integer.", + d.doCheckResourceNumber("Five", null,null, null, p).getMessage()); + + assertEquals( + "Given amount 4 is greater than amount of resources: 1.", + d.doCheckResourceNumber("4", "resource1", null,null, p).getMessage()); + assertEquals( + "Given amount 5 is greater than amount of resources: 1.", + d.doCheckResourceNumber("5", null, "some-label",null, p).getMessage()); + } }