tools: add coverage reporting & enforcement to router check

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 <envoy_api_msg_RouteConfiguration>` 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

docs/root/intro/version_history.rst

@@ -14,6 +14,7 @@ Version history
 * http: added the ability to :ref:`merge adjacent slashes<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.merge_slashes>` in the path.
 * listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
 * router: added :ref:`rq_retry_skipped_request_not_complete <config_http_filters_router_stats>` 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<envoy_api_field_Cluster.filters>`.

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",
     ],

test/tools/router_check/coverage.cc

@@ -0,0 +1,23 @@
+#include "test/tools/router_check/coverage.h"
+
+#include <algorithm>
+
+#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<double>(seen_routes_.size()) / num_routes;
+}
+} // namespace Envoy

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<Logger::Id::testing> {
+public:
+  Coverage(envoy::api::v2::RouteConfiguration config) : route_config_(config){};
+  void markCovered(const Envoy::Router::RouteEntry& route);
+  double report();
+
+private:
+  std::vector<const Envoy::Router::RouteEntry*> seen_routes_;
+  const envoy::api::v2::RouteConfiguration route_config_;
+};
+} // namespace Envoy

test/tools/router_check/router.cc

@@ -74,15 +74,15 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file) {
   auto config = std::make_unique<Router::ConfigImpl>(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<NiceMock<Server::Configuration::MockFactoryContext>> factory_context,
     std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> 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<const envoy::type::FractionalPercent&>(),
                          testing::An<uint64_t>()))
@@ -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<double> fail_under("f", "fail-under",
+                                     "Fail if test coverage is under a specified amount", false,
+                                     0.0, "float", cmd);
   TCLAP::ValueArg<std::string> config_path("c", "config-path", "Path to configuration file.", false,
                                            "", "string", cmd);
   TCLAP::ValueArg<std::string> 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();

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<Logger::Id::testing> {
    */
   void setShowDetails() { details_ = true; }
 
+  float coverage() { return coverage_.report(); }
+
 private:
   RouterCheckTool(
       std::unique_ptr<NiceMock<Server::Configuration::MockFactoryContext>> factory_context,
       std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> 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<Logger::Id::testing> {
   std::unique_ptr<Stats::IsolatedStoreImpl> 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_;
 };

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;

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