From 457a732c0a031cde3dbf23169c29b3ae48d34531 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 22 May 2019 07:34:38 -0700 Subject: [PATCH] coverage: fix some misc coverage (#7033) Signed-off-by: Matt Klein --- source/exe/process_wide.cc | 2 +- source/extensions/access_loggers/http_grpc/BUILD | 1 + .../extensions/access_loggers/http_grpc/config.cc | 2 ++ .../http_grpc/grpc_access_log_proto_descriptors.cc | 6 ++++-- .../http_grpc/grpc_access_log_proto_descriptors.h | 2 +- .../extensions/filters/network/ratelimit/config.cc | 8 -------- .../extensions/filters/network/ratelimit/config.h | 5 ----- source/extensions/stat_sinks/metrics_service/BUILD | 1 + .../stat_sinks/metrics_service/config.cc | 2 ++ .../grpc_metrics_proto_descriptors.cc | 6 ++++-- .../grpc_metrics_proto_descriptors.h | 2 +- source/server/BUILD | 1 + source/server/proto_descriptors.cc | 14 ++++++-------- source/server/proto_descriptors.h | 2 +- .../header_to_metadata_filter_test.cc | 10 ++++++++++ .../filters/http/health_check/health_check_test.cc | 4 ++-- test/server/options_impl_test.cc | 5 +++++ 17 files changed, 42 insertions(+), 31 deletions(-) diff --git a/source/exe/process_wide.cc b/source/exe/process_wide.cc index f709e9785bdc..62ae2a7515ea 100644 --- a/source/exe/process_wide.cc +++ b/source/exe/process_wide.cc @@ -27,7 +27,7 @@ ProcessWide::ProcessWide() : initialization_depth_(process_wide_initialized) { #endif ares_library_init(ARES_LIB_INIT_ALL); Event::Libevent::Global::initialize(); - RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors(), ""); + Envoy::Server::validateProtoDescriptors(); Http::Http2::initializeNghttp2Logging(); } } diff --git a/source/extensions/access_loggers/http_grpc/BUILD b/source/extensions/access_loggers/http_grpc/BUILD index fc664d1fd46d..8acc86766c42 100644 --- a/source/extensions/access_loggers/http_grpc/BUILD +++ b/source/extensions/access_loggers/http_grpc/BUILD @@ -36,6 +36,7 @@ envoy_cc_library( srcs = ["grpc_access_log_proto_descriptors.cc"], hdrs = ["grpc_access_log_proto_descriptors.h"], deps = [ + "//source/common/common:assert_lib", "//source/common/protobuf", "@envoy_api//envoy/service/accesslog/v2:als_cc", ], diff --git a/source/extensions/access_loggers/http_grpc/config.cc b/source/extensions/access_loggers/http_grpc/config.cc index 3fb77307ecd5..9e4d4f2d2dd8 100644 --- a/source/extensions/access_loggers/http_grpc/config.cc +++ b/source/extensions/access_loggers/http_grpc/config.cc @@ -26,6 +26,8 @@ AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context) { + validateProtoDescriptors(); + const auto& proto_config = MessageUtil::downcastAndValidate< const envoy::config::accesslog::v2::HttpGrpcAccessLogConfig&>(config); std::shared_ptr grpc_access_log_streamer = diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc index 3e9b3f39ce2c..4abb54f024cb 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc +++ b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc @@ -2,6 +2,7 @@ #include "envoy/service/accesslog/v2/als.pb.h" +#include "common/common/assert.h" #include "common/common/fmt.h" #include "common/protobuf/protobuf.h" @@ -10,10 +11,11 @@ namespace Extensions { namespace AccessLoggers { namespace HttpGrpc { -bool validateProtoDescriptors() { +void validateProtoDescriptors() { const auto method = "envoy.service.accesslog.v2.AccessLogService.StreamAccessLogs"; - return Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr; + RELEASE_ASSERT(Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr, + ""); }; } // namespace HttpGrpc } // namespace AccessLoggers diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h index 55b1eda0d5bc..62b70a387a7b 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h +++ b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h @@ -7,7 +7,7 @@ namespace HttpGrpc { // This function validates that the method descriptors for gRPC services and type descriptors that // are referenced in Any messages are available in the descriptor pool. -bool validateProtoDescriptors(); +void validateProtoDescriptors(); } // namespace HttpGrpc } // namespace AccessLoggers } // namespace Extensions diff --git a/source/extensions/filters/network/ratelimit/config.cc b/source/extensions/filters/network/ratelimit/config.cc index 80d8e8ae1ac8..a38c850d562b 100644 --- a/source/extensions/filters/network/ratelimit/config.cc +++ b/source/extensions/filters/network/ratelimit/config.cc @@ -39,14 +39,6 @@ Network::FilterFactoryCb RateLimitConfigFactory::createFilterFactoryFromProtoTyp }; } -Network::FilterFactoryCb -RateLimitConfigFactory::createFilterFactory(const Json::Object& json_config, - Server::Configuration::FactoryContext& context) { - envoy::config::filter::network::rate_limit::v2::RateLimit proto_config; - Envoy::Config::FilterJson::translateTcpRateLimitFilter(json_config, proto_config); - return createFilterFactoryFromProtoTyped(proto_config, context); -} - /** * Static registration for the rate limit filter. @see RegisterFactory. */ diff --git a/source/extensions/filters/network/ratelimit/config.h b/source/extensions/filters/network/ratelimit/config.h index 6ed71520d7fb..b7bd967b6b19 100644 --- a/source/extensions/filters/network/ratelimit/config.h +++ b/source/extensions/filters/network/ratelimit/config.h @@ -20,11 +20,6 @@ class RateLimitConfigFactory public: RateLimitConfigFactory() : FactoryBase(NetworkFilterNames::get().RateLimit) {} - // NamedNetworkFilterConfigFactory - Network::FilterFactoryCb - createFilterFactory(const Json::Object& json_config, - Server::Configuration::FactoryContext& context) override; - private: Network::FilterFactoryCb createFilterFactoryFromProtoTyped( const envoy::config::filter::network::rate_limit::v2::RateLimit& proto_config, diff --git a/source/extensions/stat_sinks/metrics_service/BUILD b/source/extensions/stat_sinks/metrics_service/BUILD index b44e1b0e0564..91cd5dcf50e7 100644 --- a/source/extensions/stat_sinks/metrics_service/BUILD +++ b/source/extensions/stat_sinks/metrics_service/BUILD @@ -30,6 +30,7 @@ envoy_cc_library( srcs = ["grpc_metrics_proto_descriptors.cc"], hdrs = ["grpc_metrics_proto_descriptors.h"], deps = [ + "//source/common/common:assert_lib", "//source/common/protobuf", "@envoy_api//envoy/service/metrics/v2:metrics_service_cc", ], diff --git a/source/extensions/stat_sinks/metrics_service/config.cc b/source/extensions/stat_sinks/metrics_service/config.cc index 8dc265550d4b..78bad7587992 100644 --- a/source/extensions/stat_sinks/metrics_service/config.cc +++ b/source/extensions/stat_sinks/metrics_service/config.cc @@ -19,6 +19,8 @@ namespace MetricsService { Stats::SinkPtr MetricsServiceSinkFactory::createStatsSink(const Protobuf::Message& config, Server::Instance& server) { + validateProtoDescriptors(); + const auto& sink_config = MessageUtil::downcastAndValidate( config); diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc index b546804a7bd1..d3adfeeb846f 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc @@ -2,6 +2,7 @@ #include "envoy/service/metrics/v2/metrics_service.pb.h" +#include "common/common/assert.h" #include "common/common/fmt.h" #include "common/protobuf/protobuf.h" @@ -10,10 +11,11 @@ namespace Extensions { namespace StatSinks { namespace MetricsService { -bool validateProtoDescriptors() { +void validateProtoDescriptors() { const auto method = "envoy.service.metrics.v2.MetricsService.StreamMetrics"; - return Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr; + RELEASE_ASSERT(Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr, + ""); }; } // namespace MetricsService } // namespace StatSinks diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h index d86371d2d608..3c4f4d29756d 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h @@ -7,7 +7,7 @@ namespace MetricsService { // This function validates that the method descriptors for gRPC services and type descriptors that // are referenced in Any messages are available in the descriptor pool. -bool validateProtoDescriptors(); +void validateProtoDescriptors(); } // namespace MetricsService } // namespace StatSinks } // namespace Extensions diff --git a/source/server/BUILD b/source/server/BUILD index 16238024cb49..28b136d9d280 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -285,6 +285,7 @@ envoy_cc_library( srcs = ["proto_descriptors.cc"], hdrs = ["proto_descriptors.h"], deps = [ + "//source/common/common:assert_lib", "//source/common/config:protobuf_link_hacks", "//source/common/protobuf", "@envoy_api//envoy/api/v2:cds_cc", diff --git a/source/server/proto_descriptors.cc b/source/server/proto_descriptors.cc index 608299149b5a..68019c209f7e 100644 --- a/source/server/proto_descriptors.cc +++ b/source/server/proto_descriptors.cc @@ -8,6 +8,7 @@ #include "envoy/service/discovery/v2/hds.pb.h" #include "envoy/service/ratelimit/v2/rls.pb.h" +#include "common/common/assert.h" #include "common/common/fmt.h" #include "common/config/protobuf_link_hacks.h" #include "common/protobuf/protobuf.h" @@ -15,7 +16,7 @@ namespace Envoy { namespace Server { -bool validateProtoDescriptors() { +void validateProtoDescriptors() { const auto methods = { "envoy.api.v2.ClusterDiscoveryService.FetchClusters", "envoy.api.v2.ClusterDiscoveryService.StreamClusters", @@ -32,9 +33,8 @@ bool validateProtoDescriptors() { }; for (const auto& method : methods) { - if (Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) == nullptr) { - return false; - } + RELEASE_ASSERT(Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr, + ""); } const auto types = { @@ -45,11 +45,9 @@ bool validateProtoDescriptors() { }; for (const auto& type : types) { - if (Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(type) == nullptr) { - return false; - } + RELEASE_ASSERT( + Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(type) != nullptr, ""); } - return true; }; } // namespace Server } // namespace Envoy diff --git a/source/server/proto_descriptors.h b/source/server/proto_descriptors.h index fdf20c2e3d86..ef18f2009172 100644 --- a/source/server/proto_descriptors.h +++ b/source/server/proto_descriptors.h @@ -5,6 +5,6 @@ namespace Server { // This function validates that the method descriptors for gRPC services and type descriptors that // are referenced in Any messages are available in the descriptor pool. -bool validateProtoDescriptors(); +void validateProtoDescriptors(); } // namespace Server } // namespace Envoy diff --git a/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc b/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc index 0a9b120c3e40..34b9e3e081a6 100644 --- a/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc +++ b/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc @@ -79,6 +79,10 @@ TEST_F(HeaderToMetadataTest, BasicRequestTest) { EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected))); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(incoming_headers, false)); + Buffer::OwnedImpl data("data"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(incoming_headers)); + filter_->onDestroy(); } /** @@ -114,10 +118,16 @@ TEST_F(HeaderToMetadataTest, HeaderRemovedTest) { EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); EXPECT_CALL(req_info_, setDynamicMetadata("envoy.filters.http.header_to_metadata", MapEq(expected))); + Http::TestHeaderMapImpl continue_response{{":status", "100"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->encode100ContinueHeaders(continue_response)); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(incoming_headers, false)); EXPECT_EQ(empty_headers, incoming_headers); Http::MetadataMap metadata_map{{"metadata", "metadata"}}; EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(metadata_map)); + Buffer::OwnedImpl data("data"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(data, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(incoming_headers)); } /** diff --git a/test/extensions/filters/http/health_check/health_check_test.cc b/test/extensions/filters/http/health_check/health_check_test.cc index 4c6bbba85425..69ee52ac31fe 100644 --- a/test/extensions/filters/http/health_check/health_check_test.cc +++ b/test/extensions/filters/http/health_check/health_check_test.cc @@ -261,9 +261,9 @@ TEST_F(HealthCheckFilterPassThroughTest, OkWithContinue) { // Goodness only knows why there would be a 100-Continue response in health // checks but we can still verify Envoy handles it. - Http::TestHeaderMapImpl continue_respnose{{":status", "100"}}; + Http::TestHeaderMapImpl continue_response{{":status", "100"}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, - filter_->encode100ContinueHeaders(continue_respnose)); + filter_->encode100ContinueHeaders(continue_response)); Http::MetadataMap metadata_map{{"metadata", "metadata"}}; EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(metadata_map)); Http::TestHeaderMapImpl service_hc_respnose{{":status", "200"}}; diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 9b3ae1f74937..0947d7c41cdd 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -303,6 +303,11 @@ TEST_F(OptionsImplTest, SetBothConcurrencyAndCpuset) { createOptionsImpl("envoy -c hello --concurrency 42 --cpuset-threads")); } +TEST_F(OptionsImplTest, SetCpusetOnly) { + std::unique_ptr options = createOptionsImpl("envoy -c hello --cpuset-threads"); + EXPECT_NE(options->concurrency(), 0); +} + #if defined(__linux__) using testing::DoAll;