From 2777b3f776d47b008233f97969d24f9f280db72d Mon Sep 17 00:00:00 2001
From: Matt Klein <mattklein123@users.noreply.github.com>
Date: Fri, 7 Apr 2017 10:59:28 -0700
Subject: [PATCH 01/12] test: make guard dog death tests run first always
 (#717)

There appears to be a race condition during thread cleanup and fork
where pthread_join can return but the thread is still being fully
destroyed. This can lead to issues during fork.
---
 test/server/guarddog_impl_test.cc | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc
index 0aebfc2322cf..a901d68629d4 100644
--- a/test/server/guarddog_impl_test.cc
+++ b/test/server/guarddog_impl_test.cc
@@ -67,6 +67,10 @@ class GuardDogDeathTest : public testing::Test {
   WatchDogSharedPtr second_dog_;
 };
 
+// These tests use threads, and need to run after the real death tests, so we need to call them
+// a different name.
+class GuardDogAlmostDeadTest : public GuardDogDeathTest {};
+
 TEST_F(GuardDogDeathTest, KillDeathTest) {
   // Is it German for "The Function"? Almost...
   auto die_function = [&]() -> void {
@@ -78,7 +82,7 @@ TEST_F(GuardDogDeathTest, KillDeathTest) {
   EXPECT_DEATH(die_function(), "");
 }
 
-TEST_F(GuardDogDeathTest, KillNoFinalCheckTest) {
+TEST_F(GuardDogAlmostDeadTest, KillNoFinalCheckTest) {
   // This does everything the death test does except the final force check that
   // should actually result in dying. The death test does not verify that there
   // was not a crash *before* the expected line, so this test checks that.
@@ -93,14 +97,14 @@ TEST_F(GuardDogDeathTest, MultiKillDeathTest) {
   EXPECT_DEATH(die_function(), "");
 }
 
-TEST_F(GuardDogDeathTest, MultiKillNoFinalCheckTest) {
+TEST_F(GuardDogAlmostDeadTest, MultiKillNoFinalCheckTest) {
   // This does everything the death test does except the final force check that
   // should actually result in dying. The death test does not verify that there
   // was not a crash *before* the expected line, so this test checks that.
   SetupForMultiDeath();
 }
 
-TEST_F(GuardDogDeathTest, NearDeathTest) {
+TEST_F(GuardDogAlmostDeadTest, NearDeathTest) {
   // This ensures that if only one thread surpasses the multiple kill threshold
   // there is no death.  The positive case is covered in MultiKillDeathTest.
   InSequence s;

From 96edf4fec36bedda971e8170e347c7d8be7c3bd6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20No=C3=A9?= <dpn@google.com>
Date: Fri, 7 Apr 2017 20:29:06 -0400
Subject: [PATCH 02/12] GuardDog: Test: Fix race in mocked time source. (#719)

The guard dog test made additional EXPECT_CALLs to update the time the
mocked time source should return after the mocked object was passed to
the GD thread.  Doing additional EXPECT_CALLs in this way is NOT thread
safe and resulted in 0.5-1% of tests failing when many were executed in
parallel.

The solution is to make the expect call action be to invoke a lambda
that shares access to a safe atomic updated in the main test thread.
---
 test/server/guarddog_impl_test.cc | 51 ++++++++++++++++---------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc
index a901d68629d4..e96ac3ea89be 100644
--- a/test/server/guarddog_impl_test.cc
+++ b/test/server/guarddog_impl_test.cc
@@ -24,8 +24,12 @@ namespace Server {
 class GuardDogDeathTest : public testing::Test {
 protected:
   GuardDogDeathTest()
-      : config_kill_(1000, 1000, 100, 1000), config_multikill_(1000, 1000, 1000, 500),
-        time_point_(std::chrono::system_clock::now()) {}
+      : config_kill_(1000, 1000, 100, 1000), config_multikill_(1000, 1000, 1000, 500) {
+    ON_CALL(time_source_, currentSystemTime())
+        .WillByDefault(testing::Invoke([&]() {
+          return std::chrono::system_clock::time_point(std::chrono::milliseconds(mock_time_));
+        }));
+  }
 
   /**
    * This does everything but the final forceCheckForTest() that should cause
@@ -33,12 +37,10 @@ class GuardDogDeathTest : public testing::Test {
    */
   void SetupForDeath() {
     InSequence s;
-    EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
     guard_dog_.reset(new GuardDogImpl(fakestats_, config_kill_, time_source_));
     unpet_dog_ = guard_dog_->createWatchDog(0);
     guard_dog_->forceCheckForTest();
-    time_point_ += std::chrono::milliseconds(500);
-    EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+    mock_time_ += 500;
   }
 
   /**
@@ -47,21 +49,19 @@ class GuardDogDeathTest : public testing::Test {
    */
   void SetupForMultiDeath() {
     InSequence s;
-    EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
     guard_dog_.reset(new GuardDogImpl(fakestats_, config_multikill_, time_source_));
     auto unpet_dog_ = guard_dog_->createWatchDog(0);
     guard_dog_->forceCheckForTest();
     auto second_dog_ = guard_dog_->createWatchDog(1);
     guard_dog_->forceCheckForTest();
-    time_point_ += std::chrono::milliseconds(501);
-    EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+    mock_time_ += 501;
   }
 
   NiceMock<Configuration::MockMain> config_kill_;
   NiceMock<Configuration::MockMain> config_multikill_;
   NiceMock<Stats::MockStore> fakestats_;
   MockSystemTimeSource time_source_;
-  std::chrono::system_clock::time_point time_point_;
+  std::atomic<unsigned int> mock_time_;
   std::unique_ptr<GuardDogImpl> guard_dog_;
   WatchDogSharedPtr unpet_dog_;
   WatchDogSharedPtr second_dog_;
@@ -108,7 +108,6 @@ TEST_F(GuardDogAlmostDeadTest, NearDeathTest) {
   // This ensures that if only one thread surpasses the multiple kill threshold
   // there is no death.  The positive case is covered in MultiKillDeathTest.
   InSequence s;
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
   GuardDogImpl gd(fakestats_, config_multikill_, time_source_);
   auto unpet_dog = gd.createWatchDog(0);
   auto pet_dog = gd.createWatchDog(1);
@@ -117,8 +116,7 @@ TEST_F(GuardDogAlmostDeadTest, NearDeathTest) {
   // only one is nonresponsive, so there should be no kill (single kill
   // threshold of 1s is not reached).
   for (int i = 0; i < 6; i++) {
-    time_point_ += std::chrono::milliseconds(100);
-    EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+    mock_time_ += 100;
     pet_dog->touch();
     gd.forceCheckForTest();
   }
@@ -126,31 +124,35 @@ TEST_F(GuardDogAlmostDeadTest, NearDeathTest) {
 
 class GuardDogMissTest : public testing::Test {
 protected:
-  GuardDogMissTest() : config_miss_(500, 1000, 0, 0), config_mega_(1000, 500, 0, 0) {}
+  GuardDogMissTest() : config_miss_(500, 1000, 0, 0), config_mega_(1000, 500, 0, 0) {
+    ON_CALL(time_source_, currentSystemTime())
+        .WillByDefault(testing::Invoke([&]() {
+          return std::chrono::system_clock::time_point(std::chrono::milliseconds(mock_time_));
+        }));
+  }
 
   NiceMock<Configuration::MockMain> config_miss_;
   NiceMock<Configuration::MockMain> config_mega_;
   Stats::IsolatedStoreImpl stats_store_;
   MockSystemTimeSource time_source_;
-  std::chrono::system_clock::time_point time_point_;
+  std::atomic<unsigned int> mock_time_;
 };
 
 TEST_F(GuardDogMissTest, MissTest) {
   // This test checks the actual collected statistics after doing some timer
   // advances that should and shouldn't increment the counters.
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
   GuardDogImpl gd(stats_store_, config_miss_, time_source_);
   // We'd better start at 0:
   EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_miss").value());
   auto unpet_dog = gd.createWatchDog(0);
   // At 300ms we shouldn't have hit the timeout yet:
-  time_point_ += std::chrono::milliseconds(300);
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+  mock_time_ += 300;
+  // EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
   gd.forceCheckForTest();
   EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_miss").value());
   // This should push it past the 500ms limit:
-  time_point_ += std::chrono::milliseconds(250);
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+  mock_time_ += 250;
+  // EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
   gd.forceCheckForTest();
   EXPECT_EQ(1UL, stats_store_.counter("server.watchdog_miss").value());
   gd.stopWatching(unpet_dog);
@@ -160,19 +162,20 @@ TEST_F(GuardDogMissTest, MissTest) {
 TEST_F(GuardDogMissTest, MegaMissTest) {
   // This test checks the actual collected statistics after doing some timer
   // advances that should and shouldn't increment the counters.
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+  ON_CALL(time_source_, currentSystemTime())
+      .WillByDefault(testing::Invoke([&]() {
+        return std::chrono::system_clock::time_point(std::chrono::milliseconds(mock_time_));
+      }));
   GuardDogImpl gd(stats_store_, config_mega_, time_source_);
   auto unpet_dog = gd.createWatchDog(0);
   // We'd better start at 0:
   EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_mega_miss").value());
   // This shouldn't be enough to increment the stat:
-  time_point_ += std::chrono::milliseconds(499);
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+  mock_time_ += 499;
   gd.forceCheckForTest();
   EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_mega_miss").value());
   // Just 2ms more will make it greater than 500ms timeout:
-  time_point_ += std::chrono::milliseconds(2);
-  EXPECT_CALL(time_source_, currentSystemTime()).WillRepeatedly(testing::Return(time_point_));
+  mock_time_ += 2;
   gd.forceCheckForTest();
   EXPECT_EQ(1UL, stats_store_.counter("server.watchdog_mega_miss").value());
   gd.stopWatching(unpet_dog);

From 29bd95beaadcd4e4af5a7c600df536a845eb658b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20No=C3=A9?= <dpn@google.com>
Date: Fri, 7 Apr 2017 20:30:44 -0400
Subject: [PATCH 03/12] Build: Fix unit test cert generation (#704) (#723)

The temporary SSL certificates used for test/common/ssl unit tests
were being generated by a Bazel build rule. However, this meant that
unless they were missing they were not regenerated.  This wasn't really
appropriate for the unit test use case since the unit test verifying
expiration had expectations about the SSL certificate being generated
less than one day before the test ran. Ultimately this resulted in Issue
704 where Bazel builds failed the test if it had been more than 24 hours
since the last clean.

To fix this the ephemeral certificates used by the unit tests are
generated immediately prior to running the test and placed into the
Bazel temp directory to be consumed by the test.
---
 test/certs/BUILD                              | 15 ----------
 test/common/ssl/BUILD                         | 10 +++++--
 test/common/ssl/connection_impl_test.cc       | 28 +++++++++----------
 test/common/ssl/context_impl_test.cc          | 14 +++++-----
 .../ssl/gen_unittest_certs.sh}                |  2 +-
 test/run_envoy_tests.sh                       |  4 ++-
 test/test_common/environment.cc               |  5 ++--
 test/test_common/environment.h                | 16 +----------
 8 files changed, 36 insertions(+), 58 deletions(-)
 delete mode 100644 test/certs/BUILD
 rename test/{certs/gen_test_certs.sh => common/ssl/gen_unittest_certs.sh} (97%)

diff --git a/test/certs/BUILD b/test/certs/BUILD
deleted file mode 100644
index 8971042728c8..000000000000
--- a/test/certs/BUILD
+++ /dev/null
@@ -1,15 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-genrule(
-    name = "test_certs",
-    outs = [
-        "unittestcert.csr",
-        "unittestcert.pem",
-        "unittestcert_expired.csr",
-        "unittestcert_expired.pem",
-        "unittestkey.pem",
-        "unittestkey_expired.pem",
-    ],
-    cmd = "$(location gen_test_certs.sh) $(@D)",
-    tools = ["gen_test_certs.sh"],
-)
diff --git a/test/common/ssl/BUILD b/test/common/ssl/BUILD
index 6e7182c65854..0c58dd4ec7fb 100644
--- a/test/common/ssl/BUILD
+++ b/test/common/ssl/BUILD
@@ -6,9 +6,12 @@ envoy_cc_test(
     name = "connection_impl_test",
     srcs = ["connection_impl_test.cc"],
     data = [
-        "//test/certs:test_certs",
         "//test/common/ssl/test_data:certs",
     ],
+    setup_cmds = [(
+        "gen_unittest_certs.sh",
+        [],
+    )],
     deps = [
         "//source/common/buffer:buffer_lib",
         "//source/common/event:dispatcher_includes",
@@ -32,9 +35,12 @@ envoy_cc_test(
     name = "context_impl_test",
     srcs = ["context_impl_test.cc"],
     data = [
-        "//test/certs:test_certs",
         "//test/common/ssl/test_data:certs",
     ],
+    setup_cmds = [(
+        "gen_unittest_certs.sh",
+        [],
+    )],
     deps = [
         "//source/common/json:json_loader_lib",
         "//source/common/ssl:context_config_lib",
diff --git a/test/common/ssl/connection_impl_test.cc b/test/common/ssl/connection_impl_test.cc
index 6261ecf93bb5..4f5fa1a4230d 100644
--- a/test/common/ssl/connection_impl_test.cc
+++ b/test/common/ssl/connection_impl_test.cc
@@ -79,8 +79,8 @@ TEST(SslConnectionImplTest, ClientAuth) {
 
   std::string server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca_with_uri_san.crt"
   }
   )EOF";
@@ -98,8 +98,8 @@ TEST(SslConnectionImplTest, ClientAuth) {
 
   server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca_with_dns_san.crt"
   }
   )EOF";
@@ -116,8 +116,8 @@ TEST(SslConnectionImplTest, ClientAuth) {
 
   server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": ""
   }
   )EOF";
@@ -134,8 +134,8 @@ TEST(SslConnectionImplTest, ClientAuth) {
 
   server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca.crt"
   }
   )EOF";
@@ -150,8 +150,8 @@ TEST(SslConnectionImplTest, ClientAuthBadVerification) {
 
   std::string server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca.crt",
     "verify_certificate_hash": "7B:0C:3F:0D:97:0E:FC:16:70:11:7A:0C:35:75:54:6B:17:AB:CF:20:D8:AA:A0:ED:87:08:0F:FB:60:4C:40:77"
   }
@@ -207,8 +207,8 @@ TEST(SslConnectionImplTest, SslError) {
 
   std::string server_ctx_json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca.crt",
     "verify_certificate_hash": "7B:0C:3F:0D:97:0E:FC:16:70:11:7A:0C:35:75:54:6B:17:AB:CF:20:D8:AA:A0:ED:87:08:0F:FB:60:4C:40:77"
   }
@@ -264,8 +264,8 @@ class SslReadBufferLimitTest : public testing::Test {
 
     std::string server_ctx_json = R"EOF(
     {
-      "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-      "private_key_file": "{{ test_certs }}/unittestkey.pem",
+      "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+      "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
       "ca_cert_file": "test/common/ssl/test_data/ca.crt"
     }
     )EOF";
diff --git a/test/common/ssl/context_impl_test.cc b/test/common/ssl/context_impl_test.cc
index 0b5aa77a901a..7feed8a00e66 100644
--- a/test/common/ssl/context_impl_test.cc
+++ b/test/common/ssl/context_impl_test.cc
@@ -73,8 +73,8 @@ TEST(SslContextImplTest, TestCipherSuites) {
 TEST(SslContextImplTest, TestExpiringCert) {
   std::string json = R"EOF(
   {
-      "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-      "private_key_file": "{{ test_certs }}/unittestkey.pem"
+      "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+      "private_key_file": "{{ test_tmpdir }}/unittestkey.pem"
   }
   )EOF";
 
@@ -96,8 +96,8 @@ TEST(SslContextImplTest, TestExpiringCert) {
 TEST(SslContextImplTest, TestExpiredCert) {
   std::string json = R"EOF(
   {
-      "cert_chain_file": "{{ test_certs }}/unittestcert_expired.pem",
-      "private_key_file": "{{ test_certs }}/unittestkey_expired.pem"
+      "cert_chain_file": "{{ test_tmpdir }}/unittestcert_expired.pem",
+      "private_key_file": "{{ test_tmpdir }}/unittestkey_expired.pem"
   }
   )EOF";
 
@@ -113,8 +113,8 @@ TEST(SslContextImplTest, TestExpiredCert) {
 TEST(SslContextImplTest, TestGetCertInformation) {
   std::string json = R"EOF(
   {
-    "cert_chain_file": "{{ test_certs }}/unittestcert.pem",
-    "private_key_file": "{{ test_certs }}/unittestkey.pem",
+    "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
+    "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
     "ca_cert_file": "test/common/ssl/test_data/ca.crt"
   }
   )EOF";
@@ -136,7 +136,7 @@ TEST(SslContextImplTest, TestGetCertInformation) {
       "Certificate Path: test/common/ssl/test_data/ca.crt, Serial Number: F0DE921A0515EB45, "
       "Days until Expiration: ");
   std::string cert_chain_partial_output(
-      TestEnvironment::substitute("Certificate Path: {{ test_certs }}/unittestcert.pem"));
+      TestEnvironment::substitute("Certificate Path: {{ test_tmpdir }}/unittestcert.pem"));
 
   EXPECT_TRUE(context->getCaCertInformation().find(ca_cert_partial_output) != std::string::npos);
   EXPECT_TRUE(context->getCertChainInformation().find(cert_chain_partial_output) !=
diff --git a/test/certs/gen_test_certs.sh b/test/common/ssl/gen_unittest_certs.sh
similarity index 97%
rename from test/certs/gen_test_certs.sh
rename to test/common/ssl/gen_unittest_certs.sh
index af9e42f65923..b8c9f7949a8c 100755
--- a/test/certs/gen_test_certs.sh
+++ b/test/common/ssl/gen_unittest_certs.sh
@@ -4,7 +4,7 @@
 
 set -e
 
-TEST_CERT_DIR=$1
+TEST_CERT_DIR=$TEST_TMPDIR
 
 mkdir -p $TEST_CERT_DIR
 openssl genrsa -out $TEST_CERT_DIR/unittestkey.pem 1024
diff --git a/test/run_envoy_tests.sh b/test/run_envoy_tests.sh
index 13453a46e4a5..d18266725e21 100755
--- a/test/run_envoy_tests.sh
+++ b/test/run_envoy_tests.sh
@@ -28,7 +28,9 @@ echo "TEST_SRCDIR=$TEST_SRCDIR"
 
 mkdir -p $TEST_TMPDIR
 
-$SOURCE_DIR/test/certs/gen_test_certs.sh $TEST_SRCDIR/test/certs
+# This places the unittest SSL certificates into $TEST_TMPDIR where the unit
+# tests in test/common/ssl expect to consume them
+$SOURCE_DIR/test/common/ssl/gen_unittest_certs.sh
 
 # Some hacks for the config file template substitution. These go away in the Bazel build.
 CONFIG_IN_DIR="$SOURCE_DIR"/test/config/integration
diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc
index 9cef2217be11..735cf15473f8 100644
--- a/test/test_common/environment.cc
+++ b/test/test_common/environment.cc
@@ -43,9 +43,8 @@ const std::string TestEnvironment::unixDomainSocketDirectory() {
 }
 
 std::string TestEnvironment::substitute(const std::string str) {
-  // TODO(htuch): Add support for {{ test_tmpdir }} etc. as needed for tests.
-  const std::regex test_cert_regex("\\{\\{ test_certs \\}\\}");
-  return std::regex_replace(str, test_cert_regex, TestEnvironment::runfilesPath("test/certs"));
+  const std::regex test_cert_regex("\\{\\{ test_tmpdir \\}\\}");
+  return std::regex_replace(str, test_cert_regex, TestEnvironment::temporaryDirectory());
 }
 
 std::string TestEnvironment::temporaryFileSubstitutePorts(const std::string& path,
diff --git a/test/test_common/environment.h b/test/test_common/environment.h
index f847cdcb9819..b958fa5f996b 100644
--- a/test/test_common/environment.h
+++ b/test/test_common/environment.h
@@ -36,20 +36,6 @@ class TestEnvironment {
     return runfilesDirectory() + "/" + path;
   }
 
-  /**
-   * Obtain pregenerated test ssl key/certificate directory.
-   * @return std::string& with the path to the pregenerated test ssl key/certificate
-   *         directory.
-   */
-  static std::string certsDirectory() { return runfilesPath("test/certs"); }
-
-  /**
-   * Prefix a given path with the pregenerated test ssl key/certificate directory.
-   * @param path path suffix.
-   * @return std::string path qualified with the pregenerated test ssl key/certificate directory.
-   */
-  static std::string certsPath(const std::string& path) { return certsDirectory() + "/" + path; }
-
   /**
    * Obtain Unix Domain Socket temporary directory.
    * @return std::string& with the path to the Unix Domain Socket temporary directory.
@@ -67,7 +53,7 @@ class TestEnvironment {
 
   /**
    * String environment path substitution.
-   * @param str string with template patterns including {{ test_certs }}.
+   * @param str string with template patterns including {{ test_tmpdir }}.
    * @return std::string with patterns replaced with environment values.
    */
   static std::string substitute(const std::string str);

From 2e6b853e2c4b7376aca7a76af5b61d557d78e627 Mon Sep 17 00:00:00 2001
From: htuch <htuch@users.noreply.github.com>
Date: Fri, 7 Apr 2017 20:43:34 -0400
Subject: [PATCH 04/12] ci: build external deps with BUILD_DISTINCT=1. (#722)

This is needed for #716, it's going to temporarily increase the CI image size, but this will go away
when we remove the need for cmake.
---
 ci/build_container/build_container.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ci/build_container/build_container.sh b/ci/build_container/build_container.sh
index 003fe7957bc4..c16baacf83c4 100755
--- a/ci/build_container/build_container.sh
+++ b/ci/build_container/build_container.sh
@@ -33,4 +33,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

From e598e36c061f8d2e8c99b2ef6ced206503f613d6 Mon Sep 17 00:00:00 2001
From: Oliver Liu <yonggangl@google.com>
Date: Sun, 9 Apr 2017 20:46:24 -0700
Subject: [PATCH 05/12] Add verify_subject_alt_name testing to integration
 tests (#725)

---
 test/common/ssl/connection_impl_test.cc      |  8 +-
 test/common/ssl/context_impl_test.cc         |  3 +-
 test/config/integration/certs/README.md      | 19 +++++
 test/config/integration/certs/certs.sh       | 13 +--
 test/config/integration/certs/servercert.cfg | 23 +++++
 test/config/integration/certs/servercert.pem | 28 +++---
 test/integration/ssl_integration_test.cc     | 90 +++++++++++++++-----
 test/integration/ssl_integration_test.h      |  8 +-
 8 files changed, 140 insertions(+), 52 deletions(-)
 create mode 100644 test/config/integration/certs/README.md
 create mode 100644 test/config/integration/certs/servercert.cfg

diff --git a/test/common/ssl/connection_impl_test.cc b/test/common/ssl/connection_impl_test.cc
index 4f5fa1a4230d..e6ff5c66fa86 100644
--- a/test/common/ssl/connection_impl_test.cc
+++ b/test/common/ssl/connection_impl_test.cc
@@ -1,17 +1,18 @@
+#include "common/ssl/connection_impl.h"
+
 #include "common/buffer/buffer_impl.h"
 #include "common/event/dispatcher_impl.h"
 #include "common/json/json_loader.h"
 #include "common/network/listen_socket_impl.h"
 #include "common/network/utility.h"
-#include "common/ssl/connection_impl.h"
 #include "common/ssl/context_config_impl.h"
 #include "common/ssl/context_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/network/mocks.h"
 #include "test/mocks/runtime/mocks.h"
-#include "test/mocks/stats/mocks.h"
 #include "test/mocks/server/mocks.h"
+#include "test/mocks/stats/mocks.h"
 #include "test/test_common/environment.h"
 
 using testing::_;
@@ -81,7 +82,8 @@ TEST(SslConnectionImplTest, ClientAuth) {
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_with_uri_san.crt"
+    "ca_cert_file": "test/common/ssl/test_data/ca_with_uri_san.crt",
+    "verify_subject_alt_name": [ "server1.example.com" ]
   }
   )EOF";
 
diff --git a/test/common/ssl/context_impl_test.cc b/test/common/ssl/context_impl_test.cc
index 7feed8a00e66..1e29c6465579 100644
--- a/test/common/ssl/context_impl_test.cc
+++ b/test/common/ssl/context_impl_test.cc
@@ -1,6 +1,7 @@
+#include "common/ssl/context_impl.h"
+
 #include "common/json/json_loader.h"
 #include "common/ssl/context_config_impl.h"
-#include "common/ssl/context_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/runtime/mocks.h"
diff --git a/test/config/integration/certs/README.md b/test/config/integration/certs/README.md
new file mode 100644
index 000000000000..e3e26d9cdf68
--- /dev/null
+++ b/test/config/integration/certs/README.md
@@ -0,0 +1,19 @@
+# What are the identities, certificates and keys
+There are 5 identities:
+- **CA**: Certificate Authority for **Client** and **Server**. It has the
+  self-signed certificate *cacert.pem*. *cakey.pem* is its private key.
+- **Client**: It has the certificate *clientcert.pem*, signed by the **CA**.
+  *clientkey.pem* is its private key.
+- **Server**: It has the certificate *servercert.pem*, which is signed by the
+  **CA** using the config *servercert.cfg*. *serverkey.pem* is its private key.
+- **Upsteam CA**: Certificate Authority for **Upstream**. It has the self-signed
+  certificate *upstreamcacert.pem*. *upstreamcakey.pem* is its private key.
+- **Upstream**: It has the certificate *upstreamcert.pem*, which is signed by
+  the **Upstream CA** using the config *upstreamcert.cfg*. *upstreamkey.pem* is
+  its private key.
+
+# How to update certificates
+**certs.sh** has the commands to generate all files. Running certs.sh directly
+will cause all files to be regenerated. So if you want to regenerate a
+particular file, please copy the corresponding commands from certs.sh and
+execute them in command line.
diff --git a/test/config/integration/certs/certs.sh b/test/config/integration/certs/certs.sh
index a7052c821773..c8592a4e7903 100755
--- a/test/config/integration/certs/certs.sh
+++ b/test/config/integration/certs/certs.sh
@@ -13,18 +13,9 @@ EOF
 openssl x509 -req -days 730 -in cacert.csr -sha256 -signkey cakey.pem -out cacert.pem
 
 openssl genrsa -out serverkey.pem 1024
-openssl req -new -key serverkey.pem -out servercert.csr -sha256  <<EOF
-US
-California
-San Francisco
-Lyft
-Test
-Test Server
-test@lyft.com
+openssl req -new -key serverkey.pem -out servercert.csr -config servercert.cfg -batch -sha256
 
-
-EOF
-openssl x509 -req -days 730 -in servercert.csr -sha256 -CA cacert.pem -CAkey cakey.pem -CAcreateserial -out servercert.pem
+openssl x509 -req -days 730 -in servercert.csr -sha256 -CA cacert.pem -CAkey cakey.pem -CAcreateserial -out servercert.pem -extensions v3_req -extfile servercert.cfg
 
 openssl genrsa -out clientkey.pem 1024
 openssl req -new -key clientkey.pem -out clientcert.csr -sha256  <<EOF
diff --git a/test/config/integration/certs/servercert.cfg b/test/config/integration/certs/servercert.cfg
new file mode 100644
index 000000000000..4c0fc15e2664
--- /dev/null
+++ b/test/config/integration/certs/servercert.cfg
@@ -0,0 +1,23 @@
+[req]
+distinguished_name = req_distinguished_name
+req_extensions = v3_req
+
+[req_distinguished_name]
+countryName = US
+countryName_default = US
+stateOrProvinceName = CA
+stateOrProvinceName_default = CA
+localityName = San Francisco
+localityName_default = San Francisco
+organizationalUnitName = Lyft
+organizationalUnitName_default = Lyft
+commonName = Test Server
+commonName_max  = 64
+
+[v3_req]
+basicConstraints = CA:FALSE
+keyUsage = nonRepudiation, digitalSignature, keyEncipherment
+subjectAltName = @alt_names
+
+[alt_names]
+URI.1 = istio:account_a.namespace_foo.cluster.local
diff --git a/test/config/integration/certs/servercert.pem b/test/config/integration/certs/servercert.pem
index 509a9cac0cd0..733edc5e3c58 100644
--- a/test/config/integration/certs/servercert.pem
+++ b/test/config/integration/certs/servercert.pem
@@ -1,16 +1,16 @@
 -----BEGIN CERTIFICATE-----
-MIICjTCCAfYCCQCGksmf8BshZDANBgkqhkiG9w0BAQsFADCBiDELMAkGA1UEBhMC
-VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28x
-DTALBgNVBAoMBEx5ZnQxDTALBgNVBAsMBFRlc3QxEDAOBgNVBAMMB1Rlc3QgQ0Ex
-HDAaBgkqhkiG9w0BCQEWDXRlc3RAbHlmdC5jb20wHhcNMTYwMTA5MjAwNjA0WhcN
-MTgwMTA4MjAwNjA0WjCBjDELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3Ju
-aWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxDTALBgNV
-BAsMBFRlc3QxFDASBgNVBAMMC1Rlc3QgU2VydmVyMRwwGgYJKoZIhvcNAQkBFg10
-ZXN0QGx5ZnQuY29tMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCqtS9bbVbo
-4ZpO1uSBCDortIibXKByL1fgl7s2uJc77+vzJnqC9uLFYygU1Z198X6jaAjc/vUk
-LFVXZhOU8607Zex8X+CdZBjQqsN90X2Ste1wqJ7G5SAGhptd/nOfb1IdGa6YtwPT
-lVitnMTfRgG4fh+3DA51UulCGTfJXCaC3wIDAQABMA0GCSqGSIb3DQEBCwUAA4GB
-AD/GeVdxA5uNOX1x8DSo1GrdhxEqDEWpmGms0jFoRStgO2PsWNhBoXo/3yPWmsam
-GovtzLF4WapdtSTdn7ku91rx0BplNGOs0uuipnEtRoC7Eo31xaay4LppWTwtVZBA
-LaRR1p0mlCtvqI9dz25Uhl1UlXdvq+lHLxkAFp49CLYf
+MIICmjCCAgOgAwIBAgIJALd7PpOmDaMoMA0GCSqGSIb3DQEBCwUAMIGIMQswCQYD
+VQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5j
+aXNjbzENMAsGA1UECgwETHlmdDENMAsGA1UECwwEVGVzdDEQMA4GA1UEAwwHVGVz
+dCBDQTEcMBoGCSqGSIb3DQEJARYNdGVzdEBseWZ0LmNvbTAeFw0xNzA0MDgwNTQ3
+MTBaFw0xOTA0MDgwNTQ3MTBaMEExCzAJBgNVBAYTAlVTMQswCQYDVQQIEwJDQTEW
+MBQGA1UEBxMNU2FuIEZyYW5jaXNjbzENMAsGA1UECxMETHlmdDCBnzANBgkqhkiG
+9w0BAQEFAAOBjQAwgYkCgYEAqrUvW21W6OGaTtbkgQg6K7SIm1ygci9X4Je7NriX
+O+/r8yZ6gvbixWMoFNWdffF+o2gI3P71JCxVV2YTlPOtO2XsfF/gnWQY0KrDfdF9
+krXtcKiexuUgBoabXf5zn29SHRmumLcD05VYrZzE30YBuH4ftwwOdVLpQhk3yVwm
+gt8CAwEAAaNSMFAwCQYDVR0TBAIwADALBgNVHQ8EBAMCBeAwNgYDVR0RBC8wLYYr
+aXN0aW86YWNjb3VudF9hLm5hbWVzcGFjZV9mb28uY2x1c3Rlci5sb2NhbDANBgkq
+hkiG9w0BAQsFAAOBgQBGUZT++ypIOByf9jOYPmoegG1k+nybIdjSHlqWXdO+T5GZ
+Ew5qEfwDH9GTSyxtlFeU32PueJuSwg/7OduL7n78cqFTMS2gHkwAG6B+LQlDo2ou
++qWZM3HvLTIdVF8/9ez0JpCsAYBWy5MUXy5E1wKBLTuPnhhLllepdrt+V+E2Tw==
 -----END CERTIFICATE-----
diff --git a/test/integration/ssl_integration_test.cc b/test/integration/ssl_integration_test.cc
index 3ebaa33d6268..4649a1fdaa8e 100644
--- a/test/integration/ssl_integration_test.cc
+++ b/test/integration/ssl_integration_test.cc
@@ -1,5 +1,6 @@
-#include "integration.h"
 #include "ssl_integration_test.h"
+
+#include "integration.h"
 #include "utility.h"
 
 #include "common/event/dispatcher_impl.h"
@@ -14,8 +15,10 @@ namespace Ssl {
 std::unique_ptr<Runtime::Loader> SslIntegrationTest::runtime_;
 std::unique_ptr<ContextManager> SslIntegrationTest::context_manager_;
 ServerContextPtr SslIntegrationTest::upstream_ssl_ctx_;
+ClientContextPtr SslIntegrationTest::client_ssl_ctx_plain_;
 ClientContextPtr SslIntegrationTest::client_ssl_ctx_alpn_;
-ClientContextPtr SslIntegrationTest::client_ssl_ctx_no_alpn_;
+ClientContextPtr SslIntegrationTest::client_ssl_ctx_san_;
+ClientContextPtr SslIntegrationTest::client_ssl_ctx_alpn_san_;
 
 void SslIntegrationTest::SetUpTestCase() {
   context_manager_.reset(new ContextManagerImpl(*runtime_));
@@ -29,16 +32,20 @@ void SslIntegrationTest::SetUpTestCase() {
   test_server_ = MockRuntimeIntegrationTestServer::create(
       TestEnvironment::temporaryFileSubstitutePorts("server_ssl.json", port_map()));
   registerTestServerPorts({"http"});
-  client_ssl_ctx_alpn_ = createClientSslContext(true);
-  client_ssl_ctx_no_alpn_ = createClientSslContext(false);
+  client_ssl_ctx_plain_ = createClientSslContext(false, false);
+  client_ssl_ctx_alpn_ = createClientSslContext(true, false);
+  client_ssl_ctx_san_ = createClientSslContext(false, true);
+  client_ssl_ctx_alpn_san_ = createClientSslContext(true, true);
 }
 
 void SslIntegrationTest::TearDownTestCase() {
   test_server_.reset();
   fake_upstreams_.clear();
   upstream_ssl_ctx_.reset();
+  client_ssl_ctx_plain_.reset();
   client_ssl_ctx_alpn_.reset();
-  client_ssl_ctx_no_alpn_.reset();
+  client_ssl_ctx_san_.reset();
+  client_ssl_ctx_alpn_san_.reset();
   context_manager_.reset();
 }
 
@@ -56,8 +63,8 @@ ServerContextPtr SslIntegrationTest::createUpstreamSslContext() {
   return context_manager_->createSslServerContext(*upstream_stats_store, cfg);
 }
 
-ClientContextPtr SslIntegrationTest::createClientSslContext(bool alpn) {
-  std::string json_no_alpn = R"EOF(
+ClientContextPtr SslIntegrationTest::createClientSslContext(bool alpn, bool san) {
+  std::string json_plain = R"EOF(
 {
   "ca_cert_file": "test/config/integration/certs/cacert.pem",
   "cert_chain_file": "test/config/integration/certs/clientcert.pem",
@@ -74,15 +81,46 @@ ClientContextPtr SslIntegrationTest::createClientSslContext(bool alpn) {
 }
 )EOF";
 
-  Json::ObjectPtr loader = Json::Factory::LoadFromString(alpn ? json_alpn : json_no_alpn);
+  std::string json_san = R"EOF(
+{
+  "ca_cert_file": "test/config/integration/certs/cacert.pem",
+  "cert_chain_file": "test/config/integration/certs/clientcert.pem",
+  "private_key_file": "test/config/integration/certs/clientkey.pem",
+  "verify_subject_alt_name": [ "istio:account_a.namespace_foo.cluster.local" ]
+}
+)EOF";
+
+  std::string json_alpn_san = R"EOF(
+{
+  "ca_cert_file": "test/config/integration/certs/cacert.pem",
+  "cert_chain_file": "test/config/integration/certs/clientcert.pem",
+  "private_key_file": "test/config/integration/certs/clientkey.pem",
+  "alpn_protocols": "h2,http/1.1",
+  "verify_subject_alt_name": [ "istio:account_a.namespace_foo.cluster.local" ]
+}
+)EOF";
+
+  std::string target;
+  if (alpn) {
+    target = san ? json_alpn_san : json_alpn;
+  } else {
+    target = san ? json_san : json_plain;
+  }
+  Json::ObjectPtr loader = Json::Factory::LoadFromString(target);
   ContextConfigImpl cfg(*loader);
   return context_manager_->createSslClientContext(test_server_->store(), cfg);
 }
 
-Network::ClientConnectionPtr SslIntegrationTest::makeSslClientConnection(bool alpn) {
-  return dispatcher_->createSslClientConnection(
-      alpn ? *client_ssl_ctx_alpn_ : *client_ssl_ctx_no_alpn_,
-      Network::Utility::resolveUrl("tcp://127.0.0.1:" + std::to_string(lookupPort("http"))));
+Network::ClientConnectionPtr SslIntegrationTest::makeSslClientConnection(bool alpn, bool san) {
+  if (alpn) {
+    return dispatcher_->createSslClientConnection(
+        san ? *client_ssl_ctx_alpn_san_ : *client_ssl_ctx_alpn_,
+        Network::Utility::resolveUrl("tcp://127.0.0.1:" + std::to_string(lookupPort("http"))));
+  } else {
+    return dispatcher_->createSslClientConnection(
+        san ? *client_ssl_ctx_san_ : *client_ssl_ctx_plain_,
+        Network::Utility::resolveUrl("tcp://127.0.0.1:" + std::to_string(lookupPort("http"))));
+  }
 }
 
 void SslIntegrationTest::checkStats() {
@@ -92,44 +130,56 @@ void SslIntegrationTest::checkStats() {
 }
 
 TEST_F(SslIntegrationTest, RouterRequestAndResponseWithGiantBodyBuffer) {
-  testRouterRequestAndResponseWithBody(makeSslClientConnection(false),
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(false, false),
                                        Http::CodecClient::Type::HTTP1, 16 * 1024 * 1024,
                                        16 * 1024 * 1024, false);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBuffer) {
-  testRouterRequestAndResponseWithBody(makeSslClientConnection(false),
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(false, false),
                                        Http::CodecClient::Type::HTTP1, 1024, 512, false);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2) {
-  testRouterRequestAndResponseWithBody(makeSslClientConnection(true),
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(true, false),
+                                       Http::CodecClient::Type::HTTP2, 1024, 512, false);
+  checkStats();
+}
+
+TEST_F(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVierfySAN) {
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(false, true),
+                                       Http::CodecClient::Type::HTTP1, 1024, 512, false);
+  checkStats();
+}
+
+TEST_F(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2VerifySAN) {
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(true, true),
                                        Http::CodecClient::Type::HTTP2, 1024, 512, false);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterHeaderOnlyRequestAndResponse) {
-  testRouterHeaderOnlyRequestAndResponse(makeSslClientConnection(false),
+  testRouterHeaderOnlyRequestAndResponse(makeSslClientConnection(false, false),
                                          Http::CodecClient::Type::HTTP1);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterUpstreamDisconnectBeforeResponseComplete) {
-  testRouterUpstreamDisconnectBeforeResponseComplete(makeSslClientConnection(false),
+  testRouterUpstreamDisconnectBeforeResponseComplete(makeSslClientConnection(false, false),
                                                      Http::CodecClient::Type::HTTP1);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterDownstreamDisconnectBeforeRequestComplete) {
-  testRouterDownstreamDisconnectBeforeRequestComplete(makeSslClientConnection(false),
+  testRouterDownstreamDisconnectBeforeRequestComplete(makeSslClientConnection(false, false),
                                                       Http::CodecClient::Type::HTTP1);
   checkStats();
 }
 
 TEST_F(SslIntegrationTest, RouterDownstreamDisconnectBeforeResponseComplete) {
-  testRouterDownstreamDisconnectBeforeResponseComplete(makeSslClientConnection(false),
+  testRouterDownstreamDisconnectBeforeResponseComplete(makeSslClientConnection(false, false),
                                                        Http::CodecClient::Type::HTTP1);
   checkStats();
 }
@@ -148,7 +198,7 @@ TEST_F(SslIntegrationTest, AltAlpn) {
       dynamic_cast<MockRuntimeIntegrationTestServer*>(test_server_.get());
   ON_CALL(server->runtime_->snapshot_, featureEnabled("ssl.alt_alpn", 0))
       .WillByDefault(Return(true));
-  testRouterRequestAndResponseWithBody(makeSslClientConnection(true),
+  testRouterRequestAndResponseWithBody(makeSslClientConnection(true, false),
                                        Http::CodecClient::Type::HTTP1, 1024, 512, false);
   checkStats();
 }
diff --git a/test/integration/ssl_integration_test.h b/test/integration/ssl_integration_test.h
index 3fc39c97707d..96b0f4b09c16 100644
--- a/test/integration/ssl_integration_test.h
+++ b/test/integration/ssl_integration_test.h
@@ -42,17 +42,19 @@ class SslIntegrationTest : public BaseIntegrationTest, public testing::Test {
    */
   static void TearDownTestCase();
 
-  Network::ClientConnectionPtr makeSslClientConnection(bool alpn);
+  Network::ClientConnectionPtr makeSslClientConnection(bool alpn, bool san);
   static ServerContextPtr createUpstreamSslContext();
-  static ClientContextPtr createClientSslContext(bool alpn);
+  static ClientContextPtr createClientSslContext(bool alpn, bool san);
   void checkStats();
 
 private:
   static std::unique_ptr<Runtime::Loader> runtime_;
   static std::unique_ptr<ContextManager> context_manager_;
   static ServerContextPtr upstream_ssl_ctx_;
+  static ClientContextPtr client_ssl_ctx_plain_;
   static ClientContextPtr client_ssl_ctx_alpn_;
-  static ClientContextPtr client_ssl_ctx_no_alpn_;
+  static ClientContextPtr client_ssl_ctx_san_;
+  static ClientContextPtr client_ssl_ctx_alpn_san_;
 };
 
 } // Ssl

From 4eeaf640232f237f144792540b92d327abb22e64 Mon Sep 17 00:00:00 2001
From: Shriram Rajagopalan <rshriram@users.noreply.github.com>
Date: Mon, 10 Apr 2017 12:51:49 -0400
Subject: [PATCH 06/12] Refactor Lightstep code (#724)

---
 source/common/CMakeLists.txt                  |   1 +
 source/common/tracing/BUILD                   |  10 +-
 source/common/tracing/http_tracer_impl.cc     | 157 ----------
 source/common/tracing/http_tracer_impl.h      |  93 ------
 .../common/tracing/lightstep_tracer_impl.cc   | 164 ++++++++++
 source/common/tracing/lightstep_tracer_impl.h | 107 +++++++
 source/server/configuration_impl.cc           |   1 +
 test/CMakeLists.txt                           |   1 +
 test/common/tracing/BUILD                     |   5 +-
 test/common/tracing/http_tracer_impl_test.cc  | 257 +---------------
 .../tracing/lightstep_tracer_impl_test.cc     | 284 ++++++++++++++++++
 11 files changed, 571 insertions(+), 509 deletions(-)
 create mode 100644 source/common/tracing/lightstep_tracer_impl.cc
 create mode 100644 source/common/tracing/lightstep_tracer_impl.h
 create mode 100644 test/common/tracing/lightstep_tracer_impl_test.cc

diff --git a/source/common/CMakeLists.txt b/source/common/CMakeLists.txt
index ce9ea91c2e29..d61d45750d92 100644
--- a/source/common/CMakeLists.txt
+++ b/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
diff --git a/source/common/tracing/BUILD b/source/common/tracing/BUILD
index fbe1499c2652..6a2b10896008 100644
--- a/source/common/tracing/BUILD
+++ b/source/common/tracing/BUILD
@@ -4,8 +4,14 @@ load("//bazel:envoy_build_system.bzl", "envoy_cc_library")
 
 envoy_cc_library(
     name = "http_tracer_lib",
-    srcs = ["http_tracer_impl.cc"],
-    hdrs = ["http_tracer_impl.h"],
+    srcs = [
+        "http_tracer_impl.cc",
+        "lightstep_tracer_impl.cc",
+    ],
+    hdrs = [
+        "http_tracer_impl.h",
+        "lightstep_tracer_impl.h",
+    ],
     external_deps = ["lightstep"],
     deps = [
         "//include/envoy/local_info:local_info_interface",
diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc
index 82f94acba919..e526426329fd 100644
--- a/source/common/tracing/http_tracer_impl.cc
+++ b/source/common/tracing/http_tracer_impl.cc
@@ -1,15 +1,12 @@
 #include "common/tracing/http_tracer_impl.h"
 
 #include "common/common/assert.h"
-#include "common/common/base64.h"
 #include "common/common/macros.h"
 #include "common/common/utility.h"
-#include "common/grpc/common.h"
 #include "common/http/access_log/access_log_formatter.h"
 #include "common/http/codes.h"
 #include "common/http/headers.h"
 #include "common/http/header_map_impl.h"
-#include "common/http/message_impl.h"
 #include "common/http/utility.h"
 #include "common/runtime/uuid_util.h"
 
@@ -175,158 +172,4 @@ SpanPtr HttpTracerImpl::startSpan(const Config& config, Http::HeaderMap& request
   return active_span;
 }
 
-LightStepSpan::LightStepSpan(lightstep::Span& span) : span_(span) {}
-
-void LightStepSpan::finishSpan() { span_.Finish(); }
-
-void LightStepSpan::setTag(const std::string& name, const std::string& value) {
-  span_.SetTag(name, value);
-}
-
-LightStepRecorder::LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver,
-                                     Event::Dispatcher& dispatcher)
-    : builder_(tracer), driver_(driver) {
-  flush_timer_ = dispatcher.createTimer([this]() -> void {
-    driver_.tracerStats().timer_flushed_.inc();
-    flushSpans();
-    enableTimer();
-  });
-
-  enableTimer();
-}
-
-void LightStepRecorder::RecordSpan(lightstep::collector::Span&& span) {
-  builder_.addSpan(std::move(span));
-
-  uint64_t min_flush_spans =
-      driver_.runtime().snapshot().getInteger("tracing.lightstep.min_flush_spans", 5U);
-  if (builder_.pendingSpans() == min_flush_spans) {
-    flushSpans();
-  }
-}
-
-bool LightStepRecorder::FlushWithTimeout(lightstep::Duration) {
-  // Note: We don't expect this to be called, since the Tracer
-  // reference is private to its LightStepSink.
-  return true;
-}
-
-std::unique_ptr<lightstep::Recorder>
-LightStepRecorder::NewInstance(LightStepDriver& driver, Event::Dispatcher& dispatcher,
-                               const lightstep::TracerImpl& tracer) {
-  return std::unique_ptr<lightstep::Recorder>(new LightStepRecorder(tracer, driver, dispatcher));
-}
-
-void LightStepRecorder::enableTimer() {
-  uint64_t flush_interval =
-      driver_.runtime().snapshot().getInteger("tracing.lightstep.flush_interval_ms", 1000U);
-  flush_timer_->enableTimer(std::chrono::milliseconds(flush_interval));
-}
-
-void LightStepRecorder::flushSpans() {
-  if (builder_.pendingSpans() != 0) {
-    driver_.tracerStats().spans_sent_.add(builder_.pendingSpans());
-    lightstep::collector::ReportRequest request;
-    std::swap(request, builder_.pending());
-
-    Http::MessagePtr message = Grpc::Common::prepareHeaders(driver_.cluster()->name(),
-                                                            lightstep::CollectorServiceFullName(),
-                                                            lightstep::CollectorMethodName());
-
-    message->body() = Grpc::Common::serializeBody(std::move(request));
-
-    uint64_t timeout =
-        driver_.runtime().snapshot().getInteger("tracing.lightstep.request_timeout", 5000U);
-    driver_.clusterManager()
-        .httpAsyncClientForCluster(driver_.cluster()->name())
-        .send(std::move(message), *this, std::chrono::milliseconds(timeout));
-  }
-}
-
-LightStepDriver::TlsLightStepTracer::TlsLightStepTracer(lightstep::Tracer tracer,
-                                                        LightStepDriver& driver)
-    : tracer_(tracer), driver_(driver) {}
-
-LightStepDriver::LightStepDriver(const Json::Object& config,
-                                 Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
-                                 ThreadLocal::Instance& tls, Runtime::Loader& runtime,
-                                 std::unique_ptr<lightstep::TracerOptions> options)
-    : cm_(cluster_manager),
-      tracer_stats_{LIGHTSTEP_TRACER_STATS(POOL_COUNTER_PREFIX(stats, "tracing.lightstep."))},
-      tls_(tls), runtime_(runtime), options_(std::move(options)), tls_slot_(tls.allocateSlot()) {
-  Upstream::ThreadLocalCluster* cluster = cm_.get(config.getString("collector_cluster"));
-  if (!cluster) {
-    throw EnvoyException(fmt::format("{} collector cluster is not defined on cluster manager level",
-                                     config.getString("collector_cluster")));
-  }
-  cluster_ = cluster->info();
-
-  if (!(cluster_->features() & Upstream::ClusterInfo::Features::HTTP2)) {
-    throw EnvoyException(
-        fmt::format("{} collector cluster must support http2 for gRPC calls", cluster_->name()));
-  }
-
-  tls_.set(tls_slot_,
-           [this](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
-             lightstep::Tracer tracer(lightstep::NewUserDefinedTransportLightStepTracer(
-                 *options_, std::bind(&LightStepRecorder::NewInstance, std::ref(*this),
-                                      std::ref(dispatcher), std::placeholders::_1)));
-
-             return ThreadLocal::ThreadLocalObjectSharedPtr{
-                 new TlsLightStepTracer(std::move(tracer), *this)};
-           });
-}
-
-SpanPtr LightStepDriver::startSpan(Http::HeaderMap& request_headers,
-                                   const std::string& operation_name, SystemTime start_time) {
-  lightstep::Tracer& tracer = tls_.getTyped<TlsLightStepTracer>(tls_slot_).tracer_;
-  LightStepSpanPtr active_span;
-
-  if (request_headers.OtSpanContext()) {
-    // Extract downstream context from HTTP carrier.
-    // This code is safe even if decode returns empty string or data is malformed.
-    std::string parent_context = Base64::decode(request_headers.OtSpanContext()->value().c_str());
-    lightstep::BinaryCarrier ctx;
-    ctx.ParseFromString(parent_context);
-
-    lightstep::SpanContext parent_span_ctx = tracer.Extract(
-        lightstep::CarrierFormat::LightStepBinaryCarrier, lightstep::ProtoReader(ctx));
-    lightstep::Span ls_span =
-        tracer.StartSpan(operation_name, {lightstep::ChildOf(parent_span_ctx),
-                                          lightstep::StartTimestamp(start_time)});
-    active_span.reset(new LightStepSpan(ls_span));
-  } else {
-    lightstep::Span ls_span =
-        tracer.StartSpan(operation_name, {lightstep::StartTimestamp(start_time)});
-    active_span.reset(new LightStepSpan(ls_span));
-  }
-
-  // Inject newly created span context into HTTP carrier.
-  lightstep::BinaryCarrier ctx;
-  tracer.Inject(active_span->context(), lightstep::CarrierFormat::LightStepBinaryCarrier,
-                lightstep::ProtoWriter(&ctx));
-  const std::string current_span_context = ctx.SerializeAsString();
-  request_headers.insertOtSpanContext().value(
-      Base64::encode(current_span_context.c_str(), current_span_context.length()));
-
-  return std::move(active_span);
-}
-
-void LightStepRecorder::onFailure(Http::AsyncClient::FailureReason) {
-  Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
-                           lightstep::CollectorMethodName(), false);
-}
-
-void LightStepRecorder::onSuccess(Http::MessagePtr&& msg) {
-  try {
-    Grpc::Common::validateResponse(*msg);
-
-    Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
-                             lightstep::CollectorMethodName(), true);
-  } catch (const Grpc::Exception& ex) {
-    Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
-                             lightstep::CollectorMethodName(), false);
-  }
-}
-
 } // Tracing
diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h
index 0bb1980861c7..a2615e67bcc5 100644
--- a/source/common/tracing/http_tracer_impl.h
+++ b/source/common/tracing/http_tracer_impl.h
@@ -9,19 +9,8 @@
 #include "common/http/header_map_impl.h"
 #include "common/json/json_loader.h"
 
-#include "lightstep/carrier.h"
-#include "lightstep/tracer.h"
-
 namespace Tracing {
 
-#define LIGHTSTEP_TRACER_STATS(COUNTER)                                                            \
-  COUNTER(spans_sent)                                                                              \
-  COUNTER(timer_flushed)
-
-struct LightstepTracerStats {
-  LIGHTSTEP_TRACER_STATS(GENERATE_COUNTER_STRUCT)
-};
-
 enum class Reason {
   NotTraceableRequestId,
   HealthCheck,
@@ -91,86 +80,4 @@ class HttpTracerImpl : public HttpTracer {
   const LocalInfo::LocalInfo& local_info_;
 };
 
-class LightStepSpan : public Span {
-public:
-  LightStepSpan(lightstep::Span& span);
-
-  // Tracing::Span
-  void finishSpan() override;
-  void setTag(const std::string& name, const std::string& value) override;
-
-  lightstep::SpanContext context() { return span_.context(); }
-
-private:
-  lightstep::Span span_;
-};
-
-typedef std::unique_ptr<LightStepSpan> LightStepSpanPtr;
-
-/**
- * LightStep (http://lightstep.com/) provides tracing capabilities, aggregation, visualization of
- * application trace data.
- *
- * LightStepSink is for flushing data to LightStep collectors.
- */
-class LightStepDriver : public Driver {
-public:
-  LightStepDriver(const Json::Object& config, Upstream::ClusterManager& cluster_manager,
-                  Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime,
-                  std::unique_ptr<lightstep::TracerOptions> options);
-
-  // Tracer::TracingDriver
-  SpanPtr startSpan(Http::HeaderMap& request_headers, const std::string& operation_name,
-                    SystemTime start_time) override;
-
-  Upstream::ClusterManager& clusterManager() { return cm_; }
-  Upstream::ClusterInfoConstSharedPtr cluster() { return cluster_; }
-  Runtime::Loader& runtime() { return runtime_; }
-  LightstepTracerStats& tracerStats() { return tracer_stats_; }
-
-private:
-  struct TlsLightStepTracer : ThreadLocal::ThreadLocalObject {
-    TlsLightStepTracer(lightstep::Tracer tracer, LightStepDriver& driver);
-
-    void shutdown() override {}
-
-    lightstep::Tracer tracer_;
-    LightStepDriver& driver_;
-  };
-
-  Upstream::ClusterManager& cm_;
-  Upstream::ClusterInfoConstSharedPtr cluster_;
-  LightstepTracerStats tracer_stats_;
-  ThreadLocal::Instance& tls_;
-  Runtime::Loader& runtime_;
-  std::unique_ptr<lightstep::TracerOptions> options_;
-  uint32_t tls_slot_;
-};
-
-class LightStepRecorder : public lightstep::Recorder, Http::AsyncClient::Callbacks {
-public:
-  LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver,
-                    Event::Dispatcher& dispatcher);
-
-  // lightstep::Recorder
-  void RecordSpan(lightstep::collector::Span&& span) override;
-  bool FlushWithTimeout(lightstep::Duration) override;
-
-  // Http::AsyncClient::Callbacks
-  void onSuccess(Http::MessagePtr&&) override;
-  void onFailure(Http::AsyncClient::FailureReason) override;
-
-  static std::unique_ptr<lightstep::Recorder> NewInstance(LightStepDriver& driver,
-                                                          Event::Dispatcher& dispatcher,
-                                                          const lightstep::TracerImpl& tracer);
-
-private:
-  void enableTimer();
-  void flushSpans();
-
-  lightstep::ReportBuilder builder_;
-  LightStepDriver& driver_;
-  Event::TimerPtr flush_timer_;
-};
-
 } // Tracing
diff --git a/source/common/tracing/lightstep_tracer_impl.cc b/source/common/tracing/lightstep_tracer_impl.cc
new file mode 100644
index 000000000000..082c3eafcc17
--- /dev/null
+++ b/source/common/tracing/lightstep_tracer_impl.cc
@@ -0,0 +1,164 @@
+#include "common/tracing/http_tracer_impl.h"
+#include "common/tracing/lightstep_tracer_impl.h"
+
+#include "common/common/base64.h"
+#include "common/grpc/common.h"
+#include "common/http/message_impl.h"
+
+namespace Tracing {
+
+LightStepSpan::LightStepSpan(lightstep::Span& span) : span_(span) {}
+
+void LightStepSpan::finishSpan() { span_.Finish(); }
+
+void LightStepSpan::setTag(const std::string& name, const std::string& value) {
+  span_.SetTag(name, value);
+}
+
+LightStepRecorder::LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver,
+                                     Event::Dispatcher& dispatcher)
+    : builder_(tracer), driver_(driver) {
+  flush_timer_ = dispatcher.createTimer([this]() -> void {
+    driver_.tracerStats().timer_flushed_.inc();
+    flushSpans();
+    enableTimer();
+  });
+
+  enableTimer();
+}
+
+void LightStepRecorder::RecordSpan(lightstep::collector::Span&& span) {
+  builder_.addSpan(std::move(span));
+
+  uint64_t min_flush_spans =
+      driver_.runtime().snapshot().getInteger("tracing.lightstep.min_flush_spans", 5U);
+  if (builder_.pendingSpans() == min_flush_spans) {
+    flushSpans();
+  }
+}
+
+bool LightStepRecorder::FlushWithTimeout(lightstep::Duration) {
+  // Note: We don't expect this to be called, since the Tracer
+  // reference is private to its LightStepSink.
+  return true;
+}
+
+std::unique_ptr<lightstep::Recorder>
+LightStepRecorder::NewInstance(LightStepDriver& driver, Event::Dispatcher& dispatcher,
+                               const lightstep::TracerImpl& tracer) {
+  return std::unique_ptr<lightstep::Recorder>(new LightStepRecorder(tracer, driver, dispatcher));
+}
+
+void LightStepRecorder::enableTimer() {
+  uint64_t flush_interval =
+      driver_.runtime().snapshot().getInteger("tracing.lightstep.flush_interval_ms", 1000U);
+  flush_timer_->enableTimer(std::chrono::milliseconds(flush_interval));
+}
+
+void LightStepRecorder::flushSpans() {
+  if (builder_.pendingSpans() != 0) {
+    driver_.tracerStats().spans_sent_.add(builder_.pendingSpans());
+    lightstep::collector::ReportRequest request;
+    std::swap(request, builder_.pending());
+
+    Http::MessagePtr message = Grpc::Common::prepareHeaders(driver_.cluster()->name(),
+                                                            lightstep::CollectorServiceFullName(),
+                                                            lightstep::CollectorMethodName());
+
+    message->body() = Grpc::Common::serializeBody(std::move(request));
+
+    uint64_t timeout =
+        driver_.runtime().snapshot().getInteger("tracing.lightstep.request_timeout", 5000U);
+    driver_.clusterManager()
+        .httpAsyncClientForCluster(driver_.cluster()->name())
+        .send(std::move(message), *this, std::chrono::milliseconds(timeout));
+  }
+}
+
+LightStepDriver::TlsLightStepTracer::TlsLightStepTracer(lightstep::Tracer tracer,
+                                                        LightStepDriver& driver)
+    : tracer_(tracer), driver_(driver) {}
+
+LightStepDriver::LightStepDriver(const Json::Object& config,
+                                 Upstream::ClusterManager& cluster_manager, Stats::Store& stats,
+                                 ThreadLocal::Instance& tls, Runtime::Loader& runtime,
+                                 std::unique_ptr<lightstep::TracerOptions> options)
+    : cm_(cluster_manager),
+      tracer_stats_{LIGHTSTEP_TRACER_STATS(POOL_COUNTER_PREFIX(stats, "tracing.lightstep."))},
+      tls_(tls), runtime_(runtime), options_(std::move(options)), tls_slot_(tls.allocateSlot()) {
+  Upstream::ThreadLocalCluster* cluster = cm_.get(config.getString("collector_cluster"));
+  if (!cluster) {
+    throw EnvoyException(fmt::format("{} collector cluster is not defined on cluster manager level",
+                                     config.getString("collector_cluster")));
+  }
+  cluster_ = cluster->info();
+
+  if (!(cluster_->features() & Upstream::ClusterInfo::Features::HTTP2)) {
+    throw EnvoyException(
+        fmt::format("{} collector cluster must support http2 for gRPC calls", cluster_->name()));
+  }
+
+  tls_.set(tls_slot_,
+           [this](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
+             lightstep::Tracer tracer(lightstep::NewUserDefinedTransportLightStepTracer(
+                 *options_, std::bind(&LightStepRecorder::NewInstance, std::ref(*this),
+                                      std::ref(dispatcher), std::placeholders::_1)));
+
+             return ThreadLocal::ThreadLocalObjectSharedPtr{
+                 new TlsLightStepTracer(std::move(tracer), *this)};
+           });
+}
+
+SpanPtr LightStepDriver::startSpan(Http::HeaderMap& request_headers,
+                                   const std::string& operation_name, SystemTime start_time) {
+  lightstep::Tracer& tracer = tls_.getTyped<TlsLightStepTracer>(tls_slot_).tracer_;
+  LightStepSpanPtr active_span;
+
+  if (request_headers.OtSpanContext()) {
+    // Extract downstream context from HTTP carrier.
+    // This code is safe even if decode returns empty string or data is malformed.
+    std::string parent_context = Base64::decode(request_headers.OtSpanContext()->value().c_str());
+    lightstep::BinaryCarrier ctx;
+    ctx.ParseFromString(parent_context);
+
+    lightstep::SpanContext parent_span_ctx = tracer.Extract(
+        lightstep::CarrierFormat::LightStepBinaryCarrier, lightstep::ProtoReader(ctx));
+    lightstep::Span ls_span =
+        tracer.StartSpan(operation_name, {lightstep::ChildOf(parent_span_ctx),
+                                          lightstep::StartTimestamp(start_time)});
+    active_span.reset(new LightStepSpan(ls_span));
+  } else {
+    lightstep::Span ls_span =
+        tracer.StartSpan(operation_name, {lightstep::StartTimestamp(start_time)});
+    active_span.reset(new LightStepSpan(ls_span));
+  }
+
+  // Inject newly created span context into HTTP carrier.
+  lightstep::BinaryCarrier ctx;
+  tracer.Inject(active_span->context(), lightstep::CarrierFormat::LightStepBinaryCarrier,
+                lightstep::ProtoWriter(&ctx));
+  const std::string current_span_context = ctx.SerializeAsString();
+  request_headers.insertOtSpanContext().value(
+      Base64::encode(current_span_context.c_str(), current_span_context.length()));
+
+  return std::move(active_span);
+}
+
+void LightStepRecorder::onFailure(Http::AsyncClient::FailureReason) {
+  Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
+                           lightstep::CollectorMethodName(), false);
+}
+
+void LightStepRecorder::onSuccess(Http::MessagePtr&& msg) {
+  try {
+    Grpc::Common::validateResponse(*msg);
+
+    Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
+                             lightstep::CollectorMethodName(), true);
+  } catch (const Grpc::Exception& ex) {
+    Grpc::Common::chargeStat(*driver_.cluster(), lightstep::CollectorServiceFullName(),
+                             lightstep::CollectorMethodName(), false);
+  }
+}
+
+} // Tracing
diff --git a/source/common/tracing/lightstep_tracer_impl.h b/source/common/tracing/lightstep_tracer_impl.h
new file mode 100644
index 000000000000..68925f2f08fc
--- /dev/null
+++ b/source/common/tracing/lightstep_tracer_impl.h
@@ -0,0 +1,107 @@
+#pragma once
+
+#include "envoy/runtime/runtime.h"
+#include "envoy/thread_local/thread_local.h"
+#include "envoy/tracing/http_tracer.h"
+#include "envoy/upstream/cluster_manager.h"
+
+#include "common/http/header_map_impl.h"
+#include "common/http/message_impl.h"
+#include "common/json/json_loader.h"
+
+#include "lightstep/carrier.h"
+#include "lightstep/tracer.h"
+
+namespace Tracing {
+
+#define LIGHTSTEP_TRACER_STATS(COUNTER)                                                            \
+  COUNTER(spans_sent)                                                                              \
+  COUNTER(timer_flushed)
+
+struct LightstepTracerStats {
+  LIGHTSTEP_TRACER_STATS(GENERATE_COUNTER_STRUCT)
+};
+
+class LightStepSpan : public Span {
+public:
+  LightStepSpan(lightstep::Span& span);
+
+  // Tracing::Span
+  void finishSpan() override;
+  void setTag(const std::string& name, const std::string& value) override;
+
+  lightstep::SpanContext context() { return span_.context(); }
+
+private:
+  lightstep::Span span_;
+};
+
+typedef std::unique_ptr<LightStepSpan> LightStepSpanPtr;
+
+/**
+ * LightStep (http://lightstep.com/) provides tracing capabilities, aggregation, visualization of
+ * application trace data.
+ *
+ * LightStepSink is for flushing data to LightStep collectors.
+ */
+class LightStepDriver : public Driver {
+public:
+  LightStepDriver(const Json::Object& config, Upstream::ClusterManager& cluster_manager,
+                  Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime,
+                  std::unique_ptr<lightstep::TracerOptions> options);
+
+  // Tracer::TracingDriver
+  SpanPtr startSpan(Http::HeaderMap& request_headers, const std::string& operation_name,
+                    SystemTime start_time) override;
+
+  Upstream::ClusterManager& clusterManager() { return cm_; }
+  Upstream::ClusterInfoConstSharedPtr cluster() { return cluster_; }
+  Runtime::Loader& runtime() { return runtime_; }
+  LightstepTracerStats& tracerStats() { return tracer_stats_; }
+
+private:
+  struct TlsLightStepTracer : ThreadLocal::ThreadLocalObject {
+    TlsLightStepTracer(lightstep::Tracer tracer, LightStepDriver& driver);
+
+    void shutdown() override {}
+
+    lightstep::Tracer tracer_;
+    LightStepDriver& driver_;
+  };
+
+  Upstream::ClusterManager& cm_;
+  Upstream::ClusterInfoConstSharedPtr cluster_;
+  LightstepTracerStats tracer_stats_;
+  ThreadLocal::Instance& tls_;
+  Runtime::Loader& runtime_;
+  std::unique_ptr<lightstep::TracerOptions> options_;
+  uint32_t tls_slot_;
+};
+
+class LightStepRecorder : public lightstep::Recorder, Http::AsyncClient::Callbacks {
+public:
+  LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver,
+                    Event::Dispatcher& dispatcher);
+
+  // lightstep::Recorder
+  void RecordSpan(lightstep::collector::Span&& span) override;
+  bool FlushWithTimeout(lightstep::Duration) override;
+
+  // Http::AsyncClient::Callbacks
+  void onSuccess(Http::MessagePtr&&) override;
+  void onFailure(Http::AsyncClient::FailureReason) override;
+
+  static std::unique_ptr<lightstep::Recorder> NewInstance(LightStepDriver& driver,
+                                                          Event::Dispatcher& dispatcher,
+                                                          const lightstep::TracerImpl& tracer);
+
+private:
+  void enableTimer();
+  void flushSpans();
+
+  lightstep::ReportBuilder builder_;
+  LightStepDriver& driver_;
+  Event::TimerPtr flush_timer_;
+};
+
+} // Tracing
diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc
index aa06454b1fc0..3c6768f6eb19 100644
--- a/source/server/configuration_impl.cc
+++ b/source/server/configuration_impl.cc
@@ -10,6 +10,7 @@
 #include "common/ratelimit/ratelimit_impl.h"
 #include "common/ssl/context_config_impl.h"
 #include "common/tracing/http_tracer_impl.h"
+#include "common/tracing/lightstep_tracer_impl.h"
 #include "common/upstream/cluster_manager_impl.h"
 
 namespace Server {
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 46103c0ee24c..4c46349c2a3f 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -99,6 +99,7 @@ add_executable(envoy-test
   common/stats/statsd_test.cc
   common/stats/thread_local_store_test.cc
   common/tracing/http_tracer_impl_test.cc
+  common/tracing/lightstep_tracer_impl_test.cc
   common/upstream/cds_api_impl_test.cc
   common/upstream/cluster_manager_impl_test.cc
   common/upstream/health_checker_impl_test.cc
diff --git a/test/common/tracing/BUILD b/test/common/tracing/BUILD
index fe926bcbd0d1..dc3b2ad97093 100644
--- a/test/common/tracing/BUILD
+++ b/test/common/tracing/BUILD
@@ -4,7 +4,10 @@ load("//bazel:envoy_build_system.bzl", "envoy_cc_test")
 
 envoy_cc_test(
     name = "http_tracer_impl_test",
-    srcs = ["http_tracer_impl_test.cc"],
+    srcs = [
+        "http_tracer_impl_test.cc",
+        "lightstep_tracer_impl_test.cc",
+    ],
     deps = [
         "//source/common/common:base64_lib",
         "//source/common/http:header_map_lib",
diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc
index 66094db919f9..df18f9f524e6 100644
--- a/test/common/tracing/http_tracer_impl_test.cc
+++ b/test/common/tracing/http_tracer_impl_test.cc
@@ -5,6 +5,7 @@
 #include "common/runtime/runtime_impl.h"
 #include "common/runtime/uuid_util.h"
 #include "common/tracing/http_tracer_impl.h"
+#include "common/tracing/lightstep_tracer_impl.h"
 
 #include "test/mocks/http/mocks.h"
 #include "test/mocks/local_info/mocks.h"
@@ -336,110 +337,6 @@ TEST(HttpTracerUtilityTest, operationTypeToString) {
   EXPECT_EQ("egress", HttpTracerUtility::toString(OperationName::Egress));
 }
 
-class LightStepDriverTest : public Test {
-public:
-  void setup(Json::Object& config, bool init_timer) {
-    std::unique_ptr<lightstep::TracerOptions> opts(new lightstep::TracerOptions());
-    opts->access_token = "sample_token";
-    opts->tracer_attributes["lightstep.component_name"] = "component";
-
-    ON_CALL(cm_, httpAsyncClientForCluster("fake_cluster"))
-        .WillByDefault(ReturnRef(cm_.async_client_));
-
-    if (init_timer) {
-      timer_ = new NiceMock<Event::MockTimer>(&tls_.dispatcher_);
-      EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1000)));
-    }
-
-    driver_.reset(new LightStepDriver(config, cm_, stats_, tls_, runtime_, std::move(opts)));
-  }
-
-  void setupValidDriver() {
-    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
-    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features())
-        .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));
-
-    std::string valid_config = R"EOF(
-      {"collector_cluster": "fake_cluster"}
-    )EOF";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
-
-    setup(*loader, true);
-  }
-
-  const std::string operation_name_{"test"};
-  Http::TestHeaderMapImpl request_headers_{
-      {":path", "/"}, {":method", "GET"}, {"x-request-id", "foo"}};
-  const Http::TestHeaderMapImpl response_headers_{{":status", "500"}};
-  SystemTime start_time_;
-  Http::AccessLog::MockRequestInfo request_info_;
-
-  std::unique_ptr<LightStepDriver> driver_;
-  NiceMock<Event::MockTimer>* timer_;
-  Stats::IsolatedStoreImpl stats_;
-  NiceMock<Upstream::MockClusterManager> cm_;
-  NiceMock<Runtime::MockRandomGenerator> random_;
-  NiceMock<Runtime::MockLoader> runtime_;
-  NiceMock<ThreadLocal::MockInstance> tls_;
-  NiceMock<LocalInfo::MockLocalInfo> local_info_;
-};
-
-TEST_F(LightStepDriverTest, InitializeDriver) {
-  {
-    std::string invalid_config = R"EOF(
-      {"fake" : "fake"}
-    )EOF";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(invalid_config);
-
-    EXPECT_THROW(setup(*loader, false), EnvoyException);
-  }
-
-  {
-    std::string empty_config = "{}";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(empty_config);
-
-    EXPECT_THROW(setup(*loader, false), EnvoyException);
-  }
-
-  {
-    // Valid config but not valid cluster.
-    EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr));
-
-    std::string valid_config = R"EOF(
-      {"collector_cluster": "fake_cluster"}
-    )EOF";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
-
-    EXPECT_THROW(setup(*loader, false), EnvoyException);
-  }
-
-  {
-    // Valid config, but upstream cluster does not support http2.
-    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
-    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()).WillByDefault(Return(0));
-
-    std::string valid_config = R"EOF(
-      {"collector_cluster": "fake_cluster"}
-    )EOF";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
-
-    EXPECT_THROW(setup(*loader, false), EnvoyException);
-  }
-
-  {
-    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
-    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features())
-        .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));
-
-    std::string valid_config = R"EOF(
-      {"collector_cluster": "fake_cluster"}
-    )EOF";
-    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
-
-    setup(*loader, true);
-  }
-}
-
 TEST(HttpNullTracerTest, BasicFunctionality) {
   HttpNullTracer null_tracer;
   MockConfig config;
@@ -492,156 +389,4 @@ TEST_F(HttpTracerImplTest, BasicFunctionalityNodeSet) {
   tracer_->startSpan(config_, request_headers_, request_info_);
 }
 
-TEST_F(LightStepDriverTest, FlushSeveralSpans) {
-  setupValidDriver();
-
-  Http::MockAsyncClientRequest request(&cm_.async_client_);
-  Http::AsyncClient::Callbacks* callback;
-  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
-
-  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout))
-      .WillOnce(
-          Invoke([&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& callbacks,
-                     const Optional<std::chrono::milliseconds>&) -> Http::AsyncClient::Request* {
-            callback = &callbacks;
-
-            EXPECT_STREQ("/lightstep.collector.CollectorService/Report",
-                         message->headers().Path()->value().c_str());
-            EXPECT_STREQ("fake_cluster", message->headers().Host()->value().c_str());
-            EXPECT_STREQ("application/grpc", message->headers().ContentType()->value().c_str());
-
-            return &request;
-          }));
-
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
-      .Times(2)
-      .WillRepeatedly(Return(2));
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
-      .WillOnce(Return(5000U));
-
-  SpanPtr first_span = driver_->startSpan(request_headers_, operation_name_, start_time_);
-  first_span->finishSpan();
-
-  SpanPtr second_span = driver_->startSpan(request_headers_, operation_name_, start_time_);
-  second_span->finishSpan();
-
-  Http::MessagePtr msg(new Http::ResponseMessageImpl(
-      Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-
-  msg->trailers(Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{"grpc-status", "0"}}});
-
-  callback->onSuccess(std::move(msg));
-
-  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
-                    .counter("grpc.lightstep.collector.CollectorService.Report.success")
-                    .value());
-
-  callback->onFailure(Http::AsyncClient::FailureReason::Reset);
-
-  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
-                    .counter("grpc.lightstep.collector.CollectorService.Report.failure")
-                    .value());
-  EXPECT_EQ(2U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
-                    .counter("grpc.lightstep.collector.CollectorService.Report.total")
-                    .value());
-  EXPECT_EQ(2U, stats_.counter("tracing.lightstep.spans_sent").value());
-}
-
-TEST_F(LightStepDriverTest, FlushSpansTimer) {
-  setupValidDriver();
-
-  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
-  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout));
-
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
-      .WillOnce(Return(5));
-
-  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
-  span->finishSpan();
-
-  // Timer should be re-enabled.
-  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1000)));
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
-      .WillOnce(Return(5000U));
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.flush_interval_ms", 1000U))
-      .WillOnce(Return(1000U));
-
-  timer_->callback_();
-
-  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.timer_flushed").value());
-  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.spans_sent").value());
-}
-
-TEST_F(LightStepDriverTest, FlushOneSpanGrpcFailure) {
-  setupValidDriver();
-
-  Http::MockAsyncClientRequest request(&cm_.async_client_);
-  Http::AsyncClient::Callbacks* callback;
-  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
-
-  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout))
-      .WillOnce(
-          Invoke([&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& callbacks,
-                     const Optional<std::chrono::milliseconds>&) -> Http::AsyncClient::Request* {
-            callback = &callbacks;
-
-            EXPECT_STREQ("/lightstep.collector.CollectorService/Report",
-                         message->headers().Path()->value().c_str());
-            EXPECT_STREQ("fake_cluster", message->headers().Host()->value().c_str());
-            EXPECT_STREQ("application/grpc", message->headers().ContentType()->value().c_str());
-
-            return &request;
-          }));
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
-      .WillOnce(Return(1));
-  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
-      .WillOnce(Return(5000U));
-
-  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
-  span->finishSpan();
-
-  Http::MessagePtr msg(new Http::ResponseMessageImpl(
-      Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-
-  // No trailers, gRPC is considered failed.
-  callback->onSuccess(std::move(msg));
-
-  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
-                    .counter("grpc.lightstep.collector.CollectorService.Report.failure")
-                    .value());
-  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
-                    .counter("grpc.lightstep.collector.CollectorService.Report.total")
-                    .value());
-  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.spans_sent").value());
-}
-
-TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) {
-  setupValidDriver();
-
-  // Supply bogus context, that will be simply ignored.
-  const std::string invalid_context = "notvalidcontext";
-  request_headers_.insertOtSpanContext().value(invalid_context);
-  driver_->startSpan(request_headers_, operation_name_, start_time_);
-
-  std::string injected_ctx = request_headers_.OtSpanContext()->value().c_str();
-  EXPECT_FALSE(injected_ctx.empty());
-
-  // Supply empty context.
-  request_headers_.removeOtSpanContext();
-  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
-
-  injected_ctx = request_headers_.OtSpanContext()->value().c_str();
-  EXPECT_FALSE(injected_ctx.empty());
-
-  // Context can be parsed fine.
-  lightstep::BinaryCarrier ctx;
-  std::string context = Base64::decode(injected_ctx);
-  ctx.ParseFromString(context);
-
-  // Supply parent context, request_headers has properly populated x-ot-span-context.
-  SpanPtr span_with_parent = driver_->startSpan(request_headers_, operation_name_, start_time_);
-  injected_ctx = request_headers_.OtSpanContext()->value().c_str();
-  EXPECT_FALSE(injected_ctx.empty());
-}
-
 } // Tracing
diff --git a/test/common/tracing/lightstep_tracer_impl_test.cc b/test/common/tracing/lightstep_tracer_impl_test.cc
new file mode 100644
index 000000000000..ec66c00524ce
--- /dev/null
+++ b/test/common/tracing/lightstep_tracer_impl_test.cc
@@ -0,0 +1,284 @@
+#include "common/common/base64.h"
+#include "common/http/headers.h"
+#include "common/http/header_map_impl.h"
+#include "common/http/message_impl.h"
+#include "common/runtime/runtime_impl.h"
+#include "common/runtime/uuid_util.h"
+#include "common/tracing/http_tracer_impl.h"
+#include "common/tracing/lightstep_tracer_impl.h"
+
+#include "test/mocks/http/mocks.h"
+#include "test/mocks/local_info/mocks.h"
+#include "test/mocks/runtime/mocks.h"
+#include "test/mocks/stats/mocks.h"
+#include "test/mocks/thread_local/mocks.h"
+#include "test/mocks/tracing/mocks.h"
+#include "test/mocks/upstream/mocks.h"
+#include "test/test_common/utility.h"
+
+using testing::_;
+using testing::Invoke;
+using testing::NiceMock;
+using testing::Return;
+using testing::ReturnRef;
+using testing::Test;
+
+namespace Tracing {
+
+class LightStepDriverTest : public Test {
+public:
+  void setup(Json::Object& config, bool init_timer) {
+    std::unique_ptr<lightstep::TracerOptions> opts(new lightstep::TracerOptions());
+    opts->access_token = "sample_token";
+    opts->tracer_attributes["lightstep.component_name"] = "component";
+
+    ON_CALL(cm_, httpAsyncClientForCluster("fake_cluster"))
+        .WillByDefault(ReturnRef(cm_.async_client_));
+
+    if (init_timer) {
+      timer_ = new NiceMock<Event::MockTimer>(&tls_.dispatcher_);
+      EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1000)));
+    }
+
+    driver_.reset(new LightStepDriver(config, cm_, stats_, tls_, runtime_, std::move(opts)));
+  }
+
+  void setupValidDriver() {
+    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
+    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features())
+        .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));
+
+    std::string valid_config = R"EOF(
+      {"collector_cluster": "fake_cluster"}
+    )EOF";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
+
+    setup(*loader, true);
+  }
+
+  const std::string operation_name_{"test"};
+  Http::TestHeaderMapImpl request_headers_{
+      {":path", "/"}, {":method", "GET"}, {"x-request-id", "foo"}};
+  const Http::TestHeaderMapImpl response_headers_{{":status", "500"}};
+  SystemTime start_time_;
+  Http::AccessLog::MockRequestInfo request_info_;
+
+  std::unique_ptr<LightStepDriver> driver_;
+  NiceMock<Event::MockTimer>* timer_;
+  Stats::IsolatedStoreImpl stats_;
+  NiceMock<Upstream::MockClusterManager> cm_;
+  NiceMock<Runtime::MockRandomGenerator> random_;
+  NiceMock<Runtime::MockLoader> runtime_;
+  NiceMock<ThreadLocal::MockInstance> tls_;
+  NiceMock<LocalInfo::MockLocalInfo> local_info_;
+};
+
+TEST_F(LightStepDriverTest, InitializeDriver) {
+  {
+    std::string invalid_config = R"EOF(
+      {"fake" : "fake"}
+    )EOF";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(invalid_config);
+
+    EXPECT_THROW(setup(*loader, false), EnvoyException);
+  }
+
+  {
+    std::string empty_config = "{}";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(empty_config);
+
+    EXPECT_THROW(setup(*loader, false), EnvoyException);
+  }
+
+  {
+    // Valid config but not valid cluster.
+    EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr));
+
+    std::string valid_config = R"EOF(
+      {"collector_cluster": "fake_cluster"}
+    )EOF";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
+
+    EXPECT_THROW(setup(*loader, false), EnvoyException);
+  }
+
+  {
+    // Valid config, but upstream cluster does not support http2.
+    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
+    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()).WillByDefault(Return(0));
+
+    std::string valid_config = R"EOF(
+      {"collector_cluster": "fake_cluster"}
+    )EOF";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
+
+    EXPECT_THROW(setup(*loader, false), EnvoyException);
+  }
+
+  {
+    EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_));
+    ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features())
+        .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2));
+
+    std::string valid_config = R"EOF(
+      {"collector_cluster": "fake_cluster"}
+    )EOF";
+    Json::ObjectPtr loader = Json::Factory::LoadFromString(valid_config);
+
+    setup(*loader, true);
+  }
+}
+
+TEST_F(LightStepDriverTest, FlushSeveralSpans) {
+  setupValidDriver();
+
+  Http::MockAsyncClientRequest request(&cm_.async_client_);
+  Http::AsyncClient::Callbacks* callback;
+  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
+
+  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout))
+      .WillOnce(
+          Invoke([&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& callbacks,
+                     const Optional<std::chrono::milliseconds>&) -> Http::AsyncClient::Request* {
+            callback = &callbacks;
+
+            EXPECT_STREQ("/lightstep.collector.CollectorService/Report",
+                         message->headers().Path()->value().c_str());
+            EXPECT_STREQ("fake_cluster", message->headers().Host()->value().c_str());
+            EXPECT_STREQ("application/grpc", message->headers().ContentType()->value().c_str());
+
+            return &request;
+          }));
+
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
+      .Times(2)
+      .WillRepeatedly(Return(2));
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
+      .WillOnce(Return(5000U));
+
+  SpanPtr first_span = driver_->startSpan(request_headers_, operation_name_, start_time_);
+  first_span->finishSpan();
+
+  SpanPtr second_span = driver_->startSpan(request_headers_, operation_name_, start_time_);
+  second_span->finishSpan();
+
+  Http::MessagePtr msg(new Http::ResponseMessageImpl(
+      Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
+
+  msg->trailers(Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{"grpc-status", "0"}}});
+
+  callback->onSuccess(std::move(msg));
+
+  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("grpc.lightstep.collector.CollectorService.Report.success")
+                    .value());
+
+  callback->onFailure(Http::AsyncClient::FailureReason::Reset);
+
+  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("grpc.lightstep.collector.CollectorService.Report.failure")
+                    .value());
+  EXPECT_EQ(2U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("grpc.lightstep.collector.CollectorService.Report.total")
+                    .value());
+  EXPECT_EQ(2U, stats_.counter("tracing.lightstep.spans_sent").value());
+}
+
+TEST_F(LightStepDriverTest, FlushSpansTimer) {
+  setupValidDriver();
+
+  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
+  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout));
+
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
+      .WillOnce(Return(5));
+
+  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
+  span->finishSpan();
+
+  // Timer should be re-enabled.
+  EXPECT_CALL(*timer_, enableTimer(std::chrono::milliseconds(1000)));
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
+      .WillOnce(Return(5000U));
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.flush_interval_ms", 1000U))
+      .WillOnce(Return(1000U));
+
+  timer_->callback_();
+
+  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.timer_flushed").value());
+  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.spans_sent").value());
+}
+
+TEST_F(LightStepDriverTest, FlushOneSpanGrpcFailure) {
+  setupValidDriver();
+
+  Http::MockAsyncClientRequest request(&cm_.async_client_);
+  Http::AsyncClient::Callbacks* callback;
+  const Optional<std::chrono::milliseconds> timeout(std::chrono::seconds(5));
+
+  EXPECT_CALL(cm_.async_client_, send_(_, _, timeout))
+      .WillOnce(
+          Invoke([&](Http::MessagePtr& message, Http::AsyncClient::Callbacks& callbacks,
+                     const Optional<std::chrono::milliseconds>&) -> Http::AsyncClient::Request* {
+            callback = &callbacks;
+
+            EXPECT_STREQ("/lightstep.collector.CollectorService/Report",
+                         message->headers().Path()->value().c_str());
+            EXPECT_STREQ("fake_cluster", message->headers().Host()->value().c_str());
+            EXPECT_STREQ("application/grpc", message->headers().ContentType()->value().c_str());
+
+            return &request;
+          }));
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.min_flush_spans", 5))
+      .WillOnce(Return(1));
+  EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.lightstep.request_timeout", 5000U))
+      .WillOnce(Return(5000U));
+
+  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
+  span->finishSpan();
+
+  Http::MessagePtr msg(new Http::ResponseMessageImpl(
+      Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
+
+  // No trailers, gRPC is considered failed.
+  callback->onSuccess(std::move(msg));
+
+  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("grpc.lightstep.collector.CollectorService.Report.failure")
+                    .value());
+  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("grpc.lightstep.collector.CollectorService.Report.total")
+                    .value());
+  EXPECT_EQ(1U, stats_.counter("tracing.lightstep.spans_sent").value());
+}
+
+TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) {
+  setupValidDriver();
+
+  // Supply bogus context, that will be simply ignored.
+  const std::string invalid_context = "notvalidcontext";
+  request_headers_.insertOtSpanContext().value(invalid_context);
+  driver_->startSpan(request_headers_, operation_name_, start_time_);
+
+  std::string injected_ctx = request_headers_.OtSpanContext()->value().c_str();
+  EXPECT_FALSE(injected_ctx.empty());
+
+  // Supply empty context.
+  request_headers_.removeOtSpanContext();
+  SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_);
+
+  injected_ctx = request_headers_.OtSpanContext()->value().c_str();
+  EXPECT_FALSE(injected_ctx.empty());
+
+  // Context can be parsed fine.
+  lightstep::BinaryCarrier ctx;
+  std::string context = Base64::decode(injected_ctx);
+  ctx.ParseFromString(context);
+
+  // Supply parent context, request_headers has properly populated x-ot-span-context.
+  SpanPtr span_with_parent = driver_->startSpan(request_headers_, operation_name_, start_time_);
+  injected_ctx = request_headers_.OtSpanContext()->value().c_str();
+  EXPECT_FALSE(injected_ctx.empty());
+}
+
+} // Tracing

From 8f663529612f3b3f776bd209a8485ac444ebe55b Mon Sep 17 00:00:00 2001
From: ccaraman <ccaramanolis@lyft.com>
Date: Mon, 10 Apr 2017 14:23:09 -0700
Subject: [PATCH 07/12] Optionally include virtual host rate limit
 configurations (#713)

---
 .../http_conn_man/route_config/route.rst      | 12 ++-
 .../http_filters/rate_limit_filter.rst        |  9 +-
 include/envoy/router/router.h                 |  5 ++
 include/envoy/router/router_ratelimit.h       |  5 ++
 source/common/http/async_client_impl.h        |  2 +
 source/common/http/filter/ratelimit.cc        |  9 +-
 source/common/json/config_schemas.cc          |  1 +
 source/common/router/config_impl.cc           |  5 ++
 source/common/router/config_impl.h            |  5 ++
 source/common/router/router_ratelimit.h       |  1 +
 test/common/http/filter/ratelimit_test.cc     | 34 +++++++
 test/common/router/config_impl_test.cc        | 89 +++++++++++++++++++
 test/common/router/router_ratelimit_test.cc   |  9 +-
 test/mocks/router/mocks.cc                    |  2 +
 test/mocks/router/mocks.h                     |  2 +
 15 files changed, 176 insertions(+), 14 deletions(-)

diff --git a/docs/configuration/http_conn_man/route_config/route.rst b/docs/configuration/http_conn_man/route_config/route.rst
index 482f3ca67770..98e57f09e398 100644
--- a/docs/configuration/http_conn_man/route_config/route.rst
+++ b/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,7 +378,7 @@ 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
@@ -377,4 +386,3 @@ string map.  Nested objects are not supported.
   [
     {"...": "..."}
   ]
-
diff --git a/docs/configuration/http_filters/rate_limit_filter.rst b/docs/configuration/http_filters/rate_limit_filter.rst
index 41b19973ef54..371cc633363c 100644
--- a/docs/configuration/http_filters/rate_limit_filter.rst
+++ b/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.
diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h
index fb7de37548d3..7bd906eb5ee7 100644
--- a/include/envoy/router/router.h
+++ b/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;
 };
 
 /**
diff --git a/include/envoy/router/router_ratelimit.h b/include/envoy/router/router_ratelimit.h
index 7ad5cd52fa81..4fcd0f2f3a13 100644
--- a/include/envoy/router/router_ratelimit.h
+++ b/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.
diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h
index 1c26c0276391..f259db54d575 100644
--- a/source/common/http/async_client_impl.h
+++ b/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_;
diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc
index 4f3478b4fdaa..4b62fad5d74c 100644
--- a/source/common/http/filter/ratelimit.cc
+++ b/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;
diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc
index 975d7b7043e8..d42acf57ae6c 100644
--- a/source/common/json/config_schemas.cc
+++ b/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" : {
diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc
index d48bfc58062e..c8ae06e96839 100644
--- a/source/common/router/config_impl.cc
+++ b/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 {
diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h
index 3c2251e16b31..8b623f0cc244 100644
--- a/source/common/router/config_impl.h
+++ b/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; }
diff --git a/source/common/router/router_ratelimit.h b/source/common/router/router_ratelimit.h
index 1120beb0b7fa..79133b7f77fc 100644
--- a/source/common/router/router_ratelimit.h
+++ b/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_;
diff --git a/test/common/http/filter/ratelimit_test.cc b/test/common/http/filter/ratelimit_test.cc
index 940dc4bd3455..c1b9abf0de21 100644
--- a/test/common/http/filter/ratelimit_test.cc
+++ b/test/common/http/filter/ratelimit_test.cc
@@ -427,6 +427,40 @@ TEST_F(HttpRateLimitFilterTest, ExternalRequestType) {
             cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ratelimit.ok").value());
 }
 
+TEST_F(HttpRateLimitFilterTest, ExcludeVirtualHost) {
+  std::string external_filter_config = R"EOF(
+  {
+    "domain": "foo"
+  }
+  )EOF";
+  SetUpTest(external_filter_config);
+  InSequence s;
+
+  EXPECT_CALL(filter_callbacks_.route_->route_entry_.rate_limit_policy_, getApplicableRateLimit(0));
+  EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _))
+      .WillOnce(SetArgReferee<1>(descriptor_));
+
+  EXPECT_CALL(filter_callbacks_.route_->route_entry_, includeVirtualHostRateLimits())
+      .WillOnce(Return(false));
+  EXPECT_CALL(filter_callbacks_.route_->route_entry_.virtual_host_.rate_limit_policy_,
+              getApplicableRateLimit(0)).Times(0);
+
+  EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector<::RateLimit::Descriptor>{
+                                            {{{"descriptor_key", "descriptor_value"}}}}),
+                              Tracing::EMPTY_CONTEXT))
+      .WillOnce(WithArgs<0>(Invoke([&](::RateLimit::RequestCallbacks& callbacks) -> void {
+        callbacks.complete(::RateLimit::LimitStatus::OK);
+      })));
+
+  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
+  EXPECT_EQ(FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false));
+  EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, false));
+  EXPECT_EQ(FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
+
+  EXPECT_EQ(1U,
+            cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ratelimit.ok").value());
+}
+
 TEST_F(HttpRateLimitFilterTest, ConfigValueTest) {
   std::string stage_filter_config = R"EOF(
   {
diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
index 38c1c4432de1..745671ee434a 100644
--- a/test/common/router/config_impl_test.cc
+++ b/test/common/router/config_impl_test.cc
@@ -1786,4 +1786,93 @@ TEST(RouteMatcherTest, TestOpaqueConfig) {
   EXPECT_EQ("value2", opaque_config.find("name2")->second);
 }
 
+TEST(RoutePropertyTest, excludeVHRateLimits) {
+  std::string json = R"EOF(
+  {
+    "virtual_hosts": [
+      {
+        "name": "www2",
+        "domains": ["*"],
+        "routes": [
+          {
+            "prefix": "/",
+            "cluster": "www2"
+          }
+        ]
+      }
+    ]
+  }
+  )EOF";
+
+  Json::ObjectPtr loader = Json::Factory::LoadFromString(json);
+  NiceMock<Runtime::MockLoader> runtime;
+  NiceMock<Upstream::MockClusterManager> cm;
+  Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/foo", "GET");
+  std::unique_ptr<ConfigImpl> config_ptr;
+
+  config_ptr.reset(new ConfigImpl(*loader, runtime, cm, true));
+  EXPECT_TRUE(config_ptr->route(headers, 0)->routeEntry()->includeVirtualHostRateLimits());
+
+  json = R"EOF(
+  {
+    "virtual_hosts": [
+      {
+        "name": "www2",
+        "domains": ["*"],
+        "routes": [
+          {
+            "prefix": "/",
+            "cluster": "www2",
+            "rate_limits": [
+              {
+                "actions": [
+                  {
+                    "type": "remote_address"
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+  )EOF";
+
+  loader = Json::Factory::LoadFromString(json);
+  config_ptr.reset(new ConfigImpl(*loader, runtime, cm, true));
+  EXPECT_FALSE(config_ptr->route(headers, 0)->routeEntry()->includeVirtualHostRateLimits());
+
+  json = R"EOF(
+  {
+    "virtual_hosts": [
+      {
+        "name": "www2",
+        "domains": ["*"],
+        "routes": [
+          {
+            "prefix": "/",
+            "cluster": "www2",
+            "include_vh_rate_limits": true,
+            "rate_limits": [
+              {
+                "actions": [
+                  {
+                    "type": "remote_address"
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+  )EOF";
+
+  loader = Json::Factory::LoadFromString(json);
+  config_ptr.reset(new ConfigImpl(*loader, runtime, cm, true));
+  EXPECT_TRUE(config_ptr->route(headers, 0)->routeEntry()->includeVirtualHostRateLimits());
+}
+
 } // Router
diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc
index f430a80c4370..0a80a6cdbbb2 100644
--- a/test/common/router/router_ratelimit_test.cc
+++ b/test/common/router/router_ratelimit_test.cc
@@ -159,11 +159,9 @@ TEST_F(RateLimitConfiguration, NoRateLimitPolicy) {
 
   SetUpTest(json);
 
-  EXPECT_EQ(0U, config_->route(genHeaders("www.lyft.com", "/bar", "GET"), 0)
-                    ->routeEntry()
-                    ->rateLimitPolicy()
-                    .getApplicableRateLimit(0)
-                    .size());
+  route_ = config_->route(genHeaders("www.lyft.com", "/bar", "GET"), 0)->routeEntry();
+  EXPECT_EQ(0U, route_->rateLimitPolicy().getApplicableRateLimit(0).size());
+  EXPECT_TRUE(route_->rateLimitPolicy().empty());
 }
 
 TEST_F(RateLimitConfiguration, TestGetApplicationRateLimit) {
@@ -197,6 +195,7 @@ TEST_F(RateLimitConfiguration, TestGetApplicationRateLimit) {
   std::string address = "10.0.0.1";
 
   route_ = config_->route(genHeaders("www.lyft.com", "/foo", "GET"), 0)->routeEntry();
+  EXPECT_FALSE(route_->rateLimitPolicy().empty());
   std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limits =
       route_->rateLimitPolicy().getApplicableRateLimit(0);
   EXPECT_EQ(1U, rate_limits.size());
diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc
index 3521622eeb45..17457ac242e7 100644
--- a/test/mocks/router/mocks.cc
+++ b/test/mocks/router/mocks.cc
@@ -27,6 +27,7 @@ MockRateLimitPolicyEntry::~MockRateLimitPolicyEntry() {}
 
 MockRateLimitPolicy::MockRateLimitPolicy() {
   ON_CALL(*this, getApplicableRateLimit(_)).WillByDefault(ReturnRef(rate_limit_policy_entry_));
+  ON_CALL(*this, empty()).WillByDefault(Return(true));
 }
 
 MockRateLimitPolicy::~MockRateLimitPolicy() {}
@@ -53,6 +54,7 @@ MockRouteEntry::MockRouteEntry() {
   ON_CALL(*this, timeout()).WillByDefault(Return(std::chrono::milliseconds(10)));
   ON_CALL(*this, virtualCluster(_)).WillByDefault(Return(&virtual_cluster_));
   ON_CALL(*this, virtualHost()).WillByDefault(ReturnRef(virtual_host_));
+  ON_CALL(*this, includeVirtualHostRateLimits()).WillByDefault(Return(true));
 }
 
 MockRouteEntry::~MockRouteEntry() {}
diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
index 6455306f5eb0..68dc6b894ecf 100644
--- a/test/mocks/router/mocks.h
+++ b/test/mocks/router/mocks.h
@@ -67,6 +67,7 @@ class MockRateLimitPolicy : public RateLimitPolicy {
   MOCK_CONST_METHOD1(
       getApplicableRateLimit,
       std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>&(uint64_t stage));
+  MOCK_CONST_METHOD0(empty, bool());
 
   std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>> rate_limit_policy_entry_;
 };
@@ -147,6 +148,7 @@ class MockRouteEntry : public RouteEntry {
   MOCK_CONST_METHOD0(virtualHost, const VirtualHost&());
   MOCK_CONST_METHOD0(autoHostRewrite, bool());
   MOCK_CONST_METHOD0(opaqueConfig, const std::multimap<std::string, std::string>&());
+  MOCK_CONST_METHOD0(includeVirtualHostRateLimits, bool());
 
   std::string cluster_name_{"fake_cluster"};
   std::multimap<std::string, std::string> opaque_config_;

From b4ce4da74a000374bf9946626da731ba71a652d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20No=C3=A9?= <dpn@google.com>
Date: Mon, 10 Apr 2017 19:07:28 -0400
Subject: [PATCH 08/12] GuardDog test: NiceMock the MockSystemTimeSource.
 (#729)

When this was changed from EXPECT_CALL to ON_CALL it introduced some
unexpected mock warnings.  Use NiceMock.
---
 test/server/guarddog_impl_test.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc
index e96ac3ea89be..5ce1d755e685 100644
--- a/test/server/guarddog_impl_test.cc
+++ b/test/server/guarddog_impl_test.cc
@@ -60,7 +60,7 @@ class GuardDogDeathTest : public testing::Test {
   NiceMock<Configuration::MockMain> config_kill_;
   NiceMock<Configuration::MockMain> config_multikill_;
   NiceMock<Stats::MockStore> fakestats_;
-  MockSystemTimeSource time_source_;
+  NiceMock<MockSystemTimeSource> time_source_;
   std::atomic<unsigned int> mock_time_;
   std::unique_ptr<GuardDogImpl> guard_dog_;
   WatchDogSharedPtr unpet_dog_;
@@ -134,7 +134,7 @@ class GuardDogMissTest : public testing::Test {
   NiceMock<Configuration::MockMain> config_miss_;
   NiceMock<Configuration::MockMain> config_mega_;
   Stats::IsolatedStoreImpl stats_store_;
-  MockSystemTimeSource time_source_;
+  NiceMock<MockSystemTimeSource> time_source_;
   std::atomic<unsigned int> mock_time_;
 };
 

From 2be5ba7b4df1d226dc877bb16905fd1a57300cd6 Mon Sep 17 00:00:00 2001
From: htuch <htuch@users.noreply.github.com>
Date: Tue, 11 Apr 2017 11:11:29 -0400
Subject: [PATCH 09/12] ci: build and install custom Bazel binary for coverage
 runs. (#727)

This version of Bazel will only be used for code coverage purposes. It's necessary due to
the regression in C++ support noted in https://github.com/bazelbuild/bazel/issues/1118.

The plan is to use regular Bazel for build types other than coverage, since we want to make sure
that we don't depend on anything in this snowflake.

Also added lcov to the build, since Bazel coverage needs this to be available for generating
trace files from the .gcno/.gcda for each test.
---
 ci/build_container/build_container.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/ci/build_container/build_container.sh b/ci/build_container/build_container.sh
index c16baacf83c4..4a32dea6068d 100755
--- a/ci/build_container/build_container.sh
+++ b/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
 

From 37b2bcf485d6cd002ee0db4019962c0e7a5acaba Mon Sep 17 00:00:00 2001
From: htuch <htuch@users.noreply.github.com>
Date: Tue, 11 Apr 2017 11:22:39 -0400
Subject: [PATCH 10/12] bazel: generate a single coverage binary for all C++
 tests. (#728)

This patch has a number of changes to how the build system works and how tests with non-trivial
environment setups get their files:

* Eliminate setup_cmds and envoy_cc_test_with_json. These setup scripts are now run where necessary
  inside the test fixture or equivalent functionality is now in TestEnvironment.

* Provide a single test binary target for all C++ (but not shell) tests in
  //test/coverage:coverage_tests. This is needed for reasonable performance coverage under Bazel,
  see https://github.com/bazelbuild/bazel/issues/1118 for why. Unfortunately, we need to generate
  the BUILD for this manually, I have a script in test/coverage/gen_build.sh to get this to happen.

* Avoid false dependency on //test/integration:integration_lib by test/main.cc. This came up while
  working on the coverage performance. Small tests, e.g. hex_test, were dragging in the integration
  test library for no reason, just because of some logging setup in main.cc.

* Set alwayslink=1 on all C++ libraries. While working on the above, strange link dependency orders
  were causing a problem. These mostly pertained to the places I had broken circular link
  dependencies, e.g. //source/common/upstream:upstream_{lib,includes}. There's cleaner ways to fix
  this than alwayslink=1, but I think that we probably do want to link all object files that the Envoy
  binary depends on and alwayslink=1 will prevent surprises for non-Bazel experts (e.g. when adding a
  new filter library).
---
 .gitignore                                    |  1 +
 bazel/BUILD                                   |  3 +
 bazel/envoy_build_system.bzl                  | 95 ++++++++++---------
 bazel/gen_sh_test_runner.sh                   | 25 +++++
 source/server/config/http/BUILD               |  6 --
 source/server/config/network/BUILD            |  7 --
 source/server/http/BUILD                      |  1 -
 test/BUILD                                    |  3 +-
 test/common/runtime/BUILD                     |  6 +-
 test/common/runtime/runtime_impl_test.cc      |  5 +
 test/common/ssl/BUILD                         | 20 ++--
 test/common/ssl/connection_impl_test.cc       | 11 ++-
 test/common/ssl/context_impl_test.cc          | 21 ++--
 test/common/ssl/ssl_certs_test.h              | 10 ++
 test/coverage/gen_build.sh                    | 44 +++++++++
 test/integration/BUILD                        | 60 ++++++------
 test/integration/http2_integration_test.h     |  2 +-
 .../http2_upstream_integration_test.h         |  3 +-
 test/integration/integration.cc               |  7 +-
 test/integration/integration.h                |  3 +-
 test/integration/integration_test.h           |  3 +-
 .../proxy_proto_integration_test.h            |  2 +-
 test/integration/ssl_integration_test.cc      |  4 +-
 test/integration/uds_integration_test.h       |  2 +-
 test/main.cc                                  | 31 +-----
 test/run_envoy_tests.sh                       | 13 ++-
 test/server/BUILD                             | 15 ++-
 test/server/server_test.cc                    |  4 +-
 test/test_common/BUILD                        |  4 +-
 test/test_common/environment.cc               | 44 ++++++++-
 test/test_common/environment.h                | 29 +++++-
 test/test_common/environment_sub.sh           |  2 +
 32 files changed, 306 insertions(+), 180 deletions(-)
 create mode 100755 bazel/gen_sh_test_runner.sh
 create mode 100644 test/common/ssl/ssl_certs_test.h
 create mode 100755 test/coverage/gen_build.sh

diff --git a/.gitignore b/.gitignore
index afa2f3df7815..0fc989004d77 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,6 +6,7 @@
 /ci/bazel-*
 /ci/prebuilt/thirdparty
 /ci/prebuilt/thirdparty_build
+/test/coverage/BUILD
 cscope.*
 BROWSE
 .deps
diff --git a/bazel/BUILD b/bazel/BUILD
index e69de29bb2d1..2484a9d0c319 100644
--- a/bazel/BUILD
+++ b/bazel/BUILD
@@ -0,0 +1,3 @@
+package(default_visibility = ["//visibility:public"])
+
+exports_files(["gen_sh_test_runner.sh"])
diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl
index 6057d86abb83..ca899c240497 100644
--- a/bazel/envoy_build_system.bzl
+++ b/bazel/envoy_build_system.bzl
@@ -40,8 +40,7 @@ def envoy_cc_library(name,
                      visibility = None,
                      external_deps = [],
                      repository = "",
-                     deps = [],
-                     alwayslink = None):
+                     deps = []):
     native.cc_library(
         name = name,
         srcs = srcs,
@@ -53,7 +52,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.
@@ -83,51 +82,33 @@ 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,
+        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 = [
+            repository + 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
@@ -139,7 +120,7 @@ def envoy_cc_test_library(name,
                           external_deps = [],
                           deps = [],
                           repository = "",
-                          alwayslink = None):
+                          tags = []):
     native.cc_library(
         name = name,
         srcs = srcs,
@@ -151,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"
diff --git a/bazel/gen_sh_test_runner.sh b/bazel/gen_sh_test_runner.sh
new file mode 100755
index 000000000000..f8de5f00af7c
--- /dev/null
+++ b/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
+)
diff --git a/source/server/config/http/BUILD b/source/server/config/http/BUILD
index 65caa0f29f36..4c0899a95927 100644
--- a/source/server/config/http/BUILD
+++ b/source/server/config/http/BUILD
@@ -12,7 +12,6 @@ envoy_cc_library(
         "//source/common/json:config_schemas_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -23,7 +22,6 @@ envoy_cc_library(
         "//source/common/dynamo:dynamo_filter_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -36,7 +34,6 @@ envoy_cc_library(
         "//source/common/json:config_schemas_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -48,7 +45,6 @@ envoy_cc_library(
         "//source/common/grpc:http1_bridge_filter_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -61,7 +57,6 @@ envoy_cc_library(
         "//source/common/http/filter:ratelimit_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -75,5 +70,4 @@ envoy_cc_library(
         "//source/common/router:shadow_writer_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
diff --git a/source/server/config/network/BUILD b/source/server/config/network/BUILD
index b7763ffd4294..5500e16993a0 100644
--- a/source/server/config/network/BUILD
+++ b/source/server/config/network/BUILD
@@ -12,7 +12,6 @@ envoy_cc_library(
         "//source/common/filter/auth:client_ssl_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -23,7 +22,6 @@ envoy_cc_library(
         "//source/common/filter:echo_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -49,7 +47,6 @@ envoy_cc_library(
         "//source/common/router:rds_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -63,7 +60,6 @@ envoy_cc_library(
         "//source/common/mongo:proxy_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -75,7 +71,6 @@ envoy_cc_library(
         "//source/common/filter:ratelimit_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -89,7 +84,6 @@ envoy_cc_library(
         "//source/common/redis:proxy_filter_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
 
 envoy_cc_library(
@@ -102,5 +96,4 @@ envoy_cc_library(
         "//source/common/filter:tcp_proxy_lib",
         "//source/server:configuration_lib",
     ],
-    alwayslink = 1,
 )
diff --git a/source/server/http/BUILD b/source/server/http/BUILD
index 6740f1230f1d..1b82b8fe6e5d 100644
--- a/source/server/http/BUILD
+++ b/source/server/http/BUILD
@@ -63,5 +63,4 @@ envoy_cc_library(
         "//source/common/json:json_loader_lib",
         "//source/server/config/network:http_connection_manager_lib",
     ],
-    alwayslink = 1,
 )
diff --git a/test/BUILD b/test/BUILD
index 22cdbec3327e..e742c94003bf 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -14,8 +14,7 @@ envoy_cc_test_library(
         "//source/common/common:version_lib",
         "//source/common/event:libevent_lib",
         "//source/common/ssl:openssl_lib",
-        "//source/server:options_lib",
-        "//test/integration:integration_lib",
+        "//test/test_common:environment_lib",
         "//test/test_common:printers_lib",
     ],
 )
diff --git a/test/common/runtime/BUILD b/test/common/runtime/BUILD
index b27d4207082a..66301184fb1c 100644
--- a/test/common/runtime/BUILD
+++ b/test/common/runtime/BUILD
@@ -12,11 +12,7 @@ filegroup(
 envoy_cc_test(
     name = "runtime_impl_test",
     srcs = ["runtime_impl_test.cc"],
-    data = glob(["test_data/**"]),
-    setup_cmds = [(
-        "filesystem_setup.sh",
-        [],
-    )],
+    data = glob(["test_data/**"]) + ["filesystem_setup.sh"],
     deps = [
         "//source/common/runtime:runtime_lib",
         "//source/common/stats:stats_lib",
diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc
index 9f1abc7dd60c..9540972d1083 100644
--- a/test/common/runtime/runtime_impl_test.cc
+++ b/test/common/runtime/runtime_impl_test.cc
@@ -36,6 +36,11 @@ TEST(UUID, sanityCheckOfUniqueness) {
 
 class RuntimeImplTest : public testing::Test {
 public:
+  static void SetUpTestCase() {
+    TestEnvironment::exec(
+        {TestEnvironment::runfilesPath("test/common/runtime/filesystem_setup.sh")});
+  }
+
   void setup(const std::string& primary_dir, const std::string& override_dir) {
     EXPECT_CALL(dispatcher, createFilesystemWatcher_())
         .WillOnce(ReturnNew<NiceMock<Filesystem::MockWatcher>>());
diff --git a/test/common/ssl/BUILD b/test/common/ssl/BUILD
index 0c58dd4ec7fb..906cab23b6fd 100644
--- a/test/common/ssl/BUILD
+++ b/test/common/ssl/BUILD
@@ -4,14 +4,14 @@ load("//bazel:envoy_build_system.bzl", "envoy_cc_test")
 
 envoy_cc_test(
     name = "connection_impl_test",
-    srcs = ["connection_impl_test.cc"],
+    srcs = [
+        "connection_impl_test.cc",
+        "ssl_certs_test.h",
+    ],
     data = [
+        "gen_unittest_certs.sh",
         "//test/common/ssl/test_data:certs",
     ],
-    setup_cmds = [(
-        "gen_unittest_certs.sh",
-        [],
-    )],
     deps = [
         "//source/common/buffer:buffer_lib",
         "//source/common/event:dispatcher_includes",
@@ -33,14 +33,14 @@ envoy_cc_test(
 
 envoy_cc_test(
     name = "context_impl_test",
-    srcs = ["context_impl_test.cc"],
+    srcs = [
+        "context_impl_test.cc",
+        "ssl_certs_test.h",
+    ],
     data = [
+        "gen_unittest_certs.sh",
         "//test/common/ssl/test_data:certs",
     ],
-    setup_cmds = [(
-        "gen_unittest_certs.sh",
-        [],
-    )],
     deps = [
         "//source/common/json:json_loader_lib",
         "//source/common/ssl:context_config_lib",
diff --git a/test/common/ssl/connection_impl_test.cc b/test/common/ssl/connection_impl_test.cc
index e6ff5c66fa86..c0481f86cfdb 100644
--- a/test/common/ssl/connection_impl_test.cc
+++ b/test/common/ssl/connection_impl_test.cc
@@ -9,6 +9,7 @@
 #include "common/ssl/context_impl.h"
 #include "common/stats/stats_impl.h"
 
+#include "test/common/ssl/ssl_certs_test.h"
 #include "test/mocks/network/mocks.h"
 #include "test/mocks/runtime/mocks.h"
 #include "test/mocks/server/mocks.h"
@@ -70,7 +71,9 @@ void testUtil(const std::string& client_ctx_json, const std::string& server_ctx_
 
 } // namespace
 
-TEST(SslConnectionImplTest, ClientAuth) {
+class SslConnectionImplTest : public SslCertsTest {};
+
+TEST_F(SslConnectionImplTest, ClientAuth) {
   std::string client_ctx_json = R"EOF(
   {
     "cert_chain_file": "test/common/ssl/test_data/approved_with_uri_san.crt",
@@ -146,7 +149,7 @@ TEST(SslConnectionImplTest, ClientAuth) {
            "2ff7d57d2e5cb9cc0bfe56727a114de8039cabcc7658715db4e80e1a75e108ed", "");
 }
 
-TEST(SslConnectionImplTest, ClientAuthBadVerification) {
+TEST_F(SslConnectionImplTest, ClientAuthBadVerification) {
   Stats::IsolatedStoreImpl stats_store;
   Runtime::MockLoader runtime;
 
@@ -203,7 +206,7 @@ TEST(SslConnectionImplTest, ClientAuthBadVerification) {
   dispatcher.run(Event::Dispatcher::RunType::Block);
 }
 
-TEST(SslConnectionImplTest, SslError) {
+TEST_F(SslConnectionImplTest, SslError) {
   Stats::IsolatedStoreImpl stats_store;
   Runtime::MockLoader runtime;
 
@@ -254,7 +257,7 @@ TEST(SslConnectionImplTest, SslError) {
   EXPECT_EQ(1UL, stats_store.counter("ssl.connection_error").value());
 }
 
-class SslReadBufferLimitTest : public testing::Test {
+class SslReadBufferLimitTest : public SslCertsTest {
 public:
   void readBufferLimitTest(uint32_t read_buffer_limit, uint32_t expected_chunk_size,
                            uint32_t write_size, uint32_t num_writes, bool reserve_write_space) {
diff --git a/test/common/ssl/context_impl_test.cc b/test/common/ssl/context_impl_test.cc
index 1e29c6465579..62c5098e4ff8 100644
--- a/test/common/ssl/context_impl_test.cc
+++ b/test/common/ssl/context_impl_test.cc
@@ -4,12 +4,15 @@
 #include "common/ssl/context_config_impl.h"
 #include "common/stats/stats_impl.h"
 
+#include "test/common/ssl/ssl_certs_test.h"
 #include "test/mocks/runtime/mocks.h"
 #include "test/test_common/environment.h"
 
 namespace Ssl {
 
-TEST(SslContextImplTest, TestdNSNameMatching) {
+class SslContextImplTest : public SslCertsTest {};
+
+TEST_F(SslContextImplTest, TestdNSNameMatching) {
   EXPECT_TRUE(ContextImpl::dNSNameMatch("lyft.com", "lyft.com"));
   EXPECT_TRUE(ContextImpl::dNSNameMatch("a.lyft.com", "*.lyft.com"));
   EXPECT_TRUE(ContextImpl::dNSNameMatch("a.b.lyft.com", "*.lyft.com"));
@@ -22,7 +25,7 @@ TEST(SslContextImplTest, TestdNSNameMatching) {
   EXPECT_FALSE(ContextImpl::dNSNameMatch("lyft.com", ""));
 }
 
-TEST(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) {
+TEST_F(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) {
   FILE* fp = fopen("test/common/ssl/test_data/san_dns.crt", "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
@@ -33,7 +36,7 @@ TEST(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) {
   fclose(fp);
 }
 
-TEST(SslContextImplTest, TestVerifySubjectAltNameURIMatched) {
+TEST_F(SslContextImplTest, TestVerifySubjectAltNameURIMatched) {
   FILE* fp = fopen("test/common/ssl/test_data/san_uri.crt", "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
@@ -45,7 +48,7 @@ TEST(SslContextImplTest, TestVerifySubjectAltNameURIMatched) {
   fclose(fp);
 }
 
-TEST(SslContextImplTest, TestVerifySubjectAltNameNotMatched) {
+TEST_F(SslContextImplTest, TestVerifySubjectAltNameNotMatched) {
   FILE* fp = fopen("test/common/ssl/test_data/san_dns.crt", "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
@@ -56,7 +59,7 @@ TEST(SslContextImplTest, TestVerifySubjectAltNameNotMatched) {
   fclose(fp);
 }
 
-TEST(SslContextImplTest, TestCipherSuites) {
+TEST_F(SslContextImplTest, TestCipherSuites) {
   std::string json = R"EOF(
   {
     "cipher_suites": "AES128-SHA:BOGUS:AES256-SHA"
@@ -71,7 +74,7 @@ TEST(SslContextImplTest, TestCipherSuites) {
   EXPECT_THROW(manager.createSslClientContext(store, cfg), EnvoyException);
 }
 
-TEST(SslContextImplTest, TestExpiringCert) {
+TEST_F(SslContextImplTest, TestExpiringCert) {
   std::string json = R"EOF(
   {
       "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
@@ -94,7 +97,7 @@ TEST(SslContextImplTest, TestExpiringCert) {
               14 == context->daysUntilFirstCertExpires());
 }
 
-TEST(SslContextImplTest, TestExpiredCert) {
+TEST_F(SslContextImplTest, TestExpiredCert) {
   std::string json = R"EOF(
   {
       "cert_chain_file": "{{ test_tmpdir }}/unittestcert_expired.pem",
@@ -111,7 +114,7 @@ TEST(SslContextImplTest, TestExpiredCert) {
   EXPECT_EQ(0U, context->daysUntilFirstCertExpires());
 }
 
-TEST(SslContextImplTest, TestGetCertInformation) {
+TEST_F(SslContextImplTest, TestGetCertInformation) {
   std::string json = R"EOF(
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
@@ -144,7 +147,7 @@ TEST(SslContextImplTest, TestGetCertInformation) {
               std::string::npos);
 }
 
-TEST(SslContextImplTest, TestNoCert) {
+TEST_F(SslContextImplTest, TestNoCert) {
   Json::ObjectPtr loader = TestEnvironment::jsonLoadFromString("{}");
   ContextConfigImpl cfg(*loader);
   Runtime::MockLoader runtime;
diff --git a/test/common/ssl/ssl_certs_test.h b/test/common/ssl/ssl_certs_test.h
new file mode 100644
index 000000000000..3a10544e6958
--- /dev/null
+++ b/test/common/ssl/ssl_certs_test.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#include "test/test_common/environment.h"
+
+class SslCertsTest : public testing::Test {
+public:
+  static void SetUpTestCase() {
+    TestEnvironment::exec({TestEnvironment::runfilesPath("test/common/ssl/gen_unittest_certs.sh")});
+  }
+};
diff --git a/test/coverage/gen_build.sh b/test/coverage/gen_build.sh
new file mode 100755
index 000000000000..5136beddeb33
--- /dev/null
+++ b/test/coverage/gen_build.sh
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+# Generate test/coverage/BUILD, which contains a single envoy_cc_test target
+# that contains all C++ based tests suitable for performing the coverage run. A
+# single binary (as opposed to multiple test targets) is require to work around
+# the crazy in https://github.com/bazelbuild/bazel/issues/1118. This is used by
+# the coverage runner script.
+
+set -e
+
+[ -z "${BAZEL_BIN}" ] && BAZEL_BIN=bazel
+[ -z "${BUILDIFIER_BIN}" ] && BUILDIFIER_BIN=buildifier
+
+BUILD_PATH="$(dirname "$0")"/BUILD
+
+TARGETS=$("${BAZEL_BIN}" query "attr('tags', 'coverage_test_lib', //test/...)")
+(
+  cat << EOF
+# This file is generated by test/coverage/gen_build.sh automatically prior to
+# coverage runs. It is under .gitignore. DO NOT EDIT, DO NOT CHECK IN.
+package(default_visibility = ["//visibility:public"])
+
+load("//bazel:envoy_build_system.bzl", "envoy_cc_test")
+
+envoy_cc_test(
+    name = "coverage_tests",
+    deps = [
+EOF
+  for t in ${TARGETS}
+  do
+    echo "        \"$t\","
+  done
+  cat << EOF
+    ],
+    tags = ["manual"],
+    coverage = False,
+    # Needed when invoking external shell tests etc.
+    local = True,
+)
+EOF
+
+) > "${BUILD_PATH}"
+
+"${BUILDIFIER_BIN}" "${BUILD_PATH}"
diff --git a/test/integration/BUILD b/test/integration/BUILD
index f590cac1268a..87ffcce1bd28 100644
--- a/test/integration/BUILD
+++ b/test/integration/BUILD
@@ -1,8 +1,8 @@
 package(default_visibility = ["//visibility:public"])
 
-load("//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_cc_test_library", "envoy_cc_test_with_json")
+load("//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_sh_test", "envoy_cc_test_library")
 
-sh_test(
+envoy_sh_test(
     name = "hotrestart_test",
     srcs = ["hotrestart_test.sh"],
     data = [
@@ -19,18 +19,16 @@ sh_test(
     local = 1,
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "http2_integration_test",
     srcs = [
         "http2_integration_test.cc",
         "http2_integration_test.h",
     ],
-    data = ["//test/config/integration:server_config_files"],
-    jsons = ["//test/config/integration:server_http2.json"],
-    setup_cmds = [(
-        "//test/common/runtime:filesystem_setup.sh",
-        [],
-    )],
+    data = [
+        "//test/config/integration:server_config_files",
+        "//test/config/integration:server_http2.json",
+    ],
     deps = [
         ":integration_lib",
         "//source/common/buffer:buffer_lib",
@@ -41,17 +39,13 @@ envoy_cc_test_with_json(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "http2_upstream_integration_test",
     srcs = [
         "http2_upstream_integration_test.cc",
         "http2_upstream_integration_test.h",
     ],
-    jsons = ["//test/config/integration:server_http2_upstream.json"],
-    setup_cmds = [(
-        "//test/common/runtime:filesystem_setup.sh",
-        [],
-    )],
+    data = ["//test/config/integration:server_http2_upstream.json"],
     deps = [
         ":integration_lib",
         "//source/common/http:header_map_lib",
@@ -59,18 +53,16 @@ envoy_cc_test_with_json(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "integration_admin_test",
     srcs = [
         "integration_admin_test.cc",
         "integration_test.h",
     ],
-    data = ["//test/config/integration:server_config_files"],
-    jsons = ["//test/config/integration:server.json"],
-    setup_cmds = [(
-        "//test/common/runtime:filesystem_setup.sh",
-        [],
-    )],
+    data = [
+        "//test/config/integration:server.json",
+        "//test/config/integration:server_config_files",
+    ],
     deps = [
         ":integration_lib",
         "//include/envoy/http:header_map_interface",
@@ -143,14 +135,16 @@ envoy_cc_test_library(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "integration_test",
     srcs = [
         "integration_test.cc",
         "integration_test.h",
     ],
-    data = ["//test/config/integration:server_config_files"],
-    jsons = ["//test/config/integration:server.json"],
+    data = [
+        "//test/config/integration:server.json",
+        "//test/config/integration:server_config_files",
+    ],
     deps = [
         ":integration_lib",
         "//source/common/http:header_map_lib",
@@ -158,13 +152,13 @@ envoy_cc_test_with_json(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "proxy_proto_integration_test",
     srcs = [
         "proxy_proto_integration_test.cc",
         "proxy_proto_integration_test.h",
     ],
-    jsons = ["//test/config/integration:server_proxy_proto.json"],
+    data = ["//test/config/integration:server_proxy_proto.json"],
     deps = [
         ":integration_lib",
         "//source/common/buffer:buffer_lib",
@@ -173,14 +167,16 @@ envoy_cc_test_with_json(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "ssl_integration_test",
     srcs = [
         "ssl_integration_test.cc",
         "ssl_integration_test.h",
     ],
-    data = ["//test/config/integration/certs"],
-    jsons = ["//test/config/integration:server_ssl.json"],
+    data = [
+        "//test/config/integration:server_ssl.json",
+        "//test/config/integration/certs",
+    ],
     deps = [
         ":integration_lib",
         "//source/common/event:dispatcher_includes",
@@ -192,13 +188,13 @@ envoy_cc_test_with_json(
     ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "uds_integration_test",
     srcs = [
         "uds_integration_test.cc",
         "uds_integration_test.h",
     ],
-    jsons = ["//test/config/integration:server_uds.json"],
+    data = ["//test/config/integration:server_uds.json"],
     deps = [
         ":integration_lib",
         "//source/common/event:dispatcher_includes",
diff --git a/test/integration/http2_integration_test.h b/test/integration/http2_integration_test.h
index 0683cc399540..f0dbddf55e72 100644
--- a/test/integration/http2_integration_test.h
+++ b/test/integration/http2_integration_test.h
@@ -12,7 +12,7 @@ class Http2IntegrationTest : public BaseIntegrationTest, public testing::Test {
     registerPort("upstream_0", fake_upstreams_.back()->localAddress()->ip()->port());
     fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP1));
     registerPort("upstream_1", fake_upstreams_.back()->localAddress()->ip()->port());
-    createTestServer("server_http2.json", {"echo", "http", "http_buffer"});
+    createTestServer("test/config/integration/server_http2.json", {"echo", "http", "http_buffer"});
   }
 
   /**
diff --git a/test/integration/http2_upstream_integration_test.h b/test/integration/http2_upstream_integration_test.h
index cdcd35712701..4170ccc770b6 100644
--- a/test/integration/http2_upstream_integration_test.h
+++ b/test/integration/http2_upstream_integration_test.h
@@ -12,7 +12,8 @@ class Http2UpstreamIntegrationTest : public BaseIntegrationTest, public testing:
     registerPort("upstream_0", fake_upstreams_.back()->localAddress()->ip()->port());
     fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP2));
     registerPort("upstream_1", fake_upstreams_.back()->localAddress()->ip()->port());
-    createTestServer("server_http2_upstream.json", {"http", "http_buffer", "http1_buffer"});
+    createTestServer("test/config/integration/server_http2_upstream.json",
+                     {"http", "http_buffer", "http1_buffer"});
   }
 
   /**
diff --git a/test/integration/integration.cc b/test/integration/integration.cc
index e5a32afbf671..ed0a36ac09a0 100644
--- a/test/integration/integration.cc
+++ b/test/integration/integration.cc
@@ -17,7 +17,6 @@
 
 IntegrationTestServerPtr BaseIntegrationTest::test_server_;
 std::vector<std::unique_ptr<FakeUpstream>> BaseIntegrationTest::fake_upstreams_;
-spdlog::level::level_enum BaseIntegrationTest::default_log_level_;
 
 IntegrationStreamDecoder::IntegrationStreamDecoder(Event::Dispatcher& dispatcher)
     : dispatcher_(dispatcher) {}
@@ -212,8 +211,8 @@ void IntegrationTcpClient::ConnectionCallbacks::onEvent(uint32_t events) {
 
 BaseIntegrationTest::BaseIntegrationTest()
     : api_(new Api::Impl(std::chrono::milliseconds(10000))),
-      dispatcher_(api_->allocateDispatcher()) {
-
+      dispatcher_(api_->allocateDispatcher()),
+      default_log_level_(TestEnvironment::getOptions().logLevel()) {
   // This is a hack, but there are situations where we disconnect fake upstream connections and
   // then we expect the server connection pool to get the disconnect before the next test starts.
   // This does not always happen. This pause should allow the server to pick up the disconnect
@@ -274,7 +273,7 @@ void BaseIntegrationTest::registerTestServerPorts(const std::vector<std::string>
 void BaseIntegrationTest::createTestServer(const std::string& json_path,
                                            const std::vector<std::string>& port_names) {
   test_server_ = IntegrationTestServer::create(
-      TestEnvironment::temporaryFileSubstitutePorts(json_path, port_map()));
+      TestEnvironment::temporaryFileSubstitute(json_path, port_map()));
   registerTestServerPorts(port_names);
 }
 
diff --git a/test/integration/integration.h b/test/integration/integration.h
index 261c1b0ed094..b52061847dfd 100644
--- a/test/integration/integration.h
+++ b/test/integration/integration.h
@@ -166,7 +166,6 @@ class BaseIntegrationTest : Logger::Loggable<Logger::Id::testing> {
 
   static IntegrationTestServerPtr test_server_;
   static std::vector<std::unique_ptr<FakeUpstream>> fake_upstreams_;
-  static spdlog::level::level_enum default_log_level_;
 
   Api::ApiPtr api_;
   Event::DispatcherPtr dispatcher_;
@@ -208,4 +207,6 @@ class BaseIntegrationTest : Logger::Loggable<Logger::Id::testing> {
     static auto* port_map = new TestEnvironment::PortMap();
     return *port_map;
   }
+
+  spdlog::level::level_enum default_log_level_;
 };
diff --git a/test/integration/integration_test.h b/test/integration/integration_test.h
index da803eb596de..7445435e8b11 100644
--- a/test/integration/integration_test.h
+++ b/test/integration/integration_test.h
@@ -12,7 +12,8 @@ class IntegrationTest : public BaseIntegrationTest, public testing::Test {
     registerPort("upstream_0", fake_upstreams_.back()->localAddress()->ip()->port());
     fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP1));
     registerPort("upstream_1", fake_upstreams_.back()->localAddress()->ip()->port());
-    createTestServer("server.json", {"echo", "http", "http_buffer", "tcp_proxy", "rds"});
+    createTestServer("test/config/integration/server.json",
+                     {"echo", "http", "http_buffer", "tcp_proxy", "rds"});
   }
 
   /**
diff --git a/test/integration/proxy_proto_integration_test.h b/test/integration/proxy_proto_integration_test.h
index 55dc313c0f8d..851aea175d6b 100644
--- a/test/integration/proxy_proto_integration_test.h
+++ b/test/integration/proxy_proto_integration_test.h
@@ -15,7 +15,7 @@ class ProxyProtoIntegrationTest : public BaseIntegrationTest, public testing::Te
   static void SetUpTestCase() {
     fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP1));
     registerPort("upstream_0", fake_upstreams_.back()->localAddress()->ip()->port());
-    createTestServer("server_proxy_proto.json", {"http"});
+    createTestServer("test/config/integration/server_proxy_proto.json", {"http"});
   }
 
   /**
diff --git a/test/integration/ssl_integration_test.cc b/test/integration/ssl_integration_test.cc
index 4649a1fdaa8e..948396273322 100644
--- a/test/integration/ssl_integration_test.cc
+++ b/test/integration/ssl_integration_test.cc
@@ -29,8 +29,8 @@ void SslIntegrationTest::SetUpTestCase() {
   fake_upstreams_.emplace_back(
       new FakeUpstream(upstream_ssl_ctx_.get(), 0, FakeHttpConnection::Type::HTTP1));
   registerPort("upstream_1", fake_upstreams_.back()->localAddress()->ip()->port());
-  test_server_ = MockRuntimeIntegrationTestServer::create(
-      TestEnvironment::temporaryFileSubstitutePorts("server_ssl.json", port_map()));
+  test_server_ = MockRuntimeIntegrationTestServer::create(TestEnvironment::temporaryFileSubstitute(
+      "test/config/integration/server_ssl.json", port_map()));
   registerTestServerPorts({"http"});
   client_ssl_ctx_plain_ = createClientSslContext(false, false);
   client_ssl_ctx_alpn_ = createClientSslContext(true, false);
diff --git a/test/integration/uds_integration_test.h b/test/integration/uds_integration_test.h
index 4692dfd3dbbc..68b18f86fbc4 100644
--- a/test/integration/uds_integration_test.h
+++ b/test/integration/uds_integration_test.h
@@ -18,7 +18,7 @@ class UdsIntegrationTest : public BaseIntegrationTest, public testing::Test {
         TestEnvironment::unixDomainSocketPath("udstest.1.sock"), FakeHttpConnection::Type::HTTP1));
     fake_upstreams_.emplace_back(new FakeUpstream(
         TestEnvironment::unixDomainSocketPath("udstest.2.sock"), FakeHttpConnection::Type::HTTP1));
-    createTestServer("server_uds.json", {"http"});
+    createTestServer("test/config/integration/server_uds.json", {"http"});
   }
 
   /**
diff --git a/test/main.cc b/test/main.cc
index ba2f1d0b9dce..d9698110359f 100644
--- a/test/main.cc
+++ b/test/main.cc
@@ -1,5 +1,3 @@
-#include "server/options_impl.h"
-
 #include "common/common/logger.h"
 #include "common/common/thread.h"
 #include "common/event/libevent.h"
@@ -8,7 +6,6 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#include "test/integration/integration.h"
 #include "test/test_common/environment.h"
 
 int main(int argc, char** argv) {
@@ -28,33 +25,9 @@ int main(int argc, char** argv) {
     ::exit(1);
   }
 
-  // Quick and dirty filtering and execution of --envoy_test_setup. Scripts run in this environment
-  // have access to env vars such as TEST_TMPDIR. This script can be used for situations where
-  // genrules are unsuitable for unit tests, since genrules don't have access to TEST_TMPDIR.
-  // TODO(rlazarus): Consider adding a TestOptionsImpl subclass of OptionsImpl during the #613 work,
-  // where we will be doing some additional command-line parsing in OptionsImpl anyway.
-  const std::string envoy_test_setup_flag = "--envoy_test_setup";
-  int i;
-  for (i = 0; i < argc; ++i) {
-    const std::string arg(argv[i]);
-    if (arg.find(envoy_test_setup_flag) == 0) {
-      if (::system(arg.substr(envoy_test_setup_flag.size() + 1).c_str()) != 0) {
-        std::cerr << "Failed to execute setup script " << arg << std::endl;
-        ::exit(1);
-      }
-      for (; i < argc - 1; ++i) {
-        argv[i] = argv[i + 1];
-      }
-      --argc;
-      break;
-    }
-  }
-
-  OptionsImpl options(argc, argv, "1", spdlog::level::err);
+  TestEnvironment::initializeOptions(argc, argv);
   Thread::MutexBasicLockable lock;
-  Logger::Registry::initialize(options.logLevel(), lock);
-  BaseIntegrationTest::default_log_level_ =
-      static_cast<spdlog::level::level_enum>(options.logLevel());
+  Logger::Registry::initialize(TestEnvironment::getOptions().logLevel(), lock);
 
   return RUN_ALL_TESTS();
 }
diff --git a/test/run_envoy_tests.sh b/test/run_envoy_tests.sh
index d18266725e21..0bcf473e4e66 100755
--- a/test/run_envoy_tests.sh
+++ b/test/run_envoy_tests.sh
@@ -45,10 +45,15 @@ do
 done
 
 # Some hacks for the runtime test filesystem. These go away in the Bazel build.
-TEST_RUNTIME_DIR="$TEST_SRCDIR/$TEST_WORKSPACE"/test/common/runtime/test_data
-mkdir -p "$TEST_RUNTIME_DIR"
-cp -r "$SOURCE_DIR"/test/common/runtime/test_data/* "$TEST_RUNTIME_DIR"
-"$SOURCE_DIR"/test/common/runtime/filesystem_setup.sh
+TEST_RUNTIME_DIR="$TEST_SRCDIR/$TEST_WORKSPACE"/test/common/runtime
+mkdir -p "$TEST_RUNTIME_DIR/test_data"
+cp -r "$SOURCE_DIR"/test/common/runtime/test_data/* "$TEST_RUNTIME_DIR"/test_data
+cp "$SOURCE_DIR"/test/common/runtime/filesystem_setup.sh "$TEST_RUNTIME_DIR"
+
+# Some hacks for the SSL cert generation. These go away in the Bazel build.
+TEST_SSL_DIR="$TEST_SRCDIR/$TEST_WORKSPACE"/test/common/ssl
+mkdir -p "$TEST_SSL_DIR"
+cp "$SOURCE_DIR"/test/common/ssl/gen_unittest_certs.sh "$TEST_SSL_DIR"
 
 if [ -n "$EXTRA_SETUP_SCRIPT" ]; then
   $EXTRA_SETUP_SCRIPT
diff --git a/test/server/BUILD b/test/server/BUILD
index 8239f24e6e01..6b0bbc66b405 100644
--- a/test/server/BUILD
+++ b/test/server/BUILD
@@ -1,6 +1,6 @@
 package(default_visibility = ["//visibility:public"])
 
-load("//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_cc_test_with_json")
+load("//bazel:envoy_build_system.bzl", "envoy_cc_test")
 
 envoy_cc_test(
     name = "configuration_impl_test",
@@ -56,14 +56,19 @@ envoy_cc_test(
 envoy_cc_test(
     name = "options_impl_test",
     srcs = ["options_impl_test.cc"],
-    deps = ["//source/server:options_lib"],
+    deps = [
+        "//source/common/common:utility_lib",
+        "//source/server:options_lib",
+    ],
 )
 
-envoy_cc_test_with_json(
+envoy_cc_test(
     name = "server_test",
     srcs = ["server_test.cc"],
-    data = ["//test/config/integration:server_config_files"],
-    jsons = ["//test/config/integration:server.json"],
+    data = [
+        "//test/config/integration:server.json",
+        "//test/config/integration:server_config_files",
+    ],
     deps = [
         "//source/server:server_lib",
         "//test/integration:integration_lib",
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index a2ad7c55f1a6..4c3d2f499b0b 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -48,8 +48,8 @@ class TestComponentFactory : public ComponentFactory {
 class ServerInstanceImplTest : public testing::Test {
 protected:
   ServerInstanceImplTest()
-      : options_(TestEnvironment::temporaryFileSubstitutePorts(
-            "server.json", {{"upstream_0", 0}, {"upstream_1", 0}})),
+      : options_(TestEnvironment::temporaryFileSubstitute("test/config/integration/server.json",
+                                                          {{"upstream_0", 0}, {"upstream_1", 0}})),
         server_(options_, hooks_, restart_, stats_store_, fakelock_, component_factory_,
                 local_info_) {}
   void TearDown() override {
diff --git a/test/test_common/BUILD b/test/test_common/BUILD
index 6cbb6361b7a7..eeac1cce8af3 100644
--- a/test/test_common/BUILD
+++ b/test/test_common/BUILD
@@ -2,8 +2,6 @@ package(default_visibility = ["//visibility:public"])
 
 load("//bazel:envoy_build_system.bzl", "envoy_cc_library", "envoy_cc_test_library")
 
-exports_files(["environment_sub.sh"])
-
 cc_library(
     name = "printers_includes",
     hdrs = ["printers.h"],
@@ -14,8 +12,10 @@ envoy_cc_test_library(
     srcs = ["environment.cc"],
     hdrs = ["environment.h"],
     deps = [
+        "//include/envoy/server:options_interface",
         "//source/common/common:assert_lib",
         "//source/common/json:json_loader_lib",
+        "//source/server:options_lib",
     ],
 )
 
diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc
index 735cf15473f8..9d9cf7e1eb01 100644
--- a/test/test_common/environment.cc
+++ b/test/test_common/environment.cc
@@ -2,6 +2,8 @@
 
 #include "common/common/assert.h"
 
+#include "server/options_impl.h"
+
 namespace {
 
 std::string getCheckedEnvVar(const std::string& var) {
@@ -24,8 +26,22 @@ std::string getOrCreateUnixDomainSocketDirectory() {
   return std::string(test_udsdir);
 }
 
+// Allow initializeOptions() to remember CLI args for getOptions().
+int argc_;
+char** argv_;
+
 } // namespace
 
+void TestEnvironment::initializeOptions(int argc, char** argv) {
+  argc_ = argc;
+  argv_ = argv;
+}
+
+Server::Options& TestEnvironment::getOptions() {
+  static OptionsImpl* options = new OptionsImpl(argc_, argv_, "1", spdlog::level::err);
+  return *options;
+}
+
 const std::string& TestEnvironment::temporaryDirectory() {
   static const std::string* temporary_directory = new std::string(getCheckedEnvVar("TEST_TMPDIR"));
   return *temporary_directory;
@@ -47,11 +63,11 @@ std::string TestEnvironment::substitute(const std::string str) {
   return std::regex_replace(str, test_cert_regex, TestEnvironment::temporaryDirectory());
 }
 
-std::string TestEnvironment::temporaryFileSubstitutePorts(const std::string& path,
-                                                          const PortMap& port_map) {
+std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,
+                                                     const PortMap& port_map) {
   // Load the entire file as a string, regex replace one at a time and write it back out. Proper
   // templating might be better one day, but this works for now.
-  const std::string tmp_json_path = TestEnvironment::temporaryPath(path);
+  const std::string tmp_json_path = TestEnvironment::runfilesPath(path);
   std::string out_json_string;
   {
     std::ifstream file(tmp_json_path);
@@ -59,10 +75,21 @@ std::string TestEnvironment::temporaryFileSubstitutePorts(const std::string& pat
     file_string_stream << file.rdbuf();
     out_json_string = file_string_stream.str();
   }
+  // Substitute ports.
   for (auto it : port_map) {
     const std::regex port_regex("\\{\\{ " + it.first + " \\}\\}");
     out_json_string = std::regex_replace(out_json_string, port_regex, std::to_string(it.second));
   }
+  // Substitute paths.
+  const std::unordered_map<std::string, std::string> path_map = {
+      {"test_tmpdir", TestEnvironment::temporaryDirectory()},
+      {"test_udsdir", TestEnvironment::unixDomainSocketDirectory()},
+      {"test_srcdir", TestEnvironment::runfilesDirectory()},
+  };
+  for (auto it : path_map) {
+    const std::regex port_regex("\\{\\{ " + it.first + " \\}\\}");
+    out_json_string = std::regex_replace(out_json_string, port_regex, it.second);
+  }
   const std::string out_json_path = tmp_json_path + ".with.ports.json";
   {
     std::ofstream out_json_file(out_json_path);
@@ -74,3 +101,14 @@ std::string TestEnvironment::temporaryFileSubstitutePorts(const std::string& pat
 Json::ObjectPtr TestEnvironment::jsonLoadFromString(const std::string& json) {
   return Json::Factory::LoadFromString(substitute(json));
 }
+
+void TestEnvironment::exec(const std::vector<std::string>& args) {
+  std::stringstream cmd;
+  for (auto& arg : args) {
+    cmd << arg << " ";
+  }
+  if (::system(cmd.str().c_str()) != 0) {
+    std::cerr << "Failed " << cmd.str() << "\n";
+    RELEASE_ASSERT(false);
+  }
+}
diff --git a/test/test_common/environment.h b/test/test_common/environment.h
index b958fa5f996b..fc595711c32a 100644
--- a/test/test_common/environment.h
+++ b/test/test_common/environment.h
@@ -1,11 +1,26 @@
 #pragma once
 
+#include "envoy/server/options.h"
+
 #include "common/json/json_loader.h"
 
 class TestEnvironment {
 public:
   typedef std::unordered_map<std::string, uint32_t> PortMap;
 
+  /**
+   * Initialize command-line options for later access by tests in getOptions().
+   * @param argc number of command-line args.
+   * @param argv array of command-line args.
+   */
+  static void initializeOptions(int argc, char** argv);
+
+  /**
+   * Obtain command-line options reference.
+   * @return Server::Options& with command-line options.
+   */
+  static Server::Options& getOptions();
+
   /**
    * Obtain a private writable temporary directory.
    * @return const std::string& with the path to the temporary directory.
@@ -59,12 +74,12 @@ class TestEnvironment {
   static std::string substitute(const std::string str);
 
   /**
-   * Substitue ports in a JSON file in the private writable test temporary directory.
-   * @param path path prefix for the input file with port templates.
+   * Substitue ports and paths in a JSON file in the private writable test temporary directory.
+   * @param path path prefix for the input file with port and path templates.
    * @param port_map map from port name to port number.
-   * @return std::string path prefix for the generated file.
+   * @return std::string path for the generated file.
    */
-  static std::string temporaryFileSubstitutePorts(const std::string& path, const PortMap& port_map);
+  static std::string temporaryFileSubstitute(const std::string& path, const PortMap& port_map);
 
   /**
    * Build JSON object from a string subject to environment path substitution.
@@ -72,4 +87,10 @@ class TestEnvironment {
    * @return Json::ObjectPtr with built JSON object.
    */
   static Json::ObjectPtr jsonLoadFromString(const std::string& json);
+
+  /**
+   * Execute a program under ::system. Any failure is fatal.
+   * @param args program path and arguments.
+   */
+  static void exec(const std::vector<std::string>& args);
 };
diff --git a/test/test_common/environment_sub.sh b/test/test_common/environment_sub.sh
index a3f015e63283..852985d62f1f 100755
--- a/test/test_common/environment_sub.sh
+++ b/test/test_common/environment_sub.sh
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+# TOOD(htuch): This is now just for run_envoy_tests.sh, remove when no more cmake.
+
 JSON=$1
 SRC_FILE="${TEST_SRCDIR}/${TEST_WORKSPACE}/${JSON}"
 DST_FILE="${TEST_TMPDIR}"/"$(basename "${JSON}")"

From 3d878d55e05a554514305f26101bd6ad15a4a602 Mon Sep 17 00:00:00 2001
From: htuch <htuch@users.noreply.github.com>
Date: Tue, 11 Apr 2017 11:23:00 -0400
Subject: [PATCH 11/12] bazel: add missing redis command_splitter_impl_test
 target. (#731)

This showed up in coverage, pushing it below 98% in the Bazel coverage runs. I think that we'll be
able to easily catch elided tests when we enable Bazel coverage in CI.
---
 test/common/redis/BUILD | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/common/redis/BUILD b/test/common/redis/BUILD
index a1e789f2b291..fccbf2e10942 100644
--- a/test/common/redis/BUILD
+++ b/test/common/redis/BUILD
@@ -14,6 +14,17 @@ envoy_cc_test(
     ],
 )
 
+envoy_cc_test(
+    name = "command_splitter_impl_test",
+    srcs = ["command_splitter_impl_test.cc"],
+    deps = [
+        "//source/common/redis:command_splitter_lib",
+        "//source/common/stats:stats_lib",
+        "//test/mocks:common_lib",
+        "//test/mocks/redis:redis_mocks",
+    ],
+)
+
 envoy_cc_test(
     name = "conn_pool_impl_test",
     srcs = ["conn_pool_impl_test.cc"],

From 6f2a5a50b6b3e2c0b406a4fd0287873938ad4551 Mon Sep 17 00:00:00 2001
From: Matt Klein <mattklein123@users.noreply.github.com>
Date: Tue, 11 Apr 2017 10:29:17 -0700
Subject: [PATCH 12/12] bazel: minor fixes for consuming projects (#732)

Thanks to @htuch for debugging help.
---
 bazel/envoy_build_system.bzl    | 3 ++-
 test/test_common/environment.cc | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl
index ca899c240497..9e0c35c519d3 100644
--- a/bazel/envoy_build_system.bzl
+++ b/bazel/envoy_build_system.bzl
@@ -96,6 +96,7 @@ def envoy_cc_test(name,
         srcs = srcs,
         data = data,
         deps = deps,
+        repository = repository,
         tags = test_lib_tags,
     )
     native.cc_test(
@@ -104,7 +105,7 @@ def envoy_cc_test(name,
         linkopts = ["-pthread"],
         linkstatic = 1,
         deps = [
-            repository + name + "_lib",
+            ":" + name + "_lib",
             repository + "//test:main"
         ],
         tags = tags + ["coverage_test"],
diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc
index 9d9cf7e1eb01..63cb24eeb57f 100644
--- a/test/test_common/environment.cc
+++ b/test/test_common/environment.cc
@@ -71,6 +71,11 @@ std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,
   std::string out_json_string;
   {
     std::ifstream file(tmp_json_path);
+    if (file.fail()) {
+      std::cerr << "failed to open: " << tmp_json_path << std::endl;
+      RELEASE_ASSERT(false);
+    }
+
     std::stringstream file_string_stream;
     file_string_stream << file.rdbuf();
     out_json_string = file_string_stream.str();