-
Notifications
You must be signed in to change notification settings - Fork 103
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-63350] update pac4j to 3.9.0 #90
Conversation
@@ -98,8 +101,13 @@ under the License. | |||
<dependency> | |||
<groupId>org.pac4j</groupId> | |||
<artifactId>pac4j-saml</artifactId> | |||
<version>3.8.3</version> | |||
<!-- versions 4.x.x require JDK 11 --> |
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.
Then set java.level
to 11
I guess? Not sure of status of jenkinsci/plugin-pom#133.
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 think is fine, 3.9.0 was released a few days ago
<exclusions> | ||
<exclusion> | ||
<groupId>org.springframework</groupId> | ||
<artifactId>spring-beans</artifactId> |
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.
You may need to exclude more. Pay attention to what gets bundled in the *.hpi
(also printed to build log).
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.
yep, there is a bunch of transitive dependencies that can be removed
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 any way to acknowledge(remove the warnings) the dependencies? there are some warnings that come from the hpi plugin, but those dependencies are needed and I do not want to add all those dependencies manually to the plugin pom.
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 any way to acknowledge(remove the warnings)
Yes but do not do that.
there are some warnings that come from the hpi plugin, but those dependencies are needed
You can exclude
dependencies you know you will not use, or you know you can use in the version supplied by core. Or you can switch to pluginFirstClassLoader
and forget about accurate testing with JenkinsRule
(must do it manually). Think twice, actually several times, before doing anything out of the ordinary. https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/
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 have excluded all that are not needed or are already in the Jenkins Core. so I guess that either I add these libraries to the pom or I live with these warnings
[WARNING] Bundling transitive dependency jcommander-1.48.jar (via pac4j-saml)
--
[WARNING] Bundling transitive dependency woodstox-core-5.0.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency istack-commons-runtime-3.0.8.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency FastInfoset-1.2.16.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency metrics-core-3.1.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency jakarta.xml.bind-api-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency javax.json-api-1.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency joda-time-2.9.9.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency xmlsectool-2.0.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency java-support-7.5.0.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency spymemcached-2.12.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency httpclient-4.5.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency httpcore-4.4.8.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency xmlsec-2.1.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency velocity-1.7.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency stax2-api-3.1.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency cryptacular-1.2.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency jaxb-runtime-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency txw2-2.3.2.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency javax.json-1.0.4.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency stax-ex-1.8.1.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency ldaptive-1.0.13.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-core-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-messaging-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-messaging-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-profile-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-profile-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-saml-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-saml-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-security-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-security-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-soap-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-soap-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-storage-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-storage-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-xmlsec-api-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency opensaml-xmlsec-impl-3.4.3.jar (via pac4j-saml)
[WARNING] Bundling transitive dependency pac4j-core-3.9.0.jar (via pac4j-saml)
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 will live with the warnings to remove them I have to add more than 300 no-sense lines to the pom to manage these dependencies manually.
Co-authored-by: Jesse Glick <[email protected]>
I have manually tested it, it works. There is a small detail on the URL for the SAML response I have to convert into optional in some way but it works. This is my test environment https://github.com/kuisathaverat/jenkins-issues/tree/master/JENKINS-63350 |
resolved, the plugin works as the previous version works now. |
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.
INFRA-2809 is resolved and the plugin builds fine locally.
I think this can be merged now @kuisathaverat - thanks for the work here!
Actually, it seems to be downloading the dependency from the shibboleth repo. It's not listed in the Jenkins one. Asked Daniel in the INFRA issue, there seems to be a missing route. |
OK, all artifacts are now available in the Jenkins repo, so all good. |
I will wait for the CI to finish to merge the PR |
finished ^^ |
I will release on the weekend |
@@ -1,15 +1,16 @@ | |||
//def buildConfiguration = buildPlugin.recommendedConfigurations() | |||
|
|||
def lts = "2.176.1" | |||
def weekly = "2.199" | |||
def lts = "2.249.3" |
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.
??
import org.pac4j.core.client.RedirectAction; | ||
import org.pac4j.core.client.RedirectAction.RedirectType; | ||
import org.pac4j.core.redirect.RedirectAction; | ||
import org.pac4j.core.redirect.RedirectAction.RedirectType; | ||
import org.springframework.dao.DataAccessException; |
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.
Do you not plan to switch to Spring Security versions of Jenkins interfaces as well, now that you are on a baseline with JEP-227?
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.
sure, what is the replacement for DataAccessException?
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 is none.
Best to look at other PRs associated with JEP-227 for examples.
See JENKINS-63350.
Update pac4j to 3.9.0 depends on jenkinsci/jenkins#4848
Submitter checklist