This is an automated email from the ASF dual-hosted git repository.
dmeden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 6c617ea130 records.yaml - conf_remap update to use records.yaml as
input files. (#9942)
6c617ea130 is described below
commit 6c617ea13016689c6b838e80c4e181008a087a6f
Author: Damian Meden <dme...@apache.org>
AuthorDate: Wed Aug 9 10:58:55 2023 +0200
records.yaml - conf_remap update to use records.yaml as input files.
(#9942)
Breaking:
From this point, conf_remap plugin will not take any records.config like
file, records.yaml format should be used instead.
The inline functionality wasn't altered at all.
All files should be converted using the provided script inside the
tools/records folder. There is also documentation available to accomplish this.
---
doc/admin-guide/plugins/conf_remap.en.rst | 7 +-
plugins/conf_remap/conf_remap.cc | 209 +++++++++++----------
.../gold_tests/remap/basic_conf_remap_yaml.test.py | 189 +++++++++++++++++++
tests/gold_tests/remap/conf_remap_float.test.py | 12 +-
tests/gold_tests/remap/gold/200OK_test.gold | 13 ++
5 files changed, 320 insertions(+), 110 deletions(-)
diff --git a/doc/admin-guide/plugins/conf_remap.en.rst
b/doc/admin-guide/plugins/conf_remap.en.rst
index 6c05caf016..bfde8f7007 100644
--- a/doc/admin-guide/plugins/conf_remap.en.rst
+++ b/doc/admin-guide/plugins/conf_remap.en.rst
@@ -89,8 +89,8 @@ sync across all the remapping rules over time.
Instead of specifying the directives and their values in :file:`remap.config`
as you do with the in-line method, you place all the affected directives in a
-separate text file. The location and name is entirely up to you, but we'll use
-`/etc/trafficserver/cdn_conf_remap.config` here. The contents of this file
+separate YAML file. The location and name is entirely up to you, but we'll use
+`/etc/trafficserver/cdn_conf_remap.yaml` here. The contents of this file
should mirror how configuration directives are written in :file:`records.yaml`:
.. code-block:: yaml
@@ -103,7 +103,7 @@ Your :file:`remap.config` will then contain remapping rules
that point to this
external file::
map http://cdn.example.com/ http://some-server.example.com \
- @plugin=conf_remap.so @pparam=/etc/trafficserver/cdn_conf_remap.config
+ @plugin=conf_remap.so @pparam=/etc/trafficserver/cdn_conf_remap.yaml
Your external configuration may contain as many directives as you wish.
@@ -122,3 +122,4 @@ For more information about the implementation of
overridable configuration
directives, you may consult the developer's documentation for
:ref:`ts-overridable-config`.
+Also if moving from a legacy config format, please have a look at
:ref:`rec-config-to-yaml`.
diff --git a/plugins/conf_remap/conf_remap.cc b/plugins/conf_remap/conf_remap.cc
index b222e1cd96..74a3983fd3 100644
--- a/plugins/conf_remap/conf_remap.cc
+++ b/plugins/conf_remap/conf_remap.cc
@@ -19,6 +19,8 @@
#include "ts/ts.h"
#include "ts/remap.h"
#include "tscore/ink_defs.h"
+#include "tscpp/util/YamlCfg.h"
+#include <swoc/bwf_base.h>
#include <cstdio>
#include <cstring>
@@ -122,16 +124,99 @@ RemapConfigs::parse_inline(const char *arg)
return true;
}
-// Config file parser, somewhat borrowed from P_RecCore.i
-bool
-RemapConfigs::parse_file(const char *filename)
+namespace
+{
+TSRecordDataType
+try_deduce_type(YAML::Node node)
+{
+ std::string_view tag = node.Tag();
+ if (tag == ts::Yaml::YAML_FLOAT_TAG_URI) {
+ return TS_RECORDDATATYPE_FLOAT;
+ } else if (tag == ts::Yaml::YAML_INT_TAG_URI) {
+ return TS_RECORDDATATYPE_INT;
+ } else if (tag == ts::Yaml::YAML_STR_TAG_URI) {
+ return TS_RECORDDATATYPE_STRING;
+ }
+ // we only care about string, int and float.
+ return TS_RECORDDATATYPE_NULL;
+}
+/// @brief Context class used to pass information to the TSYAMLRecNodeHandler
as the data obj.
+struct Context {
+ RemapConfigs::Item *items;
+ int *current;
+};
+
+TSReturnCode
+scalar_node_handler(const TSYAMLRecCfgFieldData *cfg, void *data)
{
- int line_num = 0;
- TSFile file;
- char buf[8192];
TSOverridableConfigKey name;
- TSRecordDataType type, expected_type;
+ TSRecordDataType expected_type;
+ std::string text;
+ auto &ctx = *static_cast<Context *>(data);
+ YAML::Node value = *reinterpret_cast<YAML::Node *>(cfg->value_node);
+
+ if (TSHttpTxnConfigFind(cfg->record_name, strlen(cfg->record_name), &name,
&expected_type) != TS_SUCCESS) {
+ TSError("[%s] '%s' is not a configuration variable or cannot be
overridden", PLUGIN_NAME, cfg->record_name);
+ return TS_ERROR;
+ }
+
+ RemapConfigs::Item *item = &ctx.items[*ctx.current];
+
+ auto type = try_deduce_type(value);
+ TSDebug(PLUGIN_NAME, "### deduced type %d for %s", type, cfg->record_name);
+ // If we detected a type but it's different from the one registered with the
in ATS, then we ignore it.
+ if (type != TS_RECORDDATATYPE_NULL && expected_type != type) {
+ TSError("%s", swoc::bwprint(text, "[{}] '{}' variable type mismatch,
expected {}, got {}", PLUGIN_NAME, cfg->record_name,
+ static_cast<int>(expected_type),
static_cast<int>(type))
+ .c_str());
+ return TS_ERROR; // Ignore the field
+ }
+
+ // If no type set or the type did match, then we assume it's safe to use the
+ // expected type.
+ try {
+ switch (expected_type) {
+ case TS_RECORDDATATYPE_INT:
+ item->_data.rec_int = value.as<int64_t>();
+ break;
+ case TS_RECORDDATATYPE_STRING: {
+ std::string str = value.as<std::string>();
+ if (value.IsNull() || str == "NULL") {
+ item->_data.rec_string = nullptr;
+ item->_data_len = 0;
+ } else {
+ item->_data.rec_string = TSstrdup(str.c_str());
+ item->_data_len = str.size();
+ }
+ } break;
+ case TS_RECORDDATATYPE_FLOAT:
+ item->_data.rec_float = value.as<float>();
+ break;
+ default:
+ TSError("[%s] field %s: type(%d) not support (unheard of)", PLUGIN_NAME,
cfg->field_name, expected_type);
+ return TS_ERROR;
+
+ ;
+ }
+ } catch (YAML::BadConversion const &e) {
+ TSError("%s", swoc::bwprint(text, "[{}] We couldn't convert the passed
field({}) value({}) to the expected type {}. {}",
+ PLUGIN_NAME, cfg->field_name,
value.as<std::string>(), static_cast<int>(expected_type), e.what())
+ .c_str());
+ return TS_ERROR;
+ }
+
+ item->_name = name;
+ item->_type = expected_type;
+ ++*ctx.current;
+
+ return TS_SUCCESS;
+}
+} // namespace
+
+bool
+RemapConfigs::parse_file(const char *filename)
+{
std::string path;
if (!filename || ('\0' == *filename)) {
@@ -148,108 +233,27 @@ RemapConfigs::parse_file(const char *filename)
path += filename;
}
- if (nullptr == (file = TSfopen(path.c_str(), "r"))) {
- TSError("[%s] Could not open config file %s", PLUGIN_NAME, path.c_str());
- return false;
- }
-
TSDebug(PLUGIN_NAME, "loading configuration file %s", path.c_str());
- while (nullptr != TSfgets(file, buf, sizeof(buf))) {
- char *ln, *tok;
- char *s = buf;
-
- ++line_num; // First line is #1 ...
- while (isspace(*s)) {
- ++s;
- }
- tok = strtok_r(s, " \t", &ln);
-
- // check for blank lines and comments
- if ((!tok) || (tok && ('#' == *tok))) {
- continue;
- }
-
- if (strncmp(tok, "CONFIG", 6)) {
- TSError("[%s] File %s, line %d: non-CONFIG line encountered",
PLUGIN_NAME, path.c_str(), line_num);
- continue;
- }
+ YAML::Node root;
+ try {
+ root = YAML::LoadFile(path);
- // Find the configuration name
- tok = strtok_r(nullptr, " \t", &ln);
- if (TSHttpTxnConfigFind(tok, -1, &name, &expected_type) != TS_SUCCESS) {
- TSError("[%s] File %s, line %d: %s is not a configuration variable or
cannot be overridden", PLUGIN_NAME, path.c_str(),
- line_num, tok);
- continue;
- }
-
- // Find the type (INT or STRING only)
- tok = strtok_r(nullptr, " \t", &ln);
- if (TS_RECORDDATATYPE_NULL == (type = str_to_datatype(tok))) {
- TSError("[%s] file %s, line %d: only INT, STRING, and FLOAT types
supported", PLUGIN_NAME, path.c_str(), line_num);
- continue;
- }
-
- if (type != expected_type) {
- TSError("[%s] file %s, line %d: mismatch between provide data type, and
expected type", PLUGIN_NAME, path.c_str(), line_num);
- continue;
- }
+ } catch (std::exception const &ex) {
+ std::string text;
+ TSError("[%s] We found an error while parsing '%s'.", PLUGIN_NAME,
swoc::bwprint(text, "{}. {}", path, ex.what()).c_str());
+ return false;
+ }
- // Find the value (which depends on the type above)
- if (ln) {
- while (isspace(*ln)) {
- ++ln;
- }
- if ('\0' == *ln) {
- tok = nullptr;
- } else {
- tok = ln;
- while (*ln != '\0') {
- ++ln;
- }
- --ln;
- while (isspace(*ln) && (ln > tok)) {
- --ln;
- }
- ++ln;
- *ln = '\0';
- }
- } else {
- tok = nullptr;
- }
- if (!tok) {
- TSError("[%s] file %s, line %d: the configuration must provide a value",
PLUGIN_NAME, path.c_str(), line_num);
- continue;
- }
+ Context ctx{&*_items, &_current}; // Context object will be passed on every
callback so the handler can fill in
+ // the details.
+ auto ret = TSRecYAMLConfigParse(reinterpret_cast<TSYaml>(&root),
scalar_node_handler, &ctx);
- // Now store the new config
- switch (type) {
- case TS_RECORDDATATYPE_INT:
- _items[_current]._data.rec_int = strtoll(tok, nullptr, 10);
- break;
- case TS_RECORDDATATYPE_STRING:
- if (strcmp(tok, "NULL") == 0) {
- _items[_current]._data.rec_string = nullptr;
- _items[_current]._data_len = 0;
- } else {
- _items[_current]._data.rec_string = TSstrdup(tok);
- _items[_current]._data_len = strlen(tok);
- }
- break;
- case TS_RECORDDATATYPE_FLOAT:
- _items[_current]._data.rec_float = strtof(tok, nullptr);
- break;
- default:
- TSError("[%s] file %s, line %d: type not support (unheard of)",
PLUGIN_NAME, path.c_str(), line_num);
- continue;
- break;
- }
- _items[_current]._name = name;
- _items[_current]._type = type;
- ++_current;
+ if (ret != TS_SUCCESS) {
+ TSError("[%s] We found an error while parsing '%s'.", PLUGIN_NAME,
path.c_str());
+ return false;
}
- TSfclose(file);
return (_current > 0);
}
@@ -327,7 +331,6 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *
/* rri ATS_UNUSED */
if (nullptr != ih) {
RemapConfigs *conf = static_cast<RemapConfigs *>(ih);
TSHttpTxn txnp = static_cast<TSHttpTxn>(rh);
-
for (int ix = 0; ix < conf->_current; ++ix) {
switch (conf->_items[ix]._type) {
case TS_RECORDDATATYPE_INT:
diff --git a/tests/gold_tests/remap/basic_conf_remap_yaml.test.py
b/tests/gold_tests/remap/basic_conf_remap_yaml.test.py
new file mode 100644
index 0000000000..b980e65f98
--- /dev/null
+++ b/tests/gold_tests/remap/basic_conf_remap_yaml.test.py
@@ -0,0 +1,189 @@
+'''
+'''
+# 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 conf_remap using a yaml file.
+'''
+
+Test.ContinueOnFail = True
+
+
+class conf_remap_yaml_load_test:
+ """Test conf_remap using a yaml file."""
+
+ client_counter: int = 0
+ ts_counter: int = 0
+ server_counter: int = 0
+
+ def __init__(self, name: str, gold_file="", remap_filename="",
remap_content=""):
+ """Initialize the test.
+ :param name: The name of the test.
+ :param gold_file: Gold file to be checked.
+ :param remap_filename: Remap yaml filename.
+ :param remap_content: remap yaml file content.
+ """
+ self.name = name
+ self.gold_file = gold_file
+ self._remap_filename = remap_filename
+ self._remap_content = remap_content
+
+ def _configure_server(self, tr: 'TestRun'):
+ """Configure the server.
+
+ :param tr: The TestRun object to associate the server process with.
+ """
+ server =
Test.MakeOriginServer(f"server-{conf_remap_yaml_load_test.ts_counter}",
lookup_key="{%Host}{PATH}")
+ request_header2 = {"headers": "GET /test HTTP/1.1\r\nHost:
www.testexample.com\r\n\r\n",
+ "timestamp": "1469733493.993", "body": ""}
+ response_header2 = {"headers": "HTTP/1.1 200 OK\r\nConnection:
close\r\n\r\n",
+ "timestamp": "1469733493.993", "body": ""}
+
+ server.addResponse("sessionfile.log", request_header2,
response_header2)
+ conf_remap_yaml_load_test.server_counter += 1
+ self._server = server
+
+ def _configure_traffic_server(self, tr: 'TestRun'):
+ """Configure Traffic Server.
+
+ :param tr: The TestRun object to associate the ts process with.
+ """
+ ts = Test.MakeATSProcess(f"ts-{conf_remap_yaml_load_test.ts_counter}")
+
+ conf_remap_yaml_load_test.ts_counter += 1
+ ts.Disk.records_config.update(
+ '''
+ diags:
+ debug:
+ enabled: 1
+ tags: conf_remap
+ dns:
+ resolv_conf: NULL
+ http:
+ referer_filter: 1
+ url_remap:
+ pristine_host_hdr: 0 # make sure is 0
+
+ '''
+ )
+ self._ts = ts
+
+ def run(self, diags_fail_exp="", ts_retcode=0):
+ """Run the test.
+ :param diags_fail_exp: Text to be included to validate the error.
+ :param ts_retcode: Expected return code from TS.
+ """
+ tr = Test.AddTestRun(self.name)
+ self._configure_server(tr)
+ self._configure_traffic_server(tr)
+
+ tr.Processes.Default.StartBefore(self._server)
+ tr.Processes.Default.StartBefore(self._ts)
+
+ self._ts.ReturnCode = ts_retcode
+
+ if ts_retcode > 0: # we could have errors logged and yet, we still
want to move on.
+ self._ts.Ready = 0
+
+ if diags_fail_exp != "":
+ # some error logs will be written to the diags.
+ self._ts.Disk.diags_log.Content = Testers.IncludesExpression(
+ diags_fail_exp, "Have a look.")
+ else:
+ tr.Processes.Default.ReturnCode = 0
+
+ if self.gold_file:
+ tr.Processes.Default.Streams.stderr = self.gold_file
+
+ if self._remap_filename != "" and self._remap_content != "":
+
self._ts.Disk.MakeConfigFile(self._remap_filename).update(self._remap_content)
+ self._ts.Disk.remap_config.AddLine(
+ f'map http://www.testexample.com/
http://127.0.0.1:{self._server.Variables.Port} @plugin=conf_remap.so
@pparam={self._remap_filename}'
+ )
+
+ tr.Processes.Default.Command = 'curl --proxy 127.0.0.1:{0}
"http://www.testexample.com/test"; -H "Host: www.testexample.com"
--verbose'.format(
+ self._ts.Variables.port)
+ conf_remap_yaml_load_test.client_counter += 1
+
+
+test0 = conf_remap_yaml_load_test(
+ "Test success",
+ gold_file="gold/200OK_test.gold",
+ remap_filename="testexample_remap.yaml",
+ remap_content='''
+ ts:
+ url_remap:
+ pristine_host_hdr: 1
+ '''
+)
+test0.run()
+
+test1 = conf_remap_yaml_load_test(
+ "Test mismatch type",
+ remap_filename="mismatch_field_type_remap.yaml",
+ remap_content='''
+ ts:
+ url_remap:
+ pristine_host_hdr: !!float '1'
+ '''
+)
+test1.run(diags_fail_exp="'proxy.config.url_remap.pristine_host_hdr' variable
type mismatch", ts_retcode=33)
+
+test2 = conf_remap_yaml_load_test(
+ "Test invalid variable",
+ remap_filename="invalid1_field_type_remap.yaml",
+ remap_content='''
+ ts:
+ plugin:
+ dynamic_reload_mode: 1
+ '''
+)
+
+test2.run(diags_fail_exp="'proxy.config.plugin.dynamic_reload_mode' is not a
configuration variable or cannot be overridden", ts_retcode=33)
+
+
+# We let the conf_remap parse two fields, only one is valid, we expect ATS to
start and the invalid fields ignored.
+test3 = conf_remap_yaml_load_test(
+ "Test success",
+ gold_file="gold/200OK_test.gold",
+ remap_filename="testexample2_remap.yaml",
+ remap_content='''
+ ts:
+ plugin:
+ dynamic_reload_mode: 1
+
+ url_remap:
+ pristine_host_hdr: 1
+ '''
+)
+test3.run(diags_fail_exp="'proxy.config.plugin.dynamic_reload_mode' is not a
configuration variable or cannot be overridden")
+
+
+# Check null values
+test4 = conf_remap_yaml_load_test(
+ "Test success - with NULL variable",
+ gold_file="gold/200OK_test.gold",
+ remap_filename="testexample_remap.yaml",
+ remap_content='''
+ ts:
+ url_remap:
+ pristine_host_hdr: 1
+ hostdb:
+ ip_resolve: "NULL" # We want to make sure this gets read as it should.
"NULL" could be the value of this field.
+ '''
+)
+test4.run()
diff --git a/tests/gold_tests/remap/conf_remap_float.test.py
b/tests/gold_tests/remap/conf_remap_float.test.py
index 4bb67f294a..29e0135528 100644
--- a/tests/gold_tests/remap/conf_remap_float.test.py
+++ b/tests/gold_tests/remap/conf_remap_float.test.py
@@ -22,12 +22,16 @@ Test.testName = 'Float in conf_remap Config Test'
ts = Test.MakeATSProcess("ts")
-ts.Disk.MakeConfigFile('conf_remap.config').AddLines([
- 'CONFIG proxy.config.http.background_fill_completed_threshold FLOAT
0.500000'
-])
+ts.Disk.MakeConfigFile('conf_remap.yaml').update(
+ '''
+ts:
+ http:
+ background_fill_completed_threshold: !!float '0.5'
+'''
+)
ts.Disk.remap_config.AddLine(
- f"map http://cdn.example.com/ http://origin.example.com/
@plugin=conf_remap.so @pparam={Test.RunDirectory}/ts/config/conf_remap.config"
+ f"map http://cdn.example.com/ http://origin.example.com/
@plugin=conf_remap.so @pparam={Test.RunDirectory}/ts/config/conf_remap.yaml"
)
tr = Test.AddTestRun("traffic_ctl command")
diff --git a/tests/gold_tests/remap/gold/200OK_test.gold
b/tests/gold_tests/remap/gold/200OK_test.gold
new file mode 100644
index 0000000000..887d5fb3ac
--- /dev/null
+++ b/tests/gold_tests/remap/gold/200OK_test.gold
@@ -0,0 +1,13 @@
+``
+> GET http://www.testexample.com/test``
+> Host: www.testexample.com``
+> User-Agent: curl/``
+> Accept: */*
+``
+< HTTP/1.1 200 OK
+< Date: ``
+< Age: ``
+< Transfer-Encoding: chunked
+< Proxy-Connection: keep-alive
+< Server: ATS/``
+``
