Add to API to get a strategy during TSRemapNewInstance by traeak · Pull Request #12593 · apache/trafficserver

traeak · 2025-10-21T21:06:35Z

This gives the API access to the RemapConfig url_mapping during TSRemapNewInstance calls. This contains both the assigned stragies pointer and a pointer to the loaded strategy factory.

This modifies the strategies ts API from the previous PR.

API is:

valid during TSRemapNewInstance:

TSStrategy TSRemapNextHopStrategyFind(const char *name);
TSStrategy TSRemapNextHopStrategyGet();
void TSRemapNextHopStrategySet(TSStrategy strategy);

valid during transaction (the Find call is changed):

TSStrategy TSHttpTxnNextHopStrategyFind(TSHttpTxn txnp, const char *name);
TSStrategy TSHttpTxnNextHopStrategyGet(TSHttpTxn txnp);
void TSHttpTxnNextHopStrategySet(TSHttpTxn txnp, TSStrategy strategy);

with utility function to get the null terminated strategy name (returns "null" if nullptr strategy):

char const *TSNextHopStrategyNameGet(TSStrategy strategy);

The header_rewrite and regex_remap plugins are modified to look up named strategies during TSRemapNewInstance instead of performing strategy factory lookups during each transaction remap hook.

Copilot

Pull Request Overview

This PR refactors the next-hop strategy API to separate transaction-level and remap-level strategy operations, introducing new TSRemap* functions for use during TSRemapNewInstance. The API functions have been renamed for consistency and clarity.

Renamed TSHttpNextHopStrategyNameGet to TSNextHopStrategyNameGet
Renamed TSHttpTxnNextHopNamedStrategyGet to TSHttpTxnNextHopStrategyFind
Added new TSRemap* API functions for strategy operations during remap rule loading

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/proxy/http/remap/RemapConfig.cc	Sets up url_mapping::instance for TSRemap* API access and assigns strategyFactory to new mappings
src/api/InkAPI.cc	Implements new TSRemap* functions and renames existing API functions
plugins/regex_remap/regex_remap.cc	Updates to use new TSRemapNextHopStrategyFind API during initialization
plugins/lua/ts_lua_http.cc	Updates function calls to use renamed API functions
plugins/header_rewrite/operators.h	Changes internal storage from Value to explicit strategy pointer
plugins/header_rewrite/operators.cc	Refactors to use TSRemapNextHopStrategyFind during initialization and removes runtime hook
plugins/header_rewrite/conditions.cc	Updates to use renamed TSNextHopStrategyNameGet function
include/ts/ts.h	Updates API documentation and function declarations
include/proxy/http/remap/UrlMapping.h	Adds strategyFactory pointer and static instance pointer to url_mapping
doc/developer-guide/api/functions/*.en.rst	Adds/updates documentation for new and renamed API functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.

 OperatorSetNextHopStrategy::initialize(Parser &p)
 {
  Operator::initialize(p);
+  _stratname = p.get_arg();

-  _value.set_value(p.get_arg(), this);
-  Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy::initialie: %s", _value.get_value().c_str());
+  if (_stratname.empty() || "null" == _stratname) {
+    Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy() 'clear'");
+  } else {
+    _strategy = TSRemapNextHopStrategyFind(_stratname.c_str());
+    if (nullptr == _strategy) {
+      TSError("[%s] Failed to get strategy '%s'", PLUGIN_NAME, _stratname.c_str());
+      _apply = false;
+    } else {
+      Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy() '%s'", _stratname.c_str());
+    }
+  }
 }


 OperatorSetNextHopStrategy::exec(const Resources &res) const
 {
-  if (!res.state.txnp) {
-    TSError("[%s] OperatorSetNextHopStrategy() failed. Transaction is null", PLUGIN_NAME);
+  if (!_apply) {
+    Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy::exec: do nothing");
+    return true;
  }
-
  auto const txnp = res.state.txnp;


+  if (nullptr == _strategy) {
+    Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy::exec: Clearing strategy");
  } else {
-    Dbg(pi_dbg_ctl, "   Setting strategy '%s'", value.c_str());
-    TSHttpTxnNextHopStrategySet(txnp, stratptr);
+    Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy::exec: Setting strategy to '%s'", _stratname.c_str());
  }

+  TSHttpTxnNextHopStrategySet(txnp, _strategy);
+
  return true;
 }


 void
 OperatorSetNextHopStrategy::initialize_hooks()
 {
-  add_allowed_hook(TS_HTTP_READ_REQUEST_HDR_HOOK);
  add_allowed_hook(TS_REMAP_PSEUDO_HOOK);
 }


+TSStrategy
+TSRemapNextHopStrategyFind(const char *name)
 {
-  char const *name = nullptr;
-  if (nullptr != stratptr) {
-    auto strategy = reinterpret_cast<NextHopSelectionStrategy const *>(stratptr);
-    name          = strategy->strategy_name.c_str();
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
+
+  NextHopSelectionStrategy *strategy = nullptr;
+
+  if (nullptr != um->strategyFactory) {
+    // HttpSM has a reference count handle to UrlRewrite which manages
+    // the NextHopStrategyFactory pointer.
+    strategy = um->strategyFactory->strategyInstance(name);
  }


 /**
    Returns either null pointer or null terminated pointer to name.
-                DO NOT FREE.
+    DO NOT FREE.

    This value may be a nullptr due to:
      - parent proxying not enabled
      - no parent selection strategy (using parent.config)

-    @param txnp HTTP transaction whose next hop strategy to get.
+    @param pointer to the NextHopStrategy.

 */
-char const *TSHttpNextHopStrategyNameGet(void const *strategy);
+char const *TSNextHopStrategyNameGet(TSStrategy strategy);


+Plugins can get a strategy by name by calling
+:func:`TSRemapNextHopStrategyGet` to get the current transaction's active
+strategy or :func:`TSRemapNextHopStrategyFind` to look up a strategy by
+name using the loading remap rule's pointer to the NextHopStrategyFactory
+strategy database.


+TSStrategy
+TSHttpTxnNextHopStrategyFind(TSHttpTxn txnp, const char *name)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+  sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+
+  auto sm = reinterpret_cast<HttpSM const *>(txnp);
+
+  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap) == TS_SUCCESS);
+  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap->strategyFactory) == TS_SUCCESS);
+
+  // HttpSM has a reference count handle to UrlRewrite which has a
+  // pointer to NextHopStrategyFactory
+  NextHopSelectionStrategy *const strategy = sm->m_remap->strategyFactory->strategyInstance(name);
+
+  return reinterpret_cast<TSStrategy>(strategy);
 }


+TSStrategy
+TSRemapNextHopStrategyFind(const char *name)
 {
-  char const *name = nullptr;
-  if (nullptr != stratptr) {
-    auto strategy = reinterpret_cast<NextHopSelectionStrategy const *>(stratptr);
-    name          = strategy->strategy_name.c_str();
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
+
+  NextHopSelectionStrategy *strategy = nullptr;
+
+  if (nullptr != um->strategyFactory) {
+    // HttpSM has a reference count handle to UrlRewrite which manages
+    // the NextHopStrategyFactory pointer.
+    strategy = um->strategyFactory->strategyInstance(name);
  }

-  return name;
+  return reinterpret_cast<TSStrategy>(strategy);
 }

-void const *
-TSHttpTxnNextHopNamedStrategyGet(TSHttpTxn txnp, const char *name)
+void
+TSRemapNextHopStrategySet(TSStrategy strategy)
 {
-  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
+  // null strategy falls back to parent.config
+  // sdk_assert(sdk_sanity_check_null_ptr(stratptr) == TS_SUCCESS);

-  auto sm = reinterpret_cast<HttpSM const *>(txnp);
+  um->strategy = reinterpret_cast<NextHopSelectionStrategy *>(strategy);
+}


+char const *
+TSNextHopStrategyNameGet(TSStrategy stratptr)
+{
+  static char const *const nullname = "null";
+  char const              *name     = nullname;
+  if (nullptr != stratptr) {
+    auto strategy = reinterpret_cast<NextHopSelectionStrategy *>(stratptr);
+    name          = strategy->strategy_name.c_str();
+  }

-  return static_cast<void const *>(strat);
+  return name;
 }


traeak self-assigned this Oct 21, 2025

traeak added Plugins strategy labels Oct 21, 2025

traeak force-pushed the strategy_init branch 2 times, most recently from fd6730e to 7e032e6 Compare October 27, 2025 12:44

traeak changed the title ~~add strategies api for getting a strategy during remap plugin init~~ Add to API to get a strategy duringTSRemapNewInstance Oct 28, 2025

traeak marked this pull request as ready for review October 28, 2025 19:45

traeak changed the title ~~Add to API to get a strategy duringTSRemapNewInstance~~ Add to API to get a strategy during TSRemapNewInstance Oct 28, 2025

traeak force-pushed the strategy_init branch 3 times, most recently from ada93a4 to 00c8021 Compare November 5, 2025 13:29

traeak requested a review from Copilot November 5, 2025 18:05

Copilot AI reviewed Nov 5, 2025

View reviewed changes

Comment thread include/ts/ts.h Outdated

Comment thread plugins/header_rewrite/conditions.cc Outdated

Comment thread include/ts/ts.h Outdated

Comment thread include/ts/ts.h

traeak force-pushed the strategy_init branch from fc5b056 to 61e5afe Compare November 6, 2025 13:57

cmcfarlen requested a review from zwoop November 10, 2025 23:15

traeak force-pushed the strategy_init branch 2 times, most recently from 6663eb1 to 0650be2 Compare November 17, 2025 15:05

traeak force-pushed the strategy_init branch from 0650be2 to 907253f Compare February 6, 2026 13:44

zwoop requested changes Mar 5, 2026

View reviewed changes

Comment thread doc/developer-guide/api/functions/TSRemapNextHopStrategySet.en.rst Outdated

traeak force-pushed the strategy_init branch from 560a02e to 3cad1f2 Compare March 9, 2026 13:17

traeak force-pushed the strategy_init branch 2 times, most recently from 9a076e8 to 8ddaa2d Compare March 17, 2026 20:31

zwoop added this to the 11.0.0 milestone May 7, 2026

traeak and others added 4 commits July 27, 2026 13:18

add strategies api for getting a strategy during remap plugin init

b5f814a

introduce TSStrategy type for plugins

844f366

regex_remap: remove std::move, causes use after free (debug)

cc49180

fix doc error

a93d7b3

Copilot AI review requested due to automatic review settings July 27, 2026 19:19

traeak force-pushed the strategy_init branch from 8ddaa2d to a93d7b3 Compare July 27, 2026 19:19

Copilot started reviewing on behalf of traeak July 27, 2026 19:36 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add to API to get a strategy during TSRemapNewInstance - #12593

Add to API to get a strategy during TSRemapNewInstance#12593
traeak wants to merge 4 commits into
apache:masterfrom
traeak:strategy_init

traeak commented Oct 21, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

traeak commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

traeak commented Oct 21, 2025 •

edited

Loading