-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Declarative Pipeline Syntax Within Jenkins Templating Engine #403
Conversation
Looks good to me. |
hey @bitwiseman - we're getting close to the JTE 2.0 release. This PR can be merged at your earliest convenience! Please let me know if there's anything further you would like me to do for this. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no tests, but we agreed that was not required for this change as it is more of an unblocking than full support. To implement tests for this, I'd probably want to add a PCT run to the CI system that runs tests in JTE against a particular change of this plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@steven-terrana
We don't need JTE to exercise this code.
Can you add a test for a pipeline that looks like this:
try{
pipeline {
... // very simple valid pipeline
}
} catch {
// error echo
}
Maybe two tests one where the pipeline throws and error and one where it doesn't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see, you already tried that (inside collapsed comments). Yes, let's circle back to that. 😄
hey @bitwiseman - JTE Would it be possible to cut a release that includes this merge so that pipeline templates written in declarative syntax work? happy to help if there's any way i can! |
JENKINS issue(s): n/a
Hey @bitwiseman, opening a Draft PR to start the conversation around supporting Declarative Syntax from a pipeline template in the Jenkins Templating Engine.
Description
This PR allows users of the Jenkins Templating Engine to write pipeline templates using the Declarative Syntax.
Pending Considerations
Scoping this change to just JTE
I was expecting to have to make a change to Declarative's
GroovyShellDecoratorImpl
to include a logical check for whether or not theFlowDefinition
that led to theCpsFlowExecution
came from the Jenkins Templating Engine (by being a subclass ofTemplateFlowDefinition
).This ended up not being necessary, since ultimately a pipeline template is just a regular Jenkinsfile. JTE works by adding things to the pipeline's runtime environment by way of some customizations to the script binding and some compile-time metaprogramming to restructure the user-provided template a bit.
Because both plugins play at compile time, i added a > 0 ordinal when registering the
GroovyShellDecorator
on the JTE side to make sure that modifications JTE makes always happen prior to the Model Parsing logic in Declarative.My concern right now is that there is no specific check ensuring that this update to how Declarative looks for a
pipeline{}
block only applies to JTE pipelines. My knowledge of AST transformations is not great but I have not been able to trick a non-JTE Jenkinsfile into parsing apipeline{}
block nested within a try-catch. It seems that the way JTE modifies the pipeline script, the first statement is always aBlockStatement
whose last statement is aTryCatchStatement
. I have been unable to reproduce this AST structure outside of a JTE pipeline. When writing:The result is that the first statement in the AST is a
TryCatchStatement
instead of aBlockStatement
and therefore the parsing does not occur. This is the behavior we likely want, even if i'm not 100% understanding why this is the case.Unrelated bug in model parsing for try-catch
In testing, i found that if you create a regular Jenkinsfile such as:
The error produced is:
groovy.lang.MissingPropertyException: No such property: any for class: groovy.lang.Binding
.I believe this is because the ModelInterpreter is invoking the closure passed to the
pipeline
block even though that closure has not been modified to return aRoot
.This might be a separate bug i could attempt to fix in another Issue/PR where we validate that the model has been parsed prior to attempting to execute the closure.
JTE Library Step Resolution
With this change, i was able to successfully run a pipeline template written in declarative syntax using JTE.
Using some dummy JTE libraries that just use print statements:
Pipeline Configuration
Pipeline Template
Build Log
The only difference in behavior i've noticed so far in a Declarative Pipeline vs Scripted Template arises when a JTE library step has been loaded that has the same name as an actual pipeline step (for example,
build
).If a
build
step had been loaded from a JTE library:build
step would take precedence over the actual pipelinebuild
stepbuild
step directly from thesteps{}
block results in the actual pipelinebuild
step being executedsteps{ script{ build() } }
results in the JTEbuild
step being executedI think this behavior is probably an acceptable difference as long as it's documented on the JTE side.