envoyproxy · mattklein123 · Oct 13, 2021 · Oct 11, 2021 · Oct 11, 2021 · Oct 12, 2021
diff --git a/api/envoy/config/core/v3/extension.proto b/api/envoy/config/core/v3/extension.proto
@@ -24,8 +24,9 @@ message TypedExtensionConfig {
   string name = 1 [(validate.rules).string = {min_len: 1}];
 
   // The typed config for the extension. The type URL will be used to identify
-  // the extension. In the case that the type URL is *udpa.type.v1.TypedStruct*,
-  // the inner type URL of *TypedStruct* will be utilized. See the
+  // the extension. In the case that the type URL is *xds.type.v3.TypedStruct*
+  // (or, for historical reasons, *udpa.type.v1.TypedStruct*), the inner type
+  // URL of *TypedStruct* will be utilized. See the
   // :ref:`extension configuration overview
   // <config_overview_extension_configuration>` for further details.
   google.protobuf.Any typed_config = 2 [(validate.rules).any = {required: true}];

diff --git a/docs/root/api/client_features.rst b/docs/root/api/client_features.rst
@@ -14,7 +14,7 @@ Currently Defined Client Features
 
 - **envoy.config.require-any-fields-contain-struct**: This feature indicates that xDS client
   requires that the configuration entries of type  *google.protobuf.Any* contain messages of type
-  *udpa.type.v1.TypedStruct* only.
+  *xds.type.v3.TypedStruct* (or, for historical reasons, *udpa.type.v1.TypedStruct*) only.
 - **envoy.lb.does_not_support_overprovisioning**: This feature indicates that the client does not
   support overprovisioning for priority failover and locality weighting as configured by the
   :ref:`overprovisioning_factor <envoy_v3_api_field_config.endpoint.v3.clusterloadassignment.policy.overprovisioning_factor>`

diff --git a/docs/root/configuration/overview/extension.rst b/docs/root/configuration/overview/extension.rst
@@ -35,7 +35,7 @@ filter configuration snippet is permitted:
         dynamic_stats: true
 
 In case the control plane lacks the schema definitions for an extension,
-``udpa.type.v1.TypedStruct`` should be used as a generic container. The type URL
+``xds.type.v3.TypedStruct`` should be used as a generic container. The type URL
 inside it is then used by a client to convert the contents to a typed
 configuration resource. For example, the above example could be written as
 follows:
@@ -44,7 +44,7 @@ follows:
 
   name: front-http-proxy
   typed_config:
-    "@type": type.googleapis.com/udpa.type.v1.TypedStruct
+    "@type": type.googleapis.com/xds.type.v3.TypedStruct
     type_url: type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
     value:
       stat_prefix: ingress_http
@@ -62,7 +62,7 @@ follows:
       http_filters:
       - name: front-router
         typed_config:
-          "@type": type.googleapis.com/udpa.type.v1.TypedStruct
+          "@type": type.googleapis.com/xds.type.v3.TypedStruct
           type_url: type.googleapis.com/envoy.extensions.filters.http.router.v3Router
 
 .. _config_overview_extension_discovery:

diff --git a/source/common/config/BUILD b/source/common/config/BUILD
@@ -387,6 +387,7 @@ envoy_cc_library(
         "//source/common/stats:stats_matcher_lib",
         "//source/common/stats:tag_producer_lib",
         "@com_github_cncf_udpa//udpa/type/v1:pkg_cc_proto",
+        "@com_github_cncf_udpa//xds/type/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",

diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
@@ -254,6 +254,8 @@ void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
   static const std::string struct_type =
       ProtobufWkt::Struct::default_instance().GetDescriptor()->full_name();
   static const std::string typed_struct_type =
+      xds::type::v3::TypedStruct::default_instance().GetDescriptor()->full_name();
+  static const std::string legacy_typed_struct_type =
       udpa::type::v1::TypedStruct::default_instance().GetDescriptor()->full_name();
 
   if (!typed_config.value().empty()) {
@@ -262,6 +264,17 @@ void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
     absl::string_view type = TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url());
 
     if (type == typed_struct_type) {
+      xds::type::v3::TypedStruct typed_struct;
+      MessageUtil::unpackTo(typed_config, typed_struct);
+      // if out_proto is expecting Struct, return directly
+      if (out_proto.GetDescriptor()->full_name() == struct_type) {
+        out_proto.CopyFrom(typed_struct.value());
+      } else {
+        // 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);
+      }
+    } else if (type == legacy_typed_struct_type) {
       udpa::type::v1::TypedStruct typed_struct;
       MessageUtil::unpackTo(typed_config, typed_struct);
       // if out_proto is expecting Struct, return directly

diff --git a/source/common/config/utility.h b/source/common/config/utility.h
@@ -31,6 +31,7 @@
 #include "source/common/singleton/const_singleton.h"
 
 #include "udpa/type/v1/typed_struct.pb.h"
+#include "xds/type/v3/typed_struct.pb.h"
 
 namespace Envoy {
 namespace Config {
@@ -329,11 +330,18 @@ class Utility {
    */
   static std::string getFactoryType(const ProtobufWkt::Any& typed_config) {
     static const std::string& typed_struct_type =
+        xds::type::v3::TypedStruct::default_instance().GetDescriptor()->full_name();
+    static const std::string& legacy_typed_struct_type =
         udpa::type::v1::TypedStruct::default_instance().GetDescriptor()->full_name();
     // Unpack methods will only use the fully qualified type name after the last '/'.
     // https://github.com/protocolbuffers/protobuf/blob/3.6.x/src/google/protobuf/any.proto#L87
     auto type = std::string(TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url()));
     if (type == typed_struct_type) {
+      xds::type::v3::TypedStruct typed_struct;
+      MessageUtil::unpackTo(typed_config, typed_struct);
+      // Not handling nested structs or typed structs in typed structs
+      return std::string(TypeUtil::typeUrlToDescriptorFullName(typed_struct.type_url()));
+    } else if (type == legacy_typed_struct_type) {
       udpa::type::v1::TypedStruct typed_struct;
       MessageUtil::unpackTo(typed_config, typed_struct);
       // Not handling nested structs or typed structs in typed structs

diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -644,9 +644,10 @@ bool redactAny(Protobuf::Message* message, bool ancestor_is_sensitive) {
 }
 
 // To redact a `TypedStruct`, we have to reify it based on its `type_url` to redact it.
-bool redactTypedStruct(Protobuf::Message* message, bool ancestor_is_sensitive) {
+bool redactTypedStruct(Protobuf::Message* message, const char* typed_struct_type,
+                       bool ancestor_is_sensitive) {
   return redactOpaque(
-      message, ancestor_is_sensitive, "udpa.type.v1.TypedStruct",
+      message, ancestor_is_sensitive, typed_struct_type,
       [message](Protobuf::Message* typed_message, const Protobuf::Reflection* reflection,
                 const Protobuf::FieldDescriptor* field_descriptor) {
         // To unpack a `TypedStruct`, convert the struct from JSON.
@@ -664,7 +665,8 @@ bool redactTypedStruct(Protobuf::Message* message, bool ancestor_is_sensitive) {
 // Recursive helper method for MessageUtil::redact() below.
 void redact(Protobuf::Message* message, bool ancestor_is_sensitive) {
   if (redactAny(message, ancestor_is_sensitive) ||
-      redactTypedStruct(message, ancestor_is_sensitive)) {
+      redactTypedStruct(message, "xds.type.v3.TypedStruct", ancestor_is_sensitive) ||
+      redactTypedStruct(message, "udpa.type.v1.TypedStruct", ancestor_is_sensitive)) {
     return;
   }
 

diff --git a/test/common/config/BUILD b/test/common/config/BUILD
@@ -421,6 +421,7 @@ envoy_cc_test(
         "//test/test_common:logging_lib",
         "//test/test_common:utility_lib",
         "@com_github_cncf_udpa//udpa/type/v1:pkg_cc_proto",
+        "@com_github_cncf_udpa//xds/type/v3:pkg_cc_proto",
         "@envoy_api//envoy/api/v2:pkg_cc_proto",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",

diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc
@@ -25,6 +25,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "udpa/type/v1/typed_struct.pb.h"
+#include "xds/type/v3/typed_struct.pb.h"
 
 using testing::Ref;
 using testing::Return;
@@ -328,37 +329,44 @@ TEST(UtilityTest, TranslateAnyToFactoryConfig) {
   EXPECT_THAT(*config, ProtoEq(source_duration));
 }
 
-void packTypedStructIntoAny(ProtobufWkt::Any& typed_config, const Protobuf::Message& inner) {
-  udpa::type::v1::TypedStruct typed_struct;
-  (*typed_struct.mutable_type_url()) =
-      absl::StrCat("type.googleapis.com/", inner.GetDescriptor()->full_name());
-  MessageUtil::jsonConvert(inner, *typed_struct.mutable_value());
-  typed_config.PackFrom(typed_struct);
-}
+template <typename T> class UtilityTypedStructTest : public ::testing::Test {
+public:
+  static void packTypedStructIntoAny(ProtobufWkt::Any& typed_config,
+                                     const Protobuf::Message& inner) {
+    T typed_struct;
+    (*typed_struct.mutable_type_url()) =
+        absl::StrCat("type.googleapis.com/", inner.GetDescriptor()->full_name());
+    MessageUtil::jsonConvert(inner, *typed_struct.mutable_value());
+    typed_config.PackFrom(typed_struct);
+  }
+};
+
+using TypedStructTypes = ::testing::Types<xds::type::v3::TypedStruct, udpa::type::v1::TypedStruct>;
+TYPED_TEST_SUITE(UtilityTypedStructTest, TypedStructTypes);
 
-// Verify that udpa.type.v1.TypedStruct can be translated into google.protobuf.Struct
-TEST(UtilityTest, TypedStructToStruct) {
+// Verify that TypedStruct can be translated into google.protobuf.Struct
+TYPED_TEST(UtilityTypedStructTest, TypedStructToStruct) {
   ProtobufWkt::Any typed_config;
   ProtobufWkt::Struct untyped_struct;
   (*untyped_struct.mutable_fields())["foo"].set_string_value("bar");
-  packTypedStructIntoAny(typed_config, untyped_struct);
+  this->packTypedStructIntoAny(typed_config, untyped_struct);
 
   ProtobufWkt::Struct out;
   Utility::translateOpaqueConfig(typed_config, ProtobufMessage::getStrictValidationVisitor(), out);
 
   EXPECT_THAT(out, ProtoEq(untyped_struct));
 }
 
-// Verify that udpa.type.v1.TypedStruct can be translated into an arbitrary message of correct type
+// Verify that TypedStruct can be translated into an arbitrary message of correct type
 // (v2 API, no upgrading).
-TEST(UtilityTest, TypedStructToClusterV2) {
+TYPED_TEST(UtilityTypedStructTest, TypedStructToClusterV2) {
   ProtobufWkt::Any typed_config;
   API_NO_BOOST(envoy::api::v2::Cluster) cluster;
   const std::string cluster_config_yaml = R"EOF(
     drain_connections_on_host_removal: true
   )EOF";
   TestUtility::loadFromYaml(cluster_config_yaml, cluster);
-  packTypedStructIntoAny(typed_config, cluster);
+  this->packTypedStructIntoAny(typed_config, cluster);
 
   {
     API_NO_BOOST(envoy::api::v2::Cluster) out;
@@ -373,16 +381,16 @@ TEST(UtilityTest, TypedStructToClusterV2) {
   }
 }
 
-// Verify that udpa.type.v1.TypedStruct can be translated into an arbitrary message of correct type
+// Verify that TypedStruct can be translated into an arbitrary message of correct type
 // (v3 API, upgrading).
-TEST(UtilityTest, TypedStructToClusterV3) {
+TYPED_TEST(UtilityTypedStructTest, TypedStructToClusterV3) {
   ProtobufWkt::Any typed_config;
   API_NO_BOOST(envoy::config::cluster::v3::Cluster) cluster;
   const std::string cluster_config_yaml = R"EOF(
     ignore_health_on_host_removal: true
   )EOF";
   TestUtility::loadFromYaml(cluster_config_yaml, cluster);
-  packTypedStructIntoAny(typed_config, cluster);
+  this->packTypedStructIntoAny(typed_config, cluster);
 
   {
     API_NO_BOOST(envoy::config::cluster::v3::Cluster) out;
@@ -397,6 +405,30 @@ TEST(UtilityTest, TypedStructToClusterV3) {
   }
 }
 
+// Verify that translation from TypedStruct into message of incorrect type fails
+TYPED_TEST(UtilityTypedStructTest, TypedStructToInvalidType) {
+  ProtobufWkt::Any typed_config;
+  envoy::config::bootstrap::v3::Bootstrap bootstrap;
+  const std::string bootstrap_config_yaml = R"EOF(
+    admin:
+      access_log:
+      - name: envoy.access_loggers.file
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
+          path: /dev/null
+      address:
+        pipe:
+          path: "/"
+  )EOF";
+  TestUtility::loadFromYaml(bootstrap_config_yaml, bootstrap);
+  this->packTypedStructIntoAny(typed_config, bootstrap);
+
+  ProtobufWkt::Any out;
+  EXPECT_THROW_WITH_REGEX(Utility::translateOpaqueConfig(
+                              typed_config, ProtobufMessage::getStrictValidationVisitor(), out),
+                          EnvoyException, "Unable to parse JSON as proto");
+}
+
 // Verify that Any can be translated into an arbitrary message of correct type
 // (v2 API, no upgrading).
 TEST(UtilityTest, AnyToClusterV2) {
@@ -429,30 +461,6 @@ TEST(UtilityTest, AnyToClusterV3) {
   EXPECT_THAT(out, ProtoEq(cluster));
 }
 
-// Verify that translation from udpa.type.v1.TypedStruct into message of incorrect type fails
-TEST(UtilityTest, TypedStructToInvalidType) {
-  ProtobufWkt::Any typed_config;
-  envoy::config::bootstrap::v3::Bootstrap bootstrap;
-  const std::string bootstrap_config_yaml = R"EOF(
-    admin:
-      access_log:
-      - name: envoy.access_loggers.file
-        typed_config:
-          "@type": type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
-          path: /dev/null
-      address:
-        pipe:
-          path: "/"
-  )EOF";
-  TestUtility::loadFromYaml(bootstrap_config_yaml, bootstrap);
-  packTypedStructIntoAny(typed_config, bootstrap);
-
-  ProtobufWkt::Any out;
-  EXPECT_THROW_WITH_REGEX(Utility::translateOpaqueConfig(
-                              typed_config, ProtobufMessage::getStrictValidationVisitor(), out),
-                          EnvoyException, "Unable to parse JSON as proto");
-}
-
 // Verify that ProtobufWkt::Empty can load into a typed factory with an empty config proto
 TEST(UtilityTest, EmptyToEmptyConfig) {
   ProtobufWkt::Any typed_config;

diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
@@ -39,6 +39,7 @@
 #include "absl/container/node_hash_set.h"
 #include "gtest/gtest.h"
 #include "udpa/type/v1/typed_struct.pb.h"
+#include "xds/type/v3/typed_struct.pb.h"
 
 using namespace std::chrono_literals;
 
@@ -999,50 +1000,55 @@ TEST_F(ProtobufUtilityTest, RedactTypedStruct) {
   EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
 }
 
+template <typename T> class TypedStructUtilityTest : public ProtobufUtilityTest {};
+
+using TypedStructTypes = ::testing::Types<xds::type::v3::TypedStruct, udpa::type::v1::TypedStruct>;
+TYPED_TEST_SUITE(TypedStructUtilityTest, TypedStructTypes);
+
 // Empty `TypedStruct` can be trivially redacted.
-TEST_F(ProtobufUtilityTest, RedactEmptyTypedStruct) {
-  udpa::type::v1::TypedStruct actual;
+TYPED_TEST(TypedStructUtilityTest, RedactEmptyTypedStruct) {
+  TypeParam actual;
   TestUtility::loadFromYaml(R"EOF(
 type_url: type.googleapis.com/envoy.test.Sensitive
 )EOF",
                             actual);
 
-  udpa::type::v1::TypedStruct expected = actual;
+  TypeParam expected = actual;
   MessageUtil::redact(actual);
   EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
 }
 
-TEST_F(ProtobufUtilityTest, RedactTypedStructWithNoTypeUrl) {
-  udpa::type::v1::TypedStruct actual;
+TYPED_TEST(TypedStructUtilityTest, RedactTypedStructWithNoTypeUrl) {
+  TypeParam actual;
   TestUtility::loadFromYaml(R"EOF(
 value:
   sensitive_string: This field is sensitive, but we have no way of knowing.
 )EOF",
                             actual);
 
-  udpa::type::v1::TypedStruct expected = actual;
+  TypeParam expected = actual;
   MessageUtil::redact(actual);
   EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
 }
 
 // Messages packed into `TypedStruct` with unknown type URLs are skipped.
-TEST_F(ProtobufUtilityTest, RedactTypedStructWithUnknownTypeUrl) {
-  udpa::type::v1::TypedStruct actual;
+TYPED_TEST(TypedStructUtilityTest, RedactTypedStructWithUnknownTypeUrl) {
+  TypeParam actual;
   TestUtility::loadFromYaml(R"EOF(
 type_url: type.googleapis.com/envoy.unknown.Message
 value:
   sensitive_string: This field is sensitive, but we have no way of knowing.
 )EOF",
                             actual);
 
-  udpa::type::v1::TypedStruct expected = actual;
+  TypeParam expected = actual;
   MessageUtil::redact(actual);
   EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
 }
 
-TEST_F(ProtobufUtilityTest, RedactEmptyTypeUrlTypedStruct) {
-  udpa::type::v1::TypedStruct actual;
-  udpa::type::v1::TypedStruct expected = actual;
+TYPED_TEST(TypedStructUtilityTest, RedactEmptyTypeUrlTypedStruct) {
+  TypeParam actual;
+  TypeParam expected = actual;
   MessageUtil::redact(actual);
   EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
 }

diff --git a/test/common/watchdog/abort_action_config_test.cc b/test/common/watchdog/abort_action_config_test.cc
@@ -28,7 +28,7 @@ TEST(AbortActionFactoryTest, CanCreateAction) {
           "config": {
             "name": "envoy.watchdog.abort_action",
             "typed_config": {
-              "@type": "type.googleapis.com/udpa.type.v1.TypedStruct",
+              "@type": "type.googleapis.com/xds.type.v3.TypedStruct",
               "type_url": "type.googleapis.com/envoy.watchdog.abort_action.v3.AbortActionConfig",
               "value": {
                 "wait_duration": "2s",

diff --git a/test/config/integration/server_xds.lds.typed_struct.yaml b/test/config/integration/server_xds.lds.typed_struct.yaml
@@ -10,7 +10,7 @@ resources:
   - filters:
     - name: http
       typed_config:
-        "@type": type.googleapis.com/udpa.type.v1.TypedStruct
+        "@type": type.googleapis.com/xds.type.v3.TypedStruct
         type_url: "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"
         value:
           codec_type: HTTP2

diff --git a/test/config/integration/server_xds.lds.with_unknown_field.typed_struct.yaml b/test/config/integration/server_xds.lds.with_unknown_field.typed_struct.yaml
@@ -10,7 +10,7 @@ resources:
   - filters:
     - name: http
       typed_config:
-        "@type": type.googleapis.com/udpa.type.v1.TypedStruct
+        "@type": type.googleapis.com/xds.type.v3.TypedStruct
         type_url: "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"
         value:
           codec_type: HTTP2