Merge remote-tracking branch 'upstream/master' into build-flags-pr

.gitignore

@@ -6,6 +6,7 @@
 /ci/bazel-*
 /ci/prebuilt/thirdparty
 /ci/prebuilt/thirdparty_build
+/test/coverage/BUILD
 cscope.*
 BROWSE
 .deps

bazel/BUILD

@@ -1,5 +1,7 @@
 package(default_visibility = ["//visibility:public"])
 
+exports_files(["gen_sh_test_runner.sh"])
+
 config_setting(
     name = "opt_build",
     values = {"compilation_mode": "opt"},

bazel/envoy_build_system.bzl

@@ -37,8 +37,7 @@ def envoy_cc_library(name,
                      visibility = None,
                      external_deps = [],
                      repository = "",
-                     deps = [],
-                     alwayslink = None):
+                     deps = []):
     native.cc_library(
         name = name,
         srcs = srcs,
@@ -50,7 +49,7 @@ def envoy_cc_library(name,
             repository + "//source/precompiled:precompiled_includes",
         ],
         include_prefix = envoy_include_prefix(PACKAGE_NAME),
-        alwayslink = alwayslink,
+        alwayslink = 1,
     )
 
 # Envoy C++ binary targets should be specified with this function.
@@ -82,51 +81,34 @@ def envoy_cc_binary(name,
 def envoy_cc_test(name,
                   srcs = [],
                   data = [],
-                  args = [],
                   # List of pairs (Bazel shell script target, shell script args)
-                  setup_cmds = [],
                   repository = "",
-                  deps = []):
-    if setup_cmds:
-        setup_cmd = "; ".join(["$(location " + setup_sh + ") " + " ".join(setup_args) for
-                               setup_sh, setup_args in setup_cmds])
-        envoy_test_setup_flag = "--envoy_test_setup=\"" + setup_cmd + "\""
-        data += [setup_sh for setup_sh, _ in setup_cmds]
-        args += [envoy_test_setup_flag]
-    native.cc_test(
-        name = name,
+                  deps = [],
+                  tags = [],
+                  coverage = True,
+                  local = False):
+    test_lib_tags = []
+    if coverage:
+      test_lib_tags.append("coverage_test_lib")
+    envoy_cc_test_library(
+        name = name + "_lib",
         srcs = srcs,
         data = data,
-        copts = ENVOY_COPTS + ["-includetest/precompiled/precompiled_test.h"],
+        deps = deps,
+        repository = repository,
+        tags = test_lib_tags,
+    )
+    native.cc_test(
+        name = name,
+        copts = ENVOY_COPTS,
         linkopts = ["-pthread"],
         linkstatic = 1,
-        args = args,
-        deps = deps + [
-            repository + "//source/precompiled:precompiled_includes",
-            repository + "//test/precompiled:precompiled_includes",
-            repository + "//test:main",
+        deps = [
+            ":" + name + "_lib",
+            repository + "//test:main"
         ],
-    )
-
-# Envoy C++ test targets that depend on JSON config files subject to template
-# environment substitution should be specified with this function.
-def envoy_cc_test_with_json(name,
-                            data = [],
-                            jsons = [],
-                            setup_cmds = [],
-                            repository = "",
-                            deps = [],
-                            **kargs):
-    envoy_cc_test(
-        name = name,
-        data = data + jsons,
-        setup_cmds = setup_cmds + [(
-            repository + "//test/test_common:environment_sub.sh",
-            ["$(location " + f + ")" for f in jsons],
-        )],
-        repository = repository,
-        deps = deps,
-        **kargs
+        tags = tags + ["coverage_test"],
+        local = local,
     )
 
 # Envoy C++ test related libraries (that want gtest, gmock) should be specified
@@ -138,7 +120,7 @@ def envoy_cc_test_library(name,
                           external_deps = [],
                           deps = [],
                           repository = "",
-                          alwayslink = None):
+                          tags = []):
     native.cc_library(
         name = name,
         srcs = srcs,
@@ -150,13 +132,41 @@ def envoy_cc_test_library(name,
             repository + "//source/precompiled:precompiled_includes",
             repository + "//test/precompiled:precompiled_includes",
         ],
-        alwayslink = alwayslink,
+        tags = tags,
+        alwayslink = 1,
     )
 
 # Envoy C++ mock targets should be specified with this function.
 def envoy_cc_mock(name, **kargs):
     envoy_cc_test_library(name = name, **kargs)
 
+# Envoy shell tests that need to be included in coverage run should be specified with this function.
+def envoy_sh_test(name,
+                  srcs = [],
+                  data = [],
+                  **kargs):
+  test_runner_cc = name + "_test_runner.cc"
+  native.genrule(
+      name = name + "_gen_test_runner",
+      srcs = srcs,
+      outs = [test_runner_cc],
+      cmd = "$(location //bazel:gen_sh_test_runner.sh) $(location " + srcs[0] + ") >> $@",
+      tools = ["//bazel:gen_sh_test_runner.sh"],
+  )
+  envoy_cc_test_library(
+      name = name + "_lib",
+      srcs = [test_runner_cc],
+      data = srcs + data,
+      tags = ["coverage_test_lib"],
+      deps = ["//test/test_common:environment_lib"],
+  )
+  native.sh_test(
+      name = name,
+      srcs = srcs,
+      data = srcs + data,
+      **kargs
+  )
+
 def _proto_header(proto_path):
   if proto_path.endswith(".proto"):
       return proto_path[:-5] + "pb.h"

bazel/gen_sh_test_runner.sh

@@ -0,0 +1,25 @@
+#!/bin/bash
+
+# Used in a genrule to wrap sh_test script for execution in
+# //test/coverage:coverage_tests single binary.
+
+RAW_TEST_NAME="$(basename "$1")"
+# Normalize to something we can use in a TEST(ShTest, ...) name
+TEST_NAME="${RAW_TEST_NAME//./_}"
+
+EXEC_ARGS="\"$1\""
+shift
+for a in $@
+do
+  EXEC_ARGS="${EXEC_ARGS}, \"$a\""
+done
+
+(
+  cat << EOF
+#include "test/test_common/environment.h"
+
+TEST(ShTest, ${TEST_NAME}) {
+  TestEnvironment::exec({${EXEC_ARGS}});
+}
+EOF
+)

ci/build_container/build_container.sh

@@ -4,7 +4,7 @@ set -e
 
 # Setup basic requirements and install them.
 apt-get update
-apt-get install -y wget software-properties-common make cmake git python python-pip clang-format-3.6 bc libtool automake
+apt-get install -y wget software-properties-common make cmake git python python-pip clang-format-3.6 bc libtool automake lcov zip
 apt-get install -y golang
 # For debugging.
 apt-get install -y gdb strace
@@ -19,6 +19,17 @@ apt-get update
 apt-get install -y bazel
 rm -rf /var/lib/apt/lists/*
 
+# Build a version of Bazel suitable for C++ code coverage collection. See
+# https://github.com/bazelbuild/bazel/issues/1118 for why we need this. This is the envoy-coverage
+# branch on the cloned repository.
+git clone https://github.com/htuch/bazel.git /tmp/bazel-coverage
+pushd /tmp/bazel-coverage
+git checkout 63f0542560773e973c9963845d5bbc30be75441a
+bazel build --spawn_strategy=standalone --genrule_strategy=standalone //src:bazel
+cp bazel-bin/src/bazel /usr/bin/bazel-coverage
+popd
+rm -rf /root/.cache /tmp/bazel-coverage
+
 # virtualenv
 pip install virtualenv
 
@@ -33,4 +44,10 @@ export CXX=g++-4.9
 export THIRDPARTY_DEPS=/tmp
 export THIRDPARTY_SRC=/thirdparty
 export THIRDPARTY_BUILD=/thirdparty_build
+# TODO(htuch): Remove the first build of the libraries in non-distinct locations when cmake is gone.
+# Below we now build/install twice and this requires 2x the space in the Docker image as is to
+# support both Bazel and cmake, but it's not worth fixing up all the cmake stuff since it's going
+# soon.
 "$(dirname "$0")"/build_and_install_deps.sh
+rm -f /tmp/*.dep
+BUILD_DISTINCT=1 "$(dirname "$0")"/build_and_install_deps.sh

docs/configuration/http_conn_man/route_config/route.rst

@@ -27,6 +27,7 @@ next (e.g., redirect, forward, rewrite, etc.).
     "priority": "...",
     "headers": [],
     "rate_limits": [],
+    "include_vh_rate_limits" : "...",
     "hash_policy": "{...}",
     "request_headers_to_add" : [],
     "opaque_config": []
@@ -156,6 +157,14 @@ priority
   *(optional, array)* Specifies a set of rate limit configurations that could be applied to the
   route.
 
+.. _config_http_conn_man_route_table_route_include_vh:
+
+include_vh_rate_limits
+  *(optional, boolean)* Specifies if the rate limit filter should include the virtual host rate
+  limits. By default, if the route configured rate limits, the virtual host
+  :ref:`rate_limits <config_http_conn_man_route_table_rate_limit_config>` are not applied to the
+  request.
+
 :ref:`hash_policy <config_http_conn_man_route_table_hash_policy>`
   *(optional, array)* Specifies the route's hashing policy if the upstream cluster uses a hashing
   :ref:`load balancer <arch_overview_load_balancing_types>`.
@@ -369,12 +378,11 @@ Opaque Config
 
 Additional configuration can be provided to filters through the "Opaque Config" mechanism. A
 list of properties are specified in the route config. The configuration is uninterpreted
-by envoy and can be accessed within a user-defined filter. The configuration is a generic 
+by envoy and can be accessed within a user-defined filter. The configuration is a generic
 string map.  Nested objects are not supported.
 
 .. code-block:: json
 
   [
     {"...": "..."}
   ]
-

docs/configuration/http_filters/rate_limit_filter.rst

@@ -5,10 +5,11 @@ Rate limit
 
 Global rate limiting :ref:`architecture overview <arch_overview_rate_limit>`.
 
-The HTTP rate limit filter will call the rate limit service when the request's route has one or
-more :ref:`rate limit configurations<config_http_conn_man_route_table_route_rate_limits>` that match the
-filter stage setting. More than one configuration can apply to a request. Each configuration
-results in a descriptor being sent to the rate limit service.
+The HTTP rate limit filter will call the rate limit service when the request's route or virtual host
+has one or more :ref:`rate limit configurations<config_http_conn_man_route_table_route_rate_limits>`
+that match the filter stage setting. The :ref:`route<config_http_conn_man_route_table_route_include_vh>`
+can optionally include the virtual host rate limit configurations. More than one configuration can
+apply to a request. Each configuration results in a descriptor being sent to the rate limit service.
 
 If the rate limit service is called, and the response for any of the descriptors is over limit, a
 429 response is returned.

include/envoy/router/router.h

@@ -231,6 +231,11 @@ class RouteEntry {
    *         with the route
    */
   virtual const std::multimap<std::string, std::string>& opaqueConfig() const PURE;
+
+  /**
+   * @return bool true if the virtual host rate limits should be included.
+   */
+  virtual bool includeVirtualHostRateLimits() const PURE;
 };
 
 /**

include/envoy/router/router_ratelimit.h

@@ -68,6 +68,11 @@ class RateLimitPolicy {
 public:
   virtual ~RateLimitPolicy() {}
 
+  /**
+   * @return true if there is no rate limit policy for all stage settings.
+   */
+  virtual bool empty() const PURE;
+
   /**
    * @param stage the value for finding applicable rate limit configurations.
    * @return set of RateLimitPolicyEntry that are applicable for a stage.

source/common/CMakeLists.txt

@@ -110,6 +110,7 @@ add_library(
   stats/thread_local_store.cc
   thread_local/thread_local_impl.cc
   tracing/http_tracer_impl.cc
+  tracing/lightstep_tracer_impl.cc
   upstream/cds_api_impl.cc
   upstream/cluster_manager_impl.cc
   upstream/health_checker_impl.cc

source/common/http/async_client_impl.h

@@ -77,6 +77,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
         getApplicableRateLimit(uint64_t) const override {
       return rate_limit_policy_entry_;
     }
+    bool empty() const override { return true; }
 
     static const std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>>
         rate_limit_policy_entry_;
@@ -132,6 +133,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     }
     const Router::VirtualHost& virtualHost() const override { return virtual_host_; }
     bool autoHostRewrite() const override { return false; }
+    bool includeVirtualHostRateLimits() const override { return true; }
 
     static const NullRateLimitPolicy rate_limit_policy_;
     static const NullRetryPolicy retry_policy_;

source/common/http/filter/ratelimit.cc

@@ -47,9 +47,12 @@ void Filter::initiateCall(const HeaderMap& headers) {
   // Get all applicable rate limit policy entries for the route.
   populateRateLimitDescriptors(route_entry->rateLimitPolicy(), descriptors, route_entry, headers);
 
-  // Get all applicable rate limit policy entries for the virtual host.
-  populateRateLimitDescriptors(route_entry->virtualHost().rateLimitPolicy(), descriptors,
-                               route_entry, headers);
+  // Get all applicable rate limit policy entries for the virtual host if the route opted to
+  // include the virtual host rate limits.
+  if (route_entry->includeVirtualHostRateLimits()) {
+    populateRateLimitDescriptors(route_entry->virtualHost().rateLimitPolicy(), descriptors,
+                                 route_entry, headers);
+  }
 
   if (!descriptors.empty()) {
     state_ = State::Calling;

source/common/json/config_schemas.cc

@@ -594,6 +594,7 @@ const std::string Json::Schema::ROUTE_ENTRY_CONFIGURATION_SCHEMA(R"EOF(
         }
       },
       "rate_limits" : {"type" : "array"},
+      "include_vh_rate_limits" : {"type" : "boolean"},
       "hash_policy" : {
         "type" : "object",
         "properties" : {

source/common/router/config_impl.cc

@@ -143,6 +143,11 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, const Json:
           {Http::LowerCaseString(header->getString("key")), header->getString("value")});
     }
   }
+
+  // Only set include_vh_rate_limits_ to true if the rate limit policy for the route is empty
+  // or the route set `include_vh_rate_limits` to true.
+  include_vh_rate_limits_ =
+      (rate_limit_policy_.empty() || route.getBoolean("include_vh_rate_limits", false));
 }
 
 bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers, uint64_t random_value) const {

source/common/router/config_impl.h

@@ -197,6 +197,7 @@ class RouteEntryImplBase : public RouteEntry,
   const std::multimap<std::string, std::string>& opaqueConfig() const override {
     return opaque_config_;
   }
+  bool includeVirtualHostRateLimits() const override { return include_vh_rate_limits_; }
 
   // Router::RedirectEntry
   std::string newPath(const Http::HeaderMap& headers) const override;
@@ -209,6 +210,7 @@ class RouteEntryImplBase : public RouteEntry,
   const bool case_sensitive_;
   const std::string prefix_rewrite_;
   const std::string host_rewrite_;
+  bool include_vh_rate_limits_;
 
   RouteConstSharedPtr clusterEntry(const Http::HeaderMap& headers, uint64_t random_value) const;
   void finalizePathHeader(Http::HeaderMap& headers, const std::string& matched_path) const;
@@ -248,6 +250,9 @@ class RouteEntryImplBase : public RouteEntry,
 
     const VirtualHost& virtualHost() const override { return parent_->virtualHost(); }
     bool autoHostRewrite() const override { return parent_->autoHostRewrite(); }
+    bool includeVirtualHostRateLimits() const override {
+      return parent_->includeVirtualHostRateLimits();
+    }
 
     // Router::Route
     const RedirectEntry* redirectEntry() const override { return nullptr; }

source/common/router/router_ratelimit.h

@@ -126,6 +126,7 @@ class RateLimitPolicyImpl : public RateLimitPolicy {
   // Router::RateLimitPolicy
   const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>&
   getApplicableRateLimit(uint64_t stage = 0) const override;
+  bool empty() const override { return rate_limit_entries_.empty(); }
 
 private:
   std::vector<std::unique_ptr<RateLimitPolicyEntry>> rate_limit_entries_;