[JIRA] (JENKINS-41308) support Use Groovy Sandbox scripts in activeChoiceParams

20 views
Skip to first unread message

glance+jenkins@acc.umu.se (JIRA)

unread,
Jan 23, 2017, 3:43:02 AM1/23/17
to jenkinsc...@googlegroups.com
Anton Lundin created an issue
 
Jenkins / Bug JENKINS-41308
support Use Groovy Sandbox scripts in activeChoiceParams
Issue Type: Bug Bug
Assignee: Daniel Spilker
Components: job-dsl-plugin
Created: 2017/Jan/23 8:42 AM
Priority: Minor Minor
Reporter: Anton Lundin

activeChoice supports placing scripts in the sandbox, which removes the requirement to have a admin approve the script.

It would be great if job-dsl could support setting the sandbox.

Currently its getting set to the default value, false.

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.1.7#71011-sha1:2526d7c)
Atlassian logo

mail@tknerr.de (JIRA)

unread,
Jan 30, 2017, 3:19:02 PM1/30/17
to jenkinsc...@googlegroups.com
Torben Knerr commented on Bug JENKINS-41308
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

Did anyone ever get this to work with the `configure` block?

I lost a day of my life trying to modify the job xml in a `configure` block to enable the sandbox. Finally I got it so far:

pipelineJob("foo") {

  parameters {
    activeChoiceParam('Param') {
      choiceType('SINGLE_SELECT')
      groovyScript {
        script('return ["a","b","c"]')
      }
    }
  }

  // we need to wrap the active choice parameters' groovy script and fallbackScript
  // inside a secureScript wrapper with the sandbox enabled
  configure { proj ->
    // find all <script class="...GroovyScript"> nodes via depth-first search
    def scriptContainers = proj.'**'.findAll{ n ->
      n.class == Node && n.'@class' == 'org.biouno.unochoice.model.GroovyScript'
    }
    scriptContainers.each { s ->
      // wrap <script> inside a <secureScript>
      s.appendNode('secureScript', [plugin: 'script-...@1.25']).with {
        appendNode('script', s.script.value)
        appendNode('sandbox', 'true')
        parent.remove(s.script)
      }
    }
  }
}

It seems to work (i.e. XML looks ok) on https://job-dsl.herokuapp.com/, but when used in Jenkins it still does not work. The generated config.xml ends up with both <script> and <secureScript> tags side-by-side, i.e. the removal of the script element via {{ parent.remove(s.script) }} does not work when this JobDSL runs in Jenkins.

I had tried a variation with {{ replaceNode }} but this would not work in https://job-dsl.herokuapp.com ("java.lang.UnsupportedOperationException: Replacing the root node is not supported"). It did not throw this exception when run in Jenkins, but also not make any difference – still both elements there and the script needs approval.

Help?!

mail@daniel-spilker.com (JIRA)

unread,
Jan 31, 2017, 3:24:01 AM1/31/17
to jenkinsc...@googlegroups.com
Daniel Spilker resolved as Fixed
 

Use the Automatically Generated DSL:

job('example') {
  parameters {
    choiceParameter {
      name('Param')
      script {
        groovyScript {
          script {
            script('return ["a","b","c"]')
            sandbox(true)
          }
          fallbackScript {
            script('')
            sandbox(false)
          }
        }
      }
      choiceType('SINGLE_SELECT')
      description('example param')
      randomName('param-4711')
      filterable(false)
    }
  } 
}
Change By: Daniel Spilker
Status: Open Resolved
Resolution: Fixed

mail@tknerr.de (JIRA)

unread,
Jan 31, 2017, 6:13:01 AM1/31/17
to jenkinsc...@googlegroups.com
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

Daniel Spilker oh that is possible?

It wasn't showing up on my local instances embedded API viewer, so I thought the automatically generated DSL would not be available. How can I tell if it is?

mail@tknerr.de (JIRA)

unread,
Jan 31, 2017, 6:13:01 AM1/31/17
to jenkinsc...@googlegroups.com

mail@daniel-spilker.com (JIRA)

unread,
Jan 31, 2017, 6:18:03 AM1/31/17
to jenkinsc...@googlegroups.com
Daniel Spilker commented on Bug JENKINS-41308
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

Torben Knerr You must use the choiceParameter instead of activeChoiceParam. choiceParameter is provided by the Automatically Generated DSL, activeChoiceParam is the built-in DSL.

mail@tknerr.de (JIRA)

unread,
Jan 31, 2017, 6:22:03 AM1/31/17
to jenkinsc...@googlegroups.com

mail@tknerr.de (JIRA)

unread,
Jan 31, 2017, 6:23:02 AM1/31/17
to jenkinsc...@googlegroups.com
Torben Knerr commented on Bug JENKINS-41308
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

Daniel Spilker nevermind, I found it. Did not notice that the generated parameter is named differently ("choiceParameter"):

Your example from above is working (in that it generates a valid config.xml at least) and gets me a big step further.

Thanks a lot!

DanaGoyette@gmail.com (JIRA)

unread,
Feb 9, 2017, 7:04:01 PM2/9/17
to jenkinsc...@googlegroups.com

Using automatically generated DSL is a workaround, not a fix. We need real / direct support for enabling sandboxing.

If I try to use automatically generated DSL, then "mvn test" in my dsl source fails, and the only examples of tests of automatically-generated DSL use gradle, not maven.

mail@daniel-spilker.com (JIRA)

unread,
Feb 16, 2017, 3:17:07 PM2/16/17
to jenkinsc...@googlegroups.com

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 3:58:03 AM12/10/19
to jenkinsc...@googlegroups.com
Daniel Steiert reopened an issue
Change By: Daniel Steiert
Resolution: Fixed
Status: Closed Reopened
Assignee: Daniel Spilker Steiert
This message was sent by Atlassian Jira (v7.13.6#713006-sha1:cc4451f)
Atlassian logo

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 4:30:03 AM12/10/19
to jenkinsc...@googlegroups.com
Daniel Steiert commented on Bug JENKINS-41308
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

To fix this issue, I created a PR according to the guidelines with the addition of the sandbox option for scripts. I made sure all the old functionality is untouched (works the same) and added tests for the new options. The build should also go through (https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fjob-dsl-plugin/detail/PR-1211/1/pipeline) and I tested it on my own Jenkins instance. Please review and let me know if you need any changes before it can be merged.

The PR can be found here: https://github.com/jenkinsci/job-dsl-plugin/pull/1211

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 4:59:05 AM12/10/19
to jenkinsc...@googlegroups.com

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 4:59:05 AM12/10/19
to jenkinsc...@googlegroups.com

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 5:08:02 AM12/10/19
to jenkinsc...@googlegroups.com
Daniel Steiert edited a comment on Bug JENKINS-41308
To fix this issue, I created a PR according to the guidelines with the addition The documentation of the {{sandbox}} option for scripts. I made sure all the old functionality course also is untouched (works the same) and added tests for the new options updated . The build should also go through ([https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fjob-dsl-plugin/detail/PR-1211/1/pipeline)] and I tested it on my own Jenkins instance. Please review and let me know if you need any changes before it can be merged.

daniel.steiert@mail.de (JIRA)

unread,
Dec 10, 2019, 5:11:03 AM12/10/19
to jenkinsc...@googlegroups.com
Daniel Steiert edited a comment on Bug JENKINS-41308
The To fix this issue, I created a PR ([https://github.com/jenkinsci/job-dsl-plugin/pull/1211)] with a green build ([https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fjob-dsl-plugin/detail/PR-1211/1/pipeline/).] All the old functionality stays unchanged and only the `sandbox` option was added in. There are tests, documentation of course also is updated and the contribution guidelines have been followed .

Please have a look and let me know if any changes need to be made before this can be integrated.

 

Please have a look at the PR

michael.tupitsyn@gmail.com (JIRA)

unread,
Mar 2, 2020, 2:41:03 PM3/2/20
to jenkinsc...@googlegroups.com

Is there any way we can expedite the PR merge? The workaround with configure block works, but it introduces performance implication (with configure block, DSL plugin re-formats config XML with every run and restores some optional element and attributes).

This message was sent by Atlassian Jira (v7.13.12#713012-sha1:6e07c38)
Atlassian logo

michael.tupitsyn@gmail.com (JIRA)

unread,
Mar 2, 2020, 2:43:05 PM3/2/20
to jenkinsc...@googlegroups.com
Michael Tupitsyn edited a comment on Bug JENKINS-41308
Is there any way we can expedite the PR merge? The workaround with configure block works, but it introduces performance implication (with configure block, DSL plugin re-formats config XML with every run and restores some optional element elements and attributes).

mail@daniel-spilker.com (JIRA)

unread,
Mar 6, 2020, 9:01:04 AM3/6/20
to jenkinsc...@googlegroups.com
Daniel Spilker closed an issue as Fixed
 

Please use Dynamic DSL or a configure block if Dynamic DSL is not available in your environment. I'm no longer accepting changes for options that are available in Dynamic DSL.

Change By: Daniel Spilker
Status: Open Closed
Resolution: Fixed

daniel.steiert@mail.de (JIRA)

unread,
Mar 9, 2020, 4:01:10 AM3/9/20
to jenkinsc...@googlegroups.com
Daniel Steiert commented on Bug JENKINS-41308
 
Re: support Use Groovy Sandbox scripts in activeChoiceParams

It is actually not fully supported by Dynamic DSL and causes problems like not showing up or not being able to be used due to conflicts. The Pull Request works, there is obviously a real need for this fix (see all the people having issues with Dynamic DSL and the ugliness of the Configure block (causing other problems for example with other plugins).
Existing functionality and tests are unchanged with the PR.

What is the real reason this is being closed?

adam.gabrys@live.com (JIRA)

unread,
Mar 9, 2020, 4:43:04 AM3/9/20
to jenkinsc...@googlegroups.com

It is possible to use Dynamic DSL. Unfortunately, it has two disadvantages:

  • it forces to set all parameters, even those which we don't want to set (the default values are fine for us). This behavior is also a cause that the code could stop working after the 3rd-party plugin update (for example a new field has been added)
  • the code which does simple stuff is much longer and less readable

Examples:

  • Dynamic DSL:
    choiceParameter {
        delegate.name(name)
        delegate.description(description)
        delegate.choiceType('PT_RADIO')
        delegate.script {
            groovyScript {
                script {
                    script(valuesToString())
                    sandbox(true)
                }
                fallbackScript{
                    script('')
                    sandbox(true)
                }
            }
        }
        delegate.randomName('')
        delegate.filterable(false)
        delegate.filterLength(0)
    }
    
  • Static DSL:
    activeChoiceParam(name) {
        delegate.description(this.description)
        delegate.choiceType('RADIO')
        delegate.groovyScript {
            script(valuesToString()) {
                sandbox()
            }
        }
    }
    

As a user I would be really happy to have this feature merged.

From the other side, if you don't want to merge anything related to static DSL, it would be good if you could mark it as deprecated. Keeping it in the current state confuses the users. They use and even create PRs to the code which is frozen (never will be changed - according to the statement that all PRs are declined).

Reply all
Reply to author
Forward
0 new messages