Skip to content

Commit

Permalink
coverage: fix some misc coverage (#7033)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Klein <mklein@lyft.com>
  • Loading branch information
mattklein123 authored May 22, 2019
1 parent cab4ce6 commit 457a732
Show file tree
Hide file tree
Showing 17 changed files with 42 additions and 31 deletions.
2 changes: 1 addition & 1 deletion source/exe/process_wide.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
1 change: 1 addition & 0 deletions source/extensions/access_loggers/http_grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/access_loggers/http_grpc/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<GrpcAccessLogStreamer> grpc_access_log_streamer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions source/extensions/filters/network/ratelimit/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
5 changes: 0 additions & 5 deletions source/extensions/filters/network/ratelimit/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions source/extensions/stat_sinks/metrics_service/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/stat_sinks/metrics_service/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace MetricsService {

Stats::SinkPtr MetricsServiceSinkFactory::createStatsSink(const Protobuf::Message& config,
Server::Instance& server) {
validateProtoDescriptors();

const auto& sink_config =
MessageUtil::downcastAndValidate<const envoy::config::metrics::v2::MetricsServiceConfig&>(
config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
14 changes: 6 additions & 8 deletions source/server/proto_descriptors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
#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"

namespace Envoy {
namespace Server {

bool validateProtoDescriptors() {
void validateProtoDescriptors() {
const auto methods = {
"envoy.api.v2.ClusterDiscoveryService.FetchClusters",
"envoy.api.v2.ClusterDiscoveryService.StreamClusters",
Expand All @@ -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 = {
Expand All @@ -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
2 changes: 1 addition & 1 deletion source/server/proto_descriptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"}};
Expand Down
5 changes: 5 additions & 0 deletions test/server/options_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,11 @@ TEST_F(OptionsImplTest, SetBothConcurrencyAndCpuset) {
createOptionsImpl("envoy -c hello --concurrency 42 --cpuset-threads"));
}

TEST_F(OptionsImplTest, SetCpusetOnly) {
std::unique_ptr<OptionsImpl> options = createOptionsImpl("envoy -c hello --cpuset-threads");
EXPECT_NE(options->concurrency(), 0);
}

#if defined(__linux__)

using testing::DoAll;
Expand Down

0 comments on commit 457a732

Please sign in to comment.