diff --git a/bazel/README.md b/bazel/README.md index ea300ded80b84..c99d8db4a4521 100644 --- a/bazel/README.md +++ b/bazel/README.md @@ -245,11 +245,14 @@ on the Bazel command line. ## Stats Tunables -The default maximum number of stats in shared memory, and the default maximum length of a stat name, -can be overriden at compile-time by defining `ENVOY_DEFAULT_MAX_STATS` and `ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH`, -respectively, to the desired value. For example: +The default maximum number of stats in shared memory, and the default +maximum length of a cluster/route config/listener name, can be +overriden at compile-time by defining `ENVOY_DEFAULT_MAX_STATS` and +`ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH`, respectively, to the desired +value. For example: + ``` -bazel build --copts=-DENVOY_DEFAULT_MAX_STATS=32768 //source/exe:envoy-static +bazel build --copts=-DENVOY_DEFAULT_MAX_STATS=32768 --copts=-DENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH=150 //source/exe:envoy-static ``` diff --git a/docs/configuration/cluster_manager/cluster.rst b/docs/configuration/cluster_manager/cluster.rst index 9448aa82caa8b..1da992ffb3b1f 100644 --- a/docs/configuration/cluster_manager/cluster.rst +++ b/docs/configuration/cluster_manager/cluster.rst @@ -31,7 +31,8 @@ Cluster name *(required, string)* Supplies the name of the cluster which must be unique across all clusters. The cluster name is used when emitting :ref:`statistics `. - The cluster name can be at most 60 characters long. + By default, the maximum length of a cluster name is limited to 60 characters. This limit can be + increased by setting the :option:`--max-obj-name-len` command line argument to the desired value. .. _config_cluster_manager_type: diff --git a/docs/configuration/http_conn_man/rds.rst b/docs/configuration/http_conn_man/rds.rst index a8e3205250b6b..fea548926cda2 100644 --- a/docs/configuration/http_conn_man/rds.rst +++ b/docs/configuration/http_conn_man/rds.rst @@ -28,7 +28,9 @@ route_config_name *(required, string)* The name of the route configuration. This name will be passed to the :ref:`RDS HTTP API `. This allows an Envoy configuration with multiple HTTP listeners (and associated HTTP connection manager filters) to use different route - configurations. + configurations. By default, the maximum length of the name is limited to 60 characters. This + limit can be increased by setting the :option:`--max-obj-name-len` command line argument to the + desired value. refresh_delay_ms *(optional, integer)* The delay, in milliseconds, between fetches to the RDS API. Envoy will add diff --git a/docs/configuration/http_conn_man/route_config/vhost.rst b/docs/configuration/http_conn_man/route_config/vhost.rst index f019d9ce5fd4f..32064487e7787 100644 --- a/docs/configuration/http_conn_man/route_config/vhost.rst +++ b/docs/configuration/http_conn_man/route_config/vhost.rst @@ -23,7 +23,9 @@ upstream cluster to route to or whether to perform a redirect. name *(required, string)* The logical name of the virtual host. This is used when emitting certain - statistics but is not relevant for forwarding. + statistics but is not relevant for forwarding. By default, the maximum length of the name is + limited to 60 characters. This limit can be increased by setting the + :option:`--max-obj-name-len` command line argument to the desired value. domains *(required, array)* A list of domains (host/authority header) that will be matched to this diff --git a/docs/configuration/listeners/listeners.rst b/docs/configuration/listeners/listeners.rst index 9170a0ea00072..5951123133356 100644 --- a/docs/configuration/listeners/listeners.rst +++ b/docs/configuration/listeners/listeners.rst @@ -34,6 +34,8 @@ name *(optional, string)* The unique name by which this listener is known. If no name is provided, Envoy will allocate an internal UUID for the listener. If the listener is to be dynamically updated or removed via :ref:`LDS ` a unique name must be provided. + By default, the maximum length of a listener's name is limited to 60 characters. This limit can be + increased by setting the :option:`--max-obj-name-len` command line argument to the desired value. address *(required, string)* The address that the listener should listen on. Currently only TCP diff --git a/docs/operations/cli.rst b/docs/operations/cli.rst index fe7d1e2d49377..297feff8318ac 100644 --- a/docs/operations/cli.rst +++ b/docs/operations/cli.rst @@ -117,10 +117,17 @@ following are the command line options that Envoy supports. during a hot restart. See the :ref:`hot restart overview ` for more information. Defaults to 900 seconds (15 minutes). -.. option:: --max-stat-name-len +.. option:: --max-obj-name-len - *(optional)* The maximum name length (in bytes) for a stat. This setting affects the output - of :option:`--hot-restart-version`; the same value must be used to hot restart. Defaults to 127. + *(optional)* The maximum name length (in bytes) of the name field in a cluster/route_config/listener. + This setting is typically used in scenarios where the cluster names are auto generated, and often exceed + the built-in limit of 60 characters. Defaults to 60. + + .. attention:: + + This setting affects the output of :option:`--hot-restart-version`. If you started envoy with this + option set to a non default value, you should use the same option (and same value) for subsequent hot + restarts. .. option:: --max-stats diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h index e961bbec12be0..4e571de4c613c 100644 --- a/include/envoy/server/options.h +++ b/include/envoy/server/options.h @@ -127,9 +127,10 @@ class Options { virtual uint64_t maxStats() PURE; /** - * @return uint64_t the maximum name length of a stat. + * @return uint64_t the maximum name length of the name field in + * router/cluster/listener. */ - virtual uint64_t maxStatNameLength() PURE; + virtual uint64_t maxObjNameLength() PURE; }; } // namespace Server diff --git a/source/common/config/BUILD b/source/common/config/BUILD index acf65f95a46d7..1317dc1171d13 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -183,6 +183,7 @@ envoy_cc_library( ":address_json_lib", ":json_utility_lib", ":tls_context_json_lib", + ":utility_lib", ":well_known_names", "//include/envoy/json:json_object_interface", "//source/common/common:assert_lib", @@ -231,6 +232,7 @@ envoy_cc_library( ":well_known_names", "//include/envoy/json:json_object_interface", "//source/common/common:assert_lib", + "//source/common/config:utility_lib", "//source/common/json:config_schemas_lib", ], ) @@ -290,6 +292,7 @@ envoy_cc_library( "//source/common/json:config_schemas_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "//source/common/stats:stats_lib", ], ) diff --git a/source/common/config/cds_json.cc b/source/common/config/cds_json.cc index 40935b0cdebd0..2f780cbf6c6a2 100644 --- a/source/common/config/cds_json.cc +++ b/source/common/config/cds_json.cc @@ -84,7 +84,11 @@ void CdsJson::translateCluster(const Json::Object& json_cluster, const Optional& eds_config, envoy::api::v2::Cluster& cluster) { json_cluster.validateSchema(Json::Schema::CLUSTER_SCHEMA); - cluster.set_name(json_cluster.getString("name")); + + const std::string name = json_cluster.getString("name"); + Utility::checkObjNameLength("Invalid cluster name", name); + cluster.set_name(name); + const std::string string_type = json_cluster.getString("type"); auto set_dns_hosts = [&json_cluster, &cluster] { const auto hosts = json_cluster.getObjectArray("hosts"); diff --git a/source/common/config/lds_json.cc b/source/common/config/lds_json.cc index 79d1147176b69..aa8ab279e7c25 100644 --- a/source/common/config/lds_json.cc +++ b/source/common/config/lds_json.cc @@ -4,6 +4,7 @@ #include "common/config/address_json.h" #include "common/config/json_utility.h" #include "common/config/tls_context_json.h" +#include "common/config/utility.h" #include "common/config/well_known_names.h" #include "common/json/config_schemas.h" #include "common/network/utility.h" @@ -15,6 +16,10 @@ void LdsJson::translateListener(const Json::Object& json_listener, envoy::api::v2::Listener& listener) { json_listener.validateSchema(Json::Schema::LISTENER_SCHEMA); + const std::string name = json_listener.getString("name", ""); + Utility::checkObjNameLength("Invalid listener name", name); + listener.set_name(name); + AddressJson::translateAddress(json_listener.getString("address"), true, true, *listener.mutable_address()); @@ -42,12 +47,9 @@ void LdsJson::translateListener(const Json::Object& json_listener, } JSON_UTIL_SET_BOOL(json_listener, *filter_chain, use_proxy_proto); - JSON_UTIL_SET_BOOL(json_listener, listener, use_original_dst); - JSON_UTIL_SET_INTEGER(json_listener, listener, per_connection_buffer_limit_bytes); - JSON_UTIL_SET_STRING(json_listener, listener, name); - JSON_UTIL_SET_BOOL(json_listener, *listener.mutable_deprecated_v1(), bind_to_port); + JSON_UTIL_SET_INTEGER(json_listener, listener, per_connection_buffer_limit_bytes); } } // namespace Config diff --git a/source/common/config/rds_json.cc b/source/common/config/rds_json.cc index feddd798ffe9d..fa79604496539 100644 --- a/source/common/config/rds_json.cc +++ b/source/common/config/rds_json.cc @@ -5,6 +5,7 @@ #include "common/config/base_json.h" #include "common/config/json_utility.h" #include "common/config/metadata.h" +#include "common/config/utility.h" #include "common/config/well_known_names.h" #include "common/json/config_schemas.h" @@ -130,7 +131,10 @@ void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config, void RdsJson::translateVirtualHost(const Json::Object& json_virtual_host, envoy::api::v2::VirtualHost& virtual_host) { json_virtual_host.validateSchema(Json::Schema::VIRTUAL_HOST_CONFIGURATION_SCHEMA); - JSON_UTIL_SET_STRING(json_virtual_host, virtual_host, name); + + const std::string name = json_virtual_host.getString("name", ""); + Utility::checkObjNameLength("Invalid virtual host name", name); + virtual_host.set_name(name); for (const std::string& domain : json_virtual_host.getStringArray("domains", true)) { virtual_host.add_domains(domain); diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index fcd1497a3a296..c8736f9ab88ec 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -8,6 +8,7 @@ #include "common/json/config_schemas.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" +#include "common/stats/stats_impl.h" #include "fmt/format.h" @@ -86,11 +87,15 @@ void Utility::translateCdsConfig(const Json::Object& json_config, void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::filter::Rds& rds) { json_rds.validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); + + const std::string name = json_rds.getString("route_config_name", ""); + checkObjNameLength("Invalid route_config name", name); + rds.set_route_config_name(name); + translateApiConfigSource(json_rds.getString("cluster"), json_rds.getInteger("refresh_delay_ms", 30000), json_rds.getString("api_type", ApiType::get().RestLegacy), *rds.mutable_config_source()->mutable_api_config_source()); - JSON_UTIL_SET_STRING(json_rds, rds, route_config_name); } void Utility::translateLdsConfig(const Json::Object& json_lds, @@ -119,5 +124,13 @@ std::string Utility::resourceName(const ProtobufWkt::Any& resource) { fmt::format("Unknown type URL {} in DiscoveryResponse", resource.type_url())); } +void Utility::checkObjNameLength(const std::string& error_prefix, const std::string& name) { + if (name.length() > Stats::RawStatData::maxObjNameLength()) { + throw EnvoyException(fmt::format("{}: Length of {} ({}) exceeds allowed maximum length ({})", + error_prefix, name, name.length(), + Stats::RawStatData::maxObjNameLength())); + } +} + } // namespace Config } // namespace Envoy diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 6e54bac4b42bf..618c91c81546d 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -212,6 +212,14 @@ class Utility { * @return std::string resource name. */ static std::string resourceName(const ProtobufWkt::Any& resource); + + /** + * Check user supplied name in RDS/CDS/LDS for sanity. + * It should be within the configured length limit. Throws on error. + * @param error_prefix supplies the prefix to use in error messages. + * @param name supplies the name to check for length limits. + */ + static void checkObjNameLength(const std::string& error_prefix, const std::string& name); }; } // namespace Config diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index fbbda92cb5a01..7860701809dd1 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1397,8 +1397,7 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( "properties" : { "name" : { "type" : "string", - "minLength" : 1, - "maxLength" : 60 + "minLength" : 1 }, "type" : { "type" : "string", diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index 2fdb75fd929b6..1c8eaf73fa295 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -30,25 +30,25 @@ size_t RawStatData::size() { return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + nameSize()); } -size_t& RawStatData::initializeAndGetMutableMaxNameLength(size_t configured_size) { +size_t& RawStatData::initializeAndGetMutableMaxObjNameLength(size_t configured_size) { // Like CONSTRUCT_ON_FIRST_USE, but non-const so that the value can be changed by tests static size_t size = configured_size; return size; } void RawStatData::configure(Server::Options& options) { - const size_t configured = options.maxStatNameLength(); + const size_t configured = options.maxObjNameLength(); RELEASE_ASSERT(configured > 0); - size_t max_name_length = initializeAndGetMutableMaxNameLength(configured); + size_t max_obj_name_length = initializeAndGetMutableMaxObjNameLength(configured); // If this fails, it means that this function was called too late during // startup because things were already using this size before it was set. - RELEASE_ASSERT(max_name_length == configured); + RELEASE_ASSERT(max_obj_name_length == configured); } void RawStatData::configureForTestsOnly(Server::Options& options) { - const size_t configured = options.maxStatNameLength(); - initializeAndGetMutableMaxNameLength(configured) = configured; + const size_t configured = options.maxObjNameLength(); + initializeAndGetMutableMaxObjNameLength(configured) = configured; } std::string Utility::sanitizeStatsName(const std::string& name) { diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h index d6d114916f5c4..456d60d26cee2 100644 --- a/source/common/stats/stats_impl.h +++ b/source/common/stats/stats_impl.h @@ -63,10 +63,22 @@ struct RawStatData { * Returns the maximum length of the name of a stat. This length * does not include a trailing NULL-terminator. */ - static size_t maxNameLength() { - return initializeAndGetMutableMaxNameLength(DEFAULT_MAX_NAME_SIZE); + static size_t maxNameLength() { return maxObjNameLength() + MAX_STAT_SUFFIX_LENGTH; } + + /** + * Returns the maximum length of a user supplied object (route/cluster/listener) + * name field in a stat. This length does not include a trailing NULL-terminator. + */ + static size_t maxObjNameLength() { + return initializeAndGetMutableMaxObjNameLength(DEFAULT_MAX_OBJ_NAME_LENGTH); } + /** + * Returns the maximum length of a stat suffix that Envoy generates (over the user supplied name). + * This length does not include a trailing NULL-terminator. + */ + static size_t maxStatSuffixLength() { return MAX_STAT_SUFFIX_LENGTH; } + /** * size in bytes of name_ */ @@ -102,9 +114,22 @@ struct RawStatData { char name_[]; private: - static const size_t DEFAULT_MAX_NAME_SIZE = 127; - - static size_t& initializeAndGetMutableMaxNameLength(size_t configured_size); + // The max name length is based on current set of stats. + // As of now, the longest stat is + // cluster..outlier_detection.ejections_consecutive_5xx + // which is 52 characters long without the cluster name. + // The max stat name length is 127 (default). So, in order to give room + // for growth to both the envoy generated stat characters + // (e.g., outlier_detection...) and user supplied names (e.g., cluster name), + // we set the max user supplied name length to 60, and the max internally + // generated stat suffixes to 67 (15 more characters to grow). + // If you want to increase the max user supplied name length, use the compiler + // option ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH or the CLI option + // max-obj-name-len + static const size_t DEFAULT_MAX_OBJ_NAME_LENGTH = 60; + static const size_t MAX_STAT_SUFFIX_LENGTH = 67; + + static size_t& initializeAndGetMutableMaxObjNameLength(size_t configured_size); }; /** diff --git a/source/server/BUILD b/source/server/BUILD index e33387e6a8f32..53d2370a1af9d 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -148,6 +148,7 @@ envoy_cc_library( "//include/envoy/server:options_interface", "//source/common/common:macros", "//source/common/common:version_lib", + "//source/common/stats:stats_lib", ], ) diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index 7109dd9d0683c..01ce1f60f224b 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -7,6 +7,7 @@ #include "common/common/macros.h" #include "common/common/version.h" +#include "common/stats/stats_impl.h" #include "fmt/format.h" #include "spdlog/spdlog.h" @@ -18,12 +19,14 @@ #endif // Can be overridden at compile time -#ifndef ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH -#define ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH 127 +// See comment in common/stat/stat_impl.h for rationale behind +// this constant. +#ifndef ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH +#define ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH 60 #endif -#if ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH < 127 -#error "ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH must be >= 127" +#if ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH < 60 +#error "ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH must be >= 60" #endif namespace Envoy { @@ -82,9 +85,12 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r "Maximum number of stats guages and counters " "that can be allocated in shared memory.", false, ENVOY_DEFAULT_MAX_STATS, "uint64_t", cmd); - TCLAP::ValueArg max_stat_name_len("", "max-stat-name-len", - "Maximum name length for a stat", false, - ENVOY_DEFAULT_MAX_STAT_NAME_LENGTH, "uint64_t", cmd); + TCLAP::ValueArg max_obj_name_len("", "max-obj-name-len", + "Maximum name length for a field in the config " + "(applies to listener name, route config name and" + " the cluster name)", + false, ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH, "uint64_t", + cmd); try { cmd.parse(argc, argv); @@ -93,14 +99,16 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r exit(1); } - if (max_stat_name_len.getValue() < 127) { - std::cerr << "error: the 'max-stat-name-len' value specified (" << max_stat_name_len.getValue() - << ") is less than the minimum value of 127" << std::endl; + if (max_obj_name_len.getValue() < 60) { + std::cerr << "error: the 'max-obj-name-len' value specified (" << max_obj_name_len.getValue() + << ") is less than the minimum value of 60" << std::endl; exit(1); } if (hot_restart_version_option.getValue()) { - std::cerr << hot_restart_version_cb(max_stats.getValue(), max_stat_name_len.getValue()); + std::cerr << hot_restart_version_cb(max_stats.getValue(), + max_obj_name_len.getValue() + + Stats::RawStatData::maxStatSuffixLength()); exit(0); } @@ -144,6 +152,6 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r drain_time_ = std::chrono::seconds(drain_time_s.getValue()); parent_shutdown_time_ = std::chrono::seconds(parent_shutdown_time_s.getValue()); max_stats_ = max_stats.getValue(); - max_stat_name_length_ = max_stat_name_len.getValue(); + max_obj_name_length_ = max_obj_name_len.getValue(); } } // namespace Envoy diff --git a/source/server/options_impl.h b/source/server/options_impl.h index e8f75a036fda3..73d3c2941a2c4 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -36,7 +36,7 @@ class OptionsImpl : public Server::Options { const std::string& serviceNodeName() override { return service_node_; } const std::string& serviceZone() override { return service_zone_; } uint64_t maxStats() override { return max_stats_; } - uint64_t maxStatNameLength() override { return max_stat_name_length_; } + uint64_t maxObjNameLength() override { return max_obj_name_length_; } private: uint64_t base_id_; @@ -55,6 +55,6 @@ class OptionsImpl : public Server::Options { std::chrono::seconds parent_shutdown_time_; Server::Mode mode_; uint64_t max_stats_; - uint64_t max_stat_name_length_; + uint64_t max_obj_name_length_; }; } // namespace Envoy diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 264bb918a71fd..ef9d50962fa99 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -152,7 +152,12 @@ envoy_cc_test( srcs = ["utility_test.cc"], external_deps = ["envoy_eds"], deps = [ + "//source/common/config:cds_json_lib", + "//source/common/config:lds_json_lib", + "//source/common/config:rds_json_lib", "//source/common/config:utility_lib", + "//source/common/stats:stats_lib", "//test/mocks/local_info:local_info_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index b5b6696ebad9c..cefedba66dcc5 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -1,9 +1,15 @@ +#include "common/config/cds_json.h" +#include "common/config/lds_json.h" +#include "common/config/rds_json.h" #include "common/config/utility.h" #include "common/protobuf/protobuf.h" +#include "common/stats/stats_impl.h" #include "test/mocks/local_info/mocks.h" +#include "test/test_common/utility.h" #include "api/eds.pb.h" +#include "fmt/format.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -69,5 +75,60 @@ TEST(UtilityTest, TranslateApiConfigSource) { EXPECT_EQ("test_grpc_cluster", api_config_source_grpc.cluster_name(0)); } +TEST(UtilityTest, ObjNameLength) { + + std::string name = "listenerwithareallyreallylongnamemorethanmaxcharsallowedbyschema"; + std::string err_prefix; + std::string err_suffix = fmt::format(": Length of {} ({}) exceeds allowed maximum length ({})", + name, name.length(), Stats::RawStatData::maxObjNameLength()); + { + err_prefix = "test"; + EXPECT_THROW_WITH_MESSAGE(Utility::checkObjNameLength(err_prefix, name), EnvoyException, + err_prefix + err_suffix); + } + + { + err_prefix = "Invalid listener name"; + std::string json = + R"EOF({ "name": ")EOF" + name + R"EOF(", "address": "foo", "filters":[]})EOF"; + auto json_object_ptr = Json::Factory::loadFromString(json); + + envoy::api::v2::Listener listener; + EXPECT_THROW_WITH_MESSAGE(Config::LdsJson::translateListener(*json_object_ptr, listener), + EnvoyException, err_prefix + err_suffix); + } + + { + err_prefix = "Invalid virtual host name"; + std::string json = R"EOF({ "name": ")EOF" + name + R"EOF(", "domains": [], "routes": []})EOF"; + auto json_object_ptr = Json::Factory::loadFromString(json); + envoy::api::v2::VirtualHost vhost; + EXPECT_THROW_WITH_MESSAGE(Config::RdsJson::translateVirtualHost(*json_object_ptr, vhost), + EnvoyException, err_prefix + err_suffix); + } + + { + err_prefix = "Invalid cluster name"; + std::string json = + R"EOF({ "name": ")EOF" + name + + R"EOF(", "type": "static", "lb_type": "random", "connect_timeout_ms" : 1})EOF"; + auto json_object_ptr = Json::Factory::loadFromString(json); + envoy::api::v2::Cluster cluster; + envoy::api::v2::ConfigSource eds_config; + EXPECT_THROW_WITH_MESSAGE( + Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster), EnvoyException, + err_prefix + err_suffix); + } + + { + err_prefix = "Invalid route_config name"; + std::string json = R"EOF({ "route_config_name": ")EOF" + name + R"EOF(", "cluster": "foo"})EOF"; + auto json_object_ptr = Json::Factory::loadFromString(json); + envoy::api::v2::filter::Rds rds; + EXPECT_THROW_WITH_MESSAGE(Config::Utility::translateRdsConfig(*json_object_ptr, rds), + EnvoyException, err_prefix + err_suffix); + } +} + } // namespace Config } // namespace Envoy diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index d1f3173240757..185947c24be6a 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -232,22 +232,6 @@ TEST_F(ClusterManagerImplTest, UnknownHcType) { EXPECT_THROW(create(parseBootstrapFromJson(json)), EnvoyException); } -TEST_F(ClusterManagerImplTest, MaxClusterName) { - const std::string json = R"EOF( - { - "clusters": [ - { - "name": "clusterwithareallyreallylongnamemorethanmaxcharsallowedbyschema" - }] - } - )EOF"; - - EXPECT_THROW_WITH_MESSAGE(create(parseBootstrapFromJson(json)), Json::Exception, - "JSON at lines 4-6 does not conform to schema.\n Invalid schema: " - "#/properties/name\n Schema violation: maxLength\n Offending " - "document key: #/name"); -} - TEST_F(ClusterManagerImplTest, ValidClusterName) { const std::string json = R"EOF( { diff --git a/test/integration/hotrestart_test.sh b/test/integration/hotrestart_test.sh index 2a794b2478bbc..ed72cb359e104 100755 --- a/test/integration/hotrestart_test.sh +++ b/test/integration/hotrestart_test.sh @@ -80,8 +80,8 @@ do exit 2 fi - echo "Checking max-stat-name-len" - CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-stat-name-len 1234 2>&1) + echo "Checking max-obj-name-len" + CLI_HOT_RESTART_VERSION=$("${ENVOY_BIN}" --hot-restart-version --max-obj-name-len 1234 2>&1) if [[ "${ADMIN_HOT_RESTART_VERSION}" = "${CLI_HOT_RESTART_VERSION}" ]]; then echo "Hot restart version match when it should mismatch: ${ADMIN_HOT_RESTART_VERSION} == " \ "${CLI_HOT_RESTART_VERSION}" diff --git a/test/integration/server.h b/test/integration/server.h index b38f21657993c..2b7a8e73c0af8 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -51,7 +51,7 @@ class TestOptionsImpl : public Options { const std::string& serviceNodeName() override { return service_node_name_; } const std::string& serviceZone() override { return service_zone_; } uint64_t maxStats() override { return 16384; } - uint64_t maxStatNameLength() override { return 127; } + uint64_t maxObjNameLength() override { return 60; } private: const std::string config_path_; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index bf55d371cf73c..fb787f2aecac7 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -26,7 +26,7 @@ MockOptions::MockOptions(const std::string& config_path) ON_CALL(*this, serviceZone()).WillByDefault(ReturnRef(service_zone_name_)); ON_CALL(*this, logPath()).WillByDefault(ReturnRef(log_path_)); ON_CALL(*this, maxStats()).WillByDefault(Return(1000)); - ON_CALL(*this, maxStatNameLength()).WillByDefault(Return(150)); + ON_CALL(*this, maxObjNameLength()).WillByDefault(Return(150)); } MockOptions::~MockOptions() {} diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index a8cf4ae9e8f15..0e1bcdcde58ed 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -59,7 +59,7 @@ class MockOptions : public Options { MOCK_METHOD0(serviceNodeName, const std::string&()); MOCK_METHOD0(serviceZone, const std::string&()); MOCK_METHOD0(maxStats, uint64_t()); - MOCK_METHOD0(maxStatNameLength, uint64_t()); + MOCK_METHOD0(maxObjNameLength, uint64_t()); std::string config_path_; std::string admin_address_path_; diff --git a/test/server/BUILD b/test/server/BUILD index d338371f65dd6..5c86a7640778c 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -59,6 +59,7 @@ envoy_cc_test( name = "hot_restart_impl_test", srcs = envoy_select_hot_restart(["hot_restart_impl_test.cc"]), deps = [ + "//source/common/stats:stats_lib", "//source/server:hot_restart_lib", "//test/mocks/server:server_mocks", ], diff --git a/test/server/hot_restart_impl_test.cc b/test/server/hot_restart_impl_test.cc index 2ef43ac122f4c..a76ce8618193d 100644 --- a/test/server/hot_restart_impl_test.cc +++ b/test/server/hot_restart_impl_test.cc @@ -1,3 +1,5 @@ +#include "common/stats/stats_impl.h" + #include "server/hot_restart_impl.h" #include "test/mocks/api/mocks.h" @@ -50,8 +52,10 @@ class HotRestartImplTest : public testing::Test { TEST_F(HotRestartImplTest, versionString) { setup(); - EXPECT_EQ(hot_restart_->version(), Envoy::Server::SharedMemory::version( - options_.maxStats(), options_.maxStatNameLength())); + EXPECT_EQ(hot_restart_->version(), + Envoy::Server::SharedMemory::version(options_.maxStats(), + options_.maxObjNameLength() + + Stats::RawStatData::maxStatSuffixLength())); } TEST_F(HotRestartImplTest, crossAlloc) { @@ -101,9 +105,9 @@ class HotRestartImplAlignmentTest : public HotRestartImplTest, public: HotRestartImplAlignmentTest() : name_len_(8 + GetParam()) { EXPECT_CALL(options_, maxStats()).WillRepeatedly(Return(num_stats_)); - EXPECT_CALL(options_, maxStatNameLength()).WillRepeatedly(Return(name_len_)); + EXPECT_CALL(options_, maxObjNameLength()).WillRepeatedly(Return(name_len_)); setup(); - EXPECT_EQ(name_len_, Stats::RawStatData::maxNameLength()); + EXPECT_EQ(name_len_, Stats::RawStatData::maxObjNameLength()); } static const uint64_t num_stats_ = 8; @@ -131,7 +135,10 @@ TEST_P(HotRestartImplAlignmentTest, objectOverlap) { }; std::vector stats; for (uint64_t i = 0; i < num_stats_; i++) { - std::string name = fmt::format("{}zzzzzzzzzzzzzzzzzzzzzzzzzzzz", i) + std::string name = fmt::format("{}zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz" + "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz" + "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", + i) .substr(0, Stats::RawStatData::maxNameLength()); TestStat ts; ts.stat_ = hot_restart_->alloc(name); diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index ad237027527c7..e87826da98f9a 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -72,8 +72,8 @@ TEST(OptionsImplTest, BadCliOption) { "error: unknown IP address version 'foo'"); } -TEST(OptionsImplTest, BadStatNameLenOption) { - EXPECT_DEATH(createOptionsImpl("envoy --max-stat-name-len 1"), - "error: the 'max-stat-name-len' value specified"); +TEST(OptionsImplTest, BadObjNameLenOption) { + EXPECT_DEATH(createOptionsImpl("envoy --max-obj-name-len 1"), + "error: the 'max-obj-name-len' value specified"); } } // namespace Envoy