diff --git a/docs/root/configuration/tools/router_check.rst b/docs/root/configuration/tools/router_check.rst index fa6dd076eafb..5a596521e95c 100644 --- a/docs/root/configuration/tools/router_check.rst +++ b/docs/root/configuration/tools/router_check.rst @@ -3,9 +3,11 @@ Route table check tool ====================== -**NOTE: The following configuration is for the route table check tool only and is not part of the Envoy binary. -The route table check tool is a standalone binary that can be used to verify Envoy's routing for a given configuration -file.** +.. note:: + + The following configuration is for the route table check tool only and is not part of the Envoy binary. + The route table check tool is a standalone binary that can be used to verify Envoy's routing for a given configuration + file. The following specifies input to the route table check tool. The route table check tool checks if the route returned by a :ref:`router ` matches what is expected. @@ -148,3 +150,23 @@ validate value *(required, string)* The value of the header field to match. + +Coverage +-------- + +The router check tool will report route coverage at the end of a successful test run. + +.. code:: bash + + > bazel-bin/test/tools/router_check/router_check_tool --config-path ... --test-path ... --useproto + Current route coverage: 0.0744863 + +This reporting can be leveraged to enforce a minimum coverage percentage by using +the `-f` or `--fail-under` flag. If coverage falls below this percentage the test +run will fail. + +.. code:: bash + + > bazel-bin/test/tools/router_check/router_check_tool --config-path ... --test-path ... --useproto --fail-under 0.08 + Current route coverage: 0.0744863 + Failed to meet coverage requirement: 0.08 diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 41c6509e9c7d..1ae489d86fef 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -14,6 +14,7 @@ Version history * http: added the ability to :ref:`merge adjacent slashes` in the path. * listeners: added :ref:`HTTP inspector listener filter `. * router: added :ref:`rq_retry_skipped_request_not_complete ` counter stat to router stats. +* router check tool: add coverage reporting & enforcement. * tls: added verification of IP address SAN fields in certificates against configured SANs in the certificate validation context. * upstream: added network filter chains to upstream connections, see :ref:`filters`. diff --git a/test/tools/router_check/BUILD b/test/tools/router_check/BUILD index ceb012625dd5..c4a1a2facf4b 100644 --- a/test/tools/router_check/BUILD +++ b/test/tools/router_check/BUILD @@ -19,6 +19,8 @@ envoy_cc_test_binary( envoy_cc_test_library( name = "router_check_main_lib", srcs = [ + "coverage.cc", + "coverage.h", "router.cc", "router.h", ], diff --git a/test/tools/router_check/coverage.cc b/test/tools/router_check/coverage.cc new file mode 100644 index 000000000000..5f8c51a51351 --- /dev/null +++ b/test/tools/router_check/coverage.cc @@ -0,0 +1,23 @@ +#include "test/tools/router_check/coverage.h" + +#include + +#include "envoy/api/v2/core/base.pb.h" + +namespace Envoy { +void Coverage::markCovered(const Envoy::Router::RouteEntry& route) { + // n.b. If we reach the end of the seen routes without finding the specified + // route we add it as seen, otherwise it's a duplicate. + if (std::find(seen_routes_.begin(), seen_routes_.end(), &route) == seen_routes_.end()) { + seen_routes_.push_back(&route); + } +} + +double Coverage::report() { + uint64_t num_routes = 0; + for (const auto& host : route_config_.virtual_hosts()) { + num_routes += host.routes_size(); + } + return 100 * static_cast(seen_routes_.size()) / num_routes; +} +} // namespace Envoy diff --git a/test/tools/router_check/coverage.h b/test/tools/router_check/coverage.h new file mode 100644 index 000000000000..778906a89fca --- /dev/null +++ b/test/tools/router_check/coverage.h @@ -0,0 +1,18 @@ +#pragma once + +#include "envoy/router/router.h" + +#include "test/mocks/server/mocks.h" + +namespace Envoy { +class Coverage : Logger::Loggable { +public: + Coverage(envoy::api::v2::RouteConfiguration config) : route_config_(config){}; + void markCovered(const Envoy::Router::RouteEntry& route); + double report(); + +private: + std::vector seen_routes_; + const envoy::api::v2::RouteConfiguration route_config_; +}; +} // namespace Envoy diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 74b1437fca59..70ed8c6fbd19 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -74,15 +74,15 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file) { auto config = std::make_unique(route_config, *factory_context, false); return RouterCheckTool(std::move(factory_context), std::move(config), std::move(stats), - std::move(api)); + std::move(api), Coverage(route_config)); } RouterCheckTool::RouterCheckTool( std::unique_ptr> factory_context, std::unique_ptr config, std::unique_ptr stats, - Api::ApiPtr api) + Api::ApiPtr api, Coverage coverage) : factory_context_(std::move(factory_context)), config_(std::move(config)), - stats_(std::move(stats)), api_(std::move(api)) { + stats_(std::move(stats)), api_(std::move(api)), coverage_(std::move(coverage)) { ON_CALL(factory_context_->runtime_loader_.snapshot_, featureEnabled(_, testing::An(), testing::An())) @@ -281,7 +281,11 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str actual = tool_config.headers_->get_(Http::Headers::get().Path); } - return compareResults(actual, expected, "path_rewrite"); + const bool matches = compareResults(actual, expected, "path_rewrite"); + if (matches) { + coverage_.markCovered(*tool_config.route_->routeEntry()); + } + return matches; } bool RouterCheckTool::compareRewritePath( @@ -415,6 +419,9 @@ 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::ValueArg fail_under("f", "fail-under", + "Fail if test coverage is under a specified amount", false, + 0.0, "float", cmd); TCLAP::ValueArg config_path("c", "config-path", "Path to configuration file.", false, "", "string", cmd); TCLAP::ValueArg test_path("t", "test-path", "Path to test file.", false, "", @@ -430,6 +437,7 @@ Options::Options(int argc, char** argv) { is_proto_ = is_proto.getValue(); is_detailed_ = is_detailed.getValue(); + fail_under_ = fail_under.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 6c507f5bc8bd..f4af695dcf7a 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -17,6 +17,7 @@ #include "test/test_common/global.h" #include "test/test_common/printers.h" #include "test/test_common/utility.h" +#include "test/tools/router_check/coverage.h" #include "test/tools/router_check/json/tool_config_schemas.h" #include "test/tools/router_check/validation.pb.h" #include "test/tools/router_check/validation.pb.validate.h" @@ -88,11 +89,13 @@ class RouterCheckTool : Logger::Loggable { */ void setShowDetails() { details_ = true; } + float coverage() { return coverage_.report(); } + private: RouterCheckTool( std::unique_ptr> factory_context, std::unique_ptr config, std::unique_ptr stats, - Api::ApiPtr api); + Api::ApiPtr api, Coverage coverage); bool compareCluster(ToolConfig& tool_config, const std::string& expected); bool compareCluster(ToolConfig& tool_config, @@ -143,6 +146,7 @@ class RouterCheckTool : Logger::Loggable { std::unique_ptr stats_; Api::ApiPtr api_; std::string active_runtime; + Coverage coverage_; }; /** @@ -172,6 +176,11 @@ class Options { */ const std::string& unlabelledTestPath() const { return unlabelled_test_path_; } + /** + * @return the minimum required percentage of routes coverage. + */ + double failUnder() const { return fail_under_; } + /** * @return true if proto schema test is used. */ @@ -187,6 +196,7 @@ class Options { std::string config_path_; std::string unlabelled_test_path_; std::string unlabelled_config_path_; + float fail_under_; bool is_proto_; bool is_detailed_; }; diff --git a/test/tools/router_check/router_check.cc b/test/tools/router_check/router_check.cc index 40f6acec3948..d7b27d99f3a3 100644 --- a/test/tools/router_check/router_check.cc +++ b/test/tools/router_check/router_check.cc @@ -7,6 +7,7 @@ int main(int argc, char* argv[]) { Envoy::Options options(argc, argv); + const bool enforce_coverage = options.failUnder() != 0.0; try { Envoy::RouterCheckTool checktool = options.isProto() ? Envoy::RouterCheckTool::create(options.configPath()) @@ -23,6 +24,16 @@ int main(int argc, char* argv[]) { if (!is_equal) { return EXIT_FAILURE; } + + const double current_coverage = checktool.coverage(); + std::cerr << "Current route coverage: " << current_coverage << "%" << std::endl; + if (enforce_coverage) { + if (current_coverage < options.failUnder()) { + std::cerr << "Failed to meet coverage requirement: " << options.failUnder() << "%" + << std::endl; + return EXIT_FAILURE; + } + } } catch (const Envoy::EnvoyException& ex) { std::cerr << ex.what() << std::endl; return EXIT_FAILURE; diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 366c23a73333..cba937d3f7e3 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -16,6 +16,20 @@ do TEST_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/${t}.yaml" "${PATH_CONFIG}/${t}.golden.json" "--details") done +# Testing coverage flag passes +COVERAGE_CMD="${PATH_BIN} ${PATH_CONFIG}/Redirect.yaml ${PATH_CONFIG}/Redirect.golden.json --details -f " +TEST_OUTPUT=$($COVERAGE_CMD "1.0") +COVERAGE_OUTPUT=$($COVERAGE_CMD "1.0" 2>&1) || echo "${COVERAGE_OUTPUT:-no-output}" +if [[ "${COVERAGE_OUTPUT}" != *"Current route coverage: "* ]] ; then + exit 1 +fi + +# Testing coverage flag fails +COVERAGE_OUTPUT=$($COVERAGE_CMD "100" 2>&1) || echo "${COVERAGE_OUTPUT:-no-output}" +if [[ "${COVERAGE_OUTPUT}" != *"Failed to meet coverage requirement: 100%"* ]] ; then + exit 1 +fi + # Testing expected matches using --useproto # --useproto needs the test schema as a validation.proto message. TESTS+=("Runtime") @@ -31,17 +45,17 @@ TEST_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/Weighted.yaml" "-t" "${PATH_CON TEST_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/Weighted.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.pb_text" "--details" "--useproto") # Bad config file -echo testing bad config output +echo "testing bad config output" BAD_CONFIG_OUTPUT=$(("${PATH_BIN}" "${PATH_CONFIG}/Redirect.golden.json" "${PATH_CONFIG}/TestRoutes.yaml") 2>&1) || - echo ${BAD_CONFIG_OUTPUT:-no-output} + echo "${BAD_CONFIG_OUTPUT:-no-output}" if [[ "${BAD_CONFIG_OUTPUT}" != *"Unable to parse"* ]]; then exit 1 fi # Failure test case -echo testing failure test case +echo "testing failure test case" FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" 2>&1) || - echo ${FAILURE_OUTPUT:-no-output} + echo "${FAILURE_OUTPUT:-no-output}" if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi