refactor(native): Fail on empty or non-existent config environment variable

Author: nmahadevuni

presto-native-execution/presto_cpp/main/common/ConfigReader.cpp

@@ -27,15 +27,13 @@ namespace {
 void extractValueIfEnvironmentVariable(std::string& value) {
   if (value.size() > 3 && value.substr(0, 2) == "${" && value.back() == '}') {
     auto envName = value.substr(2, value.size() - 3);
-
     const char* envVal = std::getenv(envName.c_str());
-    if (envVal != nullptr) {
-      if (strlen(envVal) == 0) {
-        LOG(WARNING) << fmt::format(
-            "Config environment variable {} is empty.", envName);
-      }
-      value = std::string(envVal);
+
+    if (envVal == nullptr || strlen(envVal) == 0) {
+      VELOX_USER_FAIL(
+          "Config environment variable {} doesn't exist or is empty.", envName);
     }
+    value = std::string(envVal);
   }
 }
 } // namespace

presto-native-execution/presto_cpp/main/common/tests/ConfigTest.cpp

@@ -319,6 +319,8 @@ TEST_F(ConfigTest, optionalNodeId) {
 TEST_F(ConfigTest, readConfigEnvVarTest) {
   const std::string kEnvVarName = "PRESTO_READ_CONFIG_TEST_VAR";
   const std::string kEmptyEnvVarName = "PRESTO_READ_CONFIG_TEST_EMPTY_VAR";
+  const std::string kNonExistEnvVarName =
+      "PRESTO_READ_CONFIG_TEST_NON_EXIST_VAR";
 
   const std::string kPlainTextKey = "plain-text";
   const std::string kPlainTextValue = "plain-text-value";
@@ -337,22 +339,42 @@ TEST_F(ConfigTest, readConfigEnvVarTest) {
       fmt::format("{}=${{{}}}\n", kEnvVarKey, kEnvVarName) +
       fmt::format("{}=${{{}\n", kEnvVarKey2, kEnvVarName) +
       fmt::format("{}={}}}\n", kEnvVarKey3, kEnvVarName) +
-      fmt::format("{}=${{}}\n", kNoEnvVarKey) +
-      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kEmptyEnvVarName));
+      fmt::format("{}=${{}}\n", kNoEnvVarKey));
 
   setenv(kEnvVarName.c_str(), kEnvVarValue.c_str(), 1);
-  setenv(kEmptyEnvVarName.c_str(), "", 1);
 
   auto properties = presto::util::readConfig(configFilePath_);
   std::unordered_map<std::string, std::string> expected{
       {kPlainTextKey, kPlainTextValue},
       {kEnvVarKey, kEnvVarValue},
       {kEnvVarKey2, "${PRESTO_READ_CONFIG_TEST_VAR"},
       {kEnvVarKey3, "PRESTO_READ_CONFIG_TEST_VAR}"},
-      {kNoEnvVarKey, "${}"},
-      {kEmptyEnvVarKey, ""}};
+      {kNoEnvVarKey, "${}"}};
   ASSERT_EQ(properties, expected);
 
+  // Empty env var
+  auto testInvalidEnvVar = [this](
+                               const std::string& fileContent,
+                               const std::string& expectedErrorMsg) {
+    cleanupConfigFilePath();
+    setUpConfigFilePath();
+    writeConfigFile(fileContent);
+    VELOX_ASSERT_THROW(
+        presto::util::readConfig(configFilePath_), expectedErrorMsg);
+  };
+
+  setenv(kEmptyEnvVarName.c_str(), "", 1);
+  testInvalidEnvVar(
+      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kEmptyEnvVarName),
+      fmt::format(
+          "Config environment variable {} doesn't exist or is empty",
+          kEmptyEnvVarName));
+  testInvalidEnvVar(
+      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kNonExistEnvVarName),
+      fmt::format(
+          "Config environment variable {} doesn't exist or is empty",
+          kNonExistEnvVarName));
+
   unsetenv(kEnvVarName.c_str());
   unsetenv(kEmptyEnvVarName.c_str());
 }