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


##########
src/proxy/Plugin.cc:
##########
@@ -376,3 +405,256 @@ plugin_init(bool validateOnly)
   }
   return retVal;
 }
+
+config::ConfigResult<PluginYAMLEntries>
+parse_plugin_yaml(const char *yaml_path)
+{
+  config::ConfigResult<PluginYAMLEntries> result;
+  YAML::Node                              root;
+
+  try {
+    root = YAML::LoadFile(yaml_path);
+  } catch (const YAML::Exception &e) {
+    result.errata.note("failed to parse: {}", e.what());
+    return result;
+  }
+
+  if (!root["plugins"] || !root["plugins"].IsSequence()) {
+    result.errata.note("missing or invalid 'plugins' sequence");
+    return result;
+  }
+
+  struct IndexedEntry {
+    int             seq_idx;
+    PluginYAMLEntry entry;
+  };
+
+  std::vector<IndexedEntry> indexed;
+  int                       seq_idx = 0;
+
+  for (const auto &node : root["plugins"]) {
+    PluginYAMLEntry entry;
+
+    if (!node["path"]) {
+      result.errata.note("plugin entry #{} missing required 'path' field", 
seq_idx + 1);
+      return result;
+    }
+    entry.path = node["path"].as<std::string>();
+
+    if (auto n = node["enabled"]; n) {
+      entry.enabled = n.as<bool>();
+    }
+    if (auto n = node["load_order"]; n) {
+      entry.load_order = n.as<int>();
+    }
+    if (auto n = node["params"]; n && n.IsSequence()) {
+      for (const auto &p : n) {
+        entry.params.emplace_back(p.as<std::string>());
+      }
+    }
+    if (auto n = node["config"]; n) {
+      if (n.IsScalar()) {
+        entry.config_literal = n.as<std::string>();
+      } else {
+        result.errata.note("plugin '{}': 'config' must be a scalar (use 
literal block '|' for multi-line content)", entry.path);
+        return result;
+      }
+    }
+
+    indexed.push_back({seq_idx++, std::move(entry)});
+  }
+
+  std::stable_sort(indexed.begin(), indexed.end(), [](const IndexedEntry &a, 
const IndexedEntry &b) {
+    const bool a_has = a.entry.load_order >= 0;
+    const bool b_has = b.entry.load_order >= 0;
+
+    if (a_has && b_has) {
+      return a.entry.load_order < b.entry.load_order;
+    }
+    return a_has && !b_has;
+  });
+
+  result.value.reserve(indexed.size());
+  for (auto &[_, entry] : indexed) {
+    result.value.emplace_back(std::move(entry));
+  }
+
+  return result;
+}
+
+/// Write inline config content to a temp file, returning the path on success.
+static std::optional<std::string>
+write_inline_config(const PluginYAMLEntry &entry, int index)
+{
+  char tmp_path[PATH_NAME_MAX];
+
+  std::string_view stem{entry.path};
+  if (auto pos = stem.rfind('/'); pos != std::string_view::npos) {
+    stem = stem.substr(pos + 1);
+  }
+  if (auto pos = stem.rfind('.'); pos != std::string_view::npos) {
+    stem = stem.substr(0, pos);
+  }
+
+  snprintf(tmp_path, sizeof(tmp_path), "%s/.%.*s_inline_%d.conf", 
RecConfigReadConfigDir().c_str(), static_cast<int>(stem.size()),
+           stem.data(), index);
+
+  int fd = open(tmp_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+  if (fd < 0) {
+    Error("%s: failed to create temp config for %s: %s", 
ts::filename::PLUGIN_YAML, entry.path.c_str(), strerror(errno));
+    return std::nullopt;
+  }
+
+  auto n = write(fd, entry.config_literal.data(), entry.config_literal.size());
+  close(fd);
+
+  if (n < 0 || static_cast<size_t>(n) != entry.config_literal.size()) {
+    Error("%s: failed to write inline config for %s", 
ts::filename::PLUGIN_YAML, entry.path.c_str());
+    return std::nullopt;
+  }
+
+  return std::string(tmp_path);
+}
+
+/// Build the argv for a single plugin: [path, inline_config_path?, params..., 
$record expansions].
+static std::optional<std::vector<std::string>>
+build_plugin_args(const PluginYAMLEntry &entry, int index)
+{
+  std::vector<std::string> args;
+  args.emplace_back(entry.path);
+
+  if (!entry.config_literal.empty()) {
+    if (auto path = write_inline_config(entry, index); path) {
+      args.emplace_back(std::move(*path));
+    } else {
+      return std::nullopt;
+    }
+  }
+
+  for (const auto &p : entry.params) {
+    args.emplace_back(p);
+  }
+
+  return args;
+}
+
+static void
+log_plugin_load_summary(int loaded, int disabled)
+{
+  Note("%s: %d plugins loaded, %d disabled", ts::filename::PLUGIN_YAML, 
loaded, disabled);
+
+  for (const auto &e : s_plugin_load_summary.entries) {
+    if (e.enabled) {
+      if (e.load_order >= 0) {
+        Note("  #%d %-30s load_order: %-5d loaded", e.index, e.path.c_str(), 
e.load_order);
+      } else {
+        Note("  #%d %-30s                 loaded", e.index, e.path.c_str());
+      }
+    } else {
+      Note("  -- %-30s                 disabled", e.path.c_str());
+    }
+  }
+}
+
+static void
+cleanup_inline_configs()
+{
+  std::string     config_dir = RecConfigReadConfigDir();
+  std::error_code ec;
+
+  try {
+    for (const auto &entry : std::filesystem::directory_iterator(config_dir, 
ec)) {
+      if (!entry.is_regular_file()) {
+        continue;
+      }
+      auto name = entry.path().filename().string();
+      if (name.front() == '.' && name.find("_inline_") != std::string::npos && 
name.ends_with(".conf")) {
+        std::filesystem::remove(entry.path(), ec);
+      }
+    }
+  } catch (const std::exception &e) {
+    Error("%s: error cleaning up inline config files: %s", 
ts::filename::PLUGIN_YAML, e.what());
+  }
+}
+
+bool
+plugin_yaml_init(bool validateOnly)
+{
+  plugin_dir_init();
+
+  ats_scoped_str yaml_path;
+
+  cleanup_inline_configs();
+
+  yaml_path = RecConfigReadConfigPath(nullptr, ts::filename::PLUGIN_YAML);
+  if (access(yaml_path, R_OK) != 0) {
+    return plugin_init(validateOnly);
+  }
+

Review Comment:
   Good point about validateOnly. Added a guard — cleanup is now skipped in 
validate-only mode.
   
   The unconditional call (before the plugin.yaml existence check) is 
intentional though: if the server previously ran with plugin.yaml and the 
operator then removes it, stale inline config files from the previous run 
should still be cleaned up. Moving cleanup after the existence check would 
leave those files behind. This matches the PluginFactory::cleanup() pattern 
which also runs unconditionally at startup.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to