This is an automated email from the ASF dual-hosted git repository.
yashmayya 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 a3af29e92b8 Trim listener access protocols and skipIndexes values
(#19648)
a3af29e92b8 is described below
commit a3af29e92b811bfc1e8cc94d3adb906a043b69ef
Author: Yash Mayya <[email protected]>
AuthorDate: Mon Sep 28 11:31:37 2026 -0700
Trim listener access protocols and skipIndexes values (#19648)
---
.../common/utils/config/QueryOptionsUtils.java | 4 +--
.../common/utils/config/QueryOptionsUtilsTest.java | 19 +++++++++++
.../controller/util/ListenerConfigUtilTest.java | 38 ++++++++++++++++++++++
.../apache/pinot/core/util/ListenerConfigUtil.java | 5 ++-
4 files changed, 63 insertions(+), 3 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
index 68eb597482e..76519cc1cc4 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
@@ -443,12 +443,12 @@ public class QueryOptionsUtils {
throw new RuntimeException("Invalid format for " +
QueryOptionKey.SKIP_INDEXES
+ ". Example of valid format: SET
skipIndexes='col1=inverted,range&col2=inverted'");
}
- String columnName = conf[0];
+ String columnName = conf[0].trim();
String[] indexTypes = StringUtils.split(conf[1], ',');
for (String indexType : indexTypes) {
skipIndexes.computeIfAbsent(columnName, k -> new HashSet<>())
- .add(FieldConfig.IndexType.valueOf(indexType.toUpperCase()));
+
.add(FieldConfig.IndexType.valueOf(indexType.trim().toUpperCase(Locale.ROOT)));
}
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
index 1bda5547c27..29dad29d5b7 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
@@ -25,6 +25,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.pinot.spi.config.table.FieldConfig;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey.*;
@@ -132,6 +133,24 @@ public class QueryOptionsUtilsTest {
assertEquals(skipIndexes.get("col2"),
Set.of(FieldConfig.IndexType.SORTED));
}
+ /// Asserts that spaces around column names and index types are ignored.
+ @Test(dataProvider = "skipIndexesWithSpaces")
+ public void testSkipIndexesParsingWithSpaces(String skipIndexesStr) {
+ Map<String, Set<FieldConfig.IndexType>> skipIndexes =
+ QueryOptionsUtils.getSkipIndexes(Map.of(SKIP_INDEXES, skipIndexesStr));
+ assertEquals(skipIndexes, Map.of("col1",
Set.of(FieldConfig.IndexType.INVERTED, FieldConfig.IndexType.RANGE),
+ "col2", Set.of(FieldConfig.IndexType.SORTED)));
+ }
+
+ @DataProvider
+ public Object[][] skipIndexesWithSpaces() {
+ return new Object[][]{
+ {"col1=inverted, range&col2=sorted"},
+ {"col1=inverted,range& col2=sorted"},
+ {" col1 = inverted , range & col2 = sorted "}
+ };
+ }
+
@Test(expectedExceptions = RuntimeException.class)
public void testSkipIndexesParsingInvalid() {
String skipIndexesStr = "col1=inverted,range&col2";
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java
index fb5f6452110..23d34b45646 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/util/ListenerConfigUtilTest.java
@@ -19,12 +19,14 @@
package org.apache.pinot.controller.util;
import java.util.List;
+import org.apache.pinot.common.config.TlsConfig;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.core.transport.HttpServerThreadPoolConfig;
import org.apache.pinot.core.transport.ListenerConfig;
import org.apache.pinot.core.util.ListenerConfigUtil;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.testng.Assert;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
@@ -154,6 +156,42 @@ public class ListenerConfigUtilTest {
assertHttpsListener(httpsListener, "0.0.0.0", 9443);
}
+ /// Asserts that protocol names are trimmed and empty names are skipped.
Cluster configs are applied with
+ /// `setProperty`, which keeps the raw value, so a value like `http, https`
is not split or trimmed by the config.
+ @Test(dataProvider = "accessProtocolsWithSpaces")
+ public void testAccessProtocolsWithSpaces(String accessProtocols) {
+ ControllerConf controllerConf = new ControllerConf();
+
+ controllerConf.setProperty("controller.access.protocols", accessProtocols);
+ controllerConf.setProperty("controller.access.protocols.http.port",
"9000");
+
+ configureHttpsProperties(controllerConf, 9443);
+
+ List<ListenerConfig> listenerConfigs =
ListenerConfigUtil.buildControllerConfigs(controllerConf);
+
+ Assert.assertEquals(listenerConfigs.size(), 2);
+
+ assertHttpListener(getListener("http", listenerConfigs), "0.0.0.0", 9000);
+ assertHttpsListener(getListener("https", listenerConfigs), "0.0.0.0",
9443);
+ }
+
+ @DataProvider
+ public Object[][] accessProtocolsWithSpaces() {
+ return new Object[][]{{"http, https"}, {" http , https "}, {"http,
,https,"}};
+ }
+
+ /// Asserts that no listener is generated when the protocols value is blank
or has only empty names.
+ @Test
+ public void testBlankAccessProtocols() {
+ for (String accessProtocols : List.of("", " ", " , ")) {
+ PinotConfiguration config = new PinotConfiguration();
+ config.setProperty("controller.access.protocols", accessProtocols);
+
+ Assert.assertEquals(ListenerConfigUtil.buildListenerConfigs(config,
"controller", new TlsConfig()), List.of(),
+ "Protocols: '" + accessProtocols + "'");
+ }
+ }
+
/// Asserts that a single listener configuration is generated with a secured
TLS port.
@Test
public void testHttpsOnly() {
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
b/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
index 13b821926db..6404f6d76fe 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
@@ -86,9 +86,12 @@ public final class ListenerConfigUtil {
return new ArrayList<>();
}
+ // Trim each protocol and skip empty ones. Cluster configs are applied
with setProperty(), which keeps the raw
+ // value, so a value like "http, https" is not split or trimmed by the
config.
String[] protocols = config.getProperty(namespace +
DOT_ACCESS_PROTOCOLS).split(",");
- return Arrays.stream(protocols).map(protocol ->
buildListenerConfig(config, namespace, protocol, tlsDefaults))
+ return Arrays.stream(protocols).map(String::trim).filter(protocol ->
!protocol.isEmpty())
+ .map(protocol -> buildListenerConfig(config, namespace, protocol,
tlsDefaults))
.collect(Collectors.toList());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
