-
-
Notifications
You must be signed in to change notification settings - Fork 9k
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-68694] Winstone 6.1: Upgrade Jetty from 9.4.46.v20220331 to 10.0.11 #6801
Conversation
c1c2c6d
to
93b0889
Compare
…10.0.11 Co-authored-by: Jesse Glick <[email protected]>
93b0889
to
f5aba77
Compare
test/src/test/java/hudson/security/csrf/CrumbExclusionTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/jenkins/security/SuspiciousRequestFilterTest.java
Outdated
Show resolved
Hide resolved
</systemProperties> | ||
<webApp> | ||
<!-- Allows resources to be reloaded, and enable nicer console logging. --> | ||
<extraClasspath>${project.basedir}/../core/src/main/resources,${project.basedir}/../core/target/classes,${project.build.directory}/support-log-formatter.jar</extraClasspath> | ||
<!-- TODO eclipse/jetty.project#7970 <extraClasspath>${project.basedir}/../core/src/main/resources,${project.basedir}/../core/target/classes,${project.build.directory}/support-log-formatter.jar</extraClasspath> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a workaround to this? i.e. without this we can't live reload resources from what I understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe there is, but the pain should be short-lived as this has already been fixed upstream and should be present in the next release of Jetty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just awaiting another core reviewer to be ready to merge?
(Cheating a bit for me to review as a co-author.)
Yes, it is. |
Co-authored-by: Jesse Glick <[email protected]>
…t.java Co-authored-by: Jesse Glick <[email protected]>
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks! |
…10.0.11 (jenkinsci#6801) Co-authored-by: Jesse Glick <[email protected]> (cherry picked from commit cb1bbc7)
See JENKINS-68694. Jetty 10.0.x implements Servlet 4.0 (JakartaEE 8/
javax.servlet.*
). The minimum required Java version for Jetty 10 is now Java 11. For more details about the Jetty 10 release, see this Jetty blog post. Winstone 6.1 removes support for OpenSSL-style PEM-encoded RSA private keys in jenkinsci/winstone#232.Testing done
--httpsKeyStore
,--httpsKeyStorePassword
, and--httpsPort
; visited the UI in my browser via TLS and clicked around successfully; build Freestyle and Pipeline jobs; verified Winstone 6.1 is displayed in the About page.wsecho/
(in both text and binary modes, including ping visible after 30s withwebsocat -vv
); Pipeline builds on a WebSocket agent; CLI in-webSocket
mode.mvn -pl war jetty:run
10 times in a row without any failures. Ensured that theJENKINS_HOME
being used was the expected$JENKINS_REPO/war/work
directory rather than$HOME/.jenkins
.RealJenkinsRule
-based test added in [JENKINS-68933] Better WebSocket testing, removal of reflection #6780 failed without the addition ofJetty10Provider
in this PR and passed with it.JenkinsRule
jenkins-test-harness#453, Test #6801 with jenkinsci/jenkins-test-harness#453 #6802, and Test jenkinsci/jenkins#6802 with jenkinsci/jenkins-test-harness#453 bom#1255, ran the core test suite and BOM/PCT test suite with a modifiedJenkinsRule
based on Jetty 10. Without the addition ofJetty10Provider
in this PR, these tests (includingjenkins.agents.WebSocketAgentsTest
) failed. With the addition ofJetty10Provider
in this PR, all tests passed.jenkins.war
before and after this change; verified that no unexpected JAR files had been added to or removed from the WAR archive.Proposed changelog entries
Proposed upgrade guidelines
Support for OpenSSL-style PEM-encoded RSA private keys has been removed when running Jenkins with the embedded Jetty (Winstone) container and TLS. Specifically, the
--httpsPrivateKey
and--httpsCertificate
flags have been removed in favor of the--httpsKeyStore
flag. The removed flags have printed deprecation warnings since 2016 and were implemented with non-standard APIs that have since been removed from Java 17. The recommendation is to migrate to the--httpsKeyStore
option, which takes a keystore as described in the documentation. As of JEP 229, PKCS12 is the recommended keystore type.Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade@Restricted
or have@since TODO
Javadoc, as appropriate.@Deprecated(since="TODO")
or@Deprecated(since="TODO", forRemoval=true)
if applicable.Desired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are accurate, human-readable, and in the imperative moodupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).