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:09 UTC
[pinot] branch listener-tls-customization-validation-fixes created (now 4c742fb)
This is an automated email from the ASF dual-hosted git repository.
apucher pushed a change to branch listener-tls-customization-validation-fixes
in repository https://gitbox.apache.org/repos/asf/pinot.git.
at 4c742fb fix config config validation failure for custom TLS listeners
This branch includes the following new commits:
new 4c742fb fix config config validation failure for custom TLS listeners
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[pinot] 01/01: fix config config validation failure for custom TLS listeners
Posted by ap...@apache.org.
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