Skip to content

Commit

Permalink
xds: enable the is_optional field for HttpFilter (envoyproxy#16119)
Browse files Browse the repository at this point in the history
Enable `is_optional` field for `HttpFilter`. The default value is `false`, it will keep the same behavior with the current implementation. The envoy will reject the unsupported HTTP filter, also will reject the unsupported HTTP filter in typed per filter config. When the value is `true`, the unsupported HTTP filter will be ignored by the envoy, also will be ignored by typed per filter config, with a warning log.

Risk Level: low
Testing: unit tests and integration tests are added
Docs Changes: API doc is added
Release Notes: added as new feature
Fixes envoyproxy#15770 
Fixes envoyproxy#15025

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
  • Loading branch information
soulxu authored and Le Yao committed Sep 30, 2021
1 parent 0009124 commit 7348d80
Show file tree
Hide file tree
Showing 30 changed files with 999 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ message HttpFilter {
// If true, clients that do not support this filter may ignore the
// filter but otherwise accept the config.
// Otherwise, clients that do not support this filter must reject the config.
// [#not-implemented-hide:]
// This is also same with typed per filter config.
bool is_optional = 6;
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ Removed Config or Runtime

New Features
------------
* http: a new field `is_optional` is added to `extensions.filters.network.http_connection_manager.v3.HttpFilter`. When
value is `true`, the unsupported http filter will be ignored by envoy. This is also same with unsupported http filter
in the typed per filter config. For more information, please reference
:ref:`HttpFilter <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpFilter.is_optional>`.

* crash support: restore crash context when continuing to processing requests or responses as a result of an asynchronous callback that invokes a filter directly. This is unlike the call stacks that go through the various network layers, to eventually reach the filter. For a concrete example see: ``Envoy::Extensions::HttpFilters::Cache::CacheFilter::getHeaders`` which posts a callback on the dispatcher that will invoke the filter directly.
* http: added support for :ref:`original IP detection extensions<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions include/envoy/router/route_config_provider_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace Router {
class RouteConfigProviderManager {
public:
virtual ~RouteConfigProviderManager() = default;
using OptionalHttpFilters = absl::flat_hash_set<std::string>;

/**
* Get a RouteConfigProviderPtr for a route from RDS. Ownership of the RouteConfigProvider is the
Expand All @@ -33,25 +34,29 @@ class RouteConfigProviderManager {
* the RouteConfigProvider. This method creates a RouteConfigProvider which may share the
* underlying RDS subscription with the same (route_config_name, cluster).
* @param rds supplies the proto configuration of an RDS-configured RouteConfigProvider.
* @param optional_http_filters a set of optional http filter names.
* @param factory_context is the context to use for the route config provider.
* @param stat_prefix supplies the stat_prefix to use for the provider stats.
* @param init_manager the Init::Manager used to coordinate initialization of a the underlying RDS
* subscription.
*/
virtual RouteConfigProviderSharedPtr createRdsRouteConfigProvider(
const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds,
const OptionalHttpFilters& optional_http_filters,
Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix,
Init::Manager& init_manager) PURE;

/**
* Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for
* getRdsRouteConfigProvider above. This method always create a new RouteConfigProvider.
* @param route_config supplies the RouteConfiguration for this route
* @param optional_http_filters a set of optional http filter names.
* @param factory_context is the context to use for the route config provider.
* @param validator is the message validator for route config.
*/
virtual RouteConfigProviderPtr
createStaticRouteConfigProvider(const envoy::config::route::v3::RouteConfiguration& route_config,
const OptionalHttpFilters& optional_http_filters,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator) PURE;
};
Expand Down
38 changes: 31 additions & 7 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,21 +249,33 @@ class Utility {
* Get a Factory from the registry with a particular name (and templated type) with error checking
* to ensure the name and factory are valid.
* @param name string identifier for the particular implementation.
* @param is_optional exception will be throw when the value is false and no factory found.
* @return factory the factory requested or nullptr if it does not exist.
*/
template <class Factory> static Factory& getAndCheckFactoryByName(const std::string& name) {
template <class Factory>
static Factory* getAndCheckFactoryByName(const std::string& name, bool is_optional) {
if (name.empty()) {
ExceptionUtil::throwEnvoyException("Provided name for static registration lookup was empty.");
}

Factory* factory = Registry::FactoryRegistry<Factory>::getFactory(name);

if (factory == nullptr) {
if (factory == nullptr && !is_optional) {
ExceptionUtil::throwEnvoyException(
fmt::format("Didn't find a registered implementation for name: '{}'", name));
}

return *factory;
return factory;
}

/**
* Get a Factory from the registry with a particular name (and templated type) with error checking
* to ensure the name and factory are valid.
* @param name string identifier for the particular implementation.
* @return factory the factory requested or nullptr if it does not exist.
*/
template <class Factory> static Factory& getAndCheckFactoryByName(const std::string& name) {
return *getAndCheckFactoryByName<Factory>(name, false);
}

/**
Expand Down Expand Up @@ -294,17 +306,29 @@ class Utility {

/**
* Get a Factory from the registry with error checking to ensure the name and the factory are
* valid.
* valid. And a flag to control return nullptr or throw an exception.
* @param message proto that contains fields 'name' and 'typed_config'.
* @param is_optional an exception will be throw when the value is true and no factory found.
* @return factory the factory requested or nullptr if it does not exist.
*/
template <class Factory, class ProtoMessage>
static Factory& getAndCheckFactory(const ProtoMessage& message) {
static Factory* getAndCheckFactory(const ProtoMessage& message, bool is_optional) {
Factory* factory = Utility::getFactoryByType<Factory>(message.typed_config());
if (factory != nullptr) {
return *factory;
return factory;
}

return Utility::getAndCheckFactoryByName<Factory>(message.name());
return Utility::getAndCheckFactoryByName<Factory>(message.name(), is_optional);
}

/**
* Get a Factory from the registry with error checking to ensure the name and the factory are
* valid.
* @param message proto that contains fields 'name' and 'typed_config'.
*/
template <class Factory, class ProtoMessage>
static Factory& getAndCheckFactory(const ProtoMessage& message) {
return *getAndCheckFactory<Factory>(message, false);
}

/**
Expand Down
Loading

0 comments on commit 7348d80

Please sign in to comment.