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 envoyproxy#8082

Signed-off-by: Harvey Tuch <htuch@google.com>

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",
     ],
 )

source/common/config/api_type_oracle.cc

@@ -1,71 +1,36 @@
 #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<std::string, std::string>;
-
-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) {
     ENVOY_LOG_MISC(trace, "No descriptor found for {}", target_type);
     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);
     ENVOY_LOG_MISC(trace, "Inferred {}", desc->full_name());
     return desc;
   }
 
+  ENVOY_LOG_MISC(trace, "No earlier descriptor found for {}", target_type);
   return nullptr;
 }
 

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

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

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);

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<const Protobuf::FieldDescriptor*> prev_field_descriptors;
-  prev_reflection->ListFields(prev_message, &prev_field_descriptors);
-  Protobuf::DynamicMessageFactory dmf;
-  std::unique_ptr<Protobuf::Message> 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<DowngradedMessage>();
+  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