build: add API_NO_BOOST* annotations. (envoyproxy#9480)

These allow specific types, expressions and files to be excluded from API boosting via annotation. API_NO_BOOST_FILE (anywhere in the file) will skip boosting of the file, API_NO_BOOST() wrapped around an expression or type will bypass boosting of that text. The idea is that there are some types that we do not ever want to upgrade as they are relevant to the cross-version wire impedance matching, or testing v2 compat when the tree is v3alpha. The sites identified in this PR were taken from my WiP in which I have the entire tree boosted and tests passing. It's possible we might need to tune some more later, but this should go a reasonable way towards reducing the amount of manual fixups required after boosting to get tests passing again. Risk level: Low (macros are nops) Testing: bazel test //test/..., new api_booster golden tests. Part of envoyproxy#8082 Signed-off-by: Harvey Tuch <[email protected]> Signed-off-by: Prakhar <[email protected]>
prakhag1 · Jan 3, 2020 · 660f860 · 660f860
1 parent 4f6566e
commit 660f860
Show file tree

Hide file tree

Showing 39 changed files with 185 additions and 59 deletions.
diff --git a/source/common/config/BUILD b/source/common/config/BUILD
@@ -20,6 +20,11 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "api_version_lib",
+    hdrs = ["api_version.h"],
+)
+
 envoy_cc_library(
     name = "config_provider_lib",
     srcs = ["config_provider_impl.cc"],

diff --git a/source/common/config/api_version.h b/source/common/config/api_version.h
@@ -0,0 +1,7 @@
+#pragma once
+
+// Use this to force a specific version of a given config proto, preventing API
+// boosting from modifying it. E.g. API_NO_BOOST(envoy::api::v2::Cluster).
+#define API_NO_BOOST(x) x
+
+namespace Envoy {}
diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc
@@ -11,6 +11,8 @@
 
 #include "common/grpc/common.h"
 
+// API_NO_BOOST_FILE
+
 namespace Envoy {
 namespace Config {
 

diff --git a/source/common/router/BUILD b/source/common/router/BUILD
@@ -131,6 +131,7 @@ envoy_cc_library(
         "//include/envoy/thread_local:thread_local_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:utility_lib",
         "//source/common/init:target_lib",
         "//source/common/protobuf:utility_lib",
@@ -160,6 +161,7 @@ envoy_cc_library(
         "//source/common/common:callback_impl_lib",
         "//source/common/common:cleanup_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/config:utility_lib",
         "//source/common/init:manager_lib",
@@ -205,6 +207,7 @@ envoy_cc_library(
         "//source/common/common:assert_lib",
         "//source/common/common:cleanup_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:config_provider_lib",
         "//source/common/init:manager_lib",
         "//source/common/init:watcher_lib",

diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc
@@ -13,6 +13,7 @@
 
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
+#include "common/config/api_version.h"
 #include "common/config/utility.h"
 #include "common/protobuf/utility.h"
 #include "common/router/config_impl.h"
@@ -73,7 +74,8 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription(
   subscription_ =
       factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
           rds.config_source(),
-          Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()),
+          Grpc::Common::typeUrl(
+              API_NO_BOOST(envoy::api::v2::RouteConfiguration)().GetDescriptor()->full_name()),
           *scope_, *this);
   config_update_info_ =
       std::make_unique<RouteConfigUpdateReceiverImpl>(factory_context.timeSource(), validator);

diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc
@@ -13,6 +13,7 @@
 #include "common/common/cleanup.h"
 #include "common/common/logger.h"
 #include "common/common/utility.h"
+#include "common/config/api_version.h"
 #include "common/config/resources.h"
 #include "common/init/manager_impl.h"
 #include "common/init/watcher_impl.h"
@@ -102,8 +103,9 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription(
   subscription_ =
       factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
           scoped_rds.scoped_rds_config_source(),
-          Grpc::Common::typeUrl(
-              envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()),
+          Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::ScopedRouteConfiguration)()
+                                    .GetDescriptor()
+                                    ->full_name()),
           *scope_, *this);
 
   initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr {

diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc
@@ -11,6 +11,7 @@
 
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
+#include "common/config/api_version.h"
 #include "common/config/utility.h"
 #include "common/protobuf/utility.h"
 #include "common/router/config_impl.h"
@@ -42,7 +43,8 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info,
   subscription_ =
       factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
           config_update_info_->routeConfiguration().vhds().config_source(),
-          Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()),
+          Grpc::Common::typeUrl(
+              API_NO_BOOST(envoy::api::v2::route::VirtualHost)().GetDescriptor()->full_name()),
           *scope_, *this);
 }
 

diff --git a/source/common/runtime/BUILD b/source/common/runtime/BUILD
@@ -32,6 +32,7 @@ envoy_cc_library(
         "//source/common/common:minimal_logger_lib",
         "//source/common/common:thread_lib",
         "//source/common/common:utility_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/filesystem:directory_lib",
         "//source/common/grpc:common_lib",
         "//source/common/init:target_lib",

diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc
@@ -17,6 +17,7 @@
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
 #include "common/common/utility.h"
+#include "common/config/api_version.h"
 #include "common/filesystem/directory.h"
 #include "common/grpc/common.h"
 #include "common/protobuf/message_validator_impl.h"
@@ -590,7 +591,8 @@ void RtdsSubscription::start() {
   // instantiated in the server instance.
   subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource(
       config_source_,
-      Grpc::Common::typeUrl(envoy::service::discovery::v2::Runtime().GetDescriptor()->full_name()),
+      Grpc::Common::typeUrl(
+          API_NO_BOOST(envoy::service::discovery::v2::Runtime)().GetDescriptor()->full_name()),
       store_, *this);
   subscription_->start({resource_name_});
 }

diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD
@@ -53,6 +53,7 @@ envoy_cc_library(
         "//include/envoy/stats:stats_interface",
         "//source/common/common:callback_impl_lib",
         "//source/common/common:cleanup_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:resources_lib",
         "//source/common/config:utility_lib",
         "//source/common/init:target_lib",

diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc
@@ -7,6 +7,7 @@
 #include "envoy/api/v2/core/config_source.pb.h"
 #include "envoy/api/v2/discovery.pb.h"
 
+#include "common/config/api_version.h"
 #include "common/config/resources.h"
 #include "common/protobuf/utility.h"
 
@@ -81,8 +82,9 @@ void SdsApi::validateUpdateSize(int num_resources) {
 void SdsApi::initialize() {
   subscription_ = subscription_factory_.subscriptionFromConfigSource(
       sds_config_,
-      Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), stats_,
-      *this);
+      Grpc::Common::typeUrl(
+          API_NO_BOOST(envoy::api::v2::auth::Secret)().GetDescriptor()->full_name()),
+      stats_, *this);
   subscription_->start({sds_config_name_});
 }
 

diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD
@@ -18,6 +18,7 @@ envoy_cc_library(
         "//include/envoy/local_info:local_info_interface",
         "//source/common/common:cleanup_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:resources_lib",
         "//source/common/config:utility_lib",
         "//source/common/protobuf:utility_lib",
@@ -347,6 +348,7 @@ envoy_cc_library(
         "//include/envoy/secret:secret_manager_interface",
         "//include/envoy/upstream:cluster_factory_interface",
         "//include/envoy/upstream:locality_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:metadata_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/config:utility_lib",

diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc
@@ -10,6 +10,7 @@
 
 #include "common/common/cleanup.h"
 #include "common/common/utility.h"
+#include "common/config/api_version.h"
 #include "common/config/resources.h"
 #include "common/config/utility.h"
 #include "common/protobuf/utility.h"
@@ -30,7 +31,8 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, Clu
     : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")),
       validation_visitor_(validation_visitor) {
   subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource(
-      cds_config, Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()),
+      cds_config,
+      Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::Cluster)().GetDescriptor()->full_name()),
       *scope_, *this);
 }
 

diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc
@@ -7,6 +7,7 @@
 #include "envoy/api/v2/eds.pb.validate.h"
 
 #include "common/common/utility.h"
+#include "common/config/api_version.h"
 
 namespace Envoy {
 namespace Upstream {
@@ -35,7 +36,7 @@ EdsClusterImpl::EdsClusterImpl(
       factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
           eds_config,
           Grpc::Common::typeUrl(
-              envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name()),
+              API_NO_BOOST(envoy::api::v2::ClusterLoadAssignment)().GetDescriptor()->full_name()),
           info_->statsScope(), *this);
 }
 

diff --git a/source/server/BUILD b/source/server/BUILD
@@ -253,6 +253,7 @@ envoy_cc_library(
         "//include/envoy/init:manager_interface",
         "//include/envoy/server:listener_manager_interface",
         "//source/common/common:cleanup_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:resources_lib",
         "//source/common/config:utility_lib",
         "//source/common/init:target_lib",

diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc
@@ -10,6 +10,7 @@
 #include "envoy/stats/scope.h"
 
 #include "common/common/cleanup.h"
+#include "common/config/api_version.h"
 #include "common/config/resources.h"
 #include "common/config/utility.h"
 #include "common/protobuf/utility.h"
@@ -27,7 +28,8 @@ LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config,
       init_target_("LDS", [this]() { subscription_->start({}); }),
       validation_visitor_(validation_visitor) {
   subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource(
-      lds_config, Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()),
+      lds_config,
+      Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::Listener)().GetDescriptor()->full_name()),
       *scope_, *this);
   init_manager.add(init_target_);
 }

diff --git a/test/common/config/BUILD b/test/common/config/BUILD
@@ -25,6 +25,7 @@ envoy_cc_test(
     srcs = ["delta_subscription_impl_test.cc"],
     deps = [
         ":delta_subscription_test_harness",
+        "//source/common/config:api_version_lib",
         "//source/common/config:delta_subscription_lib",
         "//source/common/stats:isolated_store_lib",
         "//test/mocks:common_lib",
@@ -89,6 +90,7 @@ envoy_cc_test(
     name = "grpc_mux_impl_test",
     srcs = ["grpc_mux_impl_test.cc"],
     deps = [
+        "//source/common/config:api_version_lib",
         "//source/common/config:grpc_mux_lib",
         "//source/common/config:protobuf_link_hacks",
         "//source/common/config:resources_lib",
@@ -157,6 +159,7 @@ envoy_cc_test_library(
     deps = [
         ":subscription_test_harness",
         "//source/common/common:hash_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:grpc_subscription_lib",
         "//source/common/config:resources_lib",
         "//test/mocks/config:config_mocks",

diff --git a/test/common/config/delta_subscription_impl_test.cc b/test/common/config/delta_subscription_impl_test.cc
@@ -3,6 +3,7 @@
 #include "envoy/api/v2/eds.pb.h"
 
 #include "common/buffer/zero_copy_input_stream_impl.h"
+#include "common/config/api_version.h"
 
 #include "test/common/config/delta_subscription_test_harness.h"
 
@@ -109,7 +110,7 @@ TEST_F(DeltaSubscriptionImplTest, PauseQueuesAcks) {
   // All ACK sendMessage()s will happen upon calling resume().
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, _))
       .WillRepeatedly(Invoke([this](Buffer::InstancePtr& buffer, bool) {
-        envoy::api::v2::DeltaDiscoveryRequest message;
+        API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) message;
         EXPECT_TRUE(Grpc::Common::parseBufferInstance(std::move(buffer), message));
         const std::string nonce = message.response_nonce();
         if (!nonce.empty()) {

diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc
@@ -4,6 +4,7 @@
 #include "envoy/api/v2/eds.pb.h"
 
 #include "common/common/empty_string.h"
+#include "common/config/api_version.h"
 #include "common/config/grpc_mux_impl.h"
 #include "common/config/protobuf_link_hacks.h"
 #include "common/config/resources.h"
@@ -68,7 +69,7 @@ class GrpcMuxImplTestBase : public testing::Test {
                          bool first = false, const std::string& nonce = "",
                          const Protobuf::int32 error_code = Grpc::Status::WellKnownGrpcStatus::Ok,
                          const std::string& error_message = "") {
-    envoy::api::v2::DiscoveryRequest expected_request;
+    API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request;
     if (first) {
       expected_request.mutable_node()->CopyFrom(local_info_.node());
     }

diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h
@@ -7,6 +7,7 @@
 #include "envoy/api/v2/eds.pb.h"
 
 #include "common/common/hash.h"
+#include "common/config/api_version.h"
 #include "common/config/grpc_subscription_impl.h"
 #include "common/config/resources.h"
 
@@ -62,7 +63,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness {
                          bool expect_node, const Protobuf::int32 error_code,
                          const std::string& error_message) {
     UNREFERENCED_PARAMETER(expect_node);
-    envoy::api::v2::DiscoveryRequest expected_request;
+    API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request;
     if (expect_node) {
       expected_request.mutable_node()->CopyFrom(node_);
     }

diff --git a/test/common/router/BUILD b/test/common/router/BUILD
@@ -104,6 +104,7 @@ envoy_cc_test(
     deps = [
         "//include/envoy/config:subscription_interface",
         "//include/envoy/init:manager_interface",
+        "//source/common/config:api_version_lib",
         "//source/common/config:utility_lib",
         "//source/common/http:message_lib",
         "//source/common/json:json_loader_lib",

diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc
@@ -11,6 +11,7 @@
 #include "envoy/init/manager.h"
 #include "envoy/stats/scope.h"
 
+#include "common/config/api_version.h"
 #include "common/config/grpc_mux_impl.h"
 #include "common/router/scoped_rds.h"
 
@@ -122,12 +123,13 @@ class ScopedRdsTest : public ScopedRoutesTestBase {
                 subscriptionFromConfigSource(_, _, _, _))
         .Times(AnyNumber());
     // rds subscription
-    EXPECT_CALL(server_factory_context_.cluster_manager_.subscription_factory_,
-                subscriptionFromConfigSource(
-                    _,
-                    Eq(Grpc::Common::typeUrl(
-                        envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name())),
-                    _, _))
+    EXPECT_CALL(
+        server_factory_context_.cluster_manager_.subscription_factory_,
+        subscriptionFromConfigSource(
+            _,
+            Eq(Grpc::Common::typeUrl(
+                API_NO_BOOST(envoy::api::v2::RouteConfiguration)().GetDescriptor()->full_name())),
+            _, _))
         .Times(AnyNumber())
         .WillRepeatedly(Invoke([this](const envoy::api::v2::core::ConfigSource&, absl::string_view,
                                       Stats::Scope&,

diff --git a/test/integration/BUILD b/test/integration/BUILD
@@ -217,6 +217,7 @@ envoy_cc_test(
     ],
     deps = [
         ":http_integration_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/protobuf",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/api/v2:pkg_cc_proto",
@@ -482,6 +483,7 @@ envoy_cc_test_library(
         "//source/common/buffer:zero_copy_input_stream_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/event:dispatcher_lib",
         "//source/common/grpc:codec_lib",
         "//source/common/grpc:common_lib",
@@ -769,6 +771,7 @@ envoy_cc_test(
     ],
     deps = [
         ":http_integration_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:protobuf_link_hacks",
         "//source/common/config:resources_lib",
         "//source/common/event:dispatcher_includes",
@@ -801,6 +804,7 @@ envoy_cc_test(
     ],
     deps = [
         ":integration_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/event:dispatcher_includes",
         "//source/common/event:dispatcher_lib",
         "//source/common/network:utility_lib",
@@ -956,6 +960,7 @@ envoy_cc_test(
     ],
     deps = [
         ":http_integration_lib",
+        "//source/common/config:api_version_lib",
         "//source/common/config:resources_lib",
         "//source/common/event:dispatcher_includes",
         "//source/common/event:dispatcher_lib",