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: use filter type URL as the primary way to discover extensions #9618

Merged
merged 32 commits into from
Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7442509
xds: use filter type URL as the primary way to discover extensions
kyessenov Jan 9, 2020
aad3af2
fix test registry
kyessenov Jan 9, 2020
ed020ad
merge fix
kyessenov Jan 9, 2020
151f640
merge fix
kyessenov Jan 9, 2020
f76c893
add unit tests
kyessenov Jan 9, 2020
224d5cf
fix asan
kyessenov Jan 10, 2020
2e7c1fa
review
kyessenov Jan 10, 2020
65113cb
asan fix
kyessenov Jan 10, 2020
6312737
hopefully fix coverage
kyessenov Jan 10, 2020
a4b77ae
coverage fails because of exception test for double registration
kyessenov Jan 10, 2020
ac5504b
change exception into a warning
kyessenov Jan 10, 2020
2e6515b
linkage-dependent test
kyessenov Jan 10, 2020
4b02241
document
kyessenov Jan 10, 2020
0c3dba7
blurb about typed struct
kyessenov Jan 10, 2020
cff30cd
typo
kyessenov Jan 10, 2020
9265f8e
issue with empty again
kyessenov Jan 10, 2020
c0d29bd
struct is ok
kyessenov Jan 10, 2020
18b57ca
fallback to name
kyessenov Jan 10, 2020
5d3342b
oops
kyessenov Jan 10, 2020
a0445ec
merge fix
kyessenov Jan 21, 2020
d546d26
fix tests
kyessenov Jan 21, 2020
6a1d486
section
kyessenov Jan 21, 2020
bc6f2bb
fix version
kyessenov Jan 21, 2020
402a86d
review
kyessenov Jan 21, 2020
7134d12
Merge remote-tracking branch 'upstream/master' into free-the-name
kyessenov Jan 22, 2020
36ad6ec
fix coverage build
kyessenov Jan 22, 2020
df32987
Merge remote-tracking branch 'upstream/master' into free-the-name
kyessenov Jan 22, 2020
858dfed
Merge remote-tracking branch 'upstream/master' into free-the-name
kyessenov Jan 22, 2020
0b875dd
trying to debug on CI since my local build passes
kyessenov Jan 22, 2020
a338edf
trying to debug on CI since my local build passes
kyessenov Jan 22, 2020
9f398d3
ODR violation
kyessenov Jan 23, 2020
1083c22
Merge remote-tracking branch 'upstream/master' into free-the-name
kyessenov Jan 23, 2020
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
63 changes: 63 additions & 0 deletions docs/root/configuration/overview/extension.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
.. _config_overview_extension_configuration:

Extension configuration
-----------------------

Each configuration resource in Envoy has a type URL in the `typed_config`. This
type corresponds to a versioned schema. If the type URL uniquely identifies an
extension capable of interpreting the configuration, then the extension is
selected regardless of the `name` field. In this case the `name` field becomes
optional and can be used as an identifier or as an annotation for the
particular instance of the extension configuration. For example, the following
filter configuration snippet is permitted:

.. code-block:: yaml

name: front-http-proxy
typed_config:
"@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
stat_prefix: ingress_http
codec_type: AUTO
rds:
route_config_name: local_route
config_source:
api_config_source:
api_type: GRPC
grpc_services:
envoy_grpc:
cluster_name: xds_cluster
http_filters:
- name: front-router
typed_config:
"@type": type.googleapis.com/envoy.config.filter.http.router.v2.Router
dynamic_stats: true

In case the control plane lacks the schema definitions for an extension,
`udpa.type.v1.TypedStruct` should be used as a generic container. The type URL
inside it is then used by a client to convert the contents to a typed
configuration resource. For example, the above example could be written as
follows:

.. code-block:: yaml

name: front-http-proxy
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
value:
stat_prefix: ingress_http
codec_type: AUTO
rds:
route_config_name: local_route
config_source:
api_config_source:
api_type: GRPC
grpc_services:
envoy_grpc:
cluster_name: xds_cluster
http_filters:
- name: front-router
typed_config:
"@type": type.googleapis.com/udpa.type.v1.TypedStruct
type_url: type.googleapis.com/envoy.config.filter.http.router.v2.Router

1 change: 1 addition & 0 deletions docs/root/configuration/overview/overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ Overview
versioning
bootstrap
examples
extension
xds_api
mgmt_server
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Version history

1.14.0 (Pending)
================
* config: use type URL to select an extension whenever the config type URL (or its previous versions) uniquely identify a typed extension, see :ref:`extension configuration <config_overview_extension_configuration>`.
* retry: added a retry predicate that :ref:`rejects hosts based on metadata. <envoy_api_field_route.RetryPolicy.retry_host_predicate>`

1.13.0 (January 20, 2020)
Expand Down
1 change: 1 addition & 0 deletions include/envoy/registry/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ envoy_cc_library(
hdrs = ["registry.h"],
deps = [
"//source/common/common:assert_lib",
"//source/common/config:api_type_oracle_lib",
"//source/common/protobuf:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
Expand Down
71 changes: 70 additions & 1 deletion include/envoy/registry/registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/common/assert.h"
#include "common/common/fmt.h"
#include "common/common/logger.h"
#include "common/config/api_type_oracle.h"
#include "common/protobuf/utility.h"

#include "absl/base/attributes.h"
Expand Down Expand Up @@ -188,6 +189,55 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
return *deprecated_factory_names;
}

/**
* Lazily constructs a mapping from the configuration message type to a factory,
* including the deprecated configuration message types.
* Must be invoked after factory registration is completed.
*/
static absl::flat_hash_map<std::string, Base*>& factoriesByType() {
static absl::flat_hash_map<std::string, Base*>* factories_by_type =
[] {
auto mapping = std::make_unique<absl::flat_hash_map<std::string, Base*>>();

for (const auto& factory : factories()) {
if (factory.second == nullptr) {
continue;
}

// Skip untyped factories.
std::string config_type = factory.second->configType();
if (config_type.empty()) {
continue;
}

// Register config types in the mapping and traverse the deprecated message type chain.
while (true) {
auto it = mapping->find(config_type);
if (it != mapping->end() && it->second != factory.second) {
// Mark double-registered types with a nullptr.
// See issue https://github.com/envoyproxy/envoy/issues/9643.
ENVOY_LOG(warn, "Double registration for type: '{}' by '{}' and '{}'", config_type,
factory.second->name(), it->second ? it->second->name() : "");
it->second = nullptr;
} else {
mapping->emplace(std::make_pair(config_type, factory.second));
}

const Protobuf::Descriptor* previous =
Config::ApiTypeOracle::getEarlierVersionDescriptor(config_type);
if (previous == nullptr) {
break;
}
config_type = previous->full_name();
}
}
return mapping;
}()
.release();

return *factories_by_type;
}

/**
* instead_value are used when passed name was deprecated.
*/
Expand Down Expand Up @@ -262,6 +312,14 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
return it->second;
}

static Base* getFactoryByType(absl::string_view type) {
auto it = factoriesByType().find(type);
if (it == factoriesByType().end()) {
return nullptr;
}
return it->second;
}

/**
* @return the canonical name of the factory. If the given name is a
* deprecated factory name, the canonical name is returned instead.
Expand Down Expand Up @@ -320,16 +378,27 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
factories().emplace(factory.name(), &factory);
RELEASE_ASSERT(getFactory(factory.name()) == &factory, "");

auto config_type = factory.configType();
Base* prev = getFactoryByType(config_type);
if (prev != nullptr) {
factoriesByType().emplace(config_type, &factory);
}

return displaced;
}

/**
* Remove a factory by name. This method should only be used for testing purposes.
* @param name is the name of the factory to remove.
*/
static void removeFactoryForTest(absl::string_view name) {
static void removeFactoryForTest(absl::string_view name, absl::string_view config_type) {
auto result = factories().erase(name);
RELEASE_ASSERT(result == 1, "");

Base* prev = getFactoryByType(config_type);
if (prev != nullptr) {
factoriesByType().erase(config_type);
}
}
};

Expand Down
8 changes: 3 additions & 5 deletions source/common/config/api_type_oracle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ namespace Envoy {
namespace Config {

const Protobuf::Descriptor*
ApiTypeOracle::getEarlierVersionDescriptor(const Protobuf::Message& message) {
const std::string target_type = message.GetDescriptor()->full_name();

// Determine if there is an earlier API version for target_type.
ApiTypeOracle::getEarlierVersionDescriptor(const std::string& message_type) {
// Determine if there is an earlier API version for message_type.
const Protobuf::Descriptor* desc =
Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(std::string{target_type});
Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(std::string{message_type});
if (desc == nullptr) {
return nullptr;
}
Expand Down
4 changes: 2 additions & 2 deletions source/common/config/api_type_oracle.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ class ApiTypeOracle {
* this message. If so, return the descriptor for the earlier
* message, to support upgrading via VersionConverter::upgrade().
*
* @param message protobuf message.
* @param message_type protobuf message type
* @return const Protobuf::Descriptor* descriptor for earlier message version
* corresponding to message, if any, otherwise nullptr.
*/
static const Protobuf::Descriptor* getEarlierVersionDescriptor(const Protobuf::Message& message);
static const Protobuf::Descriptor* getEarlierVersionDescriptor(const std::string& message_type);
};

} // namespace Config
Expand Down
2 changes: 0 additions & 2 deletions source/common/config/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#include "common/stats/stats_matcher_impl.h"
#include "common/stats/tag_producer_impl.h"

#include "udpa/type/v1/typed_struct.pb.h"

namespace Envoy {
namespace Config {

Expand Down
22 changes: 22 additions & 0 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include "common/protobuf/utility.h"
#include "common/singleton/const_singleton.h"

#include "udpa/type/v1/typed_struct.pb.h"

namespace Envoy {
namespace Config {

Expand Down Expand Up @@ -215,6 +217,26 @@ class Utility {
*/
template <class Factory, class ProtoMessage>
static Factory& getAndCheckFactory(const ProtoMessage& message) {
const ProtobufWkt::Any& typed_config = message.typed_config();
static const std::string& typed_struct_type =
udpa::type::v1::TypedStruct::default_instance().GetDescriptor()->full_name();

if (!typed_config.value().empty()) {
// Unpack methods will only use the fully qualified type name after the last '/'.
// https://github.com/protocolbuffers/protobuf/blob/3.6.x/src/google/protobuf/any.proto#L87
auto type = std::string(TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url()));
if (type == typed_struct_type) {
udpa::type::v1::TypedStruct typed_struct;
MessageUtil::unpackTo(typed_config, typed_struct);
// Not handling nested structs or typed structs in typed structs
type = typed_struct.type_url();
}
Factory* factory = Registry::FactoryRegistry<Factory>::getFactoryByType(type);
if (factory != nullptr) {
return *factory;
}
}

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

Expand Down
3 changes: 2 additions & 1 deletion source/common/config/version_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ DynamicMessagePtr VersionConverter::recoverOriginal(const Protobuf::Message& upg
}

DynamicMessagePtr VersionConverter::downgrade(const Protobuf::Message& message) {
const Protobuf::Descriptor* prev_desc = ApiTypeOracle::getEarlierVersionDescriptor(message);
const Protobuf::Descriptor* prev_desc =
ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
return createForDescriptorWithCast(message, prev_desc);
}

Expand Down
4 changes: 2 additions & 2 deletions source/common/protobuf/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class ApiBoostRetryException : public EnvoyException {
// vN/v(N+1) mechanical transforms.
void tryWithApiBoosting(MessageXformFn f, Protobuf::Message& message) {
const Protobuf::Descriptor* earlier_version_desc =
Config::ApiTypeOracle::getEarlierVersionDescriptor(message);
Config::ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
// If there is no earlier version of a message, just apply f directly.
if (earlier_version_desc == nullptr) {
f(message, MessageVersion::LATEST_VERSION);
Expand Down Expand Up @@ -528,7 +528,7 @@ void MessageUtil::unpackTo(const ProtobufWkt::Any& any_message, Protobuf::Messag
TypeUtil::typeUrlToDescriptorFullName(any_message.type_url());
if (any_full_name != message.GetDescriptor()->full_name()) {
const Protobuf::Descriptor* earlier_version_desc =
Config::ApiTypeOracle::getEarlierVersionDescriptor(message);
Config::ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
// If the earlier version matches, unpack and upgrade.
if (earlier_version_desc != nullptr && any_full_name == earlier_version_desc->full_name()) {
Protobuf::DynamicMessageFactory dmf;
Expand Down
9 changes: 6 additions & 3 deletions test/common/config/api_type_oracle_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ TEST(ApiTypeOracleTest, All) {
envoy::extensions::filters::http::ip_tagging::v3::IPTagging v3_config;
ProtobufWkt::Any non_api_type;

EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(non_api_type));
EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(v2_config));
const auto* desc = ApiTypeOracle::getEarlierVersionDescriptor(v3_config);
EXPECT_EQ(nullptr,
ApiTypeOracle::getEarlierVersionDescriptor(non_api_type.GetDescriptor()->full_name()));
EXPECT_EQ(nullptr,
ApiTypeOracle::getEarlierVersionDescriptor(v2_config.GetDescriptor()->full_name()));
const auto* desc =
ApiTypeOracle::getEarlierVersionDescriptor(v3_config.GetDescriptor()->full_name());
EXPECT_EQ(envoy::config::filter::http::ip_tagging::v2::IPTagging::descriptor()->full_name(),
desc->full_name());
}
Expand Down
4 changes: 2 additions & 2 deletions test/common/router/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ using testing::StartsWith;
namespace Envoy {
namespace Router {

class TestFilter : public Filter {
class RouterTestFilter : public Filter {
public:
using Filter::Filter;
// Filter
Expand Down Expand Up @@ -267,7 +267,7 @@ class RouterTestBase : public testing::Test {
MockShadowWriter* shadow_writer_;
NiceMock<LocalInfo::MockLocalInfo> local_info_;
FilterConfig config_;
TestFilter router_;
RouterTestFilter router_;
Event::MockTimer* response_timeout_{};
Event::MockTimer* per_try_timeout_{};
Network::Address::InstanceConstSharedPtr host_address_{
Expand Down
Loading