This is an automated email from the ASF dual-hosted git repository.
raghavyadav01 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 dc4e9577177 add validateEnvironmentVariables (#18851)
dc4e9577177 is described below
commit dc4e95771776483b5f0b2b10062beda78b9bece1
Author: Jhow <[email protected]>
AuthorDate: Thu Jun 25 13:33:08 2026 -0700
add validateEnvironmentVariables (#18851)
---
.../api/resources/TableConfigValidationUtils.java | 29 +++++++++
.../resources/TableConfigValidationUtilsTest.java | 76 ++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtils.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtils.java
index 7058bb04812..ec51fbccec7 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtils.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtils.java
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.controller.api.resources;
+import com.google.common.annotations.VisibleForTesting;
import javax.annotation.Nullable;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
@@ -25,6 +26,7 @@ import
org.apache.pinot.controller.helix.core.minion.PinotTaskManager;
import org.apache.pinot.controller.helix.core.rebalance.TableRebalancer;
import org.apache.pinot.controller.util.TaskConfigUtils;
import org.apache.pinot.segment.local.utils.TableConfigUtils;
+import org.apache.pinot.spi.config.ConfigUtils;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableConfigValidatorRegistry;
import org.apache.pinot.spi.config.table.TableType;
@@ -57,6 +59,7 @@ public final class TableConfigValidationUtils {
public static void validateTableConfig(TableConfig tableConfig, Schema
schema,
@Nullable String typesToSkip, PinotHelixResourceManager resourceManager,
ControllerConf controllerConf, @Nullable PinotTaskManager taskManager) {
+ validateEnvironmentVariables(tableConfig);
TableConfigUtils.validate(tableConfig, schema, typesToSkip);
TableConfigUtils.validateTableName(tableConfig);
TableConfigUtils.ensureMinReplicas(tableConfig,
controllerConf.getDefaultTableMinReplicas());
@@ -69,6 +72,32 @@ public final class TableConfigValidationUtils {
TableConfigValidatorRegistry.validate(tableConfig, schema);
}
+ /**
+ * Validates that every environment variable / system property referenced by
the table config (via the
+ * {@code ${VAR}} template syntax, without a default value) can be resolved
at the time the config is written.
+ *
+ * Table configs are stored as templates and resolved lazily every time they
are read. If a referenced variable
+ * does not exist, the failure surfaces only at read time and can break
operations (e.g. segment commit)
+ * Resolving here makes the write fail fast so the bad config is never
persisted.
+ */
+ @VisibleForTesting
+ static void validateEnvironmentVariables(TableConfig tableConfig) {
+ try {
+ // Returns a resolved copy without mutating the original config; we only
care about whether it throws.
+ ConfigUtils.applyConfigWithEnvVariablesAndSystemProperties(tableConfig);
+ } catch (RuntimeException e) {
+ // ConfigUtils wraps the underlying "Missing environment Variable:
<name>" message in its cause, so surface
+ // the root cause to make the offending variable visible in the
rejection message.
+ Throwable rootCause = e;
+ while (rootCause.getCause() != null) {
+ rootCause = rootCause.getCause();
+ }
+ String reason = rootCause.getMessage() != null ? rootCause.getMessage()
: rootCause.toString();
+ throw new IllegalStateException("Failed to resolve environment
variables/system properties referenced in table "
+ + "config for table: " + tableConfig.getTableName() + ", reason: " +
reason, e);
+ }
+ }
+
private static void checkHybridTableConfig(PinotHelixResourceManager
resourceManager, TableConfig tableConfig) {
String rawTableName =
TableNameBuilder.extractRawTableName(tableConfig.getTableName());
if (tableConfig.getTableType() == TableType.REALTIME) {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtilsTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtilsTest.java
new file mode 100644
index 00000000000..c59b1aab42a
--- /dev/null
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/TableConfigValidationUtilsTest.java
@@ -0,0 +1,76 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.api.resources;
+
+import java.util.Collections;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableCustomConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+
+
+public class TableConfigValidationUtilsTest {
+
+ private TableConfig tableConfigWithCustomConfig(String key, String value) {
+ return new TableConfigBuilder(TableType.OFFLINE)
+ .setTableName("envVarTable")
+ .setCustomConfig(new TableCustomConfig(Collections.singletonMap(key,
value)))
+ .build();
+ }
+
+ @Test
+ public void testMissingEnvironmentVariableIsRejected() {
+ TableConfig tableConfig = tableConfigWithCustomConfig("key",
"${PINOT_NON_EXISTENT_ENV_VAR_XYZ}");
+ IllegalStateException e = expectThrows(IllegalStateException.class,
+ () ->
TableConfigValidationUtils.validateEnvironmentVariables(tableConfig));
+ assertTrue(e.getMessage().contains("PINOT_NON_EXISTENT_ENV_VAR_XYZ"),
+ "Expected message to reference the missing variable, got: " +
e.getMessage());
+ assertTrue(e.getMessage().contains("envVarTable"),
+ "Expected message to reference the table name, got: " +
e.getMessage());
+ }
+
+ @Test
+ public void testMissingEnvironmentVariableWithDefaultIsAccepted() {
+ // A template with a default value (${VAR:default}) resolves to the
default and must not be rejected.
+ TableConfig tableConfig = tableConfigWithCustomConfig("key",
"${PINOT_NON_EXISTENT_ENV_VAR_XYZ:fallback}");
+ TableConfigValidationUtils.validateEnvironmentVariables(tableConfig);
+ }
+
+ @Test
+ public void testResolvableSystemPropertyIsAccepted() {
+ String propertyName = "pinot.test.env.var.validation.prop";
+ System.setProperty(propertyName, "resolvedValue");
+ try {
+ TableConfig tableConfig = tableConfigWithCustomConfig("key", "${" +
propertyName + "}");
+ TableConfigValidationUtils.validateEnvironmentVariables(tableConfig);
+ } finally {
+ System.clearProperty(propertyName);
+ }
+ }
+
+ @Test
+ public void testConfigWithoutTemplatesIsAccepted() {
+ TableConfig tableConfig = tableConfigWithCustomConfig("key", "plainValue");
+ TableConfigValidationUtils.validateEnvironmentVariables(tableConfig);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
