From 660f8606417bc7bae54cd0b93bfd917240496138 Mon Sep 17 00:00:00 2001 From: htuch Date: Thu, 26 Dec 2019 08:43:21 -0500 Subject: [PATCH] build: add API_NO_BOOST* annotations. (#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 #8082 Signed-off-by: Harvey Tuch Signed-off-by: Prakhar --- source/common/config/BUILD | 5 +++ source/common/config/api_version.h | 7 ++++ source/common/config/type_to_endpoint.cc | 2 + source/common/router/BUILD | 3 ++ source/common/router/rds_impl.cc | 4 +- source/common/router/scoped_rds.cc | 6 ++- source/common/router/vhds.cc | 4 +- source/common/runtime/BUILD | 1 + source/common/runtime/runtime_impl.cc | 4 +- source/common/secret/BUILD | 1 + source/common/secret/sds_api.cc | 6 ++- source/common/upstream/BUILD | 2 + source/common/upstream/cds_api_impl.cc | 4 +- source/common/upstream/eds.cc | 3 +- source/server/BUILD | 1 + source/server/lds_api.cc | 4 +- test/common/config/BUILD | 3 ++ .../config/delta_subscription_impl_test.cc | 3 +- test/common/config/grpc_mux_impl_test.cc | 3 +- .../config/grpc_subscription_test_harness.h | 3 +- test/common/router/BUILD | 1 + test/common/router/scoped_rds_test.cc | 14 ++++--- test/integration/BUILD | 5 +++ test/integration/header_integration_test.cc | 9 +++-- test/integration/integration.cc | 7 ++-- test/integration/integration.h | 5 ++- .../scoped_rds_integration_test.cc | 7 ++-- .../sds_dynamic_integration_test.cc | 3 +- .../integration/tcp_proxy_integration_test.cc | 25 ++++++------ tools/api_boost/api_boost.py | 7 ++++ tools/api_boost/api_boost_test.py | 1 + tools/api_boost/testdata/BUILD | 6 +++ tools/api_boost/testdata/decl_ref_expr.cc | 5 +++ .../api_boost/testdata/decl_ref_expr.cc.gold | 6 +++ tools/api_boost/testdata/no_boost_file.cc | 12 ++++++ .../api_boost/testdata/no_boost_file.cc.gold | 12 ++++++ tools/clang_tools/api_booster/main.cc | 40 ++++++++++++------- tools/type_whisperer/api_type_db.cc | 9 +++++ tools/type_whisperer/api_type_db.h | 1 + 39 files changed, 185 insertions(+), 59 deletions(-) create mode 100644 source/common/config/api_version.h create mode 100644 tools/api_boost/testdata/no_boost_file.cc create mode 100644 tools/api_boost/testdata/no_boost_file.cc.gold diff --git a/source/common/config/BUILD b/source/common/config/BUILD index c8bbe5a64781..9b61f27df4ca 100644 --- 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 new file mode 100644 index 000000000000..fddef009aec0 --- /dev/null +++ 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 index e1b0170f7a2f..15e95d6d47eb 100644 --- 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 index 1034f252c039..4495396cfa43 100644 --- 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 index 98ef89251792..96c9952eac17 100644 --- 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(factory_context.timeSource(), validator); diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 83bb0c0e411a..ec1564fe802d 100644 --- 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 index 11efea2d4901..ac5fa000e24a 100644 --- 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 index d92492af69e2..e2abe6597a7e 100644 --- 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 index 7b51f36935b4..7dc5ed7bf843 100644 --- 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 index db35adccc31a..45b4f11f5d39 100644 --- 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 index 53d495f9d84a..8ecea2b50bf1 100644 --- 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 index 6993b0d28fa9..ad02bc359511 100644 --- 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 index 26bb6de76f64..2b9b25db9b85 100644 --- 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 index 285e9ea158ce..2c8dd16c19a0 100644 --- 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 index 5e4cc5a66719..88a73a5f69ce 100644 --- 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 index 3fadc71662bc..df19f72fc6dd 100644 --- 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 index 6e3369de52be..8490bd6923b5 100644 --- 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 index 4a3694d903ff..8f92ff09bf09 100644 --- 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 index c9fb873d6158..2cb73cdda08b 100644 --- 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 index 25dbff0c3fb3..08ac0165891e 100644 --- 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 index 66d722872999..69243a849536 100644 --- 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 index 796e2a795b24..7d50efa32eee 100644 --- 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 index 3c3d24948e36..6d01661a7e7c 100644 --- 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", diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 4c1a311e9a3f..0292571dcf75 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -6,6 +6,7 @@ #include "envoy/config/filter/http/router/v2/router.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" +#include "common/config/api_version.h" #include "common/config/metadata.h" #include "common/config/resources.h" #include "common/http/exception.h" @@ -368,16 +369,16 @@ class HeaderIntegrationTest RELEASE_ASSERT(result, result.message()); eds_stream_->startGrpcStream(); - envoy::api::v2::DiscoveryRequest discovery_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; result = eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); RELEASE_ASSERT(result, result.message()); - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info("1"); discovery_response.set_type_url(Config::TypeUrl::get().ClusterLoadAssignment); - envoy::api::v2::ClusterLoadAssignment cluster_load_assignment = - TestUtility::parseYaml(fmt::format( + auto cluster_load_assignment = + TestUtility::parseYaml(fmt::format( R"EOF( cluster_name: cluster_0 endpoints: diff --git a/test/integration/integration.cc b/test/integration/integration.cc index f41111d8c048..728c32e8a228 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -21,6 +21,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/common/stack_array.h" +#include "common/config/api_version.h" #include "common/event/dispatcher_impl.h" #include "common/event/libevent.h" #include "common/network/connection_impl.h" @@ -335,7 +336,7 @@ void BaseIntegrationTest::createEnvoy() { if (use_lds_) { // After the config has been finalized, write the final listener config to the lds file. const std::string lds_path = config_helper_.bootstrap().dynamic_resources().lds_config().path(); - envoy::api::v2::DiscoveryResponse lds; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) lds; lds.set_version_info("0"); for (auto& listener : config_helper_.bootstrap().static_resources().listeners()) { ProtobufWkt::Any* resource = lds.add_resources(); @@ -582,7 +583,7 @@ AssertionResult BaseIntegrationTest::compareSotwDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, const std::vector& expected_resource_names, bool expect_node, const Protobuf::int32 expected_error_code, const std::string& expected_error_substring) { - envoy::api::v2::DiscoveryRequest discovery_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); if (expect_node) { @@ -640,7 +641,7 @@ AssertionResult BaseIntegrationTest::compareDeltaDiscoveryRequest( const std::vector& expected_resource_subscriptions, const std::vector& expected_resource_unsubscriptions, FakeStreamPtr& xds_stream, const Protobuf::int32 expected_error_code, const std::string& expected_error_substring) { - envoy::api::v2::DeltaDiscoveryRequest request; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) request; VERIFY_ASSERTION(xds_stream->waitForGrpcMessage(*dispatcher_, request)); // Verify all we care about node. diff --git a/test/integration/integration.h b/test/integration/integration.h index b3839198b3a8..1297379c5042 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -9,6 +9,7 @@ #include "envoy/api/v2/endpoint/endpoint.pb.h" #include "envoy/server/process_context.h" +#include "common/config/api_version.h" #include "common/http/codec_client.h" #include "test/common/grpc/grpc_client_integration.h" @@ -271,7 +272,7 @@ class BaseIntegrationTest : protected Logger::Loggable { template void sendSotwDiscoveryResponse(const std::string& type_url, const std::vector& messages, const std::string& version) { - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info(version); discovery_response.set_type_url(type_url); for (const auto& message : messages) { @@ -293,7 +294,7 @@ class BaseIntegrationTest : protected Logger::Loggable { const std::vector& added_or_updated, const std::vector& removed, const std::string& version, FakeStreamPtr& stream) { - envoy::api::v2::DeltaDiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryResponse) response; response.set_system_version_info("system_version_info_this_is_a_test"); response.set_type_url(type_url); for (const auto& message : added_or_updated) { diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc index c1ac2d9c9f09..e59342544aa3 100644 --- a/test/integration/scoped_rds_integration_test.cc +++ b/test/integration/scoped_rds_integration_test.cc @@ -6,6 +6,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "test/common/grpc/grpc_client_integration.h" @@ -160,7 +161,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, } void sendRdsResponse(const std::string& route_config, const std::string& version) { - envoy::api::v2::DiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) response; response.set_version_info(version); response.set_type_url(Config::TypeUrl::get().RouteConfiguration); auto route_configuration = @@ -187,7 +188,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, const std::string& version) { ASSERT(scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_] != nullptr); - envoy::api::v2::DeltaDiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryResponse) response; response.set_system_version_info(version); response.set_type_url(Config::TypeUrl::get().ScopedRouteConfiguration); @@ -210,7 +211,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, const std::string& version) { ASSERT(scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_] != nullptr); - envoy::api::v2::DiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) response; response.set_version_info(version); response.set_type_url(Config::TypeUrl::get().ScopedRouteConfiguration); diff --git a/test/integration/sds_dynamic_integration_test.cc b/test/integration/sds_dynamic_integration_test.cc index 1027dd95304f..fe91152ef23b 100644 --- a/test/integration/sds_dynamic_integration_test.cc +++ b/test/integration/sds_dynamic_integration_test.cc @@ -7,6 +7,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/service/discovery/v2/sds.pb.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/event/dispatcher_impl.h" #include "common/network/connection_impl.h" @@ -117,7 +118,7 @@ class SdsDynamicIntegrationBaseTest : public Grpc::GrpcClientIntegrationParamTes } void sendSdsResponse(const envoy::api::v2::auth::Secret& secret) { - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info("1"); discovery_response.set_type_url(Config::TypeUrl::get().Secret); discovery_response.add_resources()->PackFrom(secret); diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 068530969862..1fa01b946972 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -8,6 +8,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.h" +#include "common/config/api_version.h" #include "common/network/utility.h" #include "extensions/transport_sockets/tls/context_manager_impl.h" @@ -240,10 +241,10 @@ TEST_P(TcpProxyIntegrationTest, AccessLog) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); auto* access_log = tcp_proxy_config.add_access_log(); access_log->set_name("envoy.file_access_log"); @@ -329,10 +330,10 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithNoData) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); tcp_proxy_config.mutable_idle_timeout()->set_nanos( std::chrono::duration_cast(std::chrono::milliseconds(100)) .count()); @@ -352,10 +353,10 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); tcp_proxy_config.mutable_idle_timeout()->set_nanos( std::chrono::duration_cast(std::chrono::milliseconds(500)) .count()); diff --git a/tools/api_boost/api_boost.py b/tools/api_boost/api_boost.py index c33bcbfbf519..7c22c9625859 100755 --- a/tools/api_boost/api_boost.py +++ b/tools/api_boost/api_boost.py @@ -32,6 +32,10 @@ def PrefixDirectory(path_prefix): # Update a C++ file to the latest API. def ApiBoostFile(llvm_include_path, debug_log, path): print('Processing %s' % path) + if 'API_NO_BOOST_FILE' in pathlib.Path(path).read_text(): + if debug_log: + print('Not boosting %s due to API_NO_BOOST_FILE\n' % path) + return None # Run the booster try: result = sp.run([ @@ -57,6 +61,9 @@ def ApiBoostFile(llvm_include_path, debug_log, path): # below, we have more control over special casing as well, so ¯\_(ツ)_/¯. def RewriteIncludes(args): path, api_includes = args + # Files with API_NO_BOOST_FILE will have None returned by ApiBoostFile. + if api_includes is None: + return # We just dump the inferred API header includes at the start of the #includes # in the file and remove all the present API header includes. This does not # match Envoy style; we rely on later invocations of fix_format.sh to take diff --git a/tools/api_boost/api_boost_test.py b/tools/api_boost/api_boost_test.py index cf8745691d99..f1c5f2a01620 100755 --- a/tools/api_boost/api_boost_test.py +++ b/tools/api_boost/api_boost_test.py @@ -26,6 +26,7 @@ ('using_decl', 'UsingDecl upgrades for named types'), ('rename', 'Annotation-based renaming'), ('decl_ref_expr', 'DeclRefExpr upgrades for named constants'), + ('no_boost_file', 'API_NO_BOOST_FILE annotations'), ])) TESTDATA_PATH = 'tools/api_boost/testdata' diff --git a/tools/api_boost/testdata/BUILD b/tools/api_boost/testdata/BUILD index d397b35b7bdd..d9b47fd0e233 100644 --- a/tools/api_boost/testdata/BUILD +++ b/tools/api_boost/testdata/BUILD @@ -32,6 +32,12 @@ envoy_cc_library( deps = ["@envoy_api//envoy/api/v2/route:pkg_cc_proto"], ) +envoy_cc_library( + name = "no_boost_file", + srcs = ["no_boost_file.cc"], + deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"], +) + envoy_cc_library( name = "using_decl", srcs = ["using_decl.cc"], diff --git a/tools/api_boost/testdata/decl_ref_expr.cc b/tools/api_boost/testdata/decl_ref_expr.cc index 0d0406b1d1b4..0acee6a2bc01 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc +++ b/tools/api_boost/testdata/decl_ref_expr.cc @@ -1,6 +1,8 @@ #include "envoy/api/v2/cds.pb.h" #include "envoy/config/overload/v2alpha/overload.pb.h" +#define API_NO_BOOST(x) x +#define BAR(x) x #define ASSERT(x) static_cast(x) using envoy::config::overload::v2alpha::Trigger; @@ -20,6 +22,9 @@ class ThresholdTriggerImpl { default: break; } + API_NO_BOOST(envoy::config::overload::v2alpha::Trigger) foo; + BAR(API_NO_BOOST(envoy::config::overload::v2alpha::Trigger)) bar; + BAR(envoy::config::overload::v2alpha::Trigger) baz; envoy::config::overload::v2alpha::ThresholdTrigger::default_instance(); ASSERT(envoy::config::overload::v2alpha::Trigger::kThreshold == Trigger::kThreshold); envoy::api::v2::Cluster_LbPolicy_Name(0); diff --git a/tools/api_boost/testdata/decl_ref_expr.cc.gold b/tools/api_boost/testdata/decl_ref_expr.cc.gold index b5fee4d8be8f..5b363fe9f770 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc.gold +++ b/tools/api_boost/testdata/decl_ref_expr.cc.gold @@ -1,6 +1,9 @@ #include "envoy/api/v3alpha/cds.pb.h" +#include "envoy/config/overload/v2alpha/overload.pb.h" #include "envoy/config/overload/v3alpha/overload.pb.h" +#define API_NO_BOOST(x) x +#define BAR(x) x #define ASSERT(x) static_cast(x) using envoy::config::overload::v3alpha::Trigger; @@ -20,6 +23,9 @@ public: default: break; } + API_NO_BOOST(envoy::config::overload::v2alpha::Trigger) foo; + BAR(API_NO_BOOST(envoy::config::overload::v2alpha::Trigger)) bar; + BAR(envoy::config::overload::v3alpha::Trigger) baz; envoy::config::overload::v3alpha::ThresholdTrigger::default_instance(); ASSERT(envoy::config::overload::v3alpha::Trigger::kThreshold == Trigger::kThreshold); envoy::api::v3alpha::Cluster_LbPolicy_Name(0); diff --git a/tools/api_boost/testdata/no_boost_file.cc b/tools/api_boost/testdata/no_boost_file.cc new file mode 100644 index 000000000000..82d11a26410b --- /dev/null +++ b/tools/api_boost/testdata/no_boost_file.cc @@ -0,0 +1,12 @@ +#include "envoy/config/overload/v2alpha/overload.pb.h" + +// API_NO_BOOST_FILE + +using envoy::config::overload::v2alpha::ThresholdTrigger; +using SomePtrAlias = std::unique_ptr; + +class ThresholdTriggerImpl { +public: + ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {} + ThresholdTriggerImpl(SomePtrAlias /*config*/) {} +}; diff --git a/tools/api_boost/testdata/no_boost_file.cc.gold b/tools/api_boost/testdata/no_boost_file.cc.gold new file mode 100644 index 000000000000..82d11a26410b --- /dev/null +++ b/tools/api_boost/testdata/no_boost_file.cc.gold @@ -0,0 +1,12 @@ +#include "envoy/config/overload/v2alpha/overload.pb.h" + +// API_NO_BOOST_FILE + +using envoy::config::overload::v2alpha::ThresholdTrigger; +using SomePtrAlias = std::unique_ptr; + +class ThresholdTriggerImpl { +public: + ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {} + ThresholdTriggerImpl(SomePtrAlias /*config*/) {} +}; diff --git a/tools/clang_tools/api_booster/main.cc b/tools/clang_tools/api_booster/main.cc index ddd22a167f0a..62e4dae00aa7 100644 --- a/tools/clang_tools/api_booster/main.cc +++ b/tools/clang_tools/api_booster/main.cc @@ -220,7 +220,7 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const clang::SourceManager& source_manager) { const std::string type_name = member_call_expr.getObjectType().getCanonicalType().getUnqualifiedType().getAsString(); - const auto latest_type_info = getLatestTypeInformationFromCType(type_name); + const auto latest_type_info = getTypeInformationFromCType(type_name, true); // If this isn't a known API type, our work here is done. if (!latest_type_info) { return; @@ -265,7 +265,7 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const clang::SourceManager& source_manager, absl::string_view debug_description, bool requires_enum_truncation, bool validation_required = false) { if (source_range) { - tryBoostType(type_name, source_manager.getSpellingLoc(source_range->getBegin()), + tryBoostType(type_name, source_range->getBegin(), sourceRangeLength(*source_range, source_manager), source_manager, debug_description, requires_enum_truncation, validation_required); } else { @@ -277,32 +277,41 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, void tryBoostType(const std::string& type_name, clang::SourceLocation begin_loc, int length, const clang::SourceManager& source_manager, absl::string_view debug_description, bool requires_enum_truncation, bool validation_required = false) { - const auto latest_type_info = getLatestTypeInformationFromCType(type_name); + bool is_skip_macro = false; + if (begin_loc.isMacroID()) { + DEBUG_LOG("macro"); + auto macro_name = clang::Lexer::getImmediateMacroName(begin_loc, source_manager, lexer_lopt_); + if (macro_name.str() == "API_NO_BOOST") { + DEBUG_LOG("Skipping replacement due to API_NO_BOOST"); + is_skip_macro = true; + } + } + const auto type_info = getTypeInformationFromCType(type_name, !is_skip_macro); // If this isn't a known API type, our work here is done. - if (!latest_type_info) { + if (!type_info) { return; } DEBUG_LOG(absl::StrCat("Matched type '", type_name, "' (", debug_description, ") length ", length, " at ", begin_loc.printToString(source_manager))); // Track corresponding imports. - source_api_proto_paths_.insert(adjustProtoSuffix(latest_type_info->proto_path_, ".pb.h")); + source_api_proto_paths_.insert(adjustProtoSuffix(type_info->proto_path_, ".pb.h")); if (validation_required) { - source_api_proto_paths_.insert( - adjustProtoSuffix(latest_type_info->proto_path_, ".pb.validate.h")); + source_api_proto_paths_.insert(adjustProtoSuffix(type_info->proto_path_, ".pb.validate.h")); } // Not all AST matchers know how to do replacements (yet?). - if (length == -1) { + if (length == -1 || is_skip_macro) { return; } + const clang::SourceLocation spelling_begin = source_manager.getSpellingLoc(begin_loc); // We need to look at the text we're replacing to decide whether we should // use the qualified C++'ified proto name. const bool qualified = - getSourceText(begin_loc, length, source_manager).find("::") != std::string::npos; + getSourceText(spelling_begin, length, source_manager).find("::") != std::string::npos; // Add corresponding replacement. const clang::tooling::Replacement type_replacement( - source_manager, begin_loc, length, - ProtoCxxUtils::protoToCxxType(latest_type_info->type_name_, qualified, - latest_type_info->enum_type_ && requires_enum_truncation)); + source_manager, spelling_begin, length, + ProtoCxxUtils::protoToCxxType(type_info->type_name_, qualified, + type_info->enum_type_ && requires_enum_truncation)); insertReplacement(type_replacement); } @@ -355,8 +364,8 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, // Obtain the latest type information for a given from C++ type, e.g. envoy:config::v2::Cluster, // from the API type database. - absl::optional - getLatestTypeInformationFromCType(const std::string& c_type_name) { + absl::optional getTypeInformationFromCType(const std::string& c_type_name, + bool latest) { // Ignore compound or non-API types. // TODO(htuch): this is all super hacky and not really right, we should be // removing qualifiers etc. to get to the underlying type name. @@ -367,7 +376,8 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const std::string proto_type_name = ProtoCxxUtils::cxxToProtoType(type_name); // Use API type database to map from proto type to path. - auto result = ApiTypeDb::getLatestTypeInformation(proto_type_name); + auto result = latest ? ApiTypeDb::getLatestTypeInformation(proto_type_name) + : ApiTypeDb::getExistingTypeInformation(proto_type_name); if (result) { // Remove the .proto extension. return result; diff --git a/tools/type_whisperer/api_type_db.cc b/tools/type_whisperer/api_type_db.cc index ceb6f6030033..fbb5b843c079 100644 --- a/tools/type_whisperer/api_type_db.cc +++ b/tools/type_whisperer/api_type_db.cc @@ -49,6 +49,15 @@ const tools::type_whisperer::TypeDb& getApiTypeDb() { } // namespace +absl::optional +ApiTypeDb::getExistingTypeInformation(const std::string& type_name) { + auto it = getApiTypeDb().types().find(type_name); + if (it == getApiTypeDb().types().end()) { + return {}; + } + return absl::make_optional(type_name, it->second.proto_path(), false); +} + absl::optional ApiTypeDb::getLatestTypeInformation(const std::string& type_name) { std::string latest_type_name; const tools::type_whisperer::TypeDbDescription* latest_type_desc{}; diff --git a/tools/type_whisperer/api_type_db.h b/tools/type_whisperer/api_type_db.h index e3c58f7577e6..5a21b4ffe040 100644 --- a/tools/type_whisperer/api_type_db.h +++ b/tools/type_whisperer/api_type_db.h @@ -31,6 +31,7 @@ struct TypeInformation { // libtool binaries). class ApiTypeDb { public: + static absl::optional getExistingTypeInformation(const std::string& type_name); static absl::optional getLatestTypeInformation(const std::string& type_name); };