Copilot commented on code in PR #13146:
URL: https://github.com/apache/trafficserver/pull/13146#discussion_r3208501875
##########
src/mgmt/config/ConfigRegistry.cc:
##########
@@ -426,6 +480,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<int>(key.size()), key.data());
+ } else {
+ Dbg(dbg_ctl, "Config '%.*s' has reload directives",
static_cast<int>(key.size()), key.data());
+ ctx.set_reload_directives(directives);
+ }
+ passed_config.remove("_reload");
Review Comment:
In apply_passed_config(), `passed_config["_reload"]` uses yaml-cpp's
non-const operator[] which inserts the key when missing. That mutates the RPC
payload and can cause `_reload` to appear (as an Undefined node) in the content
passed to handlers, violating the documented contract that `_reload` is
stripped from supplied_yaml. Use a const lookup (or other non-inserting check)
before deciding whether to extract/remove `_reload`.
##########
doc/developer-guide/api/functions/TSCfgRegister.en.rst:
##########
@@ -0,0 +1,561 @@
+.. 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 <ts/ts.h>
+
+.. enum:: TSCfgSourceType
+
+ What content sources a plugin's config handler supports.
+
+ .. enumerator:: TS_CFG_SOURCE_FILE_ONLY
+
+ Handler reloads only from a file on disk. RPC-supplied YAML is
+ rejected.
+
+ .. enumerator:: 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, also used as the YAML node name when an
+ operator reloads via
+ ``traffic_ctl config reload --data '{<key>: {...}}'``.
+ 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
+
+ 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 :enumerator:`TSCfgSourceType::TS_CFG_SOURCE_FILE_ONLY`.
+
+ .. var:: bool is_required
+
+ 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
+
+ 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
+
+ Hint propagated to FileManager (catalog/inspection only); see the
+ equivalent field on :type:`TSCfgRegistrationInfo`. 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`.
+
+.. 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 TSCfgAttachReloadTrigger(std::string_view key,
std::string_view record_name)
+.. function:: TSReturnCode TSCfgAddFileDependency(const
TSCfgFileDependencyInfo *info)
+.. 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, 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)
+.. 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: <name>]``. 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.
Review Comment:
This documentation says TSCfgRegister returns TS_ERROR if another
plugin/core has already registered the same key, but the implementation
intentionally does not signal duplicate-key registration via the return value
(it logs a Warning and still returns TS_SUCCESS once preconditions pass).
Please reconcile the doc with the actual API behavior (or change the
implementation to match the documented contract).
##########
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'),
Review Comment:
The test asserts `[plugin]` in output, but traffic_ctl prints `[plugin:
<name>]` and (if these are regex-based matchers) `[plugin]` is interpreted as a
character class. Update the matcher to escape brackets and/or match the
`[plugin:` prefix so the assertion is meaningful.
##########
tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py:
##########
@@ -0,0 +1,273 @@
+'''
+Test the TSCfg* plugin config API end-to-end.
+
+Registration APIs (called in TSPluginInit):
+ 1. TSCfgRegister — plugin loads and registers
+ 2. TSCfgAttachReloadTrigger — 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 + TSCfgAttachReloadTrigger +
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('TSCfgAttachReloadTrigger OK',
'TSCfgAttachReloadTrigger 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'),
+)
Review Comment:
The status output now tags plugin-owned tasks as `[plugin: <name>]`. These
assertions use the regex pattern `[plugin]`, which (if treated as a regex) is a
character class and won’t match the literal bracketed tag. Consider
matching/escaping the literal brackets (e.g. `\[plugin:`) so the test reliably
validates plugin attribution.
##########
tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py:
##########
@@ -0,0 +1,273 @@
+'''
+Test the TSCfg* plugin config API end-to-end.
+
+Registration APIs (called in TSPluginInit):
+ 1. TSCfgRegister — plugin loads and registers
+ 2. TSCfgAttachReloadTrigger — 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 + TSCfgAttachReloadTrigger +
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('TSCfgAttachReloadTrigger OK',
'TSCfgAttachReloadTrigger 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'),
Review Comment:
Same issue as above: `[plugin]` is not a safe regex to assert the plugin
attribution tag (and doesn’t match the new `[plugin: <name>]` format). Escape
the brackets / match the prefix so this verifies the intended output.
##########
plugins/regex_revalidate/regex_revalidate.cc:
##########
@@ -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<plugin_state_t *>(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");
+}
Review Comment:
regex_revalidate now participates in the reload framework via
config_reload(), but do_config_reload() is called there without any
synchronization. The timed reload path still locks the continuation mutex
around do_config_reload(), so a timed reload and a framework-triggered reload
can run concurrently and race/free the invalidate list. Consider adding a
shared mutex in plugin_state_t (or otherwise reusing the same lock) and taking
it in both config_handler() and config_reload().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
