envoyproxy · alyssawilk · Feb 4, 2019 · Jan 29, 2019 · Jan 29, 2019 · Jan 29, 2019
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -7,6 +7,7 @@ Version history
 * admin: the admin server can now be accessed via HTTP/2 (prior knowledge).
 * buffer: fix vulnerabilities when allocation fails.
 * config: added support of using google.protobuf.Any in opaque configs for extensions.
+* config: logging warnings when deprecated fields are in use.
 * config: removed deprecated --v2-config-only from command line config.
 * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config <config_overview_v2_bootstrap>`.
 * config: removed REST_LEGACY as a valid :ref:`ApiType <envoy_api_field_core.ApiConfigSource.api_type>`.

diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -106,6 +106,46 @@ void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& messa
   }
 }
 
+void MessageUtil::checkForDeprecation(const Protobuf::Message& message, bool warn_only) {
+  const Protobuf::Descriptor* descriptor = message.GetDescriptor();
+  const Protobuf::Reflection* reflection = message.GetReflection();
+  for (int i = 0; i < descriptor->field_count(); ++i) {
+    const auto* field = descriptor->field(i);
+
+    // If this field is not in use, continue.
+    if ((field->is_repeated() && reflection->FieldSize(message, field) == 0) ||
+        (!field->is_repeated() && !reflection->HasField(message, field))) {
+      continue;
+    }
+
+    // If this field is deprecated, warn or throw an error.
+    if (field->options().deprecated()) {
+      std::string err = fmt::format(
+          "Using deprecated option '{}'. This configuration will be removed from Envoy soon. "
+          "Please see https://github.com/envoyproxy/envoy/blob/master/DEPRECATED.md for "
+          "details.",
+          field->full_name());
+      if (warn_only) {
+        ENVOY_LOG_MISC(warn, "{}", err);
+      } else {
+        throw ProtoValidationException(err, message);
+      }
+    }
+
+    // If this is a message, recurse to check for deprecated fields in the sub-message.
+    if (field->cpp_type() == Protobuf::FieldDescriptor::CPPTYPE_MESSAGE) {
+      if (field->is_repeated()) {
+        const int size = reflection->FieldSize(message, field);
+        for (int j = 0; j < size; ++j) {
+          checkForDeprecation(reflection->GetRepeatedMessage(message, field, j), warn_only);
+        }
+      } else {
+        checkForDeprecation(reflection->GetMessage(message, field), warn_only);
+      }
+    }
+  }
+}
+
 std::string MessageUtil::getJsonStringFromMessage(const Protobuf::Message& message,
                                                   const bool pretty_print,
                                                   const bool always_print_primitive_fields) {

diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
@@ -178,6 +178,15 @@ class MessageUtil {
   static void loadFromYaml(const std::string& yaml, Protobuf::Message& message);
   static void loadFromFile(const std::string& path, Protobuf::Message& message, Api::Api& api);
 
+  /**
+   * Checks for use of deprecated fields in message and all sub-messages.
+   * @param message message to validate.
+   * @param warn_only if true, logs a warning rather than throwing an exception if deprecated fields
+   *   are in use.
+   * @throw ProtoValidationException if deprecated fields are used and warn_only is false.
+   */
+  static void checkForDeprecation(const Protobuf::Message& message, bool warn_only);
+
   /**
    * Validate protoc-gen-validate constraints on a given protobuf.
    * Note the corresponding `.pb.validate.h` for the message has to be included in the source file
@@ -186,6 +195,9 @@ class MessageUtil {
    * @throw ProtoValidationException if the message does not satisfy its type constraints.
    */
   template <class MessageType> static void validate(const MessageType& message) {
+    // Log warnings if deprecated fields are in use.
+    checkForDeprecation(message, true);
+
     std::string err;
     if (!Validate(message, &err)) {
       throw ProtoValidationException(err, message);

diff --git a/test/common/protobuf/BUILD b/test/common/protobuf/BUILD
@@ -15,7 +15,9 @@ envoy_cc_test(
     deps = [
         "//source/common/protobuf:utility_lib",
         "//source/common/stats:isolated_store_lib",
+        "//test/proto:deprecated_proto_cc",
         "//test/test_common:environment_lib",
+        "//test/test_common:logging_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc",
     ],

diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
@@ -7,7 +7,9 @@
 #include "common/protobuf/utility.h"
 #include "common/stats/isolated_store_impl.h"
 
+#include "test/proto/deprecated.pb.h"
 #include "test/test_common/environment.h"
+#include "test/test_common/logging.h"
 #include "test/test_common/test_base.h"
 #include "test/test_common/utility.h"
 
@@ -337,6 +339,72 @@ TEST(DurationUtilTest, OutOfRange) {
   }
 }
 
+TEST(DeprecatedFields, NoErrorWhenDeprecatedFieldsUnused) {
+  envoy::test::deprecation_test::Base base;
+  base.set_not_deprecated("foo");
+  // Fatal checks for a non-deprecated field should cause no problem.
+  MessageUtil::checkForDeprecation(base, true);
+}
+
+TEST(DeprecatedFields, IndividualFieldDeprecated) {
+  envoy::test::deprecation_test::Base base;
+  base.set_is_deprecated("foo");
+  // Non-fatal checks for a deprecated field should log rather than throw an exception.
+  EXPECT_LOG_CONTAINS("warning",
+                      "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'.",
+                      MessageUtil::checkForDeprecation(base, true));
+  // Fatal checks for a deprecated field should result in an exception.
+  EXPECT_THROW_WITH_REGEX(
+      MessageUtil::checkForDeprecation(base, false), ProtoValidationException,
+      "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'.");
+}
+
+TEST(DeprecatedFields, MessageDeprecated) {
+  envoy::test::deprecation_test::Base base;
+  base.mutable_deprecated_message();
+  // Fatal checks for a present (unused) deprecated message should result in an exception.
+  EXPECT_THROW_WITH_REGEX(
+      MessageUtil::checkForDeprecation(base, false), ProtoValidationException,
+      "Using deprecated option 'envoy.test.deprecation_test.Base.deprecated_message'.");
+}
+
+TEST(DeprecatedFields, InnerMessageDeprecated) {
+  envoy::test::deprecation_test::Base base;
+  base.mutable_not_deprecated_message()->set_inner_not_deprecated("foo");
+  // Non-fatal checks for a deprecated field shouldn't throw an exception.
+  MessageUtil::checkForDeprecation(base, true);
+
+  base.mutable_not_deprecated_message()->set_inner_deprecated("bar");
+  // Fatal checks for a deprecated sub-message should result in an exception.
+  EXPECT_THROW_WITH_REGEX(MessageUtil::checkForDeprecation(base, false), ProtoValidationException,
+                          "Using deprecated option "
+                          "'envoy.test.deprecation_test.Base.InnerMessage.inner_deprecated'.");
+}
+
+// Check that repeated sub-messages get validated.
+TEST(DeprecatedFields, SubMessageDeprecated) {
+  envoy::test::deprecation_test::Base base;
+  base.add_repeated_message();
+  base.add_repeated_message()->set_inner_deprecated("foo");
+  base.add_repeated_message();
+
+  // Fatal checks for a repeated deprecated sub-message should result in an exception.
+  EXPECT_THROW_WITH_REGEX(MessageUtil::checkForDeprecation(base, false), ProtoValidationException,
+                          "Using deprecated option "
+                          "'envoy.test.deprecation_test.Base.InnerMessage.inner_deprecated'.");
+}
+
+// Check that deprecated repeated messages trigger
+TEST(DeprecatedFields, RepeatedMessageDeprecated) {
+  envoy::test::deprecation_test::Base base;
+  base.add_deprecated_repeated_message();
+
+  // Fatal checks for a repeated deprecated sub-message should result in an exception.
+  EXPECT_THROW_WITH_REGEX(
+      MessageUtil::checkForDeprecation(base, false), ProtoValidationException,
+      "Using deprecated option 'envoy.test.deprecation_test.Base.deprecated_repeated_message'.");
+}
+
 class TimestampUtilTest : public TestBase, public ::testing::WithParamInterface<int64_t> {};
 
 TEST_P(TimestampUtilTest, SystemClockToTimestampTest) {

diff --git a/test/proto/BUILD b/test/proto/BUILD
@@ -11,6 +11,11 @@ envoy_package()
 
 exports_files(["bookstore.proto"])
 
+envoy_proto_library(
+    name = "deprecated_proto",
+    srcs = [":deprecated.proto"],
+)
+
 envoy_proto_library(
     name = "helloworld_proto",
     srcs = [":helloworld.proto"],

diff --git a/test/proto/deprecated.proto b/test/proto/deprecated.proto
@@ -0,0 +1,16 @@
+syntax = "proto3";
+
+package envoy.test.deprecation_test;
+
+message Base {
+  string not_deprecated = 1;
+  string is_deprecated = 2 [deprecated = true];
+  message InnerMessage {
+    string inner_not_deprecated = 1;
+    string inner_deprecated = 2 [deprecated = true];
+  }
+  InnerMessage deprecated_message = 3 [deprecated = true];
+  InnerMessage not_deprecated_message = 4;
+  repeated InnerMessage repeated_message = 5;
+  repeated InnerMessage deprecated_repeated_message = 6 [deprecated = true];
+}