From 9bbc159748761b041a1eacc8f7515a3074e62e62 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 22 Aug 2018 13:24:20 -0400 Subject: [PATCH 1/7] proto: unify envoy_proto_library/api_proto_library. In the latest iteration of https://github.com/envoyproxy/envoy/pull/4220, it was necessary to use PGV constraints on fuzzer inputs. To do this would require PGV generation in envoy_build_system.bzl. There is also quite a bit of mess in how we were doing envoy_proto_library() today. So, this PR allows us to throw away the custom envoy_proto_library() and benefit from leveraging a single source of Envoy proto build truth. Risk level: Low Testing: bazel test //test/... Signed-off-by: Harvey Tuch --- api/bazel/api_build_system.bzl | 12 +++++ bazel/envoy_build_system.bzl | 52 ++----------------- source/common/ratelimit/BUILD | 2 +- test/common/access_log/BUILD | 3 +- test/common/grpc/BUILD | 10 ++-- test/common/http/BUILD | 9 ++-- .../http/conn_manager_impl_corpus/example | 2 +- test/common/http/conn_manager_impl_fuzz.proto | 2 +- .../http/conn_manager_impl_fuzz_test.cc | 4 +- test/common/http/http2/BUILD | 4 +- .../http/http2/codec_impl_corpus/100-continue | 2 +- .../http/http2/codec_impl_corpus/example | 4 +- test/common/http/http2/codec_impl_fuzz.proto | 4 +- .../common/http/http2/codec_impl_fuzz_test.cc | 9 ++-- test/common/router/BUILD | 12 ++--- test/common/router/header_parser_fuzz_test.cc | 3 +- .../filters/http/grpc_json_transcoder/BUILD | 4 +- test/fuzz/BUILD | 5 +- test/integration/BUILD | 6 +-- test/proto/BUILD | 5 -- 20 files changed, 54 insertions(+), 100 deletions(-) diff --git a/api/bazel/api_build_system.bzl b/api/bazel/api_build_system.bzl index 497d82c5ccc07..820e5d0bfe277 100644 --- a/api/bazel/api_build_system.bzl +++ b/api/bazel/api_build_system.bzl @@ -126,14 +126,26 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de deps = [_LibrarySuffix(d, _CC_SUFFIX) for d in deps], external_deps = [ "@com_google_protobuf//:cc_wkt_protos", + "@googleapis//:api_httpbody_protos", "@googleapis//:http_api_protos", "@googleapis//:rpc_status_protos", "@com_github_gogo_protobuf//:gogo_proto_cc", ], visibility = ["//visibility:public"], ) + py_export_suffixes = [] if (require_py == 1): api_py_proto_library(name, srcs, deps, has_services) + py_export_suffixes = ["_py", "_py_genproto"] + + # Allow unlimited visibility for consumers + export_suffixes = ["", "_cc", "_cc_validate", "_cc_proto", "_cc_proto_genproto"] + py_export_suffixes + for s in export_suffixes: + native.alias( + name = name + "_export" + s, + actual = name + s, + visibility = ["//visibility:public"], + ) def api_cc_test(name, srcs, proto_deps): native.cc_test( diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index e66f38be1470f..293cc67798a15 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -1,4 +1,6 @@ load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library") +load("@com_lyft_protoc_gen_validate//bazel:pgv_proto_library.bzl", "pgv_cc_proto_library") +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library") def envoy_package(): native.package(default_visibility = ["//visibility:public"]) @@ -398,54 +400,8 @@ def _proto_header(proto_path): return None # Envoy proto targets should be specified with this function. -def envoy_proto_library( - name, - srcs = [], - deps = [], - external_deps = [], - generate_python = True): - # Ideally this would be native.{proto_library, cc_proto_library}. - # Unfortunately, this doesn't work with http_api_protos due to the PGV - # requirement to also use them in the non-native protobuf.bzl - # cc_proto_library; you end up with the same file built twice. So, also - # using protobuf.bzl cc_proto_library here. - cc_proto_deps = [] - py_proto_deps = ["@com_google_protobuf//:protobuf_python"] - - if "api_httpbody_protos" in external_deps: - cc_proto_deps.append("@googleapis//:api_httpbody_protos") - py_proto_deps.append("@googleapis//:api_httpbody_protos_py") - - if "http_api_protos" in external_deps: - cc_proto_deps.append("@googleapis//:http_api_protos") - py_proto_deps.append("@googleapis//:http_api_protos_py") - - if "well_known_protos" in external_deps: - # WKT is already included for Python as part of standard deps above. - cc_proto_deps.append("@com_google_protobuf//:cc_wkt_protos") - - cc_proto_library( - name = name, - srcs = srcs, - default_runtime = "@com_google_protobuf//:protobuf", - protoc = "@com_google_protobuf//:protoc", - deps = deps + cc_proto_deps, - # Avoid generating .so, we don't need it, can interfere with builds - # such as OSS-Fuzz. - linkstatic = 1, - alwayslink = 1, - visibility = ["//visibility:public"], - ) - - if generate_python: - py_proto_library( - name = name + "_py", - srcs = srcs, - default_runtime = "@com_google_protobuf//:protobuf_python", - protoc = "@com_google_protobuf//:protoc", - deps = deps + py_proto_deps, - visibility = ["//visibility:public"], - ) +def envoy_proto_library(name, **kwargs): + return api_proto_library(name, visibility = ["//visibility:public"], **kwargs) # Envoy proto descriptor targets should be specified with this function. # This is used for testing only. diff --git a/source/common/ratelimit/BUILD b/source/common/ratelimit/BUILD index 60ea694e3a06f..2038bf255ce77 100644 --- a/source/common/ratelimit/BUILD +++ b/source/common/ratelimit/BUILD @@ -14,7 +14,7 @@ envoy_cc_library( srcs = ["ratelimit_impl.cc"], hdrs = ["ratelimit_impl.h"], deps = [ - ":ratelimit_proto", + ":ratelimit_proto_cc", "//include/envoy/grpc:async_client_interface", "//include/envoy/grpc:async_client_manager_interface", "//include/envoy/ratelimit:ratelimit_interface", diff --git a/test/common/access_log/BUILD b/test/common/access_log/BUILD index 0ef1332e41f0a..4f9d2c1d621b9 100644 --- a/test/common/access_log/BUILD +++ b/test/common/access_log/BUILD @@ -24,7 +24,6 @@ envoy_cc_library( envoy_proto_library( name = "access_log_formatter_fuzz_proto", srcs = ["access_log_formatter_fuzz.proto"], - generate_python = 0, deps = ["//test/fuzz:common_proto"], ) @@ -33,7 +32,7 @@ envoy_cc_fuzz_test( srcs = ["access_log_formatter_fuzz_test.cc"], corpus = "access_log_formatter_corpus", deps = [ - ":access_log_formatter_fuzz_proto", + ":access_log_formatter_fuzz_proto_cc", "//source/common/access_log:access_log_formatter_lib", "//test/fuzz:utility_lib", ], diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index 3294b133c88aa..5d05415e4b9ed 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -18,7 +18,7 @@ envoy_cc_test( "//test/mocks/http:http_mocks", "//test/mocks/tracing:tracing_mocks", "//test/mocks/upstream:upstream_mocks", - "//test/proto:helloworld_proto", + "//test/proto:helloworld_proto_cc", ], ) @@ -40,7 +40,7 @@ envoy_cc_test( deps = [ "//source/common/buffer:buffer_lib", "//source/common/grpc:codec_lib", - "//test/proto:helloworld_proto", + "//test/proto:helloworld_proto_cc", ], ) @@ -51,7 +51,7 @@ envoy_cc_test( "//source/common/grpc:common_lib", "//source/common/http:headers_lib", "//test/mocks/upstream:upstream_mocks", - "//test/proto:helloworld_proto", + "//test/proto:helloworld_proto_cc", "//test/test_common:utility_lib", ], ) @@ -66,7 +66,7 @@ envoy_cc_test( "//source/common/tracing:http_tracer_lib", "//test/mocks/grpc:grpc_mocks", "//test/mocks/tracing:tracing_mocks", - "//test/proto:helloworld_proto", + "//test/proto:helloworld_proto_cc", ] + envoy_select_google_grpc(["//source/common/grpc:google_async_client_lib"]), ) @@ -98,7 +98,7 @@ envoy_cc_test_library( "//source/common/http/http2:conn_pool_lib", "//test/integration:integration_lib", "//test/mocks/local_info:local_info_mocks", - "//test/proto:helloworld_proto", + "//test/proto:helloworld_proto_cc", ], ) diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 13c84e925b469..580f3c675bb80 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -94,8 +94,6 @@ envoy_cc_test_library( envoy_proto_library( name = "conn_manager_impl_fuzz_proto", srcs = ["conn_manager_impl_fuzz.proto"], - external_deps = ["well_known_protos"], - generate_python = 0, deps = [ "//test/fuzz:common_proto", ], @@ -106,7 +104,7 @@ envoy_cc_fuzz_test( srcs = ["conn_manager_impl_fuzz_test.cc"], corpus = "conn_manager_impl_corpus", deps = [ - ":conn_manager_impl_fuzz_proto", + ":conn_manager_impl_fuzz_proto_cc", "//source/common/common:empty_string", "//source/common/http:conn_manager_lib", "//source/common/http:date_provider_lib", @@ -207,7 +205,6 @@ envoy_cc_test( envoy_proto_library( name = "header_map_impl_fuzz_proto", srcs = ["header_map_impl_fuzz.proto"], - external_deps = ["well_known_protos"], ) envoy_cc_fuzz_test( @@ -215,7 +212,7 @@ envoy_cc_fuzz_test( srcs = ["header_map_impl_fuzz_test.cc"], corpus = "header_map_impl_corpus", deps = [ - ":header_map_impl_fuzz_proto", + ":header_map_impl_fuzz_proto_cc", "//source/common/http:header_map_lib", ], ) @@ -250,7 +247,7 @@ envoy_cc_fuzz_test( srcs = ["utility_fuzz_test.cc"], corpus = "utility_corpus", deps = [ - ":utility_fuzz_proto", + ":utility_fuzz_proto_cc", "//source/common/http:utility_lib", "//test/test_common:utility_lib", ], diff --git a/test/common/http/conn_manager_impl_corpus/example b/test/common/http/conn_manager_impl_corpus/example index 4d19ae6d11b37..9a12212cfdfc9 100644 --- a/test/common/http/conn_manager_impl_corpus/example +++ b/test/common/http/conn_manager_impl_corpus/example @@ -60,7 +60,7 @@ actions { stream_action { stream_id: 0 response { - continue_100_headers {} + continue_headers {} } } } diff --git a/test/common/http/conn_manager_impl_fuzz.proto b/test/common/http/conn_manager_impl_fuzz.proto index c781e2ec34dec..a6a3617d0165e 100644 --- a/test/common/http/conn_manager_impl_fuzz.proto +++ b/test/common/http/conn_manager_impl_fuzz.proto @@ -66,7 +66,7 @@ message RequestAction { // TODO(htuch): Model and fuzz encoder filter buffering/resumption and different status returns. message ResponseAction { oneof response_action_selector { - test.fuzz.Headers continue_100_headers = 1; + test.fuzz.Headers continue_headers = 1; test.fuzz.Headers headers = 2; uint32 data = 3; test.fuzz.Headers trailers = 4; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 094d3b04fade8..c7711ac51a1fa 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -286,10 +286,10 @@ class FuzzStream { const test::common::http::ResponseAction& response_action) { const bool end_stream = response_action.end_stream(); switch (response_action.response_action_selector_case()) { - case test::common::http::ResponseAction::kContinue100Headers: { + case test::common::http::ResponseAction::kContinueHeaders: { if (state == StreamState::PendingHeaders) { auto headers = std::make_unique( - Fuzz::fromHeaders(response_action.continue_100_headers())); + Fuzz::fromHeaders(response_action.continue_headers())); headers->setReferenceKey(Headers::get().Status, "100"); decoder_filter_->callbacks_->encode100ContinueHeaders(std::move(headers)); } diff --git a/test/common/http/http2/BUILD b/test/common/http/http2/BUILD index b9f7908934b73..7d26121eeedcf 100644 --- a/test/common/http/http2/BUILD +++ b/test/common/http/http2/BUILD @@ -13,8 +13,6 @@ envoy_package() envoy_proto_library( name = "codec_impl_fuzz_proto", srcs = ["codec_impl_fuzz.proto"], - external_deps = ["well_known_protos"], - generate_python = 0, deps = ["//test/fuzz:common_proto"], ) @@ -23,7 +21,7 @@ envoy_cc_fuzz_test( srcs = ["codec_impl_fuzz_test.cc"], corpus = "codec_impl_corpus", deps = [ - ":codec_impl_fuzz_proto", + ":codec_impl_fuzz_proto_cc", "//source/common/http:header_map_lib", "//source/common/http/http2:codec_lib", "//test/fuzz:utility_lib", diff --git a/test/common/http/http2/codec_impl_corpus/100-continue b/test/common/http/http2/codec_impl_corpus/100-continue index 28a1ec9e46ffc..48b2c8a3d91f6 100644 --- a/test/common/http/http2/codec_impl_corpus/100-continue +++ b/test/common/http/http2/codec_impl_corpus/100-continue @@ -29,7 +29,7 @@ actions { stream_action { stream_id: 0 response { - continue_100_headers { + continue_headers { headers { key: ":status" value: "100" diff --git a/test/common/http/http2/codec_impl_corpus/example b/test/common/http/http2/codec_impl_corpus/example index 8ea4e2eec6641..65a0fb0d12f09 100644 --- a/test/common/http/http2/codec_impl_corpus/example +++ b/test/common/http/http2/codec_impl_corpus/example @@ -221,7 +221,7 @@ actions { stream_action { stream_id: 3 request { - reset: 0 + reset_stream: 0 } } } @@ -252,7 +252,7 @@ actions { stream_action { stream_id: 4 response { - reset: 0 + reset_stream: 0 } } } diff --git a/test/common/http/http2/codec_impl_fuzz.proto b/test/common/http/http2/codec_impl_fuzz.proto index 62e85b34c5b1b..9b7a3a9a8387a 100644 --- a/test/common/http/http2/codec_impl_fuzz.proto +++ b/test/common/http/http2/codec_impl_fuzz.proto @@ -15,11 +15,11 @@ message NewStream { message DirectionalAction { oneof directional_action_selector { - test.fuzz.Headers continue_100_headers = 1; + test.fuzz.Headers continue_headers = 1; test.fuzz.Headers headers = 2; uint32 data = 3; test.fuzz.Headers trailers = 4; - uint32 reset = 5; + uint32 reset_stream = 5; bool read_disable = 6; } bool end_stream = 7; diff --git a/test/common/http/http2/codec_impl_fuzz_test.cc b/test/common/http/http2/codec_impl_fuzz_test.cc index 0e0b6f9952dc6..e3ac0a28dd52c 100644 --- a/test/common/http/http2/codec_impl_fuzz_test.cc +++ b/test/common/http/http2/codec_impl_fuzz_test.cc @@ -99,10 +99,9 @@ class Stream : public LinkedObject { const test::common::http::http2::DirectionalAction& directional_action) { const bool end_stream = directional_action.end_stream(); switch (directional_action.directional_action_selector_case()) { - case test::common::http::http2::DirectionalAction::kContinue100Headers: { + case test::common::http::http2::DirectionalAction::kContinueHeaders: { if (state == StreamState::PendingHeaders) { - Http::TestHeaderMapImpl headers = - Fuzz::fromHeaders(directional_action.continue_100_headers()); + Http::TestHeaderMapImpl headers = Fuzz::fromHeaders(directional_action.continue_headers()); headers.setReferenceKey(Headers::get().Status, "100"); encoder.encode100ContinueHeaders(headers); } @@ -130,10 +129,10 @@ class Stream : public LinkedObject { } break; } - case test::common::http::http2::DirectionalAction::kReset: { + case test::common::http::http2::DirectionalAction::kResetStream: { if (state != StreamState::Closed) { encoder.getStream().resetStream( - static_cast(directional_action.reset())); + static_cast(directional_action.reset_stream())); request_state_ = response_state_ = StreamState::Closed; } break; diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 5207bb84845b6..b7acdb092b4b7 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -14,7 +14,7 @@ envoy_cc_test( name = "config_impl_test", srcs = ["config_impl_test.cc"], deps = [ - ":route_fuzz_proto", + ":route_fuzz_proto_cc", "//source/common/config:metadata_lib", "//source/common/config:rds_json_lib", "//source/common/http:header_map_lib", @@ -33,10 +33,9 @@ envoy_cc_test( envoy_proto_library( name = "header_parser_fuzz_proto", srcs = ["header_parser_fuzz.proto"], - generate_python = 0, deps = [ "//test/fuzz:common_proto", - "@envoy_api//envoy/api/v2/core:base_cc_proto", + "@envoy_api//envoy/api/v2/core:base_export", ], ) @@ -45,7 +44,7 @@ envoy_cc_fuzz_test( srcs = ["header_parser_fuzz_test.cc"], corpus = "header_parser_corpus", deps = [ - ":header_parser_fuzz_proto", + ":header_parser_fuzz_proto_cc", "//source/common/http:header_map_lib", "//source/common/router:header_parser_lib", "//test/fuzz:utility_lib", @@ -88,10 +87,9 @@ envoy_cc_test( envoy_proto_library( name = "route_fuzz_proto", srcs = ["route_fuzz.proto"], - generate_python = 0, deps = [ "//test/fuzz:common_proto", - "@envoy_api//envoy/api/v2:rds_cc_proto", + "@envoy_api//envoy/api/v2:rds_export", ], ) @@ -100,7 +98,7 @@ envoy_cc_fuzz_test( srcs = ["route_fuzz_test.cc"], corpus = "route_corpus", deps = [ - ":route_fuzz_proto", + ":route_fuzz_proto_cc", "//source/common/router:config_lib", "//test/fuzz:utility_lib", "//test/mocks/server:server_mocks", diff --git a/test/common/router/header_parser_fuzz_test.cc b/test/common/router/header_parser_fuzz_test.cc index 73bcfbb66a179..ed28befac9ffd 100644 --- a/test/common/router/header_parser_fuzz_test.cc +++ b/test/common/router/header_parser_fuzz_test.cc @@ -1,7 +1,7 @@ #include "common/http/header_map_impl.h" #include "common/router/header_parser.h" -#include "test/common/router/header_parser_fuzz.pb.h" +#include "test/common/router/header_parser_fuzz.pb.validate.h" #include "test/fuzz/fuzz_runner.h" #include "test/fuzz/utility.h" @@ -10,6 +10,7 @@ namespace Fuzz { DEFINE_PROTO_FUZZER(const test::common::router::TestCase& input) { try { + MessageUtil::validate(input); Router::HeaderParserPtr parser = Router::HeaderParser::configure(input.headers_to_add(), input.headers_to_remove()); Http::HeaderMapImpl header_map; diff --git a/test/extensions/filters/http/grpc_json_transcoder/BUILD b/test/extensions/filters/http/grpc_json_transcoder/BUILD index f0744a54b7126..65c913db317a7 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/BUILD +++ b/test/extensions/filters/http/grpc_json_transcoder/BUILD @@ -24,7 +24,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/grpc_json_transcoder:json_transcoder_filter_lib", "//test/mocks/http:http_mocks", "//test/mocks/upstream:upstream_mocks", - "//test/proto:bookstore_proto", + "//test/proto:bookstore_proto_cc", "//test/test_common:environment_lib", "//test/test_common:utility_lib", ], @@ -54,7 +54,7 @@ envoy_extension_cc_test( "//source/common/http:header_map_lib", "//source/extensions/filters/http/grpc_json_transcoder:config", "//test/integration:http_integration_lib", - "//test/proto:bookstore_proto", + "//test/proto:bookstore_proto_cc", "//test/test_common:utility_lib", ], ) diff --git a/test/fuzz/BUILD b/test/fuzz/BUILD index df9e26e901cb5..25192eddd07d0 100644 --- a/test/fuzz/BUILD +++ b/test/fuzz/BUILD @@ -12,8 +12,7 @@ envoy_package() envoy_proto_library( name = "common_proto", srcs = ["common.proto"], - generate_python = 0, - deps = ["@envoy_api//envoy/api/v2/core:base_cc_proto"], + deps = ["@envoy_api//envoy/api/v2/core:base_export"], ) envoy_cc_test_library( @@ -47,7 +46,7 @@ envoy_cc_test_library( name = "utility_lib", hdrs = ["utility.h"], deps = [ - ":common_proto", + ":common_proto_cc", "//source/common/network:utility_lib", "//test/common/access_log:test_util", "//test/mocks/upstream:upstream_mocks", diff --git a/test/integration/BUILD b/test/integration/BUILD index 25534d61eec20..deecb9a761285 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -60,7 +60,7 @@ py_binary( envoy_proto_library( name = "capture_fuzz_proto", srcs = [":capture_fuzz.proto"], - external_deps = ["well_known_protos"], + require_py = 1, ) envoy_cc_test( @@ -423,7 +423,7 @@ envoy_cc_test( "//source/common/buffer:zero_copy_input_stream_lib", "//source/common/grpc:codec_lib", "//source/common/grpc:common_lib", - "//source/common/ratelimit:ratelimit_proto", + "//source/common/ratelimit:ratelimit_proto_cc", "//source/extensions/filters/http/ratelimit:config", "//test/common/grpc:grpc_client_integration_lib", "@envoy_api//envoy/service/ratelimit/v2:rls_cc", @@ -581,7 +581,7 @@ envoy_cc_test_library( "h1_fuzz.h", ], deps = [ - ":capture_fuzz_proto", + ":capture_fuzz_proto_cc", ":http_integration_lib", "//source/common/common:assert_lib", "//source/common/common:logger_lib", diff --git a/test/proto/BUILD b/test/proto/BUILD index fa16742517196..4ffdbbf968887 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -19,11 +19,6 @@ envoy_proto_library( envoy_proto_library( name = "bookstore_proto", srcs = [":bookstore.proto"], - external_deps = [ - "api_httpbody_protos", - "http_api_protos", - "well_known_protos", - ], ) envoy_proto_descriptor( From 3c48acfa24c2108b1e84c049b1732cca80a02352 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 22 Aug 2018 17:41:05 -0400 Subject: [PATCH 2/7] Remove unused line from dev. Signed-off-by: Harvey Tuch --- bazel/envoy_build_system.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index 293cc67798a15..793da9fb489ec 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -1,5 +1,4 @@ load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library") -load("@com_lyft_protoc_gen_validate//bazel:pgv_proto_library.bzl", "pgv_cc_proto_library") load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library") def envoy_package(): From 0d4384b4978a03b3d232ac2fec63629ed2ca9b8d Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 23 Aug 2018 23:54:42 -0400 Subject: [PATCH 3/7] Make api_httpbody_protos optional, should workaround OS X issue. Signed-off-by: Harvey Tuch --- api/bazel/api_build_system.bzl | 16 +++++++++++----- test/proto/BUILD | 2 ++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/api/bazel/api_build_system.bzl b/api/bazel/api_build_system.bzl index 820e5d0bfe277..6c1af86a1d199 100644 --- a/api/bazel/api_build_system.bzl +++ b/api/bazel/api_build_system.bzl @@ -88,7 +88,15 @@ def api_proto_library_internal(visibility = ["//visibility:private"], **kwargs): # gRPC stub generation. # TODO(htuch): Automatically generate go_proto_library and go_grpc_library # from api_proto_library. -def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], deps = [], has_services = 0, require_py = 1): +def api_proto_library( + name, + visibility = ["//visibility:private"], + srcs = [], + deps = [], + external_proto_deps = [], + external_cc_proto_deps = [], + has_services = 0, + require_py = 1): # This is now vestigial, since there are no direct consumers in # the data plane API. However, we want to maintain native proto_library support # in the proto graph to (1) support future C++ use of native rules with @@ -99,7 +107,7 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de native.proto_library( name = name, srcs = srcs, - deps = deps + [ + deps = deps + external_proto_deps + [ "@com_google_protobuf//:any_proto", "@com_google_protobuf//:descriptor_proto", "@com_google_protobuf//:duration_proto", @@ -107,7 +115,6 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de "@com_google_protobuf//:struct_proto", "@com_google_protobuf//:timestamp_proto", "@com_google_protobuf//:wrappers_proto", - "@googleapis//:api_httpbody_protos_proto", "@googleapis//:http_api_protos_proto", "@googleapis//:rpc_status_protos_lib", "@com_github_gogo_protobuf//:gogo_proto", @@ -124,9 +131,8 @@ def api_proto_library(name, visibility = ["//visibility:private"], srcs = [], de name = _Suffix(name, _CC_SUFFIX), srcs = srcs, deps = [_LibrarySuffix(d, _CC_SUFFIX) for d in deps], - external_deps = [ + external_deps = external_cc_proto_deps + [ "@com_google_protobuf//:cc_wkt_protos", - "@googleapis//:api_httpbody_protos", "@googleapis//:http_api_protos", "@googleapis//:rpc_status_protos", "@com_github_gogo_protobuf//:gogo_proto_cc", diff --git a/test/proto/BUILD b/test/proto/BUILD index 4ffdbbf968887..cce7d5f28225a 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -19,6 +19,8 @@ envoy_proto_library( envoy_proto_library( name = "bookstore_proto", srcs = [":bookstore.proto"], + external_cc_proto_deps = ["@googleapis//:api_httpbody_protos"], + external_proto_deps = ["@googleapis//:api_httpbody_protos_proto"], ) envoy_proto_descriptor( From 5515ef1b04fcff945aba4b831ffafd6de847e136 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 24 Aug 2018 07:44:07 -0400 Subject: [PATCH 4/7] .bzl feedback. Signed-off-by: Harvey Tuch --- bazel/envoy_build_system.bzl | 15 +++++++++++++-- test/proto/BUILD | 3 +-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index 793da9fb489ec..e07f8ac94209a 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -399,8 +399,19 @@ def _proto_header(proto_path): return None # Envoy proto targets should be specified with this function. -def envoy_proto_library(name, **kwargs): - return api_proto_library(name, visibility = ["//visibility:public"], **kwargs) +def envoy_proto_library(name, external_deps = [], **kwargs): + external_proto_deps = [] + external_cc_proto_deps = [] + if "api_httpbody_protos" in external_deps: + external_cc_proto_deps.append("@googleapis//:api_httpbody_protos") + external_proto_deps.append("@googleapis//:api_httpbody_protos_proto") + return api_proto_library( + name, + external_cc_proto_deps = external_cc_proto_deps, + external_proto_deps = external_proto_deps, + visibility = ["//visibility:public"], + **kwargs + ) # Envoy proto descriptor targets should be specified with this function. # This is used for testing only. diff --git a/test/proto/BUILD b/test/proto/BUILD index cce7d5f28225a..3e339d903b55f 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -19,8 +19,7 @@ envoy_proto_library( envoy_proto_library( name = "bookstore_proto", srcs = [":bookstore.proto"], - external_cc_proto_deps = ["@googleapis//:api_httpbody_protos"], - external_proto_deps = ["@googleapis//:api_httpbody_protos_proto"], + external_deps = ["api_httpbody_protos"], ) envoy_proto_descriptor( From f440098f7a6678819498741aca1c41885164bea9 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 24 Aug 2018 08:40:39 -0400 Subject: [PATCH 5/7] Fix ratelimit test legacy proto deps. Signed-off-by: Harvey Tuch --- api/bazel/api_build_system.bzl | 3 +++ test/common/ratelimit/ratelimit_impl_test.cc | 2 ++ test/integration/ratelimit_integration_test.cc | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/api/bazel/api_build_system.bzl b/api/bazel/api_build_system.bzl index 6c1af86a1d199..446864086f170 100644 --- a/api/bazel/api_build_system.bzl +++ b/api/bazel/api_build_system.bzl @@ -130,6 +130,9 @@ def api_proto_library( pgv_cc_proto_library( name = _Suffix(name, _CC_SUFFIX), srcs = srcs, + # Avoid generating .so, we don't need it, can interfere with builds + # such as OSS-Fuzz. + linkstatic = 1, deps = [_LibrarySuffix(d, _CC_SUFFIX) for d in deps], external_deps = external_cc_proto_deps + [ "@com_google_protobuf//:cc_wkt_protos", diff --git a/test/common/ratelimit/ratelimit_impl_test.cc b/test/common/ratelimit/ratelimit_impl_test.cc index 7c2fe8e9d7b13..1ff219a9dc4db 100644 --- a/test/common/ratelimit/ratelimit_impl_test.cc +++ b/test/common/ratelimit/ratelimit_impl_test.cc @@ -47,6 +47,8 @@ class RateLimitGrpcClientTest : public testing::TestWithParam { Grpc::AsyncClientPtr{async_client_}, absl::optional(), "envoy.service.ratelimit.v2.RateLimitService.ShouldRateLimit")); } else { + // Force link time dependency on deprecated message type. + pb::lyft::ratelimit::RateLimit _ignore; client_.reset(new GrpcClientImpl(Grpc::AsyncClientPtr{async_client_}, absl::optional(), "pb.lyft.ratelimit.RateLimitService.ShouldRateLimit")); diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index 67269b26a09e1..02cbbaae183e1 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -4,6 +4,8 @@ #include "common/grpc/codec.h" #include "common/grpc/common.h" +#include "source/common/ratelimit/ratelimit.pb.h" + #include "test/common/grpc/grpc_client_integration.h" #include "test/integration/http_integration.h" @@ -22,7 +24,11 @@ class RatelimitGrpcClientIntegrationParamTest ~RatelimitGrpcClientIntegrationParamTest() {} Network::Address::IpVersion ipVersion() const override { return std::get<0>(GetParam()); } Grpc::ClientType clientType() const override { return std::get<1>(GetParam()); } - bool useDataPlaneProto() const { return std::get<2>(GetParam()); } + bool useDataPlaneProto() const { + // Force link time dependency on deprecated message type. + pb::lyft::ratelimit::RateLimit _ignore; + return std::get<2>(GetParam()); + } }; class RatelimitIntegrationTest : public HttpIntegrationTest, From f848653cde081f014ee29495da83d1a73cc9a50f Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 24 Aug 2018 14:18:36 -0400 Subject: [PATCH 6/7] Fix @envoy_api//... tests. Signed-off-by: Harvey Tuch --- api/bazel/api_build_system.bzl | 5 ++--- bazel/envoy_build_system.bzl | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/api/bazel/api_build_system.bzl b/api/bazel/api_build_system.bzl index 446864086f170..af275ec53c87c 100644 --- a/api/bazel/api_build_system.bzl +++ b/api/bazel/api_build_system.bzl @@ -96,6 +96,7 @@ def api_proto_library( external_proto_deps = [], external_cc_proto_deps = [], has_services = 0, + linkstatic = None, require_py = 1): # This is now vestigial, since there are no direct consumers in # the data plane API. However, we want to maintain native proto_library support @@ -130,9 +131,7 @@ def api_proto_library( pgv_cc_proto_library( name = _Suffix(name, _CC_SUFFIX), srcs = srcs, - # Avoid generating .so, we don't need it, can interfere with builds - # such as OSS-Fuzz. - linkstatic = 1, + linkstatic = linkstatic, deps = [_LibrarySuffix(d, _CC_SUFFIX) for d in deps], external_deps = external_cc_proto_deps + [ "@com_google_protobuf//:cc_wkt_protos", diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index e07f8ac94209a..6aede4ad339fb 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -409,6 +409,9 @@ def envoy_proto_library(name, external_deps = [], **kwargs): name, external_cc_proto_deps = external_cc_proto_deps, external_proto_deps = external_proto_deps, + # Avoid generating .so, we don't need it, can interfere with builds + # such as OSS-Fuzz. + linkstatic = 1, visibility = ["//visibility:public"], **kwargs ) From f2394a1cf3a2ce916862e62968556ceeca82aeb8 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 24 Aug 2018 15:11:29 -0400 Subject: [PATCH 7/7] Fix merge. Signed-off-by: Harvey Tuch --- test/common/grpc/BUILD | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index 81e86c3bb5cb2..a448f215ef975 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -99,12 +99,8 @@ envoy_cc_test_library( "//source/common/http/http2:conn_pool_lib", "//test/integration:integration_lib", "//test/mocks/local_info:local_info_mocks", -<<<<<<< HEAD "//test/proto:helloworld_proto_cc", -======= - "//test/proto:helloworld_proto", "//test/test_common:test_time_lib", ->>>>>>> upstream/master ], )