Build: Fix unit test cert generation (envoyproxy#704)

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.
dnoe · Apr 7, 2017 · 51442b2 · 51442b2
1 parent 2777b3f
commit 51442b2
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 58 deletions.
diff --git a/test/certs/BUILD b/test/certs/BUILD
diff --git 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
@@ -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
@@ -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 → test/common/ssl/gen_unittest_certs.sh b/test/certs/gen_test_certs.sh → 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
@@ -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
@@ -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
@@ -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);