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

xds: define base interfaces for filter factories #9391

Merged
merged 18 commits into from
Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ci/build_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ if [ "$1" != "-nofetch" ]; then
fi

# This is the hash on https://github.com/envoyproxy/envoy-filter-example.git we pin to.
(cd "${ENVOY_FILTER_EXAMPLE_SRCDIR}" && git fetch origin && git checkout -f 03b45933284b332fd1df42cfb3270751fe543842)
(cd "${ENVOY_FILTER_EXAMPLE_SRCDIR}" && git fetch origin && git checkout -f 5a05fb056a632a30b890ca85e69792558e90ffa9)
sed -e "s|{ENVOY_SRCDIR}|${ENVOY_SRCDIR}|" "${ENVOY_SRCDIR}"/ci/WORKSPACE.filter.example > "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/WORKSPACE
cp -f "${ENVOY_SRCDIR}"/.bazelversion "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/.bazelversion
lizan marked this conversation as resolved.
Show resolved Hide resolved
fi

# Also setup some space for building Envoy standalone.
Expand Down
1 change: 1 addition & 0 deletions include/envoy/access_log/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ envoy_cc_library(
name = "access_log_interface",
hdrs = ["access_log.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/http:header_map_interface",
"//include/envoy/stream_info:stream_info_interface",
"//source/common/protobuf",
Expand Down
10 changes: 10 additions & 0 deletions include/envoy/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,20 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "typed_config_interface",
hdrs = ["typed_config.h"],
deps = [
"//source/common/common:assert_lib",
"//source/common/protobuf",
],
)

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

#include "envoy/common/pure.h"

#include "common/common/assert.h"
#include "common/protobuf/protobuf.h"

namespace Envoy {
namespace Config {

/**
* Base class for an extension factory.
*/
class UntypedFactory {
public:
virtual ~UntypedFactory() = default;

/**
* Name of the factory, a reversed DNS name is encouraged to avoid cross-org conflict.
* It's used as key in the metadata map, as well as key in the factory registry.
*/
virtual std::string name() const PURE;

/**
* @return std::string the identifying category name for objects
* created by this factory. Used for automatic registration with
* FactoryCategoryRegistry.
*/
virtual std::string category() const PURE;

/**
* @return configuration proto full name, or empty for untyped factories.
*/
virtual std::string configType() { return ""; }
};

/**
* Base class for an extension factory configured by a typed proto message.
*/
class TypedFactory : public UntypedFactory {
public:
virtual ~TypedFactory() = default;

/**
* @return ProtobufTypes::MessagePtr create empty config proto message for v2. The config, which
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that folks are going to be starting to move to v3 in early 2020, do we want to do a lot of work to make this supported in v2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some bridge to associate a protobuf type with a factory. We probably could invert this, and just use type to instantiate a proto. Either way works, I'm aiming for minimal disruption in this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about getting #8933 landed? In this world, all extensions are typed..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be wonderful. I was going to special case empty config but would be better without that.

* arrives in an opaque google.protobuf.Struct message, will be converted to JSON and then parsed
* into this empty proto.
*/
virtual ProtobufTypes::MessagePtr createEmptyConfigProto() PURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing we could do, and it doesn't have to be this PR, is move away from having each individual filter return empty protos. Instead, they could either return a type name or descriptor, and we could have more general machinery for assembling the empty proto. Might not be a big win, but just thought I'd throw it out there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. It also helps with the registration initialization fiasco, since protobuf initializer may run after factory initializers.


std::string configType() override {
auto ptr = createEmptyConfigProto();
ASSERT(ptr != nullptr);
return ptr->GetDescriptor()->full_name();
}
};

} // namespace Config
} // namespace Envoy
14 changes: 3 additions & 11 deletions include/envoy/config/typed_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <string>

#include "envoy/common/pure.h"
#include "envoy/config/typed_config.h"

#include "common/protobuf/protobuf.h"

Expand Down Expand Up @@ -50,19 +51,10 @@ class TypedMetadata {
* Typed metadata should implement this factory and register via Registry::registerFactory or the
* convenience class RegisterFactory.
*/
class TypedMetadataFactory {
class TypedMetadataFactory : public UntypedFactory {
public:
virtual ~TypedMetadataFactory() = default;

/**
* Name of the factory, a reversed DNS name is encouraged to avoid cross-org conflict.
* It's used as key in the metadata map, as well as key in the factory registry.
* When building a TypedMetadata from envoy::api::v2::core::Metadata, if the key is not found, the
* parse will not be called and the corresponding typedMetadata entry will be set to nullptr.
* @return the name of the factory.
*/
virtual const std::string name() const PURE;

/**
* Convert the google.protobuf.Struct into an instance of TypedMetadata::Object.
* It should throw an EnvoyException in case the conversion can't be completed.
Expand All @@ -73,7 +65,7 @@ class TypedMetadataFactory {
virtual std::unique_ptr<const TypedMetadata::Object>
parse(const ProtobufWkt::Struct& data) const PURE;

static std::string category() { return "typed_metadata"; }
std::string category() const override { return "typed_metadata"; }
};

} // namespace Config
Expand Down
1 change: 1 addition & 0 deletions include/envoy/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ envoy_cc_library(
],
deps = [
"//include/envoy/api:api_interface",
"//include/envoy/config:typed_config_interface",
"@envoy_api//envoy/config/core/v3alpha:pkg_cc_proto",
],
)
Expand Down
16 changes: 3 additions & 13 deletions include/envoy/grpc/google_grpc_creds.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "envoy/api/api.h"
#include "envoy/common/pure.h"
#include "envoy/config/core/v3alpha/grpc_service.pb.h"
#include "envoy/config/typed_config.h"

#include "grpcpp/grpcpp.h"

Expand All @@ -14,7 +15,7 @@ namespace Grpc {
/**
* Interface for all Google gRPC credentials factories.
*/
class GoogleGrpcCredentialsFactory {
class GoogleGrpcCredentialsFactory : public Config::UntypedFactory {
public:
virtual ~GoogleGrpcCredentialsFactory() = default;

Expand All @@ -34,18 +35,7 @@ class GoogleGrpcCredentialsFactory {
getChannelCredentials(const envoy::config::core::v3alpha::GrpcService& grpc_service_config,
Api::Api& api) PURE;

/**
* @return std::string the identifying name for a particular implementation of
* a Google gRPC credentials factory.
*/
virtual std::string name() const PURE;

/**
* @return std::string the identifying category name for objects
* created by this factory. Used for automatic registration with
* FactoryCategoryRegistry.
*/
static std::string category() { return "grpc_credentials"; }
std::string category() const override { return "grpc_credentials"; }
};

} // namespace Grpc
Expand Down
1 change: 1 addition & 0 deletions include/envoy/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ envoy_cc_library(
hdrs = ["resolver.h"],
deps = [
":address_interface",
"//include/envoy/config:typed_config_interface",
"@envoy_api//envoy/config/core/v3alpha:pkg_cc_proto",
],
)
16 changes: 3 additions & 13 deletions include/envoy/network/resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "envoy/common/pure.h"
#include "envoy/config/core/v3alpha/address.pb.h"
#include "envoy/config/typed_config.h"
#include "envoy/network/address.h"

namespace Envoy {
Expand All @@ -16,7 +17,7 @@ namespace Address {
/**
* Interface for all network address resolvers.
*/
class Resolver {
class Resolver : public Config::UntypedFactory {
public:
virtual ~Resolver() = default;

Expand All @@ -28,18 +29,7 @@ class Resolver {
virtual InstanceConstSharedPtr
resolve(const envoy::config::core::v3alpha::SocketAddress& socket_address) PURE;

/**
* @return std::string the identifying name for a particular implementation of
* a resolver.
*/
virtual std::string name() const PURE;

/**
* @return std::string the identifying category name for objects
* created by this factory. Used for automatic registration with
* FactoryCategoryRegistry.
*/
static std::string category() { return "resolvers"; }
std::string category() const override { return "resolvers"; }
};

} // namespace Address
Expand Down
10 changes: 6 additions & 4 deletions include/envoy/registry/registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
if (!result.second) {
throw EnvoyException(fmt::format("Double registration for name: '{}'", factory.name()));
}

if (!instead_value.empty()) {
deprecatedFactoryNames().emplace(std::make_pair(name, instead_value));
}
Expand Down Expand Up @@ -250,6 +251,7 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id

factories().emplace(factory.name(), &factory);
RELEASE_ASSERT(getFactory(factory.name()) == &factory, "");

return displaced;
}

Expand Down Expand Up @@ -290,8 +292,8 @@ template <class T, class Base> class RegisterFactory {
// register its category here. This means that we have to ignore
// multiple attempts to register the same category and can't detect
// duplicate categories.
if (!FactoryCategoryRegistry::isRegistered(Base::category())) {
FactoryCategoryRegistry::registerCategory(Base::category(),
if (!FactoryCategoryRegistry::isRegistered(instance_.category())) {
FactoryCategoryRegistry::registerCategory(instance_.category(),
new FactoryRegistryProxyImpl<Base>());
}
}
Expand All @@ -312,8 +314,8 @@ template <class T, class Base> class RegisterFactory {
FactoryRegistry<Base>::registerFactory(instance_, deprecated_name, instance_.name());
}

if (!FactoryCategoryRegistry::isRegistered(Base::category())) {
FactoryCategoryRegistry::registerCategory(Base::category(),
if (!FactoryCategoryRegistry::isRegistered(instance_.category())) {
FactoryCategoryRegistry::registerCategory(instance_.category(),
new FactoryRegistryProxyImpl<Base>());
}
}
Expand Down
10 changes: 9 additions & 1 deletion include/envoy/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ envoy_cc_library(
name = "health_checker_config_interface",
hdrs = ["health_checker_config.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/upstream:health_checker_interface",
"@envoy_api//envoy/config/core/v3alpha:pkg_cc_proto",
],
Expand Down Expand Up @@ -155,6 +156,7 @@ envoy_cc_library(
":process_context_interface",
"//include/envoy/access_log:access_log_interface",
"//include/envoy/api:api_interface",
"//include/envoy/config:typed_config_interface",
"//include/envoy/grpc:context_interface",
"//include/envoy/http:codes_interface",
"//include/envoy/http:context_interface",
Expand Down Expand Up @@ -209,6 +211,7 @@ envoy_cc_library(
name = "transport_socket_config_interface",
hdrs = ["transport_socket_config.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/event:dispatcher_interface",
"//include/envoy/init:manager_interface",
"//include/envoy/local_info:local_info_interface",
Expand All @@ -228,6 +231,7 @@ envoy_cc_library(
name = "resource_monitor_interface",
hdrs = ["resource_monitor.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//source/common/protobuf",
],
)
Expand Down Expand Up @@ -257,12 +261,16 @@ envoy_cc_library(
hdrs = ["tracer_config.h"],
deps = [
":instance_interface",
"//include/envoy/config:typed_config_interface",
"//source/common/protobuf",
],
)

envoy_cc_library(
name = "active_udp_listener_config_interface",
hdrs = ["active_udp_listener_config.h"],
deps = ["//include/envoy/network:connection_handler_interface"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/network:connection_handler_interface",
],
)
23 changes: 3 additions & 20 deletions include/envoy/server/access_log_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <string>

#include "envoy/access_log/access_log.h"
#include "envoy/config/typed_config.h"
#include "envoy/server/filter_config.h"

#include "common/protobuf/protobuf.h"
Expand All @@ -15,7 +16,7 @@ namespace Configuration {
* Implemented for each AccessLog::Instance and registered via Registry::registerFactory or the
* convenience class RegisterFactory.
*/
class AccessLogInstanceFactory {
class AccessLogInstanceFactory : public Config::TypedFactory {
public:
virtual ~AccessLogInstanceFactory() = default;

Expand All @@ -32,25 +33,7 @@ class AccessLogInstanceFactory {
AccessLog::FilterPtr&& filter,
FactoryContext& context) PURE;

/**
* @return ProtobufTypes::MessagePtr create empty config proto message for v2. The config, which
* arrives in an opaque google.protobuf.Struct message, will be converted to JSON and then parsed
* into this empty proto.
*/
virtual ProtobufTypes::MessagePtr createEmptyConfigProto() PURE;

/**
* @return std::string the identifying name for a particular AccessLog::Instance implementation
* produced by the factory.
*/
virtual std::string name() const PURE;

/**
* @return std::string the identifying category name for objects
* created by this factory. Used for automatic registration with
* FactoryCategoryRegistry.
*/
static std::string category() { return "access_loggers"; }
std::string category() const override { return "access_loggers"; }
};

} // namespace Configuration
Expand Down
15 changes: 3 additions & 12 deletions include/envoy/server/active_udp_listener_config.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "envoy/config/typed_config.h"
#include "envoy/network/connection_handler.h"

#include "common/protobuf/protobuf.h"
Expand All @@ -11,7 +12,7 @@ namespace Server {
* Interface to create udp listener according to
* envoy::api::v2::listener::UdpListenerConfig.udp_listener_name.
*/
class ActiveUdpListenerConfigFactory {
class ActiveUdpListenerConfigFactory : public Config::UntypedFactory {
public:
virtual ~ActiveUdpListenerConfigFactory() = default;

Expand All @@ -23,17 +24,7 @@ class ActiveUdpListenerConfigFactory {
virtual Network::ActiveUdpListenerFactoryPtr
createActiveUdpListenerFactory(const Protobuf::Message& message) PURE;

/**
* Used to identify which udp listener to create: quic or raw udp.
*/
virtual std::string name() PURE;

/**
* @return std::string the identifying category name for objects
* created by this factory. Used for automatic registration with
* FactoryCategoryRegistry.
*/
static std::string category() { return "udp_listeners"; }
std::string category() const override { return "udp_listeners"; }
};

} // namespace Server
Expand Down
Loading