You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ap...@apache.org on 2022/02/02 03:00:10 UTC

[pinot] branch master updated: fix controller config validation failure for customized TLS listeners (#8106)

This is an automated email from the ASF dual-hosted git repository.

apucher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new d08c7f8  fix controller config validation failure for customized TLS listeners (#8106)
d08c7f8 is described below

commit d08c7f8841360f46dc9ed68e07bf1850665c09d7
Author: Alexander Pucher <ap...@apache.org>
AuthorDate: Tue Feb 1 18:59:49 2022 -0800

    fix controller config validation failure for customized TLS listeners (#8106)
    
    This is a fix of a config validation error when using the new TLS listerner specs intoduced in #8082
---
 .../integration/tests/TlsIntegrationTest.java      | 33 +++++++++++++++++++++-
 .../apache/pinot/tools/utils/PinotConfigUtils.java | 20 +++++++------
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
index 53b09a7..2070aa0 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
@@ -28,6 +28,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.httpclient.methods.PostMethod;
 import org.apache.commons.io.FileUtils;
 import org.apache.http.Header;
@@ -45,6 +46,7 @@ import org.apache.pinot.client.JsonAsyncHttpPinotClientTransportFactory;
 import org.apache.pinot.client.Request;
 import org.apache.pinot.client.ResultSetGroup;
 import org.apache.pinot.common.utils.FileUploadDownloadClient;
+import org.apache.pinot.controller.ControllerConf;
 import org.apache.pinot.core.common.MinionConstants;
 import org.apache.pinot.integration.tests.access.CertBasedTlsChannelAccessControlFactory;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -54,6 +56,7 @@ import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.utils.CommonConstants;
 import org.apache.pinot.spi.utils.JsonUtils;
 import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
 import org.apache.pinot.util.TestUtils;
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
@@ -145,7 +148,7 @@ public class TlsIntegrationTest extends BaseClusterIntegrationTest {
 
     prop.put("controller.broker.protocol", "https");
 
-    // announce external only
+    // announce internal only
     prop.put("controller.vip.protocol", "https");
     prop.put("controller.vip.port", DEFAULT_CONTROLLER_PORT);
 
@@ -274,6 +277,34 @@ public class TlsIntegrationTest extends BaseClusterIntegrationTest {
   }
 
   @Test
+  public void testControllerConfigValidation()
+      throws Exception {
+    PinotConfigUtils.validateControllerConfig(new ControllerConf(getDefaultControllerConfiguration()));
+  }
+
+  @Test
+  public void testControllerConfigValidationImplicitProtocol()
+      throws Exception {
+    Map<String, Object> prop = new HashMap<>(getDefaultControllerConfiguration());
+    prop.put("controller.access.protocols", "https,http");
+    prop.put("controller.access.protocols.https.port", DEFAULT_CONTROLLER_PORT);
+    prop.put("controller.access.protocols.http.port", EXTERNAL_CONTROLLER_PORT);
+
+    PinotConfigUtils.validateControllerConfig(new ControllerConf(prop));
+  }
+
+  @Test(expectedExceptions = ConfigurationException.class)
+  public void testControllerConfigValidationNoProtocol()
+      throws Exception {
+    Map<String, Object> prop = new HashMap<>(getDefaultControllerConfiguration());
+    prop.put("controller.access.protocols", "invalid,http");
+    prop.put("controller.access.protocols.invalid.port", DEFAULT_CONTROLLER_PORT);
+    prop.put("controller.access.protocols.http.port", EXTERNAL_CONTROLLER_PORT);
+
+    PinotConfigUtils.validateControllerConfig(new ControllerConf(prop));
+  }
+
+  @Test
   public void testControllerExternalTrustedServer()
       throws Exception {
     try (CloseableHttpClient client = makeClient(JKS, _tlsStoreJKS, _tlsStoreJKS)) {
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/utils/PinotConfigUtils.java b/pinot-tools/src/main/java/org/apache/pinot/tools/utils/PinotConfigUtils.java
index 41244bb..239a19b 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/utils/PinotConfigUtils.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/utils/PinotConfigUtils.java
@@ -217,18 +217,18 @@ public class PinotConfigUtils {
 
   private static List<String> validateControllerAccessProtocols(ControllerConf conf)
       throws ConfigurationException {
-    List<String> protocols = conf.getControllerAccessProtocols();
+    List<String> listeners = conf.getControllerAccessProtocols();
 
-    if (!protocols.isEmpty()) {
-      Optional<String> invalidProtocol =
-          protocols.stream().filter(protocol -> !protocol.equals("http") && !protocol.equals("https")).findFirst();
+    if (!listeners.isEmpty()) {
+      Optional<String> invalidProtocol = listeners.stream().filter(name -> !isValidProtocol(name) && !isValidProtocol(
+          conf.getControllerAccessProtocolProperty(name, "protocol"))).findFirst();
 
       if (invalidProtocol.isPresent()) {
         throw new ConfigurationException(String.format(CONTROLLER_CONFIG_VALIDATION_ERROR_MESSAGE_FORMAT,
             invalidProtocol.get() + " is not a valid protocol for the 'controller.access.protocols' property."));
       }
 
-      Optional<ConfigurationException> invalidPort = protocols.stream()
+      Optional<ConfigurationException> invalidPort = listeners.stream()
           .map(protocol -> validatePort(protocol, conf.getControllerAccessProtocolProperty(protocol, "port")))
 
           .filter(Optional::isPresent)
@@ -242,14 +242,18 @@ public class PinotConfigUtils {
       }
     }
 
-    return protocols;
+    return listeners;
+  }
+
+  private static boolean isValidProtocol(String protocol) {
+    return "http".equals(protocol) || "https".equals(protocol);
   }
 
   private static Optional<ConfigurationException> validatePort(String protocol, String port) {
     if (port == null) {
       return Optional.of(new ConfigurationException(String.format(CONTROLLER_CONFIG_VALIDATION_ERROR_MESSAGE_FORMAT,
           "missing controller " + protocol + " port, please fix 'controller.access.protocols." + protocol
-              + ".port' property in config file.")));
+              + ".port' property in the config file.")));
     }
 
     try {
@@ -257,7 +261,7 @@ public class PinotConfigUtils {
     } catch (NumberFormatException e) {
       return Optional.of(new ConfigurationException(String.format(CONTROLLER_CONFIG_VALIDATION_ERROR_MESSAGE_FORMAT,
           port + " is not a valid port, please fix 'controller.access.protocols." + protocol
-              + ".port' property in config file.")));
+              + ".port' property in the config file.")));
     }
 
     return Optional.empty();

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org