add bouncycastle / eddsa plugins as optional and remove trilead #98
Conversation
jtnord
changed the title
|
(needs appropriate labels, will break the bom unless jenkinsci/bom#3231 is addressed) |
Plugin depending on trilead (a different ssh implementaion) makes no logical sense. Add optional dependencies to bouncycasle and eddsa plugin. The latter is needed for EdDSA keys, the former can add extra things from BouncyCastle. This prevents some classloading excpeptions where the context classloader is set to a class that can see bouncycastle or eddsa code yet the classloader of code in these plugin can not. Now if eddsa or bouncycastle plugins are installed this plugin will consistently be able to see the code. remove management of ssh-credentials plugin
|
@jtnord we should be ok to merge it now? |
|
jenkinsci/bom#3231 is still unaddressed so merging will lead to errors like: https://ci.jenkins.io/job/Tools/job/bom/job/PR-3218/4/testReport/ TBH - move fast and break things - I maintain the way the BOM is working is broken, I have provided a PR to change the way it works that is shown to work and it is not the job of this or any other plugins to work around it. I am neither a maintainer of this plugin nor of the BOM :-) |
|
ah this remember the usual issue (such jenkinsci/maven-hpi-plugin#391) |
I don't think that we should try and work around this in (it would also mean that all the FIPS tests would need to be disabled when running in the BOM build as anything that gets the eddsa plugin (which is in the fat war!) and sets FIPS mode in the test harness will get a failed startup!) |
what can we consider as trigger to detect the current context is BOM build? or add a new specific property in the bom build script? |
|
Better not Volkswagen it. jenkinsci/bom#3232 should suffice (I am just waiting for an opinion from @MarkEWaite today). |
There was a problem hiding this comment.
OK I think, though I can hardly predict the consequences. Can I assume this has been tested in RealJenkinsRule or otherwise with realistic class loading, ideally also with some of the optional dependencies disabled? I suppose this should be marked developer insofar as it is needed for something else but not an interesting update for end users?
| <artifactId>bom-2.426.x</artifactId> | ||
| <!-- | ||
| TODO remove the eddsa-api management below when the bom upadte includes eddsa-api | ||
| https://github.com/jenkinsci/bom/pull/3230 |
Apache Mina only currently supports EDDSA curves via the eddsa library.
It can additionally support more types if BouncyCastle is present.
Whilst the
eddsalibrary should not be needed (as BC supports this) mina makes no use of this, nor will it attempt to use EdDSA from the java platform if running on jdk > 15 that has this added.As such these 2 libraries are added via optional plugins, so that if they are present then this extra support will be available.
obsoletes #91
Testing done
has been used in interactice testing with a customer git and ssh server in combination with jenkinsci/git-client-plugin#1127
Submitter checklist