Skip to content
Permalink
Browse files

[FIXED JENKINS-39011] Error out at parse time when pipeline step is n…

…ested.

We don't want to support nesting the pipeline step within other blocks
- until JENKINS-38152, we never noticed that this was actually
possible, since the parse-time validation just ignored any Jenkinsfile
without a root-level pipeline step, but with JENKINS-38152, we not
only do another parse round but need to grab the stages from the model
to attach to the run, so we hit an NPE when there's a pipeline step
nested under other blocks.

So - this will now catch when the pipeline step is nested within
another block and give an error message at parse-time, rather than
NPEing at runtime. There is a valid use case for wanting to wrap the
entire build in something like a timeout or timestamper, but we're
going to address that with JENKINS-37823.
  • Loading branch information...
abayer committed Oct 20, 2016
1 parent 55b7014 commit f25d40a2a5ffe0469ce80b070610d07fcdd3bb2a
@@ -112,6 +112,19 @@ class ModelParser {
}
}

public void checkForNestedPipelineStep(Statement statement) {
def b = matchBlockStatement(statement)
if (b != null) {
if (b.methodName == ModelStepLoader.STEP_NAME) {
ModelASTPipelineDef p = new ModelASTPipelineDef(statement)
errorCollector.error(p, "pipeline block must be at the top-level, not within another block.")
}
eachStatement(b.body.code) { s ->
checkForNestedPipelineStep(s)
}
}
}

/**
* Given a Groovy AST that represents a parsed source code, parses
* that into {@link ModelASTPipelineDef}
@@ -122,7 +135,12 @@ class ModelParser {
def pst = src.statementBlock.statements.find {
matchMethodCall(it)?.methodAsString == ModelStepLoader.STEP_NAME
}
if (pst==null) return null; // no 'pipeline', so this doesn't apply

if (pst==null) {
// Check if there's a 'pipeline' step somewhere nested within the other statements and error out if that's the case.
src.statementBlock.statements.each { checkForNestedPipelineStep(it) }
return null; // no 'pipeline', so this doesn't apply
}

ModelASTPipelineDef r = new ModelASTPipelineDef(pst);

@@ -32,6 +32,7 @@
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;

/**
* @author Andrew Bayer
@@ -48,6 +49,14 @@ public static void setUpAgent() throws Exception {

}

@Issue("JENKINS-39011")
@Test
public void pipelineStepWithinOtherBlockFailure() throws Exception {
prepRepoWithJenkinsfile("errors", "pipelineStepWithinOtherBlocksFailure");

assertFailWithError("pipeline block must be at the top-level, not within another block");
}

@Test
public void rejectStageInSteps() throws Exception {
prepRepoWithJenkinsfile("errors", "rejectStageInSteps");
@@ -0,0 +1,38 @@
/*
* The MIT License
*
* Copyright (c) 2016, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
timeout(time: 5, unit: 'SECONDS') {
pipeline {
agent none
stages {
stage("foo") {
steps {
echo "hello"
}
}
}
}
}



0 comments on commit f25d40a

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.