michaeljmarshall opened a new pull request, #15328:
URL: https://github.com/apache/pulsar/pull/15328

   Master Issue: https://github.com/apache/pulsar/issues/11548
   
   ### Motivation
   
   Some use security sensitive users prefer to disable plaintext ports to 
ensure that there is no accidentally non-encrypted traffic. We added the 
ability to disable the non-TLS server on the broker 
https://github.com/apache/pulsar/pull/11681, and this PR adds the same feature 
for the function worker.
   
   I have two questions about the implementation:
   
   1. Is it a breaking change to update the worker id logic? It looks like the 
existing logic has two different methods of construction. The first is modified 
in this PR and the second is below. They differ only slightly. 
https://github.com/apache/pulsar/blob/74eae1a729c33c9fead9d54594bdee5fac8ab153/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L1681-L1685
   2. In the `initializeWorkerConfigFromBrokerConfig` method, I am updating the 
worker config so that it maps the broker's ports to the worker's ports for all 
values. This seems logical to me since there are so many other mapped ports, 
and when a user is disabling the plaintext port on the broker, it's highly 
likely that they'd also want to disable the plaintext port for the broker's 
function worker.
   
   ### Modifications
   
   * Update log lines to accurately indicate when the Function Worker is 
starting the HTTP or the HTTPS servers
   * Skip creation of the function worker's HTTP server when the `workerPort` 
is `null`
   * Update `WorkerConfig#getWorkerId()` so that it uses the TLS port when the 
plaintext port is null.
   * Update the documentation.
   * Update the broker's `initializeWorkerConfigFromBrokerConfig` method to 
align with the new changes.
   * Update a test to verify that the non-TLS port service does not get started.
   
   ### Verifying this change
   
   I added a test and verified the associated logs, too.
   
   ### Does this pull request potentially affect one of the following parts:
   
   This change does not affect any defaults.
   
   ### Documentation
   
   - [x] `doc-added`
   This PR includes updates to the docs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to