From 9357ac1464c98e441a002bfb1dcd76d2cd59ba4d Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Mon, 13 Feb 2023 16:25:14 +0000 Subject: [PATCH 1/3] records.yaml - Add support to load multiple YAML docs from the same stream/file. This will work as the legacy records.config where a later variable would override a previous value. --- src/records/RecConfigParse.cc | 11 +++- .../autest-site/trafficserver.test.ext | 60 ++++++++++++++----- tests/gold_tests/records/records_yaml.test.py | 51 ++++++++++++++++ 3 files changed, 103 insertions(+), 19 deletions(-) diff --git a/src/records/RecConfigParse.cc b/src/records/RecConfigParse.cc index c989323e86c..8dc8a8efae0 100644 --- a/src/records/RecConfigParse.cc +++ b/src/records/RecConfigParse.cc @@ -268,9 +268,14 @@ swoc::Errata RecYAMLConfigFileParse(const char *path, RecYAMLNodeHandler handler) { try { - auto root = YAML::LoadFile(path); - if (auto ret = ParseRecordsFromYAML(root, handler); !ret.empty()) { - return ret; + auto nodes = YAML::LoadAllFromFile(path); + // We will parse each doc from the stream, subsequently config node will overwrite + // the value for previously loaded records. This would work similar to the old + // records.config which a later CONFIG variable would overwrite a previous one. + for (auto const &root : nodes) { + if (auto ret = ParseRecordsFromYAML(root, handler); !ret.empty()) { + return ret; + } } } catch (std::exception const &ex) { return swoc::Errata(ERRATA_ERROR, "Error parsing {}. {}", path, ex.what()); diff --git a/tests/gold_tests/autest-site/trafficserver.test.ext b/tests/gold_tests/autest-site/trafficserver.test.ext index 06b863c46b8..ba6335572f8 100755 --- a/tests/gold_tests/autest-site/trafficserver.test.ext +++ b/tests/gold_tests/autest-site/trafficserver.test.ext @@ -514,7 +514,10 @@ class YAMLFile(File): execute=False, runtime=True) self.__root_tag = root_tag - self.__content = {} + # We support multiple docs in the same file, every time append_to_document + # is called, a new item will be created in the following list. At the end + # all of them will be written in the same stream. + self.__content = [{}] self.WriteCustomOn(self._do_write) @@ -531,18 +534,29 @@ class YAMLFile(File): with open(name, 'w') as f: yaml.add_representer(float, float_representer) yaml.add_representer(int, int_representer) - updated_content = {} - if self.__root_tag: - updated_content[self.__root_tag] = self.__content - else: - updated_content = self.__content + docs = [] + for content in self.__content: + updated_content = {} - yaml.dump(updated_content, f) + if self.__root_tag: + updated_content[self.__root_tag] = content + else: + updated_content = content + + docs.append(updated_content) + + yaml.dump_all(docs, f) return (True, "Writing config file {0}".format(os.path.split(self.Name)[-1]), "Success") + def __transform_content(self, content): + if isinstance(content, str): # from new style + return yaml.load(content, Loader=yaml.Loader) + else: + return content # Already an object. + def __update(self, content, out): for key, value in content.items(): if key in out: @@ -556,13 +570,13 @@ class YAMLFile(File): out[key] = value def update(self, content): - data = {} - if isinstance(content, str): # from new style - data = yaml.load(content, Loader=yaml.Loader) - else: - data = content # Already an object. + self.__update(self.__transform_content(content), self.__content[0]) - self.__update(data, self.__content) + def append_to_document(self, content): + # We want to append a new doc in the same YAML stream. + new_doc = {} + self.__update(self.__transform_content(content), new_doc) + self.__content.append(new_doc) class RecordsYAML(YAMLFile): @@ -599,7 +613,7 @@ class RecordsYAML(YAMLFile): self._make_obj_from_legacy_record(config[key], var[index + 1:], value) - def __legacy_update(self, obj): + def __legacy_update(self, obj, new_doc=False): config = {} for name, value in obj.items(): ori_name = name @@ -612,7 +626,10 @@ class RecordsYAML(YAMLFile): # unwrap the record and make it a YAML field. self._make_obj_from_legacy_record(config, name, value) - super(RecordsYAML, self).update(config) + if new_doc: + super(RecordsYAML, self).append_to_document(config) + else: + super(RecordsYAML, self).update(config) def update(self, data): if isinstance(data, dict): @@ -627,7 +644,18 @@ class RecordsYAML(YAMLFile): ''' super(RecordsYAML, self).update(data) - + def append_to_document(self, data): + if isinstance(data, dict): + ''' + If the data is already parsed or (more likely) is a legacy objects + we still support it, so update from the passed object. + ''' + self.__legacy_update(data, new_doc=True) + else: + ''' + We also support plain YAML(str) to be passed on. Base class will load from the yaml parser. + ''' + super(RecordsYAML, self).append_to_document(data) ########################################################################## diff --git a/tests/gold_tests/records/records_yaml.test.py b/tests/gold_tests/records/records_yaml.test.py index e00a5e3f8c2..cc1d736e39c 100644 --- a/tests/gold_tests/records/records_yaml.test.py +++ b/tests/gold_tests/records/records_yaml.test.py @@ -106,3 +106,54 @@ def check_response(resp: Response): 'proxy.config.cache.ram_cache.size: 32212254720', 'Should hold the configured value.' ) + + +# The whole idea is to test how ATS handles having multiple docs in the same records.yaml +# file. +# Every subsequent document will overwrite the previous one, this tests are meant to +# exercise that logic. + +ts2 = Test.MakeATSProcess("ts2") + +# We are adding a new doc at the end of the original one created by the unit test. +ts2.Disk.records_config.append_to_document( + ''' + diags: + debug: + enabled: 0 + tags: rpc|rec + ''' +) + +# This will append a new doc in the same file, this config value should overwrite +# the previous one. +ts2.Disk.records_config.append_to_document( + ''' + diags: + debug: + tags: filemanager + ''' +) + +# After all the loading completed we want to have enabled=0 and tags=filemanager + +tr = Test.AddTestRun("Start a new ATS instance.") +tr.Processes.Default.Command = 'echo 1' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.StartBefore(ts2) +tr.Processes.StillRunningAfter = ts2 + +tr2 = Test.AddTestRun("Test multiple docs from the same file") +tr2.Processes.Default.Command = 'traffic_ctl config get proxy.config.diags.debug.enabled proxy.config.diags.debug.tags' +tr2.Processes.Default.Env = ts2.Env +tr2.Processes.Default.ReturnCode = 0 + +# Make sure it's what we want. +tr2.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.enabled: 0', + 'Config should show debug disabled' +) +tr2.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: filemanager', + 'Config should show a different tag' +) From e060a17071da40891e84c780a81b65a4a2f9d3ad Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 14 Feb 2023 13:14:41 +0000 Subject: [PATCH 2/3] traffic_ctl: Allow config set/get from a yaml configuration file. --- src/traffic_ctl/CtrlCommands.cc | 15 +- src/traffic_ctl/CtrlCommands.h | 61 ++-- src/traffic_ctl/FileConfigCommand.cc | 340 ++++++++++++++++++ src/traffic_ctl/FileConfigCommand.h | 88 +++++ src/traffic_ctl/Makefile.inc | 1 + src/traffic_ctl/traffic_ctl.cc | 19 +- src/traffic_ctl/utils/yaml_utils.h | 32 ++ .../records/gold/records.yaml.cold_test0.gold | 9 + .../records/gold/records.yaml.cold_test2.gold | 9 + .../records/gold/records.yaml.cold_test4.gold | 10 + .../records/gold/records.yaml.cold_test5.gold | 8 + .../records/traffic_ctl_cold_config.test.py | 100 ++++++ 12 files changed, 661 insertions(+), 31 deletions(-) create mode 100644 src/traffic_ctl/FileConfigCommand.cc create mode 100644 src/traffic_ctl/FileConfigCommand.h create mode 100644 src/traffic_ctl/utils/yaml_utils.h create mode 100644 tests/gold_tests/records/gold/records.yaml.cold_test0.gold create mode 100644 tests/gold_tests/records/gold/records.yaml.cold_test2.gold create mode 100644 tests/gold_tests/records/gold/records.yaml.cold_test4.gold create mode 100644 tests/gold_tests/records/gold/records.yaml.cold_test5.gold create mode 100644 tests/gold_tests/records/traffic_ctl_cold_config.test.py diff --git a/src/traffic_ctl/CtrlCommands.cc b/src/traffic_ctl/CtrlCommands.cc index 682879b82d5..cbfbdb41c4b 100644 --- a/src/traffic_ctl/CtrlCommands.cc +++ b/src/traffic_ctl/CtrlCommands.cc @@ -24,11 +24,16 @@ #include "CtrlCommands.h" #include "jsonrpc/CtrlRPCRequests.h" #include "jsonrpc/ctrl_yaml_codecs.h" +#include "utils/yaml_utils.h" +#include "swoc/TextView.h" +#include "swoc/BufferWriter.h" +#include "swoc/bwf_base.h" + namespace { /// We use yamlcpp as codec implementation. using Codec = yamlcpp_json_emitter; - +} // namespace const std::unordered_map _Fmt_str_to_enum = { {"pretty", BasePrinter::Options::OutputFormat::PRETTY}, {"legacy", BasePrinter::Options::OutputFormat::LEGACY}, @@ -59,7 +64,6 @@ parse_print_opts(ts::Arguments *args) { return {parse_format(args)}; } -} // namespace //------------------------------------------------------------------------------------------------------------------------------------ CtrlCommand::CtrlCommand(ts::Arguments *args) : _arguments(args) {} @@ -75,7 +79,7 @@ CtrlCommand::execute() } std::string -CtrlCommand::invoke_rpc(std::string const &request) +RPCAccessor::invoke_rpc(std::string const &request) { if (_printer->print_rpc_message()) { std::string text; @@ -96,7 +100,7 @@ CtrlCommand::invoke_rpc(std::string const &request) } shared::rpc::JSONRPCResponse -CtrlCommand::invoke_rpc(shared::rpc::ClientRequest const &request) +RPCAccessor::invoke_rpc(shared::rpc::ClientRequest const &request) { std::string encodedRequest = Codec::encode(request); std::string resp = invoke_rpc(encodedRequest); @@ -104,7 +108,7 @@ CtrlCommand::invoke_rpc(shared::rpc::ClientRequest const &request) } void -CtrlCommand::invoke_rpc(shared::rpc::ClientRequest const &request, std::string &resp) +RPCAccessor::invoke_rpc(shared::rpc::ClientRequest const &request, std::string &resp) { std::string encodedRequest = Codec::encode(request); resp = invoke_rpc(encodedRequest); @@ -207,6 +211,7 @@ ConfigCommand::config_set() _printer->write_output(response); } + void ConfigCommand::config_reload() { diff --git a/src/traffic_ctl/CtrlCommands.h b/src/traffic_ctl/CtrlCommands.h index 3fc9a2d38ab..abb7583a1bd 100644 --- a/src/traffic_ctl/CtrlCommands.h +++ b/src/traffic_ctl/CtrlCommands.h @@ -26,6 +26,30 @@ limitations under the License. #include "shared/rpc/RPCClient.h" #include "CtrlPrinters.h" +/// @brief Class that provides access to the RPC side of things. +struct RPCAccessor { +protected: + /// @brief Invoke the remote server. This is the very basic function which does not play or interact with any codec. Request + /// and message should be already en|de coded. + /// @param request A string representation of the json/yaml request. + /// @return a string with the json/yaml response. + /// @note This function does print the raw string if requested by the "--format". No printer involved, standard output. + std::string invoke_rpc(std::string const &request); + + /// @brief Function that calls the rpc server. This function takes a json objects and uses the defined coded to convert them to a + /// string. This function will call invoke_rpc(string) overload. + /// @param A Client request. + /// @return A server response. + shared::rpc::JSONRPCResponse invoke_rpc(shared::rpc::ClientRequest const &request); + + /// @brief Function that calls the rpc server. The response will not be decoded, it will be a raw string. + void invoke_rpc(shared::rpc::ClientRequest const &request, std::string &bw); + + std::unique_ptr _printer; //!< Specific output formatter. This should be created by the derived class. +private: + shared::rpc::RPCClient _rpcClient; //!< RPC socket client implementation. +}; + // ---------------------------------------------------------------------------------------------------------------------------------- /// /// @brief Base Control Command class. @@ -49,24 +73,6 @@ class CtrlCommand virtual void execute(); protected: - /// @brief Invoke the remote server. This is the very basic function which does not play or interact with any codec. Request - /// and message should be already en|de coded. - /// @param request A string representation of the json/yaml request. - /// @return a string with the json/yaml response. - /// @note This function does print the raw string if requested by the "--format". No printer involved, standard output. - std::string invoke_rpc(std::string const &request); - - /// @brief Function that calls the rpc server. This function takes a json objects and uses the defined coded to convert them to a - /// string. This function will call invoke_rpc(string) overload. - /// @param A Client request. - /// @return A server response. - shared::rpc::JSONRPCResponse invoke_rpc(shared::rpc::ClientRequest const &request); - - /// @brief Function that calls the rpc server. The response will not be decoded, it will be a raw string. - void invoke_rpc(shared::rpc::ClientRequest const &request, std::string &bw); - - std::unique_ptr _printer; //!< Specific output formatter. This should be created by the derived class. - /// @brief The whole design is that the command will execute the @c _invoked_func once invoked. This function ptr should be /// set by the appropriated derived class base on the passed parameters. The derived class have the option to override /// the execute() function and do something else. Check @c RecordCommand as an example. @@ -81,7 +87,6 @@ class CtrlCommand private: ts::Arguments *_arguments = nullptr; //!< parsed traffic_ctl arguments. - shared::rpc::RPCClient _rpcClient; //!< RPC socket client implementation. }; // ----------------------------------------------------------------------------------------------------------------------------------- @@ -89,7 +94,7 @@ class CtrlCommand /// @brief Record Command Implementation /// Used as base class for any command that needs to access to a TS record. /// If deriving from this class, make sure you implement @c execute_subcommand() and call the _invoked_func yourself. -class RecordCommand : public CtrlCommand +class RecordCommand : public CtrlCommand, public RPCAccessor { public: using CtrlCommand::CtrlCommand; @@ -114,6 +119,8 @@ class ConfigCommand : public RecordCommand static inline const std::string DIFF_STR{"diff"}; static inline const std::string DEFAULTS_STR{"defaults"}; static inline const std::string SET_STR{"set"}; + static inline const std::string COLD_STR{"cold"}; + static inline const std::string APPEND_STR{"append"}; static inline const std::string STATUS_STR{"status"}; static inline const std::string RELOAD_STR{"reload"}; static inline const std::string REGISTRY_STR{"registry"}; @@ -125,6 +132,7 @@ class ConfigCommand : public RecordCommand void config_diff(); void config_status(); void config_set(); + void file_config_set(); void config_reload(); void config_show_file_registry(); @@ -147,7 +155,7 @@ class MetricCommand : public RecordCommand MetricCommand(ts::Arguments *args); }; // ----------------------------------------------------------------------------------------------------------------------------------- -class HostCommand : public CtrlCommand +class HostCommand : public CtrlCommand, public RPCAccessor { public: HostCommand(ts::Arguments *args); @@ -163,7 +171,7 @@ class HostCommand : public CtrlCommand void status_up(); }; // ----------------------------------------------------------------------------------------------------------------------------------- -class PluginCommand : public CtrlCommand +class PluginCommand : public CtrlCommand, public RPCAccessor { public: PluginCommand(ts::Arguments *args); @@ -173,7 +181,7 @@ class PluginCommand : public CtrlCommand void plugin_msg(); }; // ----------------------------------------------------------------------------------------------------------------------------------- -class DirectRPCCommand : public CtrlCommand +class DirectRPCCommand : public CtrlCommand, public RPCAccessor { public: DirectRPCCommand(ts::Arguments *args); @@ -194,7 +202,7 @@ class DirectRPCCommand : public CtrlCommand bool validate_input(std::string const &in) const; }; // ----------------------------------------------------------------------------------------------------------------------------------- -class ServerCommand : public CtrlCommand +class ServerCommand : public CtrlCommand, public RPCAccessor { public: ServerCommand(ts::Arguments *args); @@ -208,7 +216,7 @@ class ServerCommand : public CtrlCommand }; // // ----------------------------------------------------------------------------------------------------------------------------------- -struct StorageCommand : public CtrlCommand { +struct StorageCommand : public CtrlCommand, public RPCAccessor { StorageCommand(ts::Arguments *args); private: @@ -219,3 +227,6 @@ struct StorageCommand : public CtrlCommand { void set_storage_offline(); }; // ----------------------------------------------------------------------------------------------------------------------------------- + +BasePrinter::Options::OutputFormat parse_format(ts::Arguments *args); +BasePrinter::Options parse_print_opts(ts::Arguments *args); diff --git a/src/traffic_ctl/FileConfigCommand.cc b/src/traffic_ctl/FileConfigCommand.cc new file mode 100644 index 00000000000..b2f53b82ac5 --- /dev/null +++ b/src/traffic_ctl/FileConfigCommand.cc @@ -0,0 +1,340 @@ +/** @file + + @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 "FileConfigCommand.h" +#include "utils/yaml_utils.h" +#include "swoc/TextView.h" +#include "swoc/BufferWriter.h" +#include "swoc/bwf_base.h" + +namespace +{ +constexpr std::string_view PREFIX{"proxy.config."}; +constexpr std::string_view TS_PREFIX{"ts."}; + +constexpr bool CREATE_IF_NOT_EXIST{true}; +constexpr bool DO_NOT_CREATE_IF_NOT_EXIST{false}; +const std::pair NOT_FOUND{false, {}}; + +/// We support either passing variables with the prefix 'proxy.config.' or 'ts.' +/// Internally we need to use 'ts.variable' as the root node starts with 'ts' for records +/// configs. +std::string +amend_variable_name(swoc::TextView variable) +{ + std::string var{TS_PREFIX}; + // If the variable is prefixed with "proxy.config" we will remove it and replace it + // with the records "ts." root name. + if (swoc::TextView{variable}.starts_with(PREFIX)) { + var += variable.substr(PREFIX.size()); + return var; + } + + // you may be using "ts." already or some other name maybe for a different file. + // we expect either `ts` or `proxy.config` + return {variable.data(), variable.size()}; +} + +/// traffic_ctl should work without the need to pass the filename, so use the data +/// we have to figure out the file path. If the filename is specified in the traffic_ctl +/// arguments, then we use that. +void +fix_filename(std::string &filename) +{ + if (filename.empty()) { + std::string sysconfdir; + if (const char *env = getenv("PROXY_CONFIG_CONFIG_DIR")) { + sysconfdir = Layout::get()->relative(env); + } else { + sysconfdir = Layout::get()->sysconfdir; + } + filename = Layout::get()->relative_to(sysconfdir, "records.yaml"); + } +} + +/// Function to open a file if it exists, if not and is requested we will create the file. +std::string +open_file(std::string const &filename, std::fstream &fs, std::ios_base::openmode mode = std::ios::in | std::ios::out, + bool create = false) +{ + fs.open(filename, mode); + + if (!fs.is_open()) { + if (create) { + fs.clear(); + if (fs.open(filename, std::ios::out); fs.is_open()) { + return {}; + } + } + std::string text; + return swoc::bwprint(text, "We couldn't open '{}': {}", filename, strerror(errno)); + } + return {}; +} + +/// Bunch of mapping flags for the TAGS. +std::string +get_tag(swoc::TextView tag) +{ + static std::vector>> const Str_to_Tag{ + {yaml_utils::YAML_INT_TAG_URI, {"int", "i", "I", "INT", "integer"} }, + {yaml_utils::YAML_FLOAT_TAG_URI, {"float", "f", "F", "FLOAT"} }, + {yaml_utils::YAML_STR_TAG_URI, {"str", "s", "S", "STR", "string", "STRING"}} + }; + + for (auto const &[yaml_tag, strs] : Str_to_Tag) { + if (auto found = std::find_if(std::begin(strs), std::end(strs), [&tag](auto t) { return tag == t; }); found != std::end(strs)) { + return std::string{yaml_tag.data(), yaml_tag.size()}; + } + } + + return std::string{tag.data(), tag.size()}; +} + +std::string +get_leading_comment() +{ + std::string text; + std::time_t result = std::time(nullptr); + return swoc::bwprint(text, "Document modified by traffic_ctl {}", std::asctime(std::localtime(&result))); +} + +std::pair +search_node(swoc::TextView variable, YAML::Node root, bool create) +{ + auto const key{variable.take_prefix_at('.').remove_suffix_at('.')}; + auto const key_str = std::string{key.data(), key.size()}; + if (variable.empty()) { + if (root.IsMap() && root[key_str]) { + return {true, root[key_str]}; + } else { + if (create) { + YAML::Node n; + root[key_str] = n; + return {true, n}; // new one created; + } else { + return NOT_FOUND; + } + } + } + if (!root[key_str]) { + if (create) { + YAML::Node n; + root[key_str] = n; + return search_node(variable, n, create); + } + } else { + return search_node(variable, root[key_str], create); + } + return NOT_FOUND; +} +} // namespace + +YAML::Node +FlatYAMLAccessor::find_or_create_node(swoc::TextView variable, bool search_all) +{ + if (!search_all) { + if (_docs.size() == 0) { + // If nothing in it. Add one, it'll be the new node. + _docs.emplace_back(YAML::NodeType::Map); + } + return search_node(variable, _docs.back(), CREATE_IF_NOT_EXIST).second; + } else { + for (auto iter = _docs.rbegin(); iter != _docs.rend(); ++iter) { + if (auto [found, node] = search_node(variable, *iter, DO_NOT_CREATE_IF_NOT_EXIST); found) { + return node; + } + } + // We haven't found the node, so we will create a new field in the latest doc. + if (_docs.size() == 0) { + // if nothing in it. Add one, it'll be the new node. + _docs.emplace_back(YAML::NodeType::Map); + } + // Use the last doc. + return search_node(variable, _docs.back(), CREATE_IF_NOT_EXIST).second; + } + + return {}; +} + +std::pair +FlatYAMLAccessor::find_node(swoc::TextView variable) +{ + if (_docs.size() > 0) { // make sure there is something + // We start from the bottom. + for (auto iter = std::rbegin(_docs); iter != std::rend(_docs); ++iter) { + if (auto [found, node] = search_node(variable, *iter, DO_NOT_CREATE_IF_NOT_EXIST); found) { + // found it. + return {true, node}; + } + } + } + + return NOT_FOUND; // couldn't find the node; +} + +void +FlatYAMLAccessor::make_tree_node(swoc::TextView variable, swoc::TextView value, swoc::TextView tag, YAML::Emitter &out) +{ + auto key{variable.take_prefix_at('.').remove_suffix_at('.')}; + auto key_str = std::string{key.data(), key.size()}; + if (variable.empty()) { + out << YAML::BeginMap << YAML::Key << key_str; + if (!tag.empty()) { + out << YAML::VerbatimTag(get_tag(tag)); + } + out << YAML::Value << std::string{value.data(), value.size()} << YAML::EndMap; + } else { + out << YAML::BeginMap << YAML::Key << key_str; + make_tree_node(variable, value, tag, out); + out << YAML::EndMap; + } +} + +FileConfigCommand::FileConfigCommand(ts::Arguments *args) : CtrlCommand(args) +{ + BasePrinter::Options printOpts{parse_print_opts(args)}; + _printer = std::make_unique(printOpts); + if (args->get(SET_STR)) { + _invoked_func = [&]() { config_set(); }; + } else if (args->get(GET_STR)) { + _invoked_func = [&]() { config_get(); }; + } else { + throw std::invalid_argument("Can't deal with the provided arguments"); + } +} + +void +FileConfigCommand::config_get() +{ + auto filename = get_parsed_arguments()->get(COLD_STR).value(); // could be empty which means we should use the default file name + auto const &data = get_parsed_arguments()->get(GET_STR); + std::string text; + + fix_filename(filename); + try { + FlatYAMLAccessor::load(YAML::LoadAllFromFile(filename)); + + for (auto const &var : data) { // we support multiple get's + std::string variable = amend_variable_name(var); + auto [found, search] = find_node(variable); + + if (found) { + _printer->write_output(swoc::bwprint(text, "{}: {}", var, search.as())); + } else { + _printer->write_output(swoc::bwprint(text, "{} not found", var)); + } + } + } catch (YAML::Exception const &ex) { + throw std::logic_error(swoc::bwprint(text, "config get error: {}", ex.what())); + } +} + +void +FileConfigCommand::config_set() +{ + static const std::string CREATE_STR{"create"}; + static const std::string TYPE_STR{"type"}; + + auto filename = get_parsed_arguments()->get(COLD_STR).value(); // could be empty which means we should use the default file name + bool append = !get_parsed_arguments()->get(UPDATE_STR); + auto const &data = get_parsed_arguments()->get(SET_STR); + + std::string passed_tag; + if (auto p = get_parsed_arguments()->get(TYPE_STR); p) { + passed_tag = p.value(); + } + // Get the default records.yaml if nothing is passed. + fix_filename(filename); + if (filename.empty()) { + throw std::logic_error("Can't deduce the file path."); + } + + std::ios_base::openmode mode = std::ios::in | std::ios::out; + if (append) { + mode = std::ios::out | std::ios::app; + } + + std::fstream fs; + if (auto err = open_file(filename, fs, mode, true); !err.empty()) { + throw std::logic_error(err); + } + + std::string const &variable = amend_variable_name(data[0]); + try { + if (append) { + YAML::Emitter doc; // we will build the document again, either to append the + // new node or to modify the existing one. + doc << YAML::Comment(get_leading_comment()) << YAML::BeginDoc; + make_tree_node(variable, data[1], passed_tag, doc); + doc << YAML::Newline; + + fs.write(doc.c_str(), doc.size()); + fs.close(); + } else { + FlatYAMLAccessor::load(YAML::LoadAll(fs)); + fs.close(); + + auto new_node = find_or_create_node(variable); + + new_node = data[1]; + + if (!passed_tag.empty()) { + new_node.SetTag(get_tag(passed_tag)); + } + // try gain + if (auto err = open_file(filename, fs, std::ios::out | std::ios::trunc); !err.empty()) { + throw std::logic_error(err); + } + + YAML::Emitter doc; + YAML::Node last_node; + if (_docs.size() > 0) { + last_node = _docs.back(); + _docs.pop_back(); + } + + for (auto const &n : _docs) { + if (!n.IsNull()) { + doc << n; + } + } + + if (doc.size() > 0) { + // There is something already, so add a new line. + doc << YAML::Newline; + } + doc << YAML::Comment(get_leading_comment()) << YAML::BeginDoc << last_node << YAML::Newline; + + fs.write(doc.c_str(), doc.size()); + fs.close(); + } + std::string text; + _printer->write_output(swoc::bwprint(text, "Set {}", variable)); + } catch (std::exception const &ex) { + if (fs.is_open()) { + fs.close(); + } + throw ex; + } +} diff --git a/src/traffic_ctl/FileConfigCommand.h b/src/traffic_ctl/FileConfigCommand.h new file mode 100644 index 00000000000..866b15d608d --- /dev/null +++ b/src/traffic_ctl/FileConfigCommand.h @@ -0,0 +1,88 @@ +/** +@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. +*/ +#pragma once + +#include +#include +#include + +#include "tscore/ArgParser.h" +#include "CtrlCommands.h" +#include "swoc/TextView.h" + +/// @brief Very basic flat YAML node handling. +/// +/// The whole idea is to be able to set some YAML nodes and being able to search for flat nodes, nodes +/// can be created if requested. This should also help on creating the whole node tree from a legacy +/// record variable style. For more complex updates we should tweak this class a bit. +struct FlatYAMLAccessor { + /// + /// @brief Find a node based on the passed record variable name. + /// + /// This function will only search for a specific node, it will not create one. + /// @param variable the record name. + /// @return std::pair First value will be set to true if found and the relevant Node will be set in the second + /// parameter + /// + + std::pair find_node(swoc::TextView variable); + /// @brief Find a node based on the passed record variable name. Create if not exist. + /// + /// This function will first try to find the passed variable, if not it will create a new with the passed tree. + /// @param variable the record name. + /// @param all_docs Search in all the nodes from the parsed stream. Default is true. No need to change this now. + /// @return + YAML::Node find_or_create_node(swoc::TextView variable, bool all_docs = true); + + /// @brief Build up a YAML node including TAG and Value. This is used to append just a single variable in a file. + /// @param variable The record name. + /// @param value The value to be set on the node. + /// @param tag The tag to be set on the node. + /// @param out The YAML::Emitter used to build up the node. This can just be streamed out to a string or a file. + void make_tree_node(swoc::TextView variable, swoc::TextView value, swoc::TextView tag, YAML::Emitter &out); + + /// @brief Set the internal list of nodes from the parsed file. Caller should deal with the YAML::LoadAll or any other way. + /// @param streams A list of docs. + void + load(std::vector streams) + { + _docs = std::move(streams); + } + +protected: + std::vector _docs; + std::unique_ptr _printer; //!< Specific output formatter. This should be created by the derived class. +}; + +/// @brief Class used to deal with the config file changes. Append or modify an existing records.yaml field +class FileConfigCommand : public CtrlCommand, FlatYAMLAccessor +{ + static inline const std::string SET_STR{"set"}; // we support get, set only for now. + static inline const std::string GET_STR{"get"}; + + static inline const std::string COLD_STR{"cold"}; // Meaning that the change is on a file. + static inline const std::string UPDATE_STR{"update"}; // Append the new field to a file. + + void config_set(); + void config_get(); + +public: + FileConfigCommand(ts::Arguments *args); +}; diff --git a/src/traffic_ctl/Makefile.inc b/src/traffic_ctl/Makefile.inc index 4cc365e36ee..0a09ca1a899 100644 --- a/src/traffic_ctl/Makefile.inc +++ b/src/traffic_ctl/Makefile.inc @@ -30,6 +30,7 @@ traffic_ctl_traffic_ctl_SOURCES = \ traffic_ctl/traffic_ctl.cc \ traffic_ctl/CtrlPrinters.cc \ traffic_ctl/CtrlCommands.cc \ + traffic_ctl/FileConfigCommand.cc \ shared/rpc/IPCSocketClient.cc traffic_ctl_traffic_ctl_LDADD = \ diff --git a/src/traffic_ctl/traffic_ctl.cc b/src/traffic_ctl/traffic_ctl.cc index 06a1b444189..764ab1cea6a 100644 --- a/src/traffic_ctl/traffic_ctl.cc +++ b/src/traffic_ctl/traffic_ctl.cc @@ -28,6 +28,7 @@ #include "tscore/ArgParser.h" #include "CtrlCommands.h" +#include "FileConfigCommand.h" constexpr int CTRL_EX_OK = 0; constexpr int CTRL_EX_ERROR = 2; @@ -78,6 +79,9 @@ main(int argc, const char **argv) .add_option("--records", "", "Emit output in records.config format"); config_command.add_command("get", "Get one or more configuration values", "", MORE_THAN_ONE_ARG_N, [&]() { command->execute(); }) .add_example_usage("traffic_ctl config get [OPTIONS] RECORD [RECORD ...]") + .add_option("--cold", "-c", + "Save the value in a configuration file. This does not save the value in TS. Local file change only", + "TS_RECORD_YAML", MORE_THAN_ZERO_ARG_N) .add_option("--records", "", "Emit output in records.config format"); config_command .add_command("match", "Get configuration matching a regular expression", "", MORE_THAN_ONE_ARG_N, [&]() { command->execute(); }) @@ -88,6 +92,14 @@ main(int argc, const char **argv) config_command.add_command("status", "Check the configuration status", [&]() { command->execute(); }) .add_example_usage("traffic_ctl config status"); config_command.add_command("set", "Set a configuration value", "", 2, [&]() { command->execute(); }) + .add_option("--cold", "-c", + "Save the value in a configuration file. This does not save the value in TS. Local file change only", + "TS_RECORD_YAML", MORE_THAN_ZERO_ARG_N) + .add_option("--update", "-u", "Update a configuration value. [only relevant if --cold set]") + .add_option( + "--type", "-t", + "Add type tag to the yaml field. This is needed if the record is not registered inside ATS. [only relevant if --cold set]", + "", 1) .add_example_usage("traffic_ctl config set RECORD VALUE"); config_command.add_command("registry", "Show configuration file registry", [&]() { command->execute(); }) @@ -173,7 +185,12 @@ main(int argc, const char **argv) Layout::create(); if (args.get("config")) { - command = std::make_shared(&args); + if (args.get("cold")) { + // We allow to just change a config file + command = std::make_shared(&args); + } else { + command = std::make_shared(&args); + } } else if (args.get("metric")) { command = std::make_shared(&args); } else if (args.get("server")) { diff --git a/src/traffic_ctl/utils/yaml_utils.h b/src/traffic_ctl/utils/yaml_utils.h new file mode 100644 index 00000000000..348b81fb0ca --- /dev/null +++ b/src/traffic_ctl/utils/yaml_utils.h @@ -0,0 +1,32 @@ +/** @file + + @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. + */ +#pragma once +#include +#include + +namespace yaml_utils +{ +// TODO: This should be sourced from another place +constexpr std::string_view YAML_FLOAT_TAG_URI{"tag:yaml.org,2002:float"}; +constexpr std::string_view YAML_INT_TAG_URI{"tag:yaml.org,2002:int"}; +constexpr std::string_view YAML_STR_TAG_URI{"tag:yaml.org,2002:str"}; + +} // namespace yaml_utils diff --git a/tests/gold_tests/records/gold/records.yaml.cold_test0.gold b/tests/gold_tests/records/gold/records.yaml.cold_test0.gold new file mode 100644 index 00000000000..33329ca0e4f --- /dev/null +++ b/tests/gold_tests/records/gold/records.yaml.cold_test0.gold @@ -0,0 +1,9 @@ +ts: +`` +# Document modified by traffic_ctl `` +#`` +--- +ts: + diags: + debug: + tags: http diff --git a/tests/gold_tests/records/gold/records.yaml.cold_test2.gold b/tests/gold_tests/records/gold/records.yaml.cold_test2.gold new file mode 100644 index 00000000000..33329ca0e4f --- /dev/null +++ b/tests/gold_tests/records/gold/records.yaml.cold_test2.gold @@ -0,0 +1,9 @@ +ts: +`` +# Document modified by traffic_ctl `` +#`` +--- +ts: + diags: + debug: + tags: http diff --git a/tests/gold_tests/records/gold/records.yaml.cold_test4.gold b/tests/gold_tests/records/gold/records.yaml.cold_test4.gold new file mode 100644 index 00000000000..01175993094 --- /dev/null +++ b/tests/gold_tests/records/gold/records.yaml.cold_test4.gold @@ -0,0 +1,10 @@ +ts: +`` +# Document modified by traffic_ctl `` +#`` +--- +ts: + cache: + limits: + http: + max_alts: ! 1 diff --git a/tests/gold_tests/records/gold/records.yaml.cold_test5.gold b/tests/gold_tests/records/gold/records.yaml.cold_test5.gold new file mode 100644 index 00000000000..29e5ad3b0c3 --- /dev/null +++ b/tests/gold_tests/records/gold/records.yaml.cold_test5.gold @@ -0,0 +1,8 @@ +# Document modified by traffic_ctl `` +#`` +--- +ts: + cache: + limits: + http: + max_alts: 3 diff --git a/tests/gold_tests/records/traffic_ctl_cold_config.test.py b/tests/gold_tests/records/traffic_ctl_cold_config.test.py new file mode 100644 index 00000000000..608865c093f --- /dev/null +++ b/tests/gold_tests/records/traffic_ctl_cold_config.test.py @@ -0,0 +1,100 @@ +''' +''' +# 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 Notification, Request, Response + +Test.Summary = 'Basic records test. Testing the new records.yaml logic and making sure it works as expected.' + +ts = Test.MakeATSProcess("ts") + +ts.Disk.records_config.update( + ''' + accept_threads: 1 + cache: + limits: + http: + max_alts: 5 + diags: + debug: + enabled: 0 + tags: http|dns + ''' +) + +# 0 - We want to make sure that the unregistered records are still being detected. +tr = Test.AddTestRun("Test Append value to existing records.yaml") + +tr.Processes.Default.Command = 'traffic_ctl config set proxy.config.diags.debug.tags rpc --cold' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Processes.Default.StartBefore(ts) +ts.Disk.records_config.Content = 'gold/records.yaml.cold_test0.gold' + +# 1 +tr = Test.AddTestRun("Get value from latest added node.") +tr.Processes.Default.Command = 'traffic_ctl config get proxy.config.diags.debug.tags --cold' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env + +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: rpc', + 'Config should show the right tags' +) + +# 2 +tr = Test.AddTestRun("Test modify latest yaml document from records.yaml") +tr.Processes.Default.Command = 'traffic_ctl config set proxy.config.diags.debug.tags http -u -c' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +ts.Disk.records_config.Content = 'gold/records.yaml.cold_test2.gold' + +# 3 +tr = Test.AddTestRun("Get value from latest added node 1.") +tr.Processes.Default.Command = 'traffic_ctl config get proxy.config.diags.debug.tags --cold' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env + +tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + 'proxy.config.diags.debug.tags: http', + 'Config should show the right tags' +) + +# 4 +tr = Test.AddTestRun("Append a new field node using a tag") +tr.Processes.Default.Command = 'traffic_ctl config set proxy.config.cache.limits.http.max_alts 1 -t int -c' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +ts.Disk.records_config.Content = 'gold/records.yaml.cold_test4.gold' + + +# 5 +file = os.path.join(ts.Variables.CONFIGDIR, "new_records.yaml") +tr = Test.AddTestRun("Adding a new node(with update flag set) to a non existing file") +tr.Processes.Default.Command = f'traffic_ctl config set proxy.config.cache.limits.http.max_alts 3 -u -c {file}' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Disk.File(file).Content = 'gold/records.yaml.cold_test5.gold' + +# 5 +file = os.path.join(ts.Variables.CONFIGDIR, "new_records2.yaml") +tr = Test.AddTestRun("Adding a new node to a non existing file") +tr.Processes.Default.Command = f'traffic_ctl config set proxy.config.cache.limits.http.max_alts 3 -c {file}' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.Disk.File(file).Content = 'gold/records.yaml.cold_test5.gold' From dc28bbb3a9f046efe71bb132fc1e5b771a7fd67e Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 14 Feb 2023 13:15:24 +0000 Subject: [PATCH 3/3] DOC: Add supported documentation --- doc/admin-guide/files/records.yaml.en.rst | 7 +- .../command-line/traffic_ctl.en.rst | 78 ++++++++++++++++++- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 28c443a6e3c..8364e8c96f6 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -31,9 +31,7 @@ records.yaml The :file:`records.yaml` file (by default, located in ``/usr/local/etc/trafficserver/``) is a YAML base configuration file used by -the |TS| software. Many of the fields in :file:`records.yaml` are set -automatically when you set configuration options with :option:`traffic_ctl config set`. After you -modify :file:`records.yaml`, run the command :option:`traffic_ctl config reload` +the |TS| software. After you modify :file:`records.yaml`, run the command :option:`traffic_ctl config reload` to apply the changes. .. note:: @@ -47,7 +45,8 @@ to apply the changes. YAML structure ============== -All fields are located inside the ``ts`` root node. +All fields are located inside the ``ts`` root node. ATS supports reading multiple documents from +the same YAML stream, subsequent documents overrides earlier fields. .. code-block:: yaml diff --git a/doc/appendices/command-line/traffic_ctl.en.rst b/doc/appendices/command-line/traffic_ctl.en.rst index e3c1493e840..ac0eeb658c5 100644 --- a/doc/appendices/command-line/traffic_ctl.en.rst +++ b/doc/appendices/command-line/traffic_ctl.en.rst @@ -170,8 +170,9 @@ Display the current value of a configuration record. .. program:: traffic_ctl config get .. option:: --records - If this flag is provided, :option:`traffic_ctl config get` will emit results in - :file:`records.yaml` format. + If this flag is provided, :option:`traffic_ctl config get` will emit results in internal ats variable format. + + The option :ref:`--cold ` is available to get the values from a file. .. program:: traffic_ctl config .. option:: match [--records] REGEX [REGEX...] @@ -179,7 +180,7 @@ Display the current value of a configuration record. :ref:`admin_lookup_records` Display the current values of all configuration variables whose names match the given regular - expression. The ``--records`` flag has the same behavior as :option:`traffic_ctl config get + expression. The ``--records`` flag has the same behavior as `traffic_ctl config get --records`. .. program:: traffic_ctl config @@ -203,6 +204,77 @@ Display the current value of a configuration record. documentation for a list of the configuration variables you can specify. Note that this is not a synchronous operation. + Supports the following options. + + +.. _traffic_ctl_config_cold: + + .. option:: --cold, -c [filename] + + + This option indicates to `traffic_ctl` that the action should be performed on a configuration file instead of using the ATS RPC + facility to store the new value. `traffic_ctl` will save the value in the passed `filename`, if no `filename` passed, then the sysconfig + :file:`records.yaml` will be attempted to be used. + + ATS supports parsing multiple documents from the same YAML stream, so if you attempt to set a variable on a document with + none, one or multiple documents then a new document will be appended. In case you want to modify an existing field then `-u` option + should be passed, so the latest(top to bottom) field will be modified, if there is no variable already set in any of the documents, + then the new variable will be set in the latest document of the stream. + + Specifying the file name is not needed as `traffic_ctl` will try to use the build(or the runroot if used) information to figure + out the path to the `records.yaml`. + + If the file exists and is empty a new document will be created. If a file does not exist, an attempt to create a new file will be done. + + This option(only for the config file changes) lets you use the prefix `proxy.config.` or `ts.` for variable names, either would work. + If different prefix name is prepend, then traffic_ctl will work away using the provided variable name, this may not be what is intended + so, make sure you use the right prefixes. + + + Appending a new field in a records.yaml file. + + .. code-block:: bash + + $ traffic_ctl config set proxy.config.diags.debug.enabled 1 -c records.yaml + $ cat records.yaml + ts: + ... + # Document modified by traffic_ctl Mon Feb 13 23:07:15 2023 + # + --- + ts: + diags: + debug: + enabled: 1 + + .. note:: + + The following options are only considered if ``--cold, -c`` is used, ignored otherwise. + + .. option:: --update -u + + Update latest field present. If there is no variable already set in any of the documents, then the new variable will be set in + the latest document. + + .. option:: --type, -t int | float | str + + Inject a tag information on the modified/new field, this is useful when you set a non registered record inside ATS. + + .. code-block:: bash + + $ traffic_ctl config set ts.some.plugin.config.max 100 -t int -c records.yaml + $ cat records.yaml + ... + # Document modified by traffic_ctl Mon Feb 13 23:07:15 2023 + # + --- + ts: + some: + plugin: + config: + max: ! 100 + + .. program:: traffic_ctl config .. option:: status