From 29f199c8625208a42483a1fa2de622c0fa105f49 Mon Sep 17 00:00:00 2001 From: Jyoti Mahapatra <49211422+jyotimahapatra@users.noreply.github.com> Date: Thu, 29 Aug 2019 11:30:50 -0700 Subject: [PATCH] tools: deprecated field check in Route Checker tool (#8058) We need a way to run the deprecated field check on the RouteConfiguration. Today the schema check tool validates the bootstrap config. This change will help achieve similar functionality on routes served from rds. Risk Level: Low Testing: Manual testing Docs Changes: included Release Notes: included Signed-off-by: Jyoti Mahapatra --- .../install/tools/route_table_check_tool.rst | 10 +++ docs/root/intro/version_history.rst | 1 + test/tools/router_check/router.cc | 9 ++- test/tools/router_check/router.h | 10 ++- test/tools/router_check/router_check.cc | 5 +- .../test/config/HeaderMatchedRouting.yaml | 4 +- .../router_check/test/config/TestRoutes.yaml | 63 ++++++++++++++----- 7 files changed, 83 insertions(+), 19 deletions(-) diff --git a/docs/root/install/tools/route_table_check_tool.rst b/docs/root/install/tools/route_table_check_tool.rst index ac0b523eec09..8acb2ac34a9d 100644 --- a/docs/root/install/tools/route_table_check_tool.rst +++ b/docs/root/install/tools/route_table_check_tool.rst @@ -40,6 +40,16 @@ Usage -p, --useproto Use Proto test file schema + -f, --fail-under + Represents a percent value for route test coverage under which the run should fail. + + --covall + Enables comprehensive code coverage percent calculation taking into account all the possible + asserts. + + --disable-deprecation-check + Disables the deprecation check for RouteConfiguration proto. + -h, --help Displays usage information and exits. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index c3598a5c7c7a..9667356cc3d8 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -45,6 +45,7 @@ Version history * router: added :ref:`rq_retry_skipped_request_not_complete ` counter stat to router stats. * router check tool: add coverage reporting & enforcement. * router check tool: add comprehensive coverage reporting. +* router check tool: add deprecated field check. * tls: added verification of IP address SAN fields in certificates against configured SANs in the certificate validation context. * tracing: added tags for gRPC response status and meesage. diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 6922d26cea2f..5eeadc7a1bb1 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -63,7 +63,8 @@ ToolConfig::ToolConfig(std::unique_ptr headers, int ran : headers_(std::move(headers)), random_value_(random_value) {} // static -RouterCheckTool RouterCheckTool::create(const std::string& router_config_file) { +RouterCheckTool RouterCheckTool::create(const std::string& router_config_file, + const bool disableDeprecationCheck) { // TODO(hennna): Allow users to load a full config and extract the route configuration from it. envoy::api::v2::RouteConfiguration route_config; auto stats = std::make_unique(); @@ -72,6 +73,9 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file) { auto factory_context = std::make_unique>(); auto config = std::make_unique(route_config, *factory_context, false); + if (!disableDeprecationCheck) { + MessageUtil::checkForDeprecation(route_config, &factory_context->runtime_loader_); + } return RouterCheckTool(std::move(factory_context), std::move(config), std::move(stats), std::move(api), Coverage(route_config)); @@ -439,6 +443,8 @@ Options::Options(int argc, char** argv) { TCLAP::CmdLine cmd("router_check_tool", ' ', "none", true); TCLAP::SwitchArg is_proto("p", "useproto", "Use Proto test file schema", cmd, false); TCLAP::SwitchArg is_detailed("d", "details", "Show detailed test execution results", cmd, false); + TCLAP::SwitchArg disable_deprecation_check("", "disable-deprecation-check", + "Disable deprecated fields check", cmd, false); TCLAP::ValueArg fail_under("f", "fail-under", "Fail if test coverage is under a specified amount", false, 0.0, "float", cmd); @@ -461,6 +467,7 @@ Options::Options(int argc, char** argv) { is_detailed_ = is_detailed.getValue(); fail_under_ = fail_under.getValue(); comprehensive_coverage_ = comprehensive_coverage.getValue(); + disable_deprecation_check_ = disable_deprecation_check.getValue(); if (is_proto_) { config_path_ = config_path.getValue(); diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 7c81835da7ae..e2156afa1072 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -65,10 +65,12 @@ class RouterCheckTool : Logger::Loggable { public: /** * @param router_config_file v2 router config file. + * @param disableDeprecationCheck flag to disable the RouteConfig deprecated field check * @return RouterCheckTool a RouterCheckTool instance with member variables set by the router * config file. * */ - static RouterCheckTool create(const std::string& router_config_file); + static RouterCheckTool create(const std::string& router_config_file, + const bool disableDeprecationCheck); /** * TODO(tonya11en): Use a YAML format for the expected routes. This will require a proto. @@ -198,6 +200,11 @@ class Options { */ bool isDetailed() const { return is_detailed_; } + /** + * @return true if the deprecated field check for RouteConfiguration is disabled. + */ + bool disableDeprecationCheck() const { return disable_deprecation_check_; } + private: std::string test_path_; std::string config_path_; @@ -207,5 +214,6 @@ class Options { bool comprehensive_coverage_; bool is_proto_; bool is_detailed_; + bool disable_deprecation_check_; }; } // namespace Envoy diff --git a/test/tools/router_check/router_check.cc b/test/tools/router_check/router_check.cc index e17f3eecc827..f3856e285d8c 100644 --- a/test/tools/router_check/router_check.cc +++ b/test/tools/router_check/router_check.cc @@ -10,8 +10,9 @@ int main(int argc, char* argv[]) { const bool enforce_coverage = options.failUnder() != 0.0; try { Envoy::RouterCheckTool checktool = - options.isProto() ? Envoy::RouterCheckTool::create(options.configPath()) - : Envoy::RouterCheckTool::create(options.unlabelledConfigPath()); + options.isProto() ? Envoy::RouterCheckTool::create(options.configPath(), + options.disableDeprecationCheck()) + : Envoy::RouterCheckTool::create(options.unlabelledConfigPath(), true); if (options.isDetailed()) { checktool.setShowDetails(); diff --git a/test/tools/router_check/test/config/HeaderMatchedRouting.yaml b/test/tools/router_check/test/config/HeaderMatchedRouting.yaml index f28c96a50a03..5f891bea08d5 100644 --- a/test/tools/router_check/test/config/HeaderMatchedRouting.yaml +++ b/test/tools/router_check/test/config/HeaderMatchedRouting.yaml @@ -30,7 +30,9 @@ virtual_hosts: prefix: / headers: - name: test_header_pattern - regex_match: ^user=test-\d+$ + safe_regex_match: + google_re2: {} + regex: ^user=test-\d+$ route: cluster: local_service_with_header_pattern_set_regex - match: diff --git a/test/tools/router_check/test/config/TestRoutes.yaml b/test/tools/router_check/test/config/TestRoutes.yaml index 34b665fb9fd5..862b6a4da8d5 100644 --- a/test/tools/router_check/test/config/TestRoutes.yaml +++ b/test/tools/router_check/test/config/TestRoutes.yaml @@ -104,26 +104,61 @@ virtual_hosts: timeout: seconds: 30 virtual_clusters: - - pattern: ^/rides$ - method: POST + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/rides$ + - name: :method + exact_match: POST name: ride_request - - pattern: ^/rides/\d+$ - method: PUT + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/rides/\d+$ + - name: :method + exact_match: PUT name: update_ride - - pattern: ^/users/\d+/chargeaccounts$ - method: POST + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/users/\d+/chargeaccounts$ + - name: :method + exact_match: POST name: cc_add - - pattern: ^/users/\d+/chargeaccounts/(?!validate)\w+$ - method: PUT + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/users/\d+/chargeaccounts/[^validate]\w+$ + - name: :method + exact_match: PUT name: cc_add - - pattern: ^/users$ - method: POST + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/users$ + - name: :method + exact_match: POST name: create_user_login - - pattern: ^/users/\d+$ - method: PUT + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/users/\d+$ + - name: :method + exact_match: PUT name: update_user - - pattern: ^/users/\d+/location$ - method: POST + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/users/\d+/location$ + - name: :method + exact_match: POST name: ulu internal_only_headers: - x-lyft-user-id