kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkaranta...@apache.org
Subject [kafka] branch 2.4 updated: KAFKA-9768: Fix handling of rest.advertised.listener config (#8360)
Date Thu, 07 May 2020 01:29:02 GMT
This is an automated email from the ASF dual-hosted git repository.

kkarantasis pushed a commit to branch 2.4
in repository https://gitbox.apache.org/repos/asf/kafka.git

The following commit(s) were added to refs/heads/2.4 by this push:
     new 1f225d2  KAFKA-9768: Fix handling of rest.advertised.listener config (#8360)
1f225d2 is described below

commit 1f225d2414f11af5b43df67b3b83e45eb2f5c55f
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