This is an automated email from the ASF dual-hosted git repository. apucher pushed a commit to branch listener-tls-customization-validation-fixes in repository https://gitbox.apache.org/repos/asf/pinot.git
commit 4c742fb0b2dab6799c72b381e3cc5d75e8276ab0 Author: Alexander Pucher <[email protected]> AuthorDate: Tue Feb 1 14:41:32 2022 -0800 fix config config validation failure for custom TLS listeners --- .../apache/pinot/controller/ControllerConf.java | 12 ++++---- .../integration/tests/TlsIntegrationTest.java | 33 +++++++++++++++++++++- .../apache/pinot/tools/utils/PinotConfigUtils.java | 20 +++++++------ 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java index fbf827d..bea3806 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java @@ -379,16 +379,16 @@ public class ControllerConf extends PinotConfiguration { getControllerPort() == null ? Arrays.asList("http") : Arrays.asList()); } - public String getControllerAccessProtocolProperty(String protocol, String property) { - return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + protocol + "." + property); + public String getControllerAccessProtocolProperty(String name, String property) { + return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + name + "." + property); } - public String getControllerAccessProtocolProperty(String protocol, String property, String defaultValue) { - return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + protocol + "." + property, defaultValue); + public String getControllerAccessProtocolProperty(String name, String property, String defaultValue) { + return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + name + "." + property, defaultValue); } - public boolean getControllerAccessProtocolProperty(String protocol, String property, boolean defaultValue) { - return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + protocol + "." + property, defaultValue); + public boolean getControllerAccessProtocolProperty(String name, String property, boolean defaultValue) { + return getProperty(CONTROLLER_ACCESS_PROTOCOLS + "." + name + "." + property, defaultValue); } public String getDataDir() { 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: [email protected] For additional commands, e-mail: [email protected]
