From d64ef445b23137cfb8393e013e71d22cf554084c Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 26 Dec 2019 17:32:20 -0500 Subject: [PATCH] config: shadow-based upgrading/downgrading for versioned messages. This PR replaces the previous unknown field smuggling style of upgrading with shadow-based. Ultimately, we want to move away from shadows, but for 1.13.0 this simplifies by removing smuggling. In addition, we make two major changes that are backported from the WiP v3alpha branch: 1. Rather than make translateOpaqueConfig() smart and version/type aware, we switch to an iterative loadFromJson() that first attempts to load as v2 and then falls back to v3. This relies on the property that any v3 configuration that is ingestible as v2 has the same semantics in v2/v3, which holds due to the highly structured vN/v(N+1) mechanical transforms. 2. Support for downgrading to earlier versions is introduced to make it easier for tests that are within the v(N-1) subset of vN to continue to validate v(N-1), while still mostly using vN messages. This is particularly useful for ensuring that e2e integration tests are using v2 resources. As new v3 tests are introduced, tests will need to eschew downgrading, but this should be a viable approach for 1.13.0. Risk level: Low Testing: There's a bunch of new tests that need to be added for version converter, loadFroJson/unpackTo() and translateOpaqueConfig(). Putting this PR out for comment to get feedback. Part of #8082 Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 6 +- source/common/config/api_type_oracle.cc | 49 +----- source/common/config/api_type_oracle.h | 18 +-- source/common/config/utility.cc | 27 +--- source/common/config/utility.h | 8 +- source/common/config/version_converter.cc | 148 +++--------------- source/common/config/version_converter.h | 39 +++-- source/common/protobuf/BUILD | 2 + source/common/protobuf/utility.cc | 117 ++++++++++---- source/common/router/config_impl.cc | 3 +- source/common/upstream/cluster_factory_impl.h | 5 +- source/common/upstream/upstream_impl.cc | 2 +- .../filters/network/dubbo_proxy/config.cc | 2 +- test/common/config/BUILD | 6 + test/common/config/api_type_oracle_test.cc | 20 ++- .../config/delta_subscription_test_harness.h | 3 +- test/common/config/grpc_mux_impl_test.cc | 35 +++-- .../config/grpc_subscription_test_harness.h | 5 +- test/common/config/new_grpc_mux_impl_test.cc | 8 +- test/common/config/utility_test.cc | 72 ++++++++- test/common/config/version_converter_test.cc | 2 + .../clusters/aggregate/cluster_test.cc | 3 +- .../dynamic_forward_proxy/cluster_test.cc | 3 +- .../clusters/redis/redis_cluster_test.cc | 9 +- test/integration/BUILD | 1 + test/integration/integration.h | 7 +- .../scoped_rds_integration_test.cc | 7 +- .../sds_dynamic_integration_test.cc | 2 +- 28 files changed, 283 insertions(+), 326 deletions(-) diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 9b61f27df4ca..3fa7358bb8de 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -13,10 +13,10 @@ envoy_cc_library( srcs = ["api_type_oracle.cc"], hdrs = ["api_type_oracle.h"], deps = [ + "//source/common/common:assert_lib", + "//source/common/common:logger_lib", "//source/common/protobuf", - "//source/common/protobuf:utility_lib", "@com_github_cncf_udpa//udpa/annotations:pkg_cc_proto", - "@com_github_cncf_udpa//udpa/type/v1:pkg_cc_proto", ], ) @@ -342,7 +342,7 @@ envoy_cc_library( srcs = ["version_converter.cc"], hdrs = ["version_converter.h"], deps = [ - "//source/common/common:assert_lib", + ":api_type_oracle_lib", "//source/common/protobuf", ], ) diff --git a/source/common/config/api_type_oracle.cc b/source/common/config/api_type_oracle.cc index 3381b4bdd3c7..b184c8320271 100644 --- a/source/common/config/api_type_oracle.cc +++ b/source/common/config/api_type_oracle.cc @@ -1,52 +1,19 @@ #include "common/config/api_type_oracle.h" -#include "common/protobuf/utility.h" +#include "common/common/assert.h" +#include "common/common/logger.h" #include "udpa/annotations/versioning.pb.h" -#include "udpa/type/v1/typed_struct.pb.h" namespace Envoy { namespace Config { -namespace { - -using V2ApiTypeMap = absl::flat_hash_map; - -const V2ApiTypeMap& v2ApiTypeMap() { - CONSTRUCT_ON_FIRST_USE(V2ApiTypeMap, - {"envoy.ip_tagging", "envoy.config.filter.http.ip_tagging.v2.IPTagging"}); -} - -} // namespace - const Protobuf::Descriptor* -ApiTypeOracle::inferEarlierVersionDescriptor(absl::string_view extension_name, - const ProtobufWkt::Any& typed_config, - absl::string_view target_type) { - ENVOY_LOG_MISC(trace, "Inferring earlier type for {} (extension {})", target_type, - extension_name); - // Determine what the type of configuration implied by typed_config is. - absl::string_view type = TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url()); - udpa::type::v1::TypedStruct typed_struct; - if (type == udpa::type::v1::TypedStruct::default_instance().GetDescriptor()->full_name()) { - MessageUtil::unpackTo(typed_config, typed_struct); - type = TypeUtil::typeUrlToDescriptorFullName(typed_struct.type_url()); - ENVOY_LOG_MISC(trace, "Extracted embedded type {}", type); - } - - // If we can't find an explicit type, this is likely v2, so we need to consult - // a static map. - if (type.empty()) { - auto it = v2ApiTypeMap().find(extension_name); - if (it == v2ApiTypeMap().end()) { - ENVOY_LOG_MISC(trace, "Missing v2 API type map"); - return nullptr; - } - type = it->second; - } +ApiTypeOracle::getEarlierVersionDescriptor(const Protobuf::Message& message) { + const std::string target_type = message.GetDescriptor()->full_name(); + ENVOY_LOG_MISC(trace, "Inferring earlier type for {} (extension {})", target_type); // Determine if there is an earlier API version for target_type. - std::string previous_target_type; const Protobuf::Descriptor* desc = Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(std::string{target_type}); if (desc == nullptr) { @@ -54,11 +21,8 @@ ApiTypeOracle::inferEarlierVersionDescriptor(absl::string_view extension_name, return nullptr; } if (desc->options().HasExtension(udpa::annotations::versioning)) { - previous_target_type = + const std::string previous_target_type = desc->options().GetExtension(udpa::annotations::versioning).previous_message_type(); - } - - if (!previous_target_type.empty() && type != target_type) { const Protobuf::Descriptor* desc = Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(previous_target_type); ASSERT(desc != nullptr); @@ -66,6 +30,7 @@ ApiTypeOracle::inferEarlierVersionDescriptor(absl::string_view extension_name, return desc; } + ENVOY_LOG_MISC(trace, "No earlier descriptor found for {}", target_type); return nullptr; } diff --git a/source/common/config/api_type_oracle.h b/source/common/config/api_type_oracle.h index b1694330fa07..e8ae307fe827 100644 --- a/source/common/config/api_type_oracle.h +++ b/source/common/config/api_type_oracle.h @@ -2,29 +2,21 @@ #include "common/protobuf/protobuf.h" -#include "absl/strings/string_view.h" - namespace Envoy { namespace Config { class ApiTypeOracle { public: /** - * Based on the presented extension config and name, determine if this is - * configuration for an earlier version than the latest alpha version - * supported by Envoy internally. If so, return the descriptor for the earlier + * Based on a given message, determine if there exists an earlier version of + * this message. If so, return the descriptor for the earlier * message, to support upgrading via VersionConverter::upgrade(). * - * @param extension_name name of extension corresponding to config. - * @param typed_config opaque config packed in google.protobuf.Any. - * @param target_type target type of conversion. + * @param message protobuf message. * @return const Protobuf::Descriptor* descriptor for earlier message version - * corresponding to config, if any, otherwise nullptr. + * corresponding to message, if any, otherwise nullptr. */ - static const Protobuf::Descriptor* - inferEarlierVersionDescriptor(absl::string_view extension_name, - const ProtobufWkt::Any& typed_config, - absl::string_view target_type); + static const Protobuf::Descriptor* getEarlierVersionDescriptor(const Protobuf::Message& message); }; } // namespace Config diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index f0abdce9e249..0221a4104716 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -248,33 +248,16 @@ envoy::api::v2::ClusterLoadAssignment Utility::translateClusterHosts( return load_assignment; } -void Utility::translateOpaqueConfig(absl::string_view extension_name, - const ProtobufWkt::Any& typed_config, +void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config, const ProtobufWkt::Struct& config, ProtobufMessage::ValidationVisitor& validation_visitor, Protobuf::Message& out_proto) { - const Protobuf::Descriptor* earlier_version_desc = ApiTypeOracle::inferEarlierVersionDescriptor( - extension_name, typed_config, out_proto.GetDescriptor()->full_name()); - - if (earlier_version_desc != nullptr) { - Protobuf::DynamicMessageFactory dmf; - // Create a previous version message. - auto message = ProtobufTypes::MessagePtr(dmf.GetPrototype(earlier_version_desc)->New()); - ASSERT(message != nullptr); - // Recurse and translateOpaqueConfig for previous version. - translateOpaqueConfig(extension_name, typed_config, config, validation_visitor, *message); - // Update from previous version to current version. - VersionConverter::upgrade(*message, out_proto); - return; - } - static const std::string struct_type = ProtobufWkt::Struct::default_instance().GetDescriptor()->full_name(); 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 absl::string_view type = TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url()); @@ -286,12 +269,8 @@ void Utility::translateOpaqueConfig(absl::string_view extension_name, if (out_proto.GetDescriptor()->full_name() == struct_type) { out_proto.CopyFrom(typed_struct.value()); } else { - type = TypeUtil::typeUrlToDescriptorFullName(typed_struct.type_url()); - if (type != out_proto.GetDescriptor()->full_name()) { - throw EnvoyException("Invalid proto type.\nExpected " + - out_proto.GetDescriptor()->full_name() + - "\nActual: " + std::string(type)); - } + // The typed struct might match out_proto, or some earlier version, let + // MessageUtil::jsonConvert sort this out. MessageUtil::jsonConvert(typed_struct.value(), validation_visitor, out_proto); } } // out_proto is expecting Struct, unpack directly diff --git a/source/common/config/utility.h b/source/common/config/utility.h index bbd25757d3e1..865f839b8b91 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -228,8 +228,8 @@ class Utility { // Fail in an obvious way if a plugin does not return a proto. RELEASE_ASSERT(config != nullptr, ""); - translateOpaqueConfig(factory.name(), enclosing_message.typed_config(), - enclosing_message.config(), validation_visitor, *config); + translateOpaqueConfig(enclosing_message.typed_config(), enclosing_message.config(), + validation_visitor, *config); return config; } @@ -271,14 +271,12 @@ class Utility { /** * Translate opaque config from google.protobuf.Any or google.protobuf.Struct to defined proto * message. - * @param extension_name name of extension corresponding to config. * @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(absl::string_view extension_name, - const ProtobufWkt::Any& typed_config, + static void translateOpaqueConfig(const ProtobufWkt::Any& typed_config, const ProtobufWkt::Struct& config, ProtobufMessage::ValidationVisitor& validation_visitor, Protobuf::Message& out_proto); diff --git a/source/common/config/version_converter.cc b/source/common/config/version_converter.cc index 4893f9a0794e..b810720dd15a 100644 --- a/source/common/config/version_converter.cc +++ b/source/common/config/version_converter.cc @@ -1,142 +1,32 @@ #include "common/config/version_converter.h" -#include "common/common/assert.h" - -// Protobuf reflection is on a per-scalar type basis, i.e. there are method to -// get/set uint32. So, we need to use macro magic to reduce boiler plate below -// when copying fields. -#define UPGRADE_SCALAR(type, method_fragment) \ - case Protobuf::FieldDescriptor::TYPE_##type: { \ - if (prev_field_descriptor->is_repeated()) { \ - const int field_size = prev_reflection->FieldSize(prev_message, prev_field_descriptor); \ - for (int n = 0; n < field_size; ++n) { \ - const auto& v = \ - prev_reflection->GetRepeated##method_fragment(prev_message, prev_field_descriptor, n); \ - target_reflection->Add##method_fragment(target_message, target_field_descriptor, v); \ - } \ - } else { \ - const auto v = prev_reflection->Get##method_fragment(prev_message, prev_field_descriptor); \ - target_reflection->Set##method_fragment(target_message, target_field_descriptor, v); \ - } \ - break; \ - } +#include "common/config/api_type_oracle.h" namespace Envoy { namespace Config { -// TODO(htuch): make the unknown field validators aware of this distinguished -// field, and don't reject if present. -constexpr uint32_t DeprecatedMessageFieldNumber = 100000; - void VersionConverter::upgrade(const Protobuf::Message& prev_message, Protobuf::Message& next_message) { - // Wow, why so complicated? Could we just do this conversion with: - // - // next_message.MergeFromString(prev_message.SerializeAsString()) - // - // and then some clever mangling of the UnknownFieldSet? - // - // Hold your horses! There's a few reasons that the approach below has been - // adopted: - // 1. We can ensure all unknown fields are placed in a distinguished - // DeprecatedMessageFieldNumber, so that the static/dynamic proto - // validators that look at unknown fields are capable of knowing the - // difference between deprecated fields smuggled in from previous versions - // and fields in the new version that are genuinely unknown by the Envoy. - // 2. We can do proto wire breaking changes between major versions. An example - // of this is promotion/demotion between wrapped (e.g. - // google.protobuf.UInt32) and unwrapped types (e.g. uint32). This isn't - // done below yet, but should be possible to automate via "next version" - // annotations on fields. - const Protobuf::Descriptor* next_descriptor = next_message.GetDescriptor(); - const Protobuf::Reflection* prev_reflection = prev_message.GetReflection(); - std::vector prev_field_descriptors; - prev_reflection->ListFields(prev_message, &prev_field_descriptors); - Protobuf::DynamicMessageFactory dmf; - std::unique_ptr deprecated_message; - - // Iterate over all the set fields in the previous version message. - for (const auto* prev_field_descriptor : prev_field_descriptors) { - const Protobuf::Reflection* target_reflection = next_message.GetReflection(); - Protobuf::Message* target_message = &next_message; - - // Does the field exist in the new version message? - const std::string& prev_name = prev_field_descriptor->name(); - const auto* target_field_descriptor = next_descriptor->FindFieldByName(prev_name); - // If we can't find this field in the next version, it must be deprecated. - // So, use deprecated_message and its reflection instead. - if (target_field_descriptor == nullptr) { - ASSERT(prev_field_descriptor->options().deprecated()); - if (!deprecated_message) { - deprecated_message.reset(dmf.GetPrototype(prev_message.GetDescriptor())->New()); - } - target_field_descriptor = prev_field_descriptor; - target_reflection = deprecated_message->GetReflection(); - target_message = deprecated_message.get(); - } - ASSERT(target_field_descriptor != nullptr); - - // These properties are guaranteed by protoxform. - ASSERT(prev_field_descriptor->type() == target_field_descriptor->type()); - ASSERT(prev_field_descriptor->number() == target_field_descriptor->number()); - ASSERT(prev_field_descriptor->type_name() == target_field_descriptor->type_name()); - ASSERT(prev_field_descriptor->is_repeated() == target_field_descriptor->is_repeated()); - - // Message fields need special handling, as we need to recurse. - if (prev_field_descriptor->type() == Protobuf::FieldDescriptor::TYPE_MESSAGE) { - if (prev_field_descriptor->is_repeated()) { - const int field_size = prev_reflection->FieldSize(prev_message, prev_field_descriptor); - for (int n = 0; n < field_size; ++n) { - const Protobuf::Message& prev_nested_message = - prev_reflection->GetRepeatedMessage(prev_message, prev_field_descriptor, n); - Protobuf::Message* target_nested_message = - target_reflection->AddMessage(target_message, target_field_descriptor); - upgrade(prev_nested_message, *target_nested_message); - } - } else { - const Protobuf::Message& prev_nested_message = - prev_reflection->GetMessage(prev_message, prev_field_descriptor); - Protobuf::Message* target_nested_message = - target_reflection->MutableMessage(target_message, target_field_descriptor); - upgrade(prev_nested_message, *target_nested_message); - } - } else { - // Scalar types. - switch (prev_field_descriptor->type()) { - UPGRADE_SCALAR(STRING, String) - UPGRADE_SCALAR(BYTES, String) - UPGRADE_SCALAR(INT32, Int32) - UPGRADE_SCALAR(INT64, Int64) - UPGRADE_SCALAR(UINT32, UInt32) - UPGRADE_SCALAR(UINT64, UInt64) - UPGRADE_SCALAR(DOUBLE, Double) - UPGRADE_SCALAR(FLOAT, Float) - UPGRADE_SCALAR(BOOL, Bool) - UPGRADE_SCALAR(ENUM, EnumValue) - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } - } - } - - if (deprecated_message) { - const Protobuf::Reflection* next_reflection = next_message.GetReflection(); - auto* unknown_field_set = next_reflection->MutableUnknownFields(&next_message); - ASSERT(unknown_field_set->empty()); - std::string* s = unknown_field_set->AddLengthDelimited(DeprecatedMessageFieldNumber); - deprecated_message->SerializeToString(s); - } + std::string s; + prev_message.SerializeToString(&s); + next_message.ParseFromString(s); } -void VersionConverter::unpackDeprecated(const Protobuf::Message& upgraded_message, - Protobuf::Message& deprecated_message) { - const Protobuf::Reflection* reflection = upgraded_message.GetReflection(); - const auto& unknown_field_set = reflection->GetUnknownFields(upgraded_message); - ASSERT(unknown_field_set.field_count() == 1); - const auto& unknown_field = unknown_field_set.field(0); - ASSERT(unknown_field.number() == DeprecatedMessageFieldNumber); - const std::string& s = unknown_field.length_delimited(); - deprecated_message.ParseFromString(s); +DowngradedMessagePtr VersionConverter::downgrade(const Protobuf::Message& message) { + auto downgraded_message = std::make_unique(); + const Protobuf::Descriptor* prev_desc = ApiTypeOracle::getEarlierVersionDescriptor(message); + if (prev_desc != nullptr) { + downgraded_message->msg_.reset(downgraded_message->dmf_.GetPrototype(prev_desc)->New()); + std::string s; + message.SerializeToString(&s); + downgraded_message->msg_->ParseFromString(s); + return downgraded_message; + } + // Unnecessary copy.. + const Protobuf::Descriptor* desc = message.GetDescriptor(); + downgraded_message->msg_.reset(downgraded_message->dmf_.GetPrototype(desc)->New()); + downgraded_message->msg_->MergeFrom(message); + return downgraded_message; } } // namespace Config diff --git a/source/common/config/version_converter.h b/source/common/config/version_converter.h index d5e2db8af36f..e837a1cbc191 100644 --- a/source/common/config/version_converter.h +++ b/source/common/config/version_converter.h @@ -2,36 +2,41 @@ #include "common/protobuf/protobuf.h" +// Convenience macro for downgrading a message and obtaining a reference. +#define API_DOWNGRADE(msg) (*Config::VersionConverter::downgrade(msg)->msg_) + namespace Envoy { namespace Config { +// A message that has been downgraded, e.g. from v3alphato v2. +struct DowngradedMessage { + // The dynamic message factory must outlive the message. + Protobuf::DynamicMessageFactory dmf_; + + // Downgraded message. + std::unique_ptr msg_; +}; + +using DowngradedMessagePtr = std::unique_ptr; + class VersionConverter { public: /** * Upgrade a message from an earlier to later version of the Envoy API. This - * uses reflection techniques to perform the vN -> v(N+1) protobuf C++ type - * lifting which is the moral equivalent of what protoxform does on a - * migration. That is, most fields are by default unchanged in name, type, - * etc. and so can be trivially copied and then a series of mechanical - * transforms allows for a stylized, information preserving upgrade via - * reflection. For example, the UnknownFieldSet in next_message generated by - * protoxform is used to provide access to deprecated fields in the previous - * API version. + * performs a simple wire-level reinterpreation of the fields. As a result of + * shadow protos, earlier deprecated fields such as foo are rematerialized as + * hidden_envoy_deprecated_foo. * * @param prev_message previous version message input. * @param next_nmessage next version message to generate. */ static void upgrade(const Protobuf::Message& prev_message, Protobuf::Message& next_message); - /** - * Access the deprecated fields stored in the UnknownFieldSet of a message - * generated via VersionConverter::upgrade(). - * - * @param upggraded_message the result of VersionConverter::upgrade(). - * @param deprecated_message a copy of the deprecated fields. - */ - static void unpackDeprecated(const Protobuf::Message& upgraded_message, - Protobuf::Message& deprecated_message); + // Downgrade a message to the previous version. If no previous version exists, + // the given message is copied in the return value. This is not super + // efficient, most uses are expected to be tests and performance agnostic + // code. + static DowngradedMessagePtr downgrade(const Protobuf::Message& message); }; } // namespace Config diff --git a/source/common/protobuf/BUILD b/source/common/protobuf/BUILD index 6f41286ad2bd..3f7f69b3c010 100644 --- a/source/common/protobuf/BUILD +++ b/source/common/protobuf/BUILD @@ -65,6 +65,8 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:hash_lib", "//source/common/common:utility_lib", + "//source/common/config:api_type_oracle_lib", + "//source/common/config:version_converter_lib", "@envoy_api//envoy/type:pkg_cc_proto", ], ) diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 561e424206b3..2b9643d0cfa1 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -8,6 +8,8 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/config/api_type_oracle.h" +#include "common/config/version_converter.h" #include "common/protobuf/message_validator_impl.h" #include "common/protobuf/protobuf.h" @@ -109,6 +111,49 @@ void jsonConvertInternal(const Protobuf::Message& source, MessageUtil::loadFromJson(json, dest, validation_visitor); } +// Apply a function transforming a message (e.g. loading JSON into the message). +// First we try with the message's earlier type, and if unsuccessful (or no +// earlier) type, then the current type. This allows us to take a v3 Envoy +// internal proto and ingest both v2 and v3 in methods such as loadFromJson. +// This relies on the property that any v3 configuration that is ingestible as +// v2 has the same semantics in v2/v3, which holds due to the highly structured +// vN/v(N+1) mechanical transforms. +void tryWithApiBoosting(std::function f, Protobuf::Message& message) { + const Protobuf::Descriptor* earlier_version_desc = + Config::ApiTypeOracle::getEarlierVersionDescriptor(message); + // If there is no earlier version of a message, just apply f directly. + if (earlier_version_desc == nullptr) { + f(message); + return; + } + Protobuf::DynamicMessageFactory dmf; + auto earlier_message = ProtobufTypes::MessagePtr(dmf.GetPrototype(earlier_version_desc)->New()); + ASSERT(earlier_message != nullptr); + try { + // Try apply f with an earlier version of the message, then upgrade the + // result. + f(*earlier_message); + Config::VersionConverter::upgrade(*earlier_message, message); + } catch (EnvoyException&) { + // If we fail at the earlier version, try f at the current version of the + // message. + bool newer_error = false; + try { + f(message); + } catch (EnvoyException&) { + // If we fail this time, we should throw. + // TODO(htuch): currently throwing the v2 error, rather than v3 error, to + // make it easier to handle existing v2 tests during API boosting. We + // probably want to provide a more details exception message contaiing + // both v2 and v3 exception info. + newer_error = true; + } + if (newer_error) { + throw; + } + } +} + } // namespace namespace ProtobufPercentHelper { @@ -174,38 +219,42 @@ size_t MessageUtil::hash(const Protobuf::Message& message) { void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message, ProtobufMessage::ValidationVisitor& validation_visitor) { - Protobuf::util::JsonParseOptions options; - options.case_insensitive_enum_parsing = true; - // Let's first try and get a clean parse when checking for unknown fields; - // this should be the common case. - options.ignore_unknown_fields = false; - const auto strict_status = Protobuf::util::JsonStringToMessage(json, &message, options); - if (strict_status.ok()) { - // Success, no need to do any extra work. - return; - } - // If we fail, we see if we get a clean parse when allowing unknown fields. - // This is essentially a workaround - // for https://github.com/protocolbuffers/protobuf/issues/5967. - // TODO(htuch): clean this up when protobuf supports JSON/YAML unknown field - // detection directly. - options.ignore_unknown_fields = true; - const auto relaxed_status = Protobuf::util::JsonStringToMessage(json, &message, options); - // If we still fail with relaxed unknown field checking, the error has nothing - // to do with unknown fields. - if (!relaxed_status.ok()) { - throw EnvoyException("Unable to parse JSON as proto (" + relaxed_status.ToString() + - "): " + json); - } - // We know it's an unknown field at this point. - validation_visitor.onUnknownField("type " + message.GetTypeName() + " reason " + - strict_status.ToString()); + tryWithApiBoosting( + [&json, &validation_visitor](Protobuf::Message& message) { + Protobuf::util::JsonParseOptions options; + options.case_insensitive_enum_parsing = true; + // Let's first try and get a clean parse when checking for unknown fields; + // this should be the common case. + options.ignore_unknown_fields = false; + const auto strict_status = Protobuf::util::JsonStringToMessage(json, &message, options); + if (strict_status.ok()) { + // Success, no need to do any extra work. + return; + } + // If we fail, we see if we get a clean parse when allowing unknown fields. + // This is essentially a workaround + // for https://github.com/protocolbuffers/protobuf/issues/5967. + // TODO(htuch): clean this up when protobuf supports JSON/YAML unknown field + // detection directly. + options.ignore_unknown_fields = true; + const auto relaxed_status = Protobuf::util::JsonStringToMessage(json, &message, options); + // If we still fail with relaxed unknown field checking, the error has nothing + // to do with unknown fields. + if (!relaxed_status.ok()) { + throw EnvoyException("Unable to parse JSON as proto (" + relaxed_status.ToString() + + "): " + json); + } + // We know it's an unknown field at this point. + validation_visitor.onUnknownField("type " + message.GetTypeName() + " reason " + + strict_status.ToString()); + }, + message); } void MessageUtil::loadFromJson(const std::string& json, ProtobufWkt::Struct& message) { // No need to validate if converting to a Struct, since there are no unknown // fields possible. - return loadFromJson(json, message, ProtobufMessage::getNullValidationVisitor()); + loadFromJson(json, message, ProtobufMessage::getNullValidationVisitor()); } void MessageUtil::loadFromYaml(const std::string& yaml, Protobuf::Message& message, @@ -431,11 +480,15 @@ std::string MessageUtil::getJsonStringFromMessage(const Protobuf::Message& messa } void MessageUtil::unpackTo(const ProtobufWkt::Any& any_message, Protobuf::Message& message) { - if (!any_message.UnpackTo(&message)) { - throw EnvoyException(fmt::format("Unable to unpack as {}: {}", - message.GetDescriptor()->full_name(), - any_message.DebugString())); - } + tryWithApiBoosting( + [&any_message](Protobuf::Message& message) { + if (!any_message.UnpackTo(&message)) { + throw EnvoyException(fmt::format("Unable to unpack as {}: {}", + message.GetDescriptor()->full_name(), + any_message.DebugString())); + } + }, + message); } void MessageUtil::jsonConvert(const Protobuf::Message& source, ProtobufWkt::Struct& dest) { diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 30646555f58a..9689bfb1940e 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -1189,8 +1189,7 @@ createRouteSpecificFilterConfig(const std::string& name, const ProtobufWkt::Any& auto& factory = Envoy::Config::Utility::getAndCheckFactory< Server::Configuration::NamedHttpFilterConfigFactory>(name); ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); - Envoy::Config::Utility::translateOpaqueConfig(name, typed_config, config, validator, - *proto_config); + Envoy::Config::Utility::translateOpaqueConfig(typed_config, config, validator, *proto_config); return factory.createRouteSpecificFilterConfig(*proto_config, factory_context, validator); } diff --git a/source/common/upstream/cluster_factory_impl.h b/source/common/upstream/cluster_factory_impl.h index 07ea992b4736..a064a8279cfd 100644 --- a/source/common/upstream/cluster_factory_impl.h +++ b/source/common/upstream/cluster_factory_impl.h @@ -174,9 +174,8 @@ template class ConfigurableClusterFactoryBase : public Clust Stats::ScopePtr&& stats_scope) override { ProtobufTypes::MessagePtr config = createEmptyConfigProto(); Config::Utility::translateOpaqueConfig( - cluster.cluster_type().name(), cluster.cluster_type().typed_config(), - ProtobufWkt::Struct::default_instance(), socket_factory_context.messageValidationVisitor(), - *config); + cluster.cluster_type().typed_config(), ProtobufWkt::Struct::default_instance(), + socket_factory_context.messageValidationVisitor(), *config); return createClusterWithConfig(cluster, MessageUtil::downcastAndValidate( *config, context.messageValidationVisitor()), diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index d359f1d5bf75..b7e20aed1647 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -143,7 +143,7 @@ createProtocolOptionsConfig(const std::string& name, const ProtobufWkt::Any& typ throw EnvoyException(fmt::format("filter {} does not support protocol options", name)); } - Envoy::Config::Utility::translateOpaqueConfig(name, typed_config, config, validation_visitor, + Envoy::Config::Utility::translateOpaqueConfig(typed_config, config, validation_visitor, *proto_config); return factory->createProtocolOptionsConfig(*proto_config, validation_visitor); diff --git a/source/extensions/filters/network/dubbo_proxy/config.cc b/source/extensions/filters/network/dubbo_proxy/config.cc index 154915b0cbd9..12214fc7d244 100644 --- a/source/extensions/filters/network/dubbo_proxy/config.cc +++ b/source/extensions/filters/network/dubbo_proxy/config.cc @@ -146,7 +146,7 @@ void ConfigImpl::registerFilter(const DubboFilterConfig& proto_config) { Envoy::Config::Utility::getAndCheckFactory( string_name); ProtobufTypes::MessagePtr message = factory.createEmptyConfigProto(); - Envoy::Config::Utility::translateOpaqueConfig(string_name, proto_config.config(), + Envoy::Config::Utility::translateOpaqueConfig(proto_config.config(), ProtobufWkt::Struct::default_instance(), context_.messageValidationVisitor(), *message); DubboFilters::FilterFactoryCb callback = diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 8490bd6923b5..043f73685d68 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -94,6 +94,7 @@ envoy_cc_test( "//source/common/config:grpc_mux_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:resources_lib", + "//source/common/config:version_converter_lib", "//source/common/protobuf", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", @@ -116,6 +117,7 @@ envoy_cc_test( "//source/common/config:new_grpc_mux_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:resources_lib", + "//source/common/config:version_converter_lib", "//source/common/protobuf", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", @@ -162,6 +164,7 @@ envoy_cc_test_library( "//source/common/config:api_version_lib", "//source/common/config:grpc_subscription_lib", "//source/common/config:resources_lib", + "//source/common/config:version_converter_lib", "//test/mocks/config:config_mocks", "//test/mocks/event:event_mocks", "//test/mocks/grpc:grpc_mocks", @@ -179,6 +182,7 @@ envoy_cc_test_library( deps = [ ":subscription_test_harness", "//source/common/config:delta_subscription_lib", + "//source/common/config:version_converter_lib", "//source/common/grpc:common_lib", "//test/mocks/config:config_mocks", "//test/mocks/event:event_mocks", @@ -289,6 +293,7 @@ envoy_cc_test( name = "utility_test", srcs = ["utility_test.cc"], deps = [ + "//source/common/config:api_version_lib", "//source/common/config:utility_lib", "//source/common/config:well_known_names", "//source/common/stats:stats_lib", @@ -302,6 +307,7 @@ envoy_cc_test( "@com_github_cncf_udpa//udpa/type/v1:pkg_cc_proto", "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/api/v2/core:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha:pkg_cc_proto", "@envoy_api//envoy/config/bootstrap/v2:pkg_cc_proto", ], ) diff --git a/test/common/config/api_type_oracle_test.cc b/test/common/config/api_type_oracle_test.cc index 4df740356b11..61b1740232f7 100644 --- a/test/common/config/api_type_oracle_test.cc +++ b/test/common/config/api_type_oracle_test.cc @@ -3,23 +3,26 @@ #include "common/config/api_type_oracle.h" -// For proto descriptors only - #include "gtest/gtest.h" -#include "udpa/type/v1/typed_struct.pb.h" + +// API_NO_BOOST_FILE namespace Envoy { namespace Config { namespace { TEST(ApiTypeOracleTest, All) { - // For proto descriptors only - static_cast(envoy::config::filter::http::ip_tagging::v2::IPTagging::RequestType()); - static_cast(envoy::config::filter::http::ip_tagging::v3alpha::IPTagging::RequestType()); + envoy::config::filter::http::ip_tagging::v2::IPTagging v2_config; + envoy::config::filter::http::ip_tagging::v3alpha::IPTagging v3_config; + ProtobufWkt::Any non_api_type; - EXPECT_EQ(nullptr, ApiTypeOracle::inferEarlierVersionDescriptor("foo", {}, "")); - EXPECT_EQ(nullptr, ApiTypeOracle::inferEarlierVersionDescriptor("envoy.ip_tagging", {}, "")); + EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(non_api_type)); + EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(v2_config)); + const auto* desc = ApiTypeOracle::getEarlierVersionDescriptor(v3_config); + EXPECT_EQ(envoy::config::filter::http::ip_tagging::v3alpha::IPTagging::descriptor()->name(), + desc->name()); +#if 0 // Struct upgrade to v3alpha. { const auto* desc = ApiTypeOracle::inferEarlierVersionDescriptor( @@ -72,6 +75,7 @@ TEST(ApiTypeOracleTest, All) { EXPECT_EQ(nullptr, ApiTypeOracle::inferEarlierVersionDescriptor( "envoy.ip_tagging", {}, "envoy.config.filter.http.ip_tagging.v2.IPTagging")); +#endif } } // namespace diff --git a/test/common/config/delta_subscription_test_harness.h b/test/common/config/delta_subscription_test_harness.h index 2ed0609d7251..778a426dbae1 100644 --- a/test/common/config/delta_subscription_test_harness.h +++ b/test/common/config/delta_subscription_test_harness.h @@ -7,6 +7,7 @@ #include "envoy/api/v2/eds.pb.h" #include "common/config/delta_subscription_impl.h" +#include "common/config/version_converter.h" #include "common/grpc/common.h" #include "test/common/config/subscription_test_harness.h" @@ -140,7 +141,7 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { auto* resource = response->add_resources(); resource->set_name(cluster); resource->set_version(version); - resource->mutable_resource()->PackFrom(*load_assignment); + resource->mutable_resource()->PackFrom(API_DOWNGRADE(*load_assignment)); } } Protobuf::RepeatedPtrField removed_resources; diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index 2cb73cdda08b..20cbe450ee42 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -9,6 +9,7 @@ #include "common/config/protobuf_link_hacks.h" #include "common/config/resources.h" #include "common/config/utility.h" +#include "common/config/version_converter.h" #include "common/protobuf/protobuf.h" #include "common/stats/isolated_store_impl.h" @@ -71,7 +72,7 @@ class GrpcMuxImplTestBase : public testing::Test { const std::string& error_message = "") { API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request; if (first) { - expected_request.mutable_node()->CopyFrom(local_info_.node()); + expected_request.mutable_node()->CopyFrom(API_DOWNGRADE(local_info_.node())); } for (const auto& resource : resource_names) { expected_request.add_resource_names(resource); @@ -254,14 +255,14 @@ TEST_F(GrpcMuxImplTest, WildcardWatch) { response->set_version_info("1"); envoy::api::v2::ClusterLoadAssignment load_assignment; load_assignment.set_cluster_name("x"); - response->add_resources()->PackFrom(load_assignment); + response->add_resources()->PackFrom(API_DOWNGRADE(load_assignment)); EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) .WillOnce( Invoke([&load_assignment](const Protobuf::RepeatedPtrField& resources, const std::string&) { EXPECT_EQ(1, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); + envoy::api::v2::ClusterLoadAssignment expected_assignment = + MessageUtil::anyConvert(resources[0]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); })); expectSendMessage(type_url, {}, "1"); @@ -290,15 +291,15 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { response->set_version_info("1"); envoy::api::v2::ClusterLoadAssignment load_assignment; load_assignment.set_cluster_name("x"); - response->add_resources()->PackFrom(load_assignment); + response->add_resources()->PackFrom(API_DOWNGRADE(load_assignment)); EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "1")).Times(0); EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1")) .WillOnce( Invoke([&load_assignment](const Protobuf::RepeatedPtrField& resources, const std::string&) { EXPECT_EQ(1, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); + envoy::api::v2::ClusterLoadAssignment expected_assignment = + MessageUtil::anyConvert(resources[0]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); })); expectSendMessage(type_url, {"y", "z", "x"}, "1"); @@ -312,22 +313,23 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { response->set_version_info("2"); envoy::api::v2::ClusterLoadAssignment load_assignment_x; load_assignment_x.set_cluster_name("x"); - response->add_resources()->PackFrom(load_assignment_x); + response->add_resources()->PackFrom(API_DOWNGRADE(load_assignment_x)); envoy::api::v2::ClusterLoadAssignment load_assignment_y; load_assignment_y.set_cluster_name("y"); - response->add_resources()->PackFrom(load_assignment_y); + response->add_resources()->PackFrom(API_DOWNGRADE(load_assignment_y)); envoy::api::v2::ClusterLoadAssignment load_assignment_z; load_assignment_z.set_cluster_name("z"); - response->add_resources()->PackFrom(load_assignment_z); + response->add_resources()->PackFrom(API_DOWNGRADE(load_assignment_z)); EXPECT_CALL(bar_callbacks, onConfigUpdate(_, "2")) .WillOnce(Invoke( [&load_assignment_y, &load_assignment_z]( const Protobuf::RepeatedPtrField& resources, const std::string&) { EXPECT_EQ(2, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); + envoy::api::v2::ClusterLoadAssignment expected_assignment = + MessageUtil::anyConvert(resources[0]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_y)); - resources[1].UnpackTo(&expected_assignment); + expected_assignment = + MessageUtil::anyConvert(resources[1]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_z)); })); EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "2")) @@ -335,10 +337,11 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { [&load_assignment_x, &load_assignment_y]( const Protobuf::RepeatedPtrField& resources, const std::string&) { EXPECT_EQ(2, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); + envoy::api::v2::ClusterLoadAssignment expected_assignment = + MessageUtil::anyConvert(resources[0]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_x)); - resources[1].UnpackTo(&expected_assignment); + expected_assignment = + MessageUtil::anyConvert(resources[1]); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_y)); })); expectSendMessage(type_url, {"y", "z", "x"}, "2"); diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h index 08ac0165891e..b173e379700a 100644 --- a/test/common/config/grpc_subscription_test_harness.h +++ b/test/common/config/grpc_subscription_test_harness.h @@ -10,6 +10,7 @@ #include "common/config/api_version.h" #include "common/config/grpc_subscription_impl.h" #include "common/config/resources.h" +#include "common/config/version_converter.h" #include "test/common/config/subscription_test_harness.h" #include "test/mocks/config/mocks.h" @@ -65,7 +66,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { UNREFERENCED_PARAMETER(expect_node); API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request; if (expect_node) { - expected_request.mutable_node()->CopyFrom(node_); + expected_request.mutable_node()->CopyFrom(API_DOWNGRADE(node_)); } for (const auto& cluster : cluster_names) { expected_request.add_resource_names(cluster); @@ -104,7 +105,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { last_cluster_names_.end()) { envoy::api::v2::ClusterLoadAssignment* load_assignment = typed_resources.Add(); load_assignment->set_cluster_name(cluster); - response->add_resources()->PackFrom(*load_assignment); + response->add_resources()->PackFrom(API_DOWNGRADE(*load_assignment)); } } EXPECT_CALL(callbacks_, onConfigUpdate(RepeatedProtoEq(response->resources()), version)) diff --git a/test/common/config/new_grpc_mux_impl_test.cc b/test/common/config/new_grpc_mux_impl_test.cc index 57b1373dab22..fd138174afdc 100644 --- a/test/common/config/new_grpc_mux_impl_test.cc +++ b/test/common/config/new_grpc_mux_impl_test.cc @@ -8,6 +8,7 @@ #include "common/config/protobuf_link_hacks.h" #include "common/config/resources.h" #include "common/config/utility.h" +#include "common/config/version_converter.h" #include "common/protobuf/protobuf.h" #include "common/stats/isolated_store_impl.h" @@ -97,15 +98,16 @@ TEST_F(NewGrpcMuxImplTest, DiscoveryResponseNonexistentSub) { response->set_system_version_info("1"); envoy::api::v2::ClusterLoadAssignment load_assignment; load_assignment.set_cluster_name("x"); - response->add_resources()->mutable_resource()->PackFrom(load_assignment); + response->add_resources()->mutable_resource()->PackFrom(API_DOWNGRADE(load_assignment)); EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "1")) .WillOnce( Invoke([&load_assignment]( const Protobuf::RepeatedPtrField& added_resources, const Protobuf::RepeatedPtrField&, const std::string&) { EXPECT_EQ(1, added_resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - added_resources[0].resource().UnpackTo(&expected_assignment); + envoy::api::v2::ClusterLoadAssignment expected_assignment = + MessageUtil::anyConvert( + added_resources[0].resource()); EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); })); grpc_mux_->onDiscoveryResponse(std::move(response)); diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index be838813150b..a02f6f50094e 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -1,10 +1,12 @@ #include "envoy/api/v2/cds.pb.h" #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/api/v2/core/grpc_service.pb.h" +#include "envoy/api/v3alpha/cds.pb.h" #include "envoy/common/exception.h" #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "common/common/fmt.h" +#include "common/config/api_version.h" #include "common/config/utility.h" #include "common/config/well_known_names.h" #include "common/protobuf/protobuf.h" @@ -272,7 +274,7 @@ TEST(UtilityTest, AnyWrongType) { typed_config.PackFrom(source_duration); ProtobufWkt::Timestamp out; EXPECT_THROW_WITH_REGEX( - Utility::translateOpaqueConfig("", typed_config, ProtobufWkt::Struct(), + Utility::translateOpaqueConfig(typed_config, ProtobufWkt::Struct(), ProtobufMessage::getStrictValidationVisitor(), out), EnvoyException, R"(Unable to unpack as google.protobuf.Timestamp: \[type.googleapis.com/google.protobuf.Duration\] .*)"); @@ -294,7 +296,7 @@ TEST(UtilityTest, TypedStructToStruct) { packTypedStructIntoAny(typed_config, untyped_struct); ProtobufWkt::Struct out; - Utility::translateOpaqueConfig("", typed_config, ProtobufWkt::Struct(), + Utility::translateOpaqueConfig(typed_config, ProtobufWkt::Struct(), ProtobufMessage::getStrictValidationVisitor(), out); EXPECT_THAT(out, ProtoEq(untyped_struct)); @@ -315,7 +317,7 @@ TEST(UtilityTest, TypedStructToBootstrap) { packTypedStructIntoAny(typed_config, bootstrap); envoy::config::bootstrap::v2::Bootstrap out; - Utility::translateOpaqueConfig("", typed_config, ProtobufWkt::Struct(), + Utility::translateOpaqueConfig(typed_config, ProtobufWkt::Struct(), ProtobufMessage::getStrictValidationVisitor(), out); EXPECT_THAT(out, ProtoEq(bootstrap)); } @@ -335,14 +337,68 @@ TEST(UtilityTest, TypedStructToInvalidType) { packTypedStructIntoAny(typed_config, bootstrap); ProtobufWkt::Any out; - EXPECT_THROW_WITH_MESSAGE( - Utility::translateOpaqueConfig("", typed_config, ProtobufWkt::Struct(), + EXPECT_THROW_WITH_REGEX( + Utility::translateOpaqueConfig(typed_config, ProtobufWkt::Struct(), ProtobufMessage::getStrictValidationVisitor(), out), - EnvoyException, - "Invalid proto type.\nExpected google.protobuf.Any\nActual: " - "envoy.config.bootstrap.v2.Bootstrap"); + EnvoyException, "Unable to parse JSON as proto"); } +// TODO(htuch): write a bunch of tests for translateOpaqueConfig that reflect +// the tests we used to have for ApiTypeOracle. +#if 0 + // Struct upgrade to v3alpha. + { + const auto* desc = ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", {}, "envoy.config.filter.http.ip_tagging.v3alpha.IPTagging"); + EXPECT_EQ("envoy.config.filter.http.ip_tagging.v2.IPTagging", desc->full_name()); + } + + // Any upgrade from v2 to v3alpha. + { + ProtobufWkt::Any typed_config; + typed_config.set_type_url("envoy.config.filter.http.ip_tagging.v2.IPTagging"); + const auto* desc = ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", typed_config, "envoy.config.filter.http.ip_tagging.v3alpha.IPTagging"); + EXPECT_EQ("envoy.config.filter.http.ip_tagging.v2.IPTagging", desc->full_name()); + } + + // There is no upgrade for same Any and target type URL. + { + ProtobufWkt::Any typed_config; + typed_config.set_type_url("envoy.config.filter.http.ip_tagging.v3alpha.IPTagging"); + EXPECT_EQ(nullptr, ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", typed_config, + "envoy.config.filter.http.ip_tagging.v3alpha.IPTagging")); + } + + // TypedStruct upgrade from v2 to v3alpha. + { + ProtobufWkt::Any typed_config; + udpa::type::v1::TypedStruct typed_struct; + typed_struct.set_type_url("envoy.config.filter.http.ip_tagging.v2.IPTagging"); + typed_config.PackFrom(typed_struct); + const auto* desc = ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", typed_config, "envoy.config.filter.http.ip_tagging.v3alpha.IPTagging"); + EXPECT_EQ("envoy.config.filter.http.ip_tagging.v2.IPTagging", desc->full_name()); + } + + // There is no upgrade for same TypedStruct and target type URL. + { + ProtobufWkt::Any typed_config; + udpa::type::v1::TypedStruct typed_struct; + typed_struct.set_type_url( + "type.googleapis.com/envoy.config.filter.http.ip_tagging.v3alpha.IPTagging"); + typed_config.PackFrom(typed_struct); + EXPECT_EQ(nullptr, ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", typed_config, + "envoy.config.filter.http.ip_tagging.v3alpha.IPTagging")); + } + + // There is no upgrade for v2. + EXPECT_EQ(nullptr, + ApiTypeOracle::inferEarlierVersionDescriptor( + "envoy.ip_tagging", {}, "envoy.config.filter.http.ip_tagging.v2.IPTagging")); +#endif TEST(CheckApiConfigSourceSubscriptionBackingClusterTest, GrpcClusterTestAcrossTypes) { envoy::api::v2::core::ConfigSource config; auto* api_config_source = config.mutable_api_config_source(); diff --git a/test/common/config/version_converter_test.cc b/test/common/config/version_converter_test.cc index e880cf10a126..0beafcbda4d2 100644 --- a/test/common/config/version_converter_test.cc +++ b/test/common/config/version_converter_test.cc @@ -9,6 +9,7 @@ namespace Envoy { namespace Config { namespace { +#if 0 TEST(VersionConverterTest, All) { test::common::config::PreviousVersion previous_version; const std::string previous_version_yaml = R"EOF( @@ -85,6 +86,7 @@ TEST(VersionConverterTest, All) { next_version.enum_field_with_deprecated_value()); EXPECT_EQ(enum_with_deprecated_value, test::common::config::PREV_DEPRECATED_VALUE); } +#endif } // namespace } // namespace Config diff --git a/test/extensions/clusters/aggregate/cluster_test.cc b/test/extensions/clusters/aggregate/cluster_test.cc index 4552132c667d..af33b6d0a221 100644 --- a/test/extensions/clusters/aggregate/cluster_test.cc +++ b/test/extensions/clusters/aggregate/cluster_test.cc @@ -76,8 +76,7 @@ class AggregateClusterTest : public testing::Test { void initialize(const std::string& yaml_config) { envoy::api::v2::Cluster cluster_config = Upstream::parseClusterFromV2Yaml(yaml_config); envoy::config::cluster::aggregate::v2alpha::ClusterConfig config; - Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().name(), - cluster_config.cluster_type().typed_config(), + Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().typed_config(), ProtobufWkt::Struct::default_instance(), ProtobufMessage::getStrictValidationVisitor(), config); Stats::ScopePtr scope = stats_store_.createScope("cluster.name."); diff --git a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc index 05f1e5c70840..5e8a3aa8277d 100644 --- a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc +++ b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc @@ -30,8 +30,7 @@ class ClusterTest : public testing::Test, void initialize(const std::string& yaml_config, bool uses_tls) { envoy::api::v2::Cluster cluster_config = Upstream::parseClusterFromV2Yaml(yaml_config); envoy::config::cluster::dynamic_forward_proxy::v2alpha::ClusterConfig config; - Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().name(), - cluster_config.cluster_type().typed_config(), + Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().typed_config(), ProtobufWkt::Struct::default_instance(), ProtobufMessage::getStrictValidationVisitor(), config); Stats::ScopePtr scope = stats_store_.createScope("cluster.name."); diff --git a/test/extensions/clusters/redis/redis_cluster_test.cc b/test/extensions/clusters/redis/redis_cluster_test.cc index 46812d6ccccf..ecc47b258352 100644 --- a/test/extensions/clusters/redis/redis_cluster_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_test.cc @@ -96,8 +96,7 @@ class RedisClusterTest : public testing::Test, singleton_manager_, tls_, validation_visitor_, *api_); envoy::config::cluster::redis::RedisClusterConfig config; - Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().name(), - cluster_config.cluster_type().typed_config(), + Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().typed_config(), ProtobufWkt::Struct::default_instance(), ProtobufMessage::getStrictValidationVisitor(), config); cluster_callback_ = std::make_shared>(); @@ -127,9 +126,9 @@ class RedisClusterTest : public testing::Test, singleton_manager_, tls_, validation_visitor_, *api_); envoy::config::cluster::redis::RedisClusterConfig config; - Config::Utility::translateOpaqueConfig( - cluster_config.cluster_type().name(), cluster_config.cluster_type().typed_config(), - ProtobufWkt::Struct::default_instance(), validation_visitor_, config); + Config::Utility::translateOpaqueConfig(cluster_config.cluster_type().typed_config(), + ProtobufWkt::Struct::default_instance(), + validation_visitor_, config); NiceMock log_manager; NiceMock outlier_event_logger; diff --git a/test/integration/BUILD b/test/integration/BUILD index 6d01661a7e7c..495e9c64951b 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -484,6 +484,7 @@ envoy_cc_test_library( "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:api_version_lib", + "//source/common/config:version_converter_lib", "//source/common/event:dispatcher_lib", "//source/common/grpc:codec_lib", "//source/common/grpc:common_lib", diff --git a/test/integration/integration.h b/test/integration/integration.h index 1297379c5042..72b057b1ae19 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -10,6 +10,7 @@ #include "envoy/server/process_context.h" #include "common/config/api_version.h" +#include "common/config/version_converter.h" #include "common/http/codec_client.h" #include "test/common/grpc/grpc_client_integration.h" @@ -276,7 +277,7 @@ class BaseIntegrationTest : protected Logger::Loggable { discovery_response.set_version_info(version); discovery_response.set_type_url(type_url); for (const auto& message : messages) { - discovery_response.add_resources()->PackFrom(message); + discovery_response.add_resources()->PackFrom(API_DOWNGRADE(message)); } static int next_nonce_counter = 0; discovery_response.set_nonce(absl::StrCat("nonce", next_nonce_counter++)); @@ -300,10 +301,10 @@ class BaseIntegrationTest : protected Logger::Loggable { for (const auto& message : added_or_updated) { auto* resource = response.add_resources(); ProtobufWkt::Any temp_any; - temp_any.PackFrom(message); + temp_any.PackFrom(API_DOWNGRADE(message)); resource->set_name(TestUtility::xdsResourceName(temp_any)); resource->set_version(version); - resource->mutable_resource()->PackFrom(message); + resource->mutable_resource()->PackFrom(API_DOWNGRADE(message)); } *response.mutable_removed_resources() = {removed.begin(), removed.end()}; static int next_nonce_counter = 0; diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc index e59342544aa3..81307ce10562 100644 --- a/test/integration/scoped_rds_integration_test.cc +++ b/test/integration/scoped_rds_integration_test.cc @@ -8,6 +8,7 @@ #include "common/config/api_version.h" #include "common/config/resources.h" +#include "common/config/version_converter.h" #include "test/common/grpc/grpc_client_integration.h" #include "test/integration/http_integration.h" @@ -166,7 +167,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, response.set_type_url(Config::TypeUrl::get().RouteConfiguration); auto route_configuration = TestUtility::parseYaml(route_config); - response.add_resources()->PackFrom(route_configuration); + response.add_resources()->PackFrom(API_DOWNGRADE(route_configuration)); ASSERT(rds_upstream_info_.stream_by_resource_name_[route_configuration.name()] != nullptr); rds_upstream_info_.stream_by_resource_name_[route_configuration.name()]->sendGrpcMessage( response); @@ -201,7 +202,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, auto resource = response.add_resources(); resource->set_name(scoped_route_proto.name()); resource->set_version(version); - resource->mutable_resource()->PackFrom(scoped_route_proto); + resource->mutable_resource()->PackFrom(API_DOWNGRADE(scoped_route_proto)); } scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_]->sendGrpcMessage( response); @@ -218,7 +219,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, for (const auto& resource_proto : resource_protos) { envoy::api::v2::ScopedRouteConfiguration scoped_route_proto; TestUtility::loadFromYaml(resource_proto, scoped_route_proto); - response.add_resources()->PackFrom(scoped_route_proto); + response.add_resources()->PackFrom(API_DOWNGRADE(scoped_route_proto)); } scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_]->sendGrpcMessage( response); diff --git a/test/integration/sds_dynamic_integration_test.cc b/test/integration/sds_dynamic_integration_test.cc index fe91152ef23b..1ea3f1bc599b 100644 --- a/test/integration/sds_dynamic_integration_test.cc +++ b/test/integration/sds_dynamic_integration_test.cc @@ -121,7 +121,7 @@ class SdsDynamicIntegrationBaseTest : public Grpc::GrpcClientIntegrationParamTes API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info("1"); discovery_response.set_type_url(Config::TypeUrl::get().Secret); - discovery_response.add_resources()->PackFrom(secret); + discovery_response.add_resources()->PackFrom(API_DOWNGRADE(secret)); xds_stream_->sendGrpcMessage(discovery_response); }