Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-37599

f:repeatable mishandles minimum (was: Custom Groovy Script for Promoted Builds always includes bogus classpath entry)

      When trying to add a custom groovy script to a promotion process on windows, an invalid classpath is set preventing the script from running. If you delete the classpath, it returns on the next save of the project.

          [JENKINS-37599] f:repeatable mishandles minimum (was: Custom Groovy Script for Promoted Builds always includes bogus classpath entry)

          Michael Fowler created issue -
          Jesse Glick made changes -
          Assignee Original: Oleg Nenashev [ oleg_nenashev ] New: Jesse Glick [ jglick ]

          Jesse Glick added a comment -

          Reproduced on Linux in 2.7.3. The repeatable list is always showing an entry even where there should be none.

          Jesse Glick added a comment - Reproduced on Linux in 2.7.3. The repeatable list is always showing an entry even where there should be none.
          Jesse Glick made changes -
          Summary Original: Custom groovy script for promoted builds not working on windows New: Custom groovy script for promoted builds always including classpath entry

          Jesse Glick added a comment -

          Also reproduced in 1.580.1. But only with Promoted Builds; not, for example, with Groovy Post-Build.

          Jesse Glick added a comment - Also reproduced in 1.580.1. But only with Promoted Builds; not, for example, with Groovy Post-Build.

          Jesse Glick added a comment -

          I think I know what is going on. This code sets minimum=1, intended for the activeItems entry. This code does not bother specifying a minimum, since the default is 0, which is what we want. But the variable is mistakenly inherited across forms. Really this should be using attrs.minimum, which this should also propagate.

          Can be worked around in script-security via

          diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          index d60c722..3154e4d 100644
          --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly
          @@ -34,6 +34,6 @@ THE SOFTWARE.
                   <f:checkbox title="${%Use Groovy Sandbox}" default="${!h.hasPermission(app.RUN_SCRIPTS)}" />
               </f:entry>
               <f:entry title="${%Additional classpath}" field="classpath">
          -        <f:repeatableProperty add="${%Add entry}" header="${%Classpath entry}" field="classpath"/>
          +        <f:repeatableProperty add="${%Add entry}" header="${%Classpath entry}" field="classpath" minimum="0"/>
               </f:entry>
           </j:jelly>
          

          but really this plugin is not at fault, and there may be many other promotion conditions which also suffer. A more tolerable workaround would be in promoted-builds, probably specifying

          <j:set var="minimum" value="0"/>
          

          inside the repeatable blocks in JobPropertyImpl/config.jelly and PublisherImpl/config.jelly, the two places where minimum is specified. But should also be fixed in core.

          Jesse Glick added a comment - I think I know what is going on. This code sets minimum=1 , intended for the activeItems entry. This code does not bother specifying a minimum , since the default is 0, which is what we want. But the variable is mistakenly inherited across forms. Really this should be using attrs.minimum , which this should also propagate. Can be worked around in script-security via diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly index d60c722..3154e4d 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/config.jelly @@ -34,6 +34,6 @@ THE SOFTWARE. <f:checkbox title= "${%Use Groovy Sandbox}" default= "${!h.hasPermission(app.RUN_SCRIPTS)}" /> </f:entry> <f:entry title= "${%Additional classpath}" field= "classpath" > - <f:repeatableProperty add= "${%Add entry}" header= "${%Classpath entry}" field= "classpath" /> + <f:repeatableProperty add= "${%Add entry}" header= "${%Classpath entry}" field= "classpath" minimum= "0" /> </f:entry> </j:jelly> but really this plugin is not at fault, and there may be many other promotion conditions which also suffer. A more tolerable workaround would be in promoted-builds , probably specifying <j:set var= "minimum" value= "0" /> inside the repeatable blocks in JobPropertyImpl/config.jelly and PublisherImpl/config.jelly , the two places where minimum is specified. But should also be fixed in core.

          Jesse Glick added a comment -

          Not exactly critical since there is a straightforward workaround: remove the stray path when you (re-)configure the project.

          Jesse Glick added a comment - Not exactly critical since there is a straightforward workaround: remove the stray path when you (re-)configure the project.
          Jesse Glick made changes -
          Component/s New: core [ 15593 ]
          Component/s Original: script-security-plugin [ 18520 ]
          Assignee Original: Jesse Glick [ jglick ]
          Priority Original: Critical [ 2 ] New: Major [ 3 ]

          Jesse Glick added a comment -

          Filed a PR in Script Security to fail fast and report the issue number.

          Jesse Glick added a comment - Filed a PR in Script Security to fail fast and report the issue number.
          Jesse Glick made changes -
          Remote Link New: This issue links to "script-security PR 84 (Web Link)" [ 14864 ]

            ikedam ikedam
            mdkf Michael Fowler
            Votes:
            8 Vote for this issue
            Watchers:
            14 Start watching this issue

              Created:
              Updated:
              Resolved: