kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkaranta...@apache.org
Subject [kafka] branch 2.5 updated: KAFKA-9768: Fix handling of rest.advertised.listener config (#8360)
Date Thu, 07 May 2020 01:30:25 GMT
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 <chrise@confluent.io>
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
    * 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 <rhauch@gmail.com>
    Reviewers: Randall Hauch <rhauch@gmail.com>, Konstantine Karantasis <konstantine@confluent.io>,
Andrew Choi <andchoi@linkedin.com>
 .../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
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
+     * @param protocol the protocol for the connector (e.g., "http" or "https").
+     * @return a {@link ServerConnector} for the server that uses the requested protocol,
+     * {@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
+            // protocol for the connector and then an underscore ("_"). We rely on that format
+            // when trying to locate a connector with the requested protocol; if the naming
+            // for the connectors we create is ever changed, we'll need to adjust the logic
+            // 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
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());

View raw message