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

Bump min version of Jenkins to 2.204.4 #836

Merged
merged 6 commits into from
Apr 23, 2021

Conversation

pjdarton
Copy link
Member

@pjdarton pjdarton commented Apr 20, 2021

We currently have a failing unit test in #835 that does not appear to be related to that PR.
What we're seeing is that the DockerNodeStepTest unit test is failing with exception:

WARNING	h.m.DownloadService$Downloadable#updateNow: signature check failed for http://localhost:37881/updates/hudson.tasks.Maven.MavenInstaller.json
ERROR: Signature verification failed in downloadable 'hudson.tasks.Maven.MavenInstaller' java.security.SignatureException: Signature length not correct: got 512 but was expecting 256
	at sun.security.rsa.RSASignature.engineVerify(RSASignature.java:189)
	at java.security.Signature$Delegate.engineVerify(Signature.java:1222)
	at java.security.Signature.verify(Signature.java:655)
	at sun.security.x509.X509CertImpl.verify(X509CertImpl.java:444)
	at sun.security.provider.certpath.BasicChecker.verifySignature(BasicChecker.java:166)
	at sun.security.provider.certpath.BasicChecker.check(BasicChecker.java:147)
	at sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:125)
Caused: java.security.cert.CertPathValidatorException: signature check failed
	at sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:135)
	at sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:233)
	at sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:141)
	at sun.security.provider.certpath.PKIXCertPathValidator.engineValidate(PKIXCertPathValidator.java:80)
	at java.security.cert.CertPathValidator.validate(CertPathValidator.java:292)
	at org.jvnet.hudson.crypto.CertificateUtil.validatePath(CertificateUtil.java:93)
	at jenkins.util.JSONSignatureValidator.verifySignature(JSONSignatureValidator.java:78)
	at hudson.model.DownloadService$Downloadable.updateNow(DownloadService.java:416)
	at io.jenkins.docker.pipeline.DockerNodeStepTest$3.runTest(DockerNodeStepTest.java:237)
	at io.jenkins.docker.pipeline.DockerNodeStepTest$3.evaluate(DockerNodeStepTest.java:228)
	at org.jvnet.hudson.test.RestartableJenkinsRule$6.evaluate(RestartableJenkinsRule.java:272)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:596)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

This may be related to JENKINS-31089 and that said it was fixed in later versions of Jenkins, so this PR fixes this issue by updating our "minimum" version of Jenkins to the oldest officially supported (2.204.4 according to this page at the time this was written) ... and doing that seems to work...

However, the docker-plugin's dependencies have, traditionally, been largely left unloved and unmaintained.
This test failure seems to be the point where we've hit a breaking point and to fix this has (I think) necessitated a dependency move forwards, and the best way of doing that is to also move the way these dependencies are handled "forwards" too.

What this PR is attempting to do is to bump the minimum version of Jenkins to the oldest officially supported (2.204.4) and then to use the Jenkins "BOM" plugin to then figure out what version of everything else that we use needs to be (in order to avoid the dreaded "upper bounds" maven error).
This has required numerous changes to the pom.xml file that really need reveiwing by someone who understands this better than I do.
(There's also a few changes elsewhere in the code as a result of dependencies changing to non-ancient versions, but they're trivial by comparison.)

* Switched to Jenkins maven plugin 4.0 and BOM for dependencies.
* Stripped out specified versions where we don't need to specify.
* Removed Jenkins version-specific stuff as it's unnecessary now.
* Removed some dependencies which didn't seem to be used directly.
@pjdarton
Copy link
Member Author

HELP WANTED

The docker-plugin's dependencies have, traditionally, been largely left unloved and unmaintained.
I'm the current custodian of this plugin and I do not have a great understanding of maven dependencies, or how Jenkins dependencies are done using maven.
I would really appeciate someone who does understand dependencies reviewing the changes I'm making to the pom.xml file here.

@rdica @andrei-barna @Osipion @bguerin - you've all got PRs and/or defects you're interested in seeing merged so I figure that I'll ask you all first ... and if you don't know this subject any better than I do but you know someone who does, please do point them in this direction.

@bguerin
Copy link
Contributor

bguerin commented Apr 22, 2021

Hello !
To get confident with these changes, the first thing to do is running mvn help:effective-pom before your changes and then again after your changes. You will obtain the full POM, completly expanded, the diff is here :
diff_effective_pom.txt

First changes, in the dependencyManagement section are just about versions of the dependencies that the plugin could use. They are really used if declared in the dependencies section. Both theses changes seems good to me.
There are also changes in the plugins section, mostly version upgrades. Also fine to me, means plugin will use more recent plugins, with bug fixes.

Second thing to do it to list the dependencies tree (again, before and after), mvn dependency:tree. Here is the diff :
diff_dependency_tree.txt

Looking only at the first level (.*[INFO] +-.*), these seems coherent with what you want to do.

If you want, I can test your branch on my installation : up-to-date jenkins and pipeline plugins, running in docker, building through docker agent, JNLP connected.

If I could, I would approve this, and thanks for your work !

@pjdarton
Copy link
Member Author

Thank you for the (excellent) explanation and detailed investigation; that is exactly what I was hoping for.

Yes, if you can test it then please do so. I'll see if I can find a few Jenkins servers to put it on at work tomorrow too.
If it all goes well, I'll press the merge button ... and hopefully that'll then lead to other PR builds passing, and make life easier for all...

Once again: thank you for your analysis - it's much appreciated.

@bguerin
Copy link
Contributor

bguerin commented Apr 23, 2021

Welcome ;)

Building your branch and upgrading the plugin on my installation. Keep you posted

@bguerin
Copy link
Contributor

bguerin commented Apr 23, 2021

Changes successfully tested on my installation

@pjdarton
Copy link
Member Author

My own testing suggests that it's harmless too - I'll merge it...

@pjdarton pjdarton merged commit 11be6b7 into jenkinsci:master Apr 23, 2021
@pjdarton pjdarton deleted the fixdockernodesteptest branch April 5, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants