From 37c5933cead2bf5860f07739b75e83c43ccb40c1 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 28 Apr 2026 13:57:41 +0200 Subject: [PATCH 1/6] Add TSCfg* plugin API for config reload framework Expose the centralized config reload framework to plugins via a new TSCfg* C API so they can register configuration files alongside core ATS configs and participate in the reload status, RPC inline-YAML, and diagnostics machinery on equal footing. Public API (include/ts/ts.h, include/ts/apidefs.h.in): - TSCfgRegister(const TSCfgRegistrationInfo *) registers a plugin config with a TSCfgLoadCb handler. The plugin's canonical name is captured automatically from TSPluginRegister and surfaced in logs and traffic_ctl output. - TSCfgRegistrationInfo and TSCfgFileDependencyInfo option structs keep the API extensible without breaking source compatibility. - TSCfgAttachTrigger, TSCfgAddFileDependency, TSCfgSetEnabled cover reload triggers, file dependencies, and runtime enable/disable. TSCfgAddFileDependency supports operator-tunable filename via filename_record and inline-YAML routing via dep_key. - TSCfgLoadCtx handler-context API: InProgress / Complete / Fail for deferred completion, AddLog for severity-aware messages, AddSubtask for nested handlers, and getters for filename, reload token, the RPC-supplied YAML, and reload directives. - TSCfgLoadCb callback type and TSCfgSourceType enum. Core integration: - ConfigRegistry::register_plugin_config attributes plugin name to the entry; ConfigContext / ConfigReloadTrace propagate it through tasks and logs. - traffic_ctl config status renders plugin attribution and task trees with severity-prefixed log lines. - regex_revalidate migrated to the new API as the reference example. Tests: - cfg_plugin_test exercises every public API entrypoint. - cfg_plugin_deferred_test demonstrates two-stage deferred completion via scheduled continuations on ET_TASK. - cfg_plugin_directives_test covers _reload directive routing. - New autests: config_reload_plugin_api, config_reload_deferred, config_reload_directives_plugin, config_reload_ssl_bulk, config_reload_ssl_state. Documentation: - doc/developer-guide/api/functions/TSCfgRegister.en.rst consolidates the reference for TSCfgRegister and TSCfgLoadCtx. - doc/developer-guide/config-reload-framework.en.rst gains a Plugin Configuration Reload section with example, lifecycle, and traffic_ctl status output. Ref: #12967 --- .../api/functions/TSCfgRegister.en.rst | 446 ++++++++++++++++++ .../config-reload-framework.en.rst | 199 +++++++- include/mgmt/config/ConfigContext.h | 25 +- include/mgmt/config/ConfigRegistry.h | 72 ++- include/mgmt/config/ConfigReloadTrace.h | 24 +- include/records/YAMLConfigReloadTaskEncoder.h | 5 +- include/ts/apidefs.h.in | 124 +++++ include/ts/ts.h | 239 ++++++++++ plugins/regex_revalidate/regex_revalidate.cc | 79 ++-- src/api/InkAPI.cc | 364 ++++++++++++++ src/mgmt/config/ConfigContext.cc | 17 + src/mgmt/config/ConfigRegistry.cc | 220 ++++++--- src/mgmt/config/FileManager.cc | 3 + src/records/unit_tests/test_ConfigRegistry.cc | 61 +++ src/traffic_ctl/CtrlPrinters.cc | 7 +- src/traffic_ctl/jsonrpc/CtrlRPCRequests.h | 7 +- src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h | 3 + .../jsonrpc/config_reload_deferred.test.py | 158 +++++++ .../config_reload_directives_plugin.test.py | 174 +++++++ .../jsonrpc/config_reload_plugin_api.test.py | 333 +++++++++++++ .../jsonrpc/config_reload_ssl_bulk.test.py | 202 ++++++++ .../jsonrpc/config_reload_ssl_state.test.py | 180 +++++++ .../gold_tests/jsonrpc/plugins/CMakeLists.txt | 9 + .../plugins/cfg_plugin_deferred_test.cc | 166 +++++++ .../plugins/cfg_plugin_directives_test.cc | 155 ++++++ .../jsonrpc/plugins/cfg_plugin_test.cc | 227 +++++++++ 26 files changed, 3374 insertions(+), 125 deletions(-) create mode 100644 doc/developer-guide/api/functions/TSCfgRegister.en.rst create mode 100644 tests/gold_tests/jsonrpc/config_reload_deferred.test.py create mode 100644 tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py create mode 100644 tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py create mode 100644 tests/gold_tests/jsonrpc/config_reload_ssl_bulk.test.py create mode 100644 tests/gold_tests/jsonrpc/config_reload_ssl_state.test.py create mode 100644 tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc create mode 100644 tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc create mode 100644 tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst new file mode 100644 index 00000000000..d509ea9363a --- /dev/null +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -0,0 +1,446 @@ +.. 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 + +.. default-domain:: cpp + +TSCfgRegister +************* + +Register a plugin's configuration file with the |TS| reload framework +and drive the per-reload context object delivered to the handler. + +Synopsis +======== + +.. code-block:: cpp + + #include + +.. type:: TSCfgSourceType + + What content sources a plugin's config handler supports. + + .. var:: TS_CFG_SOURCE_FILE_ONLY + + Handler reloads only from a file on disk. RPC-supplied YAML is + rejected. + + .. var:: TS_CFG_SOURCE_FILE_AND_RPC + + Handler can also process YAML content supplied via RPC. + +.. type:: TSCfgRegistrationInfo + + Options struct passed to :func:`TSCfgRegister`. Fields are set with + C++20 designated initializers; new fields may be appended in future + |TS| versions without breaking source compatibility for existing + plugins. + + .. var:: std::string_view key + + Unique registry key. Doubles as the YAML node name when an operator + reloads via ``traffic_ctl config reload --content '{: {...}}'``. + Convention: use the plugin name, or ``.`` when a single + plugin owns multiple configs. Required. + + .. var:: std::string_view config_path + + Default config file path (absolute, or relative to ``sysconfdir``). + Required. + + .. var:: std::string_view filename_record + + Fully-qualified record name that holds the active config filename, + or ``{}`` (default) if the path is fixed. When set, the operator + can change the active config file at runtime by editing this + record; ``config_path`` is used as the fallback when the record is + empty or unset. Optional. + + .. var:: TSCfgLoadCb handler + + Reload callback invoked when the file changes, when an attached + trigger record changes, or when the operator triggers an RPC + reload. Required. + + .. var:: void *data + + Opaque pointer passed unmodified to ``handler`` on every invocation. + The plugin owns it; ATS does not copy or free it. Must outlive the + registration - typically a ``static`` or once-allocated heap object + stashed in ``TSPluginInit``:: + + static PluginState state; + state.config_path = ...; + + TSCfgRegistrationInfo info{}; + info.handler = config_reload; + info.data = &state; + TSCfgRegister(&info); + + .. var:: TSCfgSourceType source + + Whether ``handler`` can also process YAML content supplied via RPC. + Defaults to :var:`TS_CFG_SOURCE_FILE_ONLY`. + + .. var:: bool is_required + + ``true`` if the file must exist on disk for a reload to be + considered successful. Defaults to ``false``. + +.. type:: TSCfgFileDependencyInfo + + Options struct passed to :func:`TSCfgAddFileDependency`. Only ``key`` + and ``config_path`` are required. + + .. var:: std::string_view key + + Parent registry key as passed to :func:`TSCfgRegister`. Required. + + .. var:: std::string_view config_path + + Default companion file path (absolute, or relative to ``sysconfdir``). + Required. + + .. var:: std::string_view filename_record + + Fully-qualified record name that holds the filename, or ``{}`` if + the path is fixed. Optional. + + .. var:: std::string_view dep_key + + Routing key for inline YAML supplied via JSONRPC. When non-empty, + content delivered under this top-level node is routed to the parent + entry's handler, giving the plugin parity with core + composite-config patterns. When empty (default), the dependency is + file-change-only. + + .. var:: bool is_required + + ``true`` if the file must exist on disk for a reload to be + considered successful. Defaults to ``false``. + +.. type:: TSCfgLoadCtx + + Opaque handle to a per-reload context. The context tracks the + lifecycle of a single reload attempt for one registered config: it + carries the reload token, supplied YAML (for RPC reloads), the + filename (for file reloads), and the task state machine the framework + uses to aggregate results. + +.. type:: void (*TSCfgLoadCb)(TSCfgLoadCtx ctx, void *data) + + Plugin reload callback signature. ``data`` is the opaque pointer the + plugin supplied via :var:`TSCfgRegistrationInfo::data`. + +.. function:: TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info) +.. function:: TSReturnCode TSCfgAttachTrigger(std::string_view key, std::string_view record_name) +.. function:: TSReturnCode TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info) +.. function:: TSReturnCode TSCfgSetEnabled(std::string_view key, int enabled) +.. function:: void TSCfgLoadCtxInProgress(TSCfgLoadCtx ctx, std::string_view msg) +.. function:: void TSCfgLoadCtxComplete(TSCfgLoadCtx ctx, std::string_view msg) +.. function:: void TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg) +.. function:: void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg) +.. function:: TSCfgLoadCtx TSCfgLoadCtxAddSubtask(TSCfgLoadCtx ctx, std::string_view description) +.. function:: std::string_view TSCfgLoadCtxGetFilename(TSCfgLoadCtx ctx) +.. function:: std::string_view TSCfgLoadCtxGetReloadToken(TSCfgLoadCtx ctx) +.. function:: TSYaml TSCfgLoadCtxGetSuppliedYaml(TSCfgLoadCtx ctx) +.. function:: TSYaml TSCfgLoadCtxGetReloadDirectives(TSCfgLoadCtx ctx) + +Description +=========== + +These functions are the plugin-facing entry points to the |TS| +configuration reload framework. A plugin registers a config file +together with a reload callback, and the framework drives the callback +whenever the file changes, an attached trigger record changes, or an +operator triggers a reload via JSONRPC. + +The plugin name (set via :func:`TSPluginRegister`) is automatically +attached to every registered entry, so reload-trace logs and +:option:`traffic_ctl config status` output identify which plugin owns +which entry as ``[plugin: ]``. Plugins do not pass their name +explicitly. + +Registration +------------ + +:func:`TSCfgRegister` + Registers a config file and reload handler. + + Must be called from :func:`TSPluginInit`, **after** + :func:`TSPluginRegister`. Returns ``TS_ERROR`` if called outside + ``TSPluginInit``, before ``TSPluginRegister``, with a null or + incomplete ``info`` struct, or if another plugin (or core) has + already registered the same key. + +:func:`TSCfgAttachTrigger` + Attaches a fully-qualified record name (e.g. + ``proxy.config.my_plugin.enabled``) to a previously registered key. + When the record value changes, the plugin's handler is invoked, the + same way it would be if the file changed on disk. May be called + multiple times to attach more than one trigger record to the same + entry. + +:func:`TSCfgAddFileDependency` + Adds a companion file to a previously registered key. When the + companion file changes on disk - or when an operator submits inline + YAML under :var:`TSCfgFileDependencyInfo::dep_key` via JSONRPC, when + set - the plugin's handler is invoked. With ``dep_key`` empty the + dependency is file-change-only. + +:func:`TSCfgSetEnabled` + Toggles a registered config at runtime. Disabled entries are skipped + during reloads (their tasks are short-circuited to a + ``"skipped (disabled)"`` completion). Unlike the other three, this + function may be called at any time, not only during ``TSPluginInit``. + +Per-reload context (TSCfgLoadCtx) +--------------------------------- + +A :type:`TSCfgLoadCtx` is created by the framework before invoking the +plugin's :type:`TSCfgLoadCb`. The handler must drive the context to a +**terminal state** - either :func:`TSCfgLoadCtxComplete` or +:func:`TSCfgLoadCtxFail` - so the reload tree can finish. See +:ref:`config-context-terminal-state` for the complete contract. + +State transitions: + +:func:`TSCfgLoadCtxInProgress` + Set the task to ``IN_PROGRESS`` and emit an optional progress + message. Calling it more than once on the same context is + discouraged - use :func:`TSCfgLoadCtxAddLog` for periodic log + output. Pass ``{}`` for ``msg`` if no message is needed. + +:func:`TSCfgLoadCtxComplete` + Mark the task as successfully completed. Pass ``{}`` for ``msg`` if + no message is needed. After this call the framework deletes the + context handle - **do not access** ``ctx`` after a successful + Complete. + +:func:`TSCfgLoadCtxFail` + Mark the task as failed. Pass ``{}`` for ``msg`` if no message is + needed. After this call the framework deletes the context handle - + **do not access** ``ctx`` after a Fail. + +:func:`TSCfgLoadCtxAddLog` + Append a log entry at ``level`` to the task. Visible in + :option:`traffic_ctl config status` output and in the + :ref:`get_reload_config_status` JSONRPC response. Does not change the + task state. + +:func:`TSCfgLoadCtxAddSubtask` + Create a child subtask. Returns a new context that the plugin must + independently drive to a terminal state. Use this when a single + config reload spawns several distinct units of work that should be + tracked separately. The parent's status aggregates from its + children: any child failing causes the parent to be marked failed. + + The subtask is born in ``CREATED`` and follows the same state + machine as the parent. Two transition patterns are valid: + + - ``CREATED -> IN_PROGRESS -> SUCCESS / FAILED`` (recommended for + substantive work) - call :func:`TSCfgLoadCtxInProgress` once you + start, then :func:`TSCfgLoadCtxComplete` / + :func:`TSCfgLoadCtxFail` when done. The "in progress" phase is + visible in :option:`traffic_ctl config status`. + - ``CREATED -> SUCCESS / FAILED`` (shortcut for trivial subtasks) - + call :func:`TSCfgLoadCtxComplete` / :func:`TSCfgLoadCtxFail` + directly on the returned handle. + +Inputs: + +:func:`TSCfgLoadCtxGetFilename` + Returns the resolved file path the framework intends the handler to + read from disk. Empty for RPC-only reloads. + +:func:`TSCfgLoadCtxGetReloadToken` + Returns the reload-cycle correlation token (e.g. + ``rldtk-``). Useful for plugin-side log lines that need + to match the operator's + ``traffic_ctl config status -t `` view. + +:func:`TSCfgLoadCtxGetSuppliedYaml` + Returns the YAML content supplied via the JSONRPC reload payload as + an opaque :type:`TSYaml` handle (a ``YAML::Node *``). The + framework-reserved ``_reload`` directives are stripped before + delivery. The returned node is undefined when the reload was driven + by file change rather than RPC. Only meaningful when the entry was + registered with :var:`TS_CFG_SOURCE_FILE_AND_RPC`. + +:func:`TSCfgLoadCtxGetReloadDirectives` + Returns the YAML map extracted from the ``_reload`` key in the + RPC-supplied content as an opaque :type:`TSYaml` handle. Used by + handlers that need scoped reload behavior (for example, reload only + one entry, dry-run mode, or a version constraint). See + :ref:`config-reload-framework` for the directive convention. + +The two ``std::string_view`` accessors return views whose backing +storage is owned by the framework and is valid for the lifetime of the +handler call. Copy the data if you need to retain it past the callback +return. The :type:`TSYaml` accessors return handles whose lifetime is +also tied to the handler call. + +Lifecycle and threading +----------------------- + +The handler runs on an ``ET_TASK`` thread. It does **not** have to +complete synchronously: the plugin may stash ``ctx`` in its own state, +return from the callback, finish work on a different thread, and then +call :func:`TSCfgLoadCtxComplete` or :func:`TSCfgLoadCtxFail` from +there. This is the standard pattern for plugins that perform +asynchronous I/O during reload. + +If the handler never reaches a terminal state, the per-reload progress +checker eventually marks the task as ``TIMEOUT`` and unblocks the +overall reload (see :ref:`config-context-terminal-state`). The context +handle then leaks for the process lifetime, so reaching a terminal +state on every code path is mandatory. + +Restrictions +============ + +- Not supported for remap plugins (:func:`TSRemapInit` / + :func:`TSRemapNewInstance`). +- :func:`TSCfgRegister`, :func:`TSCfgAttachTrigger`, and + :func:`TSCfgAddFileDependency` must all be called from + :func:`TSPluginInit`, after :func:`TSPluginRegister`. +- :func:`TSCfgSetEnabled` may be called at any time after registration. + +Example - registration and synchronous handler +============================================== + +.. code-block:: cpp + + #include + #include + + namespace + { + constexpr char PLUGIN_NAME[] = "my_plugin"; + + struct PluginState { + std::string config_path; + }; + + void + config_reload(TSCfgLoadCtx ctx, void *data) + { + auto *state = static_cast(data); + + std::string_view filename = TSCfgLoadCtxGetFilename(ctx); + if (filename.empty()) { + TSCfgLoadCtxFail(ctx, "no filename available"); + return; + } + + if (!parse_and_apply(state, std::string{filename})) { + TSCfgLoadCtxFail(ctx, "parse failed"); + return; + } + + TSCfgLoadCtxComplete(ctx, "Reloaded my_plugin"); + } + } // anonymous namespace + + void + TSPluginInit(int /* argc */, const char * /* argv */[]) + { + TSPluginRegistrationInfo plugin{}; + plugin.plugin_name = PLUGIN_NAME; + plugin.vendor_name = "Example Inc."; + plugin.support_email = "support@example.com"; + + if (TSPluginRegister(&plugin) != TS_SUCCESS) { + TSError("[%s] plugin registration failed", PLUGIN_NAME); + return; + } + + static PluginState state; + state.config_path = std::string{TSConfigDirGet()} + "/my_plugin.yaml"; + + TSCfgRegistrationInfo info{}; + info.key = PLUGIN_NAME; + info.config_path = state.config_path; + info.handler = config_reload; + info.data = &state; + info.source = TS_CFG_SOURCE_FILE_AND_RPC; + info.is_required = false; + if (TSCfgRegister(&info) != TS_SUCCESS) { + TSError("[%s] TSCfgRegister failed for '%s'", PLUGIN_NAME, + state.config_path.c_str()); + return; + } + + // Optional: attach a record so changing it fires the handler. + TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); + } + +Example - RPC content with directives +===================================== + +.. code-block:: cpp + + #include + + void + config_reload(TSCfgLoadCtx ctx, void * /* data */) + { + auto *yaml = static_cast(TSCfgLoadCtxGetSuppliedYaml(ctx)); + auto *dirs = static_cast(TSCfgLoadCtxGetReloadDirectives(ctx)); + + if (dirs && dirs->IsDefined()) { + // operator passed --directive ... + } + if (yaml && yaml->IsDefined()) { + // RPC-supplied content; do not read the file. + } else { + // file-driven reload; read TSCfgLoadCtxGetFilename(ctx). + } + TSCfgLoadCtxComplete(ctx, {}); + } + +Example - deferred completion +============================= + +A plugin that schedules background work returns from the callback +without calling Complete or Fail; it stashes ``ctx`` and finishes from +another thread: + +.. code-block:: cpp + + struct Work { TSCfgLoadCtx ctx; PluginState *state; }; + + void + config_reload(TSCfgLoadCtx ctx, void *data) + { + auto *work = new Work{ctx, static_cast(data)}; + // schedule heavy_work(work) on a worker thread; it will eventually + // call TSCfgLoadCtxComplete(work->ctx, {}) (or Fail) and delete work. + schedule_async(work); + } + +A runnable example of the deferred pattern lives in +``tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc``. + +See Also +======== + +:ref:`config-reload-framework` is the complete framework guide, +covering the registry, terminal-state rule, RPC reload payload format, +``[plugin: ]`` attribution in :option:`traffic_ctl config status`, +diagnostic macros, and ``diags.log`` summaries. diff --git a/doc/developer-guide/config-reload-framework.en.rst b/doc/developer-guide/config-reload-framework.en.rst index 549512fec22..3816651654a 100644 --- a/doc/developer-guide/config-reload-framework.en.rst +++ b/doc/developer-guide/config-reload-framework.en.rst @@ -695,6 +695,201 @@ This means the same handler code works in both cases without branching: } +.. _config-reload-plugin-api: + +Plugin Configuration Reload +=========================== + +Plugins integrate with the same registry and same task tree described +above, through the public ``TSCfg*`` C++ API in :file:`ts/ts.h`. The +framework treats plugin-registered configs as first-class entries: they +appear in :option:`traffic_ctl config status`, accept inline YAML via +JSONRPC, honor file-mtime change detection, react to attached trigger +records, and follow the same :ref:`terminal state rule +` as core handlers. + +What changes for plugins is only the surface API: + +- ``ConfigRegistry::register_config`` becomes :func:`TSCfgRegister`, + which takes a :type:`TSCfgRegistrationInfo` options struct. +- ``ConfigRegistry::attach`` becomes :func:`TSCfgAttachTrigger`. +- ``ConfigRegistry::add_file_dependency`` becomes + :func:`TSCfgAddFileDependency`. +- ``ConfigContext`` becomes the opaque ``TSCfgLoadCtx`` handle, with + the same in-progress / complete / fail / log / supplied-yaml / + reload-directives / add-subtask operations exposed as plain + ``TSCfgLoadCtx*`` functions. +- :func:`TSCfgSetEnabled` allows the plugin to disable a registered + entry at runtime, in which case the framework short-circuits the + task to a ``"skipped (disabled)"`` completion. + +Lifecycle and preconditions +--------------------------- + +All ``TSCfg*`` registration calls must be made from :func:`TSPluginInit`, +**after** :func:`TSPluginRegister` has succeeded. The framework reads the +calling plugin's canonical name from ``TSPluginRegister`` and attaches it +to every registered entry; plugins do not pass their name explicitly. +Calling :func:`TSCfgRegister` outside ``TSPluginInit``, before +``TSPluginRegister``, or with a null ``info`` returns ``TS_ERROR``. + +The reload framework is global-plugin only. Remap plugins +(:func:`TSRemapInit` / :func:`TSRemapNewInstance`) cannot register +config entries. + +Plugin example +-------------- + +A minimal global plugin that registers ``my_plugin.yaml`` and accepts +either file-driven or RPC-driven reload: + +.. code-block:: cpp + + #include + #include + + namespace + { + constexpr char PLUGIN_NAME[] = "my_plugin"; + + struct PluginState { + std::string config_path; + }; + + void + config_reload(TSCfgLoadCtx ctx, void *data) + { + auto *state = static_cast(data); + + // Optionally: announce that work has started. + TSCfgLoadCtxInProgress(ctx, "Reloading my_plugin"); + + std::string_view fn = TSCfgLoadCtxGetFilename(ctx); + if (!parse_file(state, std::string{fn})) { + TSCfgLoadCtxFail(ctx, "Failed to parse my_plugin.yaml"); + return; + } + + TSCfgLoadCtxComplete(ctx, "Reloaded my_plugin"); + } + } // anonymous namespace + + void + TSPluginInit(int /* argc */, const char * /* argv */[]) + { + TSPluginRegistrationInfo plugin{}; + plugin.plugin_name = PLUGIN_NAME; + plugin.vendor_name = "Example Inc."; + plugin.support_email = "support@example.com"; + + if (TSPluginRegister(&plugin) != TS_SUCCESS) { + TSError("[%s] plugin registration failed", PLUGIN_NAME); + return; + } + + static PluginState state; + state.config_path = std::string{TSConfigDirGet()} + "/my_plugin.yaml"; + + TSCfgRegistrationInfo info{}; + info.key = PLUGIN_NAME; + info.config_path = state.config_path; + info.handler = config_reload; + info.data = &state; + info.source = TS_CFG_SOURCE_FILE_AND_RPC; + info.is_required = false; + if (TSCfgRegister(&info) != TS_SUCCESS) { + TSError("[%s] TSCfgRegister failed", PLUGIN_NAME); + return; + } + + // Optional: trigger the handler whenever this record changes. + TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); + } + +The handler obeys the same terminal-state rule as core handlers - every +code path must end in ``TSCfgLoadCtxComplete`` or ``TSCfgLoadCtxFail``. +Deferred completion (return from the callback, finish from another +thread, then call Complete or Fail there) is fully supported; see the +deferred-handler example in :doc:`api/functions/TSCfgRegister`. + +Plugin name attribution in ``traffic_ctl`` +------------------------------------------ + +Because the plugin's canonical name is attached automatically by the +framework, :option:`traffic_ctl config status` tags every plugin-owned +entry with ``[plugin: ]``. After a successful reload of the +example plugin above: + +.. code-block:: text + + $ traffic_ctl config reload + ✔ Reload scheduled [rldtk-1714061200] + + Monitor : traffic_ctl config reload -t rldtk-1714061200 -m + Details : traffic_ctl config reload -t rldtk-1714061200 -s -l + + $ traffic_ctl config status -t rldtk-1714061200 + ✔ Reload [success] — rldtk-1714061200 + Started : 2026 Apr 25 14:30:12.345 + Finished: 2026 Apr 25 14:30:12.349 + Duration: 4ms + + ✔ 1 success ◌ 0 in-progress ✗ 0 failed (1 total) + + Tasks: + ✔ my_plugin [plugin: my_plugin] ················ 4ms + [Note] Reloading my_plugin + [Note] Reloaded my_plugin + +When a plugin registers more than one entry under a single key (or +several plugins each register their own entries), the ``[plugin: ...]`` +tag makes ownership unambiguous. Entries owned by core code carry no +``[plugin: ...]`` tag. + +The same attribution is exposed under ``meta.plugin_name`` in the +JSONRPC :ref:`get_reload_config_status` response, so automation can +filter, group, or alarm on a per-plugin basis. + +Inline RPC reload of a plugin entry uses the registry key as the +top-level YAML node: + +.. code-block:: bash + + $ traffic_ctl config reload --content '{"my_plugin": {"rules": ["x", "y"]}}' + +The handler then calls ``TSCfgLoadCtxGetSuppliedYaml`` to read the +content and ``TSCfgLoadCtxGetReloadDirectives`` for any operator +directives passed via ``--directive``. + +Test plugins +------------ + +The autest suite ships small plugins that exercise the public +``TSCfg*`` surface end-to-end. They are the recommended reference for +how to wire registration, handler logic, and deferred completion: + +- ``tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc`` - basic + registration and synchronous handler. +- ``tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc`` - + reading inline YAML and reload directives. +- ``tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc`` - + asynchronous / deferred completion pattern. + +The matching autests +(``config_reload_plugin_api.test.py`` and friends in +``tests/gold_tests/jsonrpc/``) demonstrate driving these plugins via +:program:`traffic_ctl` and validating both the task tree and the +``[plugin: ]`` attribution. + +Reference +--------- + +:doc:`api/functions/TSCfgRegister` covers the full plugin-facing +surface: the :type:`TSCfgRegistrationInfo` options struct, the +registration / trigger / dependency / enable functions, and every +``TSCfgLoadCtx*`` operation available inside the handler callback. + + Thread Model ============ @@ -949,7 +1144,9 @@ line is logged to ``diags.log``: WARNING: Config reload [my-token] finished with failures: 1 succeeded, 1 failed (3 total) — run: traffic_ctl config status -t my-token When the ``config.reload`` debug tag is enabled, a detailed dump of all subtasks and their -log entries is written to ``traffic.out`` / ``diags.log``: +log entries is written to ``traffic.out`` / ``diags.log``. The same tag covers diagnostics +from the plugin-facing API (:func:`TSCfgRegister` and friends), so a single tag is enough to +trace the full reload pipeline end-to-end: .. code-block:: text diff --git a/include/mgmt/config/ConfigContext.h b/include/mgmt/config/ConfigContext.h index 725b0a2be49..4d2ebb24389 100644 --- a/include/mgmt/config/ConfigContext.h +++ b/include/mgmt/config/ConfigContext.h @@ -43,6 +43,10 @@ namespace config { class ConfigRegistry; } +namespace detail +{ +class RecordTriggeredReloadContinuation; +} /// /// @brief Context passed to config handlers during load/reload operations. @@ -58,7 +62,7 @@ class ConfigRegistry; /// At startup there is no active reload task, so all status operations /// (in_progress, complete, fail, log) are safe **no-ops**. To keep the /// existing code logic for loading/reloading this design aims to avoid -/// having two separate code paths for startup vs. reload — handlers +/// having two separate code paths for startup vs. reload - handlers /// can use the same API in both cases. /// /// Usage: @@ -92,7 +96,7 @@ class ConfigContext ~ConfigContext(); - // Copy only — move is intentionally suppressed. + // Copy only - move is intentionally suppressed. // ConfigContext holds a weak_ptr (cheap to copy) and a YAML::Node (ref-counted). // Suppressing move ensures that std::move(ctx) silently copies, keeping the // original valid. This is critical for execute_reload()'s post-handler check: @@ -166,6 +170,11 @@ class ConfigContext /// For dependent contexts it is the label passed to add_dependent_ctx(). [[nodiscard]] std::string get_description() const; + /// Get the reload token identifying the current reload cycle. + /// All tasks within the same reload share the same token. + /// Returns empty string for default-constructed (no-op) contexts. + [[nodiscard]] std::string get_reload_token() const; + /// Create a dependent sub-task that tracks progress independently under this parent. /// Each dependent reports its own status (in_progress/complete/fail) and the parent /// task aggregates them. The dependent context also inherits the parent's supplied YAML node. @@ -177,22 +186,27 @@ class ConfigContext /// @code /// if (auto yaml = ctx.supplied_yaml()) { /* use yaml node */ } /// @endcode - /// @return copy of the supplied YAML node (cheap — YAML::Node is internally reference-counted). + /// @return copy of the supplied YAML node (cheap - YAML::Node is internally reference-counted). [[nodiscard]] YAML::Node supplied_yaml() const; /// Get reload directives extracted from the _reload key. /// Directives are operational parameters that modify how the handler performs - /// the reload (e.g. scope to a single entry, dry-run) — distinct from config content. + /// the reload (e.g. scope to a single entry, dry-run) - distinct from config content. /// The framework extracts _reload from the supplied node before passing content /// to the handler, so supplied_yaml() never contains _reload. /// Returns Undefined when no directives were provided (operator bool() == false). /// @code /// if (auto directives = ctx.reload_directives()) { /* use directives */ } /// @endcode - /// @return copy of the directives YAML node (cheap — YAML::Node is internally reference-counted). + /// @return copy of the directives YAML node (cheap - YAML::Node is internally reference-counted). [[nodiscard]] YAML::Node reload_directives() const; private: + /// Attach the registering plugin's name. A non-empty name marks the context's + /// task as plugin-originated (is_plugin = true); an empty view marks it as + /// core. Used for diagnostics and traffic_ctl status attribution. + void set_plugin_name(std::string_view name); + /// Set supplied YAML node. Only ConfigRegistry should call this during reload setup. void set_supplied_yaml(YAML::Node node); @@ -205,6 +219,7 @@ class ConfigContext friend class ReloadCoordinator; friend class config::ConfigRegistry; + friend class detail::RecordTriggeredReloadContinuation; }; namespace config diff --git a/include/mgmt/config/ConfigRegistry.h b/include/mgmt/config/ConfigRegistry.h index 2ff1a8380f4..24b62eee9d5 100644 --- a/include/mgmt/config/ConfigRegistry.h +++ b/include/mgmt/config/ConfigRegistry.h @@ -60,8 +60,12 @@ enum class ConfigSource { FileAndRpc ///< Handler can also process YAML content supplied via RPC }; -/// Handler signature for config reload - receives ConfigContext -/// Handler can check ctx.supplied_yaml() for rpc-supplied content +/// Handler signature for config reload - receives ConfigContext by value. +/// Handler can check ctx.supplied_yaml() for rpc-supplied content. +/// +/// Plugin handlers use the same signature: the plugin C API layer wraps the +/// supplied context by value into its own TSCfgLoadCtx wrapper so the opaque +/// handle outlives this call (for deferred completion). using ConfigReloadHandler = std::function; /// @@ -104,19 +108,17 @@ class ConfigRegistry std::string filename_record; ///< Record containing filename (e.g., "proxy.config.cache.ip_allow.filename") ConfigType type; ///< YAML or LEGACY - we set that based on the filename extension. ConfigSource source{ConfigSource::FileOnly}; ///< What content sources this handler supports - ConfigReloadHandler handler; ///< Handler function (empty = static file/not reloadable) + ConfigReloadHandler handler; ///< Reload handler (empty for static / non-reloadable entries) std::vector trigger_records; ///< Records that trigger reload bool is_required{false}; ///< Whether the file must exist on disk + bool is_plugin{false}; ///< Registered by a plugin (via TSCfgRegister) + bool enabled{true}; ///< Runtime toggle - disabled entries are skipped during reload + /// Plugin that registered this entry (from TSPluginRegister). Empty for core entries. + /// Used in conflict diagnostics, reload-trace logs, and traffic_ctl status output. + std::string plugin_name; /// Resolve the actual filename (reads from record, falls back to default) std::string resolve_filename() const; - - /// Whether this entry has a reload handler (false for static/non-reloadable files). - bool - has_handler() const - { - return static_cast(handler); - } }; /// @@ -143,13 +145,28 @@ class ConfigRegistry ConfigReloadHandler handler, ConfigSource source, std::initializer_list trigger_records = {}, bool is_required = false); + /// @brief Register a plugin config file. + /// + /// Same as register_config() but marks the entry as plugin-originated so that + /// reload tracing and traffic_ctl distinguish plugin tasks from core tasks. + /// Called exclusively from the TSCfgRegister plugin API. + /// + /// @param plugin_name Name of the registering plugin (from TSPluginRegister). + /// Required: must be non-empty. Stored on the Entry for + /// conflict diagnostics, log attribution, and traffic_ctl + /// status display. + /// + void register_plugin_config(const std::string &key, const std::string &plugin_name, const std::string &default_filename, + const std::string &filename_record, ConfigReloadHandler handler, ConfigSource source, + std::initializer_list trigger_records = {}, bool is_required = false); + /// @brief Register a record-only config handler (no file). /// /// Convenience method for modules that have no config file but need their /// reload handler to participate in the config tracking system (tracing, /// status reporting, traffic_ctl config reload). /// - /// This is NOT for arbitrary record-change callbacks — use RecRegisterConfigUpdateCb + /// This is NOT for arbitrary record-change callbacks - use RecRegisterConfigUpdateCb /// for that. This is for config modules like SSLTicketKeyConfig that are reloaded /// via record changes and need visibility in the reload infrastructure. /// @@ -164,7 +181,7 @@ class ConfigRegistry /// @brief Register a non-reloadable config file (startup files). /// - /// Static files are registered for informational purposes only — no reload + /// Static files are registered for informational purposes only - no reload /// handler and no trigger records. This allows the registry to serve as the /// single source of truth for all known configuration files, so that RPC /// endpoints can gather and expose this information. @@ -190,6 +207,18 @@ class ConfigRegistry /// int attach(const std::string &key, const char *record_name); + /// + /// @brief Enable or disable a config entry at runtime + /// + /// When disabled, the handler is skipped during reloads. The entry + /// remains registered and visible in status, but marked as skipped. + /// + /// @param key The registered config key + /// @param enabled true to enable, false to disable + /// @return 0 on success, -1 if key not found + /// + int set_enabled(const std::string &key, bool enabled); + /// /// @brief Add a file dependency to an existing config /// @@ -247,6 +276,10 @@ class ConfigRegistry /// @param key The key to look up (can be a direct entry key or a dependency key) /// @return pair of {resolved_parent_key, entry_pointer} /// + /// The returned pointer follows the same stability contract as @c find: it + /// remains valid for the lifetime of the process while the registry stays + /// append-only. + /// std::pair resolve(const std::string &key) const; /// @@ -286,7 +319,11 @@ class ConfigRegistry /// void execute_reload(const std::string &key); - /// look up. + /// Look up an entry by key. + /// + /// The returned pointer is stable for the lifetime of the process: the registry + /// is append-only - there is no unregister API. If one is ever added, callers + /// of @c find / @c resolve must be revisited. bool contains(const std::string &key) const; Entry const *find(const std::string &key) const; @@ -311,9 +348,16 @@ class ConfigRegistry void setup_triggers(Entry &entry); /// Internal: wire a record callback to fire on_record_change for a config key. - /// Does NOT modify trigger_records — callers decide whether to store the record. + /// Does NOT modify trigger_records - callers decide whether to store the record. int wire_record_callback(const char *record_name, const std::string &config_key); + /// Internal: split the rpc-passed YAML into _reload directives and remaining + /// content, and apply both slices onto @p ctx (via the friend relationship + /// with ConfigContext). On invalid _reload (non-map), the directives are + /// dropped with a warning. @p passed_config is mutated (the "_reload" key + /// is stripped). + static void apply_passed_config(ConfigContext &ctx, YAML::Node &passed_config, std::string_view key); + /// Hash for lookup. struct StringHash { using is_transparent = void; diff --git a/include/mgmt/config/ConfigReloadTrace.h b/include/mgmt/config/ConfigReloadTrace.h index 7d9dadbca07..8e764979657 100644 --- a/include/mgmt/config/ConfigReloadTrace.h +++ b/include/mgmt/config/ConfigReloadTrace.h @@ -50,7 +50,7 @@ template struct convert; using ConfigReloadTaskPtr = std::shared_ptr; /// -/// @brief Progress checker for reload tasks — detects stuck/hanging tasks. +/// @brief Progress checker for reload tasks - detects stuck/hanging tasks. /// /// Created per-reload by ConfigReloadTask::start_progress_checker(), which is called /// only for main tasks in the IN_PROGRESS state. Each instance is bound to a single @@ -64,7 +64,7 @@ using ConfigReloadTaskPtr = std::shared_ptr; /// * The task exceeds the configured timeout (marked as TIMEOUT, then stops). /// * The _reload pointer is null (defensive). /// - Reschedules itself (EVENT_CONT) only while the task is still non-terminal. -/// - No idle polling — when no reload is in progress, no checker exists. +/// - No idle polling - when no reload is in progress, no checker exists. /// /// Configurable via records: /// - proxy.config.admin.reload.timeout: Duration string (default: "1h") @@ -129,7 +129,7 @@ class ConfigReloadTask : public std::enable_shared_from_this public: enum class State { INVALID = -1, - CREATED, ///< Initial state — task exists but not started + CREATED, ///< Initial state - task exists but not started IN_PROGRESS, ///< Work is actively happening SUCCESS, ///< Terminal: completed successfully FAIL, ///< Terminal: error occurred @@ -202,6 +202,9 @@ class ConfigReloadTask : public std::enable_shared_from_this std::string filename; ///< source file, if applicable std::vector sub_tasks; ///< child tasks (if any) bool main_task{false}; ///< true for the top-level reload task + bool is_plugin{false}; ///< registered by a plugin (via TSConfigRegister) + /// Plugin name supplied at registration (only meaningful when is_plugin is true; empty for core entries). + std::string plugin_name; }; using self_type = ConfigReloadTask; @@ -264,6 +267,17 @@ class ConfigReloadTask : public std::enable_shared_from_this _info.config_key = key; } + /// Set the registering plugin's name. Also flips @c is_plugin to (!name.empty()) + /// so callers don't need a separate boolean setter; pass an empty view for core + /// (non-plugin) entries. + void + set_plugin_name(std::string_view name) + { + std::unique_lock lock(_mutex); + _info.plugin_name = name; + _info.is_plugin = !name.empty(); + } + /// Check if any immediate subtask has the given config_key. /// Used by ReloadCoordinator to prevent duplicate subtasks within a single reload cycle. [[nodiscard]] bool has_subtask_for_key(std::string_view key) const; @@ -282,7 +296,7 @@ class ConfigReloadTask : public std::enable_shared_from_this } /// Get created time in seconds (for Date formatting and metrics). - /// No lock needed — created_time_ms is immutable after construction. + /// No lock needed - created_time_ms is immutable after construction. [[nodiscard]] std::time_t get_created_time() const { @@ -291,7 +305,7 @@ class ConfigReloadTask : public std::enable_shared_from_this } /// Get created time in milliseconds since epoch. - /// No lock needed — created_time_ms is immutable after construction. + /// No lock needed - created_time_ms is immutable after construction. [[nodiscard]] int64_t get_created_time_ms() const { diff --git a/include/records/YAMLConfigReloadTaskEncoder.h b/include/records/YAMLConfigReloadTaskEncoder.h index a86b863ba9d..697e37235dc 100644 --- a/include/records/YAMLConfigReloadTaskEncoder.h +++ b/include/records/YAMLConfigReloadTaskEncoder.h @@ -1,6 +1,6 @@ /** @file - YAML encoder for ConfigReloadTask::Info — serializes reload task snapshots to YAML nodes + YAML encoder for ConfigReloadTask::Info - serializes reload task snapshots to YAML nodes for JSONRPC responses. @section license License @@ -49,6 +49,9 @@ template <> struct convert { meta["created_time_ms"] = info.created_time_ms; meta["last_updated_time_ms"] = info.last_updated_time_ms; meta["main_task"] = info.main_task ? "true" : "false"; + if (!info.plugin_name.empty()) { + meta["plugin_name"] = info.plugin_name; + } node["meta"] = meta; diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index f458884779e..85b8eff50dc 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1591,6 +1591,130 @@ enum TSHttpCntlType { TS_HTTP_CNTL_SKIP_REMAPPING, }; +// Config reload registration interface. +using TSCfgLoadCtx = struct tsapi_cfgloadctx *; + +/// Plugin config load callback - invoked when the config file changes or a reload is requested. +/// The handler receives a TSCfgLoadCtx for status reporting and an opaque data pointer. +/// +/// Contract: +/// - The plugin MUST call exactly one of TSCfgLoadCtxComplete() / +/// TSCfgLoadCtxFail() on @a ctx, and on every handle returned by +/// TSCfgLoadCtxAddSubtask(). That call frees the handle. +/// - After the completing call returns, the handle is invalid: any +/// subsequent TSCfgLoadCtx* call on it (including Complete, Fail, +/// InProgress, AddLog, AddSubtask, getters) is a use-after-free. +/// - Work may be deferred to another thread/continuation: store the handle +/// and call Complete or Fail exactly once when done. Intermediate progress +/// or diagnostic messages should be published via TSCfgLoadCtxAddLog(). +/// - If the plugin never completes, the core reload framework still +/// unblocks the overall reload via its progress-timeout, but the +/// per-load handle leaks for the process lifetime. +using TSCfgLoadCb = void (*)(TSCfgLoadCtx ctx, void *data); + +/// Declares what content sources a plugin config handler supports. +enum TSCfgSourceType { + TS_CFG_SOURCE_FILE_ONLY, ///< Handler only reloads from file on disk + TS_CFG_SOURCE_FILE_AND_RPC ///< Handler can also process YAML content supplied via RPC +}; + +/// Options for TSCfgRegister(). +/// +/// Passed by const-pointer so future fields can be appended without breaking +/// source compatibility for existing plugins, provided new fields default to +/// a sane value. Populate the fields on a default-constructed instance: +/// +/// TSCfgRegistrationInfo info{}; +/// info.key = "my_config"; +/// info.config_path = "my_config.yaml"; +/// info.filename_record = "proxy.config.my_plugin.filename"; // optional +/// info.handler = my_reload_cb; +/// info.data = my_state; +/// info.source = TS_CFG_SOURCE_FILE_AND_RPC; +/// info.is_required = false; +/// TSCfgRegister(&info); +/// +/// The plugin name is taken automatically from the active TSPluginRegister() +/// call and is not part of this struct. +struct TSCfgRegistrationInfo { + /// Unique registry key. Also used as the YAML node name when an operator + /// reloads via `traffic_ctl config reload --content '{: {...}}'`. + /// Must be unique across all registered configs (core and plugin). + /// Convention: use the plugin name, or a "." prefix when + /// a plugin owns multiple configs. Required. + std::string_view key; + + /// Default config file path (absolute, or relative to sysconfdir). + /// Required. + std::string_view config_path; + + /// Fully-qualified record name that holds the active config filename, or + /// `{}` (default) if the path is fixed. When set, the operator can change + /// the active config file at runtime by editing this record; @c config_path + /// is used as the fallback when the record is empty or unset. Optional. + std::string_view filename_record; + + /// Reload callback invoked when the file changes, when an attached trigger + /// record changes, or when the operator issues an RPC reload. Required. + TSCfgLoadCb handler{nullptr}; + + /// Opaque pointer passed unmodified to @c handler on every invocation. The + /// plugin owns it - ATS does not copy or free it; must outlive the registration. + void *data{nullptr}; + + /// Whether @c handler can also process YAML content supplied via RPC. + TSCfgSourceType source{TS_CFG_SOURCE_FILE_ONLY}; + + /// True if the file must exist on disk for a reload to be considered + /// successful. False (default) lets the plugin tolerate a missing file. + bool is_required{false}; +}; + +/// Options for TSCfgAddFileDependency(). +/// +/// Only @c key and @c config_path are required. Adding fields in future +/// versions does not break source compatibility for existing plugins. +/// +/// Typical use: +/// +/// TSCfgFileDependencyInfo dep{}; +/// dep.key = "my_plugin"; +/// dep.config_path = "my_plugin_extras.yaml"; +/// TSCfgAddFileDependency(&dep); +/// +/// Composite plugin (RPC-routable child file): +/// +/// TSCfgFileDependencyInfo dep{}; +/// dep.key = "my_plugin"; +/// dep.config_path = "my_plugin_extras.yaml"; +/// dep.filename_record = "proxy.config.my_plugin.extras_filename"; +/// dep.dep_key = "my_plugin_extras"; +/// dep.is_required = false; +/// TSCfgAddFileDependency(&dep); +struct TSCfgFileDependencyInfo { + /// Parent registry key as passed to TSCfgRegister(). Required. + std::string_view key; + + /// Default companion file path (absolute, or relative to sysconfdir). + /// Required. + std::string_view config_path; + + /// Fully-qualified record name that holds the filename, or `{}` if the + /// path is fixed. Optional. + std::string_view filename_record; + + /// Routing key for inline YAML supplied via JSONRPC. When non-empty, + /// content delivered under this top-level node is routed to the parent + /// entry's handler, giving the plugin parity with core composite-config + /// patterns (see ConfigRegistry::add_file_and_node_dependency()). When + /// empty, the dependency is file-change-only. + std::string_view dep_key; + + /// True if the file must exist on disk for a reload to be considered + /// successful. False (default) lets the plugin tolerate a missing file. + bool is_required{false}; +}; + // JSONRPC 2.0 related interface. using TSRPCProviderHandle = struct tsapi_rpcproviderhandle *; using TSYaml = struct tsapi_yaml *; diff --git a/include/ts/ts.h b/include/ts/ts.h index 9b62d64fe8c..de11d73ef64 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1228,6 +1228,245 @@ void *TSConfigDataGet(TSConfig configp); TSReturnCode TSMgmtConfigFileAdd(const char *parent, const char *fileName); +/* -------------------------------------------------------------------------- + Config Registry - plugin config reload registration + + Registers a plugin config file with the centralized reload framework + (ConfigRegistry). Plugins registered here: + - Participate in traffic_ctl config reload + - Report success/failure visible via traffic_ctl config status + - Get automatic file-change detection via FileManager + - Can optionally receive RPC-supplied YAML content +*/ + +/** + Register a plugin config file with the reload framework. + + Must be called from TSPluginInit(), AFTER TSPluginRegister(). The plugin + name (set via TSPluginRegister) is automatically associated with the + registered entry for diagnostics, conflict reporting, and traffic_ctl + status output - plugins do not pass it explicitly. + + The struct shape (@see TSCfgRegistrationInfo) is used so that future ATS + versions can add fields without breaking source compatibility for existing + plugins. Pass the struct by pointer; null is rejected as invalid input. + + The @c key field plays a triple role: + - registry lookup key (unique across all registered configs), + - YAML node name for RPC-driven reload (`traffic_ctl config reload + --content '{: {...}}'`), + - human-readable label in diagnostics and status output. + By convention plugins use their own plugin name as the key (or a + "." prefix when a plugin owns multiple configs) to + avoid collisions with core entries (e.g. "ip_allow", "sni"). + + The optional @c filename_record field lets the operator override the + configured @c config_path at runtime via a record (e.g. + "proxy.config.my_plugin.filename"). When the record is empty or unset + the registered @c config_path is used as the fallback. + + @note Not supported for remap plugins (TSRemapInit / TSRemapNewInstance). + + @param info Registration parameters; required fields are @c key, + @c config_path, and @c handler. Must not be null. + + @return TS_SUCCESS once preconditions pass and registration was attempted. + TS_ERROR if: + - called outside TSPluginInit(), + - called before TSPluginRegister(), + - @a info is null or required fields are missing. + + @note Duplicate-key registrations are not signalled via the return value: + the framework logs a Warning identifying both the existing owner + and the new registrant, and the second registration is dropped + (one registration per key). This mirrors how core configs handle + the same situation. +*/ +TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info); + +/** + Attach a trigger record to a registered plugin config. When the record + value changes, the plugin's config handler is invoked (same as if the + file changed on disk). + + @note Must be called from TSPluginInit() after TSCfgRegister(). + + @param key The config key passed to TSCfgRegister(). + @param record_name Fully-qualified record name (e.g., "proxy.config.my_plugin.enabled"). + @return TS_SUCCESS on success, TS_ERROR if key not found or record invalid. +*/ +TSReturnCode TSCfgAttachTrigger(std::string_view key, std::string_view record_name); + +/** + Add a companion file dependency to a registered plugin config. When the + companion file changes on disk - or when an operator submits inline YAML + under @c info->dep_key via JSONRPC, when set - the plugin's config handler + is invoked. + + @note Must be called from TSPluginInit() after TSCfgRegister(). + + @param info Populated TSCfgFileDependencyInfo. Required fields are + @c key and @c config_path. + @return TS_SUCCESS on success. TS_ERROR if: + - called outside TSPluginInit() / before TSPluginRegister(), + - @a info is null or required fields are missing, + - the parent key has not been registered. +*/ +TSReturnCode TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info); + +/** + Enable or disable a registered config entry at runtime. + + When disabled, the handler is skipped during reloads. The entry + remains registered and visible in traffic_ctl config status, but + is marked as "skipped (disabled)". + + Unlike TSCfgRegister/TSCfgAttachTrigger/TSCfgAddFileDependency, this + function can be called at any time, not only during TSPluginInit(). + + @param key The config key passed to TSCfgRegister(). + @param enabled Non-zero to enable, zero to disable. + @return TS_SUCCESS on success, TS_ERROR if key not found. +*/ +TSReturnCode TSCfgSetEnabled(std::string_view key, int enabled); + +/** + Mark a load context as in-progress. + + Intended to be called at most once, near the start of the handler, to + transition the reload task from CREATED to IN_PROGRESS and optionally + attach a single startup message. Subsequent calls are safe (the state + transition is a no-op once already IN_PROGRESS) but discouraged: use + TSCfgLoadCtxAddLog() for additional progress or diagnostic messages. + + This call does NOT extend the reload timeout; long-running handlers + that defer past the configured reload timeout will still be marked + TIMEOUT by the core progress monitor. + + Null-safety: passing a nullptr @a ctx makes the call a silent no-op, + so handlers can share code between startup loading (no context) and + reload (real context). For string arguments, an empty std::string_view + means "no message"; do not construct a string_view from a null pointer. + + @param ctx Context handle, or nullptr for no-op. + @param msg Optional one-line progress message (logged to diags and load + status). Pass {} for no message. +*/ +void TSCfgLoadCtxInProgress(TSCfgLoadCtx ctx, std::string_view msg); + +/** + Report successful config load. Frees the context handle. + Do not use @a ctx after this call. + + Null-safe: calling with a nullptr @a ctx is a no-op. An empty @a msg + means "no message" - do not construct a string_view from a null pointer. + + @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. + @param msg Optional human-readable success message (logged to diags and + load status). Pass {} for no message. +*/ +void TSCfgLoadCtxComplete(TSCfgLoadCtx ctx, std::string_view msg); + +/** + Report failed config load. Frees the context handle. + Do not use @a ctx after this call. + + Null-safe: calling with a nullptr @a ctx is a no-op. An empty @a msg + means "no message" - do not construct a string_view from a null pointer. + + @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. + @param msg Optional human-readable failure message (logged to diags and + load status). Pass {} for no message. +*/ +void TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg); + +/** + Log an intermediate message to both diags and the reload task log, + without changing state. This is the preferred channel for progress + and diagnostic messages emitted between handler entry and the eventual + Complete/Fail call (including from deferred continuations); prefer this + over repeated TSCfgLoadCtxInProgress() calls. + + Null-safe: calling with a nullptr @a ctx or an empty @a msg is a no-op. + + @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. + @param level Severity level (DL_Note, DL_Warning, DL_Error, etc.). + @param msg Human-readable message; empty is a no-op. +*/ +void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg); + +/** + Create a dependent subtask under this context. The subtask tracks its own + status independently and aggregates into the parent's overall state. + The returned handle must be completed/failed independently of the parent. + + Null-safe: calling with a nullptr @a ctx returns nullptr. Since all + TSCfgLoadCtx* functions are also null-safe, the returned nullptr can be + used directly without checking. + + @param ctx Parent context handle, or nullptr. + @param description Label for the subtask (shown in traffic_ctl config status). + @return New context handle for the subtask, or nullptr. +*/ +TSCfgLoadCtx TSCfgLoadCtxAddSubtask(TSCfgLoadCtx ctx, std::string_view description); + +/** + Get the resolved config file path for this load. + + Null-safe: calling with nullptr returns an empty string_view. + + @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. + @return Path string. Valid until TSCfgLoadCtxComplete/Fail. The view + points into storage owned by the context handle; do not retain + past the completing call. The underlying buffer is + null-terminated, so .data() may be passed where a C string is + expected, but prefer string_view-aware APIs. +*/ +std::string_view TSCfgLoadCtxGetFilename(TSCfgLoadCtx ctx); + +/** + Get the reload token identifying the current reload cycle. + + All tasks within the same reload share the same token. Useful for + correlating log entries or diagnostics across handlers. + + Null-safe: calling with nullptr returns an empty string_view. + + @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. + @return Token string. Valid until TSCfgLoadCtxComplete/Fail. The view + points into storage owned by the context handle; do not retain + past the completing call. The underlying buffer is + null-terminated. +*/ +std::string_view TSCfgLoadCtxGetReloadToken(TSCfgLoadCtx ctx); + +/** + Get RPC-supplied YAML content for this load. + + For file-based reloads this returns nullptr. For RPC reloads (when the config + was registered with TS_CFG_SOURCE_FILE_AND_RPC), returns an opaque TSYaml + handle that can be cast to YAML::Node*. + + Null-safe: calling with nullptr returns nullptr. + + @param ctx Context handle from the TSCfgLoadCb callback. + @return TSYaml handle, or nullptr if this is a file-based reload. +*/ +TSYaml TSCfgLoadCtxGetSuppliedYaml(TSCfgLoadCtx ctx); + +/** Get reload directives extracted from the _reload key in RPC-supplied YAML. + Directives are operational parameters (e.g. scope) that modify how + the handler performs the reload - distinct from config content itself. + The framework strips _reload from the supplied node, so TSCfgLoadCtxGetSuppliedYaml + never contains _reload. + + Null-safe: calling with nullptr returns nullptr. + + @param ctx Context handle from the TSCfgLoadCb callback. + @return TSYaml handle, or nullptr if no directives were provided. +*/ +TSYaml TSCfgLoadCtxGetReloadDirectives(TSCfgLoadCtx ctx); + /* -------------------------------------------------------------------------- Management */ void TSMgmtUpdateRegister(TSCont contp, const char *plugin_name, const char *plugin_file_name = nullptr); diff --git a/plugins/regex_revalidate/regex_revalidate.cc b/plugins/regex_revalidate/regex_revalidate.cc index 5e8d8cf2f7b..4d7bc8b0ce2 100644 --- a/plugins/regex_revalidate/regex_revalidate.cc +++ b/plugins/regex_revalidate/regex_revalidate.cc @@ -558,25 +558,16 @@ free_handler(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata ATS_UN return 0; } -static int -config_handler(TSCont cont, TSEvent event, void * /* edata ATS_UNUSED */) +static bool +do_config_reload(plugin_state_t *pstate) { - plugin_state_t *pstate; - invalidate_t *i, *iptr; - TSCont free_cont; - bool updated; - TSMutex mutex; - - Dbg(dbg_ctl, "In config_handler"); + invalidate_t *i, *iptr; + TSCont free_cont; - mutex = TSContMutexGet(cont); - TSMutexLock(mutex); - - pstate = (plugin_state_t *)TSContDataGet(cont); - i = copy_config(pstate->invalidate_list); + i = copy_config(pstate->invalidate_list); - updated = prune_config(&i); - updated = load_config(pstate, &i) || updated; + bool updated = prune_config(&i); + updated = load_config(pstate, &i) || updated; if (updated) { list_config(pstate, i); @@ -587,19 +578,43 @@ config_handler(TSCont cont, TSEvent event, void * /* edata ATS_UNUSED */) TSContDataSet(free_cont, (void *)iptr); TSContScheduleOnPool(free_cont, FREE_TMOUT, TS_THREAD_POOL_TASK); } - } else { - Dbg(dbg_ctl, "No Changes"); - if (i) { - free_invalidate_t_list(i); - } + return true; } + Dbg(dbg_ctl, "No Changes"); + if (i) { + free_invalidate_t_list(i); + } + return false; +} + +static void +config_reload(TSCfgLoadCtx ctx, void *data) +{ + auto *pstate = static_cast(data); + + Dbg(dbg_ctl, "Config reload via ConfigRegistry"); + bool const updated = do_config_reload(pstate); + TSCfgLoadCtxComplete(ctx, updated ? "regex_revalidate config reloaded" : "regex_revalidate config unchanged"); +} + +static int +config_handler(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata ATS_UNUSED */) +{ + plugin_state_t *pstate; + TSMutex mutex; + + Dbg(dbg_ctl, "In config_handler (timed reload)"); + + mutex = TSContMutexGet(cont); + TSMutexLock(mutex); + + pstate = (plugin_state_t *)TSContDataGet(cont); + do_config_reload(pstate); + TSMutexUnlock(mutex); - // Don't reschedule for TS_EVENT_MGMT_UPDATE - if (event == TS_EVENT_TIMEOUT) { - TSContScheduleOnPool(cont, CONFIG_TMOUT, TS_THREAD_POOL_TASK); - } + TSContScheduleOnPool(cont, CONFIG_TMOUT, TS_THREAD_POOL_TASK); return 0; } @@ -842,12 +857,18 @@ TSPluginInit(int argc, const char *argv[]) TSContDataSet(main_cont, (void *)pstate); TSHttpHookAdd(TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, main_cont); - config_cont = TSContCreate(config_handler, TSMutexCreate()); - TSContDataSet(config_cont, (void *)pstate); - - TSMgmtUpdateRegister(config_cont, PLUGIN_NAME); + TSCfgRegistrationInfo cfg_info{}; + cfg_info.key = PLUGIN_NAME; + cfg_info.config_path = pstate->config_path; + cfg_info.handler = config_reload; + cfg_info.data = pstate; + cfg_info.source = TS_CFG_SOURCE_FILE_ONLY; + cfg_info.is_required = false; + TSCfgRegister(&cfg_info); // errors are logged inside TSCfgRegister if (!disable_timed_reload) { + config_cont = TSContCreate(config_handler, TSMutexCreate()); + TSContDataSet(config_cont, (void *)pstate); TSContScheduleOnPool(config_cont, CONFIG_TMOUT, TS_THREAD_POOL_TASK); } diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 2f62c45f755..0e4a3e05d5a 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -90,6 +90,9 @@ #include "proxy/http/HttpProxyServerMain.h" #include "shared/overridable_txn_vars.h" #include "mgmt/config/FileManager.h" +#include "mgmt/config/ConfigRegistry.h" +#include "mgmt/config/ConfigContext.h" +#include "mgmt/config/ConfigContextDiags.h" #include "mgmt/rpc/jsonrpc/JsonRPC.h" #include @@ -3299,6 +3302,367 @@ TSMgmtUpdateRegister(TSCont contp, const char *plugin_name, const char *plugin_f global_config_cbs->insert(reinterpret_cast(contp), plugin_name, plugin_file_name); } +//////////////////////////////////////////////////////////////////// +// +// Config Registry - plugin config reload registration +// +//////////////////////////////////////////////////////////////////// + +namespace +{ +// Handle shape behind the opaque TSCfgLoadCtx. +// +// Holds a value copy of the ConfigContext plus stable backing storage for the +// C getters. The wrapper lambda new's this before calling the plugin handler; +// TSCfgLoadCtx{Complete,Fail} delete it. Plugin must finalize exactly once per +// handle (parent + each subtask); if not, the task eventually TIMEOUTs and +// this wrapper leaks for the process lifetime. +struct PluginConfigContext { + ConfigContext ctx; + // Cached so the C-API getters can return stable pointers/views (ctx returns + // these by value). filename is resolved once at handle creation. + std::string filename; + std::string reload_token; + YAML::Node supplied_yaml; + YAML::Node reload_directives; + std::atomic consumed{false}; +}; + +DbgCtl dbg_ctl_plugin_config{"config.reload"}; +} // anonymous namespace + +TSReturnCode +TSCfgRegister(const TSCfgRegistrationInfo *info) +{ + // Precondition 1: must be inside TSPluginInit (loader sets plugin_reg_current there). + if (!plugin_reg_current) { + Error("[unknown-plugin] TSCfgRegister must be called from TSPluginInit()"); + return TS_ERROR; + } + // Precondition 2: TSPluginRegister must have been called first; we rely on + // its plugin_name to attribute the registered entry. plugin_registered is + // flipped only by TSPluginRegister, and plugin_name is non-null whenever + // the caller passed a non-null plugin_name in TSPluginRegistrationInfo. + if (!plugin_reg_current->plugin_registered || plugin_reg_current->plugin_name == nullptr) { + Error("[plugin %s] TSCfgRegister must be called after TSPluginRegister() with a non-null plugin_name", + plugin_reg_current->plugin_path ? plugin_reg_current->plugin_path : "?"); + return TS_ERROR; + } + + // From here on plugin_name is usable; capture it once for log prefixing and registration. + std::string plugin_name_str{plugin_reg_current->plugin_name}; + + // Precondition 3: input struct must be non-null and required fields populated. + if (info == nullptr) { + Error("[%s] TSCfgRegister: info pointer is null", plugin_name_str.c_str()); + return TS_ERROR; + } + if (info->key.empty() || info->config_path.empty() || info->handler == nullptr) { + Error("[%s] TSCfgRegister: missing required fields (key/config_path/handler) in TSCfgRegistrationInfo", + plugin_name_str.c_str()); + return TS_ERROR; + } + + std::string key_str{info->key}; + std::string config_path_str{info->config_path}; + std::string filename_record_str{info->filename_record}; + + config::ConfigSource cfg_source = + (info->source == TS_CFG_SOURCE_FILE_AND_RPC) ? config::ConfigSource::FileAndRpc : config::ConfigSource::FileOnly; + + auto cb = info->handler; + auto udata = info->data; + + config::ConfigReloadHandler wrapper = [cb, udata, k = key_str, p = plugin_name_str](ConfigContext ctx) { + auto *handle = new PluginConfigContext{}; + handle->reload_token = ctx.get_reload_token(); + handle->supplied_yaml = ctx.supplied_yaml(); + handle->reload_directives = ctx.reload_directives(); + if (auto const *entry = config::ConfigRegistry::Get_Instance().find(k); entry != nullptr) { + handle->filename = entry->resolve_filename(); + } + handle->ctx = std::move(ctx); + + Dbg(dbg_ctl_plugin_config, "[%s] Invoking plugin config handler for '%s'", p.c_str(), k.c_str()); + cb(reinterpret_cast(handle), udata); + // Do not dereference @p handle past this point - if the plugin completed + // synchronously it has already been deleted. Deferred-state detection is + // handled by ConfigRegistry::execute_reload via ctx.is_terminal(). If the + // plugin never completes, the handle leaks but core's progress timeout + // still unblocks the reload. + }; + + // Hand off to the registry. Duplicate-key detection lives inside do_register() + // (single critical section) and emits a Warning identifying both owners, + // identical to how core registration paths handle it. We support one + // registration per key; the plugin has no useful response to a duplicate, + // so we don't surface it via the return value. + config::ConfigRegistry::Get_Instance().register_plugin_config(key_str, plugin_name_str, config_path_str, filename_record_str, + std::move(wrapper), cfg_source, {}, info->is_required); + + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgRegister: registered '%s' (file: %s)", plugin_name_str.c_str(), key_str.c_str(), + config_path_str.c_str()); + return TS_SUCCESS; +} + +TSReturnCode +TSCfgAttachTrigger(std::string_view key, std::string_view record_name) +{ + if (!plugin_reg_current || plugin_reg_current->plugin_name == nullptr) { + return TS_ERROR; + } + + if (key.empty() || record_name.empty()) { + return TS_ERROR; + } + + char const *plugin_name = plugin_reg_current->plugin_name; + auto ®istry = config::ConfigRegistry::Get_Instance(); + std::string key_str{key}; + std::string record_str{record_name}; + + if (registry.attach(key_str, record_str.c_str()) != 0) { + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachTrigger: key '%s' not found", plugin_name, key_str.c_str()); + return TS_ERROR; + } + + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachTrigger: attached record '%s' to config '%s'", plugin_name, record_str.c_str(), + key_str.c_str()); + return TS_SUCCESS; +} + +TSReturnCode +TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info) +{ + if (!plugin_reg_current || plugin_reg_current->plugin_name == nullptr) { + return TS_ERROR; + } + + char const *plugin_name = plugin_reg_current->plugin_name; + + if (info == nullptr) { + Error("[%s] TSCfgAddFileDependency: info pointer is null", plugin_name); + return TS_ERROR; + } + if (info->key.empty() || info->config_path.empty()) { + Error("[%s] TSCfgAddFileDependency: missing required fields (key/config_path) in TSCfgFileDependencyInfo", plugin_name); + return TS_ERROR; + } + + auto ®istry = config::ConfigRegistry::Get_Instance(); + std::string key_str{info->key}; + std::string path_str{info->config_path}; + std::string filename_record_str{info->filename_record}; + std::string dep_key_str{info->dep_key}; + + // Branch on dep_key: empty -> file-change-only (today's behavior), + // non-empty -> also register the dependency as RPC-routable so inline + // YAML under that top-level node is delivered to the parent's handler. + int rc = + (dep_key_str.empty()) ? + registry.add_file_dependency(key_str, filename_record_str.c_str(), path_str.c_str(), info->is_required) : + registry.add_file_and_node_dependency(key_str, dep_key_str, filename_record_str.c_str(), path_str.c_str(), info->is_required); + + if (rc != 0) { + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAddFileDependency: key '%s' not found", plugin_name, key_str.c_str()); + return TS_ERROR; + } + + if (dep_key_str.empty()) { + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAddFileDependency: added file '%s' to config '%s'", plugin_name, path_str.c_str(), + key_str.c_str()); + } else { + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAddFileDependency: added file '%s' (dep_key '%s') to config '%s'", plugin_name, + path_str.c_str(), dep_key_str.c_str(), key_str.c_str()); + } + return TS_SUCCESS; +} + +TSReturnCode +TSCfgSetEnabled(std::string_view key, int enabled) +{ + if (key.empty()) { + Error("[unknown-plugin] TSCfgSetEnabled: empty key"); + return TS_ERROR; + } + + auto ®istry = config::ConfigRegistry::Get_Instance(); + std::string key_str{key}; + + // Resolve the owning plugin's name for log attribution. Unlike the other + // TSCfg* APIs, TSCfgSetEnabled may be called outside TSPluginInit, so + // plugin_reg_current is unreliable; pull the name from the registry entry. + char const *plugin_name = "unknown-plugin"; + if (auto const *entry = registry.find(key_str); entry != nullptr && !entry->plugin_name.empty()) { + plugin_name = entry->plugin_name.c_str(); + } + + if (registry.set_enabled(key_str, enabled != 0) != 0) { + Warning("[%s] TSCfgSetEnabled: key '%s' not found", plugin_name, key_str.c_str()); + return TS_ERROR; + } + + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgSetEnabled: config '%s' %s", plugin_name, key_str.c_str(), + enabled ? "enabled" : "disabled"); + return TS_SUCCESS; +} + +namespace +{ +// Finalize a plugin context: forward completion/failure to the underlying core +// ConfigContext, then delete the self-owned handle. The @c consumed flag short- +// circuits a second in-call entry; cross-thread post-delete double-calls are +// plugin-side UB and slip past us. After return, @p pctx is dangling. +void +finalize_plugin_ctx(PluginConfigContext *pctx, std::string_view msg, bool complete) +{ + if (pctx == nullptr) { + return; + } + if (pctx->consumed.exchange(true)) { + // In-call double-finalize. Cross-thread post-delete cases slip past us. + Warning("Plugin double-finalized TSCfgLoadCtx in-call (reload_token=%s); ignoring second call", pctx->reload_token.c_str()); + ink_assert(!"TSCfgLoadCtx finalized more than once"); + return; + } + + if (complete) { + if (!msg.empty()) { + CfgLoadComplete(pctx->ctx, "%.*s", static_cast(msg.size()), msg.data()); + } else { + pctx->ctx.complete(); + } + } else { + if (!msg.empty()) { + CfgLoadFail(pctx->ctx, "%.*s", static_cast(msg.size()), msg.data()); + } else { + pctx->ctx.fail(); + } + } + + delete pctx; +} +} // anonymous namespace + +void +TSCfgLoadCtxInProgress(TSCfgLoadCtx ctx, std::string_view msg) +{ + if (ctx == nullptr) { + return; + } + + auto *pctx = reinterpret_cast(ctx); + + if (!msg.empty()) { + CfgLoadInProgress(pctx->ctx, "%.*s", static_cast(msg.size()), msg.data()); + } else { + pctx->ctx.in_progress(); + } +} + +void +TSCfgLoadCtxComplete(TSCfgLoadCtx ctx, std::string_view msg) +{ + finalize_plugin_ctx(reinterpret_cast(ctx), msg, /*complete=*/true); +} + +void +TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg) +{ + finalize_plugin_ctx(reinterpret_cast(ctx), msg, /*complete=*/false); +} + +void +TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg) +{ + if (ctx == nullptr || msg.empty()) { + return; + } + + auto *pctx = reinterpret_cast(ctx); + CfgLoadLog(pctx->ctx, level, "%.*s", static_cast(msg.size()), msg.data()); +} + +TSCfgLoadCtx +TSCfgLoadCtxAddSubtask(TSCfgLoadCtx ctx, std::string_view description) +{ + if (ctx == nullptr) { + return nullptr; + } + + auto *pctx = reinterpret_cast(ctx); + + ConfigContext child_ctx = pctx->ctx.add_dependent_ctx(description); + if (!child_ctx) { + return nullptr; + } + + auto *child_handle = new PluginConfigContext{}; + child_handle->filename = pctx->filename; + child_handle->reload_token = pctx->reload_token; + child_handle->supplied_yaml = child_ctx.supplied_yaml(); + child_handle->reload_directives = child_ctx.reload_directives(); + child_handle->ctx = std::move(child_ctx); + + return reinterpret_cast(child_handle); +} + +std::string_view +TSCfgLoadCtxGetFilename(TSCfgLoadCtx ctx) +{ + if (ctx == nullptr) { + return {}; + } + + auto const *pctx = reinterpret_cast(ctx); + + return pctx->filename; +} + +std::string_view +TSCfgLoadCtxGetReloadToken(TSCfgLoadCtx ctx) +{ + if (ctx == nullptr) { + return {}; + } + + auto const *pctx = reinterpret_cast(ctx); + + return pctx->reload_token; +} + +TSYaml +TSCfgLoadCtxGetSuppliedYaml(TSCfgLoadCtx ctx) +{ + if (ctx == nullptr) { + return nullptr; + } + + auto *pctx = reinterpret_cast(ctx); + + if (pctx->supplied_yaml.IsDefined()) { + return reinterpret_cast(&pctx->supplied_yaml); + } + + return nullptr; +} + +TSYaml +TSCfgLoadCtxGetReloadDirectives(TSCfgLoadCtx ctx) +{ + if (ctx == nullptr) { + return nullptr; + } + + auto *pctx = reinterpret_cast(ctx); + + if (pctx->reload_directives.IsDefined()) { + return reinterpret_cast(&pctx->reload_directives); + } + + return nullptr; +} + TSReturnCode TSMgmtIntGet(const char *var_name, TSMgmtInt *result) { diff --git a/src/mgmt/config/ConfigContext.cc b/src/mgmt/config/ConfigContext.cc index 4604911b5b1..792da79ff3c 100644 --- a/src/mgmt/config/ConfigContext.cc +++ b/src/mgmt/config/ConfigContext.cc @@ -141,6 +141,15 @@ ConfigContext::get_description() const return {}; } +std::string +ConfigContext::get_reload_token() const +{ + if (auto p = _task.lock()) { + return p->get_token(); + } + return {}; +} + ConfigContext ConfigContext::add_dependent_ctx(std::string_view description) { @@ -162,6 +171,14 @@ ConfigContext::set_supplied_yaml(YAML::Node node) _supplied_yaml = node; // YAML::Node has no move semantics; copy is cheap (ref-counted). } +void +ConfigContext::set_plugin_name(std::string_view name) +{ + if (auto p = _task.lock()) { + p->set_plugin_name(name); + } +} + YAML::Node ConfigContext::supplied_yaml() const { diff --git a/src/mgmt/config/ConfigRegistry.cc b/src/mgmt/config/ConfigRegistry.cc index 776fa265d2f..de4ec6ab8e6 100644 --- a/src/mgmt/config/ConfigRegistry.cc +++ b/src/mgmt/config/ConfigRegistry.cc @@ -51,7 +51,7 @@ infer_config_type(swoc::TextView filename) // Resolve a config filename: read the current value from the named record, // fallback to default_filename if the record is empty or absent. -// Returns the bare filename (no sysconfdir prefix) — suitable for FileManager::addFile(). +// Returns the bare filename (no sysconfdir prefix) - suitable for FileManager::addFile(). std::string resolve_config_filename(const char *record_name, const std::string &default_filename) { @@ -88,6 +88,10 @@ class ScheduledReloadContinuation : public Continuation std::string _config_key; }; +} // anonymous namespace + +namespace detail +{ /// // Continuation used by record-triggered reloads (via on_record_change callback) // This is separate from ScheduledReloadContinuation as it always reloads from file @@ -109,20 +113,27 @@ class RecordTriggeredReloadContinuation : public Continuation if (entry == nullptr) { Warning("Config key '%s' not found in registry", _config_key.c_str()); - } else if (!entry->has_handler()) { + } else if (!entry->handler) { Warning("Config '%s' has no handler", _config_key.c_str()); + } else if (!entry->enabled) { + Dbg(dbg_ctl, "Config '%s' skipped (disabled)", _config_key.c_str()); + auto ctx = ReloadCoordinator::Get_Instance().create_config_context(_config_key, _config_key, entry->resolve_filename()); + if (ctx) { + ctx.set_plugin_name(entry->plugin_name); + ctx.complete("skipped (disabled)"); + } } else { auto ctx = ReloadCoordinator::Get_Instance().create_config_context(_config_key, _config_key, entry->resolve_filename()); if (!ctx) { if (ReloadCoordinator::Get_Instance().is_reload_in_progress()) { - // True duplicate — same config key already handled in this reload cycle Dbg(dbg_ctl, "Config '%s' reload skipped (duplicate in this reload cycle)", _config_key.c_str()); } else { - // Standalone record change (no active reload) — run handler directly Dbg(dbg_ctl, "Config '%s' standalone record-triggered reload (no active reload task)", _config_key.c_str()); + ctx.set_plugin_name(entry->plugin_name); entry->handler(ctx); } } else { + ctx.set_plugin_name(entry->plugin_name); ctx.in_progress(); entry->handler(ctx); Dbg(dbg_ctl, "Config '%s' file reload completed", _config_key.c_str()); @@ -136,7 +147,10 @@ class RecordTriggeredReloadContinuation : public Continuation private: std::string _config_key; }; +} // namespace detail +namespace +{ /// /// Callback invoked by the Records system when a trigger record changes. /// Only fires for records registered with ConfigRegistry (via trigger_records @@ -147,11 +161,11 @@ class RecordTriggeredReloadContinuation : public Continuation /// When a config key has N trigger records (e.g., ssl_client_coordinator has 11), /// setup_triggers() registers an independent on_record_change callback for each. /// The Records system (RecExecConfigUpdateCbs) fires all record -/// callbacks synchronously in one pass — N records produce N calls here, each +/// callbacks synchronously in one pass - N records produce N calls here, each /// scheduling its own RecordTriggeredReloadContinuation on ET_TASK. /// /// All N continuations carry the same config_key and would invoke the same handler. -/// The handler doesn't know which specific record triggered it — trigger records are +/// The handler doesn't know which specific record triggered it - trigger records are /// an OR-set meaning "any of these changed → reconfigure this subsystem." /// /// If different records need different handlers, register them under separate config keys. @@ -170,7 +184,7 @@ on_record_change(const char *name, RecDataT /* data_type */, RecData /* data */, ReloadCoordinator::Get_Instance().reserve_subtask(ctx->config_key); // Schedule file reload on ET_TASK thread (always file-based, no rpc-supplied content) - eventProcessor.schedule_imm(new RecordTriggeredReloadContinuation(ctx->mutex, ctx->config_key), ET_TASK); + eventProcessor.schedule_imm(new detail::RecordTriggeredReloadContinuation(ctx->mutex, ctx->config_key), ET_TASK); return 0; } @@ -202,10 +216,11 @@ ConfigRegistry::Entry::resolve_filename() const void ConfigRegistry::do_register(Entry entry) { - const char *type_str = (entry.type == ConfigType::YAML) ? "YAML" : "legacy"; + const char *type_str = (entry.type == ConfigType::YAML) ? "YAML" : "legacy"; + const char *owner_str = entry.is_plugin ? (entry.plugin_name.empty() ? "plugin:unknown" : entry.plugin_name.c_str()) : "core"; - Dbg(dbg_ctl, "Registering %s config '%s' (default: %s, record: %s, triggers: %zu)", type_str, entry.key.c_str(), - entry.default_filename.c_str(), entry.filename_record.empty() ? "" : entry.filename_record.c_str(), + Dbg(dbg_ctl, "Registering %s config '%s' [owner=%s] (default: %s, record: %s, triggers: %zu)", type_str, entry.key.c_str(), + owner_str, entry.default_filename.c_str(), entry.filename_record.empty() ? "" : entry.filename_record.c_str(), entry.trigger_records.size()); std::unique_lock lock(_mutex); @@ -221,10 +236,20 @@ ConfigRegistry::do_register(Entry entry) if (!it->second.default_filename.empty()) { auto resolved = resolve_config_filename(it->second.filename_record.empty() ? nullptr : it->second.filename_record.c_str(), it->second.default_filename); - FileManager::instance().addFile(resolved.c_str(), it->second.filename_record.c_str(), false, it->second.is_required); + // When filename_record is empty (e.g. plugin configs), pass the registry key + // as the configName so process_config_update can route mtime changes back to + // ConfigRegistry::schedule_reload(). + const char *config_name = it->second.filename_record.empty() ? it->second.key.c_str() : it->second.filename_record.c_str(); + FileManager::instance().addFile(resolved.c_str(), config_name, false, it->second.is_required); } } else { - Warning("Config '%s' already registered, ignoring", it->first.c_str()); + auto const &existing = it->second; + char const *existing_owner = + existing.is_plugin ? (existing.plugin_name.empty() ? "unknown-plugin" : existing.plugin_name.c_str()) : "core"; + char const *incoming_owner = + entry.is_plugin ? (entry.plugin_name.empty() ? "unknown-plugin" : entry.plugin_name.c_str()) : "core"; + Warning("Config '%s' already registered by %s; ignoring registration from %s", it->first.c_str(), existing_owner, + incoming_owner); } } @@ -249,6 +274,31 @@ ConfigRegistry::register_config(const std::string &key, const std::string &defau do_register(std::move(entry)); } +void +ConfigRegistry::register_plugin_config(const std::string &key, const std::string &plugin_name, const std::string &default_filename, + const std::string &filename_record, ConfigReloadHandler handler, ConfigSource source, + std::initializer_list trigger_records, bool is_required) +{ + ink_release_assert(!plugin_name.empty()); // contract; only TSCfgRegister calls this + + Entry entry; + entry.key = key; + entry.plugin_name = plugin_name; + entry.default_filename = default_filename; + entry.filename_record = filename_record; + entry.handler = std::move(handler); + entry.source = source; + entry.is_required = is_required; + entry.is_plugin = true; + entry.type = infer_config_type(default_filename); + + for (auto const *record : trigger_records) { + entry.trigger_records.emplace_back(record); + } + + do_register(std::move(entry)); +} + void ConfigRegistry::register_record_config(const std::string &key, ConfigReloadHandler handler, std::initializer_list trigger_records) @@ -260,7 +310,7 @@ void ConfigRegistry::register_static_file(const std::string &key, const std::string &default_filename, const std::string &filename_record, bool is_required) { - // Delegate — no handler, no trigger records, FileOnly source. + // Delegate - no handler, no trigger records, FileOnly source. register_config(key, default_filename, filename_record, nullptr, ConfigSource::FileOnly, {}, is_required); } @@ -275,7 +325,7 @@ ConfigRegistry::setup_triggers(Entry &entry) int ConfigRegistry::wire_record_callback(const char *record_name, const std::string &config_key) { - // TriggerContext lives for the lifetime of the process — intentionally not deleted + // TriggerContext lives for the lifetime of the process - intentionally not deleted // as RecRegisterConfigUpdateCb stores the pointer and may invoke the callback at any time. // This is a small, bounded allocation (one per trigger record). auto *ctx = new TriggerContext(); @@ -307,7 +357,7 @@ ConfigRegistry::attach(const std::string &key, const char *record_name) return -1; } - // Store record in entry — owned trigger + // Store record in entry - owned trigger it->second.trigger_records.emplace_back(record_name); config_key = it->second.key; } @@ -317,6 +367,21 @@ ConfigRegistry::attach(const std::string &key, const char *record_name) return wire_record_callback(record_name, config_key); } +int +ConfigRegistry::set_enabled(const std::string &key, bool enabled) +{ + std::unique_lock lock(_mutex); + auto it = _entries.find(key); + if (it == _entries.end()) { + Warning("Cannot set enabled on unknown config: %s", key.c_str()); + return -1; + } + + it->second.enabled = enabled; + Dbg(dbg_ctl, "Config '%s' %s", key.c_str(), enabled ? "enabled" : "disabled"); + return 0; +} + int ConfigRegistry::add_file_dependency(const std::string &key, const char *filename_record, const char *default_filename, bool is_required) @@ -333,40 +398,54 @@ ConfigRegistry::add_file_dependency(const std::string &key, const char *filename config_key = it->second.key; } - auto resolved = resolve_config_filename(filename_record, default_filename); + bool has_record = (filename_record != nullptr && filename_record[0] != '\0'); + auto resolved = resolve_config_filename(has_record ? filename_record : nullptr, default_filename); - Dbg(dbg_ctl, "Adding file dependency '%s' (resolved: %s) to config '%s'", filename_record, resolved.c_str(), key.c_str()); + Dbg(dbg_ctl, "Adding file dependency '%s' (resolved: %s) to config '%s'", has_record ? filename_record : "", + resolved.c_str(), key.c_str()); // Register with FileManager for mtime-based change detection. - // When rereadConfig() detects the file changed, it calls RecSetSyncRequired() - // on the filename_record, which triggers on_record_change below. - FileManager::instance().addFile(resolved.c_str(), filename_record, false, is_required); + // When filename_record is empty (e.g. plugin configs), pass the config key + // as the configName so process_config_update routes mtime changes back to + // ConfigRegistry::schedule_reload(). + const char *config_name = has_record ? filename_record : config_key.c_str(); + FileManager::instance().addFile(resolved.c_str(), config_name, false, is_required); + + if (has_record) { + return wire_record_callback(filename_record, config_key); + } - // Wire callback — dependency trigger, not stored in trigger_records. - return wire_record_callback(filename_record, config_key); + return 0; } int ConfigRegistry::add_file_and_node_dependency(const std::string &key, const std::string &dep_key, const char *filename_record, const char *default_filename, bool is_required) { - // Do the normal file dependency work (FileManager registration + record callback wiring) - int ret = add_file_dependency(key, filename_record, default_filename, is_required); - if (ret != 0) { - return ret; + // Claim the dep_key first so a collision can't leave FileManager half-registered. + { + std::unique_lock lock(_mutex); + if (_entries.find(key) == _entries.end()) { + Warning("Cannot add file dependency to unknown config: %s", key.c_str()); + return -1; + } + if (_entries.count(dep_key)) { + Warning("ConfigRegistry: dep_key '%s' collides with an existing entry key, ignoring", dep_key.c_str()); + return -1; + } + if (_dep_key_to_parent.count(dep_key)) { + Warning("ConfigRegistry: dep_key '%s' already registered, ignoring", dep_key.c_str()); + return -1; + } + _dep_key_to_parent[dep_key] = key; } - // Register the dep_key -> parent mapping for RPC routing - std::unique_lock lock(_mutex); - if (_entries.count(dep_key)) { - Warning("ConfigRegistry: dep_key '%s' collides with an existing entry key, ignoring", dep_key.c_str()); - return -1; - } - if (_dep_key_to_parent.count(dep_key)) { - Warning("ConfigRegistry: dep_key '%s' already registered, ignoring", dep_key.c_str()); - return -1; + if (int ret = add_file_dependency(key, filename_record, default_filename, is_required); ret != 0) { + std::unique_lock lock(_mutex); + _dep_key_to_parent.erase(dep_key); + return ret; } - _dep_key_to_parent[dep_key] = key; + Dbg(dbg_ctl, "Dependency key '%s' routes to parent '%s'", dep_key.c_str(), key.c_str()); return 0; } @@ -426,6 +505,28 @@ ConfigRegistry::schedule_reload(const std::string &key) eventProcessor.schedule_imm(new ScheduledReloadContinuation(mutex, key), ET_TASK); } +void +ConfigRegistry::apply_passed_config(ConfigContext &ctx, YAML::Node &passed_config, std::string_view key) +{ + // Extract _reload directives before passing content to the handler. This keeps + // supplied_yaml() clean (pure config data) and exposes reload_directives() as a + // separate accessor for operational parameters. + if (passed_config.IsMap() && passed_config["_reload"]) { + auto directives = passed_config["_reload"]; + if (!directives.IsMap()) { + Warning("Config '%.*s': _reload must be a YAML map, ignoring directives", static_cast(key.size()), key.data()); + } else { + Dbg(dbg_ctl, "Config '%.*s' has reload directives", static_cast(key.size()), key.data()); + ctx.set_reload_directives(directives); + } + passed_config.remove("_reload"); + } + + if (passed_config.size() > 0) { + ctx.set_supplied_yaml(passed_config); + } +} + void ConfigRegistry::execute_reload(const std::string &key) { @@ -452,49 +553,38 @@ ConfigRegistry::execute_reload(const std::string &key) } } - ink_release_assert(entry_copy.has_handler()); + ink_release_assert(entry_copy.handler); - // Create context with subtask tracking + // Create context with subtask tracking. // For rpc reload: use key as description, no filename (source: rpc) // For file reload: use key as description, filename indicates source: file std::string filename = has_passed_config ? "" : entry_copy.resolve_filename(); - auto ctx = ReloadCoordinator::Get_Instance().create_config_context(entry_copy.key, entry_copy.key, filename); + + auto ctx = ReloadCoordinator::Get_Instance().create_config_context(entry_copy.key, entry_copy.key, filename); + ctx.set_plugin_name(entry_copy.plugin_name); + + if (!entry_copy.enabled) { + Dbg(dbg_ctl, "Config '%s' skipped (disabled)", entry_copy.key.c_str()); + ctx.complete("skipped (disabled)"); + return; + } + ctx.in_progress(); if (has_passed_config) { Dbg(dbg_ctl, "Config '%s' reloading from rpc-supplied content", entry_copy.key.c_str()); - - // Extract _reload directives before passing content to the handler. - // This keeps supplied_yaml() clean (pure config data) and provides - // reload_directives() as a separate accessor for operational parameters. - if (passed_config.IsMap() && passed_config["_reload"]) { - auto directives = passed_config["_reload"]; - if (!directives.IsMap()) { - Warning("Config '%s': _reload must be a YAML map, ignoring directives", entry_copy.key.c_str()); - } else { - Dbg(dbg_ctl, "Config '%s' has reload directives", entry_copy.key.c_str()); - ctx.set_reload_directives(directives); - } - passed_config.remove("_reload"); - } - - // After stripping _reload, pass remaining content (if any) as supplied_yaml - if (passed_config.size() > 0) { - ctx.set_supplied_yaml(passed_config); - } + apply_passed_config(ctx, passed_config, entry_copy.key); } else { Dbg(dbg_ctl, "Config '%s' reloading from file '%s'", entry_copy.key.c_str(), filename.c_str()); } - // Handler checks ctx.supplied_yaml() for rpc-supplied content, otherwise reads from the - // module's known filename. + // Handler checks ctx.supplied_yaml() for rpc-supplied content, otherwise reads from + // the module's known filename. Plugin handlers run through the same path: the plugin + // layer wraps @c ctx by value into its own TSCfgLoadCtx handle for deferred completion. try { entry_copy.handler(ctx); - if (!ctx.is_terminal()) { // handler did not call ctx.complete() or ctx.fail(). It may have deferred work to another thread. - Warning("Config '%s' handler returned without reaching a terminal state. " - "If the handler deferred work to another thread, ensure ctx.complete() or ctx.fail() " - "is called when processing finishes; otherwise the task will remain in progress " - "until the timeout checker marks it as TIMEOUT.", + if (!ctx.is_terminal()) { + Warning("Config '%s' reload still in progress after handler return - deferred completion expected, otherwise will TIMEOUT", entry_copy.key.c_str()); } Dbg(dbg_ctl, "Config '%s' reload completed", entry_copy.key.c_str()); diff --git a/src/mgmt/config/FileManager.cc b/src/mgmt/config/FileManager.cc index 6928b40cc5e..fbec0f1c8ea 100644 --- a/src/mgmt/config/FileManager.cc +++ b/src/mgmt/config/FileManager.cc @@ -61,6 +61,9 @@ process_config_update(std::string const &fileName, std::string const &configName RecT rec_type; if (auto r = RecGetRecordType(configName.c_str(), &rec_type); r == REC_ERR_OKAY && rec_type == RECT_CONFIG) { RecSetSyncRequired(configName.c_str()); + } else if (config::ConfigRegistry::Get_Instance().contains(configName)) { + Dbg(dbg_ctl, "Config '%s' is a registry key (plugin config), scheduling reload directly", configName.c_str()); + config::ConfigRegistry::Get_Instance().schedule_reload(configName); } else { Dbg(dbg_ctl, "Couldn't set RecSetSyncRequired for %s - RecGetRecordType ret = %d", configName.c_str(), r); } diff --git a/src/records/unit_tests/test_ConfigRegistry.cc b/src/records/unit_tests/test_ConfigRegistry.cc index 29e26de9c40..8ddc0b15058 100644 --- a/src/records/unit_tests/test_ConfigRegistry.cc +++ b/src/records/unit_tests/test_ConfigRegistry.cc @@ -205,3 +205,64 @@ TEST_CASE("ConfigRegistry resolve() does not confuse entries and deps", "[config REQUIRE(key_b == "test_entry_b"); REQUIRE(entry_b->source == ConfigSource::FileAndRpc); } + +// ─── register_plugin_config: filename_record propagation ────────────────────── + +TEST_CASE("ConfigRegistry register_plugin_config stores filename_record on the entry", + "[config][registry][plugin][filename_record]") +{ + auto ® = ConfigRegistry::Get_Instance(); + + // Plugin path: register with a filename_record so an operator can override + // the active filename via that record at runtime. The plumbing tested here + // is the only difference between the plugin layer and core register_config: + // a typo in TSCfgRegister would silently fall back to default_filename. + reg.register_plugin_config("test_plugin_filename_record", "test_plugin", "default.yaml", "proxy.config.test_plugin.filename", + noop_handler, ConfigSource::FileOnly, {}, false); + + auto const *entry = reg.find("test_plugin_filename_record"); + REQUIRE(entry != nullptr); + CHECK(entry->key == "test_plugin_filename_record"); + CHECK(entry->plugin_name == "test_plugin"); + CHECK(entry->is_plugin); + CHECK(entry->default_filename == "default.yaml"); + CHECK(entry->filename_record == "proxy.config.test_plugin.filename"); + CHECK(entry->source == ConfigSource::FileOnly); + CHECK_FALSE(entry->is_required); +} + +TEST_CASE("ConfigRegistry add_file_and_node_dependency leaves no half-state on dep_key collision", "[config][registry][dependency]") +{ + ensure_test_records(); + auto ® = ConfigRegistry::Get_Instance(); + + reg.register_config("test_no_half_state", "", "", noop_handler, ConfigSource::FileAndRpc, {}); + + int ret1 = reg.add_file_and_node_dependency("test_no_half_state", "test_no_half_dep", "test.registry.dep.filename1", + "test_sni.yaml", false); + REQUIRE(ret1 == 0); + + // Duplicate dep_key: must fail without disturbing the live mapping. + int ret2 = reg.add_file_and_node_dependency("test_no_half_state", "test_no_half_dep", "test.registry.dep.filename1", + "test_sni.yaml", false); + REQUIRE(ret2 == -1); + + auto [parent_key, entry] = reg.resolve("test_no_half_dep"); + REQUIRE(entry != nullptr); + REQUIRE(parent_key == "test_no_half_state"); +} + +TEST_CASE("ConfigRegistry register_plugin_config accepts empty filename_record", "[config][registry][plugin][filename_record]") +{ + auto ® = ConfigRegistry::Get_Instance(); + + // Empty filename_record is the default for plugins that don't expose a + // runtime-tunable path; resolve_filename() then always uses default_filename. + reg.register_plugin_config("test_plugin_no_record", "test_plugin", "fixed.yaml", "", noop_handler, ConfigSource::FileOnly, {}, + false); + + auto const *entry = reg.find("test_plugin_no_record"); + REQUIRE(entry != nullptr); + CHECK(entry->filename_record.empty()); + CHECK(entry->default_filename == "fixed.yaml"); +} diff --git a/src/traffic_ctl/CtrlPrinters.cc b/src/traffic_ctl/CtrlPrinters.cc index 22b934179df..49563d0dca5 100644 --- a/src/traffic_ctl/CtrlPrinters.cc +++ b/src/traffic_ctl/CtrlPrinters.cc @@ -320,7 +320,10 @@ print_task_tree(const ConfigReloadResponse::ReloadInfo &f, bool full_report, con int dur_ms = duration_ms(f.meta.created_time_ms, f.meta.last_updated_time_ms); // Build label and right-aligned duration - std::string label = std::string(status_icon(f.status)) + " " + fname; + std::string label = std::string(status_icon(f.status)) + " " + fname; + if (!f.meta.plugin_name.empty()) { + label += " [plugin: " + f.meta.plugin_name + "]"; + } std::string dur_str = format_duration(dur_ms); // Right-pad duration to fixed width so values align @@ -355,7 +358,7 @@ print_task_tree(const ConfigReloadResponse::ReloadInfo &f, bool full_report, con std::cout << log_pfx; if (entry.level != DL_Undefined) { // Indexed by DiagsLevel enum. In practice only [Dbg], [Note], [Warn], [Err] appear - // in task logs — Fatal/Alert/Emergency terminate the process before any task completes. + // in task logs - Fatal/Alert/Emergency terminate the process before any task completes. static constexpr const char *severity_tags[] = { "[Diag] ", "[Dbg] ", "[Stat] ", "[Note] ", "[Warn] ", "[Err] ", "[Fatal] ", "[Alert] ", "[Emrg] ", }; diff --git a/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h b/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h index f3e51b77ce6..bd1b81d645a 100644 --- a/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h +++ b/src/traffic_ctl/jsonrpc/CtrlRPCRequests.h @@ -75,9 +75,10 @@ struct ConfigReloadResponse { std::vector logs; std::vector sub_tasks; struct Meta { // internal info. - int64_t created_time_ms{0}; - int64_t last_updated_time_ms{0}; - bool is_main_task{false}; + int64_t created_time_ms{0}; + int64_t last_updated_time_ms{0}; + bool is_main_task{false}; + std::string plugin_name; ///< Plugin name for plugin-owned entries (empty for core). } meta; }; diff --git a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h index 3f9ff1016fd..5dbcf3c2d66 100644 --- a/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h +++ b/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h @@ -121,6 +121,9 @@ template <> struct convert { info.meta.created_time_ms = helper::try_extract(meta, "created_time_ms"); info.meta.last_updated_time_ms = helper::try_extract(meta, "last_updated_time_ms"); info.meta.is_main_task = helper::try_extract(meta, "main_task"); + if (meta["plugin_name"]) { + info.meta.plugin_name = helper::try_extract(meta, "plugin_name"); + } } return info; }; diff --git a/tests/gold_tests/jsonrpc/config_reload_deferred.test.py b/tests/gold_tests/jsonrpc/config_reload_deferred.test.py new file mode 100644 index 00000000000..5e2a7d86d8e --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_deferred.test.py @@ -0,0 +1,158 @@ +''' +Test deferred TSCfgLoadCtx completion as part of a full config reload. + +The deferred plugin uses a two-stage schedule: + Stage 0: fires 3s after handler returns (simulates "reschedule later") + Stage 1: fires 2s after stage 0 (simulates "heavy work") + Total deferred time: ~5 seconds + +All tests touch config files to trigger a real full reload so every +registered config reloads alongside the deferred plugin. + +Scenarios: + A. Plugin registers successfully + B. Full reload — deferred success: in-progress immediately, success after ~5s + C. Full reload — deferred fail: rewrite config with "defer_fail", verify fail + D. Verify core tasks complete while plugin is still deferred +''' +# 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. + +import os +from jsonrpc import Request, Response + +Test.Summary = 'Test deferred two-stage TSCfgLoadCtx completion in full reload' +Test.ContinueOnFail = True + +ts = Test.MakeATSProcess('ts', dump_runroot=True, enable_tls=True) + +Test.testName = 'config_reload_deferred' + +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rpc|config|config.reload|cfg_plugin_deferred_test', + }) + +# Write initial valid plugin config file (no "defer_fail" -> success path) +ts.Disk.MakeConfigFile('cfg_plugin_deferred_test.conf').AddLines([ + 'mode: success', +]) + +# Load the deferred test plugin +Test.PrepareTestPlugin( + os.path.join(Test.Variables.AtsBuildGoldTestsDir, 'jsonrpc', 'plugins', '.libs', 'cfg_plugin_deferred_test.so'), ts, + 'cfg_plugin_deferred_test.conf') + +# ============================================================================ +# Test A: Plugin startup — TSCfgRegister +# ============================================================================ +tr = Test.AddTestRun("Plugin loads and registers") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "sleep 2" +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Init-API confirmation is emitted via Dbg() under the cfg_plugin_deferred_test tag, +# so it lands in traffic.out, not diags.log. +ts.Disk.traffic_out.Content = Testers.IncludesExpression('TSCfgRegister OK', 'TSCfgRegister should succeed') + +ts.Disk.diags_log.Content = All( + Testers.IncludesExpression( + r'Config reload \[full-deferred-ok\] completed', 'Full reload with deferred success should eventually complete'), + Testers.IncludesExpression( + r'Config reload \[full-deferred-fail\] finished with failures', + 'Full reload with deferred fail should eventually report failure'), +) + +# ============================================================================ +# Test B: Full reload — deferred success +# Touch config files to force them to be detected as changed, then trigger +# a normal reload. Don't touch logging.yaml (not present in sandbox). +# ============================================================================ +tr = Test.AddTestRun("Touch config files and trigger full reload (deferred success)") +tr.DelayStart = 2 +touch_cmd = ( + f'touch {ts.Variables.CONFIGDIR}/ip_allow.yaml ' + f'{ts.Variables.CONFIGDIR}/sni.yaml ' + f'{ts.Variables.CONFIGDIR}/cfg_plugin_deferred_test.conf ' + f'&& traffic_ctl config reload -t full-deferred-ok') +tr.Processes.Default.Command = touch_cmd +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Query immediately — plugin should be in-progress (core tasks may already be done) +tr = Test.AddTestRun("Verify deferred plugin is in-progress while core tasks complete") +tr.DelayStart = 1 +tr.Processes.Default.Command = "traffic_ctl config status -t full-deferred-ok" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('in-progress', 'Overall reload should still be in-progress'), + Testers.IncludesExpression('cfg_plugin_deferred_test', 'Plugin task should appear'), + Testers.IncludesExpression('deferring work', 'InProgress message from handler should be visible'), +) +tr.StillRunningAfter = ts + +# Wait for full deferred completion (~5s + margin) +tr = Test.AddTestRun("Verify full reload reached success after deferred completion") +tr.DelayStart = 12 +tr.Processes.Default.Command = "traffic_ctl config status -t full-deferred-ok" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('success', 'Overall reload should be success'), + Testers.IncludesExpression('deferred complete after heavy work', 'Plugin deferred Complete message should appear'), + Testers.IncludesExpression('stage 0', 'Stage 0 log should appear'), + Testers.IncludesExpression('stage 1', 'Stage 1 log should appear'), + # Core tasks should also appear as completed in the same reload + Testers.IncludesExpression('ip_allow', 'Core ip_allow task should appear in full reload'), + Testers.IncludesExpression('sni', 'Core sni task should appear in full reload'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test C: Full reload — deferred fail +# Rewrite the plugin config file with "defer_fail" so the deferred handler +# reads it and calls Fail. +# ============================================================================ +tr = Test.AddTestRun("Rewrite config with defer_fail and trigger full reload") +tr.DelayStart = 2 +fail_cmd = ( + f'echo "defer_fail: true" > {ts.Variables.CONFIGDIR}/cfg_plugin_deferred_test.conf ' + f'&& touch {ts.Variables.CONFIGDIR}/ip_allow.yaml ' + f'{ts.Variables.CONFIGDIR}/sni.yaml ' + f'&& traffic_ctl config reload -t full-deferred-fail') +tr.Processes.Default.Command = fail_cmd +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Wait for deferred fail (~5s + margin) +tr = Test.AddTestRun("Verify full reload reached fail after deferred failure") +tr.DelayStart = 12 +tr.Processes.Default.Command = "traffic_ctl config status -t full-deferred-fail" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('fail', 'Overall reload should show fail'), + Testers.IncludesExpression('deferred fail after heavy work', 'Plugin deferred Fail message should appear'), + Testers.IncludesExpression('heavy work failed', 'Stage 1 fail log should appear'), + # Core tasks should still have completed successfully despite plugin failure + Testers.IncludesExpression('ip_allow', 'Core ip_allow should appear even when plugin fails'), +) +tr.StillRunningAfter = ts diff --git a/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py b/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py new file mode 100644 index 00000000000..b92cbf25039 --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py @@ -0,0 +1,174 @@ +''' +Test TSCfgLoadCtxGetReloadDirectives plugin API — verifies that _reload +directives are correctly delivered to the plugin handler, separate from +the supplied config YAML content. + +Test scenarios: + A. Plugin startup: register succeeds + B. RPC reload with _reload directives (version) + config content (greeting) + C. File-based reload (touch config) — no directives + D. RPC reload with empty _reload directives — no version key +''' +# 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. + +import os +from jsonrpc import Request, Response + +Test.Summary = 'Test TSCfgLoadCtxGetReloadDirectives plugin API' +Test.ContinueOnFail = True + +ts = Test.MakeATSProcess('ts', dump_runroot=True) + +Test.testName = 'config_reload_directives_plugin' + +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rpc|config|config.reload|cfg_plugin_directives_test', + }) + +# Write initial valid plugin config file +ts.Disk.MakeConfigFile('cfg_plugin_directives_test.conf').AddLines([ + 'initial: config', +]) + +# Load the directives test plugin +Test.PrepareTestPlugin( + os.path.join(Test.Variables.AtsBuildGoldTestsDir, 'jsonrpc', 'plugins', '.libs', 'cfg_plugin_directives_test.so'), ts, + 'cfg_plugin_directives_test.conf') + +# ============================================================================ +# Test A: Plugin startup — TSCfgRegister succeeds +# ============================================================================ +tr = Test.AddTestRun("Plugin loads and registers") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "sleep 2" +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Init-API confirmation is emitted via Dbg() under the cfg_plugin_directives_test tag, +# so it lands in traffic.out, not diags.log. +ts.Disk.traffic_out.Content = Testers.IncludesExpression('TSCfgRegister OK', 'TSCfgRegister should succeed') + +ts.Disk.diags_log.Content = All( + Testers.IncludesExpression(r'Config reload \[rpc-with-directives\] completed', + 'Reload with directives should appear in diags'),) + +# ============================================================================ +# Test B: RPC reload with _reload directives + config content +# The framework extracts _reload into directives, remaining content becomes +# supplied_yaml. Plugin should see both directive_version and content_greeting. +# ============================================================================ +tr = Test.AddTestRun("RPC reload with _reload directives and config content") +tr.AddJsonRPCClientRequest( + ts, + Request.admin_config_reload( + token='rpc-with-directives', + configs={'cfg_plugin_directives_test': { + 'greeting': 'hello_directives', + '_reload': { + 'version': '2.0' + } + }})) + + +def validate_rpc_directives(resp: Response): + result = resp.result + errors = result.get('errors', []) + if errors: + return (False, f"Should accept RPC content: {errors}") + return (True, f"RPC with directives accepted: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_rpc_directives) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify directives and content in status") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-with-directives" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('directive_version=2.0', 'Plugin should see version directive'), + Testers.IncludesExpression('content_greeting=hello_directives', 'Plugin should see greeting in content'), + Testers.IncludesExpression('success', 'Should complete successfully'), + Testers.IncludesExpression('[plugin]', 'Should have plugin tag'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test C: File-based reload — no directives expected +# Touch the config file to trigger a file-based reload. +# ============================================================================ +tr = Test.AddTestRun("File-based reload (touch config) — no directives") +tr.DelayStart = 2 +touch_cmd = ( + f'touch {ts.Variables.CONFIGDIR}/cfg_plugin_directives_test.conf ' + f'&& traffic_ctl config reload -t file-no-directives') +tr.Processes.Default.Command = touch_cmd +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify file mode — no directives in status") +tr.DelayStart = 8 +tr.Processes.Default.Command = "traffic_ctl config status -t file-no-directives" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('no_directives', 'File-based reload should report no directives'), + Testers.IncludesExpression('file_mode', 'Should show file_mode path'), + Testers.IncludesExpression('success', 'Should complete successfully'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test D: RPC reload with empty _reload directives — no version key +# ============================================================================ +tr = Test.AddTestRun("RPC reload with empty _reload directives") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, + Request.admin_config_reload( + token='rpc-empty-directives', configs={'cfg_plugin_directives_test': { + 'greeting': 'empty_dir', + '_reload': {} + }}, force=True)) + + +def validate_empty_directives(resp: Response): + result = resp.result + errors = result.get('errors', []) + if errors: + return (False, f"Should accept RPC content: {errors}") + return (True, f"RPC with empty directives accepted: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_empty_directives) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify empty directives — version=none") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-empty-directives" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('directive_version=none', 'Empty directives should show version=none'), + Testers.IncludesExpression('content_greeting=empty_dir', 'Should see greeting content'), + Testers.IncludesExpression('success', 'Should complete successfully'), +) +tr.StillRunningAfter = ts diff --git a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py new file mode 100644 index 00000000000..bae3c232130 --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py @@ -0,0 +1,333 @@ +''' +Test the TSCfg* plugin config API end-to-end — all 10 public functions. + +Registration APIs (called in TSPluginInit): + 1. TSCfgRegister — plugin loads and registers + 2. TSCfgAttachTrigger — record change fires handler + 3. TSCfgAddFileDependency — companion file change fires handler + +Handler APIs (called during reload): + 4. TSCfgLoadCtxGetSuppliedYaml — RPC vs file detection + 5. TSCfgLoadCtxGetFilename — file path resolution + 6. TSCfgLoadCtxInProgress — in-progress marker (always called) + 7. TSCfgLoadCtxAddLog — intermediate log entries + 8. TSCfgLoadCtxComplete — success reporting + 9. TSCfgLoadCtxFail — failure reporting + 10. TSCfgLoadCtxAddSubtask — child subtask creation + +Test scenarios: + A. Plugin startup: register + attach trigger + add dependency + B. RPC reload — success with greeting + C. RPC reload — fail_on_purpose + D. RPC reload — with_subtask (parent + child both complete) + E. RPC reload — subtask_fail (child fails, parent completes) + F. Record-trigger reload (change proxy.config.http.insert_age_in_response) + G. Core tasks lack [plugin] tag +''' +# 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. + +import os +from jsonrpc import Request, Response + +Test.Summary = 'Test TSCfg* plugin config API: all 10 functions' +Test.ContinueOnFail = True + +ts = Test.MakeATSProcess('ts', dump_runroot=True) + +Test.testName = 'config_reload_plugin_api' + +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rpc|config|config.reload|cfg_plugin_test', + }) + +# Write initial valid plugin config file +plugin_config_file = os.path.join(ts.Variables.CONFIGDIR, 'cfg_plugin_test.conf') +ts.Disk.MakeConfigFile('cfg_plugin_test.conf').AddLines([ + 'greeting: hello', +]) + +# Write companion file for TSCfgAddFileDependency +companion_file = os.path.join(ts.Variables.CONFIGDIR, 'cfg_plugin_companion.conf') +ts.Disk.MakeConfigFile('cfg_plugin_companion.conf').AddLines([ + 'companion: data', +]) + +# Load the test plugin with main config + companion file +Test.PrepareTestPlugin( + os.path.join(Test.Variables.AtsBuildGoldTestsDir, 'jsonrpc', 'plugins', '.libs', 'cfg_plugin_test.so'), ts, + 'cfg_plugin_test.conf cfg_plugin_companion.conf') + +# ============================================================================ +# Test A: Plugin startup — TSCfgRegister + TSCfgAttachTrigger + TSCfgAddFileDependency +# ============================================================================ +tr = Test.AddTestRun("Plugin loads and registers all init APIs") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "sleep 2" +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# Init-API confirmations are emitted via Dbg() under the cfg_plugin_test tag, +# so they land in traffic.out, not diags.log. +ts.Disk.traffic_out.Content = All( + Testers.IncludesExpression('TSCfgRegister OK', 'TSCfgRegister should succeed'), + Testers.IncludesExpression('TSCfgAttachTrigger OK', 'TSCfgAttachTrigger should succeed'), + Testers.IncludesExpression('TSCfgAddFileDependency OK', 'TSCfgAddFileDependency should succeed'), +) + +ts.Disk.diags_log.Content = All( + # Reload summaries should land in diags.log (escape brackets for regex) + Testers.IncludesExpression(r'Config reload \[rpc-greet\] completed', 'Reload summary for rpc-greet should appear in diags'), + Testers.IncludesExpression( + r'Config reload \[rpc-fail\] finished with failures', 'Reload summary for rpc-fail should report failure in diags'), + Testers.IncludesExpression(r'Config reload \[rpc-subtask\] completed', 'Reload summary for rpc-subtask should appear in diags'), + Testers.IncludesExpression( + r'Config reload \[rpc-subtask-fail\] finished with failures', + 'Reload summary for rpc-subtask-fail should report failure in diags'), + Testers.IncludesExpression(r'Config reload \[core-check\] completed', 'Reload summary for core-check should appear in diags'), + Testers.ExcludesExpression('ignoring transition from', 'No terminal state conflicts'), +) + +# ============================================================================ +# Test B: RPC reload — greet (exercises GetSuppliedYaml, InProgress, AddLog, Complete) +# ============================================================================ +tr = Test.AddTestRun("RPC reload with greet key") +tr.AddJsonRPCClientRequest(ts, Request.admin_config_reload(token='rpc-greet', configs={'cfg_plugin_test': {'greet': 'world'}})) + + +def validate_rpc_greet(resp: Response): + result = resp.result + errors = result.get('errors', []) + if errors: + return (False, f"Should accept RPC content: {errors}") + return (True, f"RPC greet accepted: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_rpc_greet) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify greet reload status") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-greet" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('[plugin]', 'Plugin task should have [plugin] tag'), + Testers.IncludesExpression('greet=world', 'Should show greeting in status'), + Testers.IncludesExpression('success', 'Should show success'), + Testers.IncludesExpression('handler entered', 'TSCfgLoadCtxAddLog message should appear'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test C: RPC reload — fail_on_purpose (exercises Fail + AddLog) +# ============================================================================ +tr = Test.AddTestRun("RPC reload with fail_on_purpose") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, Request.admin_config_reload(token='rpc-fail', configs={'cfg_plugin_test': { + 'fail_on_purpose': True + }}, force=True)) + + +def validate_rpc_fail(resp: Response): + result = resp.result + errors = result.get('errors', []) + if errors: + error_str = str(errors) + if '6011' in error_str or '6010' in error_str: + return (False, f"Plugin should accept RPC content: {errors}") + return (True, f"RPC fail dispatched: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_rpc_fail) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify fail reload status") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-fail" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('[plugin]', 'Failed task should have [plugin] tag'), + Testers.IncludesExpression('fail', 'Should show fail state'), + Testers.IncludesExpression('fail requested', 'TSCfgLoadCtxAddLog error message should appear'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test D: RPC reload — with_subtask (exercises AddSubtask + InProgress on child) +# ============================================================================ +tr = Test.AddTestRun("RPC reload with subtask") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, Request.admin_config_reload(token='rpc-subtask', configs={'cfg_plugin_test': { + 'with_subtask': True + }}, force=True)) + + +def validate_rpc_subtask(resp: Response): + result = resp.result + return (True, f"RPC subtask dispatched: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_rpc_subtask) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify subtask in status tree") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-subtask" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('cfg_plugin_test', 'Parent task should appear'), + Testers.IncludesExpression('cfg_plugin_test_subtask', 'Child subtask should appear in tree'), + Testers.IncludesExpression('subtask done', 'Child should show completion message'), + Testers.IncludesExpression('subtask log entry', 'TSCfgLoadCtxAddLog on child should appear'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test E: RPC reload — subtask_fail (child fails, parent completes) +# ============================================================================ +tr = Test.AddTestRun("RPC reload with failing subtask") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, Request.admin_config_reload(token='rpc-subtask-fail', configs={'cfg_plugin_test': { + 'subtask_fail': True + }}, force=True)) + + +def validate_rpc_subtask_fail(resp: Response): + result = resp.result + return (True, f"RPC subtask-fail dispatched: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_rpc_subtask_fail) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify failing subtask in status tree") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-subtask-fail" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('cfg_plugin_test_failing_subtask', 'Failing subtask should appear'), + Testers.IncludesExpression('subtask failed on purpose', 'Subtask fail message should appear'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test F: Record-trigger reload (TSCfgAttachTrigger) +# Change proxy.config.http.insert_age_in_response to trigger handler +# ============================================================================ +tr = Test.AddTestRun("Trigger reload via record change (TSCfgAttachTrigger)") +tr.DelayStart = 2 +tr.Processes.Default.Command = "traffic_ctl config set proxy.config.http.insert_age_in_response 0" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Wait for record-triggered reload to complete") +tr.DelayStart = 8 +tr.Processes.Default.Command = "traffic_ctl config status -c all" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +# The record-triggered reload should show our plugin task +tr.Processes.Default.Streams.stdout = Testers.IncludesExpression( + 'cfg_plugin_test', 'Plugin handler should have been called by record trigger') +tr.StillRunningAfter = ts + +# ============================================================================ +# Test H: TSCfgSetEnabled — disable self, verify skipped, re-enable +# ============================================================================ + +# Step 1: Plugin disables itself (this reload still runs the handler) +tr = Test.AddTestRun("RPC reload: plugin disables itself") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, Request.admin_config_reload(token='rpc-disable-self', configs={'cfg_plugin_test': { + 'disable_self': True + }}, force=True)) + + +def validate_disable_self(resp: Response): + result = resp.result + return (True, f"disable_self dispatched: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_disable_self) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify disable_self completed") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-disable-self" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.IncludesExpression( + 'disabled self', 'Handler should have run and reported disabling itself') +tr.StillRunningAfter = ts + +# Step 2: Now the plugin is disabled — reload should skip it +tr = Test.AddTestRun("RPC reload while disabled: should be skipped") +tr.DelayStart = 2 +tr.AddJsonRPCClientRequest( + ts, + Request.admin_config_reload( + token='rpc-while-disabled', configs={'cfg_plugin_test': { + 'greet': 'should_not_appear' + }}, force=True)) + + +def validate_while_disabled(resp: Response): + result = resp.result + return (True, f"while-disabled dispatched: token={result.get('token', '')}") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_while_disabled) +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify skipped (disabled) in status") +tr.DelayStart = 5 +tr.Processes.Default.Command = "traffic_ctl config status -t rpc-while-disabled" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('skipped', 'Disabled plugin should show skipped'), + Testers.ExcludesExpression('should_not_appear', 'Handler should NOT have run'), +) +tr.StillRunningAfter = ts + +# ============================================================================ +# Test G: Core tasks should NOT have [plugin] tag +# ============================================================================ +tr = Test.AddTestRun("Full reload for core-check") +tr.DelayStart = 2 +tr.Processes.Default.Command = "traffic_ctl config reload -t core-check -F" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +tr = Test.AddTestRun("Verify core tasks lack [plugin] tag") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t core-check" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression( + 'ip_allow [plugin]', 'Core task ip_allow must not have [plugin] tag') +tr.StillRunningAfter = ts diff --git a/tests/gold_tests/jsonrpc/config_reload_ssl_bulk.test.py b/tests/gold_tests/jsonrpc/config_reload_ssl_bulk.test.py new file mode 100644 index 00000000000..684450b7198 --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_ssl_bulk.test.py @@ -0,0 +1,202 @@ +''' +Bulk SSL cert reload with rich status output. + +Starts ATS with a single default cert, then on reload pushes 20 certs +via ssl_multicert.yaml. Exercises all-success and partial-failure +scenarios to show per-cert detail in traffic_ctl and diags.log. +''' +# 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. + +import os + +Test.Summary = 'Bulk SSL cert reload — 20 certs, partial failure, recovery' +Test.ContinueOnFail = True + +NUM_CERTS = 20 +BAD_CERTS = { + 5: "garbage", # garbage PEM content + 9: "empty", # empty file + 13: "mismatch", # cert with wrong key + 17: "missing", # cert file deleted entirely +} + +ts = Test.MakeATSProcess("ts", enable_tls=True, dump_runroot=True) +ts.addDefaultSSLFiles() + +ssl_dir = ts.Variables.SSLDir +ssl_src_dir = os.path.join(ts.Variables.AtsTestToolsDir, "ssl") + +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': ssl_dir, + 'proxy.config.ssl.server.private_key.path': ssl_dir, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'config.reload', + }) + +# Startup with just the default cert — simple and reliable +ts.Disk.ssl_multicert_yaml.AddLines( + [ + 'ssl_multicert:', + ' - dest_ip: "*"', + ' ssl_cert_name: server.pem', + ' ssl_key_name: server.key', + ]) + +ts.Disk.sni_yaml.AddLines([ + 'sni:', + '- fqdn: "*.example.com"', + ' verify_client: NONE', +]) + +ts.Disk.parent_config.AddLine("# empty") +ts.Disk.cache_config.AddLine("# empty") + +multicert_path = ts.Disk.ssl_multicert_yaml.AbsRunTimePath +sni_path = ts.Disk.sni_yaml.AbsRunTimePath + +# Shell: create 20 cert/key copies + write a new ssl_multicert.yaml with all 20 +copy_cmds = " && ".join( + [ + f"cp {ssl_dir}/server.pem {ssl_dir}/cert-{i:02d}.pem && cp {ssl_dir}/server.key {ssl_dir}/cert-{i:02d}.key" + for i in range(1, NUM_CERTS + 1) + ]) + +# Build the 20-entry ssl_multicert.yaml content +multicert_content = "ssl_multicert:\\n" +for i in range(1, NUM_CERTS + 1): + multicert_content += f" - ssl_cert_name: cert-{i:02d}.pem\\n" + multicert_content += f" ssl_key_name: cert-{i:02d}.key\\n" + +# ============================================================================ +# Test 1: Start ATS with default single-cert config +# ============================================================================ +tr = Test.AddTestRun("Start ATS with default cert") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "sleep 3 && echo ATS started" +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 2: Create 20 certs, rewrite ssl_multicert.yaml, trigger reload +# ============================================================================ +tr = Test.AddTestRun("Push 20 certs and reload") +tr.Processes.Default.Command = ( + f'{copy_cmds}' + f' && printf "{multicert_content}" > {multicert_path}' + f' && touch {sni_path}' + f' && traffic_ctl config reload -t ssl-bulk-ok') +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 3: Verify all 20 certs loaded +# ============================================================================ +tr = Test.AddTestRun("Verify all 20 certs loaded") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-bulk-ok" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("FAIL", "Baseline should have no failures") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("ssl_client_coordinator", "Coordinator should appear") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("SSLCertificateConfig", "SSLCertificateConfig should appear") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "ssl_multicert.yaml finished loading", "ssl_multicert.yaml should finish loading") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 4: Break 4 certs in different ways, trigger reload +# ============================================================================ +break_cmds = [ + f'echo "GARBAGE_NOT_A_CERT" > {ssl_dir}/cert-05.pem', # garbage content + f'truncate -s 0 {ssl_dir}/cert-09.pem', # empty file + f'cp {ssl_dir}/cert-01.key {ssl_dir}/cert-13.key && touch {ssl_dir}/cert-13.pem', # key mismatch + f'rm -f {ssl_dir}/cert-17.pem', # missing file +] +tr = Test.AddTestRun("Break 4 certs in different ways and reload") +tr.Processes.Default.Command = ( + " && ".join(break_cmds) + f' && touch {multicert_path} {sni_path}' + f' && traffic_ctl config reload -t ssl-bulk-partial') +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 5: Verify partial failure — multiple cert errors, rest succeed +# ============================================================================ +tr = Test.AddTestRun("Verify partial failure — multiple bad certs") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-bulk-partial" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("FAIL", "Should show failure from bad certs") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("\\[Err\\]", "Error entries should carry [Err] tag") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("cert-05", "Garbage cert-05 should appear in error") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "ssl_multicert.yaml failed to load", "Overall multicert failure should appear") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 6: --min-level warning on partial failure +# ============================================================================ +tr = Test.AddTestRun("--min-level warning filters bulk Note entries") +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-bulk-partial --min-level warning" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("\\[Err\\]", "Error entries should pass filter") +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("\\[Note\\]", "Note entries should be filtered out") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 7: Fix all broken certs, trigger recovery +# ============================================================================ +fix_cmds = [ + f'cp {ssl_dir}/server.pem {ssl_dir}/cert-{i:02d}.pem && cp {ssl_dir}/server.key {ssl_dir}/cert-{i:02d}.key' for i in BAD_CERTS +] +tr = Test.AddTestRun("Fix all broken certs and recover") +tr.Processes.Default.Command = ( + " && ".join(fix_cmds) + f' && touch {multicert_path} {sni_path}' + f' && traffic_ctl config reload -t ssl-bulk-recover') +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 8: Verify recovery +# ============================================================================ +tr = Test.AddTestRun("Verify recovery — all certs load again") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-bulk-recover" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("FAIL", "Recovery should have no failures") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "ssl_multicert.yaml finished loading", "ssl_multicert.yaml should finish loading after recovery") +tr.StillRunningAfter = ts + +# ============================================================================ +# Global diags.log assertions +# ============================================================================ +ts.Disk.diags_log.Content = Testers.ContainsExpression( + "Config reload \\[ssl-bulk-ok\\] completed", "Baseline reload summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ContainsExpression( + "Config reload \\[ssl-bulk-partial\\] finished with failures", "Partial failure summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ContainsExpression( + "Config reload \\[ssl-bulk-recover\\] completed", "Recovery reload summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ExcludesExpression("ignoring transition from", "No conflicting terminal state transitions") diff --git a/tests/gold_tests/jsonrpc/config_reload_ssl_state.test.py b/tests/gold_tests/jsonrpc/config_reload_ssl_state.test.py new file mode 100644 index 00000000000..826123c3eb7 --- /dev/null +++ b/tests/gold_tests/jsonrpc/config_reload_ssl_state.test.py @@ -0,0 +1,180 @@ +''' +Test SSL coordinator reload state propagation. + +Exercises the ssl_client_coordinator tree (SSLConfig, SNIConfig, +SSLCertificateConfig) through success, partial failure (broken sni.yaml), +and recovery. Verifies: + - State propagation: child FAIL -> parent FAIL via aggregate_status + - Severity tags in traffic_ctl output ([Note], [Err]) + - Reload summary in diags.log + - Debug dump under config.reload tag with severity tags + - Recovery after fixing broken config +''' +# 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 = 'Test SSL coordinator reload state propagation and severity tags' +Test.ContinueOnFail = True + +ts = Test.MakeATSProcess("ts", enable_tls=True, dump_runroot=True) + +ts.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir, + 'proxy.config.ssl.server.private_key.path': ts.Variables.SSLDir, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'config.reload', + }) + +ts.addDefaultSSLFiles() + +ts.Disk.ssl_multicert_yaml.AddLines( + [ + 'ssl_multicert:', + ' - dest_ip: "*"', + ' ssl_cert_name: server.pem', + ' ssl_key_name: server.key', + ]) + +ts.Disk.sni_yaml.AddLines([ + 'sni:', + '- fqdn: "*.example.com"', + ' verify_client: NONE', +]) + +ssl_files_to_touch = [ts.Disk.sni_yaml, ts.Disk.ssl_multicert_yaml] +touch_ssl = "touch " + " ".join([f.AbsRunTimePath for f in ssl_files_to_touch]) + +# ============================================================================ +# Test 1: Start ATS, wait for it to settle +# ============================================================================ +tr = Test.AddTestRun("Start ATS") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "sleep 3 && echo 'ATS started'" +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 2: Touch SSL files + trigger baseline reload +# ============================================================================ +tr = Test.AddTestRun("Baseline reload - touch SSL files and reload") +tr.Processes.Default.Command = f"{touch_ssl} && traffic_ctl config reload -t ssl-baseline" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 3: Verify baseline succeeded +# ============================================================================ +tr = Test.AddTestRun("Verify baseline - all subtasks success") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-baseline" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("in_progress", "No task should remain in progress") +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("FAIL", "Baseline should have no failures") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "\\[Note\\]", "State-transition entries should carry [Note] severity tag") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "ssl_client_coordinator", "ssl_client_coordinator should appear in task tree") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("SSLConfig loading", "SSLConfig should show loading message") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("SSLConfig reloaded", "SSLConfig should show reloaded message") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 4: Break sni.yaml and trigger reload +# ============================================================================ +sni_path = ts.Disk.sni_yaml.AbsRunTimePath +multicert_path = ts.Disk.ssl_multicert_yaml.AbsRunTimePath +tr = Test.AddTestRun("Break sni.yaml and reload") +tr.Processes.Default.Command = ( + f'printf "sni:\\n- fqdn: example.com\\n client_cert: /nonexistent/bad.pem\\n client_key: /nonexistent/bad.key\\n verify_client: STRICT\\n" > {sni_path}' + f' && touch {multicert_path}' + f' && traffic_ctl config reload -t ssl-sni-fail') +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 5: Verify SNI failure propagates to ssl_client_coordinator +# ============================================================================ +tr = Test.AddTestRun("Verify SNI failure propagates to coordinator") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-sni-fail" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("FAIL", "Coordinator should propagate FAIL from SNIConfig") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("\\[Err\\]", "SNI failure should carry [Err] severity tag") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "SSLConfig reloaded", "SSLConfig should succeed even when SNI fails") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression("sni.yaml failed to load", "SNI failure message should appear") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 6: Verify --min-level warning filters out Note entries +# ============================================================================ +tr = Test.AddTestRun("Verify --min-level warning filters Note entries") +tr.Processes.Default.Command = ("traffic_ctl config status -t ssl-sni-fail --min-level warning") +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "\\[Err\\]", "Error entries should pass --min-level warning filter") +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression( + "\\[Note\\]", "Note entries should be filtered by --min-level warning") +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 7: Fix sni.yaml and trigger recovery reload +# ============================================================================ +tr = Test.AddTestRun("Fix sni.yaml and trigger recovery reload") +tr.Processes.Default.Command = ( + f'printf "sni:\\n- fqdn: \\"*.example.com\\"\\n verify_client: NONE\\n" > {sni_path}' + f' && touch {multicert_path}' + f' && traffic_ctl config reload -t ssl-recovery') +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.StillRunningAfter = ts + +# ============================================================================ +# Test 8: Verify full recovery +# ============================================================================ +tr = Test.AddTestRun("Verify full recovery - all subtasks success") +tr.DelayStart = 10 +tr.Processes.Default.Command = "traffic_ctl config status -t ssl-recovery" +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("FAIL", "Recovery reload should have no failures") +tr.Processes.Default.Streams.stdout += Testers.ExcludesExpression("in_progress", "No task should remain in progress after recovery") +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "\\[Note\\]", "Recovery entries should carry [Note] severity tags") +tr.StillRunningAfter = ts + +# ============================================================================ +# Global diags.log assertions +# ============================================================================ + +# Override default diags check — this test intentionally triggers SSL errors +ts.Disk.diags_log.Content = Testers.ContainsExpression( + "Config reload \\[ssl-baseline\\] completed", "Successful reload should produce a Note summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ContainsExpression( + "Config reload \\[ssl-sni-fail\\] finished with failures", "Failed reload should produce a Warning summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ContainsExpression( + "Config reload \\[ssl-recovery\\] completed", "Recovery reload should produce a Note summary in diags.log") + +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + "ignoring transition from", "No handler should have conflicting terminal state transitions") diff --git a/tests/gold_tests/jsonrpc/plugins/CMakeLists.txt b/tests/gold_tests/jsonrpc/plugins/CMakeLists.txt index b6f38c1417b..2bb9003e696 100644 --- a/tests/gold_tests/jsonrpc/plugins/CMakeLists.txt +++ b/tests/gold_tests/jsonrpc/plugins/CMakeLists.txt @@ -18,3 +18,12 @@ add_autest_plugin(jsonrpc_plugin_handler_test jsonrpc_plugin_handler_test.cc) target_link_libraries(jsonrpc_plugin_handler_test PRIVATE jsonrpc_protocol libswoc::libswoc) + +add_autest_plugin(cfg_plugin_test cfg_plugin_test.cc) +target_link_libraries(cfg_plugin_test PRIVATE yaml-cpp::yaml-cpp) + +add_autest_plugin(cfg_plugin_deferred_test cfg_plugin_deferred_test.cc) +target_link_libraries(cfg_plugin_deferred_test PRIVATE yaml-cpp::yaml-cpp) + +add_autest_plugin(cfg_plugin_directives_test cfg_plugin_directives_test.cc) +target_link_libraries(cfg_plugin_directives_test PRIVATE yaml-cpp::yaml-cpp) diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc new file mode 100644 index 00000000000..f88d2247734 --- /dev/null +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc @@ -0,0 +1,166 @@ +/** @file + + Test plugin for deferred TSCfgLoadCtx completion - handler stores the context + and completes/fails from a two-stage scheduled continuation on ET_TASK. + + Stage 0: fires 3 seconds after handler returns ("rescheduled work") + Stage 1: fires 2 seconds after stage 0 ("heavy work simulation") + Total deferred time: ~5 seconds + + Registration (TSPluginInit): + TSCfgRegister - registers "cfg_plugin_deferred_test" with FILE_AND_RPC source + + Handler (config_reload): + Stores TSCfgLoadCtx + behavior flag, schedules stage 0 at 3s, returns + WITHOUT calling Complete/Fail. + + Behavior selection: + RPC mode: {defer_fail: ...} → deferred fail, anything else → deferred success + File mode: file contains "defer_fail" → deferred fail, otherwise → deferred success + + @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 +#include +#include +#include +#include + +#include + +#define PLUGIN_NAME "cfg_plugin_deferred_test" + +namespace +{ +DbgCtl dbg_ctl{PLUGIN_NAME}; + +struct DeferredWork { + TSCfgLoadCtx ctx; + bool should_fail{false}; + int stage{0}; +}; + +int +deferred_handler(TSCont contp, TSEvent /* event */, void * /* edata */) +{ + auto *work = static_cast(TSContDataGet(contp)); + + if (work->stage == 0) { + Dbg(dbg_ctl, "Stage 0: deferred work starting after 3s wait"); + TSCfgLoadCtxAddLog(work->ctx, DL_Note, "cfg_plugin_deferred_test: stage 0 - deferred work starting, simulating heavy work"); + work->stage = 1; + TSContScheduleOnPool(contp, 2000, TS_THREAD_POOL_TASK); + return 0; + } + + Dbg(dbg_ctl, "Stage 1: heavy work done after 2s, completing"); + if (work->should_fail) { + TSCfgLoadCtxAddLog(work->ctx, DL_Error, "cfg_plugin_deferred_test: stage 1 - heavy work failed"); + TSCfgLoadCtxFail(work->ctx, "cfg_plugin_deferred_test: deferred fail after heavy work"); + } else { + TSCfgLoadCtxAddLog(work->ctx, DL_Note, "cfg_plugin_deferred_test: stage 1 - heavy work succeeded"); + TSCfgLoadCtxComplete(work->ctx, "cfg_plugin_deferred_test: deferred complete after heavy work"); + } + + delete work; + TSContDestroy(contp); + return 0; +} + +void +config_reload(TSCfgLoadCtx ctx, void * /* data */) +{ + TSCfgLoadCtxInProgress(ctx, "cfg_plugin_deferred_test: deferring work, will reschedule in 3s"); + TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_deferred_test: scheduling two-stage deferred completion (3s + 2s)"); + + bool should_fail = false; + + TSYaml yaml = TSCfgLoadCtxGetSuppliedYaml(ctx); + if (yaml != nullptr) { + auto *node = reinterpret_cast(yaml); + if ((*node)["defer_fail"]) { + should_fail = true; + } + } else { + std::string_view filename = TSCfgLoadCtxGetFilename(ctx); + if (!filename.empty()) { + std::ifstream file(std::string{filename}); + if (file.is_open()) { + std::ostringstream ss; + ss << file.rdbuf(); + std::string content = ss.str(); + if (content.find("defer_fail") != std::string::npos) { + should_fail = true; + } + } + } + } + + // NOTE: If ATS shuts down between this schedule and the deferred fire, + // `work` and `contp` leak. Acceptable for a test plugin (process is + // exiting) and intentional - tracking shutdown signals would add noise + // unrelated to what this test exercises. + auto *work = new DeferredWork{ctx, should_fail, 0}; + TSCont contp = TSContCreate(deferred_handler, TSMutexCreate()); + TSContDataSet(contp, work); + TSContScheduleOnPool(contp, 3000, TS_THREAD_POOL_TASK); + + Dbg(dbg_ctl, "Handler returning without Complete/Fail - stage 0 fires in 3s, stage 1 in 5s total (fail=%d)", should_fail); +} + +} // anonymous namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + return; + } + + const char *config_path = (argc >= 2) ? argv[1] : "cfg_plugin_deferred_test.conf"; + + std::string full_path; + if (config_path[0] == '/') { + full_path = config_path; + } else { + full_path = std::string(TSConfigDirGet()) + "/" + config_path; + } + + TSCfgRegistrationInfo cfg_info{}; + cfg_info.key = PLUGIN_NAME; + cfg_info.config_path = full_path; + cfg_info.handler = config_reload; + cfg_info.data = nullptr; + cfg_info.source = TS_CFG_SOURCE_FILE_AND_RPC; + cfg_info.is_required = false; + TSReturnCode ret = TSCfgRegister(&cfg_info); + if (ret == TS_SUCCESS) { + Dbg(dbg_ctl, "TSCfgRegister OK"); + } else { + TSError("[%s] TSCfgRegister FAILED", PLUGIN_NAME); + } +} diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc new file mode 100644 index 00000000000..3d725645ca1 --- /dev/null +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc @@ -0,0 +1,155 @@ +/** @file + + Test plugin for TSCfgLoadCtxGetReloadDirectives - verifies that _reload + directives are correctly extracted by the framework and delivered separately + from the supplied YAML config content. + + Registration (TSPluginInit): + TSCfgRegister - registers "cfg_plugin_directives_test" with FILE_AND_RPC source + + Handler (config_reload): + TSCfgLoadCtxGetReloadDirectives - read _reload directives (version key) + TSCfgLoadCtxGetSuppliedYaml - read config content (greeting key) + TSCfgLoadCtxGetFilename - get resolved file path (file mode) + TSCfgLoadCtxInProgress - mark task in-progress + TSCfgLoadCtxAddLog - add intermediate log entry + TSCfgLoadCtxComplete - report success + + Behavior: + RPC mode with directives: + _reload: {version: "X.Y"} → logs "directive_version=X.Y" + content: {greeting: "hi"} → logs "content_greeting=hi" + RPC mode without directives: + → logs "no_directives" + File mode: + → logs "file_mode" with byte count + + @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 +#include +#include +#include +#include + +#include + +#define PLUGIN_NAME "cfg_plugin_directives_test" + +namespace +{ +DbgCtl dbg_ctl{PLUGIN_NAME}; + +void +config_reload(TSCfgLoadCtx ctx, void * /* data */) +{ + TSCfgLoadCtxInProgress(ctx, PLUGIN_NAME ": processing started"); + + // --- Check for reload directives (_reload key, extracted by framework) --- + TSYaml directives_yaml = TSCfgLoadCtxGetReloadDirectives(ctx); + + if (directives_yaml != nullptr) { + auto *directives = reinterpret_cast(directives_yaml); + std::string version = "none"; + + if ((*directives)["version"]) { + version = (*directives)["version"].as(); + } + + TSCfgLoadCtxAddLog(ctx, DL_Note, std::string{PLUGIN_NAME ": directive_version="} + version); + Dbg(dbg_ctl, "Directives present: version=%s", version.c_str()); + } else { + TSCfgLoadCtxAddLog(ctx, DL_Note, PLUGIN_NAME ": no_directives"); + Dbg(dbg_ctl, "No directives present"); + } + + // --- Check for supplied YAML content (config body, without _reload) --- + TSYaml yaml = TSCfgLoadCtxGetSuppliedYaml(ctx); + + if (yaml != nullptr) { + auto *node = reinterpret_cast(yaml); + + std::string greeting = "none"; + if ((*node)["greeting"]) { + greeting = (*node)["greeting"].as(); + } + + TSCfgLoadCtxAddLog(ctx, DL_Note, std::string{PLUGIN_NAME ": content_greeting="} + greeting); + TSCfgLoadCtxComplete(ctx, PLUGIN_NAME ": RPC reload OK"); + return; + } + + // --- File mode fallback --- + std::string_view filename = TSCfgLoadCtxGetFilename(ctx); + if (!filename.empty()) { + std::ifstream file(std::string{filename}); + if (file.is_open()) { + std::ostringstream ss; + ss << file.rdbuf(); + std::string content = ss.str(); + std::string msg = std::string{PLUGIN_NAME ": file_mode ("} + std::to_string(content.size()) + " bytes)"; + TSCfgLoadCtxAddLog(ctx, DL_Note, msg); + TSCfgLoadCtxComplete(ctx, msg); + return; + } + } + + TSCfgLoadCtxComplete(ctx, PLUGIN_NAME ": reload OK (no content)"); +} + +} // anonymous namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + return; + } + + const char *config_path = (argc >= 2) ? argv[1] : "cfg_plugin_directives_test.conf"; + + std::string full_path; + if (config_path[0] == '/') { + full_path = config_path; + } else { + full_path = std::string(TSConfigDirGet()) + "/" + config_path; + } + + TSCfgRegistrationInfo cfg_info{}; + cfg_info.key = PLUGIN_NAME; + cfg_info.config_path = full_path; + cfg_info.handler = config_reload; + cfg_info.data = nullptr; + cfg_info.source = TS_CFG_SOURCE_FILE_AND_RPC; + cfg_info.is_required = false; + TSReturnCode ret = TSCfgRegister(&cfg_info); + if (ret == TS_SUCCESS) { + Dbg(dbg_ctl, "TSCfgRegister OK"); + } else { + TSError("[%s] TSCfgRegister FAILED", PLUGIN_NAME); + } +} diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc new file mode 100644 index 00000000000..05a9d56a538 --- /dev/null +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc @@ -0,0 +1,227 @@ +/** @file + + Test plugin for TSCfg* plugin config API - exercises every public function. + + Registration (TSPluginInit): + TSCfgRegister - registers "cfg_plugin_test" with FILE_AND_RPC source + TSCfgAttachTrigger - attaches a record trigger + TSCfgAddFileDependency - adds a companion file dependency + + Handler (config_reload): + TSCfgLoadCtxGetSuppliedYaml - detect RPC vs file mode + TSCfgLoadCtxGetFilename - get resolved file path + TSCfgLoadCtxInProgress - mark task in-progress + TSCfgLoadCtxAddLog - add intermediate log entry + TSCfgLoadCtxAddSubtask - create child subtask + TSCfgLoadCtxComplete - report success + TSCfgLoadCtxFail - report failure + + Behavior is driven by supplied YAML keys: + {greet: } - success with greeting message + {fail_on_purpose: ...} - handler reports failure + {with_subtask: ...} - creates a subtask, completes both + {subtask_fail: ...} - creates a subtask that fails + (no YAML / file mode) - reads file, fails if "fail_on_purpose" in content + + @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 +#include +#include +#include +#include + +#include + +#define PLUGIN_NAME "cfg_plugin_test" + +namespace +{ +DbgCtl dbg_ctl{PLUGIN_NAME}; + +struct PluginState { + std::string config_path; +}; + +void +config_reload(TSCfgLoadCtx ctx, void *data) +{ + auto *state = static_cast(data); + + // --- TSCfgLoadCtxInProgress: mark in-progress with a message --- + TSCfgLoadCtxInProgress(ctx, "cfg_plugin_test: processing started"); + + // --- TSCfgLoadCtxAddLog: add an intermediate log entry --- + TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_test: handler entered"); + + // --- TSCfgLoadCtxGetSuppliedYaml: detect RPC vs file mode --- + TSYaml yaml = TSCfgLoadCtxGetSuppliedYaml(ctx); + + if (yaml != nullptr) { + auto *node = reinterpret_cast(yaml); + + // --- fail_on_purpose: test TSCfgLoadCtxFail --- + if ((*node)["fail_on_purpose"]) { + TSCfgLoadCtxAddLog(ctx, DL_Error, "cfg_plugin_test: fail requested"); + TSCfgLoadCtxFail(ctx, "cfg_plugin_test: fail_on_purpose via RPC"); + return; + } + + // --- with_subtask: test TSCfgLoadCtxAddSubtask + complete both --- + if ((*node)["with_subtask"]) { + TSCfgLoadCtx child = TSCfgLoadCtxAddSubtask(ctx, "cfg_plugin_test_subtask"); + TSCfgLoadCtxInProgress(child, "subtask working"); + TSCfgLoadCtxAddLog(child, DL_Note, "cfg_plugin_test: subtask log entry"); + TSCfgLoadCtxComplete(child, "cfg_plugin_test: subtask done"); + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: parent with subtask done"); + return; + } + + // --- subtask_fail: test subtask that fails --- + if ((*node)["subtask_fail"]) { + TSCfgLoadCtx child = TSCfgLoadCtxAddSubtask(ctx, "cfg_plugin_test_failing_subtask"); + TSCfgLoadCtxInProgress(child, "subtask starting"); + TSCfgLoadCtxFail(child, "cfg_plugin_test: subtask failed on purpose"); + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: parent ok but subtask failed"); + return; + } + + // --- disable_self: test TSCfgSetEnabled(key, 0) --- + if ((*node)["disable_self"]) { + TSCfgSetEnabled(PLUGIN_NAME, 0); + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: disabled self"); + return; + } + + // --- enable_self: test TSCfgSetEnabled(key, 1) --- + if ((*node)["enable_self"]) { + TSCfgSetEnabled(PLUGIN_NAME, 1); + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: re-enabled self"); + return; + } + + // --- greet: test TSCfgLoadCtxComplete with message --- + if ((*node)["greet"]) { + std::string greet = (*node)["greet"].as(); + TSCfgLoadCtxComplete(ctx, std::string{"cfg_plugin_test: RPC reload OK, greet="} + greet); + return; + } + + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: RPC reload OK (no special keys)"); + return; + } + + // --- File mode: TSCfgLoadCtxGetFilename --- + std::string_view filename = TSCfgLoadCtxGetFilename(ctx); + std::string filename_str{filename.empty() ? state->config_path : std::string{filename}}; + + TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_test: reading file " + filename_str); + + std::string content; + std::ifstream file(filename_str); + if (file.is_open()) { + std::ostringstream ss; + ss << file.rdbuf(); + content = ss.str(); + } else { + TSCfgLoadCtxFail(ctx, "cfg_plugin_test: cannot open config file: " + filename_str); + return; + } + + if (content.find("fail_on_purpose") != std::string::npos) { + TSCfgLoadCtxFail(ctx, "cfg_plugin_test: fail_on_purpose in file"); + return; + } + + TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: file reload OK (" + std::to_string(content.size()) + " bytes)"); +} + +} // anonymous namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + return; + } + + if (argc < 2) { + TSError("[%s] Usage: cfg_plugin_test.so [companion_file]", PLUGIN_NAME); + return; + } + + static PluginState state; + + if (argv[1][0] == '/') { + state.config_path = argv[1]; + } else { + state.config_path = std::string(TSConfigDirGet()) + "/" + argv[1]; + } + + // --- TSCfgRegister --- + TSCfgRegistrationInfo cfg_info{}; + cfg_info.key = PLUGIN_NAME; + cfg_info.config_path = state.config_path; + cfg_info.handler = config_reload; + cfg_info.data = &state; + cfg_info.source = TS_CFG_SOURCE_FILE_AND_RPC; + cfg_info.is_required = false; + TSReturnCode ret = TSCfgRegister(&cfg_info); + if (ret == TS_SUCCESS) { + Dbg(dbg_ctl, "TSCfgRegister OK for '%s'", state.config_path.c_str()); + } else { + TSError("[%s] TSCfgRegister FAILED for '%s'", PLUGIN_NAME, state.config_path.c_str()); + return; + } + + // --- TSCfgAttachTrigger: attach a record so changing it fires our handler --- + ret = TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.http.insert_age_in_response"); + if (ret == TS_SUCCESS) { + Dbg(dbg_ctl, "TSCfgAttachTrigger OK"); + } else { + TSError("[%s] TSCfgAttachTrigger FAILED", PLUGIN_NAME); + } + + // --- TSCfgAddFileDependency: add companion file --- + if (argc >= 3) { + std::string companion; + if (argv[2][0] == '/') { + companion = argv[2]; + } else { + companion = std::string(TSConfigDirGet()) + "/" + argv[2]; + } + TSCfgFileDependencyInfo dep{}; + dep.key = PLUGIN_NAME; + dep.config_path = companion; + ret = TSCfgAddFileDependency(&dep); + if (ret == TS_SUCCESS) { + Dbg(dbg_ctl, "TSCfgAddFileDependency OK for '%s'", companion.c_str()); + } else { + TSError("[%s] TSCfgAddFileDependency FAILED for '%s'", PLUGIN_NAME, companion.c_str()); + } + } +} From cb3ae7499cfca0efca4a466e24fb7f98f6773e34 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Thu, 7 May 2026 15:04:24 +0200 Subject: [PATCH 2/6] cleanup and some renaming --- .../api/functions/TSCfgRegister.en.rst | 92 ++++++++++++++----- .../config-reload-framework.en.rst | 7 +- include/mgmt/config/ConfigContext.h | 4 +- include/mgmt/config/ConfigRegistry.h | 18 +--- include/mgmt/config/ConfigReloadTrace.h | 12 +-- include/ts/apidefs.h.in | 27 +++++- include/ts/ts.h | 35 +++---- src/api/InkAPI.cc | 53 ++++------- src/mgmt/config/ConfigRegistry.cc | 39 +------- src/records/unit_tests/test_ConfigRegistry.cc | 2 +- .../jsonrpc/config_reload_plugin_api.test.py | 72 ++------------- .../plugins/cfg_plugin_deferred_test.cc | 9 +- .../plugins/cfg_plugin_directives_test.cc | 8 +- .../jsonrpc/plugins/cfg_plugin_test.cc | 32 ++----- 14 files changed, 168 insertions(+), 242 deletions(-) diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst index d509ea9363a..e4e0b27a591 100644 --- a/doc/developer-guide/api/functions/TSCfgRegister.en.rst +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -99,8 +99,12 @@ Synopsis .. var:: bool is_required - ``true`` if the file must exist on disk for a reload to be - considered successful. Defaults to ``false``. + Hint propagated to FileManager: marks the file as "required" in + catalog/inspection output and emits a ``Dbg`` line if the file + is missing at registration time. The framework does **not** + enforce this flag at reload-time today: the handler is still + invoked even when the file is missing or invalid, and the + plugin chooses whether to fail the reload. Defaults to ``false``. .. type:: TSCfgFileDependencyInfo @@ -131,8 +135,9 @@ Synopsis .. var:: bool is_required - ``true`` if the file must exist on disk for a reload to be - considered successful. Defaults to ``false``. + Hint propagated to FileManager (catalog/inspection only); see the + equivalent field on :type:`TSCfgRegistrationInfo`. Defaults to + ``false``. .. type:: TSCfgLoadCtx @@ -147,14 +152,35 @@ Synopsis Plugin reload callback signature. ``data`` is the opaque pointer the plugin supplied via :var:`TSCfgRegistrationInfo::data`. +.. enum:: TSCfgLogLevel + + Severity for log entries emitted via :func:`TSCfgLoadCtxAddLog`. + + .. enumerator:: TS_CFG_LOG_NOTE + + Informational. Default level. Maps to ``DL_Note`` internally. + + .. enumerator:: TS_CFG_LOG_WARNING + + Concerning; the reload may still complete. Maps to ``DL_Warning``. + + .. enumerator:: TS_CFG_LOG_ERROR + + Failure cause; pair with a subsequent :func:`TSCfgLoadCtxFail`. + Maps to ``DL_Error``. + + The framework deliberately does not expose the fatal-class diagnostics + levels here: a reload handler should not be able to terminate the + process. Plugins that truly need debug-only output should use + ``Dbg(ctl, ...)`` instead of this API. + .. function:: TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info) -.. function:: TSReturnCode TSCfgAttachTrigger(std::string_view key, std::string_view record_name) +.. function:: TSReturnCode TSCfgAttachReloadTrigger(std::string_view key, std::string_view record_name) .. function:: TSReturnCode TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info) -.. function:: TSReturnCode TSCfgSetEnabled(std::string_view key, int enabled) .. function:: void TSCfgLoadCtxInProgress(TSCfgLoadCtx ctx, std::string_view msg) .. function:: void TSCfgLoadCtxComplete(TSCfgLoadCtx ctx, std::string_view msg) .. function:: void TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg) -.. function:: void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg) +.. function:: void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, TSCfgLogLevel level, std::string_view msg) .. function:: TSCfgLoadCtx TSCfgLoadCtxAddSubtask(TSCfgLoadCtx ctx, std::string_view description) .. function:: std::string_view TSCfgLoadCtxGetFilename(TSCfgLoadCtx ctx) .. function:: std::string_view TSCfgLoadCtxGetReloadToken(TSCfgLoadCtx ctx) @@ -188,13 +214,20 @@ Registration incomplete ``info`` struct, or if another plugin (or core) has already registered the same key. -:func:`TSCfgAttachTrigger` +:func:`TSCfgAttachReloadTrigger` Attaches a fully-qualified record name (e.g. ``proxy.config.my_plugin.enabled``) to a previously registered key. - When the record value changes, the plugin's handler is invoked, the - same way it would be if the file changed on disk. May be called - multiple times to attach more than one trigger record to the same - entry. + When the record value changes, the plugin's reload handler is + invoked, the same way it would be if the file changed on disk. May + be called multiple times to attach more than one trigger record to + the same entry. + + This is **not** a general record-change subscription primitive: the + only callback the plugin gets is the existing config-reload + handler, the reload is treated as file-driven (no RPC payload), and + standalone record changes (i.e. ``traffic_ctl config set`` outside + an active reload cycle) invoke the handler with an empty context + that does not surface in :option:`traffic_ctl config status`. :func:`TSCfgAddFileDependency` Adds a companion file to a previously registered key. When the @@ -203,12 +236,6 @@ Registration set - the plugin's handler is invoked. With ``dep_key`` empty the dependency is file-change-only. -:func:`TSCfgSetEnabled` - Toggles a registered config at runtime. Disabled entries are skipped - during reloads (their tasks are short-circuited to a - ``"skipped (disabled)"`` completion). Unlike the other three, this - function may be called at any time, not only during ``TSPluginInit``. - Per-reload context (TSCfgLoadCtx) --------------------------------- @@ -311,15 +338,38 @@ overall reload (see :ref:`config-context-terminal-state`). The context handle then leaks for the process lifetime, so reaching a terminal state on every code path is mandatory. +Recommended pattern: parse first, apply second +---------------------------------------------- + +The framework does not provide a separate validation phase. The +handler is the single point where the plugin parses the new +configuration and applies it. To avoid leaving live state half-mutated +on a partial parse, plugins should follow a two-step pattern inside +the same handler: + +1. **Parse** the file (or supplied YAML) into a fresh, staging-side + structure. Validate fully. Do not touch live state during this + step. On parse failure, call :func:`TSCfgLoadCtxFail` and return - + live state remains untouched. + +2. **Swap** the staging structure into place atomically. This is + typically a pointer swap into a ``std::shared_ptr`` / + ``std::atomic`` slot the request path reads from. After the swap + succeeds, call :func:`TSCfgLoadCtxComplete`. + +This pattern matches what core configs already do (for example, +``ip_allow``, ``remap.config``) and gives operators predictable +behavior on a malformed reload: the previous configuration stays in +effect until a fully valid one is loaded. + Restrictions ============ - Not supported for remap plugins (:func:`TSRemapInit` / :func:`TSRemapNewInstance`). -- :func:`TSCfgRegister`, :func:`TSCfgAttachTrigger`, and +- :func:`TSCfgRegister`, :func:`TSCfgAttachReloadTrigger`, and :func:`TSCfgAddFileDependency` must all be called from :func:`TSPluginInit`, after :func:`TSPluginRegister`. -- :func:`TSCfgSetEnabled` may be called at any time after registration. Example - registration and synchronous handler ============================================== @@ -387,7 +437,7 @@ Example - registration and synchronous handler } // Optional: attach a record so changing it fires the handler. - TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); + TSCfgAttachReloadTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); } Example - RPC content with directives diff --git a/doc/developer-guide/config-reload-framework.en.rst b/doc/developer-guide/config-reload-framework.en.rst index 3816651654a..74eccb8cd50 100644 --- a/doc/developer-guide/config-reload-framework.en.rst +++ b/doc/developer-guide/config-reload-framework.en.rst @@ -712,16 +712,13 @@ What changes for plugins is only the surface API: - ``ConfigRegistry::register_config`` becomes :func:`TSCfgRegister`, which takes a :type:`TSCfgRegistrationInfo` options struct. -- ``ConfigRegistry::attach`` becomes :func:`TSCfgAttachTrigger`. +- ``ConfigRegistry::attach`` becomes :func:`TSCfgAttachReloadTrigger`. - ``ConfigRegistry::add_file_dependency`` becomes :func:`TSCfgAddFileDependency`. - ``ConfigContext`` becomes the opaque ``TSCfgLoadCtx`` handle, with the same in-progress / complete / fail / log / supplied-yaml / reload-directives / add-subtask operations exposed as plain ``TSCfgLoadCtx*`` functions. -- :func:`TSCfgSetEnabled` allows the plugin to disable a registered - entry at runtime, in which case the framework short-circuits the - task to a ``"skipped (disabled)"`` completion. Lifecycle and preconditions --------------------------- @@ -803,7 +800,7 @@ either file-driven or RPC-driven reload: } // Optional: trigger the handler whenever this record changes. - TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); + TSCfgAttachReloadTrigger(PLUGIN_NAME, "proxy.config.my_plugin.enabled"); } The handler obeys the same terminal-state rule as core handlers - every diff --git a/include/mgmt/config/ConfigContext.h b/include/mgmt/config/ConfigContext.h index 4d2ebb24389..7df828098b6 100644 --- a/include/mgmt/config/ConfigContext.h +++ b/include/mgmt/config/ConfigContext.h @@ -203,8 +203,8 @@ class ConfigContext private: /// Attach the registering plugin's name. A non-empty name marks the context's - /// task as plugin-originated (is_plugin = true); an empty view marks it as - /// core. Used for diagnostics and traffic_ctl status attribution. + /// task as plugin-originated; an empty view marks it as core. Used for + /// diagnostics and traffic_ctl status attribution. void set_plugin_name(std::string_view name); /// Set supplied YAML node. Only ConfigRegistry should call this during reload setup. diff --git a/include/mgmt/config/ConfigRegistry.h b/include/mgmt/config/ConfigRegistry.h index 24b62eee9d5..6569681ebd4 100644 --- a/include/mgmt/config/ConfigRegistry.h +++ b/include/mgmt/config/ConfigRegistry.h @@ -111,10 +111,10 @@ class ConfigRegistry ConfigReloadHandler handler; ///< Reload handler (empty for static / non-reloadable entries) std::vector trigger_records; ///< Records that trigger reload bool is_required{false}; ///< Whether the file must exist on disk - bool is_plugin{false}; ///< Registered by a plugin (via TSCfgRegister) - bool enabled{true}; ///< Runtime toggle - disabled entries are skipped during reload /// Plugin that registered this entry (from TSPluginRegister). Empty for core entries. - /// Used in conflict diagnostics, reload-trace logs, and traffic_ctl status output. + /// Non-empty <=> the entry was registered via TSCfgRegister; used as the + /// canonical "is plugin?" predicate throughout the framework. Surfaces in + /// conflict diagnostics, reload-trace logs, and traffic_ctl status output. std::string plugin_name; /// Resolve the actual filename (reads from record, falls back to default) @@ -207,18 +207,6 @@ class ConfigRegistry /// int attach(const std::string &key, const char *record_name); - /// - /// @brief Enable or disable a config entry at runtime - /// - /// When disabled, the handler is skipped during reloads. The entry - /// remains registered and visible in status, but marked as skipped. - /// - /// @param key The registered config key - /// @param enabled true to enable, false to disable - /// @return 0 on success, -1 if key not found - /// - int set_enabled(const std::string &key, bool enabled); - /// /// @brief Add a file dependency to an existing config /// diff --git a/include/mgmt/config/ConfigReloadTrace.h b/include/mgmt/config/ConfigReloadTrace.h index 8e764979657..e7618bd4a81 100644 --- a/include/mgmt/config/ConfigReloadTrace.h +++ b/include/mgmt/config/ConfigReloadTrace.h @@ -202,8 +202,9 @@ class ConfigReloadTask : public std::enable_shared_from_this std::string filename; ///< source file, if applicable std::vector sub_tasks; ///< child tasks (if any) bool main_task{false}; ///< true for the top-level reload task - bool is_plugin{false}; ///< registered by a plugin (via TSConfigRegister) - /// Plugin name supplied at registration (only meaningful when is_plugin is true; empty for core entries). + /// Plugin name supplied at registration. Empty for core entries; non-empty + /// <=> plugin-originated (via TSCfgRegister). Used as the canonical + /// "is plugin?" predicate throughout the framework. std::string plugin_name; }; @@ -267,15 +268,14 @@ class ConfigReloadTask : public std::enable_shared_from_this _info.config_key = key; } - /// Set the registering plugin's name. Also flips @c is_plugin to (!name.empty()) - /// so callers don't need a separate boolean setter; pass an empty view for core - /// (non-plugin) entries. + /// Set the registering plugin's name. Pass an empty view for core (non-plugin) + /// entries; non-empty marks the task as plugin-originated throughout the + /// framework. void set_plugin_name(std::string_view name) { std::unique_lock lock(_mutex); _info.plugin_name = name; - _info.is_plugin = !name.empty(); } /// Check if any immediate subtask has the given config_key. diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index 85b8eff50dc..b589f82de83 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1618,6 +1618,19 @@ enum TSCfgSourceType { TS_CFG_SOURCE_FILE_AND_RPC ///< Handler can also process YAML content supplied via RPC }; +/// Severity for log entries emitted via TSCfgLoadCtxAddLog(). +/// +/// Three levels intentionally - the framework deliberately does not expose +/// the fatal-class diagnostics levels (Fatal/Alert/Emergency) here: a +/// reload handler should not be able to terminate the process. Plugins that +/// truly need debug-only output should use Dbg(ctl, ...) instead of this +/// API. +enum TSCfgLogLevel { + TS_CFG_LOG_NOTE, ///< Informational; default. Maps to DL_Note internally. + TS_CFG_LOG_WARNING, ///< Concerning; the reload may still complete. Maps to DL_Warning. + TS_CFG_LOG_ERROR ///< Failure cause; pair with a subsequent TSCfgLoadCtxFail. Maps to DL_Error. +}; + /// Options for TSCfgRegister(). /// /// Passed by const-pointer so future fields can be appended without breaking @@ -1665,8 +1678,13 @@ struct TSCfgRegistrationInfo { /// Whether @c handler can also process YAML content supplied via RPC. TSCfgSourceType source{TS_CFG_SOURCE_FILE_ONLY}; - /// True if the file must exist on disk for a reload to be considered - /// successful. False (default) lets the plugin tolerate a missing file. + /// Hint propagated to FileManager: marks the file as "required" in + /// catalog/inspection output and emits a Dbg line if the file is + /// missing at registration time. The framework does NOT enforce this + /// flag at reload-time today: the handler is still invoked even when + /// the file is missing or invalid, and the plugin chooses whether to + /// fail the reload. False (default) is appropriate for most plugins. + /// Mirrored on TSCfgFileDependencyInfo for parity with core configs. bool is_required{false}; }; @@ -1710,8 +1728,9 @@ struct TSCfgFileDependencyInfo { /// empty, the dependency is file-change-only. std::string_view dep_key; - /// True if the file must exist on disk for a reload to be considered - /// successful. False (default) lets the plugin tolerate a missing file. + /// Hint propagated to FileManager (catalog/inspection only). See the + /// note on TSCfgRegistrationInfo::is_required for details. False + /// (default) is appropriate for most plugins. bool is_required{false}; }; diff --git a/include/ts/ts.h b/include/ts/ts.h index de11d73ef64..2f93adfbe7d 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1285,9 +1285,16 @@ TSReturnCode TSMgmtConfigFileAdd(const char *parent, const char *fileName); TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info); /** - Attach a trigger record to a registered plugin config. When the record - value changes, the plugin's config handler is invoked (same as if the - file changed on disk). + Attach a record so its value changes trigger a reload of a previously + registered plugin config. When the record changes, the plugin's reload + handler is invoked (same as if the file changed on disk). + + This is NOT a general record-change subscription primitive: the only + callback the plugin gets is the existing config-reload handler, the + reload is treated as file-driven (no RPC payload), and standalone + record changes (e.g. traffic_ctl config set with no config reload in + flight) invoke the handler with an empty context that does not surface + in `traffic_ctl config status`. See the developer guide for details. @note Must be called from TSPluginInit() after TSCfgRegister(). @@ -1295,7 +1302,7 @@ TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info); @param record_name Fully-qualified record name (e.g., "proxy.config.my_plugin.enabled"). @return TS_SUCCESS on success, TS_ERROR if key not found or record invalid. */ -TSReturnCode TSCfgAttachTrigger(std::string_view key, std::string_view record_name); +TSReturnCode TSCfgAttachReloadTrigger(std::string_view key, std::string_view record_name); /** Add a companion file dependency to a registered plugin config. When the @@ -1314,22 +1321,6 @@ TSReturnCode TSCfgAttachTrigger(std::string_view key, std::string_view record_na */ TSReturnCode TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info); -/** - Enable or disable a registered config entry at runtime. - - When disabled, the handler is skipped during reloads. The entry - remains registered and visible in traffic_ctl config status, but - is marked as "skipped (disabled)". - - Unlike TSCfgRegister/TSCfgAttachTrigger/TSCfgAddFileDependency, this - function can be called at any time, not only during TSPluginInit(). - - @param key The config key passed to TSCfgRegister(). - @param enabled Non-zero to enable, zero to disable. - @return TS_SUCCESS on success, TS_ERROR if key not found. -*/ -TSReturnCode TSCfgSetEnabled(std::string_view key, int enabled); - /** Mark a load context as in-progress. @@ -1390,10 +1381,10 @@ void TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg); Null-safe: calling with a nullptr @a ctx or an empty @a msg is a no-op. @param ctx Context handle from the TSCfgLoadCb callback, or nullptr. - @param level Severity level (DL_Note, DL_Warning, DL_Error, etc.). + @param level Severity level (TS_CFG_LOG_NOTE / WARNING / ERROR). @param msg Human-readable message; empty is a no-op. */ -void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg); +void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, TSCfgLogLevel level, std::string_view msg); /** Create a dependent subtask under this context. The subtask tracks its own diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 0e4a3e05d5a..eb99298f2b5 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -3406,7 +3406,7 @@ TSCfgRegister(const TSCfgRegistrationInfo *info) } TSReturnCode -TSCfgAttachTrigger(std::string_view key, std::string_view record_name) +TSCfgAttachReloadTrigger(std::string_view key, std::string_view record_name) { if (!plugin_reg_current || plugin_reg_current->plugin_name == nullptr) { return TS_ERROR; @@ -3422,11 +3422,11 @@ TSCfgAttachTrigger(std::string_view key, std::string_view record_name) std::string record_str{record_name}; if (registry.attach(key_str, record_str.c_str()) != 0) { - Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachTrigger: key '%s' not found", plugin_name, key_str.c_str()); + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachReloadTrigger: key '%s' not found", plugin_name, key_str.c_str()); return TS_ERROR; } - Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachTrigger: attached record '%s' to config '%s'", plugin_name, record_str.c_str(), + Dbg(dbg_ctl_plugin_config, "[%s] TSCfgAttachReloadTrigger: attached record '%s' to config '%s'", plugin_name, record_str.c_str(), key_str.c_str()); return TS_SUCCESS; } @@ -3478,35 +3478,6 @@ TSCfgAddFileDependency(const TSCfgFileDependencyInfo *info) return TS_SUCCESS; } -TSReturnCode -TSCfgSetEnabled(std::string_view key, int enabled) -{ - if (key.empty()) { - Error("[unknown-plugin] TSCfgSetEnabled: empty key"); - return TS_ERROR; - } - - auto ®istry = config::ConfigRegistry::Get_Instance(); - std::string key_str{key}; - - // Resolve the owning plugin's name for log attribution. Unlike the other - // TSCfg* APIs, TSCfgSetEnabled may be called outside TSPluginInit, so - // plugin_reg_current is unreliable; pull the name from the registry entry. - char const *plugin_name = "unknown-plugin"; - if (auto const *entry = registry.find(key_str); entry != nullptr && !entry->plugin_name.empty()) { - plugin_name = entry->plugin_name.c_str(); - } - - if (registry.set_enabled(key_str, enabled != 0) != 0) { - Warning("[%s] TSCfgSetEnabled: key '%s' not found", plugin_name, key_str.c_str()); - return TS_ERROR; - } - - Dbg(dbg_ctl_plugin_config, "[%s] TSCfgSetEnabled: config '%s' %s", plugin_name, key_str.c_str(), - enabled ? "enabled" : "disabled"); - return TS_SUCCESS; -} - namespace { // Finalize a plugin context: forward completion/failure to the underlying core @@ -3573,14 +3544,28 @@ TSCfgLoadCtxFail(TSCfgLoadCtx ctx, std::string_view msg) } void -TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, DiagsLevel level, std::string_view msg) +TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, TSCfgLogLevel level, std::string_view msg) { if (ctx == nullptr || msg.empty()) { return; } + DiagsLevel diags_level = DL_Note; + switch (level) { + case TS_CFG_LOG_WARNING: + diags_level = DL_Warning; + break; + case TS_CFG_LOG_ERROR: + diags_level = DL_Error; + break; + case TS_CFG_LOG_NOTE: + default: + diags_level = DL_Note; + break; + } + auto *pctx = reinterpret_cast(ctx); - CfgLoadLog(pctx->ctx, level, "%.*s", static_cast(msg.size()), msg.data()); + CfgLoadLog(pctx->ctx, diags_level, "%.*s", static_cast(msg.size()), msg.data()); } TSCfgLoadCtx diff --git a/src/mgmt/config/ConfigRegistry.cc b/src/mgmt/config/ConfigRegistry.cc index de4ec6ab8e6..7038c4769a1 100644 --- a/src/mgmt/config/ConfigRegistry.cc +++ b/src/mgmt/config/ConfigRegistry.cc @@ -115,13 +115,6 @@ class RecordTriggeredReloadContinuation : public Continuation Warning("Config key '%s' not found in registry", _config_key.c_str()); } else if (!entry->handler) { Warning("Config '%s' has no handler", _config_key.c_str()); - } else if (!entry->enabled) { - Dbg(dbg_ctl, "Config '%s' skipped (disabled)", _config_key.c_str()); - auto ctx = ReloadCoordinator::Get_Instance().create_config_context(_config_key, _config_key, entry->resolve_filename()); - if (ctx) { - ctx.set_plugin_name(entry->plugin_name); - ctx.complete("skipped (disabled)"); - } } else { auto ctx = ReloadCoordinator::Get_Instance().create_config_context(_config_key, _config_key, entry->resolve_filename()); if (!ctx) { @@ -217,7 +210,7 @@ void ConfigRegistry::do_register(Entry entry) { const char *type_str = (entry.type == ConfigType::YAML) ? "YAML" : "legacy"; - const char *owner_str = entry.is_plugin ? (entry.plugin_name.empty() ? "plugin:unknown" : entry.plugin_name.c_str()) : "core"; + const char *owner_str = entry.plugin_name.empty() ? "core" : entry.plugin_name.c_str(); Dbg(dbg_ctl, "Registering %s config '%s' [owner=%s] (default: %s, record: %s, triggers: %zu)", type_str, entry.key.c_str(), owner_str, entry.default_filename.c_str(), entry.filename_record.empty() ? "" : entry.filename_record.c_str(), @@ -243,11 +236,9 @@ ConfigRegistry::do_register(Entry entry) FileManager::instance().addFile(resolved.c_str(), config_name, false, it->second.is_required); } } else { - auto const &existing = it->second; - char const *existing_owner = - existing.is_plugin ? (existing.plugin_name.empty() ? "unknown-plugin" : existing.plugin_name.c_str()) : "core"; - char const *incoming_owner = - entry.is_plugin ? (entry.plugin_name.empty() ? "unknown-plugin" : entry.plugin_name.c_str()) : "core"; + auto const &existing = it->second; + char const *existing_owner = existing.plugin_name.empty() ? "core" : existing.plugin_name.c_str(); + char const *incoming_owner = entry.plugin_name.empty() ? "core" : entry.plugin_name.c_str(); Warning("Config '%s' already registered by %s; ignoring registration from %s", it->first.c_str(), existing_owner, incoming_owner); } @@ -289,7 +280,6 @@ ConfigRegistry::register_plugin_config(const std::string &key, const std::string entry.handler = std::move(handler); entry.source = source; entry.is_required = is_required; - entry.is_plugin = true; entry.type = infer_config_type(default_filename); for (auto const *record : trigger_records) { @@ -367,21 +357,6 @@ ConfigRegistry::attach(const std::string &key, const char *record_name) return wire_record_callback(record_name, config_key); } -int -ConfigRegistry::set_enabled(const std::string &key, bool enabled) -{ - std::unique_lock lock(_mutex); - auto it = _entries.find(key); - if (it == _entries.end()) { - Warning("Cannot set enabled on unknown config: %s", key.c_str()); - return -1; - } - - it->second.enabled = enabled; - Dbg(dbg_ctl, "Config '%s' %s", key.c_str(), enabled ? "enabled" : "disabled"); - return 0; -} - int ConfigRegistry::add_file_dependency(const std::string &key, const char *filename_record, const char *default_filename, bool is_required) @@ -563,12 +538,6 @@ ConfigRegistry::execute_reload(const std::string &key) auto ctx = ReloadCoordinator::Get_Instance().create_config_context(entry_copy.key, entry_copy.key, filename); ctx.set_plugin_name(entry_copy.plugin_name); - if (!entry_copy.enabled) { - Dbg(dbg_ctl, "Config '%s' skipped (disabled)", entry_copy.key.c_str()); - ctx.complete("skipped (disabled)"); - return; - } - ctx.in_progress(); if (has_passed_config) { diff --git a/src/records/unit_tests/test_ConfigRegistry.cc b/src/records/unit_tests/test_ConfigRegistry.cc index 8ddc0b15058..263162681d9 100644 --- a/src/records/unit_tests/test_ConfigRegistry.cc +++ b/src/records/unit_tests/test_ConfigRegistry.cc @@ -224,7 +224,7 @@ TEST_CASE("ConfigRegistry register_plugin_config stores filename_record on the e REQUIRE(entry != nullptr); CHECK(entry->key == "test_plugin_filename_record"); CHECK(entry->plugin_name == "test_plugin"); - CHECK(entry->is_plugin); + CHECK_FALSE(entry->plugin_name.empty()); // plugin_name non-empty == plugin entry CHECK(entry->default_filename == "default.yaml"); CHECK(entry->filename_record == "proxy.config.test_plugin.filename"); CHECK(entry->source == ConfigSource::FileOnly); diff --git a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py index bae3c232130..21b2087d7a0 100644 --- a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py +++ b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py @@ -1,9 +1,9 @@ ''' -Test the TSCfg* plugin config API end-to-end — all 10 public functions. +Test the TSCfg* plugin config API end-to-end. Registration APIs (called in TSPluginInit): 1. TSCfgRegister — plugin loads and registers - 2. TSCfgAttachTrigger — record change fires handler + 2. TSCfgAttachReloadTrigger — record change fires handler 3. TSCfgAddFileDependency — companion file change fires handler Handler APIs (called during reload): @@ -74,7 +74,7 @@ 'cfg_plugin_test.conf cfg_plugin_companion.conf') # ============================================================================ -# Test A: Plugin startup — TSCfgRegister + TSCfgAttachTrigger + TSCfgAddFileDependency +# Test A: Plugin startup — TSCfgRegister + TSCfgAttachReloadTrigger + TSCfgAddFileDependency # ============================================================================ tr = Test.AddTestRun("Plugin loads and registers all init APIs") tr.Processes.Default.StartBefore(ts) @@ -86,7 +86,7 @@ # so they land in traffic.out, not diags.log. ts.Disk.traffic_out.Content = All( Testers.IncludesExpression('TSCfgRegister OK', 'TSCfgRegister should succeed'), - Testers.IncludesExpression('TSCfgAttachTrigger OK', 'TSCfgAttachTrigger should succeed'), + Testers.IncludesExpression('TSCfgAttachReloadTrigger OK', 'TSCfgAttachReloadTrigger should succeed'), Testers.IncludesExpression('TSCfgAddFileDependency OK', 'TSCfgAddFileDependency should succeed'), ) @@ -233,10 +233,10 @@ def validate_rpc_subtask_fail(resp: Response): tr.StillRunningAfter = ts # ============================================================================ -# Test F: Record-trigger reload (TSCfgAttachTrigger) +# Test F: Record-trigger reload (TSCfgAttachReloadTrigger) # Change proxy.config.http.insert_age_in_response to trigger handler # ============================================================================ -tr = Test.AddTestRun("Trigger reload via record change (TSCfgAttachTrigger)") +tr = Test.AddTestRun("Trigger reload via record change (TSCfgAttachReloadTrigger)") tr.DelayStart = 2 tr.Processes.Default.Command = "traffic_ctl config set proxy.config.http.insert_age_in_response 0" tr.Processes.Default.Env = ts.Env @@ -253,66 +253,6 @@ def validate_rpc_subtask_fail(resp: Response): 'cfg_plugin_test', 'Plugin handler should have been called by record trigger') tr.StillRunningAfter = ts -# ============================================================================ -# Test H: TSCfgSetEnabled — disable self, verify skipped, re-enable -# ============================================================================ - -# Step 1: Plugin disables itself (this reload still runs the handler) -tr = Test.AddTestRun("RPC reload: plugin disables itself") -tr.DelayStart = 2 -tr.AddJsonRPCClientRequest( - ts, Request.admin_config_reload(token='rpc-disable-self', configs={'cfg_plugin_test': { - 'disable_self': True - }}, force=True)) - - -def validate_disable_self(resp: Response): - result = resp.result - return (True, f"disable_self dispatched: token={result.get('token', '')}") - - -tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_disable_self) -tr.StillRunningAfter = ts - -tr = Test.AddTestRun("Verify disable_self completed") -tr.DelayStart = 5 -tr.Processes.Default.Command = "traffic_ctl config status -t rpc-disable-self" -tr.Processes.Default.Env = ts.Env -tr.Processes.Default.ReturnCode = 0 -tr.Processes.Default.Streams.stdout = Testers.IncludesExpression( - 'disabled self', 'Handler should have run and reported disabling itself') -tr.StillRunningAfter = ts - -# Step 2: Now the plugin is disabled — reload should skip it -tr = Test.AddTestRun("RPC reload while disabled: should be skipped") -tr.DelayStart = 2 -tr.AddJsonRPCClientRequest( - ts, - Request.admin_config_reload( - token='rpc-while-disabled', configs={'cfg_plugin_test': { - 'greet': 'should_not_appear' - }}, force=True)) - - -def validate_while_disabled(resp: Response): - result = resp.result - return (True, f"while-disabled dispatched: token={result.get('token', '')}") - - -tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_while_disabled) -tr.StillRunningAfter = ts - -tr = Test.AddTestRun("Verify skipped (disabled) in status") -tr.DelayStart = 5 -tr.Processes.Default.Command = "traffic_ctl config status -t rpc-while-disabled" -tr.Processes.Default.Env = ts.Env -tr.Processes.Default.ReturnCode = 0 -tr.Processes.Default.Streams.stdout = All( - Testers.IncludesExpression('skipped', 'Disabled plugin should show skipped'), - Testers.ExcludesExpression('should_not_appear', 'Handler should NOT have run'), -) -tr.StillRunningAfter = ts - # ============================================================================ # Test G: Core tasks should NOT have [plugin] tag # ============================================================================ diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc index f88d2247734..eb8979efd42 100644 --- a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_deferred_test.cc @@ -64,7 +64,8 @@ deferred_handler(TSCont contp, TSEvent /* event */, void * /* edata */) if (work->stage == 0) { Dbg(dbg_ctl, "Stage 0: deferred work starting after 3s wait"); - TSCfgLoadCtxAddLog(work->ctx, DL_Note, "cfg_plugin_deferred_test: stage 0 - deferred work starting, simulating heavy work"); + TSCfgLoadCtxAddLog(work->ctx, TS_CFG_LOG_NOTE, + "cfg_plugin_deferred_test: stage 0 - deferred work starting, simulating heavy work"); work->stage = 1; TSContScheduleOnPool(contp, 2000, TS_THREAD_POOL_TASK); return 0; @@ -72,10 +73,10 @@ deferred_handler(TSCont contp, TSEvent /* event */, void * /* edata */) Dbg(dbg_ctl, "Stage 1: heavy work done after 2s, completing"); if (work->should_fail) { - TSCfgLoadCtxAddLog(work->ctx, DL_Error, "cfg_plugin_deferred_test: stage 1 - heavy work failed"); + TSCfgLoadCtxAddLog(work->ctx, TS_CFG_LOG_ERROR, "cfg_plugin_deferred_test: stage 1 - heavy work failed"); TSCfgLoadCtxFail(work->ctx, "cfg_plugin_deferred_test: deferred fail after heavy work"); } else { - TSCfgLoadCtxAddLog(work->ctx, DL_Note, "cfg_plugin_deferred_test: stage 1 - heavy work succeeded"); + TSCfgLoadCtxAddLog(work->ctx, TS_CFG_LOG_NOTE, "cfg_plugin_deferred_test: stage 1 - heavy work succeeded"); TSCfgLoadCtxComplete(work->ctx, "cfg_plugin_deferred_test: deferred complete after heavy work"); } @@ -88,7 +89,7 @@ void config_reload(TSCfgLoadCtx ctx, void * /* data */) { TSCfgLoadCtxInProgress(ctx, "cfg_plugin_deferred_test: deferring work, will reschedule in 3s"); - TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_deferred_test: scheduling two-stage deferred completion (3s + 2s)"); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, "cfg_plugin_deferred_test: scheduling two-stage deferred completion (3s + 2s)"); bool should_fail = false; diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc index 3d725645ca1..a3f92ca98b9 100644 --- a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_directives_test.cc @@ -73,10 +73,10 @@ config_reload(TSCfgLoadCtx ctx, void * /* data */) version = (*directives)["version"].as(); } - TSCfgLoadCtxAddLog(ctx, DL_Note, std::string{PLUGIN_NAME ": directive_version="} + version); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, std::string{PLUGIN_NAME ": directive_version="} + version); Dbg(dbg_ctl, "Directives present: version=%s", version.c_str()); } else { - TSCfgLoadCtxAddLog(ctx, DL_Note, PLUGIN_NAME ": no_directives"); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, PLUGIN_NAME ": no_directives"); Dbg(dbg_ctl, "No directives present"); } @@ -91,7 +91,7 @@ config_reload(TSCfgLoadCtx ctx, void * /* data */) greeting = (*node)["greeting"].as(); } - TSCfgLoadCtxAddLog(ctx, DL_Note, std::string{PLUGIN_NAME ": content_greeting="} + greeting); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, std::string{PLUGIN_NAME ": content_greeting="} + greeting); TSCfgLoadCtxComplete(ctx, PLUGIN_NAME ": RPC reload OK"); return; } @@ -105,7 +105,7 @@ config_reload(TSCfgLoadCtx ctx, void * /* data */) ss << file.rdbuf(); std::string content = ss.str(); std::string msg = std::string{PLUGIN_NAME ": file_mode ("} + std::to_string(content.size()) + " bytes)"; - TSCfgLoadCtxAddLog(ctx, DL_Note, msg); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, msg); TSCfgLoadCtxComplete(ctx, msg); return; } diff --git a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc index 05a9d56a538..ee30390c274 100644 --- a/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc +++ b/tests/gold_tests/jsonrpc/plugins/cfg_plugin_test.cc @@ -4,7 +4,7 @@ Registration (TSPluginInit): TSCfgRegister - registers "cfg_plugin_test" with FILE_AND_RPC source - TSCfgAttachTrigger - attaches a record trigger + TSCfgAttachReloadTrigger - attaches a record trigger that reloads TSCfgAddFileDependency - adds a companion file dependency Handler (config_reload): @@ -69,7 +69,7 @@ config_reload(TSCfgLoadCtx ctx, void *data) TSCfgLoadCtxInProgress(ctx, "cfg_plugin_test: processing started"); // --- TSCfgLoadCtxAddLog: add an intermediate log entry --- - TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_test: handler entered"); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, "cfg_plugin_test: handler entered"); // --- TSCfgLoadCtxGetSuppliedYaml: detect RPC vs file mode --- TSYaml yaml = TSCfgLoadCtxGetSuppliedYaml(ctx); @@ -79,7 +79,7 @@ config_reload(TSCfgLoadCtx ctx, void *data) // --- fail_on_purpose: test TSCfgLoadCtxFail --- if ((*node)["fail_on_purpose"]) { - TSCfgLoadCtxAddLog(ctx, DL_Error, "cfg_plugin_test: fail requested"); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_ERROR, "cfg_plugin_test: fail requested"); TSCfgLoadCtxFail(ctx, "cfg_plugin_test: fail_on_purpose via RPC"); return; } @@ -88,7 +88,7 @@ config_reload(TSCfgLoadCtx ctx, void *data) if ((*node)["with_subtask"]) { TSCfgLoadCtx child = TSCfgLoadCtxAddSubtask(ctx, "cfg_plugin_test_subtask"); TSCfgLoadCtxInProgress(child, "subtask working"); - TSCfgLoadCtxAddLog(child, DL_Note, "cfg_plugin_test: subtask log entry"); + TSCfgLoadCtxAddLog(child, TS_CFG_LOG_NOTE, "cfg_plugin_test: subtask log entry"); TSCfgLoadCtxComplete(child, "cfg_plugin_test: subtask done"); TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: parent with subtask done"); return; @@ -103,20 +103,6 @@ config_reload(TSCfgLoadCtx ctx, void *data) return; } - // --- disable_self: test TSCfgSetEnabled(key, 0) --- - if ((*node)["disable_self"]) { - TSCfgSetEnabled(PLUGIN_NAME, 0); - TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: disabled self"); - return; - } - - // --- enable_self: test TSCfgSetEnabled(key, 1) --- - if ((*node)["enable_self"]) { - TSCfgSetEnabled(PLUGIN_NAME, 1); - TSCfgLoadCtxComplete(ctx, "cfg_plugin_test: re-enabled self"); - return; - } - // --- greet: test TSCfgLoadCtxComplete with message --- if ((*node)["greet"]) { std::string greet = (*node)["greet"].as(); @@ -132,7 +118,7 @@ config_reload(TSCfgLoadCtx ctx, void *data) std::string_view filename = TSCfgLoadCtxGetFilename(ctx); std::string filename_str{filename.empty() ? state->config_path : std::string{filename}}; - TSCfgLoadCtxAddLog(ctx, DL_Note, "cfg_plugin_test: reading file " + filename_str); + TSCfgLoadCtxAddLog(ctx, TS_CFG_LOG_NOTE, "cfg_plugin_test: reading file " + filename_str); std::string content; std::ifstream file(filename_str); @@ -198,12 +184,12 @@ TSPluginInit(int argc, const char *argv[]) return; } - // --- TSCfgAttachTrigger: attach a record so changing it fires our handler --- - ret = TSCfgAttachTrigger(PLUGIN_NAME, "proxy.config.http.insert_age_in_response"); + // --- TSCfgAttachReloadTrigger: attach a record so changing it fires our handler --- + ret = TSCfgAttachReloadTrigger(PLUGIN_NAME, "proxy.config.http.insert_age_in_response"); if (ret == TS_SUCCESS) { - Dbg(dbg_ctl, "TSCfgAttachTrigger OK"); + Dbg(dbg_ctl, "TSCfgAttachReloadTrigger OK"); } else { - TSError("[%s] TSCfgAttachTrigger FAILED", PLUGIN_NAME); + TSError("[%s] TSCfgAttachReloadTrigger FAILED", PLUGIN_NAME); } // --- TSCfgAddFileDependency: add companion file --- From cc17185b4fd444adc3a688094a2dd55596e5cfdf Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Thu, 7 May 2026 18:06:12 +0200 Subject: [PATCH 3/6] improve docs --- .../api/functions/TSCfgRegister.en.rst | 78 +++++++++++++++---- include/ts/ts.h | 70 +++++++++++++---- 2 files changed, 120 insertions(+), 28 deletions(-) diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst index e4e0b27a591..88cab7c9153 100644 --- a/doc/developer-guide/api/functions/TSCfgRegister.en.rst +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -215,19 +215,46 @@ Registration already registered the same key. :func:`TSCfgAttachReloadTrigger` - Attaches a fully-qualified record name (e.g. - ``proxy.config.my_plugin.enabled``) to a previously registered key. - When the record value changes, the plugin's reload handler is - invoked, the same way it would be if the file changed on disk. May - be called multiple times to attach more than one trigger record to - the same entry. - - This is **not** a general record-change subscription primitive: the - only callback the plugin gets is the existing config-reload - handler, the reload is treated as file-driven (no RPC payload), and - standalone record changes (i.e. ``traffic_ctl config set`` outside - an active reload cycle) invoke the handler with an empty context - that does not surface in :option:`traffic_ctl config status`. + Wires a record so that changing its value re-runs the reload handler + registered for ``key``. Internally registers a record-change + callback (``RecRegisterConfigUpdateCb``) and routes the event back + into the same reload pipeline as file changes: the plugin's + :type:`TSCfgLoadCb` is invoked with the resolved file path + available via :func:`TSCfgLoadCtxGetFilename`, exactly as it would + be for an on-disk file change. May be called multiple times to + attach more than one trigger record to the same entry. + + **Core analog.** :cpp:func:`!ConfigRegistry::register_config()` + accepts a ``trigger_records`` initializer-list at registration time + (e.g. ``ssl_multicert`` lists ~10 record names there). The + post-registration form is :cpp:func:`!ConfigRegistry::attach`. This + function is the plugin-facing wrapper for the latter; the + initializer-list shape is intentionally not exposed on + :type:`TSCfgRegistrationInfo` so the option struct stays + ABI-stable. + + **It triggers a reload, nothing else.** Specifically the plugin + cannot: + + - Register a free-form record-change callback. There is no + ``TSRecordRegisterChangeCb`` today; the underlying primitive + (``RecRegisterConfigUpdateCb``) is internal-only. + - Receive record-change details. The handler gets no record name, + no old/new value, no event payload. Use ``TSMgmt*Get()`` inside + the handler if it needs the value. + - Subscribe a record without a registered config key. Calling with + an unknown ``key`` returns ``TS_ERROR``. + - Multiplex one record across two keys, or attach in any shape + other than (one record, one config key, per call). + + The reload is always treated as file-driven (no RPC payload): + :func:`TSCfgLoadCtxGetSuppliedYaml` and + :func:`TSCfgLoadCtxGetReloadDirectives` return ``nullptr`` for + record-triggered invocations. Standalone record changes (i.e. + ``traffic_ctl config set`` outside an active reload cycle) invoke + the handler with an empty context that does not surface in + :option:`traffic_ctl config status` - same as core record-triggered + reloads outside a reload cycle. :func:`TSCfgAddFileDependency` Adds a companion file to a previously registered key. When the @@ -292,8 +319,29 @@ State transitions: Inputs: :func:`TSCfgLoadCtxGetFilename` - Returns the resolved file path the framework intends the handler to - read from disk. Empty for RPC-only reloads. + Returns the path the framework expects this handler to read. + Two-step resolution: + + 1. If the registration's ``filename_record`` was set AND the record + currently has a non-empty value, that value is returned (the + operator can override the filename at runtime via + ``traffic_ctl config set ``). + 2. Otherwise, returns ``config_path`` as-registered. + + Most plugins don't set ``filename_record`` and could equivalently + use their own stashed copy of the registered path - this function + is the canonical way to get the filename only when + ``filename_record`` is in play. + + **Always populated for plugin handlers**, including on RPC reloads. + To detect RPC content, check :func:`TSCfgLoadCtxGetSuppliedYaml` - + not this function. + + **Core analog.** :cpp:func:`!ConfigReloadTask::get_filename`. For + core handlers, the value is empty on RPC reloads; the plugin + wrapper always populates it so plugins can use + :func:`TSCfgLoadCtxGetSuppliedYaml` as the canonical RPC-detection + signal. :func:`TSCfgLoadCtxGetReloadToken` Returns the reload-cycle correlation token (e.g. diff --git a/include/ts/ts.h b/include/ts/ts.h index 2f93adfbe7d..e5647114032 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1285,18 +1285,43 @@ TSReturnCode TSMgmtConfigFileAdd(const char *parent, const char *fileName); TSReturnCode TSCfgRegister(const TSCfgRegistrationInfo *info); /** - Attach a record so its value changes trigger a reload of a previously - registered plugin config. When the record changes, the plugin's reload - handler is invoked (same as if the file changed on disk). - - This is NOT a general record-change subscription primitive: the only - callback the plugin gets is the existing config-reload handler, the - reload is treated as file-driven (no RPC payload), and standalone - record changes (e.g. traffic_ctl config set with no config reload in - flight) invoke the handler with an empty context that does not surface - in `traffic_ctl config status`. See the developer guide for details. - - @note Must be called from TSPluginInit() after TSCfgRegister(). + Wire a record so that changing its value re-runs the reload handler + registered for @c key. Internally registers a record-change callback + (@c RecRegisterConfigUpdateCb) and routes the event back into the same + reload pipeline as file changes: the plugin's @c TSCfgLoadCb is invoked + with the resolved file path available via @c TSCfgLoadCtxGetFilename, + exactly as it would be for an on-disk file change. + + It triggers a reload, nothing else. Specifically the plugin cannot: + - Register a free-form record-change callback. There is no + @c TSRecordRegisterChangeCb today; the underlying primitive + (@c RecRegisterConfigUpdateCb) is internal-only. + - Receive record-change details. The handler gets no record name, + no old/new value, no event payload. Use @c TSMgmt*Get() inside + the handler if it needs the value. + - Subscribe a record without a registered config key. Calling with + an unknown @a key returns @c TS_ERROR. + - Multiplex one record across two keys, or attach in any shape + other than (one record, one config key, per call). + + When the record change is caused by @c "traffic_ctl config reload", + the handler runs as a subtask of that reload and surfaces in + @c "traffic_ctl config status". When the record is set standalone + (@c "traffic_ctl config set" outside a reload cycle), the handler + still runs and applies config, but no reload task is created so the + invocation is invisible to @c "config status" - same as core + record-triggered reloads outside a reload cycle. + + Core analog: @c ConfigRegistry::register_config() accepts a + @c trigger_records initializer-list at registration time (e.g. + @c ssl_multicert lists ~10 record names there), and + @c ConfigRegistry::attach(key, record) is the post-registration form. + This function is the plugin-facing wrapper for the latter; the + initializer-list shape is intentionally not exposed on + @c TSCfgRegistrationInfo so the option struct stays ABI-stable. + + @note Must be called from TSPluginInit() after TSCfgRegister(). The + record must already exist (e.g. created via TSMgmtIntCreate). @param key The config key passed to TSCfgRegister(). @param record_name Fully-qualified record name (e.g., "proxy.config.my_plugin.enabled"). @@ -1402,7 +1427,26 @@ void TSCfgLoadCtxAddLog(TSCfgLoadCtx ctx, TSCfgLogLevel level, std::string_view TSCfgLoadCtx TSCfgLoadCtxAddSubtask(TSCfgLoadCtx ctx, std::string_view description); /** - Get the resolved config file path for this load. + Returns the path the framework expects this handler to read. + + Two-step resolution: + 1. If the registration's @c filename_record was set AND the record + currently has a non-empty value, that value is returned (the + operator can override the filename at runtime via + @c "traffic_ctl config set "). + 2. Otherwise, returns @c config_path as-registered. + + Most plugins don't set @c filename_record and could equivalently use + their own stashed copy of the registered path; this function is the + canonical way to get the filename only when @c filename_record is in + play. Always populated for plugin handlers, including on RPC + reloads. To detect RPC content, check + @c TSCfgLoadCtxGetSuppliedYaml(ctx) - not this function. + + Core analog: @c ConfigReloadTask::get_filename(). For core handlers, + the value is empty on RPC reloads; the plugin wrapper always + populates it so plugins can use @c SuppliedYaml as the canonical + RPC-detection signal. Null-safe: calling with nullptr returns an empty string_view. From 321710fee59e1884d3dbdb24a4abf33cdc84e8ed Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Fri, 8 May 2026 12:03:23 +0200 Subject: [PATCH 4/6] update docs --- doc/developer-guide/api/functions/TSCfgRegister.en.rst | 10 +++++----- doc/developer-guide/config-reload-framework.en.rst | 8 +++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst index 88cab7c9153..6381ddb3788 100644 --- a/doc/developer-guide/api/functions/TSCfgRegister.en.rst +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -31,16 +31,16 @@ Synopsis #include -.. type:: TSCfgSourceType +.. enum:: TSCfgSourceType What content sources a plugin's config handler supports. - .. var:: TS_CFG_SOURCE_FILE_ONLY + .. enumerator:: TS_CFG_SOURCE_FILE_ONLY Handler reloads only from a file on disk. RPC-supplied YAML is rejected. - .. var:: TS_CFG_SOURCE_FILE_AND_RPC + .. enumerator:: TS_CFG_SOURCE_FILE_AND_RPC Handler can also process YAML content supplied via RPC. @@ -95,7 +95,7 @@ Synopsis .. var:: TSCfgSourceType source Whether ``handler`` can also process YAML content supplied via RPC. - Defaults to :var:`TS_CFG_SOURCE_FILE_ONLY`. + Defaults to :enumerator:`TSCfgSourceType::TS_CFG_SOURCE_FILE_ONLY`. .. var:: bool is_required @@ -355,7 +355,7 @@ Inputs: framework-reserved ``_reload`` directives are stripped before delivery. The returned node is undefined when the reload was driven by file change rather than RPC. Only meaningful when the entry was - registered with :var:`TS_CFG_SOURCE_FILE_AND_RPC`. + registered with :enumerator:`TSCfgSourceType::TS_CFG_SOURCE_FILE_AND_RPC`. :func:`TSCfgLoadCtxGetReloadDirectives` Returns the YAML map extracted from the ``_reload`` key in the diff --git a/doc/developer-guide/config-reload-framework.en.rst b/doc/developer-guide/config-reload-framework.en.rst index 74eccb8cd50..f780a9fcae8 100644 --- a/doc/developer-guide/config-reload-framework.en.rst +++ b/doc/developer-guide/config-reload-framework.en.rst @@ -17,6 +17,8 @@ .. include:: ../common.defs +.. default-domain:: cpp + .. _config-reload-framework: Configuration Reload Framework @@ -701,7 +703,7 @@ Plugin Configuration Reload =========================== Plugins integrate with the same registry and same task tree described -above, through the public ``TSCfg*`` C++ API in :file:`ts/ts.h`. The +above, through the public ``TSCfg*`` C++ API in ``ts/ts.h``. The framework treats plugin-registered configs as first-class entries: they appear in :option:`traffic_ctl config status`, accept inline YAML via JSONRPC, honor file-mtime change detection, react to attached trigger @@ -807,7 +809,7 @@ The handler obeys the same terminal-state rule as core handlers - every code path must end in ``TSCfgLoadCtxComplete`` or ``TSCfgLoadCtxFail``. Deferred completion (return from the callback, finish from another thread, then call Complete or Fail there) is fully supported; see the -deferred-handler example in :doc:`api/functions/TSCfgRegister`. +deferred-handler example in :doc:`api/functions/TSCfgRegister.en`. Plugin name attribution in ``traffic_ctl`` ------------------------------------------ @@ -881,7 +883,7 @@ The matching autests Reference --------- -:doc:`api/functions/TSCfgRegister` covers the full plugin-facing +:doc:`api/functions/TSCfgRegister.en` covers the full plugin-facing surface: the :type:`TSCfgRegistrationInfo` options struct, the registration / trigger / dependency / enable functions, and every ``TSCfgLoadCtx*`` operation available inside the handler callback. From 883b499fd6df5828601a2a4f8cb1ccc4e0e741e2 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Fri, 8 May 2026 13:45:30 +0200 Subject: [PATCH 5/6] cleanup 2 --- .../api/functions/TSCfgRegister.en.rst | 25 ++++++++++++++++--- .../config-reload-framework.en.rst | 2 +- include/ts/apidefs.h.in | 22 +++++++++++++--- include/ts/ts.h | 12 ++++++--- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst index 6381ddb3788..56c345734ae 100644 --- a/doc/developer-guide/api/functions/TSCfgRegister.en.rst +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -53,10 +53,27 @@ Synopsis .. var:: std::string_view key - Unique registry key. Doubles as the YAML node name when an operator - reloads via ``traffic_ctl config reload --content '{: {...}}'``. - Convention: use the plugin name, or ``.`` when a single - plugin owns multiple configs. Required. + Unique registry key, also used as the YAML node name when an + operator reloads via + ``traffic_ctl config reload --data '{: {...}}'``. + Must be unique across all registered configs (core and plugin). + + **Convention.** Use the plugin name (``my_plugin``). Keep the key + alphanumeric + underscore; avoid dots, since + ``traffic_ctl --directive my_plugin.dry_run=true`` parses the + first ``.`` as the directive separator. + + **Plugins with multiple files:** + + - If the files share the same logical config and should reload + together, register once and add the rest via + :func:`TSCfgAddFileDependency` (single task per reload). + - If the files are genuinely independent and reloadable on their + own, register each separately with a disambiguating key + (e.g. ``my_plugin_main``, ``my_plugin_aux``). Each becomes its + own task entry in :option:`traffic_ctl config status`. + + Required. .. var:: std::string_view config_path diff --git a/doc/developer-guide/config-reload-framework.en.rst b/doc/developer-guide/config-reload-framework.en.rst index f780a9fcae8..ae1074d7eba 100644 --- a/doc/developer-guide/config-reload-framework.en.rst +++ b/doc/developer-guide/config-reload-framework.en.rst @@ -854,7 +854,7 @@ top-level YAML node: .. code-block:: bash - $ traffic_ctl config reload --content '{"my_plugin": {"rules": ["x", "y"]}}' + $ traffic_ctl config reload --data '{"my_plugin": {"rules": ["x", "y"]}}' The handler then calls ``TSCfgLoadCtxGetSuppliedYaml`` to read the content and ``TSCfgLoadCtxGetReloadDirectives`` for any operator diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index b589f82de83..8b7b21514e1 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1650,11 +1650,25 @@ enum TSCfgLogLevel { /// The plugin name is taken automatically from the active TSPluginRegister() /// call and is not part of this struct. struct TSCfgRegistrationInfo { - /// Unique registry key. Also used as the YAML node name when an operator - /// reloads via `traffic_ctl config reload --content '{: {...}}'`. + /// Unique registry key, also used as the YAML node name when an operator + /// reloads via `traffic_ctl config reload --data '{: {...}}'`. /// Must be unique across all registered configs (core and plugin). - /// Convention: use the plugin name, or a "." prefix when - /// a plugin owns multiple configs. Required. + /// + /// Convention: use the plugin name (`my_plugin`). Keep the key + /// alphanumeric + underscore; avoid dots, since `traffic_ctl + /// --directive my_plugin.dry_run=true` parses the first `.` as the + /// directive separator. + /// + /// If the plugin owns multiple files that share the same logical config + /// and should reload together, use ONE TSCfgRegister and add the rest + /// via TSCfgAddFileDependency (single task per reload). + /// + /// If the plugin owns multiple genuinely independent configs that + /// should be reloadable on their own, register each separately with a + /// disambiguating key (e.g. `my_plugin_main`, `my_plugin_aux`). Each + /// becomes its own task entry in `traffic_ctl config status`. + /// + /// Required. std::string_view key; /// Default config file path (absolute, or relative to sysconfdir). diff --git a/include/ts/ts.h b/include/ts/ts.h index e5647114032..8d5b2d9277f 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1254,11 +1254,15 @@ TSReturnCode TSMgmtConfigFileAdd(const char *parent, const char *fileName); The @c key field plays a triple role: - registry lookup key (unique across all registered configs), - YAML node name for RPC-driven reload (`traffic_ctl config reload - --content '{: {...}}'`), + --data '{: {...}}'`), - human-readable label in diagnostics and status output. - By convention plugins use their own plugin name as the key (or a - "." prefix when a plugin owns multiple configs) to - avoid collisions with core entries (e.g. "ip_allow", "sni"). + By convention plugins use their own plugin name as the key (e.g. + "my_plugin"). Keep the key alphanumeric + underscore; avoid dots + so `traffic_ctl --directive` parsing stays unambiguous. When a + plugin owns multiple files that should reload together, use one + TSCfgRegister + TSCfgAddFileDependency rather than multiple + registrations. Avoid collisions with core entries (e.g. "ip_allow", + "sni"). The optional @c filename_record field lets the operator override the configured @c config_path at runtime via a record (e.g. From 85d0fda843794f29b120b953b2efbc0e7b71c1d9 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Mon, 11 May 2026 11:05:42 +0200 Subject: [PATCH 6/6] Address Copilot review and FreeBSD build fixes --- doc/developer-guide/api/functions/TSCfgRegister.en.rst | 10 +++++++--- include/ts/apidefs.h.in | 5 +++-- plugins/regex_revalidate/regex_revalidate.cc | 9 +++++++++ src/mgmt/config/ConfigRegistry.cc | 5 +++-- .../jsonrpc/config_reload_directives_plugin.test.py | 2 +- .../jsonrpc/config_reload_plugin_api.test.py | 4 ++-- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/doc/developer-guide/api/functions/TSCfgRegister.en.rst b/doc/developer-guide/api/functions/TSCfgRegister.en.rst index 56c345734ae..bd8dda00ede 100644 --- a/doc/developer-guide/api/functions/TSCfgRegister.en.rst +++ b/doc/developer-guide/api/functions/TSCfgRegister.en.rst @@ -227,9 +227,13 @@ Registration Must be called from :func:`TSPluginInit`, **after** :func:`TSPluginRegister`. Returns ``TS_ERROR`` if called outside - ``TSPluginInit``, before ``TSPluginRegister``, with a null or - incomplete ``info`` struct, or if another plugin (or core) has - already registered the same key. + ``TSPluginInit``, before ``TSPluginRegister``, or with a null or + incomplete ``info`` struct. + + If another plugin (or core) has already registered the same key, + the duplicate registration is rejected and a warning is logged + identifying both owners; ``TS_SUCCESS`` is still returned, since + the plugin has no useful recovery path at init time. :func:`TSCfgAttachReloadTrigger` Wires a record so that changing its value re-runs the reload handler diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index 8b7b21514e1..f45f792c720 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -45,12 +45,13 @@ #include #include #include +#include +#include +#include #include #include #include #include -#include -#include /** Apply printf format string compile-time argument checking to a function. * diff --git a/plugins/regex_revalidate/regex_revalidate.cc b/plugins/regex_revalidate/regex_revalidate.cc index 4d7bc8b0ce2..bb74025c422 100644 --- a/plugins/regex_revalidate/regex_revalidate.cc +++ b/plugins/regex_revalidate/regex_revalidate.cc @@ -126,6 +126,7 @@ typedef struct { time_t last_load; TSTextLogObject log; char *state_path; + TSMutex reload_mutex; ///< serializes do_config_reload() calls } plugin_state_t; static invalidate_t * @@ -173,6 +174,7 @@ init_plugin_state_t(plugin_state_t *pstate) pstate->last_load = 0; pstate->log = nullptr; pstate->state_path = nullptr; + pstate->reload_mutex = TSMutexCreate(); return pstate; } @@ -194,6 +196,9 @@ free_plugin_state_t(plugin_state_t *pstate) if (pstate->state_path) { TSfree(pstate->state_path); } + if (pstate->reload_mutex) { + TSMutexDestroy(pstate->reload_mutex); + } TSfree(pstate); } @@ -594,7 +599,9 @@ config_reload(TSCfgLoadCtx ctx, void *data) auto *pstate = static_cast(data); Dbg(dbg_ctl, "Config reload via ConfigRegistry"); + TSMutexLock(pstate->reload_mutex); bool const updated = do_config_reload(pstate); + TSMutexUnlock(pstate->reload_mutex); TSCfgLoadCtxComplete(ctx, updated ? "regex_revalidate config reloaded" : "regex_revalidate config unchanged"); } @@ -610,7 +617,9 @@ config_handler(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata ATS_ TSMutexLock(mutex); pstate = (plugin_state_t *)TSContDataGet(cont); + TSMutexLock(pstate->reload_mutex); do_config_reload(pstate); + TSMutexUnlock(pstate->reload_mutex); TSMutexUnlock(mutex); diff --git a/src/mgmt/config/ConfigRegistry.cc b/src/mgmt/config/ConfigRegistry.cc index 7038c4769a1..535c60b9645 100644 --- a/src/mgmt/config/ConfigRegistry.cc +++ b/src/mgmt/config/ConfigRegistry.cc @@ -486,8 +486,9 @@ ConfigRegistry::apply_passed_config(ConfigContext &ctx, YAML::Node &passed_confi // Extract _reload directives before passing content to the handler. This keeps // supplied_yaml() clean (pure config data) and exposes reload_directives() as a // separate accessor for operational parameters. - if (passed_config.IsMap() && passed_config["_reload"]) { - auto directives = passed_config["_reload"]; + const YAML::Node &cfg_view = passed_config; + if (cfg_view.IsMap() && cfg_view["_reload"]) { + auto directives = cfg_view["_reload"]; if (!directives.IsMap()) { Warning("Config '%.*s': _reload must be a YAML map, ignoring directives", static_cast(key.size()), key.data()); } else { diff --git a/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py b/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py index b92cbf25039..5c43f6789ac 100644 --- a/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py +++ b/tests/gold_tests/jsonrpc/config_reload_directives_plugin.test.py @@ -106,7 +106,7 @@ def validate_rpc_directives(resp: Response): Testers.IncludesExpression('directive_version=2.0', 'Plugin should see version directive'), Testers.IncludesExpression('content_greeting=hello_directives', 'Plugin should see greeting in content'), Testers.IncludesExpression('success', 'Should complete successfully'), - Testers.IncludesExpression('[plugin]', 'Should have plugin tag'), + Testers.IncludesExpression(r'\[plugin: ', 'Should have plugin tag'), ) tr.StillRunningAfter = ts diff --git a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py index 21b2087d7a0..69b7db93058 100644 --- a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py +++ b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py @@ -127,7 +127,7 @@ def validate_rpc_greet(resp: Response): tr.Processes.Default.Env = ts.Env tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.stdout = All( - Testers.IncludesExpression('[plugin]', 'Plugin task should have [plugin] tag'), + Testers.IncludesExpression(r'\[plugin: ', 'Plugin task should have [plugin: ] tag'), Testers.IncludesExpression('greet=world', 'Should show greeting in status'), Testers.IncludesExpression('success', 'Should show success'), Testers.IncludesExpression('handler entered', 'TSCfgLoadCtxAddLog message should appear'), @@ -164,7 +164,7 @@ def validate_rpc_fail(resp: Response): tr.Processes.Default.Env = ts.Env tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.stdout = All( - Testers.IncludesExpression('[plugin]', 'Failed task should have [plugin] tag'), + Testers.IncludesExpression(r'\[plugin: ', 'Failed task should have [plugin: ] tag'), Testers.IncludesExpression('fail', 'Should show fail state'), Testers.IncludesExpression('fail requested', 'TSCfgLoadCtxAddLog error message should appear'), )