[PROPOSAL] Parameters Support

[Andrew Harmel-Law]

Hi all,

As promised in another thread, here's the proposal for Jenkins Build Parameters support:

To begin with, here is a sample config.xml with examples of setting up all the parameter types currently supported by Jenkins:

<project>

<actions/>

<description/>

<keepDependencies>false</keepDependencies>

<properties>

<hudson.model.ParametersDefinitionProperty>

<parameterDefinitions>

<hudson.model.BooleanParameterDefinition>

<name>booleanValue</name>

<description>ths description of the boolean value</description>

<defaultValue>true</defaultValue>

</hudson.model.BooleanParameterDefinition>

<hudson.model.ChoiceParameterDefinition>

<name>choiceValue</name>

<description>the description of the choice value</description>

<choices class="java.util.Arrays$ArrayList">

<a class="string-array">

<string>choiceA_Default</string>

<string>choiceB</string>

<string>choiceC</string>

</a>

</choices>

</hudson.model.ChoiceParameterDefinition>

<hudson.model.FileParameterDefinition>

<name>the/file/location/relative/to/the/workspace</name>

<description>the description of the file value</description>

</hudson.model.FileParameterDefinition>

<hudson.scm.listtagsparameter.ListSubversionTagsParameterDefinition>

<name>listSvnTagsValue</name>

<description>Select a Subversion entry</description>

<tagsDir>http://kenai.com/svn</tagsDir>

<tagsFilter>theTagFilterRegex</tagsFilter>

<reverseByDate>true</reverseByDate>

<reverseByName>true</reverseByName>

<defaultValue>theDefaultValue</defaultValue>

<maxTags>maxTagsToDisplayValue</maxTags>

<uuid>e434beb2-10dd-4444-a054-44fec8c86ff8</uuid>

</hudson.scm.listtagsparameter.ListSubversionTagsParameterDefinition>

<hudson.model.PasswordParameterDefinition>

<name>passwordValue</name>

<description>the description of the password value</description>

<defaultValue>2PlY1mNgLa3LS0Rn35RTsQAxYSbWbMCi3sS3h6TfS9A=</defaultValue>

</hudson.model.PasswordParameterDefinition>

<hudson.model.RunParameterDefinition>

<name>runValue</name>

<description>the description of the run value</description>

<projectName>2</projectName>

</hudson.model.RunParameterDefinition>

<hudson.model.StringParameterDefinition>

<name>stringValue</name>

<description>the description of the string value</description>

<defaultValue>theDefaultStringValue</defaultValue>

</hudson.model.StringParameterDefinition>

<hudson.model.TextParameterDefinition>

<name>textValue</name>

<description>the description of the text value</description>

<defaultValue>defaultTextValue</defaultValue>

</hudson.model.TextParameterDefinition>

</parameterDefinitions>

</hudson.model.ParametersDefinitionProperty>

</properties>

<scm class="hudson.scm.NullSCM"/>

<canRoam>true</canRoam>

<disabled>false</disabled>

<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>

<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>

<triggers class="vector"/>

<concurrentBuild>false</concurrentBuild>

<builders/>

<publishers/>

<buildWrappers/>

</project>

It would be nice if we could have this in supported in the DSL with something like:

job {

parameters {

boolean (String parameterName, boolean defaultValue(optional - "false" if not specified), String description (optional))

listTags(String parameterName, URL scmUrl, RegexPattern tagFilterRegex, boolean sortNewestFirst (default = "false"), boolean sortZtoA (default = "false"), String defaultValue (optional), int maxTagsToDisplay (optional - "all" if not specified), String description (optional))

choice (String parameterName, String[] {choiceA_Default, choiceB, choiceC}, String description (optional))

file (String parameterName, File fileLocation_relativeToTheWorkspace, String description (optional))

password (String parameterName, String defaultValue(optional), String description (optional))

run(String parameterName, String jobToRun, String description (optional))

string(String parameterName, String defaultValue(optional), String description (optional))

text(String parameterName, String defaultValue(optional), String description (optional))

}

}

This raises the following questions:

1) Passwords are difficult, they aren't stored in plain text in the config.xml, so we don't want to provide them in plain text in our job DSL scripts.  What should we do?

2) How do we (do we?) see ourselves using these elsewhere in the scripts? If so, will we use Ant-like support (${...})

I think the declaration looks nice and clean however.

What do you all think?

Cheers, Andrew

P.S. I've never used this plugin so if I've missed something blindingly obvious, please let me know.  A discussion is always better than me forging off on my own...

Cheers, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

[Daniel Spilker]

Hi Andrew,

+1 for the DSL syntax. We could really need this. Let me now if you need help for implementing or testing.

Passwords are difficult. I would not want them stored in plain text anywhere either. It should be possible to use the Mask Passwords plugin to store the passwords encrypted in the seed job config.xml and to access them in the job DSL script using the environment.

env = Thread.currentThread().executable.getEnvironment(hudson.model.TaskListener.NULL)

password = env["SECRET"]

Jenkins also contains a built-in helper class hudson.util.Secret for encrypting passwords. Maybe we could utilize this to provide a solution in the Job DSL plugin without requiring another plugin.

I'm fine, when we do not have the password parameter in a first version of the build parameters to avoid these problems. It can be added later including a solution to securely specify passwords.

Regards,

Daniel

[Andrew Harmel-Law]

Hi Daniel,

All sounds great.  I think you're right - the password thing might well be the hardest bit of this, and to do a staged release where we support the other types first is a good idea.

Your ideas for storing the passwords securely are good.  Justin might have gone down one of these routes already in some of the code he's written elsewhere (I've not, but I've contributed a whole lot less).  If he has taken some design decisions which we should also follow that'd make sense.  Justin, can you help on this?  (Additionally, if there are, I'll put it up on the wiki on one of the developer pages so its clear for folks in the future.)

That leaves us for now about to collaborate on the everything-but-the-password support.  I'm relatively new to Git/Github.  I guess we should fork the master repo to a place where we can then both clone and contribute to.  Once we're happy with our fork we can request it's merged back in.  If there's another, better, more git-flow-y way of doing things, please let me know.

Once we have that set up we can continue mailing back and forth on this thread to figure out what code we need and how we can divide the work up between us.

Looking forward to working with you on this.

Cheers, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

--
You received this message because you are subscribed to the Google Groups "job-dsl-plugin" group.
To unsubscribe from this group and stop receiving emails from it, send an email to job-dsl-plugi...@googlegroups.com.
To post to this group, send email to job-dsl...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msg/job-dsl-plugin/-/9jWvhEsK5YEJ.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Daniel Spilker]

Hi Andrew,

sounds great. How about using your fork at https://github.com/andrewharmellaw/job-dsl-plugin? On that page is a branch drop-down where you can create a new branch for this effort. Before branching, make sure that your master branch is up to date with the upstream repo so that we do not branch from an older revision. You can add me (daspilker) as a collaborator at https://github.com/andrewharmellaw/job-dsl-plugin/settings/collaboration.

Regards,

Daniel

--
You received this message because you are subscribed to a topic in the Google Groups "job-dsl-plugin" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/job-dsl-plugin/ZEh5QxjHz_g/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to job-dsl-plugi...@googlegroups.com.

To post to this group, send email to job-dsl...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.
 
 

--

    daniel spilker

    ma...@daniel-spilker.com
    +49.179.1093202
    geschwister-scholl-straße 62, 20251 hamburg
    http://daniel-spilker.com

[Andrew Harmel-Law]

Super.  Cheers Daniel.  I'll do that now.

Regs, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

[Andrew Harmel-Law]

Done.  I've run out of time for now, but I guess the next step is to do some quick chatting here around the code and work-split.

Cheers, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

[Justin Ryan]

+1 on the proposal, looks good.

Concerning the password, we can absolutely just punt on it right now. I had to deal with it for p4, svn, git. I just took in a parameter and let the DSL user deal with keeping it secret. The one crux was that P4 has it's own insecure way of encrypting the password. I could have put a dependency on the P4 plugin, but that seem overkill for a 10 line method, so in the end it was just copied. For a core feature like parameters, we might have to start depending on jenkins itself to get access to how they encode the password from clear-text.

Concerning listTags, the arguments data types look wrong. URL is a maybe, but Regex isn't needed though you might to validate the String input. The maxTagsToDisplay field will cause problems, if someone want's 'all' and provide a description. It might have to be an Integer so that people can leave it null to get that 'all' value. Typically if there's a few booleans, I'd try to get them into a context object since a bunch of "true, true, true"'s get confusing, but this case doesn't justify it.

I think you're on the right path. Good luck, and ping the list if you need any help.

[Andrew Harmel-Law]

Great feedback. Cheers Justin. We'll fold it all in. I think we should follow you on the passwords support for now too. We can document it clearly and pos. flag as an extension point for someone in the future unless we get round to it first.

Cheers, Andrew

[Andrew Harmel-Law]

Right, I'm back.  I'm knocking up a stub implementation of BuildParametersContextHelper.groovy so we can divvy up the work effectively.

Look out for the push to the branch on https://github.com/andrewharmellaw/job-dsl-plugin/tree/BuildParameters sometime soon

Cheers, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

[Andrew Harmel-Law]

Update: I got a good chunk done on the train home. I should have something pushed by 8:00AM GMT +1 tomorrow.

Andrew

[Daniel Spilker]

Hi Andrew,

sorry for coming back so late on this. I could not compile my local repo due to the Gradle/Maven dependency hell and I was postponing to fix that...

Your changes look good. I just have a few remarks on the method signatures in BuildParametersContext:

* I think it is not necessary to have a Closure parameter for most parameter types. All options can be specified using the method parameters, so why do we need an extra closure?

* The description should be optional.

* Maybe the default value should be optional as well.

* I am not a Groovy expert, but isn't the usage of the listTagsParam method ambigious? There are two optional boolean parameters next to each other. Which one is used if I specify only one? Maybe that method could use a custom closure the handle the complexity. I would be fine if we skip the listTagsParam in the first version to avoid dealing with the complexity of this parameter.

How shall we proceed? I can offer to work on getting one of the parameter types working end-to-end. After that we can split up the remaining types and work on them in parallel.

Regards,

Daniel

[Daniel Spilker]

Hi Andrew,

I could not resist to write some code. Please have a look at https://github.com/CoreMedia/job-dsl-plugin/commit/cda856107db4bb5f84b23cfa2d2fea8cac66b42c. I committed them to my fork, because I did some refactoring. I can commit that to your fork when you are OK with the changes I made.

Here is what I did in detail:

• The buildParameterNodes list now contains parameter definition nodes (BooleanParameterDefinition, StringParameterDefinition, ...) and the generateWithXmlClosure method creates the project/properties/hudson.model.ParametersDefinitionProperty/parameterDefinitions node and appends the parameter definition notes. I removed the getListOfBuildParameterNodes method since it is no longer needed.
  
• I changed to booleanParam, stringParam and textParam methods to use a common helper method for creating the parameter definition node since the XML for all three parameter types is more or less the same.
  
• I removed the closure parameter from booleanParam, stringParam and textParam because I think it is not needed.
  
• I removed the overloaded variants of booleanParam, stringParam and textParam. The default value and the description are optional, but you have to specify a default value in order to add a description. To be able to specify a description without a defaultValue we have to add a closure, but I could live without that.
  

In a second commit, I implemented the choice parameter: https://github.com/CoreMedia/job-dsl-plugin/commit/6853ccf02402da6c55ff0bebdd8814149eb5a98c. In that commit I also removed the BuildParametersContext constructors because only the default constructor is used.

Regards,

Daniel

[Andrew Harmel-Law]

Hey Daniel,

Welcome back! It all makes sense. I'm on the train at the moment so ill take a look at your commit when I get home. Thanks for all this. Some of the things you point out were already on my list, but most weren't.  The removal of getListWithBuildParameterNodes is especially good - it was really ugly. The further reduction in code and DRY-er result is great too. I love OSS development. :D

One thing we still need to think about is the clash with other types of parameters. It was mentioned in another thread. Ill refresh my memory of that tonight too and post something here for us to discuss further.

Cheers, Andrew

To view this discussion on the web visit https://groups.google.com/d/msg/job-dsl-plugin/-/PTTcDAEgrwMJ.

[Andrew Harmel-Law]

I just looked at the first commit. (turns out browsing virgin on my phone isn't as terrible an idea as first thought.) Beautiful. I am learning all the time. Thanks Daniel. Push it into the shared repo right now if you like.

Ill take a look at your next commit too now.

Cheers, Andrew

To view this discussion on the web visit https://groups.google.com/d/msg/job-dsl-plugin/-/PTTcDAEgrwMJ.

[Andrew Harmel-Law]

... And now the second. Looks superb. I'd push that too. Have you looked at a test for multiple, mixed parameterise definitions? I could give that a try if you like.

Abdrew

[Daniel Spilker]

OK, I pushed both commits. I didn't test multiple/mixed parameter definitions, but that is a good idea. Feel free to add a test ;-)

Regards,

Daniel

[Andrew Harmel-Law]

I'm on it! :)

[Daniel Spilker]

Hi Andrew,

I just pushed support for the file parameter type. The only thing left is the listTagsParam. Do you want to implement that? I could live without it, especially since it seems to be quite complex. How about saving that for later and creating a pull request with the stuff we already have?

Regards,

Daniel

[Andrew Harmel-Law]

excellent.  I agree.  We can flag it as "not implemented yet".  I'll make the request tonight.  Cheers for doing 90% of the work on this!

Regs, Andrew

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

To view this discussion on the web visit https://groups.google.com/d/msg/job-dsl-plugin/-/b5nSrxXiyFQJ.

[Daniel Spilker]

Thanks! Kudos to you for laying the foundation!

Regards,

Daniel

[Andrew Harmel-Law]

Pull request sent.  Watch the skies!

:D

Andrew Harmel-Law

twitter: @al94781

blog: the-music-of-time.blogspot.com

flickr: flickr.com/photos/andrewbrucelaw

To view this discussion on the web visit https://groups.google.com/d/msg/job-dsl-plugin/-/jz8-gU6z1eUJ.