-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix: remove bc optional dependency #6051
Conversation
@coopstah13 : Could you please review it to see if it meets your expectations? |
c4fd44a
to
29c8c19
Compare
@rohanKanojia unfortunately it still does have an issue. The remaining failures are related to:
JDK 15+ dropped support for secp256k1, but we're using a key from that algorithm for KubernetesClientLoadWithFipsProviderTest. We can either:
|
I think it would be acceptable to leave fall-back code with the optional dependency for supporting that. Basically, first you'd try to do it using JDK internals, but maybe catch the exception and fall back to the Callable code which lets it dynamically link to the BC dependency and leave BC as an optional dependency defined in your pom. Appreciate the effort on this. |
I've opted for suggesting that BC be installed as a provider if there's a problem with the algorithm. This approach means that we won't have any direct compile / runtime dependency on BC. |
closes: fabric8io#6008 Signed-off-by: Steve Hawkins <[email protected]>
<optional>true</optional> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.bouncycastle</groupId> | ||
<artifactId>bcpkix-jdk18on</artifactId> | ||
<optional>true</optional> |
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.
this change seems odd to me
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.
Why? It's now a test dependency.
Quality Gate failedFailed conditions |
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.
LGTM, nice job.
Thx!
Description
Removes the bc optional dependency
closes: #6008
Type of change
test, version modification, documentation, etc.)
Checklist