You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "serrislew (via GitHub)" <gi...@apache.org> on 2026/04/20 23:49:04 UTC

[PR] virtualhost.yaml with remap rules [trafficserver]

serrislew opened a new pull request, #13108:
URL: https://github.com/apache/trafficserver/pull/13108

   V2 of #12669 but including remap.yaml (#12997)
   
   - Resolves to a single virtualhost entry
      - Remap rule logic remains the same. Order ranking still applies (i.e. rules defined first have priority)
      - Virtualhost remap rules have priority. If no remap rule matches or virtualhost is found, existing remap.yaml or .config is then considered
   - Virtualhost domains support exact and wildcard (only single left-most "*") matching
   - Supports traffic_ctl config reload of single virtualhost entry
      - `$ traffic_ctl config reload --virtualhost foo`
      - Uses token model reload


-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r3848291651


##########
include/proxy/http/remap/UrlRewrite.h:
##########
@@ -80,13 +80,15 @@ class UrlRewrite
    */
   bool load(ConfigContext ctx = {});
 
+  bool load_table(const std::string &config_file_path, YAML::Node const *remap_node, ConfigContext ctx = {});
+

Review Comment:
   `UrlRewrite.h` now exposes `YAML::Node` in the public API (`load_table` / `BuildTable`) but the header doesn't declare `YAML::Node`. This will fail to compile for translation units that include `UrlRewrite.h` without including yaml-cpp first. Add a forward declaration (or include `<yaml-cpp/yaml.h>`) before `class UrlRewrite`.



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r4077496140


##########
src/proxy/http/remap/RemapYamlConfig.cc:
##########
@@ -1046,3 +1091,20 @@ remap_parse_yaml(const char *path, UrlRewrite *rewrite, ConfigContext ctx)
 
   return status;
 }
+
+bool
+remap_parse_yaml(YAML::Node const *remap_node, UrlRewrite *rewrite, ConfigContext ctx)
+{
+  BUILD_TABLE_INFO bti;
+
+  rewrite->pluginFactory.indicatePreReload();

Review Comment:
   Each virtualhost invokes this YAML overload, but `PluginFactory::indicatePreReload()` and `indicatePostReload()` broadcast over the process-wide loaded-plugin list, while the `pluginUsed` map is local to the current factory. With multiple virtualhost tables (or a global table plus a virtualhost table), later builds send another reload notification and mark plugins used only by earlier tables as `TSREMAP_CONFIG_RELOAD_SUCCESS_PLUGIN_UNUSED`, so plugin reload callbacks can reset live state. Aggregate one notification for the whole virtualhost rebuild or make the callbacks table-scoped.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,586 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    The single-entry reload is a read-copy-modify-publish against the live config, so its read and
+    its publish must be atomic with respect to any other reload. The full reload has the same
+    requirement for a different reason: two full reloads that parse concurrently can publish in the
+    opposite order to their reads, so the older read wins and resurrects entries the newer one had
+    dropped. Both therefore hold this for the parse as well as the publish. Reload paths are
+    scheduled on ET_TASK with unrelated mutexes (a fresh one per @c ConfigRegistry::schedule_reload
+    and one per trigger record), so nothing else serializes them. Neither path is on the request
+    path, so serializing the parse costs nothing.
+ */
+std::mutex vhost_reconfigure_mutex;
+} // namespace
+
+std::atomic<int> VirtualHost::_configid{0};
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+namespace
+{
+const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+/** Check that @a name is composed of non-empty hostname labels.
+
+    Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as
+    an exact domain that no Host header can ever equal: the entry loads clean and never fires.
+ */
+bool
+is_hostname(std::string_view name)
+{
+  // A bracketed IPv6 literal is matched verbatim against the Host header.
+  if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+    return true;
+  }
+
+  size_t label_len = 0;
+
+  for (char c : name) {
+    if (c == '.') {
+      if (label_len == 0) {
+        return false;
+      }
+      label_len = 0;
+      continue;
+    }
+    if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') {
+      return false;
+    }
+    ++label_len;
+  }
+  return label_len > 0;
+}
+
+/** Decode a single `virtualhost` sequence element.
+
+    Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload
+    task log, not just in diags.log.
+ */
+bool
+decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx)
+{
+  if (!node["id"]) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+    return false;
+  }
+  item.id = node["id"].as<std::string>();

Review Comment:
   An empty `id` is accepted during startup, but the reload handler explicitly rejects empty IDs, leaving a live virtualhost entry that cannot be addressed by the documented single-entry reload. Reject empty identifiers while decoding the entry.
   
   This issue also appears in the following locations of the same file:
   - line 471
   - line 547



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#issuecomment-4334326961

   I was talking with @serrislew about some parts of this PR, specially related to the interaction with the reload handler. I think https://github.com/apache/trafficserver/pull/13110 is the plumbing that the id base reloading could benefit from.


-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r3337482724


##########
include/proxy/VirtualHost.h:
##########
@@ -0,0 +1,102 @@
+/** @file
+  Virtual Host configuration
+  @section license License
+  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.
+ */
+
+#pragma once
+
+#include <string>
+#include <string_view>
+
+#include "iocore/eventsystem/ConfigProcessor.h"
+#include "proxy/http/remap/UrlRewrite.h"
+#include "tscore/Ptr.h"

Review Comment:
   VirtualHost.h uses std::vector and std::unordered_map but doesn't include <vector> / <unordered_map>. This makes the header non-self-contained and can break compilation depending on include order.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,449 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <set>
+#include <string_view>
+#include <string>
+#include <yaml-cpp/yaml.h>

Review Comment:
   This file uses struct stat/stat(), errno/ENOENT, std::none_of, std::make_unique, and index() but doesn't include the headers that declare them. Relying on transitive includes is fragile and can fail to compile on some platforms/toolchains.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,449 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <set>
+#include <string_view>
+#include <string>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+VirtualHostConfig::Entry *
+VirtualHostConfig::Entry::acquire() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self) {
+    self->refcount_inc();
+  }
+  return self;
+}
+
+void
+VirtualHostConfig::Entry::release() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self && self->refcount_dec() == 0) {
+    self->free();
+  }
+}
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide `id`");
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide at least one domain in `domains` sequence");
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Dbg(dbg_ctl_virtualhost, "Virtual host entry can't have empty domain entry");
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        const char *subdomain = index(domain, '*');
+        if (subdomain && subdomain[1] == '.') {
+          item.wildcard_domains.push_back(subdomain + 2);
+        } else {
+          Dbg(dbg_ctl_virtualhost, "Virtual host wildcard entry must have '*.[domain]' format");
+        }
+      } else {
+        item.exact_domains.push_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must have at least one domain defined");
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Dbg(dbg_ctl_virtualhost, "Failed to parse virtualhost entry");
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Dbg(dbg_ctl_virtualhost, "Failed to load remap rules for virtualhost entry");
+      return false;
+    }
+    conf.remap_table = make_ptr(table.release());
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    Warning("Virtualhost configuration '%s' doesn't exist", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Dbg(dbg_ctl_virtualhost, "Expected toplevel 'virtualhost' key to be a sequence");
+      return false;
+    }
+
+    for (auto const &node : config) {
+      Ptr<Entry> entry;
+      if (!build_virtualhost_entry(node, entry)) {
+        return false;
+      }
+
+      std::string vhost_id{entry->id};
+      if (_entries.contains(vhost_id)) {
+        Dbg(dbg_ctl_virtualhost, "Duplicate virtualhost id: %s", vhost_id.c_str());
+        return false;
+      }
+
+      for (auto const &domain : entry->exact_domains) {
+        if (_exact_domains_to_id.contains(domain)) {
+          Dbg(dbg_ctl_virtualhost, "Exact domain (%s) already in another virtualhost config", domain.c_str());
+          return false;
+        }
+        _exact_domains_to_id.emplace(domain, vhost_id);
+      }
+
+      for (auto const &domain_suffix : entry->wildcard_domains) {
+        if (_wildcard_domains_to_id.contains(domain_suffix)) {
+          Dbg(dbg_ctl_virtualhost, "Wildcard domain (%s) already in another virtualhost config", domain_suffix.c_str());
+          return false;
+        }
+        _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+      }
+
+      _entries.emplace(vhost_id, std::move(entry));
+    }
+
+  } catch (std::exception &ex) {
+    Dbg(dbg_ctl_virtualhost, "Failed to load %s: %s", config_path.c_str(), ex.what());
+    return false;
+  }
+  return true;
+}
+
+bool
+VirtualHostConfig::load_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  entry.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }

Review Comment:
   load_entry() treats an empty virtualhost config as success (returns true) while leaving 'entry' unset. In VirtualHost::reconfigure(id), this ends up calling set_entry(id, nullptr), which will delete any existing entry for that id. Empty config should fail the single-entry reload rather than silently removing entries.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,449 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <set>
+#include <string_view>
+#include <string>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+VirtualHostConfig::Entry *
+VirtualHostConfig::Entry::acquire() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self) {
+    self->refcount_inc();
+  }
+  return self;
+}
+
+void
+VirtualHostConfig::Entry::release() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self && self->refcount_dec() == 0) {
+    self->free();
+  }
+}
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide `id`");
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide at least one domain in `domains` sequence");
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Dbg(dbg_ctl_virtualhost, "Virtual host entry can't have empty domain entry");
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        const char *subdomain = index(domain, '*');
+        if (subdomain && subdomain[1] == '.') {
+          item.wildcard_domains.push_back(subdomain + 2);
+        } else {
+          Dbg(dbg_ctl_virtualhost, "Virtual host wildcard entry must have '*.[domain]' format");
+        }
+      } else {
+        item.exact_domains.push_back(domain);
+      }
+    }

Review Comment:
   Wildcard domain validation doesn't enforce the documented "single left-most *" rule. Patterns like "*.*.baz.com" will currently be accepted and stored as a suffix containing '*', which will never match and is likely a config mistake.



##########
doc/admin-guide/files/virtualhost.yaml.en.rst:
##########
@@ -0,0 +1,228 @@
+
+.. 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.
+
+.. include:: ../../common.defs
+
+.. configfile:: virtualhost.yaml
+
+virtualhost.yaml
+****************
+
+The :file:`virtualhost.yaml` file defines configuration blocks that apply to a group of domains.
+Each virtual host entry defines a set of domains and the remap rules associated with those domains.
+Virtual host remap rules override global :file:`remap.yaml` rules but remain fully backward compatible
+with existing configurations. If absent, ATS behaves exactly as before.
+
+Currently, this file only supports :file:`remap.yaml` overrides. Future versions will expand virtual
+host support to additional configuration types (e.g. :file:`sni.yaml`, :file:`ssl_multicert.yaml`,
+:file:`parent.config`, etc)
+
+By default this is named :file:`virtualhost.yaml`. The filename can be changed by setting
+:ts:cv:`proxy.config.virtualhost.filename`.
+
+
+Configuration
+=============
+
+:file:`virtualhost.yaml` is YAML format with top level namespace **virtualhost** and a list of virtual host
+entries. Each virtual host entry must provide an **id** and at least one domain defined in **domains**.
+
+An example configuration looks like:
+
+.. code-block:: yaml
+
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+
+===================== ==========================================================
+Field Name            Description
+===================== ==========================================================
+``id``                 Virtual host identifier to perform specific operations on
+``domains``            List of domains to resolve a request to
+``remap``              List of remap rules as defined in remap.yaml
+===================== ==========================================================
+
+``domains``
+   Domains can be defined as request domain name or subdomains using wildcard feature.
+   Wildcard support only allows single left most ``*``. This does not support regex.
+   When matching to a virtual host entry, domains with exact match have precedence
+   over wildcard. If a domain matches to multiple wildcard domains, the virtual host
+   config defined first has precedence.
+
+   For example:
+      Supported:
+      - ``foo.example.com``
+      - ``*.example.com``
+      - ``*.com``
+
+      NOT Supported:
+      - ``foo[0-9]+.example.com`` (regex)
+      - ``bar.*.example.net`` (``*`` in the middle)
+      - ``*.bar.*.com`` (multiple ``*``)
+      - ``*.*.baz.com`` (multiple ``*``)
+      - ``baz*.example.net`` (partial wildcard)
+      - ``*baz.example.net`` (partial wildcard)
+      - ``b*z.example.net`` (partial wildcard)
+      - ``*`` (global)
+
+Evaluation Order
+----------------
+
+|TS| evaluates a request using deterministic precedence in the following order:
+
+1. Resolve to a single virtualhost
+   a. Check for an exact domain match. If any virtual host lists the request hostname explicitly, that virtual host is selected.
+   b. Check for a wildcard domain match. If any virtual host wildcard domains define a subdomain of the request hostname in the form ``*.[domain]``, that virtual host is selected.
+   c. If no matching virtual host exists, the request proceeds using global configuration (i.e :file:`remap.config`). Skip to step 3.
+2. Within selected virtual host config, use virtual host remap rules.
+   a. Follow existing :file:`remap.yaml` rules and matching orders. If a matching remap rule is found, that remap rule is selected.
+3. If neither virtual host nor remap rules match, ATS falls back to global :file:`remap.yaml` resolution.
+
+Only one virtual host entry may match a given request. If multiple entries could match, ATS uses the first matching
+entry defined in :file:`virtualhost.yaml`.

Review Comment:
   This section says ATS uses the first matching virtualhost entry in the file when multiple entries match. The current matching code instead selects exact matches first, then the most-specific wildcard suffix (longest suffix) rather than file order. Please adjust this description to match the actual selection logic (or update the implementation).



##########
doc/admin-guide/files/virtualhost.yaml.en.rst:
##########
@@ -0,0 +1,228 @@
+
+.. 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.
+
+.. include:: ../../common.defs
+
+.. configfile:: virtualhost.yaml
+
+virtualhost.yaml
+****************
+
+The :file:`virtualhost.yaml` file defines configuration blocks that apply to a group of domains.
+Each virtual host entry defines a set of domains and the remap rules associated with those domains.
+Virtual host remap rules override global :file:`remap.yaml` rules but remain fully backward compatible
+with existing configurations. If absent, ATS behaves exactly as before.
+
+Currently, this file only supports :file:`remap.yaml` overrides. Future versions will expand virtual
+host support to additional configuration types (e.g. :file:`sni.yaml`, :file:`ssl_multicert.yaml`,
+:file:`parent.config`, etc)
+
+By default this is named :file:`virtualhost.yaml`. The filename can be changed by setting
+:ts:cv:`proxy.config.virtualhost.filename`.
+
+
+Configuration
+=============
+
+:file:`virtualhost.yaml` is YAML format with top level namespace **virtualhost** and a list of virtual host
+entries. Each virtual host entry must provide an **id** and at least one domain defined in **domains**.
+
+An example configuration looks like:
+
+.. code-block:: yaml
+
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+
+===================== ==========================================================
+Field Name            Description
+===================== ==========================================================
+``id``                 Virtual host identifier to perform specific operations on
+``domains``            List of domains to resolve a request to
+``remap``              List of remap rules as defined in remap.yaml
+===================== ==========================================================
+
+``domains``
+   Domains can be defined as request domain name or subdomains using wildcard feature.
+   Wildcard support only allows single left most ``*``. This does not support regex.
+   When matching to a virtual host entry, domains with exact match have precedence
+   over wildcard. If a domain matches to multiple wildcard domains, the virtual host
+   config defined first has precedence.
+
+   For example:
+      Supported:
+      - ``foo.example.com``
+      - ``*.example.com``
+      - ``*.com``
+
+      NOT Supported:
+      - ``foo[0-9]+.example.com`` (regex)
+      - ``bar.*.example.net`` (``*`` in the middle)
+      - ``*.bar.*.com`` (multiple ``*``)
+      - ``*.*.baz.com`` (multiple ``*``)
+      - ``baz*.example.net`` (partial wildcard)
+      - ``*baz.example.net`` (partial wildcard)
+      - ``b*z.example.net`` (partial wildcard)
+      - ``*`` (global)
+
+Evaluation Order
+----------------
+
+|TS| evaluates a request using deterministic precedence in the following order:
+
+1. Resolve to a single virtualhost
+   a. Check for an exact domain match. If any virtual host lists the request hostname explicitly, that virtual host is selected.
+   b. Check for a wildcard domain match. If any virtual host wildcard domains define a subdomain of the request hostname in the form ``*.[domain]``, that virtual host is selected.
+   c. If no matching virtual host exists, the request proceeds using global configuration (i.e :file:`remap.config`). Skip to step 3.
+2. Within selected virtual host config, use virtual host remap rules.
+   a. Follow existing :file:`remap.yaml` rules and matching orders. If a matching remap rule is found, that remap rule is selected.
+3. If neither virtual host nor remap rules match, ATS falls back to global :file:`remap.yaml` resolution.
+
+Only one virtual host entry may match a given request. If multiple entries could match, ATS uses the first matching
+entry defined in :file:`virtualhost.yaml`.
+
+
+Granular Reload
+===============
+
+|TS| now supports granular configuration reloads for individual virtual hosts defined in :file:`virtualhost.yaml`.
+In addition to reloading the entire |TS| configuration with :option:`traffic_ctl config reload`, users can
+selectively reload a single virtual host entry without affecting other virtual host entries.
+
+By only updating the necessary changes, this reduces configuration deployment time and improves visibility on the changes made.
+
+To reload for a specific virtual host, use new reload directive:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=<id>
+
+Where **<id>** is the virtual host ID defined in :file:`virtualhost.yaml`. Only the **<id>** virtual host
+configuration will be reloaded. This does not affect other virtual hosts or global configuration files.
+
+Example:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=foo
+    ✔ Reload scheduled [rpc-123456789]
+
+    Monitor : traffic_ctl config reload -t rpc-123456789 -m
+    Details : traffic_ctl config reload -t rpc-123456789 -s -l
+
+  $ traffic_ctl config reload -t rpc-123456789 -s -l
+    ✗ Token 'rpc-123456789' already in use
+    ✔ Reload [success] — rpc-123456789
+    Started : 2026 May 19 19:20:20.691
+    Finished: 2026 May 19 19:20:20.692
+    Duration: 1ms
+
+    ✔ 1 success  ◌ 0 in-progress  ✗ 0 failed  (1 total)
+
+    Tasks:
+    ✔ virtualhost ··································    1ms
+        [Note]  Reloaded virtualhost entry: foo
+
+
+Examples
+========
+
+.. code-block:: yaml
+
+   # virtualhost.yaml
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+   # remap.yaml
+   remap:
+     - type: map
+        from:
+          url: http://www.x.com
+        to:
+          url: http://other.example.com/
+
+This rules translates in the following translation.
+
+================================================ ========================================================
+Client Request                                   Translated Request
+================================================ ========================================================
+``http://example.com/index.html``                ``http://origin.example.com/index.html``
+``http://www.x.com/index.html``                  ``http://other.example.com/index.html``
+================================================ ========================================================
+
+.. code-block:: yaml
+
+   # virtualhost.yaml
+   virtualhost:
+     - id: example
+       domains:
+         - "*.example.com"
+
+       remap:
+         - type: regex_map
+           from:
+            url: http://sub[0-9]+.example.com/
+           to:
+            url: http://origin$1.example.com/
+
+
+      - id: foo
+        domains:
+         - foo.example.com
+
+       remap:
+         - type: map
+           from:
+            url: http:/foo.example.com/
+           to:
+            url: http://foo.origin.com/

Review Comment:
   This virtualhost.yaml example has invalid indentation for the second list item and a malformed URL ("http:/foo..."). As written, it won't parse and could mislead users.



##########
tests/gold_tests/jsonrpc/config_reload_rpc.test.py:
##########
@@ -424,30 +424,36 @@ def validate_directive_fileonly(resp: Response):
 tr.StillRunningAfter = ts
 
 # ============================================================================
-# Test 12: Reload directive for unregistered config (virtualhost)
-# virtualhost is not registered yet — should get 6010.
-# This is the intended use case once the virtualhost handler is registered.
+# Test 12: Reload directive routes to registered handler (virtualhost)
+# virtualhost is registered as FileAndRpc — the _reload directive should be
+# accepted by the framework and forwarded to VirtualHost's handler, which
+# schedules an inline reload and returns a task token.
 # ============================================================================
-tr = Test.AddTestRun("Reload directive for unregistered config (virtualhost)")
+tr = Test.AddTestRun("Reload directive routed to registered virtualhost handler")
 tr.DelayStart = 2
 tr.AddJsonRPCClientRequest(ts, Request.admin_config_reload(configs={"virtualhost": {"_reload": {"id": "myhost.example.com"}}}))
 
 
-def validate_directive_unregistered(resp: Response):
+def validate_directive_routed(resp: Response):
     '''virtualhost is not registered — rejected with 6010'''
     result = resp.result
     errors = result.get('errors', [])

Review Comment:
   The docstring still says virtualhost is unregistered / rejected with 6010, but this test now expects the directive to be routed to the registered handler. Update the docstring to match the behavior being validated.



##########
doc/admin-guide/files/virtualhost.yaml.en.rst:
##########
@@ -0,0 +1,228 @@
+
+.. 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.
+
+.. include:: ../../common.defs
+
+.. configfile:: virtualhost.yaml
+
+virtualhost.yaml
+****************
+
+The :file:`virtualhost.yaml` file defines configuration blocks that apply to a group of domains.
+Each virtual host entry defines a set of domains and the remap rules associated with those domains.
+Virtual host remap rules override global :file:`remap.yaml` rules but remain fully backward compatible
+with existing configurations. If absent, ATS behaves exactly as before.
+
+Currently, this file only supports :file:`remap.yaml` overrides. Future versions will expand virtual
+host support to additional configuration types (e.g. :file:`sni.yaml`, :file:`ssl_multicert.yaml`,
+:file:`parent.config`, etc)
+
+By default this is named :file:`virtualhost.yaml`. The filename can be changed by setting
+:ts:cv:`proxy.config.virtualhost.filename`.
+
+
+Configuration
+=============
+
+:file:`virtualhost.yaml` is YAML format with top level namespace **virtualhost** and a list of virtual host
+entries. Each virtual host entry must provide an **id** and at least one domain defined in **domains**.
+
+An example configuration looks like:
+
+.. code-block:: yaml
+
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+
+===================== ==========================================================
+Field Name            Description
+===================== ==========================================================
+``id``                 Virtual host identifier to perform specific operations on
+``domains``            List of domains to resolve a request to
+``remap``              List of remap rules as defined in remap.yaml
+===================== ==========================================================
+
+``domains``
+   Domains can be defined as request domain name or subdomains using wildcard feature.
+   Wildcard support only allows single left most ``*``. This does not support regex.
+   When matching to a virtual host entry, domains with exact match have precedence
+   over wildcard. If a domain matches to multiple wildcard domains, the virtual host
+   config defined first has precedence.
+

Review Comment:
   The stated wildcard precedence doesn't match the implementation. VirtualHostConfig::find_by_domain() checks suffixes from most-specific to least-specific (e.g. "example.com" before "com"), not "first defined in virtualhost.yaml". Please update this text (or change the matching logic) so behavior is unambiguous.



##########
src/proxy/http/HttpSM.cc:
##########
@@ -4569,13 +4574,65 @@ HttpSM::check_sni_host()
   }
 }
 
+void
+HttpSM::set_virtualhost_entry(std::string_view domain)
+{
+  VirtualHost::scoped_config vhost_config;
+  // If already set, don't need to look at configs
+  if (m_virtualhost_entry || domain.empty() || !vhost_config) {
+    return;
+  }
+
+  auto vhost_entry = vhost_config->find_by_domain(domain);
+  if (vhost_entry) {
+    SMDbg(dbg_ctl_url_rewrite, "Found virtualhost: %s", vhost_entry->get_id().c_str());
+    // Explicitly acquire() since HttpSM holds raw pointer
+    m_virtualhost_entry = vhost_entry->acquire();
+  }
+}
+
 void
 HttpSM::do_remap_request(bool run_inline)
 {
   SMDbg(dbg_ctl_http_seq, "Remapping request");
   SMDbg(dbg_ctl_url_rewrite, "Starting a possible remapping for request");
+
+  if (!m_virtualhost_entry) {
+    auto host_name{t_state.hdr_info.client_request.host_get()};
+    set_virtualhost_entry(host_name);
+  }
+
+  // Check virtualhost remap rules before looking at remap.config
+  bool virtualhost_remap = false;
+  if (m_virtualhost_entry && m_virtualhost_entry->remap_table) {

Review Comment:
   This change adds new runtime behavior (virtualhost selection + per-virtualhost remap table override + fallback to global remap rules). There are extensive gold tests for remap.yaml behavior, but there doesn't appear to be a gold test covering virtualhost.yaml domain matching (exact vs wildcard), remap precedence, and fallback behavior. Adding a focused gold test would help prevent regressions.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,449 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <set>
+#include <string_view>
+#include <string>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+VirtualHostConfig::Entry *
+VirtualHostConfig::Entry::acquire() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self) {
+    self->refcount_inc();
+  }
+  return self;
+}
+
+void
+VirtualHostConfig::Entry::release() const
+{
+  auto *self = const_cast<Entry *>(this);
+  if (self && self->refcount_dec() == 0) {
+    self->free();
+  }
+}
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide `id`");
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must provide at least one domain in `domains` sequence");
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Dbg(dbg_ctl_virtualhost, "Virtual host entry can't have empty domain entry");
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        const char *subdomain = index(domain, '*');
+        if (subdomain && subdomain[1] == '.') {
+          item.wildcard_domains.push_back(subdomain + 2);
+        } else {
+          Dbg(dbg_ctl_virtualhost, "Virtual host wildcard entry must have '*.[domain]' format");
+        }
+      } else {
+        item.exact_domains.push_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Dbg(dbg_ctl_virtualhost, "Virtual host entry must have at least one domain defined");
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Dbg(dbg_ctl_virtualhost, "Failed to parse virtualhost entry");
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Dbg(dbg_ctl_virtualhost, "Failed to load remap rules for virtualhost entry");
+      return false;
+    }
+    conf.remap_table = make_ptr(table.release());
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    Warning("Virtualhost configuration '%s' doesn't exist", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Dbg(dbg_ctl_virtualhost, "Expected toplevel 'virtualhost' key to be a sequence");
+      return false;
+    }
+
+    for (auto const &node : config) {
+      Ptr<Entry> entry;
+      if (!build_virtualhost_entry(node, entry)) {
+        return false;
+      }
+
+      std::string vhost_id{entry->id};
+      if (_entries.contains(vhost_id)) {
+        Dbg(dbg_ctl_virtualhost, "Duplicate virtualhost id: %s", vhost_id.c_str());
+        return false;
+      }
+
+      for (auto const &domain : entry->exact_domains) {
+        if (_exact_domains_to_id.contains(domain)) {
+          Dbg(dbg_ctl_virtualhost, "Exact domain (%s) already in another virtualhost config", domain.c_str());
+          return false;
+        }
+        _exact_domains_to_id.emplace(domain, vhost_id);
+      }
+
+      for (auto const &domain_suffix : entry->wildcard_domains) {
+        if (_wildcard_domains_to_id.contains(domain_suffix)) {
+          Dbg(dbg_ctl_virtualhost, "Wildcard domain (%s) already in another virtualhost config", domain_suffix.c_str());
+          return false;
+        }
+        _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+      }
+
+      _entries.emplace(vhost_id, std::move(entry));
+    }
+
+  } catch (std::exception &ex) {
+    Dbg(dbg_ctl_virtualhost, "Failed to load %s: %s", config_path.c_str(), ex.what());
+    return false;
+  }
+  return true;
+}
+
+bool
+VirtualHostConfig::load_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  entry.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Dbg(dbg_ctl_virtualhost, "Expected toplevel 'virtualhost' key to be a sequence");
+      return false;
+    }
+
+    for (auto const &node : config) {
+      auto config_id = node["id"];
+      if (!config_id || config_id.as<std::string>() != id) {
+        continue;
+      }
+
+      Ptr<Entry> vhost_entry;
+      if (!build_virtualhost_entry(node, vhost_entry)) {
+        return false;
+      }
+      entry = std::move(vhost_entry);
+      return true;
+    }
+
+  } catch (std::exception &ex) {
+    Dbg(dbg_ctl_virtualhost, "Failed to load virtualhost entry (%s) in %s: %s", id.data(), config_path.c_str(), ex.what());
+    return false;
+  }
+  Dbg(dbg_ctl_virtualhost, "Virtualhost with id (%s) not found", id.data());
+  return false;
+}
+
+bool
+VirtualHostConfig::set_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  std::string vhost_id{id};
+  // If virtualhost entry already exists, remove current entry
+  if (auto it = _entries.find(vhost_id); it != _entries.end()) {
+    Ptr<Entry> curr_entry = std::move(it->second);
+    for (auto const &domain : curr_entry->exact_domains) {
+      _exact_domains_to_id.erase(domain);
+    }
+    for (auto const &domain : curr_entry->wildcard_domains) {
+      _wildcard_domains_to_id.erase(domain);
+    }
+    _entries.erase(vhost_id);
+  }
+
+  // Add new entry into virtualhost config
+  if (entry) {
+    for (auto const &domain : entry->exact_domains) {
+      if (_exact_domains_to_id.contains(domain)) {
+        Dbg(dbg_ctl_virtualhost, "Exact domain (%s) already in another virtualhost config", domain.c_str());
+        return false;
+      }
+      _exact_domains_to_id.emplace(domain, vhost_id);
+    }
+
+    for (auto const &domain_suffix : entry->wildcard_domains) {
+      if (_wildcard_domains_to_id.contains(domain_suffix)) {
+        Dbg(dbg_ctl_virtualhost, "Wildcard domain (%s) already in another virtualhost config", domain_suffix.c_str());
+        return false;
+      }
+      _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+    }
+
+    _entries.emplace(vhost_id, std::move(entry));
+  }
+  return true;
+}
+
+Ptr<VirtualHostConfig::Entry>
+VirtualHostConfig::find_by_id(std::string_view id) const
+{
+  if (_entries.empty()) {
+    return Ptr<VirtualHostConfig::Entry>();
+  }
+
+  auto entry = _entries.find(std::string{id});
+  if (entry != _entries.end()) {
+    return entry->second;
+  }
+  return Ptr<VirtualHostConfig::Entry>();
+}
+
+Ptr<VirtualHostConfig::Entry>
+VirtualHostConfig::find_by_domain(std::string_view domain) const
+{
+  if (_entries.empty() || domain.empty()) {
+    return Ptr<VirtualHostConfig::Entry>();
+  }
+
+  char lower_domain[TS_MAX_HOST_NAME_LEN + 1];
+  ts::transform_lower(std::string{domain}, lower_domain);
+
+  // Check for exact match domains first
+  auto id = _exact_domains_to_id.find(lower_domain);
+  if (id != _exact_domains_to_id.end()) {
+    auto entry = _entries.find(id->second);
+    if (entry != _entries.end()) {
+      return entry->second;
+    }
+  }
+
+  // Check wildcard suffixes
+  const char *subdomain = index(lower_domain, '.');
+  while (subdomain) {
+    subdomain++;
+    if (auto suffix_id = _wildcard_domains_to_id.find(subdomain); suffix_id != _wildcard_domains_to_id.end()) {
+      auto entry = _entries.find(suffix_id->second);
+      if (entry != _entries.end()) {
+        return entry->second;
+      }
+    }
+    subdomain = index(subdomain, '.');
+  }
+
+  return Ptr<VirtualHostConfig::Entry>();
+}
+
+void
+VirtualHost::startup()
+{
+  if (!reconfigure()) {
+    Fatal("failed to load %s", ts::filename::VIRTUALHOST);
+  }
+  RecRegisterConfigUpdateCb("proxy.config.virtualhost.filename", &VirtualHost::config_callback, nullptr);
+
+  config::ConfigRegistry::Get_Instance().register_config("virtualhost",                       // registry key
+                                                         ts::filename::VIRTUALHOST,           // default filename
+                                                         "proxy.config.virtualhost.filename", // record holding the filename
+                                                         [](ConfigContext ctx) {
+                                                           ctx.in_progress();
+
+                                                           // Single-entry reload requested via -D virtualhost.id=<id>
+                                                           if (auto directives = ctx.reload_directives();
+                                                               directives && directives["id"]) {
+                                                             std::string id = directives["id"].as<std::string>();
+                                                             if (VirtualHost::reconfigure(id)) {
+                                                               ctx.complete("Reloaded virtualhost entry: " + id);
+                                                             } else {
+                                                               ctx.fail("Failed to reload virtualhost entry: " + id);
+                                                             }
+                                                             return;
+                                                           }

Review Comment:
   reload_directives() returns a YAML::Node by value. Using non-const operator[] (directives["id"]) will create the key even when it doesn't exist, causing this branch to run and then .as<std::string>() to throw. Make the node const (so operator[] doesn't insert) and validate the id node before converting.



##########
doc/admin-guide/files/virtualhost.yaml.en.rst:
##########
@@ -0,0 +1,228 @@
+
+.. 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.
+
+.. include:: ../../common.defs
+
+.. configfile:: virtualhost.yaml
+
+virtualhost.yaml
+****************
+
+The :file:`virtualhost.yaml` file defines configuration blocks that apply to a group of domains.
+Each virtual host entry defines a set of domains and the remap rules associated with those domains.
+Virtual host remap rules override global :file:`remap.yaml` rules but remain fully backward compatible
+with existing configurations. If absent, ATS behaves exactly as before.
+
+Currently, this file only supports :file:`remap.yaml` overrides. Future versions will expand virtual
+host support to additional configuration types (e.g. :file:`sni.yaml`, :file:`ssl_multicert.yaml`,
+:file:`parent.config`, etc)
+
+By default this is named :file:`virtualhost.yaml`. The filename can be changed by setting
+:ts:cv:`proxy.config.virtualhost.filename`.
+
+
+Configuration
+=============
+
+:file:`virtualhost.yaml` is YAML format with top level namespace **virtualhost** and a list of virtual host
+entries. Each virtual host entry must provide an **id** and at least one domain defined in **domains**.
+
+An example configuration looks like:
+
+.. code-block:: yaml
+
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+
+===================== ==========================================================
+Field Name            Description
+===================== ==========================================================
+``id``                 Virtual host identifier to perform specific operations on
+``domains``            List of domains to resolve a request to
+``remap``              List of remap rules as defined in remap.yaml
+===================== ==========================================================
+
+``domains``
+   Domains can be defined as request domain name or subdomains using wildcard feature.
+   Wildcard support only allows single left most ``*``. This does not support regex.
+   When matching to a virtual host entry, domains with exact match have precedence
+   over wildcard. If a domain matches to multiple wildcard domains, the virtual host
+   config defined first has precedence.
+
+   For example:
+      Supported:
+      - ``foo.example.com``
+      - ``*.example.com``
+      - ``*.com``
+
+      NOT Supported:
+      - ``foo[0-9]+.example.com`` (regex)
+      - ``bar.*.example.net`` (``*`` in the middle)
+      - ``*.bar.*.com`` (multiple ``*``)
+      - ``*.*.baz.com`` (multiple ``*``)
+      - ``baz*.example.net`` (partial wildcard)
+      - ``*baz.example.net`` (partial wildcard)
+      - ``b*z.example.net`` (partial wildcard)
+      - ``*`` (global)
+
+Evaluation Order
+----------------
+
+|TS| evaluates a request using deterministic precedence in the following order:
+
+1. Resolve to a single virtualhost
+   a. Check for an exact domain match. If any virtual host lists the request hostname explicitly, that virtual host is selected.
+   b. Check for a wildcard domain match. If any virtual host wildcard domains define a subdomain of the request hostname in the form ``*.[domain]``, that virtual host is selected.
+   c. If no matching virtual host exists, the request proceeds using global configuration (i.e :file:`remap.config`). Skip to step 3.
+2. Within selected virtual host config, use virtual host remap rules.
+   a. Follow existing :file:`remap.yaml` rules and matching orders. If a matching remap rule is found, that remap rule is selected.
+3. If neither virtual host nor remap rules match, ATS falls back to global :file:`remap.yaml` resolution.
+
+Only one virtual host entry may match a given request. If multiple entries could match, ATS uses the first matching
+entry defined in :file:`virtualhost.yaml`.
+
+
+Granular Reload
+===============
+
+|TS| now supports granular configuration reloads for individual virtual hosts defined in :file:`virtualhost.yaml`.
+In addition to reloading the entire |TS| configuration with :option:`traffic_ctl config reload`, users can
+selectively reload a single virtual host entry without affecting other virtual host entries.
+
+By only updating the necessary changes, this reduces configuration deployment time and improves visibility on the changes made.
+
+To reload for a specific virtual host, use new reload directive:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=<id>
+
+Where **<id>** is the virtual host ID defined in :file:`virtualhost.yaml`. Only the **<id>** virtual host
+configuration will be reloaded. This does not affect other virtual hosts or global configuration files.
+
+Example:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=foo
+    ✔ Reload scheduled [rpc-123456789]
+
+    Monitor : traffic_ctl config reload -t rpc-123456789 -m
+    Details : traffic_ctl config reload -t rpc-123456789 -s -l
+
+  $ traffic_ctl config reload -t rpc-123456789 -s -l
+    ✗ Token 'rpc-123456789' already in use
+    ✔ Reload [success] — rpc-123456789
+    Started : 2026 May 19 19:20:20.691
+    Finished: 2026 May 19 19:20:20.692
+    Duration: 1ms
+
+    ✔ 1 success  ◌ 0 in-progress  ✗ 0 failed  (1 total)
+
+    Tasks:
+    ✔ virtualhost ··································    1ms
+        [Note]  Reloaded virtualhost entry: foo
+
+
+Examples
+========
+
+.. code-block:: yaml
+
+   # virtualhost.yaml
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+   # remap.yaml
+   remap:
+     - type: map
+        from:
+          url: http://www.x.com
+        to:
+          url: http://other.example.com/

Review Comment:
   The remap.yaml example has incorrect YAML indentation ("from" is indented too far), so the snippet is not valid YAML as written.



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r4096746689


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,645 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    Each reload must hold this from the moment it reads the file until it publishes. Otherwise two
+    reloads that parse concurrently can publish in the opposite order to their reads, and the older
+    read wins: a full reload resurrects entries a newer one had dropped, and a single-entry reload
+    parsed before a full reload that removed its id re-adds that entry on top of it. The single-entry
+    reload additionally does a read-copy-modify-publish against the live config, which the same
+    critical section keeps atomic. It also keeps the remap plugin reload notifications of one rebuild
+    from interleaving with another's. Reload paths are scheduled on ET_TASK with unrelated mutexes
+    (a fresh one per @c ConfigRegistry::schedule_reload and one per trigger record), so nothing else
+    serializes them. Neither path is on the request path, so serializing the parse costs nothing.
+ */
+std::mutex vhost_reconfigure_mutex;
+} // namespace
+
+std::atomic<int> VirtualHost::_configid{0};
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+namespace
+{
+const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+/** Check that @a name is composed of non-empty hostname labels.
+
+    Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as
+    an exact domain that no Host header can ever equal: the entry loads clean and never fires.
+ */
+bool
+is_hostname(std::string_view name)
+{
+  // A bracketed IPv6 literal is matched verbatim against the Host header.
+  if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+    return true;
+  }
+
+  size_t label_len = 0;
+
+  for (char c : name) {
+    if (c == '.') {
+      if (label_len == 0) {
+        return false;
+      }
+      label_len = 0;
+      continue;
+    }
+    if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') {
+      return false;
+    }
+    ++label_len;
+  }
+  return label_len > 0;
+}
+
+/** Decode a single `virtualhost` sequence element.
+
+    Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload
+    task log, not just in diags.log.
+ */
+bool
+decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx)
+{
+  if (!node["id"]) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+    return false;
+  }
+  item.id = node["id"].as<std::string>();
+  if (item.id.empty()) {
+    // A single-entry reload cannot name an empty id, so such an entry could never be reloaded on its own.
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide a non-empty `id`", node.Mark().line + 1);
+    return false;
+  }
+
+  for (const auto &elem : node) {
+    auto key = elem.first.as<std::string>();
+    if (!valid_vhost_keys.contains(key)) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has unsupported key '%s' (line %d)", item.id.c_str(), key.c_str(),
+                 elem.first.Mark().line + 1);
+      return false;
+    }
+  }
+
+  auto domains = node["domains"];
+  if (!domains || !domains.IsSequence() || domains.size() == 0) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)",
+               item.id.c_str(), node.Mark().line + 1);
+    return false;
+  }
+  item.exact_domains.clear();
+  item.wildcard_domains.clear();
+
+  for (const auto &it : domains) {
+    auto domain_entry = it.as<std::string>();
+    if (domain_entry.empty()) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+      return false;
+    }
+    char domain[TS_MAX_HOST_NAME_LEN + 1];
+    ts::transform_lower(domain_entry, domain);
+
+    // Check if domain is wildcard, prefixed with *
+    if (domain[0] == '*') {
+      if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(),
+                   domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain + 2)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' suffix is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.wildcard_domains.emplace_back(domain + 2);
+    } else {
+      if (strchr(domain, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' may only use a wildcard in the leading '*.[domain]' form (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.exact_domains.emplace_back(domain);
+    }
+  }
+
+  if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(),
+               node.Mark().line + 1);
+    return false;
+  }
+
+  return true;
+}
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry, ConfigContext ctx,
+                        VirtualHostPluginReload *plugin_reload)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!decode_virtualhost_entry(node, conf, ctx)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    CfgLoadLog(ctx, DL_Error, "Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    if (plugin_reload) {
+      plugin_reload->begin();
+    }
+    auto table = std::make_unique<UrlRewrite>();
+    table->set_remap_yaml(true);
+    if (!table->load_table({}, &remap_node, ctx)) {
+      CfgLoadLog(ctx, DL_Error, "Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(),
+                 remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+} // namespace
+
+namespace
+{
+/// Number of entries in the currently published config, so a refused reload can say what it would
+/// otherwise have dropped.
+size_t
+live_entry_count()
+{
+  VirtualHost::scoped_config config;
+
+  return config ? config->entry_count() : 0;
+}
+} // namespace
+
+bool
+VirtualHostConfig::load(ConfigContext ctx, bool initial_load, VirtualHostPluginReload *plugin_reload)
+{
+  _entries.clear();
+  _exact_domains_to_id.clear();
+  _wildcard_domains_to_id.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    if (!initial_load) {
+      CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' doesn't exist; keeping the %zu live entry(s)",
+                 config_path.c_str(), live_entry_count());
+      return false;
+    }
+    CfgLoadLog(ctx, DL_Warning, "Virtualhost configuration '%s' doesn't exist, no virtualhost entries loaded", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      if (!initial_load) {
+        CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' is empty; keeping the %zu live entry(s)",
+                   config_path.c_str(), live_entry_count());
+        return false;
+      }
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      CfgLoadLog(ctx, DL_Error, "%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());

Review Comment:
   The reload guard only treats an entirely null YAML document as empty. A valid `virtualhost: []` document reaches the loop and returns success, so a full reload publishes an empty `VirtualHostConfig` and drops every currently live entry, contrary to the documented reload safety guarantee that an empty config leaves the previous routing table in place. Reject an empty top-level sequence on reload (while retaining the startup behavior) before publishing it.



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "serrislew (via GitHub)" <gi...@apache.org>.
serrislew commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r3848158847


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,468 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cerrno>
+#include <memory>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Error("Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Error("Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)", item.id.c_str(),
+            node.Mark().line + 1);
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Error("Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+          Error("Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(), domain,
+                it.Mark().line + 1);
+          return false;
+        }
+        item.wildcard_domains.emplace_back(domain + 2);
+      } else {
+        item.exact_domains.emplace_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Error("Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(), node.Mark().line + 1);
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Error("Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Error("Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(), remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    Warning("Virtualhost configuration '%s' doesn't exist", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Error("%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      Ptr<Entry> entry;
+      if (!build_virtualhost_entry(node, entry)) {
+        return false;
+      }
+
+      std::string vhost_id{entry->id};
+      if (_entries.contains(vhost_id)) {
+        Error("%s: duplicate virtualhost id '%s' (line %d)", config_path.c_str(), vhost_id.c_str(), node.Mark().line + 1);
+        return false;
+      }
+
+      for (auto const &domain : entry->exact_domains) {
+        if (_exact_domains_to_id.contains(domain)) {
+          Error("%s: domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'", config_path.c_str(), domain.c_str(),
+                vhost_id.c_str(), _exact_domains_to_id.at(domain).c_str());
+          return false;
+        }
+        _exact_domains_to_id.emplace(domain, vhost_id);
+      }
+
+      for (auto const &domain_suffix : entry->wildcard_domains) {
+        if (_wildcard_domains_to_id.contains(domain_suffix)) {
+          Error("%s: wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'", config_path.c_str(),
+                domain_suffix.c_str(), vhost_id.c_str(), _wildcard_domains_to_id.at(domain_suffix).c_str());
+          return false;
+        }
+        _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+      }
+
+      _entries.emplace(vhost_id, std::move(entry));
+    }
+
+  } catch (std::exception &ex) {
+    Error("Failed to load %s: %s", config_path.c_str(), ex.what());
+    return false;
+  }
+  return true;
+}
+
+bool
+VirtualHostConfig::load_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  entry.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return false;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Error("%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      auto config_id = node["id"];
+      if (!config_id || config_id.as<std::string>() != id) {
+        continue;
+      }
+
+      Ptr<Entry> vhost_entry;
+      if (!build_virtualhost_entry(node, vhost_entry)) {
+        return false;
+      }
+      entry = std::move(vhost_entry);
+      return true;
+    }
+
+  } catch (std::exception &ex) {
+    Error("Failed to load virtualhost entry '%.*s' in %s: %s", static_cast<int>(id.size()), id.data(), config_path.c_str(),
+          ex.what());
+    return false;
+  }
+  Error("%s: virtualhost with id '%.*s' not found", config_path.c_str(), static_cast<int>(id.size()), id.data());
+  return false;
+}
+
+bool
+VirtualHostConfig::set_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  std::string vhost_id{id};
+  // If virtualhost entry already exists, remove current entry
+  if (auto it = _entries.find(vhost_id); it != _entries.end()) {
+    Ptr<Entry> curr_entry = std::move(it->second);
+    for (auto const &domain : curr_entry->exact_domains) {
+      _exact_domains_to_id.erase(domain);
+    }
+    for (auto const &domain : curr_entry->wildcard_domains) {
+      _wildcard_domains_to_id.erase(domain);
+    }
+    _entries.erase(vhost_id);
+  }
+
+  // Add new entry into virtualhost config
+  if (entry) {
+    for (auto const &domain : entry->exact_domains) {
+      if (_exact_domains_to_id.contains(domain)) {
+        Error("Domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'", domain.c_str(), vhost_id.c_str(),
+              _exact_domains_to_id.at(domain).c_str());
+        return false;
+      }
+      _exact_domains_to_id.emplace(domain, vhost_id);
+    }
+
+    for (auto const &domain_suffix : entry->wildcard_domains) {
+      if (_wildcard_domains_to_id.contains(domain_suffix)) {
+        Error("Wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'", domain_suffix.c_str(),
+              vhost_id.c_str(), _wildcard_domains_to_id.at(domain_suffix).c_str());
+        return false;
+      }

Review Comment:
   But the only caller is `VirtualHost::reconfigure(std::string_view)`, which mutates a copy and on `set_entry()` returning false returns 0 at line 443 without `configProcessor.set()`. The copy is destroyed; the live config never saw the removal. No config drop.



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "serrislew (via GitHub)" <gi...@apache.org>.
serrislew commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r3848144446


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,468 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cerrno>
+#include <memory>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Error("Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Error("Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)", item.id.c_str(),
+            node.Mark().line + 1);
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Error("Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+          Error("Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(), domain,
+                it.Mark().line + 1);
+          return false;
+        }
+        item.wildcard_domains.emplace_back(domain + 2);
+      } else {
+        item.exact_domains.emplace_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Error("Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(), node.Mark().line + 1);
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Error("Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Error("Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(), remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();

Review Comment:
   `load()` is only called from `VirtualHost::reconfigure() (VirtualHost.cc:406-408) — make_unique<VirtualHostConfig>()` which creates a new object, so it can't carry stale entries



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "serrislew (via GitHub)" <gi...@apache.org>.
serrislew merged PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108


-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "serrislew (via GitHub)" <gi...@apache.org>.
serrislew commented on PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#issuecomment-4491732822

   [approve ci autest 3]


-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r3815361607


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,468 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cerrno>
+#include <memory>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Error("Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Error("Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)", item.id.c_str(),
+            node.Mark().line + 1);
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Error("Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+          Error("Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(), domain,
+                it.Mark().line + 1);
+          return false;
+        }
+        item.wildcard_domains.emplace_back(domain + 2);
+      } else {
+        item.exact_domains.emplace_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Error("Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(), node.Mark().line + 1);
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Error("Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Error("Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(), remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    Warning("Virtualhost configuration '%s' doesn't exist", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Error("%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      Ptr<Entry> entry;
+      if (!build_virtualhost_entry(node, entry)) {
+        return false;
+      }
+
+      std::string vhost_id{entry->id};
+      if (_entries.contains(vhost_id)) {
+        Error("%s: duplicate virtualhost id '%s' (line %d)", config_path.c_str(), vhost_id.c_str(), node.Mark().line + 1);
+        return false;
+      }
+
+      for (auto const &domain : entry->exact_domains) {
+        if (_exact_domains_to_id.contains(domain)) {
+          Error("%s: domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'", config_path.c_str(), domain.c_str(),
+                vhost_id.c_str(), _exact_domains_to_id.at(domain).c_str());
+          return false;
+        }
+        _exact_domains_to_id.emplace(domain, vhost_id);
+      }
+
+      for (auto const &domain_suffix : entry->wildcard_domains) {
+        if (_wildcard_domains_to_id.contains(domain_suffix)) {
+          Error("%s: wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'", config_path.c_str(),
+                domain_suffix.c_str(), vhost_id.c_str(), _wildcard_domains_to_id.at(domain_suffix).c_str());
+          return false;
+        }
+        _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+      }
+
+      _entries.emplace(vhost_id, std::move(entry));
+    }
+
+  } catch (std::exception &ex) {
+    Error("Failed to load %s: %s", config_path.c_str(), ex.what());
+    return false;
+  }
+  return true;
+}
+
+bool
+VirtualHostConfig::load_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  entry.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return false;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      Error("%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      auto config_id = node["id"];
+      if (!config_id || config_id.as<std::string>() != id) {
+        continue;
+      }
+
+      Ptr<Entry> vhost_entry;
+      if (!build_virtualhost_entry(node, vhost_entry)) {
+        return false;
+      }
+      entry = std::move(vhost_entry);
+      return true;
+    }
+
+  } catch (std::exception &ex) {
+    Error("Failed to load virtualhost entry '%.*s' in %s: %s", static_cast<int>(id.size()), id.data(), config_path.c_str(),
+          ex.what());
+    return false;
+  }
+  Error("%s: virtualhost with id '%.*s' not found", config_path.c_str(), static_cast<int>(id.size()), id.data());
+  return false;
+}
+
+bool
+VirtualHostConfig::set_entry(std::string_view id, Ptr<Entry> &entry)
+{
+  std::string vhost_id{id};
+  // If virtualhost entry already exists, remove current entry
+  if (auto it = _entries.find(vhost_id); it != _entries.end()) {
+    Ptr<Entry> curr_entry = std::move(it->second);
+    for (auto const &domain : curr_entry->exact_domains) {
+      _exact_domains_to_id.erase(domain);
+    }
+    for (auto const &domain : curr_entry->wildcard_domains) {
+      _wildcard_domains_to_id.erase(domain);
+    }
+    _entries.erase(vhost_id);
+  }
+
+  // Add new entry into virtualhost config
+  if (entry) {
+    for (auto const &domain : entry->exact_domains) {
+      if (_exact_domains_to_id.contains(domain)) {
+        Error("Domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'", domain.c_str(), vhost_id.c_str(),
+              _exact_domains_to_id.at(domain).c_str());
+        return false;
+      }
+      _exact_domains_to_id.emplace(domain, vhost_id);
+    }
+
+    for (auto const &domain_suffix : entry->wildcard_domains) {
+      if (_wildcard_domains_to_id.contains(domain_suffix)) {
+        Error("Wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'", domain_suffix.c_str(),
+              vhost_id.c_str(), _wildcard_domains_to_id.at(domain_suffix).c_str());
+        return false;
+      }

Review Comment:
   `set_entry()` deletes the existing entry (and erases its domain claims) before validating that the replacement can be installed. If the new entry conflicts and returns `false`, the old entry is already removed, causing an unintended config drop. Consider validating conflicts first (or performing a two-phase update with rollback) so a failed single-entry reload leaves the previous virtualhost entry intact.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,468 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cerrno>
+#include <memory>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+}
+
+int VirtualHost::_configid = 0;
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+template <> struct YAML::convert<VirtualHostConfig::Entry> {
+  static bool
+  decode(const YAML::Node &node, VirtualHostConfig::Entry &item)
+  {
+    for (const auto &elem : node) {
+      if (std::none_of(valid_vhost_keys.begin(), valid_vhost_keys.end(),
+                       [&elem](const std::string &s) { return s == elem.first.as<std::string>(); })) {
+        Warning("unsupported key '%s' in VirtualHost config", elem.first.as<std::string>().c_str());
+      }
+    }
+
+    if (!node["id"]) {
+      Error("Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+      return false;
+    }
+    item.id = node["id"].as<std::string>();
+
+    auto domains = node["domains"];
+    if (!domains || !domains.IsSequence() || domains.size() == 0) {
+      Error("Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)", item.id.c_str(),
+            node.Mark().line + 1);
+      return false;
+    }
+    item.exact_domains.clear();
+    item.wildcard_domains.clear();
+
+    for (const auto &it : domains) {
+      auto domain_entry = it.as<std::string>();
+      if (domain_entry.empty()) {
+        Error("Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+        return false;
+      }
+      char domain[TS_MAX_HOST_NAME_LEN + 1];
+      ts::transform_lower(domain_entry, domain);
+
+      // Check if domain is wildcard, prefixed with *
+      if (domain[0] == '*') {
+        if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+          Error("Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(), domain,
+                it.Mark().line + 1);
+          return false;
+        }
+        item.wildcard_domains.emplace_back(domain + 2);
+      } else {
+        item.exact_domains.emplace_back(domain);
+      }
+    }
+
+    if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+      Error("Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(), node.Mark().line + 1);
+      return false;
+    }
+
+    return true;
+  }
+};
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!YAML::convert<VirtualHostConfig::Entry>::decode(node, conf)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    Error("Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    if (!table->load_table(conf.id, &remap_node)) {
+      Error("Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(), remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+
+bool
+VirtualHostConfig::load()
+{
+  _entries.clear();

Review Comment:
   `VirtualHostConfig::load()` clears `_entries` but does not clear `_exact_domains_to_id` / `_wildcard_domains_to_id`. This will leave stale domain→id mappings across reloads and can cause false 'already claimed' errors or incorrect routing after reload. Clear both maps at the start of `load()` (alongside `_entries.clear()`).



##########
src/proxy/http/remap/UrlRewrite.cc:
##########
@@ -95,6 +95,15 @@ UrlRewrite::load(ConfigContext ctx)
       return false;
     }
   }
+  return load_table(std::string(config_file_path.get()), nullptr, ctx);
+}
+
+bool
+UrlRewrite::load_table(const std::string &config_file_path, YAML::Node const *remap_node, ConfigContext ctx)
+{
+  if (remap_node) {
+    this->_remap_yaml = true;
+  }

Review Comment:
   `load_table()` sets `_remap_yaml` to `true` when `remap_node` is provided, but does not reset it when `remap_node` is null. If the same `UrlRewrite` instance is reused across loads, this can cause `BuildTable()` to parse a non-YAML remap file as YAML (or otherwise carry stale state). Set `_remap_yaml` deterministically for each call (e.g., `_remap_yaml = (remap_node != nullptr)` and/or derive from the file extension when `remap_node == nullptr`).



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r4085069213


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,586 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    The single-entry reload is a read-copy-modify-publish against the live config, so its read and
+    its publish must be atomic with respect to any other reload. The full reload has the same
+    requirement for a different reason: two full reloads that parse concurrently can publish in the
+    opposite order to their reads, so the older read wins and resurrects entries the newer one had
+    dropped. Both therefore hold this for the parse as well as the publish. Reload paths are
+    scheduled on ET_TASK with unrelated mutexes (a fresh one per @c ConfigRegistry::schedule_reload
+    and one per trigger record), so nothing else serializes them. Neither path is on the request
+    path, so serializing the parse costs nothing.
+ */
+std::mutex vhost_reconfigure_mutex;
+} // namespace
+
+std::atomic<int> VirtualHost::_configid{0};
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+namespace
+{
+const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+/** Check that @a name is composed of non-empty hostname labels.
+
+    Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as
+    an exact domain that no Host header can ever equal: the entry loads clean and never fires.
+ */
+bool
+is_hostname(std::string_view name)
+{
+  // A bracketed IPv6 literal is matched verbatim against the Host header.
+  if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+    return true;
+  }
+
+  size_t label_len = 0;
+
+  for (char c : name) {
+    if (c == '.') {
+      if (label_len == 0) {
+        return false;
+      }
+      label_len = 0;
+      continue;
+    }
+    if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') {
+      return false;
+    }
+    ++label_len;
+  }
+  return label_len > 0;
+}
+
+/** Decode a single `virtualhost` sequence element.
+
+    Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload
+    task log, not just in diags.log.
+ */
+bool
+decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx)
+{
+  if (!node["id"]) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+    return false;
+  }
+  item.id = node["id"].as<std::string>();
+
+  for (const auto &elem : node) {
+    auto key = elem.first.as<std::string>();
+    if (!valid_vhost_keys.contains(key)) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has unsupported key '%s' (line %d)", item.id.c_str(), key.c_str(),
+                 elem.first.Mark().line + 1);
+      return false;
+    }
+  }
+
+  auto domains = node["domains"];
+  if (!domains || !domains.IsSequence() || domains.size() == 0) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)",
+               item.id.c_str(), node.Mark().line + 1);
+    return false;
+  }
+  item.exact_domains.clear();
+  item.wildcard_domains.clear();
+
+  for (const auto &it : domains) {
+    auto domain_entry = it.as<std::string>();
+    if (domain_entry.empty()) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+      return false;
+    }
+    char domain[TS_MAX_HOST_NAME_LEN + 1];
+    ts::transform_lower(domain_entry, domain);
+
+    // Check if domain is wildcard, prefixed with *
+    if (domain[0] == '*') {
+      if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(),
+                   domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain + 2)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' suffix is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.wildcard_domains.emplace_back(domain + 2);
+    } else {
+      if (strchr(domain, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' may only use a wildcard in the leading '*.[domain]' form (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.exact_domains.emplace_back(domain);
+    }
+  }
+
+  if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(),
+               node.Mark().line + 1);
+    return false;
+  }
+
+  return true;
+}
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry, ConfigContext ctx)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!decode_virtualhost_entry(node, conf, ctx)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    CfgLoadLog(ctx, DL_Error, "Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    auto table = std::make_unique<UrlRewrite>();
+    table->set_remap_yaml(true);
+    if (!table->load_table(conf.id, &remap_node, ctx)) {
+      CfgLoadLog(ctx, DL_Error, "Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(),
+                 remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }

Review Comment:
   `UrlRewrite::load_table` is passed `conf.id` as the `config_file_path`. This becomes the effective 'path' for YAML remap parsing, which can break features that rely on the config file location (e.g., `include` directives resolving relative paths) and can also produce misleading diagnostics that report an id as a filename. Prefer passing the actual on-disk `virtualhost.yaml` path (or at least its directory) as the base path, and keep `id` only as a display label (e.g., add a separate `display_name` parameter or extend `ConfigContext` logs to include the vhost id).



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,586 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    The single-entry reload is a read-copy-modify-publish against the live config, so its read and
+    its publish must be atomic with respect to any other reload. The full reload has the same
+    requirement for a different reason: two full reloads that parse concurrently can publish in the
+    opposite order to their reads, so the older read wins and resurrects entries the newer one had
+    dropped. Both therefore hold this for the parse as well as the publish. Reload paths are
+    scheduled on ET_TASK with unrelated mutexes (a fresh one per @c ConfigRegistry::schedule_reload
+    and one per trigger record), so nothing else serializes them. Neither path is on the request
+    path, so serializing the parse costs nothing.

Review Comment:
   The mutex comment states that *both* reload paths hold the mutex for the parse and publish, but `VirtualHost::reconfigure(std::string_view id, ...)` currently parses the entry (`load_entry`) outside the lock. This mismatch makes the concurrency reasoning harder to trust/maintain. Either update the comment to reflect the current design (parse outside lock for single-entry reload; lock only for read-copy-modify-publish), or move the single-entry parse under the mutex if the intention is truly to serialize parsing too.



##########
doc/admin-guide/files/virtualhost.yaml.en.rst:
##########
@@ -0,0 +1,236 @@
+
+.. 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.
+
+.. include:: ../../common.defs
+
+.. configfile:: virtualhost.yaml
+
+virtualhost.yaml
+****************
+
+The :file:`virtualhost.yaml` file defines configuration blocks that apply to a group of domains.
+Each virtual host entry defines a set of domains and the remap rules associated with those domains.
+Virtual host remap rules override global :file:`remap.yaml` rules but remain fully backward compatible
+with existing configurations. If absent, ATS behaves exactly as before.
+
+Currently, this file only supports :file:`remap.yaml` overrides. Future versions will expand virtual
+host support to additional configuration types (e.g. :file:`sni.yaml`, :file:`ssl_multicert.yaml`,
+:file:`parent.config`, etc)
+
+By default this is named :file:`virtualhost.yaml`. The filename can be changed by setting
+:ts:cv:`proxy.config.virtualhost.filename`.
+
+
+Configuration
+=============
+
+:file:`virtualhost.yaml` is YAML format with top level namespace **virtualhost** and a list of virtual host
+entries. Each virtual host entry must provide an **id** and at least one domain defined in **domains**.
+
+An example configuration looks like:
+
+.. code-block:: yaml
+
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+
+===================== ==========================================================
+Field Name            Description
+===================== ==========================================================
+``id``                 Virtual host identifier to perform specific operations on
+``domains``            List of domains to resolve a request to
+``remap``              List of remap rules as defined in remap.yaml
+===================== ==========================================================
+
+``domains``
+   Domains can be defined as request domain name or subdomains using wildcard feature.
+   Wildcard support only allows a single left most ``*``. This does not support regex.
+   When matching to a virtual host entry, domains with exact match have precedence
+   over wildcard. If a domain matches to multiple wildcard domains, the most specific
+   (longest) suffix match is selected.
+
+   For example:
+      Supported:
+      - ``foo.example.com``
+      - ``*.example.com``
+      - ``*.com``
+
+      NOT Supported:
+      - ``foo[0-9]+.example.com`` (regex)
+      - ``bar.*.example.net`` (``*`` in the middle)
+      - ``*.bar.*.com`` (multiple ``*``)
+      - ``*.*.baz.com`` (multiple ``*``)
+      - ``baz*.example.net`` (partial wildcard)
+      - ``*baz.example.net`` (partial wildcard)
+      - ``b*z.example.net`` (partial wildcard)
+      - ``*`` (global)
+
+Evaluation Order
+----------------
+
+|TS| evaluates a request using deterministic precedence in the following order:
+
+1. Resolve to a single virtualhost
+   a. Check for an exact domain match. If any virtual host lists the request hostname explicitly, that virtual host is selected.
+   b. Check for a wildcard domain match. If any virtual host wildcard domains define a subdomain of the request hostname in the form ``*.[domain]``, that virtual host is selected.
+   c. If no matching virtual host exists, the request proceeds using global configuration (i.e :file:`remap.config`). Skip to step 3.
+2. Within selected virtual host config, use virtual host remap rules.
+   a. Follow existing :file:`remap.yaml` rules and matching orders. If a matching remap rule is found, that remap rule is selected.
+3. If neither virtual host nor remap rules match, ATS falls back to global :file:`remap.yaml` resolution.
+
+Only one virtual host entry may match a given request. Exact domain matches take precedence over wildcard matches. For wildcard matches,
+ATS selects the most specific (longest) matching suffix (e.g. ``*.example.com`` before ``*.com``).
+
+
+Granular Reload
+===============
+
+|TS| now supports granular configuration reloads for individual virtual hosts defined in :file:`virtualhost.yaml`.
+In addition to reloading the entire |TS| configuration with :option:`traffic_ctl config reload`, users can
+selectively reload a single virtual host entry without affecting other virtual host entries.
+
+By only updating the necessary changes, this reduces configuration deployment time and improves visibility on the changes made.
+
+To reload for a specific virtual host, use new reload directive:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=<id>
+
+Where **<id>** is the virtual host ID defined in :file:`virtualhost.yaml`. Only the **<id>** virtual host
+configuration will be reloaded. This does not affect other virtual hosts or global configuration files.
+
+Example:
+
+::
+
+   $ traffic_ctl config reload -D virtualhost.id=foo
+    ✔ Reload scheduled [rpc-123456789]
+
+    Monitor : traffic_ctl config reload -t rpc-123456789 -m
+    Details : traffic_ctl config reload -t rpc-123456789 -s -l
+
+  $ traffic_ctl config reload -t rpc-123456789 -s -l
+    ✗ Token 'rpc-123456789' already in use
+    ✔ Reload [success] — rpc-123456789
+    Started : 2026 May 19 19:20:20.691
+    Finished: 2026 May 19 19:20:20.692
+    Duration: 1ms
+
+    ✔ 1 success  ◌ 0 in-progress  ✗ 0 failed  (1 total)
+
+    Tasks:
+    ✔ virtualhost ··································    1ms
+        [Note]  Reloaded virtualhost entry: foo
+
+The **<id>** must name an entry. An empty value is rejected rather than treated as a request to
+reload the whole file, so a reload scoped to one entry never rebuilds the entire table from disk.
+
+A reload, whether of a single entry or of the whole file, fails and leaves the running
+configuration in place if :file:`virtualhost.yaml` is missing, empty, or invalid. Only the initial
+load at startup treats a missing or empty file as "no virtual hosts configured"; on a reload,
+replacing a live routing table with an empty one is reported as a failure instead.
+
+
+Examples
+========
+
+.. code-block:: yaml
+
+   # virtualhost.yaml
+   virtualhost:
+     - id: example
+       domains:
+         - example.com
+
+       remap:
+         - type: map
+           from:
+            url: http://example.com
+           to:
+            url: http://origin.example.com/
+
+   # remap.yaml
+   remap:
+     - type: map
+       from:
+         url: http://www.x.com
+       to:
+         url: http://other.example.com/
+
+This rules translates in the following translation.

Review Comment:
   Fix grammar: change 'This rules translates' to 'These rules translate' (or similar) for correct subject/verb agreement.



##########
tests/gold_tests/remap/virtualhost_remap.test.py:
##########
@@ -0,0 +1,255 @@
+'''
+Verify that per-domain remap tables in virtualhost.yaml are applied to requests.
+'''
+#  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.
+
+Test.Summary = '''
+Verify virtualhost.yaml domain resolution and per-domain remap rules on the request path.
+'''
+
+import os
+
+Test.ContinueOnFail = True
+Test.testName = 'virtualhost_remap'
+
+ts = Test.MakeATSProcess("ts")
+
+# The origin is keyed on the request path only (the default lookup key), so each
+# remap rule can be given a private target path. Which rule won is then decided
+# by which body comes back, not merely by getting a 200.
+server = Test.MakeOriginServer("server")
+
+
+def add_origin_response(path: str, body: str) -> None:
+    """Register an origin response for `path` carrying a rule-specific body."""
+    request_header = {
+        "headers": f"GET {path} HTTP/1.1\r\nHost: origin.example.com\r\n\r\n",
+        "timestamp": "1469733493.993",
+        "body": ""
+    }
+    response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": body}
+    server.addResponse("sessionfile.log", request_header, response_header)
+
+
+# One target path per remap rule that could plausibly fire. The bodies are chosen
+# so that no expected body is a substring of another.
+add_origin_response("/vhost-exact-domain/", "hit:vhost-exact-domain")
+add_origin_response("/vhost-deep-wildcard/", "hit:vhost-deep-wildcard")
+add_origin_response("/vhost-wide-wildcard/", "hit:vhost-wide-wildcard")
+add_origin_response("/vhost-wildcard-precedence/", "hit:vhost-wildcard-precedence")
+add_origin_response("/vhost-exact-precedence/", "hit:vhost-exact-precedence")
+add_origin_response("/vhost-fallback-rule/", "hit:vhost-fallback-rule")
+add_origin_response("/global-fallback/other/", "hit:global-fallback")
+add_origin_response("/global-plain/", "hit:global-plain")
+# Only reachable if the rejected reload below is wrongly published.
+add_origin_response("/vhost-conflict/", "hit:vhost-conflict")
+
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'virtualhost|url_rewrite',
+})
+
+# The refused reload at the end of this test logs the conflict as an ERROR, which
+# replaces the default diags expectations.
+ts.Disk.diags_log.Content = Testers.ContainsExpression(
+    "is already claimed by virtualhost 'exact-only'", "The conflicting reload should name the virtualhost holding the domain")
+ts.Disk.diags_log.Content += Testers.ExcludesExpression("FATAL:", "A refused reload must not be fatal")
+
+origin = f'127.0.0.1:{server.Variables.Port}'
+
+# Global table, in remap.config (legacy) format. The virtualhost tables below are
+# always YAML, so this also covers the mixed case.
+ts.Disk.remap_config.AddLines(
+    [
+        f'map http://fallback.example.net/ http://{origin}/global-fallback/',
+        f'map http://none.example.net/ http://{origin}/global-plain/',
+    ])
+
+vhost_config_lines = [
+    'virtualhost:',
+    # Plain exact-domain match. No wildcard in this config matches .example.org.
+    '  - id: exact-only',
+    '    domains:',
+    '      - exact.example.org',
+    '    remap:',
+    '      - type: map',
+    '        from:',
+    '          url: http://exact.example.org/',
+    '        to:',
+    f'          url: http://{origin}/vhost-exact-domain/',
+    # x.deep.example.com matches both this wildcard and the wider one below.
+    # The longest (most specific) suffix must win.
+    '  - id: deep-wildcard',
+    '    domains:',
+    '      - "*.deep.example.com"',
+    '    remap:',
+    '      - type: map',
+    '        from:',
+    '          url: http://x.deep.example.com/',
+    '        to:',
+    f'          url: http://{origin}/vhost-deep-wildcard/',
+    '  - id: wide-wildcard',
+    '    domains:',
+    '      - "*.example.com"',
+    '    remap:',
+    '      - type: map',
+    '        from:',
+    '          url: http://x.deep.example.com/',
+    '        to:',
+    f'          url: http://{origin}/vhost-wide-wildcard/',
+    '      - type: map',
+    '        from:',
+    '          url: http://precedence.example.com/',
+    '        to:',
+    f'          url: http://{origin}/vhost-wildcard-precedence/',
+    # precedence.example.com is claimed exactly here and by the wildcard
+    # above. The exact domain must win.
+    '  - id: exact-precedence',
+    '    domains:',
+    '      - precedence.example.com',
+    '    remap:',
+    '      - type: map',
+    '        from:',
+    '          url: http://precedence.example.com/',
+    '        to:',
+    f'          url: http://{origin}/vhost-exact-precedence/',
+    # This virtualhost resolves for fallback.example.net but its only rule
+    # covers a different path, so requests elsewhere must fall back to the
+    # global table.
+    '  - id: path-miss',
+    '    domains:',
+    '      - fallback.example.net',
+    '    remap:',
+    '      - type: map',
+    '        from:',
+    '          url: http://fallback.example.net/only-here/',
+    '        to:',
+    f'          url: http://{origin}/vhost-fallback-rule/',
+]
+
+ts.Disk.virtualhost_yaml.AddLines(vhost_config_lines)
+
+
+def add_request(name: str, host: str, path: str, expected: str, not_expected: str = "") -> 'TestRun':
+    """Send one request through the proxy and assert which remap rule served it.
+
+    :param name: Test run name.
+    :param host: Host header, which selects the virtualhost.
+    :param path: Request path.
+    :param expected: Body of the rule that must have won.
+    :param not_expected: Body of the rule that must have lost, if any.
+    """
+    tr = Test.AddTestRun(name)
+    tr.MakeCurlCommand(f'-s -H"Host: {host}" http://127.0.0.1:{ts.Variables.port}{path} --verbose', ts=ts)
+    tr.Processes.Default.ReturnCode = 0
+    tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(expected, f"{host}{path} should be served by {expected}")
+    if not_expected:
+        tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression(
+            not_expected, f"{host}{path} must not be served by {not_expected}")
+    tr.StillRunningAfter = ts
+    tr.StillRunningAfter = server

Review Comment:
   These two assignments will overwrite each other in normal Python attribute semantics, which can cause the test runner to only enforce one of the two processes as 'still running'. If the harness supports multiple dependencies, set them in a single assignment (e.g., a list/tuple) or use the harness’ documented API for adding multiple `StillRunningAfter` processes.



-- 
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


Re: [PR] virtualhost.yaml with remap rules [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13108:
URL: https://github.com/apache/trafficserver/pull/13108#discussion_r4086805591


##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,637 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    Each reload must hold this from the moment it reads the file until it publishes. Otherwise two
+    reloads that parse concurrently can publish in the opposite order to their reads, and the older
+    read wins: a full reload resurrects entries a newer one had dropped, and a single-entry reload
+    parsed before a full reload that removed its id re-adds that entry on top of it. The single-entry
+    reload additionally does a read-copy-modify-publish against the live config, which the same
+    critical section keeps atomic. It also keeps the remap plugin reload notifications of one rebuild
+    from interleaving with another's. Reload paths are scheduled on ET_TASK with unrelated mutexes
+    (a fresh one per @c ConfigRegistry::schedule_reload and one per trigger record), so nothing else
+    serializes them. Neither path is on the request path, so serializing the parse costs nothing.
+ */
+std::mutex vhost_reconfigure_mutex;
+} // namespace
+
+std::atomic<int> VirtualHost::_configid{0};
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+namespace
+{
+const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+/** Check that @a name is composed of non-empty hostname labels.
+
+    Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as
+    an exact domain that no Host header can ever equal: the entry loads clean and never fires.
+ */
+bool
+is_hostname(std::string_view name)
+{
+  // A bracketed IPv6 literal is matched verbatim against the Host header.
+  if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+    return true;
+  }
+
+  size_t label_len = 0;
+
+  for (char c : name) {
+    if (c == '.') {
+      if (label_len == 0) {
+        return false;
+      }
+      label_len = 0;
+      continue;
+    }
+    if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') {
+      return false;
+    }
+    ++label_len;
+  }
+  return label_len > 0;
+}
+
+/** Decode a single `virtualhost` sequence element.
+
+    Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload
+    task log, not just in diags.log.
+ */
+bool
+decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx)
+{
+  if (!node["id"]) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+    return false;
+  }
+  item.id = node["id"].as<std::string>();
+  if (item.id.empty()) {
+    // A single-entry reload cannot name an empty id, so such an entry could never be reloaded on its own.
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide a non-empty `id`", node.Mark().line + 1);
+    return false;
+  }
+
+  for (const auto &elem : node) {
+    auto key = elem.first.as<std::string>();
+    if (!valid_vhost_keys.contains(key)) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has unsupported key '%s' (line %d)", item.id.c_str(), key.c_str(),
+                 elem.first.Mark().line + 1);
+      return false;
+    }
+  }
+
+  auto domains = node["domains"];
+  if (!domains || !domains.IsSequence() || domains.size() == 0) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)",
+               item.id.c_str(), node.Mark().line + 1);
+    return false;
+  }
+  item.exact_domains.clear();
+  item.wildcard_domains.clear();
+
+  for (const auto &it : domains) {
+    auto domain_entry = it.as<std::string>();
+    if (domain_entry.empty()) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+      return false;
+    }
+    char domain[TS_MAX_HOST_NAME_LEN + 1];
+    ts::transform_lower(domain_entry, domain);

Review Comment:
   `transform_lower` clips the source to `TS_MAX_HOST_NAME_LEN` before `is_hostname()` validates and stores it. An overlong configured domain is therefore silently truncated, so distinct domains sharing the prefix can collide or a request can be routed to the truncated entry. Reject overlong `domain_entry` values before transforming them.



##########
src/proxy/VirtualHost.cc:
##########
@@ -0,0 +1,637 @@
+/** @file
+
+  Virtual Host configuration implementation
+
+  @section license License
+
+  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.
+ */
+
+#include <algorithm>
+#include <cctype>
+#include <cerrno>
+#include <atomic>
+#include <memory>
+#include <mutex>
+#include <set>
+#include <string>
+#include <string_view>
+#include <strings.h>
+#include <sys/stat.h>
+#include <yaml-cpp/yaml.h>
+
+#include "proxy/VirtualHost.h"
+#include "proxy/ReverseProxy.h"
+#include "mgmt/config/ConfigContextDiags.h"
+#include "mgmt/config/ConfigRegistry.h"
+#include "records/RecCore.h"
+#include "tscore/Filenames.h"
+#include "tsutil/Convert.h"
+
+namespace
+{
+DbgCtl dbg_ctl_virtualhost("virtualhost");
+
+/** Serializes both reload paths against each other.
+
+    Each reload must hold this from the moment it reads the file until it publishes. Otherwise two
+    reloads that parse concurrently can publish in the opposite order to their reads, and the older
+    read wins: a full reload resurrects entries a newer one had dropped, and a single-entry reload
+    parsed before a full reload that removed its id re-adds that entry on top of it. The single-entry
+    reload additionally does a read-copy-modify-publish against the live config, which the same
+    critical section keeps atomic. It also keeps the remap plugin reload notifications of one rebuild
+    from interleaving with another's. Reload paths are scheduled on ET_TASK with unrelated mutexes
+    (a fresh one per @c ConfigRegistry::schedule_reload and one per trigger record), so nothing else
+    serializes them. Neither path is on the request path, so serializing the parse costs nothing.
+ */
+std::mutex vhost_reconfigure_mutex;
+} // namespace
+
+std::atomic<int> VirtualHost::_configid{0};
+
+std::string
+VirtualHostConfig::Entry::get_id() const
+{
+  return id;
+}
+
+namespace
+{
+const std::set<std::string> valid_vhost_keys = {"id", "domains", "remap"};
+
+/** Check that @a name is composed of non-empty hostname labels.
+
+    Without this, a documented-as-unsupported form such as `foo[0-9]+.example.com` is accepted as
+    an exact domain that no Host header can ever equal: the entry loads clean and never fires.
+ */
+bool
+is_hostname(std::string_view name)
+{
+  // A bracketed IPv6 literal is matched verbatim against the Host header.
+  if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+    return true;
+  }
+
+  size_t label_len = 0;
+
+  for (char c : name) {
+    if (c == '.') {
+      if (label_len == 0) {
+        return false;
+      }
+      label_len = 0;
+      continue;
+    }
+    if (!isalnum(static_cast<unsigned char>(c)) && c != '-' && c != '_') {
+      return false;
+    }
+    ++label_len;
+  }
+  return label_len > 0;
+}
+
+/** Decode a single `virtualhost` sequence element.
+
+    Diagnostics go through @a ctx so the operator who asked for the reload sees them in the reload
+    task log, not just in diags.log.
+ */
+bool
+decode_virtualhost_entry(YAML::Node const &node, VirtualHostConfig::Entry &item, ConfigContext ctx)
+{
+  if (!node["id"]) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide `id`", node.Mark().line + 1);
+    return false;
+  }
+  item.id = node["id"].as<std::string>();
+  if (item.id.empty()) {
+    // A single-entry reload cannot name an empty id, so such an entry could never be reloaded on its own.
+    CfgLoadLog(ctx, DL_Error, "Virtualhost entry at line %d must provide a non-empty `id`", node.Mark().line + 1);
+    return false;
+  }
+
+  for (const auto &elem : node) {
+    auto key = elem.first.as<std::string>();
+    if (!valid_vhost_keys.contains(key)) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has unsupported key '%s' (line %d)", item.id.c_str(), key.c_str(),
+                 elem.first.Mark().line + 1);
+      return false;
+    }
+  }
+
+  auto domains = node["domains"];
+  if (!domains || !domains.IsSequence() || domains.size() == 0) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must provide at least one domain in a `domains` sequence (line %d)",
+               item.id.c_str(), node.Mark().line + 1);
+    return false;
+  }
+  item.exact_domains.clear();
+  item.wildcard_domains.clear();
+
+  for (const auto &it : domains) {
+    auto domain_entry = it.as<std::string>();
+    if (domain_entry.empty()) {
+      CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' has an empty entry in `domains` (line %d)", item.id.c_str(), it.Mark().line + 1);
+      return false;
+    }
+    char domain[TS_MAX_HOST_NAME_LEN + 1];
+    ts::transform_lower(domain_entry, domain);
+
+    // Check if domain is wildcard, prefixed with *
+    if (domain[0] == '*') {
+      if (domain[1] != '.' || domain[2] == '\0' || domain[2] == '.' || strchr(domain + 2, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' must match '*.[domain]' format (line %d)", item.id.c_str(),
+                   domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain + 2)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' wildcard '%s' suffix is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.wildcard_domains.emplace_back(domain + 2);
+    } else {
+      if (strchr(domain, '*') != nullptr) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' may only use a wildcard in the leading '*.[domain]' form (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      if (!is_hostname(domain)) {
+        CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' domain '%s' is not a valid hostname; regex is not supported (line %d)",
+                   item.id.c_str(), domain, it.Mark().line + 1);
+        return false;
+      }
+      item.exact_domains.emplace_back(domain);
+    }
+  }
+
+  if (item.exact_domains.empty() && item.wildcard_domains.empty()) {
+    CfgLoadLog(ctx, DL_Error, "Virtualhost '%s' must have at least one domain defined (line %d)", item.id.c_str(),
+               node.Mark().line + 1);
+    return false;
+  }
+
+  return true;
+}
+
+bool
+build_virtualhost_entry(YAML::Node const &node, Ptr<VirtualHostConfig::Entry> &entry, ConfigContext ctx,
+                        VirtualHostPluginReload *plugin_reload)
+{
+  entry.clear();
+  Ptr<VirtualHostConfig::Entry> vhost = make_ptr(new VirtualHostConfig::Entry);
+  auto                         &conf  = *vhost;
+  try {
+    if (!decode_virtualhost_entry(node, conf, ctx)) {
+      return false;
+    }
+  } catch (YAML::Exception const &ex) {
+    CfgLoadLog(ctx, DL_Error, "Failed to parse virtualhost entry at line %d: %s", node.Mark().line + 1, ex.what());
+    return false;
+  }
+
+  // Build UrlRewrite table for remap rules
+  auto remap_node = node["remap"];
+  if (remap_node) {
+    if (plugin_reload) {
+      plugin_reload->begin();
+    }
+    auto table = std::make_unique<UrlRewrite>();
+    table->set_remap_yaml(true);
+    if (!table->load_table({}, &remap_node, ctx)) {
+      CfgLoadLog(ctx, DL_Error, "Failed to load remap rules for virtualhost '%s' at line %d", conf.id.c_str(),
+                 remap_node.Mark().line + 1);
+      return false;
+    }
+    conf.remap_table = make_managed_url_rewrite(std::move(table));
+  }
+  entry = std::move(vhost);
+  return true;
+}
+} // namespace
+
+namespace
+{
+/// Number of entries in the currently published config, so a refused reload can say what it would
+/// otherwise have dropped.
+size_t
+live_entry_count()
+{
+  VirtualHost::scoped_config config;
+
+  return config ? config->entry_count() : 0;
+}
+} // namespace
+
+bool
+VirtualHostConfig::load(ConfigContext ctx, bool initial_load, VirtualHostPluginReload *plugin_reload)
+{
+  _entries.clear();
+  _exact_domains_to_id.clear();
+  _wildcard_domains_to_id.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    if (!initial_load) {
+      CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' doesn't exist; keeping the %zu live entry(s)",
+                 config_path.c_str(), live_entry_count());
+      return false;
+    }
+    CfgLoadLog(ctx, DL_Warning, "Virtualhost configuration '%s' doesn't exist, no virtualhost entries loaded", config_path.c_str());
+    return true;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      if (!initial_load) {
+        CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost config: '%s' is empty; keeping the %zu live entry(s)",
+                   config_path.c_str(), live_entry_count());
+        return false;
+      }
+      Dbg(dbg_ctl_virtualhost, "Empty virtualhost config: %s", config_path.c_str());
+      return true;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      CfgLoadLog(ctx, DL_Error, "%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      Ptr<Entry> entry;
+      if (!build_virtualhost_entry(node, entry, ctx, plugin_reload)) {
+        return false;
+      }
+
+      std::string vhost_id{entry->id};
+      if (_entries.contains(vhost_id)) {
+        CfgLoadLog(ctx, DL_Error, "%s: duplicate virtualhost id '%s' (line %d)", config_path.c_str(), vhost_id.c_str(),
+                   node.Mark().line + 1);
+        return false;
+      }
+
+      for (auto const &domain : entry->exact_domains) {
+        if (_exact_domains_to_id.contains(domain)) {
+          CfgLoadLog(ctx, DL_Error, "%s: domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'",
+                     config_path.c_str(), domain.c_str(), vhost_id.c_str(), _exact_domains_to_id.at(domain).c_str());
+          return false;
+        }
+        _exact_domains_to_id.emplace(domain, vhost_id);
+      }
+
+      for (auto const &domain_suffix : entry->wildcard_domains) {
+        if (_wildcard_domains_to_id.contains(domain_suffix)) {
+          CfgLoadLog(ctx, DL_Error, "%s: wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'",
+                     config_path.c_str(), domain_suffix.c_str(), vhost_id.c_str(),
+                     _wildcard_domains_to_id.at(domain_suffix).c_str());
+          return false;
+        }
+        _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+      }
+
+      _entries.emplace(vhost_id, std::move(entry));
+    }
+
+  } catch (std::exception &ex) {
+    CfgLoadLog(ctx, DL_Error, "Failed to load %s: %s", config_path.c_str(), ex.what());
+    return false;
+  }
+  return true;
+}
+
+bool
+VirtualHostConfig::load_entry(std::string_view id, Ptr<Entry> &entry, ConfigContext ctx, VirtualHostPluginReload *plugin_reload)
+{
+  entry.clear();
+  std::string config_path = RecConfigReadConfigPath("proxy.config.virtualhost.filename", ts::filename::VIRTUALHOST);
+
+  struct stat sbuf;
+  if (stat(config_path.c_str(), &sbuf) == -1 && errno == ENOENT) {
+    CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost entry '%.*s': configuration '%s' doesn't exist",
+               static_cast<int>(id.size()), id.data(), config_path.c_str());
+    return false;
+  }
+
+  try {
+    YAML::Node config = YAML::LoadFile(config_path);
+    if (config.IsNull()) {
+      CfgLoadLog(ctx, DL_Error, "Cannot reload virtualhost entry '%.*s': '%s' is empty", static_cast<int>(id.size()), id.data(),
+                 config_path.c_str());
+      return false;
+    }
+
+    config = config["virtualhost"];
+    if (config.IsNull() || !config.IsSequence()) {
+      CfgLoadLog(ctx, DL_Error, "%s: expected toplevel 'virtualhost' key to be a sequence", config_path.c_str());
+      return false;
+    }
+
+    for (auto const &node : config) {
+      auto config_id = node["id"];
+      if (!config_id || config_id.as<std::string>() != id) {
+        continue;
+      }
+
+      Ptr<Entry> vhost_entry;
+      if (!build_virtualhost_entry(node, vhost_entry, ctx, plugin_reload)) {
+        return false;
+      }
+      entry = std::move(vhost_entry);
+      return true;
+    }
+
+  } catch (std::exception &ex) {
+    CfgLoadLog(ctx, DL_Error, "Failed to load virtualhost entry '%.*s' in %s: %s", static_cast<int>(id.size()), id.data(),
+               config_path.c_str(), ex.what());
+    return false;
+  }
+  CfgLoadLog(ctx, DL_Error, "%s: virtualhost with id '%.*s' not found", config_path.c_str(), static_cast<int>(id.size()),
+             id.data());
+  return false;
+}
+
+bool
+VirtualHostConfig::set_entry(std::string_view id, Ptr<Entry> &entry, ConfigContext ctx)
+{
+  std::string vhost_id{id};
+  // If virtualhost entry already exists, remove current entry
+  if (auto it = _entries.find(vhost_id); it != _entries.end()) {
+    Ptr<Entry> curr_entry = std::move(it->second);
+    for (auto const &domain : curr_entry->exact_domains) {
+      _exact_domains_to_id.erase(domain);
+    }
+    for (auto const &domain : curr_entry->wildcard_domains) {
+      _wildcard_domains_to_id.erase(domain);
+    }
+    _entries.erase(vhost_id);
+  }
+
+  // Add new entry into virtualhost config
+  if (entry) {
+    for (auto const &domain : entry->exact_domains) {
+      if (_exact_domains_to_id.contains(domain)) {
+        CfgLoadLog(ctx, DL_Error, "Domain '%s' in virtualhost '%s' is already claimed by virtualhost '%s'", domain.c_str(),
+                   vhost_id.c_str(), _exact_domains_to_id.at(domain).c_str());
+        return false;
+      }
+      _exact_domains_to_id.emplace(domain, vhost_id);
+    }
+
+    for (auto const &domain_suffix : entry->wildcard_domains) {
+      if (_wildcard_domains_to_id.contains(domain_suffix)) {
+        CfgLoadLog(ctx, DL_Error, "Wildcard domain '*.%s' in virtualhost '%s' is already claimed by virtualhost '%s'",
+                   domain_suffix.c_str(), vhost_id.c_str(), _wildcard_domains_to_id.at(domain_suffix).c_str());
+        return false;
+      }
+      _wildcard_domains_to_id.emplace(domain_suffix, vhost_id);
+    }
+
+    _entries.emplace(vhost_id, std::move(entry));
+  }
+  return true;
+}
+
+void
+VirtualHostConfig::collect_used_plugins(std::unordered_map<PluginDso *, int> &used) const
+{
+  for (auto const &[id, entry] : _entries) {
+    if (entry->remap_table) {
+      entry->remap_table->pluginFactory.collectUsedPlugins(used);
+    }
+  }
+}
+
+VirtualHostPluginReload::~VirtualHostPluginReload()
+{
+  if (_started) {
+    PluginDso::loadedPlugins()->indicatePostReload(false, {}, "virtualhost");
+  }
+}
+
+void
+VirtualHostPluginReload::begin()
+{
+  if (!_started) {
+    PluginDso::loadedPlugins()->indicatePreReload("virtualhost");
+    _started = true;
+  }
+}
+
+void
+VirtualHostPluginReload::finish(VirtualHostConfig const &config)
+{
+  if (!_started) {
+    return;
+  }
+
+  std::unordered_map<PluginDso *, int> used;
+  config.collect_used_plugins(used);
+
+  PluginDso::loadedPlugins()->indicatePostReload(true, used, "virtualhost");
+  _started = false;
+}
+
+Ptr<VirtualHostConfig::Entry>
+VirtualHostConfig::find_by_id(std::string_view id) const
+{
+  if (_entries.empty()) {
+    return Ptr<VirtualHostConfig::Entry>();
+  }
+
+  auto entry = _entries.find(std::string{id});
+  if (entry != _entries.end()) {
+    return entry->second;
+  }
+  return Ptr<VirtualHostConfig::Entry>();
+}
+
+Ptr<VirtualHostConfig::Entry>
+VirtualHostConfig::find_by_domain(std::string_view domain) const
+{
+  if (_entries.empty() || domain.empty()) {
+    return Ptr<VirtualHostConfig::Entry>();
+  }
+
+  char lower_domain[TS_MAX_HOST_NAME_LEN + 1];
+  ts::transform_lower(std::string{domain}, lower_domain);
+
+  // Check for exact match domains first
+  auto id = _exact_domains_to_id.find(lower_domain);
+  if (id != _exact_domains_to_id.end()) {
+    auto entry = _entries.find(id->second);
+    if (entry != _entries.end()) {
+      return entry->second;
+    }
+  }
+
+  // Check wildcard suffixes
+  const char *subdomain = index(lower_domain, '.');
+  while (subdomain) {
+    subdomain++;
+    if (auto suffix_id = _wildcard_domains_to_id.find(subdomain); suffix_id != _wildcard_domains_to_id.end()) {
+      auto entry = _entries.find(suffix_id->second);
+      if (entry != _entries.end()) {
+        return entry->second;
+      }
+    }
+    subdomain = index(subdomain, '.');
+  }
+
+  return Ptr<VirtualHostConfig::Entry>();
+}
+
+namespace
+{
+/** Reload handler for the `virtualhost` config.
+
+    Registered as FileAndRpc so that `admin_config_reload` can carry `_reload` directives (currently
+    just `id`, for a single-entry reload). Pushed config *content* is deliberately not supported:
+    both reload paths re-read the on-disk file, so silently dropping a supplied body would report
+    success for a change that never took effect.
+ */
+void
+virtualhost_reload(ConfigContext ctx)
+{
+  ctx.in_progress();
+
+  if (ctx.supplied_yaml()) {
+    ctx.fail("virtualhost does not accept config content over rpc; only '_reload' directives are supported. "
+             "Update " +
+             std::string{ts::filename::VIRTUALHOST} + " and reload without a body.");
+    return;
+  }
+
+  // Single-entry reload requested via -D virtualhost.id=<id>
+  if (auto directives = ctx.reload_directives(); directives) {
+    if (const auto id_dir = directives["id"]; id_dir) {
+      if (!id_dir.IsScalar()) {
+        ctx.fail("virtualhost '_reload' directive 'id' must be a scalar");
+        return;
+      }
+      std::string id = id_dir.as<std::string>();
+      if (id.empty()) {
+        // An empty id would otherwise fall through to a full reload, turning a request scoped to
+        // one entry into a rebuild of the whole table from whatever is currently on disk.
+        ctx.fail("virtualhost '_reload' directive 'id' must name an entry; omit 'id' entirely to reload the whole file");
+        return;
+      }
+      if (VirtualHost::reconfigure(id, ctx)) {
+        ctx.complete("Reloaded virtualhost entry: " + id);
+      } else {
+        ctx.fail("Failed to reload virtualhost entry: " + id);
+      }
+      return;
+    }
+  }
+
+  if (VirtualHost::reconfigure(ctx)) {
+    ctx.complete("Finished loading virtualhost config");
+  } else {
+    ctx.fail("Failed to load virtualhost config");
+  }
+}
+} // namespace
+
+void
+VirtualHost::startup()
+{
+  if (!reconfigure({}, true)) {
+    Fatal("failed to load %s", ts::filename::VIRTUALHOST);
+  }
+
+  config::ConfigRegistry::Get_Instance().register_config(
+    "virtualhost",                          // registry key
+    ts::filename::VIRTUALHOST,              // default filename
+    "proxy.config.virtualhost.filename",    // record holding the filename
+    virtualhost_reload,                     // reload handler
+    config::ConfigSource::FileAndRpc,       // rpc may supply '_reload' directives; content is rejected
+    {"proxy.config.virtualhost.filename"}); // trigger records
+}
+
+int
+VirtualHost::reconfigure(ConfigContext ctx, bool initial_load)
+{
+  CfgLoadLog(ctx, DL_Note, "%s loading ...", ts::filename::VIRTUALHOST);
+  auto config = std::make_unique<VirtualHostConfig>();
+
+  // The parse is inside the lock as well as the publish; see vhost_reconfigure_mutex.
+  std::scoped_lock        lock(vhost_reconfigure_mutex);
+  VirtualHostPluginReload plugin_reload;

Review Comment:
   This mutex only serializes virtualhost reloads; the global `reloadUrlRewrite()` path independently sends the same remap-plugin pre/post notifications. A file-triggered global reload can therefore interleave between a virtualhost reload's callbacks (the LoadedPlugins lock is released after each callback), exposing plugins to a pre(A), pre(B), post(A), post(B) sequence and inconsistent instance state. Serialize virtualhost and global remap plugin reloads with a common lock.



-- 
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