diff --git a/mgmt/Makefile.am b/mgmt/Makefile.am index b5ce714dae9..9ad6ea9ae68 100644 --- a/mgmt/Makefile.am +++ b/mgmt/Makefile.am @@ -34,6 +34,7 @@ AM_CPPFLAGS += \ -I$(abs_top_srcdir)/proxy \ -I$(abs_top_srcdir)/proxy/http \ -I$(abs_top_srcdir)/proxy/hdrs \ + -I$(abs_top_srcdir)/lib/yamlcpp/include \ $(TS_INCLUDES) libmgmt_c_la_SOURCES = \ @@ -51,7 +52,9 @@ libmgmt_p_la_SOURCES = \ ProcessManager.cc \ ProcessManager.h \ ProxyConfig.cc \ - ProxyConfig.h + ProxyConfig.h \ + YamlCfg.cc \ + YamlCfg.h libmgmt_lm_la_SOURCES = \ $(libmgmt_COMMON) \ diff --git a/mgmt/YamlCfg.cc b/mgmt/YamlCfg.cc new file mode 100644 index 00000000000..f3e95ad30ac --- /dev/null +++ b/mgmt/YamlCfg.cc @@ -0,0 +1,83 @@ +/** @file + + Implementation file for YamlCfg.h. + + @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 "YamlCfg.h" + +#include +#include + +#include + +namespace ts +{ +namespace Yaml +{ + Map::Map(YAML::Node map) : _map{map} + { + if (!_map.IsMap()) { + throw YAML::ParserException(_map.Mark(), "map expected"); + } + } + + YAML::Node + Map::operator[](std::string_view key) + { + auto n = _map[std::string(key)]; + + if (n) { + // Add key to _used_key if not in it already. + // + if (std::find(_used_key.begin(), _used_key.end(), key) == _used_key.end()) { + _used_key.push_back(key); + } + } + + return n; + } + + void + Map::done() + { + if (!_bad && (_used_key.size() != _map.size())) { + ink_assert(_used_key.size() < _map.size()); + + std::string msg{(_map.size() - _used_key.size()) > 1 ? "keys " : "key "}; + bool first{true}; + + for (auto const &kv : _map) { + auto key = kv.first.as(); + + if (std::find(_used_key.begin(), _used_key.end(), key) == _used_key.end()) { + if (!first) { + msg += ", "; + } + first = false; + msg += key; + } + } + throw YAML::ParserException(_map.Mark(), msg + " invalid in this map"); + } + } + +} // end namespace Yaml +} // end namespace ts diff --git a/mgmt/YamlCfg.h b/mgmt/YamlCfg.h new file mode 100644 index 00000000000..e7d3adc21be --- /dev/null +++ b/mgmt/YamlCfg.h @@ -0,0 +1,77 @@ +/** @file + + Utilites to help with parsing YAML files with good error reporting. + + @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 + +namespace ts +{ +namespace Yaml +{ + // A class that is a wrapper for a YAML::Node that corresponds to a map in a YAML input file. + // It's purpose is to make sure all keys in the map are processed. + // + class Map + { + public: + // A YAML::ParserException will be thrown if 'map' isn't actually a map. + // + explicit Map(YAML::Node map); + + // Get the node for a key. Throw a YAML::Exception if 'key' is not in the map. The node for each key in the + // map must be gotten at least once. The lifetime of the char array referenced by passed key must be as long + // as this instance. + // + YAML::Node operator[](std::string_view key); + + // Call this after the last call to the [] operator. Will throw a YAML::ParserException if instance not + // already marked bad, and all keys in the map were not accessed at least once with the [] operator. The + // 'what' of the exception will list the keys that were not accessed as invalid for the map. + // + void done(); + + // Mark instance as bad. + // + void + bad() + { + _bad = true; + } + + // No copy/move. + // + Map(Map const &) = delete; + Map &operator=(Map const &) = delete; + + private: + YAML::Node _map; + std::vector _used_key; + bool _bad{false}; + }; + +} // end namespace Yaml +} // end namespace ts diff --git a/proxy/http/remap/NextHopConsistentHash.cc b/proxy/http/remap/NextHopConsistentHash.cc index 02ce5334979..d7b20c3b5b1 100644 --- a/proxy/http/remap/NextHopConsistentHash.cc +++ b/proxy/http/remap/NextHopConsistentHash.cc @@ -26,6 +26,7 @@ #include "tscore/HashSip.h" #include "HttpSM.h" #include "I_Machine.h" +#include "YamlCfg.h" #include "NextHopConsistentHash.h" // hash_key strings. @@ -63,7 +64,7 @@ NextHopConsistentHash::~NextHopConsistentHash() } bool -NextHopConsistentHash::Init(const YAML::Node &n) +NextHopConsistentHash::Init(ts::Yaml::Map &n) { ATSHash64Sip24 hash; diff --git a/proxy/http/remap/NextHopConsistentHash.h b/proxy/http/remap/NextHopConsistentHash.h index 6ce8cffb877..c2adf12da4f 100644 --- a/proxy/http/remap/NextHopConsistentHash.h +++ b/proxy/http/remap/NextHopConsistentHash.h @@ -48,6 +48,6 @@ class NextHopConsistentHash : public NextHopSelectionStrategy NextHopConsistentHash() = delete; NextHopConsistentHash(const std::string_view name, const NHPolicyType &policy) : NextHopSelectionStrategy(name, policy) {} ~NextHopConsistentHash(); - bool Init(const YAML::Node &n); + bool Init(ts::Yaml::Map &n); void findNextHop(TSHttpTxn txnp, void *ih = nullptr, time_t now = 0) override; }; diff --git a/proxy/http/remap/NextHopRoundRobin.h b/proxy/http/remap/NextHopRoundRobin.h index a7cb8233153..024efee70d7 100644 --- a/proxy/http/remap/NextHopRoundRobin.h +++ b/proxy/http/remap/NextHopRoundRobin.h @@ -36,7 +36,7 @@ class NextHopRoundRobin : public NextHopSelectionStrategy NextHopRoundRobin(const std::string_view &name, const NHPolicyType &policy) : NextHopSelectionStrategy(name, policy) {} ~NextHopRoundRobin(); bool - Init(const YAML::Node &n) + Init(ts::Yaml::Map &n) { return NextHopSelectionStrategy::Init(n); } diff --git a/proxy/http/remap/NextHopSelectionStrategy.cc b/proxy/http/remap/NextHopSelectionStrategy.cc index f44d10aec73..c57555ee1cc 100644 --- a/proxy/http/remap/NextHopSelectionStrategy.cc +++ b/proxy/http/remap/NextHopSelectionStrategy.cc @@ -21,7 +21,10 @@ limitations under the License. */ +#include + #include +#include #include "I_Machine.h" #include "HttpSM.h" #include "NextHopSelectionStrategy.h" @@ -57,7 +60,7 @@ NextHopSelectionStrategy::NextHopSelectionStrategy(const std::string_view &name, // parse out the data for this strategy. // bool -NextHopSelectionStrategy::Init(const YAML::Node &n) +NextHopSelectionStrategy::Init(ts::Yaml::Map &n) { NH_Debug(NH_DEBUG_TAG, "calling Init()"); @@ -95,9 +98,9 @@ NextHopSelectionStrategy::Init(const YAML::Node &n) } // failover node. - YAML::Node failover_node; - if (n["failover"]) { - failover_node = n["failover"]; + YAML::Node failover_node_n = n["failover"]; + if (failover_node_n) { + ts::Yaml::Map failover_node{failover_node_n}; if (failover_node["ring_mode"]) { auto ring_mode_val = failover_node["ring_mode"].Scalar(); if (ring_mode_val == alternate_rings) { @@ -174,12 +177,12 @@ NextHopSelectionStrategy::Init(const YAML::Node &n) } } } + failover_node.done(); } // parse and load the host data - YAML::Node groups_node; - if (n["groups"]) { - groups_node = n["groups"]; + YAML::Node groups_node = n["groups"]; + if (groups_node) { // a groups list is required. if (groups_node.Type() != YAML::NodeType::Sequence) { throw std::invalid_argument("Invalid groups definition, expected a sequence, '" + strategy_name + "' cannot be loaded."); @@ -224,7 +227,7 @@ NextHopSelectionStrategy::Init(const YAML::Node &n) } } } catch (std::exception &ex) { - NH_Note("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), ex.what()); + NH_Error("Error parsing the strategy named '%s' due to '%s', this strategy will be ignored.", strategy_name.c_str(), ex.what()); return false; } @@ -296,26 +299,26 @@ template <> struct convert { static bool decode(const Node &node, HostRecord &nh) { - YAML::Node nd; - bool merge_tag_used = false; + ts::Yaml::Map map{node}; + ts::Yaml::Map *mmap{&map}; + std::optional mergeable_map; // check for YAML merge tag. - if (node["<<"]) { - nd = node["<<"]; - merge_tag_used = true; - } else { - nd = node; + YAML::Node mergeable_map_n = map["<<"]; + if (mergeable_map_n) { + mergeable_map.emplace(mergeable_map_n); + mmap = &mergeable_map.value(); } // lookup the hostname - if (nd["host"]) { - nh.hostname = nd["host"].Scalar(); + if ((*mmap)["host"]) { + nh.hostname = (*mmap)["host"].Scalar(); } else { throw std::invalid_argument("Invalid host definition, missing host name."); } // lookup the port numbers supported by this host. - YAML::Node proto = nd["protocol"]; + YAML::Node proto = (*mmap)["protocol"]; if (proto.Type() != YAML::NodeType::Sequence) { throw std::invalid_argument("Invalid host protocol definition, expected a sequence."); @@ -327,12 +330,17 @@ template <> struct convert { } } - // get the host's weight - YAML::Node weight; - if (merge_tag_used) { - weight = node["weight"]; - nh.weight = weight.as(); - } else if ((weight = nd["weight"])) { + // get the host's weight, allowing override of weight in merged map + YAML::Node weight = map["weight"]; + if (mmap != &map) { + // weight must always be looked up in the merged map, even if overridden, so it's presence will not + // cause an exception when mmap->done() is called + YAML::Node w = (*mmap)["weight"]; + if (!weight) { + weight = w; + } + } + if (weight) { nh.weight = weight.as(); } else { NH_Note("No weight is defined for the host '%s', using default 1.0", nh.hostname.data()); @@ -340,11 +348,16 @@ template <> struct convert { } // get the host's optional hash_string - YAML::Node hash; - if ((hash = nd["hash_string"])) { + YAML::Node hash{(*mmap)["hash_string"]}; + if (hash) { nh.hash_string = hash.Scalar(); } + map.done(); + if (mmap != &map) { + mmap->done(); + } + return true; } }; @@ -353,21 +366,27 @@ template <> struct convert { static bool decode(const Node &node, NHProtocol &nh) { - if (node["scheme"]) { - if (node["scheme"].Scalar() == "http") { + ts::Yaml::Map map{node}; + + if (map["scheme"]) { + if (map["scheme"].Scalar() == "http") { nh.scheme = NH_SCHEME_HTTP; - } else if (node["scheme"].Scalar() == "https") { + } else if (map["scheme"].Scalar() == "https") { nh.scheme = NH_SCHEME_HTTPS; } else { nh.scheme = NH_SCHEME_NONE; } } - if (node["port"]) { - nh.port = node["port"].as(); + if (map["port"]) { + nh.port = map["port"].as(); + if ((nh.port <= 0) || (nh.port > 65535)) { + throw YAML::ParserException(map["port"].Mark(), "port number must be in (inclusive) range 0 - 65,536"); + } } - if (node["health_check_url"]) { - nh.health_check_url = node["health_check_url"].Scalar(); + if (map["health_check_url"]) { + nh.health_check_url = map["health_check_url"].Scalar(); } + map.done(); return true; } }; diff --git a/proxy/http/remap/NextHopSelectionStrategy.h b/proxy/http/remap/NextHopSelectionStrategy.h index 2e5b2c2fa46..0c483a3668b 100644 --- a/proxy/http/remap/NextHopSelectionStrategy.h +++ b/proxy/http/remap/NextHopSelectionStrategy.h @@ -39,10 +39,13 @@ constexpr const char *NH_DEBUG_TAG = "next_hop"; -namespace YAML +namespace ts { -class Node; +namespace Yaml +{ + class Map; } +} // namespace ts enum NHCmd { NH_MARK_UP, NH_MARK_DOWN }; @@ -235,7 +238,7 @@ class NextHopSelectionStrategy NextHopSelectionStrategy(); NextHopSelectionStrategy(const std::string_view &name, const NHPolicyType &type); virtual ~NextHopSelectionStrategy(){}; - bool Init(const YAML::Node &n); + bool Init(ts::Yaml::Map &n); virtual void findNextHop(TSHttpTxn txnp, void *ih = nullptr, time_t now = 0) = 0; void markNextHop(TSHttpTxn txnp, const char *hostname, const int port, const NHCmd status, void *ih = nullptr, const time_t now = 0); diff --git a/proxy/http/remap/NextHopStrategyFactory.cc b/proxy/http/remap/NextHopStrategyFactory.cc index 4de0b8aa521..653e53fff3d 100644 --- a/proxy/http/remap/NextHopStrategyFactory.cc +++ b/proxy/http/remap/NextHopStrategyFactory.cc @@ -29,14 +29,14 @@ #include "NextHopStrategyFactory.h" #include "NextHopConsistentHash.h" #include "NextHopRoundRobin.h" +#include -NextHopStrategyFactory::NextHopStrategyFactory(const char *file) +NextHopStrategyFactory::NextHopStrategyFactory(const char *file) : fn(file) { YAML::Node config; YAML::Node strategies; std::stringstream doc; std::unordered_set include_once; - std::string_view fn = file; // strategy policies. constexpr std::string_view consistent_hash = "consistent_hash"; @@ -46,12 +46,12 @@ NextHopStrategyFactory::NextHopStrategyFactory(const char *file) constexpr std::string_view latched = "latched"; strategies_loaded = true; - const char *basename = fn.substr(fn.find_last_of('/') + 1).data(); + const char *basename = std::string_view(fn).substr(fn.find_last_of('/') + 1).data(); // load the strategies yaml config file. try { NH_Note("%s loading ...", basename); - loadConfigFile(fn.data(), doc, include_once); + loadConfigFile(fn.c_str(), doc, include_once); config = YAML::Load(doc); if (config.IsNull()) { @@ -66,9 +66,9 @@ NextHopStrategyFactory::NextHopStrategyFactory(const char *file) } // loop through the strategies document. for (auto &&strategie : strategies) { - YAML::Node strategy = strategie; - auto name = strategy["strategy"].as(); - auto policy = strategy["policy"]; + ts::Yaml::Map strategy{strategie}; + auto name = strategy["strategy"].as(); + auto policy = strategy["policy"]; if (!policy) { NH_Error("No policy is defined for the strategy named '%s', this strategy will be ignored.", name.c_str()); continue; @@ -92,6 +92,7 @@ NextHopStrategyFactory::NextHopStrategyFactory(const char *file) name.c_str()); } else { createStrategy(name, policy_type, strategy); + strategy.done(); } } } catch (std::exception &ex) { @@ -109,7 +110,7 @@ NextHopStrategyFactory::~NextHopStrategyFactory() } void -NextHopStrategyFactory::createStrategy(const std::string &name, const NHPolicyType policy_type, const YAML::Node &node) +NextHopStrategyFactory::createStrategy(const std::string &name, const NHPolicyType policy_type, ts::Yaml::Map &node) { std::shared_ptr strat; std::shared_ptr strat_rr; @@ -118,6 +119,7 @@ NextHopStrategyFactory::createStrategy(const std::string &name, const NHPolicyTy strat = strategyInstance(name.c_str()); if (strat != nullptr) { NH_Note("A strategy named '%s' has already been loaded and another will not be created.", name.data()); + node.bad(); return; } diff --git a/proxy/http/remap/NextHopStrategyFactory.h b/proxy/http/remap/NextHopStrategyFactory.h index ecb25a8943c..25adca8b40b 100644 --- a/proxy/http/remap/NextHopStrategyFactory.h +++ b/proxy/http/remap/NextHopStrategyFactory.h @@ -31,10 +31,13 @@ #include "tscore/Diags.h" #include "NextHopSelectionStrategy.h" -namespace YAML +namespace ts { -class Node; -}; +namespace Yaml +{ + class Map; +} +} // namespace ts class NextHopStrategyFactory { @@ -49,6 +52,6 @@ class NextHopStrategyFactory private: std::string fn; void loadConfigFile(const std::string &file, std::stringstream &doc, std::unordered_set &include_once); - void createStrategy(const std::string &name, const NHPolicyType policy_type, const YAML::Node &node); + void createStrategy(const std::string &name, const NHPolicyType policy_type, ts::Yaml::Map &node); std::unordered_map> _strategies; }; diff --git a/tests/gold_tests/next_hop/strategies_ch2/strategies_ch2.test.py b/tests/gold_tests/next_hop/strategies_ch2/strategies_ch2.test.py index 3dcf1d53996..e451d42f981 100644 --- a/tests/gold_tests/next_hop/strategies_ch2/strategies_ch2.test.py +++ b/tests/gold_tests/next_hop/strategies_ch2/strategies_ch2.test.py @@ -73,7 +73,7 @@ 'proxy.config.http.cache.http': 0, 'proxy.config.http.uncacheable_requests_bypass_parent': 0, 'proxy.config.http.no_dns_just_forward_to_parent': 1, - 'proxy.config.http.parent_proxy.mark_down_hostdb': 1, + 'proxy.config.http.parent_proxy.mark_down_hostdb': 0, 'proxy.config.http.parent_proxy.self_detect': 0, })