virtualhost.yaml with remap rules

[serrislew]

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 using reload directives
  • $ traffic_ctl config reload -D virtualhost.id=foo
  • Uses token model reload

[brbzull0]

I was talking with @serrislew about some parts of this PR, specially related to the interaction with the reload handler. I think #13110 is the plumbing that the id base reloading could benefit from.

[Copilot]

Pull request overview

This PR introduces virtualhost.yaml as a new configuration file that maps request hostnames (exact and wildcard) to a single virtual host entry, enabling per-virtualhost remap rule overrides (in remap.yaml format) with support for granular reload via reload directives / JSONRPC.

Changes:

• Add virtualhost.yaml configuration + record proxy.config.virtualhost.filename, default config stub, and admin-guide documentation.
• Integrate virtualhost lookup into HttpSM::do_remap_request() so virtualhost remap rules are attempted before global remap rules, with fallback to the global remap table when no match is found.
• Extend remap.yaml handling so UrlRewrite / remap parser can build tables from an inline YAML node (used by virtualhost remap blocks) and enable reload-directive routing to the virtualhost handler.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/gold_tests/jsonrpc/config_reload_rpc.test.py	Updates JSONRPC reload-directive test to expect virtualhost directives to route to the handler.
src/records/RecordsConfig.cc	Adds proxy.config.virtualhost.filename dynamic record.
src/proxy/VirtualHost.cc	Implements virtualhost config loading, domain matching, per-entry reload, and config registry registration.
src/proxy/ReverseProxy.cc	Calls VirtualHost::startup() during reverse proxy initialization.
src/proxy/http/remap/UrlRewrite.cc	Factors out load_table() and allows table building from an inline YAML node.
src/proxy/http/remap/RemapYamlConfig.cc	Adds overloads to parse inline remap YAML sequences into remap tables.
src/proxy/http/HttpSM.cc	Adds per-transaction virtualhost entry selection and remap table override/fallback logic.
src/proxy/CMakeLists.txt	Adds VirtualHost.cc to the proxy library build.
include/tscore/Filenames.h	Adds virtualhost.yaml to known filenames.
include/proxy/VirtualHost.h	Declares virtualhost config/entry types and VirtualHost API.
include/proxy/http/remap/UrlRewrite.h	Declares load_table() and updated BuildTable() signature.
include/proxy/http/remap/RemapYamlConfig.h	Declares new inline YAML parsing overloads.
include/proxy/http/HttpSM.h	Adds virtualhost state to HttpSM and declares helper method.
doc/admin-guide/files/virtualhost.yaml.en.rst	New documentation for virtualhost.yaml, evaluation order, and granular reload.
doc/admin-guide/files/records.yaml.en.rst	Documents the new proxy.config.virtualhost.filename record.
doc/admin-guide/files/index.en.rst	Adds virtualhost.yaml to the admin-guide files index.
configs/virtualhost.yaml.default	Adds a default/example virtualhost.yaml template.

[brbzull0]

Looks good to me.
I'll approve it but I'd wait for @bryancall to do his review as well before merging it.

[bryancall]

I read the full diff across all 17 files and traced the new code against current master. The design work here is real: the domain resolution is deterministic and validated at load time, duplicate ids and duplicate exact and wildcard domains are all rejected across entries, wildcards are restricted to a single left-most *. form, and find_by_domain walks dot-suffixes longest to shortest so the documented "most specific wildcard wins" rule is actually what the code does. It follows the ConfigProcessor/ConfigRegistry idiom closely, it is opt-in and backward compatible, and it ships a full admin-guide page rather than a stub.

Requesting changes. Two blocking items, one of which means the PR cannot build against master as it stands.

Blocking 1: the inline remap parser clobbers the process-global IP allow accept-check flag

src/proxy/http/remap/RemapYamlConfig.cc:~1057

The new inline-node parser ends with IpAllow::enableAcceptCheck(bti->accept_check_p). IpAllow::accept_check_p is a single process-wide static (src/proxy/IPAllow.cc:75, setter at include/proxy/IPAllow.h:398-403), written from exactly three places: RemapConfig.cc:1555, the existing file parser at RemapYamlConfig.cc:1016, and now this.

The ordering makes it reachable. init_reverse_proxy() calls initial_table->load() first, and this PR appends VirtualHost::startup() at the very end of the same function, so every virtualhost table is parsed after the authoritative global table. build_virtualhost_entry to UrlRewrite::load_table to BuildTable to remap_parse_yaml constructs a fresh BUILD_TABLE_INFO whose accept_check_p defaults to true (include/proxy/http/remap/RemapConfig.h:67) and is only lowered by a rule inside that virtualhost.

So a global remap.yaml containing deactivate_filter: ip_allow, which is documented at remap.yaml.en.rst:1035, leaves accept_check_p false, and then the last virtualhost parsed resets it to true. A per-domain config silently rewrites process-wide IP access-control enforcement, last writer wins, at startup and on every granular reload. That is a security-relevant global being set from a per-domain scope.

Blocking 2: the refcount handling targets an ownership model that no longer exists

src/proxy/http/HttpSM.cc:4578-4633 and include/proxy/http/HttpSM.h:307-311

Master commit 709443e870 ("Fix race in remap table refcount during reload") removed UrlRewrite's RefCountObj base. On current master, include/proxy/http/remap/UrlRewrite.h has no acquire, release or RefCountObj; HttpSM.h:315 is std::shared_ptr<UrlRewrite> m_remap and every call site uses m_remap.get(). ReverseProxy.cc now exposes AtomicSharedPtr<UrlRewrite> rewrite_table with a custom deleter and a shutdown path that stores nullptr.

This PR still declares UrlRewrite *m_remap and calls acquire()/release() on UrlRewrite in four places, and rewrite_table.load()->acquire() is both a compile error and a null-dereference hazard during shutdown. GitHub reports the branch as conflicting, and the 15 green checks were run against the pre-709443e870 base, so they say nothing about the current state.

I want to flag that this is not a textual merge. The virtualhost table lifetime needs redesigning against the new shared-pointer ownership, and that redesign is worth doing deliberately, since getting per-domain table lifetime wrong under reload is exactly the class of race 709443e870 was fixing.

Should fix

src/proxy/VirtualHost.cc:385 The config is registered as ConfigSource::FileAndRpc, but the reload handler never reads ctx.supplied_yaml(). It reads only ctx.reload_directives() looking for id, then re-reads the on-disk file in both branches and calls ctx.complete(). Configuration.cc:300 rejects a pushed body only when the source is not FileAndRpc, and ConfigRegistry::execute_reload calls ctx.set_supplied_yaml(passed_config) before invoking the handler, with the registry comment at line 489 stating the contract that the handler is supposed to check it. So an admin_config_reload carrying virtualhost content is accepted, silently discarded, and answered with "Finished loading virtualhost config". IPAllow.cc:101 shows the deliberate alternative: register FileOnly with a comment saying why.

src/proxy/VirtualHost.cc:140 The YAML exception handler is catch (YAML::Exception const &ex) { Dbg(dbg_ctl_virtualhost, "Failed to parse virtualhost entry"); return false; }. Fixed string, ex bound and unused, no entry id, no line number. Every validation failure in convert<Entry>::decode (missing id, empty domains, malformed wildcard) and every failure in VirtualHostConfig::load (non-sequence top level, duplicate id, duplicate domain) is debug-only; only the unknown-key case uses Warning. The failure then surfaces as Fatal("failed to load %s") at startup with no cause attached. An operator with a typo in virtualhost.yaml gets a fatal exit and nothing to act on. RemapYamlConfig.cc routes the same class of failure through CfgLoadLog(ctx, DL_Error, ...) with ex.what(), which is the model to follow.

Smaller items

• src/proxy/VirtualHost.cc:72 std::set<std::string> valid_vhost_keys is a mutable namespace-scope global with external linkage in a .cc file. Should be const and in an anonymous namespace.
• src/proxy/VirtualHost.cc:257 Dbg(..., "%s", id.data()) is called on a std::string_view in three places. Not guaranteed NUL-terminated.
• include/proxy/VirtualHost.h:56-58 Entry::acquire()/release() hand-roll refcounting that Ptr<Entry> already provides, with dead if (self) null checks after a const_cast of this.
• src/proxy/VirtualHost.cc:148 UrlRewrite::load_table(const std::string &config_file_path, ...) is called with the virtualhost id as the config file path, which then flows into BuildTable as a path.
• src/proxy/http/HttpSM.cc:4578-4582 set_virtualhost_entry constructs VirtualHost::scoped_config, a config processor get plus a refcount, before the early-return checks, so every transaction pays for it even when no virtualhost is configured.
• doc/admin-guide/files/virtualhost.yaml.en.rst:212 The second example still has url: http:/foo.example.com/ with a single slash. Copilot raised this last round.
• configs/virtualhost.yaml.default:21 The shipped default uses - "*.com" as its wildcard example, which is an unfortunate thing to have someone uncomment.
• tests/gold_tests/jsonrpc/config_reload_rpc.test.py:440 The docstring of validate_directive_routed still says virtualhost is not registered and is rejected with 6010, contradicting the assertions directly below it.

Two things I initially suspected and then ruled out, so nobody re-litigates them: internal redirects do not leave a stale virtualhost table in a way that matters here, and the missing acl_filters section in the inline parser is not actually a gap.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Suppressed comments (6)

src/proxy/VirtualHost.cc:135

• For inline remap YAML, load_table() is passed conf.id as config_file_path. If remap rules use features that rely on an actual source path (e.g., include directives resolved relative to a file location, or path-based diagnostics), using the virtualhost id as a 'path' can produce incorrect behavior or confusing logs. Consider passing the actual virtualhost.yaml path (or a base directory) separately from a human-readable label, so inline parsing has a correct filesystem context.

  // 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;
    }

src/proxy/VirtualHost.cc:316

• find_by_domain() allocates a temporary std::string{domain} to lowercase, and then performs map lookups using a char* key on std::unordered_map<std::string, ...> (which typically constructs a temporary std::string for lookup). This runs on every request, so the extra allocations can add measurable overhead. Consider lowercasing without allocating (if an overload exists) and/or enabling heterogeneous lookup (transparent hash/equal) so lookups can be done with std::string_view/char* without constructing a std::string.

  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()) {

tests/gold_tests/jsonrpc/config_reload_rpc.test.py:438

• The docstring for validate_directive_routed contradicts the updated test intent (virtualhost is now registered and should be routed/accepted). Update the docstring to reflect the new expected behavior so the test remains self-describing.

def validate_directive_routed(resp: Response):
    '''virtualhost is not registered — rejected with 6010'''
    result = resp.result

tests/gold_tests/jsonrpc/config_reload_rpc.test.py:449

• result.get('message', []) defaults to a list, but message is typically a string in JSON-RPC responses. Using a consistent default type (e.g., empty string) makes the intent clearer and avoids surprising truthiness/type behavior in validations.

    tasks = result.get('tasks', [])
    message = result.get('message', [])
    if tasks or message:

doc/admin-guide/files/virtualhost.yaml.en.rst:210

• The example URL is malformed (http:/... should be http://...). Since this is a copy/paste-able config example, it should be corrected to prevent user misconfiguration.

            url: http:/foo.example.com/

doc/admin-guide/files/virtualhost.yaml.en.rst:178

• Fix grammar: 'This rules translates' should be 'These rules translate'.

This rules translates in the following translation.