Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-48957] Prevent wildfly to deliver old jackson for blue ocean and other plugins #3235

Conversation

3 participants
@ManuelB
Copy link
Contributor

commented Jan 16, 2018

See JENKINS-48957.

This change has no test cases because it is a system issue and it would require to have test cases for every application server jenkins is running on.

Proposed changelog entries

  • JENKINS-48957 isolated Jenkins from JAX-RS classes shipped with wildfly (former JBoss) implementation to make blue ocean works (Jackson 2.7 vs 2.8)

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change).
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@michaelneale

@ManuelB ManuelB changed the title Bug/jenkins 48957 blue ocean wildfly old jackson [JENKINS-48957] Prevent wildfly to deliver old jackson for blue ocean and other plugins Jan 16, 2018

@michaelneale

This comment has been minimized.

Copy link
Member

commented Jan 16, 2018

I think this LGTM - I can't see how this would cause problems for the common use cases, as it is an addition of a config file for WF specifically

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Jan 20, 2018

IIUC this is generally a defect in BlueOcean which uses newer versions of libs instead of ones included into the core. Right @michaelneale ?

@ManuelB

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2018

@oleg-nenashev here is a list of all plugins that contain the name jackson on our installation:

cd /mnt/.jenkins/plugins
$ find . -name "*jackson-core*"
./jackson2-api/WEB-INF/lib/jackson-core-2.8.10.jar
./github/WEB-INF/lib/jackson-core-2.6.0.jar
./pipeline-model-api/WEB-INF/lib/jackson-core-2.2.2.jar
./pipeline-model-definition/WEB-INF/lib/jackson-core-2.7.3.jar
./cucumber-reports/WEB-INF/lib/jackson-core-2.8.6.jar
./cloudbees-bitbucket-branch-source/WEB-INF/lib/jackson-core-asl-1.9.13.jar
./blueocean-rest-impl/WEB-INF/lib/jackson-core-2.8.9.jar
./jira/WEB-INF/lib/jackson-core-2.8.9.jar
./jira/WEB-INF/lib/jackson-core-asl-1.5.5.jar
./jenkins-jira-plugin/WEB-INF/lib/jackson-core-asl-1.9.1.jar
./blueocean-commons/WEB-INF/lib/jackson-core-2.8.9.jar

As you can see most of your mobiles are bring your own library but this is not the problem.

The problem lies in the classloading policy of wildfly.
https://docs.jboss.org/author/display/WFLY10/Class+Loading+in+WildFly

The policy is the following:

In order of highest priority to lowest priority

  1. System Dependencies - These are dependencies that are added to the module automatically by the container, including the Java EE api's.
  2. User Dependencies - These are dependencies that are added through jboss-deployment-structure.xml or through the Dependencies: manifest entry.
  3. Local Resource - Class files packaged up inside the deployment itself, e.g. class files from WEB-INF/classes or WEB-INF/lib of a war.
  4. Inter deployment dependencies - These are dependencies on other deployments in an ear deployment. This can include classes in an ear's lib directory, or classes defined in other ejb jars.

With my addition I explicitly excluded the system dependency to the wildfly jax rs module.

/Manuel

@ManuelB

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2018

As a none native english speaker I had to lookup the abbreviations:

LGTM - Looks good to me
IICU - If I understand correctly

@ManuelB

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2018

Just for completeness this issue fixes the following stacktrace that happens when running jenkins, blue ocean on wildfly 10:

java.lang.NoSuchMethodError: com.fasterxml.jackson.core.io.JsonStringEncoder.quoteAsString(Ljava/lang/CharSequence;Ljava/lang/StringBuilder;)V
	at io.jenkins.blueocean.commons.stapler.export.JSONDataWriter.value(JSONDataWriter.java:112)
	at io.jenkins.blueocean.commons.stapler.export.JSONDataWriter.startObject(JSONDataWriter.java:154)
	at io.jenkins.blueocean.commons.stapler.export.Model.writeTo(Model.java:198)
	at io.jenkins.blueocean.commons.stapler.Export.writeOne(Export.java:148)
	at io.jenkins.blueocean.commons.stapler.Export.serveExposedBean(Export.java:139)
	at io.jenkins.blueocean.commons.stapler.Export.doJson(Export.java:79)
	at io.jenkins.blueocean.commons.stapler.TreeResponse$Processor$1.generateResponse(TreeResponse.java:48)
	at org.kohsuke.stapler.HttpResponseRenderer$Default.handleHttpResponse(HttpResponseRenderer.java:124)
	at org.kohsuke.stapler.HttpResponseRenderer$Default.generateResponse(HttpResponseRenderer.java:69)
	at org.kohsuke.stapler.Function.renderResponse(Function.java:136)
	at org.kohsuke.stapler.Function.bindAndInvokeAndServeResponse(Function.java:119)
	at org.kohsuke.stapler.IndexDispatcher.dispatch(IndexDispatcher.java:26)
	at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:715)
	at org.kohsuke.stapler.Stapler.invoke(Stapler.java:845)
	at org.kohsuke.stapler.MetaClass$10.dispatch(MetaClass.java:374)
	at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:715)
	at org.kohsuke.stapler.Stapler.invoke(Stapler.java:845)
	at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:209)
@oleg-nenashev
Copy link
Member

left a comment

The proposed fix for not impact non-JBoss platforms, so I think it's fine to merge it. Thanks for the behavior clarification, @ManuelB

P.S: I will try to use less abbreviations in PR reviews

@oleg-nenashev oleg-nenashev merged commit 832f17f into jenkinsci:master Jan 21, 2018

1 check passed

continuous-integration/jenkins/pr-head This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.