Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-46854] Do parsing pass on script/expression blocks in …
…validation

This doesn't catch all possible script/expression errors that'll show
up in full Groovy parsing and validation, but it'll catch errors like
mismatched quotes in JSON form and report back their location and that
compilation failed. Getting a more granular error would be too much
text, so I opted for just the "hey, this wasn't valid Groovy syntax"
approach.
  • Loading branch information
abayer committed Nov 9, 2017
1 parent b139483 commit 54b18a3
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 5 deletions.
Expand Up @@ -14,6 +14,6 @@ public ModelASTScriptBlock(Object sourceLocation) {

@Override
public void validate(final ModelValidator validator) {
// no-op - we don't do validation of script blocks
validator.validateElement(this);
}
}
Expand Up @@ -37,6 +37,6 @@ public ModelASTWhenExpression(Object sourceLocation) {

@Override
public void validate(ModelValidator validator) {
// no-op - we don't do validation of script blocks
validator.validateElement(this);
}
}
Expand Up @@ -32,6 +32,9 @@ import hudson.tools.ToolDescriptor
import hudson.tools.ToolInstallation
import hudson.util.EditDistance
import jenkins.model.Jenkins
import org.codehaus.groovy.control.CompilationFailedException
import org.codehaus.groovy.control.CompilationUnit
import org.codehaus.groovy.control.Phases
import org.codehaus.groovy.runtime.ScriptBytecodeAdapter
import org.jenkinsci.plugins.pipeline.modeldefinition.DescriptorLookupCache
import org.jenkinsci.plugins.pipeline.modeldefinition.Messages
Expand All @@ -50,7 +53,6 @@ import org.jenkinsci.plugins.structs.describable.DescribableModel
import org.jenkinsci.plugins.structs.describable.DescribableParameter
import org.jenkinsci.plugins.workflow.flow.FlowExecution

import javax.annotation.CheckForNull
import javax.annotation.Nonnull

/**
Expand Down Expand Up @@ -359,7 +361,17 @@ class ModelValidatorImpl implements ModelValidator {
private boolean validateStep(ModelASTStep step, DescribableModel<? extends Describable> model, Descriptor desc) {

if (step instanceof AbstractModelASTCodeBlock) {
// No validation needed for code blocks like expression and script
// Verify that the code block can be parsed - we'll still get garbage for errors around class imports, etc,
// but you can't do that from the editor anyway.
String codeBlock = step.codeBlockAsString()
CompilationUnit cu = new CompilationUnit()
cu.addSource(step.name, codeBlock)
try {
cu.compile(Phases.PARSING)
} catch (CompilationFailedException e) {
errorCollector.error(step, Messages.ModelValidatorImpl_CompilationErrorInCodeBlock(step.name))
return false
}
return true
} else {
return validateDescribable(step, step.name, step.args, model, lookup.stepTakesClosure(desc))
Expand All @@ -375,7 +387,7 @@ class ModelValidatorImpl implements ModelValidator {
Descriptor desc = lookup.lookupStepFirstThenFunction(step.name)
DescribableModel<? extends Describable> model = lookup.modelForStepFirstThenFunction(step.name)

if (model != null) {
if (model != null || step instanceof AbstractModelASTCodeBlock) {
valid = validateStep(step, model, desc)
}
}
Expand Down
Expand Up @@ -142,6 +142,7 @@ ModelValidatorImpl.BothStagesAndSteps=Only one of "parallel" or "steps" allowed
ModelValidatorImpl.AgentInNestedStages="agent" is not allowed in stage "{0}" as it contains parallel stages
ModelValidatorImpl.ToolsInNestedStages="tools" is not allowed in stage "{0}" as it contains parallel stages
ModelValidatorImpl.NoNestedWithinNestedStages=Parallel stages or branches can only be included in a top-level stage.
ModelValidatorImpl.CompilationErrorInCodeBlock=Groovy compilation error in {0}

WhenConditionalValidator.changelog.missingParameter=Changelog is missing required parameter "pattern".
WhenConditionalValidator.changelog.badPattern="{0}" is not a valid regular expression. {1}
Expand Down
Expand Up @@ -107,4 +107,10 @@ public void bareDollarCurly() throws Exception {
findErrorInJSON(Messages.ModelParser_BareDollarCurly("${env.BUILD_NUMBER}"), "bareDollarCurly");
findErrorInJSON(Messages.ModelParser_BareDollarCurly("${FOO}"), "bareDollarCurly");
}

@Issue("JENKINS-46854")
@Test
public void quoteRoundTripping() throws Exception {
findErrorInJSON(Messages.ModelValidatorImpl_CompilationErrorInCodeBlock("script"), "quoteRoundTripping");
}
}
@@ -0,0 +1,64 @@
{"pipeline": {
"stages": [ {
"name": "Each of these takes ten seconds",
"parallel": [
{
"name": "first",
"branches": [ {
"name": "default",
"steps": [
{
"name": "echo",
"arguments": [ {
"key": "message",
"value": {
"isLiteral": true,
"value": "First branch"
}
}]
},
{
"name": "script",
"arguments": [ {
"key": "scriptBlock",
"value": {
"isLiteral": true,
"value": "sh 'for i in `seq 1 10`; do echo \"Sleeping 1S\"; sleep 1;' done'"
}
}]
}
]
}]
},
{
"name": "second",
"branches": [ {
"name": "default",
"steps": [
{
"name": "echo",
"arguments": [ {
"key": "message",
"value": {
"isLiteral": true,
"value": "Second branch"
}
}]
},
{
"name": "script",
"arguments": [ {
"key": "scriptBlock",
"value": {
"isLiteral": true,
"value": "sh 'for i in `seq 1 10`; do echo \"Sleeping 1S\"; sleep 1; done'"
}
}]
}
]
}]
}
]
}],
"agent": {"type": "any"}
}}

0 comments on commit 54b18a3

Please sign in to comment.