Closed timja closed 6 years ago
timja
commented
6 years ago It would also have made a huge amount of sense to offer a tool which did a census of the jobs already written, reading them and checking for unserializables, rather than having us discover this only when we try to run. Might also have helped you flush out the issues before releasing.
timja
commented
6 years ago > It would also have made a huge amount of sense to offer a tool which did a census of the jobs already written, reading them and checking for unserializables, rather than having us discover this only when we try to run. Might also have helped you flush out the issues before releasing.
Do not disagree. We actually have such tool: https://github.com/jenkinsci/plugin-compat-tester . Hundreds of plugins have been tested with it, but we can test only plugins using modern Plugin POM and having proper test automation. Extended choice was briefly tested from what I see, but we had a limited test coverage since the plugin has zero autotests.
The next step to improve testing for us is to adopt Telemetry API in order to get heads up from live instances before the release (https://github.com/jenkinsci/jep/tree/master/jep/308, CC batmat and rtyler).
> java.lang.UnsupportedOperationException: Refusing to marshal com.cwctravel.hudson.plugins.extended_choice_parameter.ExtendedChoiceParameterDefinition for security reasons;
This is a thing I do not understand. This class is a part of the plugin, and it should be permitted. So something weird is going on. If it happens, most likely the plugin is bundled as a library in another plugin. It could make classloader crazy and finally lead to this issue.
Please provide a full list of plugins you have installed. I also need to know versions of Custom Tools and Extended Choice Parameter plugin versions
timja
commented
6 years ago > Y'know, you REALLY should have implemented this change first in Warning/Deprecation mode, giving us a switch to turn on hard whitelisting when we had achieved a stable environment under the new rules. Oh well.
Maybe. The concern was that any discovered security issue would have became publicly visible then. So such rollout was an intentional decision, and we did our best to notify users about potential issues.
We have a switch which allows to opt-out from the logic. In your case I am not sure it is going to help, because something seems to go REALLY wrong on the instance
timja
commented
6 years ago As far as I know, we're just invoking ExtendedChoiceParameter normally. But we did have to patch the jarfile to fix its rendering of Text Boxes (as shipped it gratutiously limits their width to a specific number of pixels, which is both the wrong approach and far too short for values like paths; we kick it back to 100% and everything is much happier), so if you're keying the approval on jarfile checksums, we're breaking that.
List of currently installed plugins: I'll try to get that to you, but I don't think it's relevant to this specific issue.
timja
commented
6 years ago keshlam "But we did have to patch the jarfile". If you modified this plugins and added it is JAR, JEP-200 engine will not work. Plugins must be always packaged as HPIs or JPIs, all other use-cases are not supported
timja
commented
6 years ago Needs to happen with actual unmodified builds of the plugin (even private snapshots) to be relevant. If this is limited to affecting manually put together jars, I wouldn't consider this a problem.
timja
commented
6 years ago The modification is to unpack the product HPI, change one line in a template file, and repack. I can give you a script which applies the change if you want to check whether this is what caused the issue.
For now, I've rolled back to earlier Jenkins.
timja
commented
6 years ago keshlam a script and the produced HPI would be helpful. Preferably it would be great to have a Docker image which reproduces the issue, or some guidelines about how to reproduce it with the script you provide.
Most likely the HPI is missing some manifest attributes so that ClassFilter logic does not recognize it as a plugin
timja
commented
6 years ago #!/bin/bash
#
# Kluge to fix the Extended Choice Parameter.
#
# ECP's "text box" mode, for whatever reason, was hardcoded to use a
# 200-pixel-wide entry box. Using absolute values in HTML rendering
# sizes is almost always wrong; in this case it means the box will
# be about 30 characters wide – painfully small if the parameter
# value is intended to be a path.
#
# Luckily, this can be fixed by changing one text file inside the
# plugin's jar, replacing that "width:200px" with "width:100%".
# This value will use as much space as the parent rendering element
# is willing to give it.
#
# Caveat: Pasting this into Jira may have glitched some line breaks...
#
# Debug trace, if desired
#set -xv######################################################################function restart_server {
if [ -z $USER ]; then
USER=${jenkinsUser}
fi
if [ -z $PASSWORD ]; then
PASSWORD=${jenkinsApiToken}
fi
if [ -z $USER -o -z $PASSWORD ]; then
echo "User and/or password not provided. Please restart Jenkins manually to pick up modified jarfile."
{{ else }}
CRUMB=`curl -s -u ${USER}:${PASSWORD} "${JENKINS_URL}/crumbIssuer/api/xml?xpath=concat(//crumbRequestField,\":\",//crumb)"`
if [[ ${CRUMB} != "Jenkins-Crumb:"* ]]; then
echo "Crumb not successfully retrieved; continuing without"
CRUMB=""
else
CRUMB="-H $CRUMB"
fi
curl -s -X POST -u $USER:$PASSWORD $CRUMB ${JENKINS_URL}/safeRestart
if [ $? -eq 0 ]; then
echo "Restart requested. There will be a delay for quiescance before restart occurs."
else
echo "User and password not accepted. Please restart Jenkins manually to pick up modified jarfile."
fi
fi
}######################################################################
# Default assumption: success
{{rc=0 }}
# Files to be patched: Jarfile, and path within it
ecpJarfile=$JENKINS_HOME/plugins/extended-choice-parameter/WEB-INF/lib/extended-choice-parameter.jar
textBox="com/cwctravel/hudson/plugins/extended_choice_parameter/ExtendedChoiceParameterDefinition/textboxContent.jelly"
# Get a scratch space
tmpdir=$(mktemp -d)
cd $tmpdir
# Unpack the jarfile for patching
jar xvf $ecpJarfile
rc=$?
if [ $rc -ne 0 ]; then
echo "Couldn't unpack $ecpJarfile, rc=$rc"
else
# Check whether this has already been fixed. (We can't count on
# exit status of sed; it thinks "no match" on a global replace is
# a successful operation.)
grep "width:200px" $textBox
rrc=$?
if [ $rrc -ne 0 ]; then
grep "width:100%" $textBox
rc=$?
if [ $rc -eq 0 ]; then
timja
commented
6 years ago .... Hm. That is assuming a jarfile rather than HPI. OK, I'm willing to believe it's my goof, then. Could have sworn otherwise.
Of course what I really want is to get this trivial change made in the extension officially rather than patching my own copy. I didn't seem to be getting traction on that, though.
timja
commented
6 years ago keshlam https://github.com/jenkinsci/extended-choice-parameter-plugin does not seem to be actively maintained (vimil please correct me if I am wrong). If you are interested, you can request plugin ownership transfer. https://wiki.jenkins.io/display/JENKINS/Adopt+a+Plugin
timja
commented
6 years ago
We just went thru a fair amount of effort to switch to Extended Choice Parameters as a replacement for Dynamic Parameters, to get scripted defaults in text-box input (eg timestamp-derived filenames) while avoiding the security concerns of DP. So we sorta need this working.
Meanwhile I'm playing with the workaround mechanism and hoping to make it work quickly. If not, I'll have to downgrade Jenkins again.
Y'know, you REALLY should have implemented this change first in Warning/Deprecation mode, giving us a switch to turn on hard whitelisting when we had achieved a stable environment under the new rules. Oh well.
Stack trace from a trivial Job which just accepts one value and echoes it back:
Originally reported by keshlam, imported from: Extended Choice Parameter broken by whitelisting change