[pulsar-broker] Make non-tls web/broker-service optional #3501
Conversation
| @@ -101,7 +101,7 @@ | |||
| category = CATEGORY_SERVER, | |||
| doc = "The port for serving binary protobuf requests" | |||
| ) | |||
| private Integer brokerServicePort = 6650; | |||
| private Integer brokerServicePort; | |||
There was a problem hiding this comment.
Wouldn't this disable the "clear-text" service by default?
There was a problem hiding this comment.
Wouldn't this disable the "clear-text" service by default?
yes. with this change, "clear-text" will disable the service.
There was a problem hiding this comment.
I mean, will it be disabled by default?
There was a problem hiding this comment.
Wouldn't this disable the "clear-text" service by default?
yes. in order to disable it, right now, we are considering that user will give "clear-text". So, by default if user doesn't provide port then it will be disable. also pulsar-service fails if user doesn't provide both tls and non-tls ports.
However, I also agree that broker service should start with default port if none of the port is provided by default. so, we can have multiple options to disable service on non-tls port
- user can provide clear-text that will disable it by default.
- user can explicitly provide -1 value to disable non-tls port else by default it will listen on non-tls ports
- if both tls and non-tls ports are not provided then broker will start service on default non-tls ports.
I think we can do 2nd option to explicitly disable non-tls ports. any thoughts?
There was a problem hiding this comment.
what if user provides brokerServicePort= in broker.conf instead of leaving that line empty?
There was a problem hiding this comment.
then right now, as we can see in this test, PulsarConfigurationLoader doesn't update the field and broker considers default field value so, it will be always 8080 if we keep private Integer brokerServicePort = 6650;. let me check if we can make change at PulsarConfigurationLoader where it sets field value null if value is not provided in config file.
|
retest this please |
|
rerun cpp tests |
|
@rdhabalia do we need this issue for 2.4.0? or can we move it to 2.5.0? |
|
@sijie yes, we need for 2.4.0 .. |
|
addressed all comments, so can we please review this pR again |
|
@merlimat Please help review this PR again |
|
@merlimat can you please check this PR again? |
Motivation
Right now, broker always listens on non-tls webservice(
8080) and brokerservice(6650) port even though when user doesn't define it. So, broker always listens on non-secure port by default even when user doesn't want. And making those port optional will be tricky because broker has direct dependencies of them at many places (eg: while registering load-balancer node, always selecting broker with http-url, creating ownership-cache with non-tls url, etc.)Modification
Result
User can start pulsar in a secure mode only.