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)

    Details

    • Similar Issues:

      Description

      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.

        Attachments

          Issue Links

            Activity

            mdkf Michael Fowler created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Assignee Oleg Nenashev [ oleg_nenashev ] Jesse Glick [ jglick ]
            Hide
            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.

            Show
            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.
            jglick Jesse Glick made changes -
            Summary Custom groovy script for promoted builds not working on windows Custom groovy script for promoted builds always including classpath entry
            Hide
            jglick Jesse Glick added a comment -

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

            Show
            jglick Jesse Glick added a comment - Also reproduced in 1.580.1. But only with Promoted Builds; not, for example, with Groovy Post-Build.
            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            jglick Jesse Glick added a comment -

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

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

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

            Show
            jglick Jesse Glick added a comment - Filed a PR in Script Security to fail fast and report the issue number.
            jglick Jesse Glick made changes -
            Remote Link This issue links to "script-security PR 84 (Web Link)" [ 14864 ]
            jglick Jesse Glick made changes -
            Summary Custom groovy script for promoted builds always including classpath entry f:repeatable mishandles minimum (was: Custom Groovy Script for Promoted Builds always includes bogus classpath entry)
            jglick Jesse Glick made changes -
            Labels jelly
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
            src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
            http://jenkins-ci.org/commit/script-security-plugin/246c22ed034258a4808da0396cb70413ae39c324
            Log:
            JENKINS-37599 Prevent empty classpath entries from being saved accidentally.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java http://jenkins-ci.org/commit/script-security-plugin/246c22ed034258a4808da0396cb70413ae39c324 Log: JENKINS-37599 Prevent empty classpath entries from being saved accidentally.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
            src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
            src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
            http://jenkins-ci.org/commit/script-security-plugin/07d1063094fdef9e090d31e6527e63f3910301cd
            Log:
            Merge pull request #84 from jglick/empty-ClasspathEntry-JENKINS-37599

            JENKINS-37599 Prevent empty classpath entries from being saved accidentally

            Compare: https://github.com/jenkinsci/script-security-plugin/compare/feb65ce6d6c2...07d1063094fd

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java http://jenkins-ci.org/commit/script-security-plugin/07d1063094fdef9e090d31e6527e63f3910301cd Log: Merge pull request #84 from jglick/empty-ClasspathEntry- JENKINS-37599 JENKINS-37599 Prevent empty classpath entries from being saved accidentally Compare: https://github.com/jenkinsci/script-security-plugin/compare/feb65ce6d6c2...07d1063094fd
            Hide
            mdkf Michael Fowler added a comment -

            Jesse Glick Thanks for looking into this. Your proposed work around didn't work for me. Even if the class path was not shown in the UI, it was saved into config.xml thus causing the build to fail.

            Show
            mdkf Michael Fowler added a comment - Jesse Glick Thanks for looking into this. Your proposed work around didn't work for me. Even if the class path was not shown in the UI, it was saved into config.xml thus causing the build to fail.
            Hide
            emoshaya_cognitoiq Ebrahim Moshaya added a comment -

            I have the same issue

            Show
            emoshaya_cognitoiq Ebrahim Moshaya added a comment - I have the same issue
            Hide
            emoshaya_cognitoiq Ebrahim Moshaya added a comment -

            Jesse Glick Any update on this?

            Show
            emoshaya_cognitoiq Ebrahim Moshaya added a comment - Jesse Glick Any update on this?
            fchuong Frédéric Chuong made changes -
            Link This issue is duplicated by JENKINS-42725 [ JENKINS-42725 ]
            Hide
            valeriefdes Valerie Fernandes added a comment - - edited

            Jesse Glick : we also have this issue. Any update when it will be resolved?

             

            Show
            valeriefdes Valerie Fernandes added a comment - - edited Jesse Glick : we also have this issue. Any update when it will be resolved?  
            Hide
            jglick Jesse Glick added a comment -

            Ebrahim Moshaya Valerie Fernandes please do not add “me-too” comments. If an issue is not marked In Progress, no one is working on it. It may or may not ever be resolved. There is far more code in the Jenkins source base than there are (competent) developers to fix even serious bugs. Use the “votes” feature in JIRA to silently register your interest, and/or file a PR with a tested fix if you have Jenkins plugin development expertise.

            Show
            jglick Jesse Glick added a comment - Ebrahim Moshaya Valerie Fernandes please do not add “me-too” comments. If an issue is not marked In Progress, no one is working on it. It may or may not ever be resolved. There is far more code in the Jenkins source base than there are (competent) developers to fix even serious bugs. Use the “votes” feature in JIRA to silently register your interest, and/or file a PR with a tested fix if you have Jenkins plugin development expertise.
            Show
            gverdoliva Giuseppe Verdoliva added a comment - Fix this issue: https://github.com/jenkinsci/script-security-plugin/pull/208
            jglick Jesse Glick made changes -
            Link This issue is duplicated by JENKINS-49487 [ JENKINS-49487 ]
            ikedam ikedam made changes -
            Assignee ikedam [ ikedam ]
            Hide
            ikedam ikedam added a comment -

            This looks an issue of Jenkins core rather than that of script-security or promoted-build, and should be fixed in core.

            Show
            ikedam ikedam added a comment - This looks an issue of Jenkins core rather than that of script-security or promoted-build, and should be fixed in core.
            ikedam ikedam made changes -
            Link This issue is related to JENKINS-44824 [ JENKINS-44824 ]
            Show
            ikedam ikedam added a comment - https://github.com/jenkinsci/jenkins/pull/3583
            ikedam ikedam made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            ikedam ikedam made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            dnusbaum Devin Nusbaum made changes -
            Released As Jenkins Core 2.139
            Hide
            dnusbaum Devin Nusbaum added a comment -

            Looks like the fix was recently released in Jenkins 2.139. Thanks ikedam!

            Show
            dnusbaum Devin Nusbaum added a comment - Looks like the fix was recently released in Jenkins 2.139 . Thanks ikedam !
            dnusbaum Devin Nusbaum made changes -
            Status In Review [ 10005 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            dnusbaum Devin Nusbaum made changes -
            Released As Jenkins Core 2.139 https://jenkins.io/changelog/#v2.139
            danielbeck Daniel Beck made changes -
            Labels jelly jelly lts-candidate
            olivergondza Oliver Gondža made changes -
            Labels jelly lts-candidate 2.138.1-fixed jelly
            olivergondza Oliver Gondža made changes -
            Labels 2.138.1-fixed jelly 2.138.2-fixed jelly

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: