config: upgrade envoy.ip_tagging to v3alpha. (#9150)

This is a manual pilot of the v3 API type upgade inside of Envoy. The IP tagging filter was picked as its API dependencies are relatively confined in scope, allowing a self-contained PoC to be written. This will be followed by automated upgrade via the API type database for the bulk of Envoy's internals. Part of #8082 Risk level: Low Testing: Additional unit and integration tests added (version_integration_test). Signed-off-by: Harvey Tuch <[email protected]>
envoyproxy · Nov 27, 2019 · 81d3beb · 81d3beb
1 parent 1f83353
commit 81d3beb
Show file tree

Hide file tree

Showing 29 changed files with 358 additions and 47 deletions.
diff --git a/source/common/config/BUILD b/source/common/config/BUILD
@@ -8,6 +8,17 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "api_type_db_lib",
+    srcs = ["api_type_db.cc"],
+    hdrs = ["api_type_db.h"],
+    deps = [
+        "//source/common/protobuf",
+        "//source/common/protobuf:utility_lib",
+        "@com_github_cncf_udpa//udpa/type/v1:typed_struct_cc",
+    ],
+)
+
 envoy_cc_library(
     name = "config_provider_lib",
     srcs = ["config_provider_impl.cc"],
@@ -289,7 +300,9 @@ envoy_cc_library(
     srcs = ["utility.cc"],
     hdrs = ["utility.h"],
     deps = [
+        ":api_type_db_lib",
         ":resources_lib",
+        ":version_converter_lib",
         "//include/envoy/config:grpc_mux_interface",
         "//include/envoy/config:subscription_interface",
         "//include/envoy/local_info:local_info_interface",

diff --git a/source/common/config/api_type_db.cc b/source/common/config/api_type_db.cc
@@ -0,0 +1,42 @@
+#include "common/config/api_type_db.h"
+
+#include "common/protobuf/utility.h"
+
+#include "udpa/type/v1/typed_struct.pb.h"
+
+namespace Envoy {
+namespace Config {
+
+const Protobuf::Descriptor*
+ApiTypeDb::inferEarlierVersionDescriptor(absl::string_view extension_name,
+                                         const ProtobufWkt::Any& typed_config,
+                                         absl::string_view target_type) {
+  // This is just a placeholder for where we will load from the API type
+  // database.
+  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(debug, "Extracted embedded type {}", type);
+  }
+
+  // We have an earlier version if (i) we know about the extension (ii) its type
+  // is different (v3alpha vs. v2) and (iii) we haven't reached the end of the
+  // upgrade chain.
+  if (extension_name == "envoy.ip_tagging" && type != target_type &&
+      target_type != "envoy.config.filter.http.ip_tagging.v2.IPTagging") {
+    const Protobuf::Descriptor* desc =
+        Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(
+            "envoy.config.filter.http.ip_tagging.v2.IPTagging");
+    ASSERT(desc != nullptr);
+    ENVOY_LOG_MISC(debug, "Inferred {}", desc->full_name());
+    return desc;
+  }
+
+  return nullptr;
+}
+
+} // namespace Config
+} // namespace Envoy
diff --git a/source/common/config/api_type_db.h b/source/common/config/api_type_db.h
@@ -0,0 +1,31 @@
+#pragma once
+
+#include "common/protobuf/protobuf.h"
+
+#include "absl/strings/string_view.h"
+
+namespace Envoy {
+namespace Config {
+
+class ApiTypeDb {
+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
+   * 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.
+   * @return const Protobuf::Descriptor* descriptor for earlier message version
+   *         corresponding to config, if any, otherwise nullptr.
+   */
+  static const Protobuf::Descriptor*
+  inferEarlierVersionDescriptor(absl::string_view extension_name,
+                                const ProtobufWkt::Any& typed_config,
+                                absl::string_view target_type);
+};
+
+} // namespace Config
+} // namespace Envoy
diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
@@ -9,7 +9,9 @@
 #include "common/common/fmt.h"
 #include "common/common/hex.h"
 #include "common/common/utility.h"
+#include "common/config/api_type_db.h"
 #include "common/config/resources.h"
+#include "common/config/version_converter.h"
 #include "common/config/well_known_names.h"
 #include "common/protobuf/protobuf.h"
 #include "common/protobuf/utility.h"
@@ -240,20 +242,26 @@ envoy::api::v2::ClusterLoadAssignment Utility::translateClusterHosts(
   return load_assignment;
 }
 
-namespace {
-absl::string_view protoTypeUrlToDescriptorFullName(absl::string_view type_url) {
-  size_t pos = type_url.find_last_of('/');
-  if (pos != absl::string_view::npos) {
-    type_url = type_url.substr(pos + 1);
-  }
-  return type_url;
-}
-} // namespace
-
-void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
+void Utility::translateOpaqueConfig(absl::string_view extension_name,
+                                    const ProtobufWkt::Any& typed_config,
                                     const ProtobufWkt::Struct& config,
                                     ProtobufMessage::ValidationVisitor& validation_visitor,
                                     Protobuf::Message& out_proto) {
+  const Protobuf::Descriptor* earlier_version_desc = ApiTypeDb::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 =
@@ -263,7 +271,7 @@ void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
 
     // 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 = protoTypeUrlToDescriptorFullName(typed_config.type_url());
+    absl::string_view type = TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url());
 
     if (type == typed_struct_type) {
       udpa::type::v1::TypedStruct typed_struct;
@@ -272,7 +280,7 @@ void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
       if (out_proto.GetDescriptor()->full_name() == struct_type) {
         out_proto.CopyFrom(typed_struct.value());
       } else {
-        type = protoTypeUrlToDescriptorFullName(typed_struct.type_url());
+        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() +

diff --git a/source/common/config/utility.h b/source/common/config/utility.h
@@ -208,6 +208,7 @@ class Utility {
 
   /**
    * Translate a nested config into a proto message provided by the implementation factory.
+   * @param extension_name name of extension corresponding to config.
    * @param enclosing_message proto that contains a field 'config'. Note: the enclosing proto is
    * provided because for statically registered implementations, a custom config is generally
    * optional, which means the conversion must be done conditionally.
@@ -225,8 +226,8 @@ class Utility {
     // Fail in an obvious way if a plugin does not return a proto.
     RELEASE_ASSERT(config != nullptr, "");
 
-    translateOpaqueConfig(enclosing_message.typed_config(), enclosing_message.config(),
-                          validation_visitor, *config);
+    translateOpaqueConfig(factory.name(), enclosing_message.typed_config(),
+                          enclosing_message.config(), validation_visitor, *config);
 
     return config;
   }
@@ -268,12 +269,14 @@ 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(const ProtobufWkt::Any& typed_config,
+  static void translateOpaqueConfig(absl::string_view extension_name,
+                                    const ProtobufWkt::Any& typed_config,
                                     const ProtobufWkt::Struct& config,
                                     ProtobufMessage::ValidationVisitor& validation_visitor,
                                     Protobuf::Message& out_proto);

diff --git a/source/common/network/BUILD b/source/common/network/BUILD
@@ -52,6 +52,7 @@ envoy_cc_library(
         "//source/common/common:assert_lib",
         "//source/common/common:utility_lib",
         "@envoy_api//envoy/api/v2/core:pkg_cc_proto",
+        "@envoy_api//envoy/api/v3alpha/core:pkg_cc_proto",
     ],
 )
 

diff --git a/source/common/network/cidr_range.cc b/source/common/network/cidr_range.cc
@@ -114,6 +114,10 @@ CidrRange CidrRange::create(const envoy::api::v2::core::CidrRange& cidr) {
   return create(Utility::parseInternetAddress(cidr.address_prefix()), cidr.prefix_len().value());
 }
 
+CidrRange CidrRange::create(const envoy::api::v3alpha::core::CidrRange& cidr) {
+  return create(Utility::parseInternetAddress(cidr.address_prefix()), cidr.prefix_len().value());
+}
+
 // static
 CidrRange CidrRange::create(const std::string& range) {
   const auto parts = StringUtil::splitToken(range, "/");

diff --git a/source/common/network/cidr_range.h b/source/common/network/cidr_range.h
@@ -4,6 +4,7 @@
 #include <vector>
 
 #include "envoy/api/v2/core/address.pb.h"
+#include "envoy/api/v3alpha/core/address.pb.h"
 #include "envoy/json/json_object.h"
 #include "envoy/network/address.h"
 
@@ -99,6 +100,9 @@ class CidrRange {
    * TODO(ccaraman): Update CidrRange::create to support only constructing valid ranges.
    */
   static CidrRange create(const envoy::api::v2::core::CidrRange& cidr);
+  // The ::v2 and ::v3alpha variants will merge once we land API boosting
+  // automation. TODO(htuch): make sure this happens.
+  static CidrRange create(const envoy::api::v3alpha::core::CidrRange& cidr);
 
   /**
    * Given an IP address and a length of high order bits to keep, returns an address

diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -613,4 +613,12 @@ void TimestampUtil::systemClockToTimestamp(const SystemTime system_clock_time,
           .count()));
 }
 
+absl::string_view TypeUtil::typeUrlToDescriptorFullName(absl::string_view type_url) {
+  const size_t pos = type_url.rfind('/');
+  if (pos != absl::string_view::npos) {
+    type_url = type_url.substr(pos + 1);
+  }
+  return type_url;
+}
+
 } // namespace Envoy
diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
@@ -116,6 +116,11 @@ class MissingFieldException : public EnvoyException {
   MissingFieldException(const std::string& field_name, const Protobuf::Message& message);
 };
 
+class TypeUtil {
+public:
+  static absl::string_view typeUrlToDescriptorFullName(absl::string_view type_url);
+};
+
 class RepeatedPtrUtil {
 public:
   static std::string join(const Protobuf::RepeatedPtrField<std::string>& source,

diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc
@@ -1153,7 +1153,8 @@ 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(typed_config, config, validator, *proto_config);
+  Envoy::Config::Utility::translateOpaqueConfig(name, 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
@@ -175,8 +175,9 @@ template <class ConfigProto> class ConfigurableClusterFactoryBase : public Clust
                     Stats::ScopePtr&& stats_scope) override {
     ProtobufTypes::MessagePtr config = createEmptyConfigProto();
     Config::Utility::translateOpaqueConfig(
-        cluster.cluster_type().typed_config(), ProtobufWkt::Struct::default_instance(),
-        socket_factory_context.messageValidationVisitor(), *config);
+        cluster.cluster_type().name(), cluster.cluster_type().typed_config(),
+        ProtobufWkt::Struct::default_instance(), socket_factory_context.messageValidationVisitor(),
+        *config);
     return createClusterWithConfig(cluster,
                                    MessageUtil::downcastAndValidate<const ConfigProto&>(
                                        *config, context.messageValidationVisitor()),

diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
@@ -137,7 +137,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(typed_config, config, validation_visitor,
+  Envoy::Config::Utility::translateOpaqueConfig(name, typed_config, config, validation_visitor,
                                                 *proto_config);
 
   return factory->createProtocolOptionsConfig(*proto_config, validation_visitor);

diff --git a/source/extensions/filters/http/ip_tagging/BUILD b/source/extensions/filters/http/ip_tagging/BUILD
@@ -24,7 +24,7 @@ envoy_cc_library(
         "//source/common/http:headers_lib",
         "//source/common/network:lc_trie_lib",
         "//source/common/stats:symbol_table_lib",
-        "@envoy_api//envoy/config/filter/http/ip_tagging/v2:pkg_cc_proto",
+        "@envoy_api//envoy/config/filter/http/ip_tagging/v3alpha:pkg_cc_proto",
     ],
 )
 
@@ -40,5 +40,6 @@ envoy_cc_extension(
         "//source/extensions/filters/http/common:factory_base_lib",
         "//source/extensions/filters/http/ip_tagging:ip_tagging_filter_lib",
         "@envoy_api//envoy/config/filter/http/ip_tagging/v2:pkg_cc_proto",
+        "@envoy_api//envoy/config/filter/http/ip_tagging/v3alpha:pkg_cc_proto",
     ],
 )
diff --git a/source/extensions/filters/http/ip_tagging/config.cc b/source/extensions/filters/http/ip_tagging/config.cc
@@ -1,6 +1,6 @@
 #include "extensions/filters/http/ip_tagging/config.h"
 
-#include "envoy/config/filter/http/ip_tagging/v2/ip_tagging.pb.validate.h"
+#include "envoy/config/filter/http/ip_tagging/v3alpha/ip_tagging.pb.validate.h"
 #include "envoy/registry/registry.h"
 
 #include "common/protobuf/utility.h"
@@ -13,7 +13,7 @@ namespace HttpFilters {
 namespace IpTagging {
 
 Http::FilterFactoryCb IpTaggingFilterFactory::createFilterFactoryFromProtoTyped(
-    const envoy::config::filter::http::ip_tagging::v2::IPTagging& proto_config,
+    const envoy::config::filter::http::ip_tagging::v3alpha::IPTagging& proto_config,
     const std::string& stat_prefix, Server::Configuration::FactoryContext& context) {
 
   IpTaggingFilterConfigSharedPtr config(

diff --git a/source/extensions/filters/http/ip_tagging/config.h b/source/extensions/filters/http/ip_tagging/config.h
@@ -1,7 +1,8 @@
 #pragma once
 
-#include "envoy/config/filter/http/ip_tagging/v2/ip_tagging.pb.h"
-#include "envoy/config/filter/http/ip_tagging/v2/ip_tagging.pb.validate.h"
+#include "envoy/config/filter/http/ip_tagging/v2/ip_tagging.pb.h" // For proto descriptors only
+#include "envoy/config/filter/http/ip_tagging/v3alpha/ip_tagging.pb.h"
+#include "envoy/config/filter/http/ip_tagging/v3alpha/ip_tagging.pb.validate.h"
 
 #include "extensions/filters/http/common/factory_base.h"
 #include "extensions/filters/http/well_known_names.h"
@@ -15,13 +16,13 @@ namespace IpTagging {
  * Config registration for the router filter. @see NamedHttpFilterConfigFactory.
  */
 class IpTaggingFilterFactory
-    : public Common::FactoryBase<envoy::config::filter::http::ip_tagging::v2::IPTagging> {
+    : public Common::FactoryBase<envoy::config::filter::http::ip_tagging::v3alpha::IPTagging> {
 public:
   IpTaggingFilterFactory() : FactoryBase(HttpFilterNames::get().IpTagging) {}
 
 private:
   Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
-      const envoy::config::filter::http::ip_tagging::v2::IPTagging& proto_config,
+      const envoy::config::filter::http::ip_tagging::v3alpha::IPTagging& proto_config,
       const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
 };
 

diff --git a/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc b/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc
@@ -11,7 +11,7 @@ namespace HttpFilters {
 namespace IpTagging {
 
 IpTaggingFilterConfig::IpTaggingFilterConfig(
-    const envoy::config::filter::http::ip_tagging::v2::IPTagging& config,
+    const envoy::config::filter::http::ip_tagging::v3alpha::IPTagging& config,
     const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime)
     : request_type_(requestTypeEnum(config.request_type())), scope_(scope), runtime_(runtime),
       stat_name_set_(scope.symbolTable().makeSet("IpTagging")),
@@ -33,7 +33,7 @@ IpTaggingFilterConfig::IpTaggingFilterConfig(
   for (const auto& ip_tag : config.ip_tags()) {
     std::vector<Network::Address::CidrRange> cidr_set;
     cidr_set.reserve(ip_tag.ip_list().size());
-    for (const envoy::api::v2::core::CidrRange& entry : ip_tag.ip_list()) {
+    for (const envoy::api::v3alpha::core::CidrRange& entry : ip_tag.ip_list()) {
 
       // Currently, CidrRange::create doesn't guarantee that the CidrRanges are valid.
       Network::Address::CidrRange cidr_entry = Network::Address::CidrRange::create(entry);