Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: dependency injection of message validation. #7072

Merged
merged 10 commits into from
May 31, 2019
15 changes: 15 additions & 0 deletions include/envoy/protobuf/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
licenses(["notice"]) # Apache 2

load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
)

envoy_package()

envoy_cc_library(
name = "message_validator_interface",
hdrs = ["message_validator.h"],
deps = ["//source/common/protobuf"],
)
29 changes: 29 additions & 0 deletions include/envoy/protobuf/message_validator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#pragma once

#include "envoy/common/pure.h"

#include "common/protobuf/protobuf.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace ProtobufMessage {

/**
* Visitor interface for a Protobuf::Message. The methods of ValidationVisitor are invoked to
* perform validation based on events encountered during or after the parsing of proto binary
* or JSON/YAML.
*/
class ValidationVisitor {
public:
virtual ~ValidationVisitor() = default;

/**
* Invoked when an unknown field is encountered.
* @param description human readable description of the field
*/
virtual void onUnknownField(absl::string_view description) PURE;
};

} // namespace ProtobufMessage
} // namespace Envoy
6 changes: 6 additions & 0 deletions include/envoy/server/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ class FactoryContext {
*/
virtual ProcessContext& processContext() PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
* messages.
*/
virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;

/**
* @return Api::Api& a reference to the api object.
*/
Expand Down
6 changes: 6 additions & 0 deletions include/envoy/server/health_checker_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ class HealthCheckerFactoryContext {
* created health checkers. This function may not be idempotent.
*/
virtual Upstream::HealthCheckEventLoggerPtr eventLogger() PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for health checker configuration
* messages.
*/
virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
};

/**
Expand Down
6 changes: 6 additions & 0 deletions include/envoy/server/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ class Instance {
* @return the flush interval of stats sinks.
*/
virtual std::chrono::milliseconds statsFlushInterval() const PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for configuration
* messages.
*/
virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
};

} // namespace Server
Expand Down
5 changes: 5 additions & 0 deletions include/envoy/server/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class Options {
*/
virtual const std::string& configYaml() const PURE;

/**
* @return bool allow unknown fields in the configuration?
*/
virtual bool allowUnknownFields() const PURE;

/**
* @return const std::string& the admin address output file.
*/
Expand Down
6 changes: 6 additions & 0 deletions include/envoy/server/transport_socket_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ class TransportSocketFactoryContext {
*/
virtual ThreadLocal::SlotAllocator& threadLocal() PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
* messages.
*/
virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;

/**
* @return reference to the Api object
*/
Expand Down
6 changes: 6 additions & 0 deletions include/envoy/upstream/cluster_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ class ClusterFactoryContext {
* @return Outlier::EventLoggerSharedPtr sink for outlier detection event logs.
*/
virtual Outlier::EventLoggerSharedPtr outlierEventLogger() PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
* messages.
*/
virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
};

/**
Expand Down
1 change: 1 addition & 0 deletions include/envoy/upstream/cluster_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ class ClusterInfoFactory {
Runtime::RandomGenerator& random_;
Singleton::Manager& singleton_manager_;
ThreadLocal::SlotAllocator& tls_;
ProtobufMessage::ValidationVisitor& validation_visitor_;
Api::Api& api_;
};

Expand Down
3 changes: 2 additions & 1 deletion source/common/access_log/access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ AccessLogFactory::fromProto(const envoy::config::filter::accesslog::v2::AccessLo
auto& factory =
Config::Utility::getAndCheckFactory<Server::Configuration::AccessLogInstanceFactory>(
config.name());
ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig(config, factory);
ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig(
config, context.messageValidationVisitor(), factory);

return factory.createAccessLogInstance(*message, std::move(filter), context);
}
Expand Down
4 changes: 4 additions & 0 deletions source/common/common/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ namespace Envoy {
static const type* objectptr = new type{__VA_ARGS__}; \
return *objectptr;

#define MUTABLE_CONSTRUCT_ON_FIRST_USE(type, ...) \
static type* objectptr = new type{__VA_ARGS__}; \
return *objectptr;

/**
* Have a generic fall-through for different versions of C++
*/
Expand Down
1 change: 1 addition & 0 deletions source/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ envoy_cc_library(
"//source/common/common:minimal_logger_lib",
"//source/common/config:utility_lib",
"//source/common/protobuf",
"//source/common/protobuf:message_validator_lib",
"//source/common/protobuf:utility_lib",
],
)
Expand Down
11 changes: 6 additions & 5 deletions source/common/config/filesystem_subscription_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
namespace Envoy {
namespace Config {

FilesystemSubscriptionImpl::FilesystemSubscriptionImpl(Event::Dispatcher& dispatcher,
absl::string_view path,
SubscriptionStats stats, Api::Api& api)
: path_(path), watcher_(dispatcher.createFilesystemWatcher()), stats_(stats), api_(api) {
FilesystemSubscriptionImpl::FilesystemSubscriptionImpl(
Event::Dispatcher& dispatcher, absl::string_view path, SubscriptionStats stats,
ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api)
: path_(path), watcher_(dispatcher.createFilesystemWatcher()), stats_(stats), api_(api),
validation_visitor_(validation_visitor) {
watcher_->addWatch(path_, Filesystem::Watcher::Events::MovedTo, [this](uint32_t events) {
UNREFERENCED_PARAMETER(events);
if (started_) {
Expand Down Expand Up @@ -44,7 +45,7 @@ void FilesystemSubscriptionImpl::refresh() {
bool config_update_available = false;
try {
envoy::api::v2::DiscoveryResponse message;
MessageUtil::loadFromFile(path_, message, api_);
MessageUtil::loadFromFile(path_, message, validation_visitor_, api_);
config_update_available = true;
callbacks_->onConfigUpdate(message.resources(), message.version_info());
stats_.version_.set(HashUtil::xxHash64(message.version_info()));
Expand Down
5 changes: 4 additions & 1 deletion source/common/config/filesystem_subscription_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "envoy/config/subscription.h"
#include "envoy/event/dispatcher.h"
#include "envoy/filesystem/filesystem.h"
#include "envoy/protobuf/message_validator.h"

#include "common/common/logger.h"

Expand All @@ -20,7 +21,8 @@ class FilesystemSubscriptionImpl : public Config::Subscription,
Logger::Loggable<Logger::Id::config> {
public:
FilesystemSubscriptionImpl(Event::Dispatcher& dispatcher, absl::string_view path,
SubscriptionStats stats, Api::Api& api);
SubscriptionStats stats,
ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api);

// Config::Subscription
void start(const std::set<std::string>& resources,
Expand All @@ -37,6 +39,7 @@ class FilesystemSubscriptionImpl : public Config::Subscription,
SubscriptionCallbacks* callbacks_{};
SubscriptionStats stats_;
Api::Api& api_;
ProtobufMessage::ValidationVisitor& validation_visitor_;
};

} // namespace Config
Expand Down
8 changes: 5 additions & 3 deletions source/common/config/http_subscription_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ HttpSubscriptionImpl::HttpSubscriptionImpl(
const std::string& remote_cluster_name, Event::Dispatcher& dispatcher,
Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval,
std::chrono::milliseconds request_timeout, const Protobuf::MethodDescriptor& service_method,
SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout)
SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout,
ProtobufMessage::ValidationVisitor& validation_visitor)
: Http::RestApiFetcher(cm, remote_cluster_name, dispatcher, random, refresh_interval,
request_timeout),
stats_(stats), dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout) {
stats_(stats), dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout),
validation_visitor_(validation_visitor) {
request_.mutable_node()->CopyFrom(local_info.node());
ASSERT(service_method.options().HasExtension(google::api::http));
const auto& http_rule = service_method.options().GetExtension(google::api::http);
Expand Down Expand Up @@ -73,7 +75,7 @@ void HttpSubscriptionImpl::parseResponse(const Http::Message& response) {
disableInitFetchTimeoutTimer();
envoy::api::v2::DiscoveryResponse message;
try {
MessageUtil::loadFromJson(response.bodyAsString(), message);
MessageUtil::loadFromJson(response.bodyAsString(), message, validation_visitor_);
} catch (const EnvoyException& e) {
ENVOY_LOG(warn, "REST config JSON conversion error: {}", e.what());
handleFailure(nullptr);
Expand Down
4 changes: 3 additions & 1 deletion source/common/config/http_subscription_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher,
Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval,
std::chrono::milliseconds request_timeout,
const Protobuf::MethodDescriptor& service_method, SubscriptionStats stats,
std::chrono::milliseconds init_fetch_timeout);
std::chrono::milliseconds init_fetch_timeout,
ProtobufMessage::ValidationVisitor& validation_visitor);

// Config::Subscription
void start(const std::set<std::string>& resources,
Expand All @@ -50,6 +51,7 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher,
Event::Dispatcher& dispatcher_;
std::chrono::milliseconds init_fetch_timeout_;
Event::TimerPtr init_fetch_timeout_timer_;
ProtobufMessage::ValidationVisitor& validation_visitor_;
};

} // namespace Config
Expand Down
9 changes: 5 additions & 4 deletions source/common/config/subscription_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ std::unique_ptr<Subscription> SubscriptionFactory::subscriptionFromConfigSource(
const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info,
Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random,
Stats::Scope& scope, const std::string& rest_method, const std::string& grpc_method,
absl::string_view type_url, Api::Api& api) {
absl::string_view type_url, ProtobufMessage::ValidationVisitor& validation_visitor,
Api::Api& api) {
std::unique_ptr<Subscription> result;
SubscriptionStats stats = Utility::generateStats(scope);
switch (config.config_source_specifier_case()) {
case envoy::api::v2::core::ConfigSource::kPath: {
Utility::checkFilesystemSubscriptionBackingPath(config.path(), api);
result =
std::make_unique<Config::FilesystemSubscriptionImpl>(dispatcher, config.path(), stats, api);
result = std::make_unique<Config::FilesystemSubscriptionImpl>(dispatcher, config.path(), stats,
validation_visitor, api);
break;
}
case envoy::api::v2::core::ConfigSource::kApiConfigSource: {
Expand All @@ -40,7 +41,7 @@ std::unique_ptr<Subscription> SubscriptionFactory::subscriptionFromConfigSource(
Utility::apiConfigSourceRefreshDelay(api_config_source),
Utility::apiConfigSourceRequestTimeout(api_config_source),
*Protobuf::DescriptorPool::generated_pool()->FindMethodByName(rest_method), stats,
Utility::configSourceInitialFetchTimeout(config));
Utility::configSourceInitialFetchTimeout(config), validation_visitor);
break;
case envoy::api::v2::core::ApiConfigSource::GRPC:
result = std::make_unique<GrpcSubscriptionImpl>(
Expand Down
4 changes: 3 additions & 1 deletion source/common/config/subscription_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ class SubscriptionFactory {
* description).
* @param grpc_method fully qualified name of v2 gRPC API bidi streaming method (as per protobuf
* service description).
* @param validation_visitor message validation visitor instance.
* @param api reference to the Api object
*/
static std::unique_ptr<Subscription> subscriptionFromConfigSource(
const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info,
Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random,
Stats::Scope& scope, const std::string& rest_method, const std::string& grpc_method,
absl::string_view type_url, Api::Api& api);
absl::string_view type_url, ProtobufMessage::ValidationVisitor& validation_visitor,
Api::Api& api);
};

} // namespace Config
Expand Down
5 changes: 3 additions & 2 deletions source/common/config/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ envoy::api::v2::ClusterLoadAssignment Utility::translateClusterHosts(

void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
const ProtobufWkt::Struct& config,
ProtobufMessage::ValidationVisitor& validation_visitor,
Protobuf::Message& out_proto) {
static const std::string& struct_type =
ProtobufWkt::Struct::default_instance().GetDescriptor()->full_name();
Expand All @@ -277,12 +278,12 @@ void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
} else {
ProtobufWkt::Struct struct_config;
typed_config.UnpackTo(&struct_config);
MessageUtil::jsonConvert(struct_config, out_proto);
MessageUtil::jsonConvert(struct_config, validation_visitor, out_proto);
}
}

if (!config.fields().empty()) {
MessageUtil::jsonConvert(config, out_proto);
MessageUtil::jsonConvert(config, validation_visitor, out_proto);
}
}

Expand Down
31 changes: 9 additions & 22 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,6 @@ typedef ConstSingleton<ApiTypeValues> ApiType;
*/
class Utility {
public:
/**
* Extract typed resources from a DiscoveryResponse.
* @param response reference to DiscoveryResponse.
* @return Protobuf::RepeatedPtrField<ResourceType> vector of typed resources in response.
*/
template <class ResourceType>
static Protobuf::RepeatedPtrField<ResourceType>
getTypedResources(const envoy::api::v2::DiscoveryResponse& response) {
Protobuf::RepeatedPtrField<ResourceType> typed_resources;
for (const auto& resource : response.resources()) {
auto* typed_resource = typed_resources.Add();
if (!resource.UnpackTo(typed_resource)) {
throw EnvoyException("Unable to unpack " + resource.DebugString());
}
MessageUtil::checkUnknownFields(*typed_resource);
}
return typed_resources;
}

/**
* Legacy APIs uses JSON and do not have an explicit version.
* @param input the input to hash.
Expand Down Expand Up @@ -240,18 +221,22 @@ class Utility {
* @param enclosing_message proto that contains a field 'config'. Note: the enclosing proto is
* provided because for statically registered implementations, a custom config is generally
* optional, which means the conversion must be done conditionally.
* @param validation_visitor message validation visitor instance.
* @param factory implementation factory with the method 'createEmptyConfigProto' to produce a
* proto to be filled with the translated configuration.
*/
template <class ProtoMessage, class Factory>
static ProtobufTypes::MessagePtr translateToFactoryConfig(const ProtoMessage& enclosing_message,
Factory& factory) {
static ProtobufTypes::MessagePtr
translateToFactoryConfig(const ProtoMessage& enclosing_message,
ProtobufMessage::ValidationVisitor& validation_visitor,
Factory& factory) {
ProtobufTypes::MessagePtr config = factory.createEmptyConfigProto();

// Fail in an obvious way if a plugin does not return a proto.
RELEASE_ASSERT(config != nullptr, "");

translateOpaqueConfig(enclosing_message.typed_config(), enclosing_message.config(), *config);
translateOpaqueConfig(enclosing_message.typed_config(), enclosing_message.config(),
validation_visitor, *config);

return config;
}
Expand Down Expand Up @@ -295,10 +280,12 @@ class Utility {
* message.
* @param typed_config opaque config packed in google.protobuf.Any
* @param config the deprecated google.protobuf.Struct config, empty struct if doesn't exist.
* @param validation_visitor message validation visitor instance.
* @param out_proto the proto message instantiated by extensions
*/
static void translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
const ProtobufWkt::Struct& config,
ProtobufMessage::ValidationVisitor& validation_visitor,
Protobuf::Message& out_proto);
};

Expand Down
Loading