more coverage (#2498)

Some of this is not stupid coverage and should definitely be covered. Probably worth a more complete audit depending on time availability. Signed-off-by: Matt Klein <[email protected]>
envoyproxy · Feb 1, 2018 · eaa6696 · eaa6696
1 parent d67729b
commit eaa6696
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 19 deletions.
diff --git a/source/common/ssl/context_impl.cc b/source/common/ssl/context_impl.cc
@@ -405,15 +405,6 @@ std::string ContextImpl::getSerialNumber(const X509* cert) {
   return "";
 }
 
-bssl::UniquePtr<X509> ContextImpl::loadCert(const std::string& cert_file) {
-  X509* cert = nullptr;
-  std::unique_ptr<FILE, decltype(&fclose)> fp(fopen(cert_file.c_str(), "r"), &fclose);
-  if (!fp.get() || !PEM_read_X509(fp.get(), &cert, nullptr, nullptr)) {
-    throw EnvoyException(fmt::format("Failed to load certificate '{}'", cert_file.c_str()));
-  }
-  return bssl::UniquePtr<X509>(cert);
-};
-
 ClientContextImpl::ClientContextImpl(ContextManagerImpl& parent, Stats::Scope& scope,
                                      const ClientContextConfig& config)
     : ContextImpl(parent, scope, config) {

diff --git a/source/common/ssl/context_impl.h b/source/common/ssl/context_impl.h
@@ -102,7 +102,6 @@ class ContextImpl : public virtual Context {
   std::vector<uint8_t> parseAlpnProtocols(const std::string& alpn_protocols);
   static SslStats generateStats(Stats::Scope& scope);
   int32_t getDaysUntilExpiration(const X509* cert) const;
-  bssl::UniquePtr<X509> loadCert(const std::string& cert_file);
   static std::string getSerialNumber(const X509* cert);
   std::string getCaFileName() const { return ca_file_path_; };
   std::string getCertChainFileName() const { return cert_chain_file_path_; };

diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
@@ -232,8 +232,8 @@ ClusterManagerImpl::ClusterManagerImpl(const envoy::config::bootstrap::v2::Boots
       break;
     }
     default:
-      throw EnvoyException(
-          "Missing config source specifier in envoy::api::v2::ConfigSource for SDS config");
+      // Validated by schema.
+      NOT_REACHED;
     }
   }
 

diff --git a/source/common/upstream/outlier_detection_impl.cc b/source/common/upstream/outlier_detection_impl.cc
@@ -75,7 +75,7 @@ void DetectorHostMonitorImpl::putHttpResponseCode(uint64_t response_code) {
   }
 }
 
-void DetectorHostMonitorImpl::putResult(Result result) {
+Http::Code DetectorHostMonitorImpl::resultToHttpCode(Result result) {
   Http::Code http_code = Http::Code::InternalServerError;
 
   switch (result) {
@@ -96,7 +96,11 @@ void DetectorHostMonitorImpl::putResult(Result result) {
     break;
   }
 
-  putHttpResponseCode(enumToInt(http_code));
+  return http_code;
+}
+
+void DetectorHostMonitorImpl::putResult(Result result) {
+  putHttpResponseCode(enumToInt(resultToHttpCode(result)));
 }
 
 DetectorConfig::DetectorConfig(const envoy::api::v2::cluster::OutlierDetection& config)

diff --git a/source/common/upstream/outlier_detection_impl.h b/source/common/upstream/outlier_detection_impl.h
@@ -12,6 +12,7 @@
 #include "envoy/access_log/access_log.h"
 #include "envoy/api/v2/cluster/outlier_detection.pb.h"
 #include "envoy/event/timer.h"
+#include "envoy/http/codes.h"
 #include "envoy/runtime/runtime.h"
 #include "envoy/upstream/outlier_detection.h"
 #include "envoy/upstream/upstream.h"
@@ -116,6 +117,7 @@ class DetectorHostMonitorImpl : public DetectorHostMonitor {
   void successRate(double new_success_rate) { success_rate_ = new_success_rate; }
   void resetConsecutive5xx() { consecutive_5xx_ = 0; }
   void resetConsecutiveGatewayFailure() { consecutive_gateway_failure_ = 0; }
+  static Http::Code resultToHttpCode(Result result);
 
   // Upstream::Outlier::DetectorHostMonitor
   uint32_t numEjections() override { return num_ejections_; }

diff --git a/source/server/config/stats/statsd.cc b/source/server/config/stats/statsd.cc
@@ -35,8 +35,8 @@ Stats::SinkPtr StatsdSinkFactory::createStatsSink(const Protobuf::Message& confi
         server.clusterManager(), server.stats()));
     break;
   default:
-    throw EnvoyException(
-        fmt::format("No tcp_cluster_name or address provided for {} Stats::Sink config", name()));
+    // Verified by schema.
+    NOT_REACHED;
   }
 }
 

diff --git a/test/common/upstream/outlier_detection_impl_test.cc b/test/common/upstream/outlier_detection_impl_test.cc
@@ -83,6 +83,12 @@ class OutlierDetectorImplTest : public testing::Test {
     }
   }
 
+  void loadRq(HostSharedPtr host, int num_rq, Result result) {
+    for (int i = 0; i < num_rq; i++) {
+      host->outlierDetector().putResult(result);
+    }
+  }
+
   NiceMock<MockCluster> cluster_;
   std::vector<HostSharedPtr>& hosts_ = cluster_.prioritySet().getMockHostSet(0)->hosts_;
   std::vector<HostSharedPtr>& failover_hosts_ = cluster_.prioritySet().getMockHostSet(1)->hosts_;
@@ -141,7 +147,7 @@ TEST_F(OutlierDetectorImplTest, DestroyWithActive) {
       cluster_, empty_outlier_detection_, dispatcher_, runtime_, time_source_, event_logger_));
   detector->addChangedStateCb([&](HostSharedPtr host) -> void { checker_.check(host); });
 
-  loadRq(hosts_[0], 4, 500);
+  loadRq(hosts_[0], 4, Result::REQUEST_FAILED);
   EXPECT_CALL(time_source_, currentTime())
       .WillOnce(Return(MonotonicTime(std::chrono::milliseconds(0))));
   EXPECT_CALL(checker_, check(hosts_[0]));
@@ -822,6 +828,17 @@ TEST(OutlierUtility, SRThreshold) {
   EXPECT_EQ(90.0, ejection_pair.success_rate_average_);
 }
 
+TEST(DetectorHostMonitorImpl, resultToHttpCode) {
+  EXPECT_EQ(Http::Code::OK, DetectorHostMonitorImpl::resultToHttpCode(Result::SUCCESS));
+  EXPECT_EQ(Http::Code::GatewayTimeout, DetectorHostMonitorImpl::resultToHttpCode(Result::TIMEOUT));
+  EXPECT_EQ(Http::Code::ServiceUnavailable,
+            DetectorHostMonitorImpl::resultToHttpCode(Result::CONNECT_FAILED));
+  EXPECT_EQ(Http::Code::InternalServerError,
+            DetectorHostMonitorImpl::resultToHttpCode(Result::REQUEST_FAILED));
+  EXPECT_EQ(Http::Code::ServiceUnavailable,
+            DetectorHostMonitorImpl::resultToHttpCode(Result::SERVER_FAILURE));
+}
+
 } // namespace Outlier
 } // namespace Upstream
 } // namespace Envoy
diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/http2_upstream_integration_test.cc
@@ -42,6 +42,7 @@ TEST_P(Http2UpstreamIntegrationTest, RouterRequestAndResponseWithZeroByteBodyBuf
 TEST_P(Http2UpstreamIntegrationTest, RouterRequestAndResponseWithBodyHttp1) {
   config_helper_.addFilter(ConfigHelper::DEFAULT_BUFFER_FILTER);
   setDownstreamProtocol(Http::CodecClient::Type::HTTP1);
+  config_helper_.setClientCodec(envoy::api::v2::filter::network::HttpConnectionManager::AUTO);
   testRouterRequestAndResponseWithBody(1024, 512, false);
 }
 

diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h
@@ -80,7 +80,8 @@ class HttpIntegrationTest : public BaseIntegrationTest {
   IntegrationCodecClientPtr makeHttpConnection(uint32_t port);
   IntegrationCodecClientPtr makeHttpConnection(Network::ClientConnectionPtr&& conn);
 
-  // sets downstream_protocol_ and alters the client protocol in the config_helper_
+  // Sets downstream_protocol_ and alters the HTTP connection manager codec type in the
+  // config_helper_.
   void setDownstreamProtocol(Http::CodecClient::Type type);
 
   // Sends |request_headers| and |request_body_size| bytes of body upstream.

diff --git a/test/integration/ssl_integration_test.cc b/test/integration/ssl_integration_test.cc
@@ -111,14 +111,15 @@ TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBuffer) {
 
 TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferHttp2) {
   setDownstreamProtocol(Http::CodecClient::Type::HTTP2);
+  config_helper_.setClientCodec(envoy::api::v2::filter::network::HttpConnectionManager::AUTO);
   ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
     return makeSslClientConnection(true, false);
   };
   testRouterRequestAndResponseWithBody(1024, 512, false, &creator);
   checkStats();
 }
 
-TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVierfySAN) {
+TEST_P(SslIntegrationTest, RouterRequestAndResponseWithBodyNoBufferVerifySAN) {
   ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
     return makeSslClientConnection(false, true);
   };