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/01 22:42:10 UTC
[pinot] 01/01: fix config config validation failure for custom TLS listeners
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 <ap...@apache.org>
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: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org