This is an automated email from the ASF dual-hosted git repository.
kkarantasis pushed a commit to branch 2.5
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/2.5 by this push:
new 050ebef KAFKA-9768: Fix handling of rest.advertised.listener config
(#8360)
050ebef is described below
commit 050ebef6b1469e3b1537ccfbbc3dc5460e6c8589
Author: Chris Egerton <[email protected]>
AuthorDate: Wed May 6 18:09:47 2020 -0700
KAFKA-9768: Fix handling of rest.advertised.listener config (#8360)
The rest.advertised.listener config is currently broken as setting it to
http when listeners are configured for both https and http will cause the
framework to choose whichever of the two listeners is listed first. The changes
here attempt to fix this by checking not only that ServerConnector::getName
begins with the specified protocol, but also that that protocol is immediately
followed by an underscore, which the framework uses as a delimiter between the
protocol and the remainder o [...]
An existing unit test for the RestServer::advertisedUrl method has been
expanded to include a case that fails with the framework in its current state
and passes with the changes in this commit.
* KAFKA-9768: Fix handling of rest.advertised.listener config
* KAFKA-9768: Add comments on server connector names
* KAFKA-9768: Update RestServerTest comment
Co-authored-by: Randall Hauch <[email protected]>
Reviewers: Randall Hauch <[email protected]>, Konstantine Karantasis
<[email protected]>, Andrew Choi <[email protected]>
---
.../org/apache/kafka/connect/runtime/rest/RestServer.java | 13 ++++++++++++-
.../apache/kafka/connect/runtime/rest/RestServerTest.java | 8 ++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
index 9ad2446..02b4677 100644
---
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
+++
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
@@ -416,10 +416,21 @@ public class RestServer {
}
}
+ /**
+ * Locate a Jetty connector for the standard (non-admin) REST API that
uses the given protocol.
+ * @param protocol the protocol for the connector (e.g., "http" or
"https").
+ * @return a {@link ServerConnector} for the server that uses the
requested protocol, or
+ * {@code null} if none exist.
+ */
ServerConnector findConnector(String protocol) {
for (Connector connector : jettyServer.getConnectors()) {
String connectorName = connector.getName();
- if (connectorName.startsWith(protocol) &&
!ADMIN_SERVER_CONNECTOR_NAME.equals(connectorName))
+ // We set the names for these connectors when instantiating them,
beginning with the
+ // protocol for the connector and then an underscore ("_"). We
rely on that format here
+ // when trying to locate a connector with the requested protocol;
if the naming format
+ // for the connectors we create is ever changed, we'll need to
adjust the logic here
+ // accordingly.
+ if (connectorName.startsWith(protocol + "_") &&
!ADMIN_SERVER_CONNECTOR_NAME.equals(connectorName))
return (ServerConnector) connector;
}
diff --git
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
index ea6e98f..575c4da 100644
---
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
+++
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
@@ -174,6 +174,14 @@ public class RestServerTest {
config = new DistributedConfig(configMap);
server = new RestServer(config);
Assert.assertEquals("http://my-hostname:8080/";,
server.advertisedUrl().toString());
+
+ // correct listener is chosen when https listener is configured before
http listener and advertised listener is http
+ configMap = new HashMap<>(baseWorkerProps());
+ configMap.put(WorkerConfig.LISTENERS_CONFIG,
"https://encrypted-localhost:42069,http://plaintext-localhost:4761";);
+ configMap.put(WorkerConfig.REST_ADVERTISED_LISTENER_CONFIG, "http");
+ config = new DistributedConfig(configMap);
+ server = new RestServer(config);
+ Assert.assertEquals("http://plaintext-localhost:4761/";,
server.advertisedUrl().toString());
}
@Test
