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.

Sign up for GitHub

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

Jump to bottom

Refuse to load the Jenkins test harness in production #8714

Merged
merged 1 commit into from Nov 23, 2023
Merged

Refuse to load the Jenkins test harness in production #8714

merged 1 commit into from Nov 23, 2023

Conversation

basil
Copy link
Member

@basil basil commented Nov 21, 2023

See JENKINS-65650, JENKINS-66060, and JENKINS-72353. Fail fast, informing the user which plugin is broken, rather than continuing execution with a known broken configuration. More aggressive enforcement of what should become a non-issue after jenkinsci/plugin-pom#864. If people feel this PR is overkill, this PR can simply be closed.

Testing done

Verified that the exception was thrown both from the (currently broken) build-pipeline-plugin's InjectedTest and from a java -jar jenkins.war invocation with build-pipeline-plugin, which printed:

java.lang.IllegalStateException: Refusing to load the Jenkins test harness in production (via build-pipeline-plugin)
	at hudson.ClassicPluginStrategy.createClassLoader(ClassicPluginStrategy.java:295)
	at hudson.ClassicPluginStrategy.createPluginWrapper(ClassicPluginStrategy.java:251)
	at hudson.PluginManager$1$3$1.run(PluginManager.java:442)
	at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:177)
	at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:305)
	at jenkins.model.Jenkins$5.runTask(Jenkins.java:1170)
	at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:221)
	at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:120)
	at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

Proposed changelog entries

  • Fail fast when attempting to load a broken plugin that contains the Jenkins test harness in production.

Proposed upgrade guidelines

N/A

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. The Jira issue, if it exists, is well-described.
    Options
  2. The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
    Options
  3. There is automated testing or an explanation as to why this change has no tests.
    Options
  4. New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
    Options
  5. New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
    Options
  6. New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
    Options
  7. For dependency updates, there are links to external changelogs and, if possible, full differentials.
    Options
  8. For new APIs and extension points, there is a link to at least one consumer.
    Options

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Edit tasklist title
Beta Give feedback Tasklist Maintainer checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
    Options
  2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
    Options
  3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
    Options
  4. Proper changelog labels are set so that the changelog can be generated automatically.
    Options
  5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
    Options
  6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
    Options

@basil basil added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Nov 21, 2023
MarkEWaite
MarkEWaite approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change makes sense to me. The memory leaks that result from loading the test harness into a production server are too high impact for us to load the harness silently.

@MarkEWaite
Copy link
Contributor

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Nov 22, 2023
@NotMyFault NotMyFault merged commit f4fe571 into jenkinsci:master Nov 23, 2023
16 checks passed
MarkEWaite added a commit to MarkEWaite/applitools-eyes-plugin that referenced this pull request Dec 18, 2023
@MarkEWaite
Remove Jenkins test harness HTMLUnit from the plugin hpi file
00bab0e 
jenkinsci/jenkins#8714 included in Jenkins 2.434
and later will refuse to load a plugin that includes the Jenkins test
harness.

The Jenkins test harness causes unexpected failures when it is included
in a Jenkins plugin.  Refer to the following issue reports for examples:

* https://issues.jenkins.io/browse/JENKINS-65650
* https://issues.jenkins.io/browse/JENKINS-66060
* https://issues.jenkins.io/browse/JENKINS-72353

The additional benefit of the change is that it makes the plugin hpi
file over 80% smaller.

Before this change, the plugin hpi file had the following contents:

  1136 META-INF/MANIFEST.MF
    73 META-INF/maven/org.jenkins-ci.plugins/applitools-eyes/pom.properties
  3153 META-INF/maven/org.jenkins-ci.plugins/applitools-eyes/pom.xml
 34814 WEB-INF/lib/applitools-eyes.jar
353793 WEB-INF/lib/commons-codec-1.15.jar
588337 WEB-INF/lib/commons-collections-3.2.2.jar
327135 WEB-INF/lib/commons-io-2.11.0.jar
434678 WEB-INF/lib/commons-lang3-3.4.jar
380196 WEB-INF/lib/cssparser-0.9.16.jar
767140 WEB-INF/lib/httpclient-4.5.6.jar
326356 WEB-INF/lib/httpcore-4.4.10.jar
4079701 WEB-INF/lib/jenkins-test-harness-htmlunit-2.18-1.jar
108240 WEB-INF/lib/jetty-io-9.2.12.v20150709.jar
357855 WEB-INF/lib/jetty-util-9.2.12.v20150709.jar
125315 WEB-INF/lib/nekohtml-1.9.22.jar
 15808 WEB-INF/lib/sac-1.3.jar
276420 WEB-INF/lib/serializer-2.7.2.jar
 43776 WEB-INF/lib/websocket-api-9.2.12.v20150709.jar
 36264 WEB-INF/lib/websocket-client-9.2.12.v20150709.jar
179845 WEB-INF/lib/websocket-common-9.2.12.v20150709.jar
3154938 WEB-INF/lib/xalan-2.7.2.jar
1367760 WEB-INF/lib/xercesImpl-2.11.0.jar
220536 WEB-INF/lib/xml-apis-1.4.01.jar
 10963 WEB-INF/licenses.xml

After this change, the plugin hpi file has the following contents:

  1136 META-INF/MANIFEST.MF
    73 META-INF/maven/org.jenkins-ci.plugins/applitools-eyes/pom.properties
  2951 META-INF/maven/org.jenkins-ci.plugins/applitools-eyes/pom.xml
 34479 WEB-INF/lib/applitools-eyes.jar
353793 WEB-INF/lib/commons-codec-1.15.jar
767140 WEB-INF/lib/httpclient-4.5.6.jar
326356 WEB-INF/lib/httpcore-4.4.10.jar
  1814 WEB-INF/licenses.xml
@MarkEWaite MarkEWaite mentioned this pull request Dec 18, 2023
Merged
5 tasks
MarkEWaite added a commit to MarkEWaite/bmc-change-manager-imstm-plugin that referenced this pull request Dec 18, 2023
@MarkEWaite
Remove Jenkins test harness from the plugin hpi file
7220d5a 
jenkinsci/jenkins#8714 included in Jenkins 2.434
and later will refuse to load a plugin that includes the Jenkins test
harness.

The Jenkins test harness causes unexpected failures when it is included
in a Jenkins plugin.  Refer to the following issue reports for examples:

* https://issues.jenkins.io/browse/JENKINS-65650
* https://issues.jenkins.io/browse/JENKINS-66060
* https://issues.jenkins.io/browse/JENKINS-72353

The additional benefit of the change is that it makes the plugin hpi
file over 90% smaller.

Before this change, the plugin hpi file had the following contents:

 61162 BuildStepController.js
 41450 DlistActions.js
 40183 images/bmc_build_step.jpg
374726 images/dlp_plugin.jpg
 33108 images/plugin_version.jpg
 42957 images/workspace.jpg
   737 META-INF/MANIFEST.MF
    81 META-INF/maven/com.bmc.ims/bmc-change-manager-imstm/pom.properties
  4254 META-INF/maven/com.bmc.ims/bmc-change-manager-imstm/pom.xml
 81188 WEB-INF/lib/bmc-change-manager-imstm.jar
1692782 WEB-INF/lib/commons-math3-3.2.jar
307305 WEB-INF/lib/commons-net-3.8.0.jar
 98115 WEB-INF/lib/dec-0.1.2.jar
160975 WEB-INF/lib/embedded-rhino-debugger-1.2.jar
123360 WEB-INF/lib/hamcrest-2.2.jar
  1499 WEB-INF/lib/hamcrest-core-2.2.jar
  1537 WEB-INF/lib/hamcrest-library-2.2.jar
 56674 WEB-INF/lib/javax.activation-api-1.2.0.jar
128032 WEB-INF/lib/jaxb-api-2.4.0-b180830.0359.jar
426248 WEB-INF/lib/jenkins-test-harness-1802.v9de0d87365d2.jar
10958672 WEB-INF/lib/jenkins-test-harness-htmlunit-106.vc41185ea_3d3d.jar
325897 WEB-INF/lib/jetty-client-9.4.48.v20220622.jar
234754 WEB-INF/lib/jetty-http-9.4.48.v20220622.jar
183018 WEB-INF/lib/jetty-io-9.4.48.v20220622.jar
118511 WEB-INF/lib/jetty-security-9.4.48.v20220622.jar
732200 WEB-INF/lib/jetty-server-9.4.48.v20220622.jar
146081 WEB-INF/lib/jetty-servlet-9.4.48.v20220622.jar
582975 WEB-INF/lib/jetty-util-9.4.48.v20220622.jar
 66654 WEB-INF/lib/jetty-util-ajax-9.4.48.v20220622.jar
140320 WEB-INF/lib/jetty-webapp-9.4.48.v20220622.jar
 68301 WEB-INF/lib/jetty-xml-9.4.48.v20220622.jar
541449 WEB-INF/lib/jmh-core-1.35.jar
 31277 WEB-INF/lib/jmh-generator-annprocess-1.35.jar
 78146 WEB-INF/lib/jopt-simple-5.0.4.jar
 65966 WEB-INF/lib/json-20200518.jar
187400 WEB-INF/lib/org-netbeans-insane-RELEASE140.jar
 65728 WEB-INF/lib/salvation2-3.0.0.jar
  6884 WEB-INF/lib/support-log-formatter-1.1.jar
  2699 WEB-INF/lib/symbol-annotation-1.23.jar
 52173 WEB-INF/lib/websocket-api-9.4.48.v20220622.jar
 45622 WEB-INF/lib/websocket-client-9.4.48.v20220622.jar
214632 WEB-INF/lib/websocket-common-9.4.48.v20220622.jar
 45510 WEB-INF/lib/websocket-server-9.4.48.v20220622.jar
 30315 WEB-INF/lib/websocket-servlet-9.4.48.v20220622.jar
 15495 WEB-INF/licenses.xml

After this change, the plugin hpi file has the following contents:

 61162 BuildStepController.js
 41450 DlistActions.js
 40183 images/bmc_build_step.jpg
374726 images/dlp_plugin.jpg
 33108 images/plugin_version.jpg
 42957 images/workspace.jpg
   767 META-INF/MANIFEST.MF
    80 META-INF/maven/com.bmc.ims/bmc-change-manager-imstm/pom.properties
  4048 META-INF/maven/com.bmc.ims/bmc-change-manager-imstm/pom.xml
 81282 WEB-INF/lib/bmc-change-manager-imstm.jar
 56674 WEB-INF/lib/javax.activation-api-1.2.0.jar
128032 WEB-INF/lib/jaxb-api-2.4.0-b180830.0359.jar
 65966 WEB-INF/lib/json-20200518.jar
  2699 WEB-INF/lib/symbol-annotation-1.23.jar
  2466 WEB-INF/licenses.xml
@MarkEWaite MarkEWaite mentioned this pull request Dec 18, 2023
Open
5 tasks
MarkEWaite added a commit to MarkEWaite/container-image-link-plugin that referenced this pull request Dec 18, 2023
@MarkEWaite
Replace Jenkins test harness in hpi with Apache commons lang3 plugin
55a3de4 
jenkinsci/jenkins#8714 included in Jenkins 2.434
and later will refuse to load a plugin that includes the Jenkins test
harness.

The Jenkins test harness causes unexpected failures when it is included
in a Jenkins plugin.  Refer to the following issue reports for examples:

* https://issues.jenkins.io/browse/JENKINS-65650
* https://issues.jenkins.io/browse/JENKINS-66060
* https://issues.jenkins.io/browse/JENKINS-72353

The additional benefit of the change is that it makes the plugin hpi
file over 90% smaller.

Before this change, the plugin hpi file had the following contents:

  1680 images/docker.svg
   765 META-INF/MANIFEST.MF
    84 META-INF/maven/io.jenkins.plugins/container-image-link/pom.properties
  5342 META-INF/maven/io.jenkins.plugins/container-image-link/pom.xml
327135 WEB-INF/lib/commons-io-2.11.0.jar
307305 WEB-INF/lib/commons-net-3.8.0.jar
 13633 WEB-INF/lib/container-image-link.jar
  9277 WEB-INF/lib/crypto-util-1.5.jar
 98115 WEB-INF/lib/dec-0.1.2.jar
10579074 WEB-INF/lib/jenkins-test-harness-htmlunit-66.v712ea44bccba.jar
 65728 WEB-INF/lib/salvation2-3.0.0.jar
276420 WEB-INF/lib/serializer-2.7.2.jar
220536 WEB-INF/lib/xml-apis-1.4.01.jar
  6546 WEB-INF/licenses.xml

After this change, the plugin hpi file has the following contents:

  1680 images/docker.svg
   816 META-INF/MANIFEST.MF
    83 META-INF/maven/io.jenkins.plugins/container-image-link/pom.properties
  5320 META-INF/maven/io.jenkins.plugins/container-image-link/pom.xml
327135 WEB-INF/lib/commons-io-2.11.0.jar
 13661 WEB-INF/lib/container-image-link.jar
  9277 WEB-INF/lib/crypto-util-1.5.jar
  3444 WEB-INF/licenses.xml
This was referenced Dec 18, 2023
Open
Open
MarkEWaite added a commit to MarkEWaite/itms-for-jira-plugin that referenced this pull request Dec 18, 2023
@MarkEWaite
Require Jenkins 2.361.4 or newer and Java 11
570f0df 
Also removes Jenkins test harness from the plugin packaging

DOES NOT FIX THE INCORRECT MODIFICATION OF ARTIFACT ID
jenkinsci#1 needs partial
revert to restore original artifact ID and package ID.

Remove Jenkins test harness HTMLUnit from the plugin hpi file

jenkinsci/jenkins#8714 included in Jenkins
2.434 and later will refuse to load a plugin that includes the Jenkins
test harness.

The Jenkins test harness causes unexpected failures when it is included
in a Jenkins plugin.  Refer to the following issue reports for examples:

* https://issues.jenkins.io/browse/JENKINS-65650
* https://issues.jenkins.io/browse/JENKINS-66060
* https://issues.jenkins.io/browse/JENKINS-72353

The additional benefit of the change is that it makes the plugin hpi
file over 80% smaller.
This was referenced Dec 18, 2023
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkEWaite MarkEWaite MarkEWaite approved these changes

@NotMyFault NotMyFault NotMyFault approved these changes

Assignees
No one assigned
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted
Projects
None yet
Milestone
No milestone
3 participants
@basil @MarkEWaite @NotMyFault