You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "brbzull0 (via GitHub)" <gi...@apache.org> on 2023/03/14 17:24:17 UTC

[GitHub] [trafficserver] brbzull0 commented on a diff in pull request #9483: Add support for multiple yaml config files for wasm plugin

brbzull0 commented on code in PR #9483:
URL: https://github.com/apache/trafficserver/pull/9483#discussion_r1135940669


##########
plugins/experimental/wasm/wasm_main.cc:
##########
@@ -289,239 +311,248 @@ read_file(const std::string &fn, std::string *s)
 static bool
 read_configuration()
 {
-  // PluginBase parameters
-  std::string name          = "";
-  std::string root_id       = "";
-  std::string configuration = "";
-  bool fail_open            = true;
-
-  // WasmBase parameters
-  std::string runtime          = "";
-  std::string vm_id            = "";
-  std::string vm_configuration = "";
-  std::string wasm_filename    = "";
-  bool allow_precompiled       = true;
-
-  proxy_wasm::AllowedCapabilitiesMap cap_maps;
-  std::unordered_map<std::string, std::string> envs;
-
-  try {
-    YAML::Node config = YAML::LoadFile(wasm_config->config_filename);
-
-    for (YAML::const_iterator it = config.begin(); it != config.end(); ++it) {
-      const std::string &node_name = it->first.as<std::string>();
-      YAML::NodeType::value type   = it->second.Type();
-
-      if (node_name != "config" || type != YAML::NodeType::Map) {
-        TSError("[wasm][%s] Invalid YAML Configuration format for wasm: %s, reason: Top level nodes must be named config and be of "
-                "type map",
-                __FUNCTION__, wasm_config->config_filename.c_str());
-        return false;
-      }
+  std::list<std::pair<std::shared_ptr<ats_wasm::Wasm>, std::shared_ptr<proxy_wasm::PluginBase>>> new_configs = {};
+
+  for (auto const &cfn : wasm_config->config_filenames) {
+    // PluginBase parameters
+    std::string name          = "";
+    std::string root_id       = "";
+    std::string configuration = "";
+    bool fail_open            = true;
+
+    // WasmBase parameters
+    std::string runtime          = "";
+    std::string vm_id            = "";
+    std::string vm_configuration = "";
+    std::string wasm_filename    = "";
+    bool allow_precompiled       = true;
+
+    proxy_wasm::AllowedCapabilitiesMap cap_maps;
+    std::unordered_map<std::string, std::string> envs;
+
+    try {
+      YAML::Node config = YAML::LoadFile(cfn);
+
+      for (YAML::const_iterator it = config.begin(); it != config.end(); ++it) {
+        const std::string &node_name = it->first.as<std::string>();
+        YAML::NodeType::value type   = it->second.Type();
+
+        if (node_name != "config" || type != YAML::NodeType::Map) {
+          TSError(
+            "[wasm][%s] Invalid YAML Configuration format for wasm: %s, reason: Top level nodes must be named config and be of "
+            "type map",
+            __FUNCTION__, cfn.c_str());
+          return false;
+        }
 
-      for (YAML::const_iterator it2 = it->second.begin(); it2 != it->second.end(); ++it2) {
-        const YAML::Node first  = it2->first;
-        const YAML::Node second = it2->second;
+        for (YAML::const_iterator it2 = it->second.begin(); it2 != it->second.end(); ++it2) {
+          const YAML::Node first  = it2->first;
+          const YAML::Node second = it2->second;
 
-        const std::string &key = first.as<std::string>();
-        if (second.IsScalar()) {
-          const std::string &value = second.as<std::string>();
-          if (key == "name") {
-            name = value;
-          }
-          if (key == "root_id" || key == "rootId") {
-            root_id = value;
-          }
-          if (key == "configuration") {
-            configuration = value;
-          }
-          if (key == "fail_open") {
-            if (value == "false") {
-              fail_open = false;
+          const std::string &key = first.as<std::string>();
+          if (second.IsScalar()) {
+            const std::string &value = second.as<std::string>();
+            if (key == "name") {
+              name = value;
+            }
+            if (key == "root_id" || key == "rootId") {
+              root_id = value;
+            }
+            if (key == "configuration") {
+              configuration = value;
+            }
+            if (key == "fail_open") {
+              if (value == "false") {
+                fail_open = false;
+              }
             }
           }
-        }
-        if (second.IsMap() && (key == "capability_restriction_config")) {
-          if (second["allowed_capabilities"]) {
-            const YAML::Node ac_node = second["allowed_capabilities"];
-            if (ac_node.IsSequence()) {
-              for (const auto &i : ac_node) {
-                auto ac = i.as<std::string>();
-                proxy_wasm::SanitizationConfig sc;
-                cap_maps[ac] = sc;
+          if (second.IsMap() && (key == "capability_restriction_config")) {
+            if (second["allowed_capabilities"]) {
+              const YAML::Node ac_node = second["allowed_capabilities"];
+              if (ac_node.IsSequence()) {
+                for (const auto &i : ac_node) {
+                  auto ac = i.as<std::string>();
+                  proxy_wasm::SanitizationConfig sc;
+                  cap_maps[ac] = sc;
+                }
               }
             }
           }
-        }
 
-        if (second.IsMap() && (key == "vm_config" || key == "vmConfig")) {
-          for (YAML::const_iterator it3 = second.begin(); it3 != second.end(); ++it3) {
-            const YAML::Node vm_config_first  = it3->first;
-            const YAML::Node vm_config_second = it3->second;
+          if (second.IsMap() && (key == "vm_config" || key == "vmConfig")) {
+            for (YAML::const_iterator it3 = second.begin(); it3 != second.end(); ++it3) {
+              const YAML::Node vm_config_first  = it3->first;
+              const YAML::Node vm_config_second = it3->second;
 
-            const std::string &vm_config_key = vm_config_first.as<std::string>();
-            if (vm_config_second.IsScalar()) {
-              const std::string &vm_config_value = vm_config_second.as<std::string>();
-              if (vm_config_key == "runtime") {
-                runtime = vm_config_value;
-              }
-              if (vm_config_key == "vm_id" || vm_config_key == "vmId") {
-                vm_id = vm_config_value;
-              }
-              if (vm_config_key == "configuration") {
-                vm_configuration = vm_config_value;
-              }
-              if (vm_config_key == "allow_precompiled") {
-                if (vm_config_value == "false") {
-                  allow_precompiled = false;
+              const std::string &vm_config_key = vm_config_first.as<std::string>();
+              if (vm_config_second.IsScalar()) {
+                const std::string &vm_config_value = vm_config_second.as<std::string>();
+                if (vm_config_key == "runtime") {
+                  runtime = vm_config_value;
+                }
+                if (vm_config_key == "vm_id" || vm_config_key == "vmId") {
+                  vm_id = vm_config_value;
+                }
+                if (vm_config_key == "configuration") {
+                  vm_configuration = vm_config_value;
+                }
+                if (vm_config_key == "allow_precompiled") {
+                  if (vm_config_value == "false") {
+                    allow_precompiled = false;
+                  }
                 }
               }
-            }
 
-            if (vm_config_key == "environment_variables" && vm_config_second.IsMap()) {
-              if (vm_config_second["host_env_keys"]) {
-                const YAML::Node ek_node = vm_config_second["host_env_keys"];
-                if (ek_node.IsSequence()) {
-                  for (const auto &i : ek_node) {
-                    auto ek = i.as<std::string>();
-                    if (auto *value = std::getenv(ek.data())) {
-                      envs[ek] = value;
+              if (vm_config_key == "environment_variables" && vm_config_second.IsMap()) {
+                if (vm_config_second["host_env_keys"]) {
+                  const YAML::Node ek_node = vm_config_second["host_env_keys"];
+                  if (ek_node.IsSequence()) {
+                    for (const auto &i : ek_node) {
+                      auto ek = i.as<std::string>();
+                      if (auto *value = std::getenv(ek.data())) {
+                        envs[ek] = value;
+                      }
                     }
                   }
                 }
-              }
-              if (vm_config_second["key_values"]) {
-                const YAML::Node kv_node = vm_config_second["key_values"];
-                if (kv_node.IsMap()) {
-                  for (YAML::const_iterator it4 = kv_node.begin(); it4 != kv_node.end(); ++it4) {
-                    envs[it4->first.as<std::string>()] = it4->second.as<std::string>();
+                if (vm_config_second["key_values"]) {
+                  const YAML::Node kv_node = vm_config_second["key_values"];
+                  if (kv_node.IsMap()) {
+                    for (YAML::const_iterator it4 = kv_node.begin(); it4 != kv_node.end(); ++it4) {
+                      envs[it4->first.as<std::string>()] = it4->second.as<std::string>();
+                    }
                   }
                 }
               }
-            }
 
-            if (vm_config_key == "code" && vm_config_second.IsMap()) {
-              if (vm_config_second["local"]) {
-                const YAML::Node local_node = vm_config_second["local"];
-                if (local_node["filename"]) {
-                  wasm_filename = local_node["filename"].as<std::string>();
+              if (vm_config_key == "code" && vm_config_second.IsMap()) {
+                if (vm_config_second["local"]) {
+                  const YAML::Node local_node = vm_config_second["local"];
+                  if (local_node["filename"]) {
+                    wasm_filename = local_node["filename"].as<std::string>();
+                  }
                 }
               }
             }
           }
         }
-      }
 
-      // only allowed one config block (first one) for now
-      break;
+        // only allowed one config block (first one) for now
+        break;
+      }
+    } catch (const YAML::Exception &e) {
+      TSError("[wasm][%s] YAML::Exception %s when parsing YAML config file %s for wasm", __FUNCTION__, e.what(), cfn.c_str());
+      return false;
     }
-  } catch (const YAML::Exception &e) {
-    TSError("[wasm][%s] YAML::Exception %s when parsing YAML config file %s for wasm", __FUNCTION__, e.what(),
-            wasm_config->config_filename.c_str());
-    return false;
-  }
 
-  std::shared_ptr<ats_wasm::Wasm> wasm;
-  if (runtime == "ats.wasm.runtime.wasmedge") {
+    std::shared_ptr<ats_wasm::Wasm> wasm;
+    if (runtime == "ats.wasm.runtime.wasmedge") {
 #ifdef WASMEDGE
-    wasm = std::make_shared<ats_wasm::Wasm>(proxy_wasm::createWasmEdgeVm(), // VM
-                                            vm_id,                          // vm_id
-                                            vm_configuration,               // vm_configuration
-                                            "",                             // vm_key,
-                                            envs,                           // envs
-                                            cap_maps                        // allowed capabilities
-    );
+      wasm = std::make_shared<ats_wasm::Wasm>(proxy_wasm::createWasmEdgeVm(), // VM
+                                              vm_id,                          // vm_id
+                                              vm_configuration,               // vm_configuration
+                                              "",                             // vm_key,
+                                              envs,                           // envs
+                                              cap_maps                        // allowed capabilities
+      );
 #else
-    TSError("[wasm][%s] wasm unable to use WasmEdge runtime", __FUNCTION__);
-    return false;
+      TSError("[wasm][%s] wasm unable to use WasmEdge runtime", __FUNCTION__);
+      return false;
 #endif
-  } else if (runtime == "ats.wasm.runtime.wamr") {
+    } else if (runtime == "ats.wasm.runtime.wamr") {
 #ifdef WAMR
-    wasm = std::make_shared<ats_wasm::Wasm>(proxy_wasm::createWamrVm(), // VM
-                                            vm_id,                      // vm_id
-                                            vm_configuration,           // vm_configuration
-                                            "",                         // vm_key,
-                                            envs,                       // envs
-                                            cap_maps                    // allowed capabilities
-    );
+      wasm = std::make_shared<ats_wasm::Wasm>(proxy_wasm::createWamrVm(), // VM
+                                              vm_id,                      // vm_id
+                                              vm_configuration,           // vm_configuration
+                                              "",                         // vm_key,
+                                              envs,                       // envs
+                                              cap_maps                    // allowed capabilities
+      );
 #else
-    TSError("[wasm][%s] wasm unable to use WAMR runtime", __FUNCTION__);
-    return false;
+      TSError("[wasm][%s] wasm unable to use WAMR runtime", __FUNCTION__);
+      return false;
 #endif
-  } else {
-    TSError("[wasm][%s] wasm unable to use %s runtime", __FUNCTION__, runtime.c_str());
-    return false;
-  }
-  wasm->wasm_vm()->integration() = std::make_unique<ats_wasm::ATSWasmVmIntegration>();
-
-  auto plugin = std::make_shared<proxy_wasm::PluginBase>(name,          // name
-                                                         root_id,       // root_id
-                                                         vm_id,         // vm_id
-                                                         runtime,       // engine
-                                                         configuration, // plugin_configuration
-                                                         fail_open,     // failopen
-                                                         ""             // TODO: plugin key from where ?
-  );
-
-  wasm_config->wasm_filename = wasm_filename;
-  if (*wasm_config->wasm_filename.begin() != '/') {
-    wasm_config->wasm_filename = std::string(TSConfigDirGet()) + "/" + wasm_config->wasm_filename;
-  }
-  std::string code;
-  if (read_file(wasm_config->wasm_filename, &code) < 0) {
-    TSError("[wasm][%s] wasm unable to read file '%s'", __FUNCTION__, wasm_config->wasm_filename.c_str());
-    return false;
-  }
+    } else {
+      TSError("[wasm][%s] wasm unable to use %s runtime", __FUNCTION__, runtime.c_str());
+      return false;
+    }
+    wasm->wasm_vm()->integration() = std::make_unique<ats_wasm::ATSWasmVmIntegration>();
+
+    auto plugin = std::make_shared<proxy_wasm::PluginBase>(name,          // name
+                                                           root_id,       // root_id
+                                                           vm_id,         // vm_id
+                                                           runtime,       // engine
+                                                           configuration, // plugin_configuration
+                                                           fail_open,     // failopen
+                                                           ""             // TODO: plugin key from where ?
+    );
 
-  if (code.empty()) {
-    TSError("[wasm][%s] code is empty", __FUNCTION__);
-    return false;
-  }
+    if (*wasm_filename.begin() != '/') {
+      wasm_filename = std::string(TSConfigDirGet()) + "/" + wasm_filename;
+    }
+    std::string code;
+    if (read_file(wasm_filename, &code) < 0) {
+      TSError("[wasm][%s] wasm unable to read file '%s'", __FUNCTION__, wasm_filename.c_str());

Review Comment:
    maybe would be handy to know why? `ERRNO` must be available I guess.



##########
doc/admin-guide/plugins/wasm.en.rst:
##########
@@ -97,14 +97,15 @@ generated wasm modules with the plugin.
 Runtime can be chosen by changing the ``runtime`` field inside the yaml configuration file for the plugin.
 ``ats.wasm.runtime.wamr`` is for WAMR while ``ats.wasm.runtime.wasmedge`` is for WasmEdge.
 
+The plugin can also take more than one yaml file as arguments and can thus load more than one wasm modules.

Review Comment:
   Does make sense to support multiple documents in a single file as well?
   
   ```yaml
   # doc 1
   config:
     ...
   ---
   # doc 2
   config:
     ...
   ```
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org