-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[improve] PIP-337: Implement SSL Factory Plugin to customize SSL Context and SSL Engine generation #23110
Conversation
pulsar-common/src/main/java/org/apache/pulsar/common/util/NetworkMode.java
Outdated
Show resolved
Hide resolved
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.
Impressive work @Apurva007! This WIP PR is almost finished. I added a comment about documenting the PulsarSslFactory which I believe you already had on your todo list.
pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslFactory.java
Outdated
Show resolved
Hide resolved
please rebase (or merge latest master) to resolve the merge conflicts with current master branch. |
8c05ae0
to
5c1da7e
Compare
5c1da7e
to
ebd9bd8
Compare
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.
A few comments, mainly about the ScheduledExecutorService lifecycle handling.
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ConnectionPoolTest.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
Outdated
Show resolved
Hide resolved
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java
Outdated
Show resolved
Hide resolved
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java
Outdated
Show resolved
Hide resolved
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java
Outdated
Show resolved
Hide resolved
e6b169e
to
5824f6e
Compare
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.
Recent changes look good to me. I added a comment about disabling refreshing.
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
Show resolved
Hide resolved
5824f6e
to
dde4770
Compare
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
Great work @Apurva007 ! |
Thanks @lhotari ! |
…ext and SSL Engine generation (apache#23110) Co-authored-by: Apurva Telang <[email protected]>
PIP: #22016
Motivation
Apache Pulsar's TLS encryption configuration is not pluggable. It only supports file-based certificates. This makes
Pulsar difficult to adopt for organizations that require loading TLS certificates by other mechanisms.
Modifications
Creating a new PulsarSslFactory plugin that has a default implementation which matches current Pulsar behavior of reading file based certificates. This factory is then introduced into each of the classes that create Netty, Jetty or Async Http objects.
It also deletes various current classes that perform the above operations based on the underlying connection framework.
Verifying this change
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: Apurva007#6