You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/01/21 11:42:18 UTC
[GitHub] [pulsar] sijie opened a new pull request #9260: [Functions] Can't create functions with m-TLS
sijie opened a new pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260
*Motivation*
#7255 re-worked Function MetaDataManager to make all metadata writes only by the leader.
This unintentionally broke Pulsar Functions when m-TLS is used for authentication. Because it doesn't
taken TLS port into consideration and always uses a non-TLS port to communicate with the leader broker.
The PR fixes the broken implementation and ensure Pulsar Functions use the right service url and
authentication plugin to communicate with leader.
*Tests*
Add an integration test to reproduce the issue and ensure functions worker with m-TLS
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] klwilson227 commented on a change in pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
klwilson227 commented on a change in pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#discussion_r562094741
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
workerConfig.getTlsEnabled() ? workerConfig.getWorkerPort(): workerConfig.getWokerPortTls()
I believe would be consistent with the usage of TLS. Infering from the fact that the config has setup the port or not may lead to differences between the usage and the naming...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-772393883
looks like the tls auth failed in the test:
```
14:14:38.240 [docker-java-stream-161014990:org.apache.pulsar.tests.integration.utils.DockerUtils$2@257] ERROR org.apache.pulsar.tests.integration.utils.DockerUtils - DOCKER.exec(PulsarFunctionsThreadTest-vvxth-pulsar-functions-worker-thread-lxgbp-1:/pulsar/bin/pulsar-admin functions status --tenant public --namespace default --name test-tumbling-window-fn-zbjaarbg): completed with non zero return code: 1
stdout:
stderr: org.apache.pulsar.client.admin.PulsarAdminException: java.util.concurrent.CompletionException: org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector$RetryException: Could not complete the operation. Number of retries has been exhausted. Failed reason: error:10000410:SSL routines:OPENSSL_internal:SSLV3_ALERT_HANDSHAKE_FAILURE
Reason: HTTP 500 Internal Server Error
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#discussion_r562137603
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
Fixed the logic to make it consistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-767127952
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-773703298
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#discussion_r562126814
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
Sure. can make that change.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
Fixed the logic to make it consistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#discussion_r562126814
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
Sure. can make that change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] zymap commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
zymap commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-789347169
I use another PR #9553 for fixing this on branch 2.7. So I will remove the `release/2.7.1` label.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-773703298
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-767217064
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-773772751
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] zymap commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
zymap commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-765967849
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-767127952
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] klwilson227 commented on a change in pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
klwilson227 commented on a change in pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#discussion_r562094741
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1316,19 +1323,24 @@ private void startPackagesManagementService() throws IOException {
public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfiguration brokerConfig,
String workerConfigFile) throws IOException {
WorkerConfig workerConfig = WorkerConfig.load(workerConfigFile);
+
+ brokerConfig.getWebServicePort()
+ .map(port -> workerConfig.setWorkerPort(port));
+ brokerConfig.getWebServicePortTls()
+ .map(port -> workerConfig.setWorkerPortTls(port));
+
// worker talks to local broker
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
brokerConfig.getAdvertisedAddress());
workerConfig.setWorkerHostname(hostname);
- workerConfig.setWorkerPort(brokerConfig.getWebServicePort().get());
workerConfig.setWorkerId(
"c-" + brokerConfig.getClusterName()
+ "-fw-" + hostname
- + "-" + workerConfig.getWorkerPort());
+ + "-" + (workerConfig.getWorkerPort() != null
+ ? workerConfig.getWorkerPort() : workerConfig.getWorkerPortTls()));
Review comment:
workerConfig.getTlsEnabled() ? workerConfig.getWorkerPort(): workerConfig.getWokerPortTls()
I believe would be consistent with the usage of TLS. Infering from the fact that the config has setup the port or not may lead to differences between the usage and the naming...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #9260: [Functions] Can't create functions with m-TLS
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9260:
URL: https://github.com/apache/pulsar/pull/9260#issuecomment-772392190
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org