test: runfiles path fixups to support site specific (e.g. google) imp…

STYLE.md

@@ -41,6 +41,26 @@
 * There are probably a few other things missing from this list. We will add them as they
   are brought to our attention.
 
+# Hermetic tests
+
+Tests should be hermetic, i.e. have all dependencies explicitly captured and not depend on the local
+environment. In general, there should be no non-local network access. In addition:
+
+* Port numbers should not be hardcoded. Tests should bind to port zero and then discover the bound
+  port when needed. This avoids flakes due to conflicting ports and allows tests to be executed
+  concurrently by Bazel. See
+  [`test/integration/integration_test.h`](test/integration/integration_test.h) and
+  [`test/common/network/listener_impl_test.cc`](test/common/network/listener_impl_test.cc)
+  for examples of tests that do this.
+
+* Paths should be constructed using:
+  * The methods in [`TestEnvironment`](test/test_common/environment.h) for C++ tests.
+  * With `${TEST_TMPDIR}` (for writable temporary space) or `${TEST_RUNDIR}` for read-only access to
+    test inputs in shell tests.
+  * With `{{ test_tmpdir }}`, `{{ test_rundir }}` and `{{ test_udsdir }}` respectively for JSON templates.
+    `{{ test_udsdir }}` is provided for pathname based Unix Domain Sockets, which must fit within a
+    108 character limit on Linux, a property that might not hold for `{{ test_tmpdir }}`.
+
 # Google style guides for other languages
 
 * [Python](https://google.github.io/styleguide/pyguide.html)

bazel/BUILD

@@ -1,6 +1,9 @@
 package(default_visibility = ["//visibility:public"])
 
-exports_files(["gen_sh_test_runner.sh"])
+exports_files([
+    "gen_sh_test_runner.sh",
+    "sh_test_wrapper.sh",
+])
 
 config_setting(
     name = "opt_build",

bazel/envoy_build_system.bzl

@@ -233,8 +233,9 @@ def envoy_sh_test(name,
   )
   native.sh_test(
       name = name,
-      srcs = srcs,
+      srcs = ["//bazel:sh_test_wrapper.sh"],
       data = srcs + data,
+      args = ["$(location " + srcs[0] + ")"],
       **kargs
   )
 

bazel/sh_test_wrapper.sh

@@ -0,0 +1,6 @@
+#!/bin/bash
+
+# Where the runfiles are for tests.
+export TEST_RUNDIR="${TEST_SRCDIR}/${TEST_WORKSPACE}"
+
+"$@"

test/BUILD

@@ -4,7 +4,10 @@ load("//bazel:envoy_build_system.bzl", "envoy_cc_test_library")
 
 envoy_cc_test_library(
     name = "main",
-    srcs = ["main.cc"],
+    srcs = [
+        "main.cc",
+        "test_runner.h",
+    ],
     external_deps = [
         "protobuf",
     ],

test/common/filter/auth/BUILD

@@ -17,6 +17,7 @@ envoy_cc_test(
         "//test/mocks/ssl:ssl_mocks",
         "//test/mocks/thread_local:thread_local_mocks",
         "//test/mocks/upstream:upstream_mocks",
+        "//test/test_common:environment_lib",
         "//test/test_common:utility_lib",
     ],
 )

test/common/filter/auth/client_ssl_test.cc

@@ -12,6 +12,7 @@
 #include "test/mocks/ssl/mocks.h"
 #include "test/mocks/thread_local/mocks.h"
 #include "test/mocks/upstream/mocks.h"
+#include "test/test_common/environment.h"
 #include "test/test_common/printers.h"
 #include "test/test_common/utility.h"
 
@@ -156,8 +157,8 @@ TEST_F(ClientSslAuthFilterTest, Ssl) {
   EXPECT_CALL(*interval_timer_, enableTimer(_));
   Http::MessagePtr message(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/filter/auth/test_data/vpn_response_1.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/filter/auth/test_data/vpn_response_1.json"))));
   callbacks_->onSuccess(std::move(message));
   EXPECT_EQ(1U, stats_store_.gauge("auth.clientssl.vpn.total_principals").value());
 

test/common/runtime/filesystem_setup.sh

@@ -2,7 +2,7 @@
 
 set -e
 
-cd ${TEST_SRCDIR}/${TEST_WORKSPACE}
+cd ${TEST_RUNDIR}
 cp -rfL --parents test/common/runtime/test_data ${TEST_TMPDIR}
 ln -sf ${TEST_TMPDIR}/test/common/runtime/test_data/root ${TEST_TMPDIR}/test/common/runtime/test_data/current
 ln -sf ${TEST_TMPDIR}/test/common/runtime/test_data/root/envoy/subdir ${TEST_TMPDIR}/test/common/runtime/test_data/root/envoy/badlink

test/common/ssl/connection_impl_test.cc

@@ -85,16 +85,16 @@ class SslConnectionImplTest : public SslCertsTest {};
 TEST_F(SslConnectionImplTest, GetCertDigest) {
   std::string client_ctx_json = R"EOF(
   {
-    "cert_chain_file": "test/common/ssl/test_data/no_san_cert.pem",
-    "private_key_file": "test/common/ssl/test_data/no_san_key.pem"
+    "cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_cert.pem",
+    "private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_key.pem"
   }
   )EOF";
 
   std::string server_ctx_json = R"EOF(
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem"
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem"
   }
   )EOF";
 
@@ -105,16 +105,16 @@ TEST_F(SslConnectionImplTest, GetCertDigest) {
 TEST_F(SslConnectionImplTest, GetUriWithUriSan) {
   std::string client_ctx_json = R"EOF(
   {
-    "cert_chain_file": "test/common/ssl/test_data/san_uri_cert.pem",
-    "private_key_file": "test/common/ssl/test_data/san_uri_key.pem"
+    "cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/san_uri_cert.pem",
+    "private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/san_uri_key.pem"
   }
   )EOF";
 
   std::string server_ctx_json = R"EOF(
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem",
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem",
     "verify_subject_alt_name": [ "istio:account1.foo.cluster.local" ]
   }
   )EOF";
@@ -125,16 +125,16 @@ TEST_F(SslConnectionImplTest, GetUriWithUriSan) {
 TEST_F(SslConnectionImplTest, GetNoUriWithDnsSan) {
   std::string client_ctx_json = R"EOF(
   {
-    "cert_chain_file": "test/common/ssl/test_data/san_dns_cert.pem",
-    "private_key_file": "test/common/ssl/test_data/san_dns_key.pem"
+    "cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/san_dns_cert.pem",
+    "private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/san_dns_key.pem"
   }
   )EOF";
 
   std::string server_ctx_json = R"EOF(
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem"
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem"
   }
   )EOF";
 
@@ -168,7 +168,7 @@ TEST_F(SslConnectionImplTest, ClientAuthBadVerification) {
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem",
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem",
     "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"
   }
   )EOF";
@@ -188,8 +188,8 @@ TEST_F(SslConnectionImplTest, ClientAuthBadVerification) {
 
   std::string client_ctx_json = R"EOF(
   {
-    "cert_chain_file": "test/common/ssl/test_data/no_san_cert.pem",
-    "private_key_file": "test/common/ssl/test_data/no_san_key.pem"
+    "cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_cert.pem",
+    "private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_key.pem"
   }
   )EOF";
 
@@ -225,7 +225,7 @@ TEST_F(SslConnectionImplTest, SslError) {
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem",
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem",
     "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"
   }
   )EOF";
@@ -282,7 +282,7 @@ class SslReadBufferLimitTest : public SslCertsTest {
     {
       "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
       "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-      "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem"
+      "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem"
     }
     )EOF";
     Json::ObjectPtr server_ctx_loader = TestEnvironment::jsonLoadFromString(server_ctx_json);
@@ -300,8 +300,8 @@ class SslReadBufferLimitTest : public SslCertsTest {
 
     std::string client_ctx_json = R"EOF(
     {
-      "cert_chain_file": "test/common/ssl/test_data/no_san_cert.pem",
-      "private_key_file": "test/common/ssl/test_data/no_san_key.pem"
+      "cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_cert.pem",
+      "private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_key.pem"
     }
     )EOF";
 

test/common/ssl/context_impl_test.cc

@@ -30,7 +30,8 @@ TEST_F(SslContextImplTest, TestdNSNameMatching) {
 }
 
 TEST_F(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) {
-  FILE* fp = fopen("test/common/ssl/test_data/san_dns_cert.pem", "r");
+  FILE* fp = fopen(
+      TestEnvironment::runfilesPath("test/common/ssl/test_data/san_dns_cert.pem").c_str(), "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
   EXPECT_NE(cert, nullptr);
@@ -42,7 +43,8 @@ TEST_F(SslContextImplTest, TestVerifySubjectAltNameDNSMatched) {
 }
 
 TEST_F(SslContextImplTest, TestVerifySubjectAltNameURIMatched) {
-  FILE* fp = fopen("test/common/ssl/test_data/san_uri_cert.pem", "r");
+  FILE* fp = fopen(
+      TestEnvironment::runfilesPath("test/common/ssl/test_data/san_uri_cert.pem").c_str(), "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
   EXPECT_NE(cert, nullptr);
@@ -54,7 +56,8 @@ TEST_F(SslContextImplTest, TestVerifySubjectAltNameURIMatched) {
 }
 
 TEST_F(SslContextImplTest, TestVerifySubjectAltNameNotMatched) {
-  FILE* fp = fopen("test/common/ssl/test_data/san_dns_cert.pem", "r");
+  FILE* fp = fopen(
+      TestEnvironment::runfilesPath("test/common/ssl/test_data/san_dns_cert.pem").c_str(), "r");
   EXPECT_NE(fp, nullptr);
   X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
   EXPECT_NE(cert, nullptr);
@@ -124,7 +127,7 @@ TEST_F(SslContextImplTest, TestGetCertInformation) {
   {
     "cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
     "private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
-    "ca_cert_file": "test/common/ssl/test_data/ca_cert.pem"
+    "ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem"
   }
   )EOF";
 
@@ -141,9 +144,10 @@ TEST_F(SslContextImplTest, TestGetCertInformation) {
   // For the cert_chain, it is dynamically created when we run_envoy_test.sh which changes the
   // serial number with
   // every build. For cert_chain output, we check only for the certificate path.
-  std::string ca_cert_partial_output(
-      "Certificate Path: test/common/ssl/test_data/ca_cert.pem, Serial Number: b776a798802a1dcd, "
-      "Days until Expiration: ");
+  std::string ca_cert_partial_output(TestEnvironment::substitute(
+      "Certificate Path: {{ test_rundir }}/test/common/ssl/test_data/ca_cert.pem, Serial Number: "
+      "b776a798802a1dcd, "
+      "Days until Expiration: "));
   std::string cert_chain_partial_output(
       TestEnvironment::substitute("Certificate Path: {{ test_tmpdir }}/unittestcert.pem"));
 

test/common/upstream/BUILD

@@ -165,6 +165,7 @@ envoy_cc_test(
         "//test/mocks/runtime:runtime_mocks",
         "//test/mocks/ssl:ssl_mocks",
         "//test/mocks/upstream:upstream_mocks",
+        "//test/test_common:environment_lib",
         "//test/test_common:utility_lib",
     ],
 )

test/common/upstream/sds_test.cc

@@ -14,6 +14,7 @@
 #include "test/mocks/runtime/mocks.h"
 #include "test/mocks/ssl/mocks.h"
 #include "test/mocks/upstream/mocks.h"
+#include "test/test_common/environment.h"
 #include "test/test_common/printers.h"
 #include "test/test_common/utility.h"
 
@@ -132,8 +133,8 @@ TEST_F(SdsTest, NoHealthChecker) {
 
   Http::MessagePtr message(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response.json"))));
 
   EXPECT_CALL(*timer_, enableTimer(_));
   callbacks_->onSuccess(std::move(message));
@@ -160,8 +161,9 @@ TEST_F(SdsTest, NoHealthChecker) {
 
   message.reset(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response_weight_change.json")));
+  message->body().reset(
+      new Buffer::OwnedImpl(Filesystem::fileReadToEnd(TestEnvironment::runfilesPath(
+          "test/common/upstream/test_data/sds_response_weight_change.json"))));
   EXPECT_CALL(*timer_, enableTimer(_));
   callbacks_->onSuccess(std::move(message));
   EXPECT_EQ(13UL, cluster_->hosts().size());
@@ -220,8 +222,8 @@ TEST_F(SdsTest, HealthChecker) {
   // all the hosts to load in unhealthy.
   Http::MessagePtr message(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response.json"))));
 
   EXPECT_CALL(*timer_, enableTimer(_));
   callbacks_->onSuccess(std::move(message));
@@ -266,8 +268,8 @@ TEST_F(SdsTest, HealthChecker) {
   EXPECT_CALL(*timer_, enableTimer(_));
   message.reset(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response_2.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_2.json"))));
   callbacks_->onSuccess(std::move(message));
   EXPECT_EQ(14UL, cluster_->hosts().size());
   EXPECT_EQ(13UL, cluster_->healthyHosts().size());
@@ -286,8 +288,8 @@ TEST_F(SdsTest, HealthChecker) {
   EXPECT_CALL(*timer_, enableTimer(_));
   message.reset(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response_2.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_2.json"))));
   callbacks_->onSuccess(std::move(message));
   EXPECT_EQ(13UL, cluster_->hosts().size());
   EXPECT_EQ(12UL, cluster_->healthyHosts().size());
@@ -305,8 +307,8 @@ TEST_F(SdsTest, HealthChecker) {
   EXPECT_CALL(*timer_, enableTimer(_));
   message.reset(new Http::ResponseMessageImpl(
       Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}));
-  message->body().reset(new Buffer::OwnedImpl(
-      Filesystem::fileReadToEnd("test/common/upstream/test_data/sds_response_3.json")));
+  message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd(
+      TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_3.json"))));
   callbacks_->onSuccess(std::move(message));
   EXPECT_EQ(13UL, cluster_->hosts().size());
   EXPECT_EQ(12UL, cluster_->healthyHosts().size());

test/config/integration/server.json

@@ -253,15 +253,15 @@
       "driver": {
         "type": "lightstep",
         "config": {
-          "access_token_file": "test/config/integration/lightstep_access_token",
+          "access_token_file": "{{ test_rundir }}/test/config/integration/lightstep_access_token",
           "collector_cluster": "lightstep_saas"
         }
       }
     }
   },
 
   "runtime": {
-    "symlink_root": "test/common/runtime/test_data/current",
+    "symlink_root": "{{ test_rundir }}/test/common/runtime/test_data/current",
     "subdirectory": "envoy",
     "override_subdirectory": "envoy_override"
   },

test/config/integration/server_http2.json

@@ -202,14 +202,14 @@
         "type": "lightstep",
         "config": {
           "collector_cluster": "lightstep_saas",
-          "access_token_file": "test/config/integration/lightstep_access_token"
+          "access_token_file": "{{ test_rundir }}/test/config/integration/lightstep_access_token"
         }
       }
     }
   },
 
   "runtime": {
-    "symlink_root": "test/common/runtime/test_data/current",
+    "symlink_root": "{{ test_rundir }}/test/common/runtime/test_data/current",
     "subdirectory": "envoy",
     "override_subdirectory": "envoy_override"
   },